Enable defining a network as redundant during restart through the UI #7405

GaOrtiga · 2023-04-05T14:27:55Z

Description

By utilizing the restartNetwork API and setting the makeredundant parameter to true, ACS allows the user to make a guest network redundant during its restart process; however, this feature was only available through the API.
An adjustment was made to add this functionality to the UI.

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)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Screenshots (if appropriate):

sonarqubecloud · 2023-04-05T15:30:56Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

codecov · 2023-04-05T15:42:45Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 3.57%. Comparing base (5bf869c) to head (92b08e9).
⚠️ Report is 30 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (5bf869c) and HEAD (92b08e9). Click for more details.

HEAD has 1 upload less than BASE

Flag BASE (5bf869c) HEAD (92b08e9)

unittests 1 0

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #7405       +/-   ##
============================================
- Coverage     17.51%   3.57%   -13.95%     
============================================
  Files          5914     446     -5468     
  Lines        529867   37731   -492136     
  Branches      64722    6959    -57763     
============================================
- Hits          92782    1347    -91435     
+ Misses       426635   36218   -390417     
+ Partials      10450     166    -10284

Flag	Coverage Δ
uitests	`3.57% <ø> (-0.02%)`	⬇️
unittests	`?`

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.

DaanHoogland · 2023-04-06T07:36:34Z

@GaOrtiga i think this was removed from the UI on purpose. I don´t mind either way but it seems to me operators want to have control on whether users are allowed to make this mistake (as i understood it).

DaanHoogland · 2023-04-06T07:36:47Z

code lgtm btw

borisstoyanov · 2023-04-06T09:00:59Z

exactly, tbh I'm against this api option to exist at all since whether redundant or not should be defined in the offering not as a single action

GaOrtiga · 2023-04-06T13:19:45Z

@GaOrtiga i think this was removed from the UI on purpose. I don´t mind either way but it seems to me operators want to have control on whether users are allowed to make this mistake (as i understood it).

I understand, however, users already can do this through the API; therefore, operators don't fully control it. Also, for VPCs this option exists in the UI while restarting them.
I see @borisstoyanov's point on redundant being a feature defined on the offering, however, the discussion would be outside of the PR's scope; could we create an issue or a thread on the mail-list to discuss the make redundant feature (for both VPC and guest networks)?

weizhouapache

As far as I know, some users wanted to disable this on UI, but some others want this.
As a compromise, can we enable it only for root admin ? if users want it to be visible for domain admins or regular users, create a new network offering and do it through network update with the new network offering.

GaOrtiga · 2023-04-18T20:23:21Z

As far as I know, some users wanted to disable this on UI, but some others want this. As a compromise, can we enable it only for root admin ? if users want it to be visible for domain admins or regular users, create a new network offering and do it through network update with the new network offering.

We could also apply these changes to the UI to maintain consistence with the VPCs section and provide an option in the backend that lets admins determine whether or not to allow users to have access to this. This would standardize the code and address both issues. What are your thoughts on this? If you guys agree on that, we can move with this PR as is, and then create a new one to introduce this other featureset into ACS.

DaanHoogland · 2023-04-28T07:18:59Z

As far as I know, some users wanted to disable this on UI, but some others want this. As a compromise, can we enable it only for root admin ? if users want it to be visible for domain admins or regular users, create a new network offering and do it through network update with the new network offering.

We could also apply these changes to the UI to maintain consistence with the VPCs section and provide an option in the backend that lets admins determine whether or not to allow users to have access to this. This would standardize the code and address both issues. What are your thoughts on this? If you guys agree on that, we can move with this PR as is, and then create a new one to introduce this other featureset into ACS.

I agree with the addition that disabling it in the backend should also remove the option on the UI.

weizhouapache · 2023-04-28T07:30:47Z

what's your opinions ?
cc @rohityadavcloud @alexandremattioli @andrijapanicsb @NuxRo

andrijapanicsb · 2023-05-03T13:46:46Z

There were a lot of cases where people have "accidentally" switched this on and didn't know the consequences, so... not sure what is the best way forward.

yadvr · 2023-05-08T06:58:03Z

Sorry -1, we had a conversation in the past. In fact this feature never had existed, until I had put it in. After discussions and feedback from users esp service providers, we decided to remove the option from the UI though the API param may still be available via UI. This is because service providers may have an explicit network offering with redundant routers, users without using/paying for one could this form to circumvent to get redundant routers for a network not intended for them.

However, there is a middle ground but probably not necessary - it could be via adding a global setting for operators to decide if they want to enable such a feature for end users. (listCapabilities or other means can export this option to the UI to show/hide such a setting).

