Allow deploy of VRs on dedicated resources by GaOrtiga · Pull Request #9531 · apache/cloudstack

GaOrtiga · 2024-08-14T16:08:48Z

Description

Currently, it is not possible to deploy VRs on dedicated resources, even if they are dedicated to the domain in which the VR will be created.

A new global configuration allowRoutersOnDedicatedResources has been created to allow VRs to be deployed on hosts, clusters, pods and zones that are dedicated to their respective domains.

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
test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

How Has This Been Tested?

With the configuration marked as true:

I dedicated resources to the domain in which the VRs would be deployd and verified that the deploy worked normally.
I dedicated resources to an account in that domain and verified that I could not deploy VRs on these resources.
I dedicated resources to a different domain and verified that I could not deploy VRs on these resources.

I marked the configuration as false and verified that I could not deploy VRs on any of the dedicated resources.

DaanHoogland · 2025-02-06T09:07:00Z

@blueorangutan package

blueorangutan · 2025-02-06T09:08: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.

codecov · 2025-02-06T09:55:31Z

Codecov Report

❌ Patch coverage is 81.81818% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 18.89%. Comparing base (3166e64) to head (65524e3).
⚠️ Report is 85 commits behind head on main.

Files with missing lines	Patch %	Lines
...n/java/com/cloud/capacity/dao/CapacityDaoImpl.java	50.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #9531      +/-   ##
============================================
+ Coverage     18.01%   18.89%   +0.87%     
- Complexity    16607    18226    +1619     
============================================
  Files          6029     6174     +145     
  Lines        542160   555230   +13070     
  Branches      66451    67775    +1324     
============================================
+ Hits          97682   104886    +7204     
- Misses       433461   438824    +5363     
- Partials      11017    11520     +503

Flag	Coverage Δ
uitests	`3.53% <ø> (+<0.01%)`	⬆️
unittests	`20.08% <81.81%> (+0.91%)`	⬆️

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

☔ View full report in Codecov by Harness.
📢 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.

blueorangutan · 2025-02-06T10:30:36Z

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

nicoschmdt · 2025-02-14T14:39:58Z

Hey @DaanHoogland, could we run the CI on this PR?

DaanHoogland · 2025-02-15T14:03:49Z

@blueorangutan test

blueorangutan · 2025-02-15T14:06:03Z

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

blueorangutan · 2025-02-16T07:04:47Z

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

Test	Result	Time (s)	Test File
test_11_isolated_network_with_dynamic_routed_mode	`Error`	2.29	test_ipv4_routing.py
test_12_vpc_and_tier_with_dynamic_routed_mode	`Error`	1.41	test_ipv4_routing.py
test_12_vpc_and_tier_with_dynamic_routed_mode	`Error`	1.41	test_ipv4_routing.py
test_12_start_vm_multiple_volumes_allocated	`Error`	20.01	test_vm_life_cycle.py

github-actions · 2025-05-02T08:39:43Z

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

nicoschmdt · 2025-05-06T20:22:26Z

@blueorangutan package

blueorangutan · 2025-05-06T20:24:04Z

@nicoschmdt 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-05-06T21:28:52Z

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

github-actions · 2025-07-15T11:11:38Z

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

winterhazel · 2026-05-25T12:58:39Z

@GaOrtiga could you have a look at the conflicts?

GaOrtiga · 2026-05-25T18:12:28Z

@rajujith @sureshanaparti @winterhazel

Sorry for the delay, this was on my backlog.

I had to to some adjustments to address some use cases that were brought up, but now its ready for review/testing.

GaOrtiga · 2026-06-01T14:13:34Z

@blueorangutan package

blueorangutan · 2026-06-01T14:14:04Z

@GaOrtiga a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2026-06-01T15:20:39Z

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

DaanHoogland

clgtm, (one style remark)

…Impl.java Co-authored-by: dahn <daan.hoogland@gmail.com>

DaanHoogland · 2026-06-16T11:49:44Z

@blueorangutan package

blueorangutan · 2026-06-16T11:52:05Z

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

blueorangutan · 2026-06-16T12:36:57Z

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

DaanHoogland

clgtm

DaanHoogland · 2026-06-18T07:55:59Z

@blueorangutan test

blueorangutan · 2026-06-18T07:58:04Z

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

