You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "MitchDrage (via GitHub)" <gi...@apache.org> on 2026/10/09 09:22:30 UTC

[PR] Allow per-Domain/Account VPC tier limits [cloudstack]

MitchDrage opened a new pull request, #14373:
URL: https://github.com/apache/cloudstack/pull/14373

   ### Description
   
   - `vpc.max.networks` can now be set per account and per domain, falling back to the global value.
   - Changed `vpc.max.networks` to be dynamic, so changes no longer need a restart.
   - Hitting the limit now returns a 431 with a readable message, where it used to return a 530 "Internal error executing command".
   - The key name is unchanged, so existing values carry over without a DB migration.
   
   **Messages:**
   Original (HTTP 530):
   - Non-admin users: `Internal error executing command, please contact your system administrator`
   - Root admins: `Number of networks per VPC cannot surpass [3].`
   
   New (HTTP 431, all users):
   `VPC web-prod has reached the maximum of 3 networks. Delete an unused network or contact your platform administrator to raise the limit.`
   
   Closes #14372
   
   ### Types of changes
   
   - [ ] Breaking change (fix or feature that would cause existing functionality to change)
   - [x] 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
   - [x] Test (unit or integration test code)
   
   ### Feature/Enhancement Scale or Bug Severity
   
   #### Feature/Enhancement Scale
   
   - [ ] Major
   - [x] Minor
   
   ### Screenshots (if appropriate):
   N/A
   
   ### How Has This Been Tested?
   
   - **Unit tests**: three new tests cover an account value above the global value, an account value below it, and the domain value applying when the account has none.
   - **Simulator**: advanced zone, with tenant calls made as non-admin users:
     - The global limit rejects the extra tier with a 431 and the message above.
     - Changing the global value applies straight away.
     - `resetConfiguration` on an account falls back to the domain value.
     - The key appears in `listConfigurations` for both an account and a domain, which the account and domain Settings tabs use.
   
   #### How did you try to break this feature and the system with this change?
   
   - Ran the new unit tests against the original code to confirm they fail there.
   - Tested an upgrade from an existing install: a custom value of 4 was kept.
   - Ran the full `server` module test suite. All tests pass.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Allow per-Domain/Account VPC tier limits [cloudstack]

Posted by "Damans227 (via GitHub)" <gi...@apache.org>.
Damans227 commented on code in PR #14373:
URL: https://github.com/apache/cloudstack/pull/14373#discussion_r4229956749


##########
engine/components-api/src/main/java/com/cloud/network/vpc/VpcManager.java:
##########
@@ -57,6 +57,13 @@ public interface VpcManager {
             true,
             ConfigKey.Scope.Global,
             null);
+    ConfigKey<Integer> VpcMaxNetworks = new ConfigKey<>("Advanced",
+            Integer.class,
+            "vpc.max.networks",
+            "3",
+            "Maximum number of networks per VPC. Can be overridden per domain and per account",
+            true,
+            List.of(ConfigKey.Scope.Account, ConfigKey.Scope.Domain));

Review Comment:
   can a domain admin now raise this for their own domain? they can change account and domain settings, so the operator cap stops being a cap



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Make vpc.max.networks dynamic and return a useful error at the limit [cloudstack]

Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #14373:
URL: https://github.com/apache/cloudstack/pull/14373#issuecomment-6315343070

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Make vpc.max.networks dynamic and return a useful error at the limit [cloudstack]

Posted by "RosiKyu (via GitHub) via commits" <co...@cloudstack.apache.org>.
RosiKyu commented on PR #14373:
URL: https://github.com/apache/cloudstack/pull/14373#issuecomment-6837431150

   @blueorangutan package


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Make vpc.max.networks dynamic and return a useful error at the limit [cloudstack]

Posted by "Damans227 (via GitHub)" <gi...@apache.org>.
Damans227 commented on code in PR #14373:
URL: https://github.com/apache/cloudstack/pull/14373#discussion_r4249933299


##########
engine/components-api/src/main/java/com/cloud/network/vpc/VpcManager.java:
##########
@@ -57,6 +57,13 @@ public interface VpcManager {
             true,
             ConfigKey.Scope.Global,
             null);