weizhouapache · 2023-05-08T09:30:56Z

@GaOrtiga
it seems difficult to get a consensus.

will you consider a new global setting to manage it ? (requires UI, API and service layer changes)

GaOrtiga · 2023-06-27T14:01:17Z

There were a lot of cases where people have "accidentally" switched this on and didn't know the consequences, so... not sure what is the best way forward.

We could add a warning message explaining the consequences when that option is selected

Sorry -1, we had a conversation in the past. In fact this feature never had existed, until I had put it in. After discussions and feedback from users esp service providers, we decided to remove the option from the UI though the API param may still be available via UI. This is because service providers may have an explicit network offering with redundant routers, users without using/paying for one could this form to circumvent to get redundant routers for a network not intended for them.

However, there is a middle ground but probably not necessary - it could be via adding a global setting for operators to decide if they want to enable such a feature for end users. (listCapabilities or other means can export this option to the UI to show/hide such a setting).

They can still do the same process using the API, so adding this to the UI would not make a difference in that case.

As for the global setting, I do agree that is a good option, however it would fall out of the scope of this PR. And I still believe we should make these changes in order to keep consistence between the UI and the API, regardless of the addition of a global setting.

DaanHoogland · 2023-10-31T14:48:04Z

@GaOrtiga how do you want to move forward with this? (moving it to 4.20 for now)

yadvr

We can't accept this because (a) this was reverted after several service providers and users complaint that they don't want users to have rvr when the offering doesn't allow that by default, (b) isn't aligned with the original use-case. So, while the API may still allow for it, we don't necessarily want to expose that via UI.

yadvr · 2023-11-03T11:59:40Z

@GaOrtiga the only way could be to introduce a feature flag in config.json (in UI) and have it disabled by default, when enabled you can pass the options in UI to show the elements. Could you review if this approach works for your use-case or close the PR. Thanks.

andrijapanicsb · 2023-11-03T12:00:45Z

Agree with what Rohit said, this was enabled some releases ago, and it created issues for the Operators. It should be "impossible" by default, with the option to make it configured/enabled if the operator really needs it.

alexandremattioli · 2023-11-09T18:58:38Z

-1 on adding this to the UI. Until the fundamental issues with VRRP are solved this feature should be avoided at all costs.

NuxRo · 2023-11-09T20:36:59Z

Agree with what Rohit said, this was enabled some releases ago, and it created issues for the Operators. It should be "impossible" by default, with the option to make it configured/enabled if the operator really needs it.

+1 what Andrija said. Not against having the feature, but let's make it depend on a global option or something, or a network offering like @borisstoyanov suggested.
We should the same feature that exists in a VPC as well to be honest.

sureshanaparti · 2025-06-05T10:14:24Z

@blueorangutan package

blueorangutan · 2025-06-05T10:16:04Z

@sureshanaparti 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-06-05T12:58:28Z

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

DaanHoogland · 2025-10-10T07:50:40Z

@JoaoJandre do you still want this (behind a feature flag)?

JoaoJandre · 2025-10-10T12:20:56Z

@JoaoJandre do you still want this (behind a feature flag)?

As long as the default value for the config is true, to maintain the same behavior as the older versions, I'm fine with it.

DaanHoogland · 2025-10-10T15:14:12Z

@JoaoJandre do you still want this (behind a feature flag)?

As long as the default value for the config is true, to maintain the same behavior as the older versions, I'm fine with it.

@GaOrtiga will you implement such a flag?

GaOrtiga · 2026-01-06T17:14:24Z

@DaanHoogland I added the config

DaanHoogland · 2026-01-07T07:37:00Z

@DaanHoogland I added the config

thanks @GaOrtiga . @andrijapanicsb @yadvr , can you concur with this solution?

server/src/main/java/com/cloud/network/NetworkServiceImpl.java

sureshanaparti · 2026-01-08T08:58:38Z

server/src/main/java/com/cloud/network/NetworkServiceImpl.java

        User callerUser = _accountMgr.getActiveUser(CallContext.current().getCallingUserId());
+        if (makeRedundant && !_accountMgr.isRootAdmin(callerUser.getAccountId()) && !AllowUsersToMakeNetworksRedundant.value() ) {
+            throw new InvalidParameterValueException(String.format("Could not make network redundant as calling user is not Root Admin and [%s] is set to false.",
+                    AllowUsersToMakeNetworksRedundant.key()));


Suggested change

AllowUsersToMakeNetworksRedundant.key()));

@sureshanaparti , this is a syntax error. I think I know what you mean, but let’s not apply this.

Co-authored-by: Suresh Kumar Anaparti <sureshkumar.anaparti@gmail.com>

