Fix for Vlan doesn't match issue while adding IP range for the shared network without any IP range by sureshanaparti · Pull Request #10837 · apache/cloudstack

sureshanaparti · 2025-05-08T15:10:40Z

Description

This PR fixes Vlan doesn't match issue while adding IP range for the shared network without any IP range.

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)
build/CI
test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

Before Fix:

SharedNetwork_AddIpRange_VlanMismatchIssue

After Fix:

How Has This Been Tested?

Create a shared network, delete the ip range and add new ip range.

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

sureshanaparti · 2025-05-08T15:11:14Z

@blueorangutan package

blueorangutan · 2025-05-08T15:12: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.

… network without any IP range

sureshanaparti · 2025-05-08T15:17:10Z

@blueorangutan package

blueorangutan · 2025-05-08T15:18:03Z

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

codecov · 2025-05-08T15:24:58Z

Codecov Report

Attention: Patch coverage is 28.88889% with 32 lines in your changes missing coverage. Please review.

Project coverage is 15.17%. Comparing base (f0838cd) to head (deb77a1).
Report is 15 commits behind head on 4.19.

Files with missing lines	Patch %	Lines
.../cloud/configuration/ConfigurationManagerImpl.java	28.88%	27 Missing and 5 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##               4.19   #10837    +/-   ##
==========================================
  Coverage     15.17%   15.17%            
- Complexity    11339    11349    +10     
==========================================
  Files          5414     5415     +1     
  Lines        475185   475591   +406     
  Branches      57991    58037    +46     
==========================================
+ Hits          72105    72167    +62     
- Misses       395018   395351   +333     
- Partials       8062     8073    +11