+    ConfigKey<Integer> VpcMaxNetworks = new ConfigKey<>("Advanced",
+            Integer.class,
+            "vpc.max.networks",
+            "3",
+            "Maximum number of networks per VPC. Can be overridden per domain and per account",
+            true,
+            List.of(ConfigKey.Scope.Account, ConfigKey.Scope.Domain));

Review Comment:
   thanks, that covers it



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Make vpc.max.networks dynamic and return a useful error at the limit [cloudstack]

Posted by "bernardodemarco (via GitHub) via commits" <co...@cloudstack.apache.org>.
bernardodemarco commented on code in PR #14373:
URL: https://github.com/apache/cloudstack/pull/14373#discussion_r4538794650


##########
server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java:
##########
@@ -2580,9 +2576,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
 
                 try {
                     // check number of active networks in vpc
-                    if (_ntwkDao.countVpcNetworks(vpc.getId()) >= _maxNetworks) {
-                        logger.warn(String.format("Failed to create a new VPC Guest Network because the number of networks per VPC has reached its maximum capacity of [%s]. Increase it by modifying global config [%s].", _maxNetworks, Config.VpcMaxNetworks));
-                        throw new CloudRuntimeException(String.format("Number of networks per VPC cannot surpass [%s].", _maxNetworks));
+                    final int maxNetworks = VpcMaxNetworks.value();
+                    if (_ntwkDao.countVpcNetworks(vpc.getId()) >= maxNetworks) {
+                        logger.warn("Failed to create a new network in VPC [{}] because it has reached the maximum of [{}] networks for {}. "
+                                + "Increase it by modifying the global setting [{}].", vpc, maxNetworks, _accountMgr.getAccount(vpc.getAccountId()), VpcMaxNetworks.key());

Review Comment:
   ```suggestion
                                   + "Increase it by modifying the global setting [{}].", vpc, maxNetworks, VpcMaxNetworks.key());
   ```



##########
server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java:
##########
@@ -2580,9 +2576,12 @@ public void doInTransactionWithoutResult(final TransactionStatus status) {
 
                 try {
                     // check number of active networks in vpc
-                    if (_ntwkDao.countVpcNetworks(vpc.getId()) >= _maxNetworks) {
-                        logger.warn(String.format("Failed to create a new VPC Guest Network because the number of networks per VPC has reached its maximum capacity of [%s]. Increase it by modifying global config [%s].", _maxNetworks, Config.VpcMaxNetworks));
-                        throw new CloudRuntimeException(String.format("Number of networks per VPC cannot surpass [%s].", _maxNetworks));
+                    final int maxNetworks = VpcMaxNetworks.value();
+                    if (_ntwkDao.countVpcNetworks(vpc.getId()) >= maxNetworks) {
+                        logger.warn("Failed to create a new network in VPC [{}] because it has reached the maximum of [{}] networks for {}. "

Review Comment:
   Since the setting is global, we do not need to mention that the maximum number of networks of a specific account has been reached.
   
   ```suggestion
                           logger.warn("Failed to create a new network in VPC [{}] because it has reached the maximum of [{}] networks. "
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Make vpc.max.networks dynamic and return a useful error at the limit [cloudstack]

Posted by "MitchDrage (via GitHub)" <gi...@apache.org>.
MitchDrage commented on PR #14373:
URL: https://github.com/apache/cloudstack/pull/14373#issuecomment-6093574024

   I've changed direction on this PR. Per-account/domain values aren't needed: the cap exists to handle the technical limitation of the VR's NIC capacity which is the same for every tenant, and VPC tiers already count toward the account and domain guest network limits.
   I've removed the per-account/domain values, and this PR now makes the global setting dynamic, with the clearer error. I've updated the title and description to match.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Make vpc.max.networks dynamic and return a useful error at the limit [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #14373:
URL: https://github.com/apache/cloudstack/pull/14373#issuecomment-6486902231

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Make vpc.max.networks dynamic and return a useful error at the limit [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #14373:
URL: https://github.com/apache/cloudstack/pull/14373#issuecomment-6507007419

   <b>[SF] Trillian test result (tid-17319)</b>
   Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
   Total time taken: 56848 seconds
   Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr14373-t17319-kvm-ol8.zip
   Smoke tests completed. 157 look OK, 0 have errors, 0 did not run
   Only failed and skipped tests results shown below:
   
   
   Test | Result | Time (s) | Test File
   --- | --- | --- | ---
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Make vpc.max.networks dynamic and return a useful error at the limit [cloudstack]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #14373:
URL: https://github.com/apache/cloudstack/pull/14373#issuecomment-6315334611

   ## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/14373?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :white_check_mark: All modified and coverable lines are covered by tests.
   :white_check_mark: Project coverage is 3.69%. Comparing base ([`f03cffc`](https://app.codecov.io/gh/apache/cloudstack/commit/f03cffc19e6e055754b1a05e3359c4c75fa6854e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`d95452e`](https://app.codecov.io/gh/apache/cloudstack/commit/d95452eb883ab0d8ec2fd05b6a9549aeca8419af?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 41 commits behind head on main.
   > :exclamation:  There is a different number of reports uploaded between BASE (f03cffc) and HEAD (d95452e). Click for more details.
   > 
   > <details><summary>HEAD has 1 upload less than BASE</summary>
   >
   >| Flag | BASE (f03cffc) | HEAD (d95452e) |
   >|------|------|------|
   >|unittests|1|0|
   ></details>
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@              Coverage Diff              @@
   ##               main   #14373       +/-   ##
   =============================================
   - Coverage     20.21%    3.69%   -16.53%     
   =============================================
     Files          6426      491     -5935     
     Lines        580181    42220   -537961     
     Branches      71033     7974    -63059     
   =============================================
   - Hits         117282     1558   -115724     
   + Misses       450184    40436   -409748     
   + Partials      12715      226    -12489     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/14373/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/14373/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `3.69% <ø> (ø)` | |
   | [unittests](https://app.codecov.io/gh/apache/cloudstack/pull/14373/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/cloudstack/pull/14373?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Make vpc.max.networks dynamic and return a useful error at the limit [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #14373:
URL: https://github.com/apache/cloudstack/pull/14373#issuecomment-6315368508

   @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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Make vpc.max.networks dynamic and return a useful error at the limit [cloudstack]

Posted by "MitchDrage (via GitHub)" <gi...@apache.org>.
MitchDrage commented on code in PR #14373:
URL: https://github.com/apache/cloudstack/pull/14373#discussion_r4236314093


##########
engine/components-api/src/main/java/com/cloud/network/vpc/VpcManager.java:
##########
@@ -57,6 +57,13 @@ public interface VpcManager {
             true,
             ConfigKey.Scope.Global,
             null);
+    ConfigKey<Integer> VpcMaxNetworks = new ConfigKey<>("Advanced",
+            Integer.class,
+            "vpc.max.networks",
+            "3",
+            "Maximum number of networks per VPC. Can be overridden per domain and per account",
+            true,
+            List.of(ConfigKey.Scope.Account, ConfigKey.Scope.Domain));

Review Comment:
   Thanks Damans. You were right - a domain admin could change the setting for their domain.
   Since I've pivoted the idea here, it's no longer applicable as it's back to being a global setting. See the other comment in the PR and the new description.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Make vpc.max.networks dynamic and return a useful error at the limit [cloudstack]

Posted by "nvazquez (via GitHub)" <gi...@apache.org>.
nvazquez commented on PR #14373:
URL: https://github.com/apache/cloudstack/pull/14373#issuecomment-6486886038

   @blueorangutan test


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Make vpc.max.networks dynamic and return a useful error at the limit [cloudstack]

Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #14373:
URL: https://github.com/apache/cloudstack/pull/14373#issuecomment-6317573205

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Make vpc.max.networks dynamic and return a useful error at the limit [cloudstack]

Posted by "blueorangutan (via GitHub) via commits" <co...@cloudstack.apache.org>.
blueorangutan commented on PR #14373:
URL: https://github.com/apache/cloudstack/pull/14373#issuecomment-6837456839

   @RosiKyu 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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Make vpc.max.networks dynamic and return a useful error at the limit [cloudstack]

Posted by "blueorangutan (via GitHub) via commits" <co...@cloudstack.apache.org>.
blueorangutan commented on PR #14373:
URL: https://github.com/apache/cloudstack/pull/14373#issuecomment-6839216312

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org