-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fix VPC restart with multi-CIDR networks: handle comma-separated CIDR… #12622
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Fix VPC restart with multi-CIDR networks: handle comma-separated CIDR… #12622
Conversation
… in NetworkVO.equals() When a network has multiple CIDRs (e.g. '192.168.2.0/24,160.0.0.0/24'), NetworkVO.equals() passes the raw comma-separated string to NetUtils.isNetworkAWithinNetworkB() which expects a single CIDR, causing 'cidr is not formatted correctly' error during VPC restart with cleanup=true. Extract only the first CIDR value before passing to NetUtils.
|
@blueorangutan package |
|
@blueorangutan package |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #12622 +/- ##
=========================================
Coverage 17.90% 17.90%
+ Complexity 16094 16093 -1
=========================================
Files 5938 5938
Lines 532864 532866 +2
Branches 65192 65192
=========================================
+ Hits 95392 95395 +3
Misses 426793 426793
+ Partials 10679 10678 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@blueorangutan package |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Fixes VPC restart failures for multi-CIDR networks by ensuring NetworkVO.equals() does not pass a raw comma-separated CIDR list into NetUtils.isNetworkAWithinNetworkB(), which expects a single CIDR.
Changes:
- Normalize
cidrvalues inNetworkVO.equals()by extracting the first CIDR from comma-separated strings before comparing.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return NetUtils.isNetworkAWithinNetworkB( | ||
| com.cloud.utils.StringUtils.getFirstValueFromCommaSeparatedString(cidr), | ||
| com.cloud.utils.StringUtils.getFirstValueFromCommaSeparatedString(that.cidr)); |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
equals is using NetUtils.isNetworkAWithinNetworkB(...), which is not symmetric (A within B does not imply B within A). That means NetworkVO.equals can violate the Java equals contract, and it is also inconsistent with hashCode() (which is based only on id). Consider switching equals/hashCode to be id-based like other VOs, or (if CIDR-based equality is truly intended) make the comparison symmetric and update hashCode accordingly (e.g., based on the same normalized CIDR(s) and trafficType).
| return NetUtils.isNetworkAWithinNetworkB( | ||
| com.cloud.utils.StringUtils.getFirstValueFromCommaSeparatedString(cidr), | ||
| com.cloud.utils.StringUtils.getFirstValueFromCommaSeparatedString(that.cidr)); |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor maintainability: getFirstValueFromCommaSeparatedString(...) is computed twice inline. Assign the normalized CIDRs to local variables before calling isNetworkAWithinNetworkB to avoid repeated splitting/trimming and make debugging/logging easier.
| return NetUtils.isNetworkAWithinNetworkB( | |
| com.cloud.utils.StringUtils.getFirstValueFromCommaSeparatedString(cidr), | |
| com.cloud.utils.StringUtils.getFirstValueFromCommaSeparatedString(that.cidr)); | |
| final String normalizedCidr = com.cloud.utils.StringUtils.getFirstValueFromCommaSeparatedString(cidr); | |
| final String normalizedThatCidr = com.cloud.utils.StringUtils.getFirstValueFromCommaSeparatedString(that.cidr); | |
| return NetUtils.isNetworkAWithinNetworkB(normalizedCidr, normalizedThatCidr); |
|
@blueorangutan package |
|
@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. |
#12621
… in NetworkVO.equals()
When a network has multiple CIDRs (e.g. '192.168.2.0/24,160.0.0.0/24'), NetworkVO.equals() passes the raw comma-separated string to NetUtils.isNetworkAWithinNetworkB() which expects a single CIDR, causing 'cidr is not formatted correctly' error during VPC restart with cleanup=true.
Extract only the first CIDR value before passing to NetUtils.
Description
This PR...
Types of changes
Bug fix (non-breaking change which fixes an issue)
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
on different VLANs assigned to the same VPC
How did you try to break this feature and the system with this change?