Flag	Coverage Δ
uitests	`4.28% <ø> (-0.01%)`	⬇️
unittests	`15.90% <28.88%> (+<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.

blueorangutan · 2025-05-08T16:23:45Z

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

sureshanaparti · 2025-05-09T06:49:10Z

@blueorangutan test

blueorangutan · 2025-05-09T06:50:03Z

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

weizhouapache

code lgtm

not tested yet

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

blueorangutan · 2025-05-09T22:57:37Z

[SF] Trillian test result (tid-13244)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 55177 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr10837-t13244-kvm-ol8.zip
Smoke tests completed. 132 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_03_create_iso_with_checksum_md5	`Error`	66.78	test_iso.py

blueorangutan · 2025-05-12T11:42:44Z

[SF] Trillian Build Failed (tid-13275)

weizhouapache · 2025-05-13T10:16:03Z

@blueorangutan test

blueorangutan · 2025-05-13T10:18:03Z

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

blueorangutan · 2025-05-14T00:21:23Z

[SF] Trillian test result (tid-13287)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 47722 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr10837-t13287-kvm-ol8.zip
Smoke tests completed. 133 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

DaanHoogland · 2025-05-14T06:05:53Z

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

+        Long networkId = cmd.getNetworkID();
+        Long physicalNetworkId = cmd.getPhysicalNetworkId();
+
+        // Verify that network exists
+        Network network = null;
+        if (networkId != null) {
+            network = _networkDao.findById(networkId);
+            if (network == null) {
+                throw new InvalidParameterValueException("Unable to find network by id " + networkId);
+            } else {
+                zoneId = network.getDataCenterId();
+                physicalNetworkId = network.getPhysicalNetworkId();
+            }
+        }


can be a seperate method

DaanHoogland · 2025-05-14T06:06:11Z

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

+        if (StringUtils.isBlank(vlanId)) {
+            vlanId = Vlan.UNTAGGED;
+            if (network != null & network.getTrafficType() == TrafficType.Guest) {
+                boolean connectivityWithoutVlan = isConnectivityWithoutVlan(network);
+                vlanId = getNetworkVlanId(network, connectivityWithoutVlan);
+            }
+        }


can be an extra method

DaanHoogland · 2025-05-14T06:07:08Z

@blueorangutan test keepEnv

blueorangutan · 2025-05-14T06:08:05Z

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

blueorangutan · 2025-05-14T22:13:16Z

[SF] Trillian test result (tid-13302)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 55096 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr10837-t13302-kvm-ol8.zip
Smoke tests completed. 132 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_10_vpc_tier_kubernetes_cluster	`Failure`	3749.69	test_kubernetes_clusters.py

sureshanaparti · 2025-05-15T07:16:51Z

@blueorangutan package

blueorangutan · 2025-05-15T07:18: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-05-15T08:19:15Z

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

kiranchavala

LGTM

Create a shared network by providing IP range and a vlan > network state in setup
Delete the shared network range
Add a IP range again
Ip range added successfully

before fix exception was thrown

After fix no exception is thrown

DaanHoogland · 2025-05-15T13:58:19Z

@blueorangutan test

blueorangutan · 2025-05-15T14:00:03Z

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

blueorangutan · 2025-05-16T05:34:03Z

[SF] Trillian test result (tid-13321)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 53102 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr10837-t13321-kvm-ol8.zip
Smoke tests completed. 132 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_02_list_cpvm_vm	`Failure`	0.06	test_ssvm.py
test_04_cpvm_internals	`Failure`	0.05	test_ssvm.py

… network without any IP range (apache#10837)

boring-cyborg bot added the component:api label May 8, 2025

Fix for Vlan doesn't match issue while adding IP range for the shared…

4efa3af

… network without any IP range

sureshanaparti force-pushed the shared-network-vlan-doesnt-match-issue-while-adding-ip-range branch from c67649a to 4efa3af Compare May 8, 2025 15:13

sureshanaparti added this to the 4.19.3 milestone May 8, 2025

sureshanaparti requested a review from weizhouapache May 8, 2025 15:29

sureshanaparti added the status:needs-testing label May 8, 2025

sureshanaparti marked this pull request as ready for review May 9, 2025 06:47

weizhouapache approved these changes May 9, 2025

View reviewed changes

weizhouapache reviewed May 9, 2025

View reviewed changes

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java Show resolved Hide resolved

DaanHoogland reviewed May 14, 2025

View reviewed changes

review comments / code refactoring

deb77a1

kiranchavala approved these changes May 15, 2025

View reviewed changes

kiranchavala removed the status:needs-testing label May 15, 2025

DaanHoogland added the status:waiting-for-smoketest-results label May 15, 2025

DaanHoogland removed the status:waiting-for-smoketest-results label May 16, 2025

DaanHoogland merged commit 13ab8a0 into apache:4.19 May 16, 2025
26 checks passed

dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Jun 19, 2025

Fix for Vlan doesn't match issue while adding IP range for the shared…

89fa4d4

… network without any IP range (apache#10837)

DaanHoogland deleted the shared-network-vlan-doesnt-match-issue-while-adding-ip-range branch July 15, 2025 12:09

DaanHoogland linked an issue Jul 29, 2025 that may be closed by this pull request

Enable changing the CIDR on shared networks when no IP ranges are defined #10459

Closed

weizhouapache mentioned this pull request Aug 22, 2025

UI: Shared network, when no IP ranges are defined user needs to specify a VLAN, but there's no such option in the UI #10460

Closed

Conversation

sureshanaparti commented May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

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 May 8, 2025

Uh oh!

blueorangutan commented May 8, 2025

Uh oh!

sureshanaparti commented May 8, 2025

Uh oh!

blueorangutan commented May 8, 2025

Uh oh!

codecov bot commented May 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

blueorangutan commented May 8, 2025

Uh oh!

sureshanaparti commented May 9, 2025

Uh oh!

blueorangutan commented May 9, 2025

Uh oh!

weizhouapache left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

blueorangutan commented May 9, 2025

Uh oh!

blueorangutan commented May 12, 2025

Uh oh!

weizhouapache commented May 13, 2025

Uh oh!

blueorangutan commented May 13, 2025

Uh oh!

blueorangutan commented May 14, 2025

Uh oh!

DaanHoogland May 14, 2025

Choose a reason for hiding this comment

Uh oh!

sureshanaparti May 15, 2025

Choose a reason for hiding this comment

Uh oh!

DaanHoogland May 14, 2025

Choose a reason for hiding this comment

Uh oh!

sureshanaparti May 15, 2025

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented May 14, 2025

Uh oh!

blueorangutan commented May 14, 2025

Uh oh!

blueorangutan commented May 14, 2025

Uh oh!

sureshanaparti commented May 15, 2025

Uh oh!

blueorangutan commented May 15, 2025

Uh oh!

blueorangutan commented May 15, 2025

Uh oh!

kiranchavala left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented May 15, 2025

Uh oh!

blueorangutan commented May 15, 2025

Uh oh!

blueorangutan commented May 16, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

sureshanaparti commented May 8, 2025 •

edited

Loading

codecov bot commented May 8, 2025 •

edited

Loading