Skip to content

Do not use direct IP allocation for isolated networks with SpecifyIpRanges - #14087

Open
nagaboinaramgopal wants to merge 1 commit into
apache:mainfrom
nagaboinaramgopal:pr/fix-guestnetworkguru-isolated-directip
Open

Do not use direct IP allocation for isolated networks with SpecifyIpRanges#14087
nagaboinaramgopal wants to merge 1 commit into
apache:mainfrom
nagaboinaramgopal:pr/fix-guestnetworkguru-isolated-directip

Conversation

@nagaboinaramgopal

Copy link
Copy Markdown
Contributor

Description

GuestNetworkGuru.allocate() called allocateDirectIp() for any network that has SpecifyIpRanges set. That path allocates from the zone's VLAN public pools and is only correct for shared networks. An isolated network with SpecifyIpRanges then tried to take an address from the public pools instead of its own CIDR, and failed with InsufficientAddressCapacityException once those pools were exhausted, even though the network's own CIDR still had free addresses.

This gates the direct allocation on the shared guest type, so isolated networks fall through to their normal CIDR allocation. Shared network behaviour is unchanged. This matches the fix suggested by weizhou in the issue.

Fixes: #12772

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

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

N/A

How Has This Been Tested?

Added unit tests on the base guru's allocate(). For an isolated network with SpecifyIpRanges the test checks that allocateDirectIp is not called and the address comes from the network CIDR instead; before this change that test fails because allocateDirectIp is called. For a shared network with SpecifyIpRanges the test checks that allocateDirectIp is still called, so shared allocation is unchanged.

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

Kept a shared network test to make sure the direct allocation path is untouched for shared networks. The only behaviour that changes is an isolated network with SpecifyIpRanges, which now uses its own CIDR the same way an isolated network without SpecifyIpRanges already does.

GuestNetworkGuru.allocate() called allocateDirectIp() whenever a network had
SpecifyIpRanges set, but that path allocates from the zone's VLAN public pools
and is only correct for shared networks. An isolated network with
SpecifyIpRanges then failed to get an address from its own CIDR and threw
InsufficientAddressCapacityException once the public pools were exhausted.

Gate the direct allocation on the shared guest type so isolated networks fall
through to their normal CIDR allocation.

Fixes: apache#12772
@codecov

codecov Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 19.79%. Comparing base (d87c804) to head (f1bd135).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
.../java/com/cloud/network/guru/GuestNetworkGuru.java 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##               main   #14087   +/-   ##
=========================================
  Coverage     19.78%   19.79%           
- Complexity    19995    19998    +3     
=========================================
  Files          6371     6371           
  Lines        575909   575909           
  Branches      70509    70509           
=========================================
+ Hits         113950   113975   +25     
+ Misses       449526   449506   -20     
+ Partials      12433    12428    -5     
Flag Coverage Δ
uitests 3.52% <ø> (ø)
unittests 21.06% <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 Harness.
📢 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.

@weizhouapache

Copy link
Copy Markdown
Member

@nagaboinaramgopal
have you reproduced the issue and verified the fix ?

@nagaboinaramgopal

Copy link
Copy Markdown
Contributor Author

@nagaboinaramgopal have you reproduced the issue and verified the fix ?

Yes, reproduced and verified live on a KVM advanced zone.

Repro steps:

  1. Create an Isolated network offering with SpecifyIpRanges=true and no SourceNat (Dhcp and Dns via VirtualRouter), and enable it.
  2. Create an Isolated network from that offering with a /24 CIDR but a small direct IP range, e.g. gateway 10.199.50.1, netmask 255.255.255.0, startip 10.199.50.10, endip 10.199.50.13 (4 addresses).
  3. Deploy VMs into it until the router plus the VMs use up that 4 address range.
  4. Deploy one more VM.

Results, before this change:

iso-b1   Running   ip=10.199.50.11
iso-b2   Running   ip=10.199.50.13
iso-b3   Running   ip=10.199.50.12
(router holds 10.199.50.10, so the .10-.13 direct range is now full)
iso-b4   FAILED    InsufficientAddressCapacityException

Step 4 fails even though the /24 CIDR still has plenty of free addresses, because allocate() takes the allocateDirectIp path (direct pool) for the Isolated network and never uses the network CIDR:

    ERROR [c.c.v.UserVmManagerImpl] error during resource reservation and allocation com.cloud.exception.InsufficientAddressCapacityException: Insufficient address capacity Scope=interface com.cloud.dc.DataCenter; id=1
        at com.cloud.network.IpAddressManagerImpl.allocateDirectIp(IpAddressManagerImpl.java:2436)
        at com.cloud.network.guru.GuestNetworkGuru.allocate(GuestNetworkGuru.java:446)
        at com.cloud.network.guru.ExternalGuestNetworkGuru.allocate(ExternalGuestNetworkGuru.java:262)
        at org.apache.cloudstack.engine.orchestration.NetworkOrchestrator.allocateNic(...)

Results, after this change (same network, direct range still exhausted):

iso-after2   Running   ip=10.199.50.9

Step 4 now succeeds, and the VM gets 10.199.50.9 from the /24 CIDR, outside the exhausted 10.199.50.10-13 direct range, so allocation comes from the network CIDR as expected. Shared networks are unchanged. The unit test covers the same behaviour: for an Isolated network with SpecifyIpRanges, allocate() no longer calls allocateDirectIp.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: GuestNetworkGuru.allocate() incorrectly calls allocateDirectIp() for Isolated networks with SpecifyIpRanges=true

2 participants