VPC restart cleanup for Public networks with multi-CIDR data by jeanvetorello · Pull Request #12622 · apache/cloudstack

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

#12621
Fix VPC restart with multi-CIDR networks: handle comma-separated CIDR 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 addresses VPC restart failures during cleanup in environments where Public networks contain legacy aggregated network data (multiple values stored in network-level CIDR and gateway fields).

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 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.60%. Comparing base (59b6c32) to head (ca2c325).
⚠️ Report is 1 commits behind head on 4.22.

Files with missing lines	Patch %	Lines
.../cloud/configuration/ConfigurationManagerImpl.java	0.00%	7 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##               4.22   #12622   +/-   ##
=========================================
  Coverage     17.60%   17.60%           
- Complexity    15676    15677    +1     
=========================================
  Files          5918     5918           
  Lines        531667   531669    +2     
  Branches      65001    65002    +1     
=========================================
+ Hits          93617    93624    +7     
+ Misses       427491   427486    -5     
  Partials      10559    10559

Flag	Coverage Δ
uitests	`3.70% <ø> (ø)`
unittests	`18.68% <0.00%> (+<0.01%)`	⬆️

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.

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

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

…orkVO.java" This reverts commit af5341f.

…ted CIDR in NetworkVO.equals()" This reverts commit a68680a.

DaanHoogland · 2026-03-19T16:12:01Z

@blueorangutan package

blueorangutan · 2026-03-19T16:14:05Z

@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-19T17:23:21Z

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

blueorangutan · 2026-03-20T09:24:21Z

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

weizhouapache · 2026-03-20T09:36:46Z

@jeanvetorello
thanks for the update

can you update the PR title add description ?

Can you also add a SQL change to update some fields of Public network to NULL ? https://github.com/apache/cloudstack/blob/4.22/engine/schema/src/main/resources/META-INF/db/schema-42200to42210.sql

I think

        broadcast_uri: NULL
              gateway: NULL
                 cidr: NULL
          ip6_gateway: NULL
             ip6_cidr: NULL

weizhouapache

code lgtm

@jeanvetorello
can you plaese update the description as well ? thanks

github-actions · 2026-03-24T19:12:00Z

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

DaanHoogland · 2026-04-01T10:14:44Z

@weizhouapache, @jeanvetorello did update the title. Is this to satisfaction?]
@sureshanaparti, is this lgty now?

DaanHoogland · 2026-04-01T10:15:12Z

@blueorangutan package

blueorangutan · 2026-04-01T10:16:05Z

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

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-01T10:23:25Z

engine/schema/src/main/resources/META-INF/db/schema-42200to42210.sql

+	`cidr` = NULL,
+	`ip6_gateway` = NULL,
+	`ip6_cidr` = NULL
+WHERE `traffic_type` = 'Public';


This migration nulls cidr/gateway (and broadcast_uri) for all traffic_type='Public' networks. With the current NetworkVO.equals() implementation, two Public networks with cidr == null compare as equal, while NetworkVO.hashCode() is based on id; this violates the equals/hashCode contract and can break HashSet/Map behavior (and may become more likely after this UPDATE makes cidr null everywhere). Consider either (a) updating NetworkVO.equals()/hashCode() to be consistent (and to handle comma-separated CIDRs as described in the PR), or (b) narrowing this UPDATE to only sanitize rows that actually contain legacy comma-separated values so you don’t increase the number of cidr == null Public networks.

Suggested change

WHERE `traffic_type` = 'Public';

WHERE `traffic_type` = 'Public`

AND (

`cidr` LIKE '%,%'

OR `ip6_cidr` LIKE '%,%'

OR `gateway` LIKE '%,%'

OR `ip6_gateway` LIKE '%,%'

);

Copilot · 2026-04-01T10:23:25Z

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

+            final NetworkVO networkVO = _networkDao.findById(networkId);