blueorangutan · 2026-06-18T08:20:37Z

[SF] Trillian Build Failed (tid-16358)

weizhouapache · 2026-06-18T08:29:45Z

            ConfigKey.Kind.Select,
            "random,firstfit,userdispersing,firstfitleastconsumed");

+    ConfigKey<Boolean> allowRoutersOnDedicatedResources = new ConfigKey<>(


this may be not required.
in my opinion, the VR should always be allowed to deploy on resource dedicated to the owner of the VR

Ok, I'll remove the config

weizhouapache · 2026-06-18T08:30:05Z


-        sql.append(ORDER_CLUSTERS_BY_AGGREGATE_CAPACITY_JOIN_1);
+        if (isVr && allowRoutersOnDedicatedResources) {
+            sql.append(ORDER_CLUSTERS_BY_AGGREGATE_CAPACITY_INCLUDE_DEDICATED_JOIN_CLAUSE.replace("ownerId", ownerId.toString()));


this is not elegant I think

I think it is very elegant, like a pig on clogs . What do you suggest, @weizhouapache ?

you can refer to

private static final String ORDER_CLUSTERS_BY_AGGREGATE_CAPACITY_JOIN_1 = "JOIN host ON capacity.host_id = host.id " + "LEFT JOIN (SELECT affinity_group.id, agvm.instance_id FROM affinity_group_vm_map agvm JOIN affinity_group ON agvm.affinity_group_id = affinity_group.id AND affinity_group.type='ExplicitDedication') AS ag ON ag.instance_id = ? " + "LEFT JOIN dedicated_resources dr_pod ON dr_pod.pod_id IS NOT NULL AND dr_pod.pod_id = host.pod_id " + "LEFT JOIN dedicated_resources dr_cluster ON dr_cluster.cluster_id IS NOT NULL AND dr_cluster.cluster_id = host.cluster_id " + "LEFT JOIN dedicated_resources dr_host ON dr_host.host_id IS NOT NULL AND dr_host.host_id = host.id ";

@weizhouapache @DaanHoogland

Using ? was my first option aswell, since it's how it's done elsewhere in the method, however, the values for ? are injected informing the order (how many ?s before it). Since this one is optional, it may or may not change the order of the remaining ? that come after it, thus, I'd have to add at least 3 more ifs to account for this possible change of order, which IMO ends up being even less elegant.

that's ok, thanks for explanation.

Maybe we could use a placeholder syntax such as {{ ownerId }} to make it clear that this is a variable that will be replaced at runtime.

Yeah, good idea.

weizhouapache · 2026-06-18T08:30:27Z

+    private static final String ORDER_CLUSTERS_BY_AGGREGATE_CAPACITY_INCLUDE_DEDICATED_JOIN_CLAUSE =
+            "JOIN host ON capacity.host_id = host.id " +
+                    "LEFT JOIN (SELECT affinity_group.id, agvm.instance_id FROM affinity_group_vm_map agvm JOIN affinity_group ON agvm.affinity_group_id = affinity_group.id AND affinity_group.type='ExplicitDedication') AS ag ON ag.instance_id = ? " +
+                    "LEFT JOIN dedicated_resources dr_pod ON dr_pod.pod_id IS NOT NULL AND dr_pod.pod_id = host.pod_id AND dr_pod.account_id IS NOT NULL AND dr_pod.account_id != ownerId " +


what about resources dedicated to domain ?

Dedication to domains is already being addressed in this query

oh really ?

I couldn't even find a domain or domain_id field in the SQL query...

I'll run a test, been a while since I altered this query, so I might be confusing

DaanHoogland reviewed Aug 16, 2024

View reviewed changes

Comment thread engine/schema/src/main/java/com/cloud/capacity/dao/CapacityDao.java Outdated

nicoschmdt force-pushed the deploy_vr_dedicated_resources branch from 5da9945 to 7b05daf Compare February 5, 2025 16:13

boring-cyborg Bot added the component:api label Feb 5, 2025

JoaoJandre approved these changes Feb 19, 2025

View reviewed changes

nicoschmdt self-assigned this Feb 19, 2025

github-actions Bot added the status:has-conflicts label May 2, 2025

nicoschmdt force-pushed the deploy_vr_dedicated_resources branch from 7b05daf to e15e366 Compare May 6, 2025 20:21

github-actions Bot removed the status:has-conflicts label May 6, 2025

DaanHoogland added this to the 4.21.0 milestone May 7, 2025

sureshanaparti added this to Apache CloudStack 4.21.0 Jun 5, 2025

sureshanaparti moved this to In Progress in Apache CloudStack 4.21.0 Jun 5, 2025

github-actions Bot added the status:has-conflicts label Jul 15, 2025

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

sureshanaparti added this to Apache CloudStack 4.22.0 Aug 29, 2025

sureshanaparti removed this from Apache CloudStack 4.21.0 Aug 29, 2025

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

sureshanaparti added this to the 4.23.0 milestone Apr 24, 2026

allow deploy when resource is dedicated to account

e2197f0

GaOrtiga force-pushed the deploy_vr_dedicated_resources branch from 2bc25d1 to e2197f0 Compare May 25, 2026 17:27

fix tests

8b20abe

GaOrtiga marked this pull request as ready for review May 25, 2026 18:13

DaanHoogland reviewed Jun 11, 2026

View reviewed changes

Comment thread engine/schema/src/main/java/com/cloud/capacity/dao/CapacityDaoImpl.java Outdated

DaanHoogland reviewed Jun 11, 2026

View reviewed changes

Comment thread engine/schema/src/main/java/com/cloud/capacity/dao/CapacityDaoImpl.java Outdated

DaanHoogland approved these changes Jun 11, 2026

View reviewed changes

GaOrtiga and others added 2 commits June 11, 2026 07:27

Update engine/schema/src/main/java/com/cloud/capacity/dao/CapacityDao…

e35c299

…Impl.java Co-authored-by: dahn <daan.hoogland@gmail.com>

Update engine/schema/src/main/java/com/cloud/capacity/dao/CapacityDao…

1bdb771

…Impl.java Co-authored-by: dahn <daan.hoogland@gmail.com>

DaanHoogland requested changes Jun 16, 2026

View reviewed changes

DaanHoogland self-requested a review June 16, 2026 11:49

github-actions Bot mentioned this pull request Jun 16, 2026

[repo-status] Daily Status Report — June 16, 2026 #13435

Closed

DaanHoogland approved these changes Jun 18, 2026

View reviewed changes

weizhouapache reviewed Jun 18, 2026

View reviewed changes

boring-cyborg Bot added component:UI distro:ubuntu labels Jun 22, 2026

remove_config

65524e3

GaOrtiga force-pushed the deploy_vr_dedicated_resources branch from 6afadba to 65524e3 Compare June 22, 2026 16:45

Conversation

GaOrtiga commented Aug 14, 2024

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

How Has This Been Tested?

Uh oh!

Uh oh!

DaanHoogland commented Feb 6, 2025

Uh oh!

blueorangutan commented Feb 6, 2025

Uh oh!

codecov Bot commented Feb 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

blueorangutan commented Feb 6, 2025

Uh oh!

nicoschmdt commented Feb 14, 2025

Uh oh!

DaanHoogland commented Feb 15, 2025

Uh oh!

blueorangutan commented Feb 15, 2025

Uh oh!

blueorangutan commented Feb 16, 2025

Uh oh!

github-actions Bot commented May 2, 2025

Uh oh!

nicoschmdt commented May 6, 2025

Uh oh!

blueorangutan commented May 6, 2025

Uh oh!

blueorangutan commented May 6, 2025

Uh oh!

github-actions Bot commented Jul 15, 2025

Uh oh!

winterhazel commented May 25, 2026

Uh oh!

GaOrtiga commented May 25, 2026

Uh oh!

GaOrtiga commented Jun 1, 2026

Uh oh!

blueorangutan commented Jun 1, 2026

Uh oh!

blueorangutan commented Jun 1, 2026

Uh oh!

Uh oh!

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Jun 16, 2026

Uh oh!

blueorangutan commented Jun 16, 2026

Uh oh!

blueorangutan commented Jun 16, 2026

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Jun 18, 2026

Uh oh!

blueorangutan commented Jun 18, 2026

Uh oh!

blueorangutan commented Jun 18, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

codecov Bot commented Feb 6, 2025 •

edited

Loading