Refactor of Allocator classes #9074

BryanMLima · 2024-05-10T20:49:48Z

Description

This PR refactors some *Allocator classes, improving modularity and code legibility. This PR also made some changes to logs across these classes.

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)
build/CI

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

How Has This Been Tested?

I tested the allocation process in my personal lab, using both the RandomAllocator and FirstFitAllocator allocators. I tried some variation of tags and offerings, and everything looks good. Furthermore, I also added a lot of unit tests for the methods that I refactored.

How did you try to break this feature and the system with this change?

codecov · 2024-05-10T20:58:21Z

Codecov Report

❌ Patch coverage is 79.49791% with 49 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.46%. Comparing base (b0c7719) to head (39a6031).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...gent/manager/allocator/impl/FirstFitAllocator.java	77.09%	24 Missing and 6 partials ⚠️
.../agent/manager/allocator/impl/RandomAllocator.java	89.09%	6 Missing ⚠️
...agent/manager/allocator/impl/TestingAllocator.java	0.00%	3 Missing ⚠️
...om/cloud/deploy/DeploymentPlanningManagerImpl.java	62.50%	2 Missing and 1 partial ⚠️
...in/java/com/cloud/server/ManagementServerImpl.java	0.00%	3 Missing ⚠️
.../src/main/java/com/cloud/host/dao/HostDaoImpl.java	0.00%	2 Missing ⚠️
...i/command/admin/host/FindHostsForMigrationCmd.java	0.00%	1 Missing ⚠️
...loudstack/api/command/admin/host/ListHostsCmd.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #9074      +/-   ##
============================================
+ Coverage     17.38%   17.46%   +0.07%     
- Complexity    15282    15384     +102     
============================================
  Files          5891     5893       +2     
  Lines        526356   526444      +88     
  Branches      64270    64240      -30     
============================================
+ Hits          91526    91942     +416     
+ Misses       424488   424155     -333     
- Partials      10342    10347       +5