-            if (ipv4) {
-                removeCidrAndGatewayForIpv4(networkId, deletedVlan);
-            } else if (ipv6) {
-                removeCidrAndGatewayForIpv6(networkId, deletedVlan);
+            if (networkVO != null && networkVO.getTrafficType() != TrafficType.Public) {
+                if (ipv4) {
+                    removeCidrAndGatewayForIpv4(networkId, deletedVlan);


networkVO is fetched here to check traffic type, but removeCidrAndGatewayForIpv4/Ipv6 immediately re-fetch the same NetworkVO again. To avoid the extra DB hit (and keep the code easier to follow), consider passing the already-fetched NetworkVO into the remove* helpers or moving the traffic-type guard into those helpers.

blueorangutan · 2026-04-01T11:12:55Z

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

DaanHoogland · 2026-04-01T13:33:19Z

@blueorangutan test

blueorangutan · 2026-04-01T13:34:06Z

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

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

engine/schema/src/main/java/com/cloud/network/dao/NetworkVO.java Outdated Show resolved Hide resolved

engine/schema/src/main/java/com/cloud/network/dao/NetworkVO.java Outdated Show resolved Hide resolved

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

engine/schema/src/main/java/com/cloud/network/dao/NetworkVO.java Outdated Show resolved Hide resolved

boring-cyborg bot added component:agent component:api component:build component:integration labels Mar 19, 2026

jeanvetorello force-pushed the fix/vpc-restart-cidr-minimal branch from a38f18b to c9e7a8b Compare March 19, 2026 15:48

jean added 2 commits March 19, 2026 13:04

Revert "Update engine/schema/src/main/java/com/cloud/network/dao/Netw…

ab3f662

…orkVO.java" This reverts commit af5341f.

Revert "Fix VPC restart with multi-CIDR networks: handle comma-separa…

1a22d35

…ted CIDR in NetworkVO.equals()" This reverts commit a68680a.

DaanHoogland changed the title ~~Fix VPC restart with multi-CIDR networks: handle comma-separated CIDR…~~ handle comma-separated CIDR in NetworkVO.equals() Mar 20, 2026

jeanvetorello changed the title ~~handle comma-separated CIDR in NetworkVO.equals()~~ VPC restart cleanup for Public networks with multi-CIDR data Mar 20, 2026

Sanitize legacy network-level addressing fields for Public networks

f6d618b

weizhouapache approved these changes Mar 20, 2026

View reviewed changes

github-actions bot added the status:has-conflicts label Mar 24, 2026

Merge branch '4.22' into fix/vpc-restart-cidr-minimal

ca2c325

DaanHoogland requested a review from Copilot April 1, 2026 10:14

Copilot started reviewing on behalf of DaanHoogland April 1, 2026 10:15 View session

Copilot AI reviewed Apr 1, 2026

View reviewed changes

github-actions bot removed the status:has-conflicts label Apr 1, 2026

-WHERE `traffic_type` = 'Public';
+WHERE `traffic_type` = 'Public`
+  AND (
+      `cidr` LIKE '%,%'
+      OR `ip6_cidr` LIKE '%,%'
+      OR `gateway` LIKE '%,%'
+      OR `ip6_gateway` LIKE '%,%'
+  );

Conversation

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

Uh oh!

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!

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!

Uh oh!

DaanHoogland commented Mar 19, 2026

Uh oh!

blueorangutan commented Mar 19, 2026

Uh oh!

blueorangutan commented Mar 19, 2026

Uh oh!

blueorangutan commented Mar 20, 2026

Uh oh!

weizhouapache commented Mar 20, 2026

Uh oh!

weizhouapache left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 24, 2026

Uh oh!

DaanHoogland commented Apr 1, 2026

Uh oh!

DaanHoogland commented Apr 1, 2026

Uh oh!

blueorangutan commented Apr 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Apr 1, 2026

Uh oh!

DaanHoogland commented Apr 1, 2026

Uh oh!

blueorangutan commented Apr 1, 2026

Uh oh!

jeanvetorello commented Feb 9, 2026 •

edited

Loading

codecov bot commented Feb 10, 2026 •

edited

Loading