Fix VPC restart with multi-CIDR networks: handle comma-separated CIDR… by jeanvetorello · Pull Request #12622 · apache/cloudstack

jeanvetorello · 2026-02-09T23:49:16Z

#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

Major

Screenshots (if appropriate):

How Has This Been Tested?

Reproduced the issue on a CloudStack 4.21.0.0 environment with two public IP ranges
on different VLANs assigned to the same VPC
Applied the fix and confirmed VPC restart with cleanup=true completes successfully
Verified unit tests pass for NetworkVO.equals() with both single and comma-separated CIDRs

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

sureshanaparti · 2026-02-10T04:59:46Z

@blueorangutan package

sureshanaparti · 2026-02-10T05:04:14Z

@blueorangutan package

codecov · 2026-02-10T05:05:36Z

Codecov Report

❌ Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.92%. Comparing base (7aa0558) to head (c9e7a8b).

Files with missing lines	Patch %	Lines
.../cloud/configuration/ConfigurationManagerImpl.java	0.00%	5 Missing ⚠️
...src/main/java/com/cloud/network/dao/NetworkVO.java	0.00%	3 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               4.22   #12622      +/-   ##
============================================
+ Coverage     17.61%   17.92%   +0.31%     
- Complexity    15664    16179     +515     
============================================
  Files          5917     5949      +32     
  Lines        531402   534064    +2662     
  Branches      64971    65303     +332     
============================================
+ Hits          93596    95751    +2155     
- Misses       427252   427555     +303     
- Partials      10554    10758     +204

Flag	Coverage Δ
uitests	`3.66% <ø> (-0.04%)`	⬇️
unittests	`19.03% <0.00%> (+0.35%)`	⬆️

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.

sureshanaparti · 2026-02-10T05:10:28Z

@blueorangutan package

Copilot

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 cidr values in NetworkVO.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.

Copilot · 2026-02-10T05:10:46Z

engine/schema/src/main/java/com/cloud/network/dao/NetworkVO.java

+        return NetUtils.isNetworkAWithinNetworkB(
+                com.cloud.utils.StringUtils.getFirstValueFromCommaSeparatedString(cidr),
+                com.cloud.utils.StringUtils.getFirstValueFromCommaSeparatedString(that.cidr));


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

engine/schema/src/main/java/com/cloud/network/dao/NetworkVO.java

DaanHoogland · 2026-02-10T07:35:00Z

@blueorangutan package

blueorangutan · 2026-02-10T07:36:04Z

@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-02-10T08:18:38Z

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

weizhouapache · 2026-02-19T13:49:58Z

as we discussed in the comments of #12621, we need to work on another solution

can you work on the fix ? cc @jeanvetorello @sureshanaparti

sureshanaparti · 2026-03-10T10:34:38Z

@jeanvetorello can you rebase this PR with 4.22 branch, and address the outstanding comments.

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

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

…works addCidrAndGatewayForIpv4/Ipv6 (introduced by PR apache#11249) was called for all network types without checking if the network is Public. This caused comma-separated CIDRs to be stored on Public networks, which then triggered 'cidr is not formatted correctly' errors during VPC restart. Add TrafficType.Public guard in both the VLAN creation (addCidr) and VLAN deletion (removeCidr) paths in ConfigurationManagerImpl.

DaanHoogland · 2026-03-10T14:51:09Z

@blueorangutan package

blueorangutan · 2026-03-10T14:52: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 · 2026-03-10T15:03:44Z

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

weizhouapache · 2026-03-10T15:58:05Z

engine/schema/src/main/java/com/cloud/network/dao/NetworkVO.java

        }

-        return NetUtils.isNetworkAWithinNetworkB(cidr, that.cidr);
+        final String normalizedCidr = com.cloud.utils.StringUtils.getFirstValueFromCommaSeparatedString(cidr);


these changes should be reverted, I think ?

boring-cyborg bot added the component:networking label Feb 9, 2026

sureshanaparti requested review from Copilot and weizhouapache February 10, 2026 05:00

Copilot started reviewing on behalf of sureshanaparti February 10, 2026 05:01 View session

Copilot AI reviewed Feb 10, 2026

View reviewed changes

jean and others added 3 commits March 10, 2026 10:55

Update engine/schema/src/main/java/com/cloud/network/dao/NetworkVO.java

af5341f

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

jeanvetorello force-pushed the fix/vpc-restart-cidr-minimal branch from 5e43fac to c9e7a8b Compare March 10, 2026 13:58

boring-cyborg bot added the component:UI label Mar 10, 2026

jeanvetorello changed the base branch from main to 4.22 March 10, 2026 14:00

DaanHoogland linked an issue Mar 10, 2026 that may be closed by this pull request

VPC restart with cleanup fails when VPC has multiple public IP ranges from different VLANs — "cidr is not formatted correctly" #12621

Open

weizhouapache reviewed Mar 10, 2026

View reviewed changes

Conversation

jeanvetorello commented Feb 9, 2026

Description

Types of changes

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

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

Uh oh!

sureshanaparti commented Feb 10, 2026

Uh oh!

sureshanaparti commented Feb 10, 2026

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

sureshanaparti commented Feb 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DaanHoogland commented Feb 10, 2026

Uh oh!

blueorangutan commented Feb 10, 2026

Uh oh!

blueorangutan commented Feb 10, 2026

Uh oh!

weizhouapache commented Feb 19, 2026

Uh oh!

sureshanaparti commented Mar 10, 2026

Uh oh!

DaanHoogland commented Mar 10, 2026

Uh oh!

blueorangutan commented Mar 10, 2026

Uh oh!

blueorangutan commented Mar 10, 2026

Uh oh!

weizhouapache Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

codecov bot commented Feb 10, 2026 •

edited

Loading