Flag	Coverage Δ
uitests	`3.61% <ø> (-0.01%)`	⬇️
unittests	`18.51% <79.49%> (+0.08%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

github-actions · 2024-06-19T04:16:42Z

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

github-actions · 2024-07-12T08:04:09Z

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

server/src/main/java/com/cloud/agent/manager/allocator/impl/TestingAllocator.java

JoaoJandre

LGTM, I did some basic testing, changed the allocator to FirstFit and created some VMs, the VM allocation worked fine with and without tags. However, my tests were limited, further testing would be good.

DaanHoogland · 2024-08-26T08:11:09Z

@blueorangutan package

blueorangutan · 2024-08-26T08:12:03Z

@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2024-08-26T09:30:59Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10801

DaanHoogland · 2024-08-26T13:06:42Z

@blueorangutan LLtest

blueorangutan · 2025-08-01T14:12:41Z

Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 14508

BryanMLima · 2025-08-01T14:26:16Z

@blueorangutan package

blueorangutan · 2025-08-01T14:28:04Z

@BryanMLima a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2025-08-01T15:46:09Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 14510

DaanHoogland · 2025-08-02T10:24:40Z

@blueorangutan test

blueorangutan · 2025-08-02T10:26:04Z

@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

blueorangutan · 2025-08-03T03:05:58Z

[SF] Trillian test result (tid-14009)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 57357 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9074-t14009-kvm-ol8.zip
Smoke tests completed. 146 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

github-actions · 2025-08-28T08:30:10Z

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

DaanHoogland · 2025-08-29T06:55:38Z

@BryanMLima @winterhazel , 4.21 is out, can we try to get this in 4.22? or 4.20.2 as you, see fit?

cc @weizhouapache @harikrishna-patnala

winterhazel · 2025-09-23T22:03:25Z

@BryanMLima @winterhazel , 4.21 is out, can we try to get this in 4.22? or 4.20.2 as you, see fit?

Sure, we just need some manual testing. I commented that I wanted to do it before, but I will hardly be able to do it in the upcoming 2 months, so if someone else could give this some attention to fit it in 4.22, that would be great.

DaanHoogland · 2025-09-24T14:58:16Z

@BryanMLima @winterhazel , 4.21 is out, can we try to get this in 4.22? or 4.20.2 as you, see fit?

Sure, we just need some manual testing. I commented that I wanted to do it before, but I will hardly be able to do it in the upcoming 2 months, so if someone else could give this some attention to fit it in 4.22, that would be great.

Maybe (this is not me volunteering) but there is also conflicts ATM.

…method # Conflicts: # plugins/host-allocators/random/pom.xml

DaanHoogland · 2025-09-25T11:58:26Z

@blueorangutan package

blueorangutan · 2025-09-25T12:00:04Z

@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2025-09-25T14:06:41Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15167

DaanHoogland · 2025-09-26T11:31:11Z

@blueorangutan test

blueorangutan · 2025-09-26T11:32:03Z

@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

blueorangutan · 2025-09-27T02:41:01Z

[SF] Trillian test result (tid-14448)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 51301 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9074-t14448-kvm-ol8.zip
Smoke tests completed. 147 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

DaanHoogland · 2025-10-10T08:39:54Z

move to v23 @winterhazel @JoaoJandre ?

github-actions bot added the status:has-conflicts label Jun 19, 2024

BryanMLima force-pushed the refactor-allocate-to-method branch from 1ff8f4a to 42d16d9 Compare July 11, 2024 15:47

boring-cyborg bot added component:api component:build component:compute component:management-server component:orchestration labels Jul 11, 2024

github-actions bot added status:has-conflicts and removed status:has-conflicts labels Jul 11, 2024

BryanMLima force-pushed the refactor-allocate-to-method branch from 537c10f to 952c273 Compare July 15, 2024 13:26

github-actions bot removed the status:has-conflicts label Jul 15, 2024

BryanMLima changed the title ~~Refactoring FirstFitAllocator class~~ Refactoring Allocator classes Jul 15, 2024

BryanMLima marked this pull request as ready for review July 15, 2024 13:44

BryanMLima changed the title ~~Refactoring Allocator classes~~ Refactor of Allocator classes Jul 15, 2024

BryanMLima requested review from DaanHoogland and JoaoJandre and removed request for JoaoJandre July 15, 2024 13:45

DaanHoogland reviewed Jul 15, 2024

View reviewed changes

server/src/main/java/com/cloud/agent/manager/allocator/impl/TestingAllocator.java Show resolved Hide resolved

JoaoJandre approved these changes Jul 18, 2024

View reviewed changes

BryanMLima force-pushed the refactor-allocate-to-method branch 2 times, most recently from dd1eb14 to 952c273 Compare July 24, 2024 14:28

BryanMLima force-pushed the refactor-allocate-to-method branch from 952c273 to b7cc66f Compare August 23, 2024 19:11

Add new line at EOF

e153515

github-actions bot added the status:has-conflicts label Aug 28, 2025

weizhouapache modified the milestones: 4.21.0, 4.22.0 Aug 29, 2025

sureshanaparti removed this from Apache CloudStack 4.21.0 Aug 29, 2025

sureshanaparti added this to Apache CloudStack 4.22.0 Aug 29, 2025

sureshanaparti moved this to In Progress in Apache CloudStack 4.22.0 Aug 29, 2025

Merge remote-tracking branch 'origin/main' into refactor-allocate-to-…

1d8e0e6

…method # Conflicts: # plugins/host-allocators/random/pom.xml

github-actions bot removed the status:has-conflicts label Sep 25, 2025

Remove unnecessary random allocator pom

39a6031

JoaoJandre modified the milestones: 4.22.0, 4.23 Oct 10, 2025

Refactor of Allocator classes #9074

Are you sure you want to change the base?

Refactor of Allocator classes #9074

Uh oh!

Conversation

BryanMLima commented May 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

How Has This Been Tested?

How did you try to break this feature and the system with this change?

Uh oh!

codecov bot commented May 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Jun 19, 2024

Uh oh!

github-actions bot commented Jul 12, 2024

Uh oh!

Uh oh!

JoaoJandre left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Aug 26, 2024

Uh oh!

blueorangutan commented Aug 26, 2024

Uh oh!

blueorangutan commented Aug 26, 2024

Uh oh!

DaanHoogland commented Aug 26, 2024

Uh oh!

blueorangutan commented Aug 1, 2025

Uh oh!

BryanMLima commented Aug 1, 2025

Uh oh!

blueorangutan commented Aug 1, 2025

Uh oh!

blueorangutan commented Aug 1, 2025

Uh oh!

DaanHoogland commented Aug 2, 2025

Uh oh!

blueorangutan commented Aug 2, 2025

Uh oh!

blueorangutan commented Aug 3, 2025

Uh oh!

github-actions bot commented Aug 28, 2025

Uh oh!

DaanHoogland commented Aug 29, 2025

Uh oh!

winterhazel commented Sep 23, 2025

Uh oh!

DaanHoogland commented Sep 24, 2025

Uh oh!

DaanHoogland commented Sep 25, 2025

Uh oh!

blueorangutan commented Sep 25, 2025

Uh oh!

blueorangutan commented Sep 25, 2025

Uh oh!

DaanHoogland commented Sep 26, 2025

Uh oh!

blueorangutan commented Sep 26, 2025

Uh oh!

blueorangutan commented Sep 27, 2025

Uh oh!

DaanHoogland commented Oct 10, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

BryanMLima commented May 10, 2024 •

edited

Loading

codecov bot commented May 10, 2024 •

edited

Loading