boring-cyborg bot added the component:UI label Apr 5, 2023

GaOrtiga changed the title ~~redundant networks~~ Enable defining a network as redundant during restart through the UI. Apr 5, 2023

GaOrtiga changed the title ~~Enable defining a network as redundant during restart through the UI.~~ Enable defining a network as redundant during restart through the UI Apr 5, 2023

weizhouapache approved these changes Apr 17, 2023

View reviewed changes

DaanHoogland approved these changes Apr 28, 2023

View reviewed changes

yadvr mentioned this pull request May 8, 2023

It is not possible to make network redundant in the UI #7260

Open

weizhouapache marked this pull request as draft May 12, 2023 07:44

weizhouapache marked this pull request as ready for review May 12, 2023 07:45

DaanHoogland added this to the 4.19.0.0 milestone Jun 22, 2023

DaanHoogland modified the milestones: 4.19.0.0, 4.20.0.0 Oct 31, 2023

yadvr requested changes Nov 3, 2023

View reviewed changes

JoaoJandre added this to the 4.21.0.0 milestone Sep 10, 2024

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

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

harikrishna-patnala modified the milestones: 4.22.0, 4.22.1 Nov 12, 2025

harikrishna-patnala removed this from Apache CloudStack 4.22.0 Nov 12, 2025

redundant networks

37da6da

rajujith added this to Apache CloudStack 4.22.1 Jan 5, 2026

GaOrtiga force-pushed the redundant_network_via_UI branch from bb53dbf to 969cd89 Compare January 6, 2026 17:12

boring-cyborg bot added the component:networking label Jan 6, 2026

addConfig

cc7eb93

GaOrtiga force-pushed the redundant_network_via_UI branch from 969cd89 to cc7eb93 Compare January 6, 2026 17:13

rajujith moved this to In Progress in Apache CloudStack 4.22.1 Jan 7, 2026

DaanHoogland requested a review from yadvr January 7, 2026 07:35

sureshanaparti reviewed Jan 8, 2026

View reviewed changes

server/src/main/java/com/cloud/network/NetworkServiceImpl.java Outdated Show resolved Hide resolved

sureshanaparti reviewed Jan 8, 2026

View reviewed changes

Update server/src/main/java/com/cloud/network/NetworkServiceImpl.java

92b08e9

Co-authored-by: Suresh Kumar Anaparti <sureshkumar.anaparti@gmail.com>

Enable defining a network as redundant during restart through the UI #7405

Are you sure you want to change the base?

Enable defining a network as redundant during restart through the UI #7405

Conversation

GaOrtiga commented Apr 5, 2023

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Screenshots (if appropriate):

Uh oh!

sonarqubecloud bot commented Apr 5, 2023

Uh oh!

codecov bot commented Apr 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

DaanHoogland commented Apr 6, 2023

Uh oh!

DaanHoogland commented Apr 6, 2023

Uh oh!

borisstoyanov commented Apr 6, 2023

Uh oh!

GaOrtiga commented Apr 6, 2023

Uh oh!

weizhouapache left a comment

Choose a reason for hiding this comment

Uh oh!

GaOrtiga commented Apr 18, 2023

Uh oh!

DaanHoogland commented Apr 28, 2023

Uh oh!

weizhouapache commented Apr 28, 2023

Uh oh!

andrijapanicsb commented May 3, 2023

Uh oh!

yadvr commented May 8, 2023

Uh oh!

weizhouapache commented May 8, 2023

Uh oh!

GaOrtiga commented Jun 27, 2023

Uh oh!

DaanHoogland commented Oct 31, 2023

Uh oh!

yadvr left a comment

Choose a reason for hiding this comment

Uh oh!

yadvr commented Nov 3, 2023

Uh oh!

andrijapanicsb commented Nov 3, 2023

Uh oh!

alexandremattioli commented Nov 9, 2023

Uh oh!

NuxRo commented Nov 9, 2023

Uh oh!

sureshanaparti commented Jun 5, 2025

Uh oh!

blueorangutan commented Jun 5, 2025

Uh oh!

blueorangutan commented Jun 5, 2025

Uh oh!

DaanHoogland commented Oct 10, 2025

Uh oh!

JoaoJandre commented Oct 10, 2025

Uh oh!

DaanHoogland commented Oct 10, 2025

Uh oh!

GaOrtiga commented Jan 6, 2026

Uh oh!

DaanHoogland commented Jan 7, 2026

Uh oh!

Uh oh!

sureshanaparti Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

DaanHoogland Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

codecov bot commented Apr 5, 2023 •

edited

Loading