From cc80ffe6ceb86f52c65bd83a428b89c8405af78a Mon Sep 17 00:00:00 2001 From: Prashanth Reddy Date: Fri, 16 Oct 2026 09:42:21 +0000 Subject: [PATCH 1/2] network: scope routed network/VPC subnets to a zone The ip4_guest_subnet_network_map table had no zone column, so the subnet bookkeeping for routed networks and VPCs was global. Creating a routed VPC or network with a CIDR already used in another zone failed with "The subnet is in use", even though the two zones are independent. Add a nullable data_center_id column to ip4_guest_subnet_network_map and scope the subnet lookup and the parentless overlap check by zone, so the same CIDR can be used once per zone. Existing rows are backfilled from their parent zone subnet, VPC or network. Fixes #14419 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017rnv8vT6LQSKfwkzp8tvg9 --- .../network/Ipv4GuestSubnetNetworkMapVO.java | 14 +++++++- .../dao/Ipv4GuestSubnetNetworkMapDao.java | 4 +-- .../dao/Ipv4GuestSubnetNetworkMapDaoImpl.java | 8 +++-- .../META-INF/db/schema-42210to42220.sql | 20 +++++++++++ .../network/RoutedIpv4ManagerImpl.java | 20 +++++------ .../network/RoutedIpv4ManagerImplTest.java | 35 ++++++++++++++----- 6 files changed, 77 insertions(+), 24 deletions(-) diff --git a/engine/schema/src/main/java/org/apache/cloudstack/network/Ipv4GuestSubnetNetworkMapVO.java b/engine/schema/src/main/java/org/apache/cloudstack/network/Ipv4GuestSubnetNetworkMapVO.java index cc726ba3d357..7e2e6f051158 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/network/Ipv4GuestSubnetNetworkMapVO.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/network/Ipv4GuestSubnetNetworkMapVO.java @@ -46,6 +46,9 @@ public class Ipv4GuestSubnetNetworkMapVO implements Ipv4GuestSubnetNetworkMap { @Column(name = "parent_id") private Long parentId; + @Column(name = "data_center_id") + private Long dataCenterId; + @Column(name = "subnet") private String subnet; @@ -72,8 +75,9 @@ protected Ipv4GuestSubnetNetworkMapVO() { uuid = UUID.randomUUID().toString(); } - protected Ipv4GuestSubnetNetworkMapVO(Long parentId, String subnet, Long networkId, Ipv4GuestSubnetNetworkMap.State state) { + protected Ipv4GuestSubnetNetworkMapVO(Long parentId, Long dataCenterId, String subnet, Long networkId, Ipv4GuestSubnetNetworkMap.State state) { this.parentId = parentId; + this.dataCenterId = dataCenterId; this.subnet = subnet; this.networkId = networkId; this.state = state; @@ -95,6 +99,14 @@ public Long getParentId() { return parentId; } + public Long getDataCenterId() { + return dataCenterId; + } + + public void setDataCenterId(Long dataCenterId) { + this.dataCenterId = dataCenterId; + } + @Override public String getSubnet() { return subnet; diff --git a/engine/schema/src/main/java/org/apache/cloudstack/network/dao/Ipv4GuestSubnetNetworkMapDao.java b/engine/schema/src/main/java/org/apache/cloudstack/network/dao/Ipv4GuestSubnetNetworkMapDao.java index c3f860009db9..be149003a964 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/network/dao/Ipv4GuestSubnetNetworkMapDao.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/network/dao/Ipv4GuestSubnetNetworkMapDao.java @@ -31,8 +31,8 @@ public interface Ipv4GuestSubnetNetworkMapDao extends GenericDao findSubnetsInStates(Ipv4GuestSubnetNetworkMap.State... states); void deleteByParentId(long parentId); - List listAllNoParent(); + List listAllNoParentByDataCenterId(Long dataCenterId); } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/network/dao/Ipv4GuestSubnetNetworkMapDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/network/dao/Ipv4GuestSubnetNetworkMapDaoImpl.java index 95e53448907e..7e778e16b409 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/network/dao/Ipv4GuestSubnetNetworkMapDaoImpl.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/network/dao/Ipv4GuestSubnetNetworkMapDaoImpl.java @@ -63,6 +63,7 @@ public void init() { ParentIdSearch.done(); NoParentSearch = createSearchBuilder(); NoParentSearch.and("parentId", NoParentSearch.entity().getParentId(), SearchCriteria.Op.NULL); + NoParentSearch.and("dataCenterId", NoParentSearch.entity().getDataCenterId(), SearchCriteria.Op.EQ); NoParentSearch.done(); NetworkIdSearch = createSearchBuilder(); NetworkIdSearch.and("networkId", NetworkIdSearch.entity().getNetworkId(), SearchCriteria.Op.EQ); @@ -70,6 +71,7 @@ public void init() { NetworkIdSearch.done(); SubnetSearch = createSearchBuilder(); SubnetSearch.and("subnet", SubnetSearch.entity().getSubnet(), SearchCriteria.Op.EQ); + SubnetSearch.and("dataCenterId", SubnetSearch.entity().getDataCenterId(), SearchCriteria.Op.EQ); SubnetSearch.done(); StatesSearch = createSearchBuilder(); StatesSearch.and("state", StatesSearch.entity().getState(), SearchCriteria.Op.IN); @@ -142,9 +144,10 @@ public Ipv4GuestSubnetNetworkMapVO findByVpcId(long vpcId) { } @Override - public Ipv4GuestSubnetNetworkMapVO findBySubnet(String subnet) { + public Ipv4GuestSubnetNetworkMapVO findBySubnetAndDataCenterId(String subnet, Long dataCenterId) { SearchCriteria sc = SubnetSearch.create(); sc.setParameters("subnet", subnet); + sc.setParameters("dataCenterId", dataCenterId); return findOneBy(sc); } @@ -163,8 +166,9 @@ public void deleteByParentId(long parentId) { } @Override - public List listAllNoParent() { + public List listAllNoParentByDataCenterId(Long dataCenterId) { SearchCriteria sc = NoParentSearch.create(); + sc.setParameters("dataCenterId", dataCenterId); return listBy(sc, null); } } diff --git a/engine/schema/src/main/resources/META-INF/db/schema-42210to42220.sql b/engine/schema/src/main/resources/META-INF/db/schema-42210to42220.sql index 7a8ee864eac3..40aa1f6a6649 100644 --- a/engine/schema/src/main/resources/META-INF/db/schema-42210to42220.sql +++ b/engine/schema/src/main/resources/META-INF/db/schema-42210to42220.sql @@ -31,3 +31,23 @@ UPDATE op_dc_vnet_alloc a a.reservation_id = NULL, a.taken = NULL WHERE n.removed IS NOT NULL; + +-- Scope routed network/VPC subnets to a zone, so that the same CIDR can be used in more than one zone. See #14419. +CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.ip4_guest_subnet_network_map', 'data_center_id', 'bigint unsigned DEFAULT NULL COMMENT "zone it belongs to"'); + +UPDATE `cloud`.`ip4_guest_subnet_network_map` m + JOIN `cloud`.`dc_ip4_guest_subnets` p ON p.id = m.parent_id + SET m.data_center_id = p.data_center_id +WHERE m.data_center_id IS NULL; + +UPDATE `cloud`.`ip4_guest_subnet_network_map` m + JOIN `cloud`.`vpc` v ON v.id = m.vpc_id + SET m.data_center_id = v.zone_id +WHERE m.data_center_id IS NULL; + +UPDATE `cloud`.`ip4_guest_subnet_network_map` m + JOIN `cloud`.`networks` n ON n.id = m.network_id + SET m.data_center_id = n.data_center_id +WHERE m.data_center_id IS NULL; + +CALL `cloud`.`IDEMPOTENT_ADD_FOREIGN_KEY`('cloud.ip4_guest_subnet_network_map', 'fk_ip4_guest_subnet_network_map__data_center_id', '(`data_center_id`)', '`data_center`(`id`)'); diff --git a/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java b/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java index ee888a61adc5..ec92051e0da7 100644 --- a/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java @@ -650,7 +650,7 @@ protected Ipv4GuestSubnetNetworkMap getOrCreateIpv4SubnetForGuestNetworkOrVpcInt } protected void getOrCreateIpv4SubnetForGuestNetworkOrVpcInternal(String networkCidr, Long ownerDomainId, Long ownerAccountId, Long zoneId) { - Ipv4GuestSubnetNetworkMapVO subnetMap = ipv4GuestSubnetNetworkMapDao.findBySubnet(networkCidr); + Ipv4GuestSubnetNetworkMapVO subnetMap = ipv4GuestSubnetNetworkMapDao.findBySubnetAndDataCenterId(networkCidr, zoneId); if (subnetMap != null) { // check if the subnet is in use if (subnetMap.getNetworkId() != null || subnetMap.getVpcId() != null) { @@ -672,7 +672,7 @@ protected void getOrCreateIpv4SubnetForGuestNetworkOrVpcInternal(String networkC // Create new record without networkId final Long parentId = parent != null ? parent.getId() : null; - subnetMap = new Ipv4GuestSubnetNetworkMapVO(parentId, NetUtils.transformCidr(networkCidr), null, State.Free); + subnetMap = new Ipv4GuestSubnetNetworkMapVO(parentId, zoneId, NetUtils.transformCidr(networkCidr), null, State.Free); ipv4GuestSubnetNetworkMapDao.persist(subnetMap); } @@ -698,7 +698,7 @@ protected DataCenterIpv4GuestSubnet getParentOfNetworkCidr(Long zoneId, String n } } // check conflicts - List subnetsForNetworkNoParents = ipv4GuestSubnetNetworkMapDao.listAllNoParent(); + List subnetsForNetworkNoParents = ipv4GuestSubnetNetworkMapDao.listAllNoParentByDataCenterId(zoneId); checkConflicts(subnetsForNetworkNoParents, networkCidr); return null; } @@ -730,7 +730,7 @@ private List getZoneSubnetsForAccount(long domainId protected Ipv4GuestSubnetNetworkMap createIpv4SubnetFromParentSubnet(DataCenterIpv4GuestSubnet parent, Integer networkCidrSize) { String networkCidr = createIpv4SubnetStringFromParentSubnet(parent, networkCidrSize); // create DB record - Ipv4GuestSubnetNetworkMapVO subnetMap = new Ipv4GuestSubnetNetworkMapVO(parent.getId(), NetUtils.transformCidr(networkCidr), null, State.Free); + Ipv4GuestSubnetNetworkMapVO subnetMap = new Ipv4GuestSubnetNetworkMapVO(parent.getId(), parent.getDataCenterId(), NetUtils.transformCidr(networkCidr), null, State.Free); return ipv4GuestSubnetNetworkMapDao.persist(subnetMap); } @@ -767,19 +767,19 @@ protected String createIpv4SubnetStringFromParentSubnet(DataCenterIpv4GuestSubne } // Allocate a subnet automatically - String networkCidr = getFreeNetworkCidr(subnetsInFreeIpRanges, networkCidrSize); + String networkCidr = getFreeNetworkCidr(subnetsInFreeIpRanges, networkCidrSize, parent.getDataCenterId()); if (networkCidr == null) { throw new InvalidParameterValueException("Failed to automatically allocate a subnet with specified cidrsize"); } return networkCidr; } - private String getFreeNetworkCidr(List> subnetsInFreeIpRanges, int networkCidrSize) { + private String getFreeNetworkCidr(List> subnetsInFreeIpRanges, int networkCidrSize, Long dataCenterId) { for (int cidrSize = networkCidrSize; cidrSize >= 1; cidrSize--) { for (Pair freeSubnet : subnetsInFreeIpRanges) { if (freeSubnet.second().equals(cidrSize)) { String networkCidr = String.format("%s/%s", NetUtils.long2Ip(freeSubnet.first()), networkCidrSize); - if (ipv4GuestSubnetNetworkMapDao.findBySubnet(networkCidr) == null) { + if (ipv4GuestSubnetNetworkMapDao.findBySubnetAndDataCenterId(networkCidr, dataCenterId) == null) { return networkCidr; } } @@ -798,7 +798,7 @@ protected Ipv4GuestSubnetNetworkMap createIpv4SubnetFromParentSubnet(DataCenterI checkConflicts(existingSubnets, networkCidr); // create DB record - Ipv4GuestSubnetNetworkMapVO subnetMap = new Ipv4GuestSubnetNetworkMapVO(parent.getId(), NetUtils.transformCidr(networkCidr), null, State.Free); + Ipv4GuestSubnetNetworkMapVO subnetMap = new Ipv4GuestSubnetNetworkMapVO(parent.getId(), parent.getDataCenterId(), NetUtils.transformCidr(networkCidr), null, State.Free); return ipv4GuestSubnetNetworkMapDao.persist(subnetMap); } @@ -807,7 +807,7 @@ public void assignIpv4SubnetToNetwork(Network network) { if (network == null || network.getCidr() == null) { return; } - Ipv4GuestSubnetNetworkMapVO subnetMap = ipv4GuestSubnetNetworkMapDao.findBySubnet(network.getCidr()); + Ipv4GuestSubnetNetworkMapVO subnetMap = ipv4GuestSubnetNetworkMapDao.findBySubnetAndDataCenterId(network.getCidr(), network.getDataCenterId()); if (subnetMap != null) { if (network.getId() > 0L) { subnetMap.setNetworkId(network.getId()); @@ -823,7 +823,7 @@ public void assignIpv4SubnetToVpc(Vpc vpc) { if (vpc == null || vpc.getCidr() == null) { return; } - Ipv4GuestSubnetNetworkMapVO subnetMap = ipv4GuestSubnetNetworkMapDao.findBySubnet(vpc.getCidr()); + Ipv4GuestSubnetNetworkMapVO subnetMap = ipv4GuestSubnetNetworkMapDao.findBySubnetAndDataCenterId(vpc.getCidr(), vpc.getZoneId()); if (subnetMap != null) { if (vpc != null && vpc.getId() > 0L) { subnetMap.setVpcId(vpc.getId()); diff --git a/server/src/test/java/org/apache/cloudstack/network/RoutedIpv4ManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/network/RoutedIpv4ManagerImplTest.java index 81110dd3f539..3615bf2552b9 100644 --- a/server/src/test/java/org/apache/cloudstack/network/RoutedIpv4ManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/network/RoutedIpv4ManagerImplTest.java @@ -89,6 +89,7 @@ import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.Mockito; @@ -498,7 +499,7 @@ public void testCreateIpv4SubnetForGuestNetworkResponse() { @Test public void testGetOrCreateIpv4SubnetForGuestNetworkOrVpcInternalByCidr1() { - when(ipv4GuestSubnetNetworkMapDao.findBySubnet(subnet)).thenReturn(ipv4GuestSubnetNetworkMap); + when(ipv4GuestSubnetNetworkMapDao.findBySubnetAndDataCenterId(subnet, zoneId)).thenReturn(ipv4GuestSubnetNetworkMap); when(ipv4GuestSubnetNetworkMap.getNetworkId()).thenReturn(null); when(ipv4GuestSubnetNetworkMap.getVpcId()).thenReturn(null); when(ipv4GuestSubnetNetworkMap.getParentId()).thenReturn(zoneSubnetId); @@ -511,7 +512,7 @@ public void testGetOrCreateIpv4SubnetForGuestNetworkOrVpcInternalByCidr1() { @Test public void testGetOrCreateIpv4SubnetForGuestNetworkOrVpcInternalByCidr2() { - when(ipv4GuestSubnetNetworkMapDao.findBySubnet(subnet)).thenReturn(null); + when(ipv4GuestSubnetNetworkMapDao.findBySubnetAndDataCenterId(subnet, zoneId)).thenReturn(null); routedIpv4Manager.getOrCreateIpv4SubnetForGuestNetworkOrVpcInternal(subnet, domainId, accountId, zoneId); @@ -520,7 +521,7 @@ public void testGetOrCreateIpv4SubnetForGuestNetworkOrVpcInternalByCidr2() { @Test public void testGetOrCreateIpv4SubnetForGuestNetworkOrVpcInternalByCidr3() { - when(ipv4GuestSubnetNetworkMapDao.findBySubnet(subnet)).thenReturn(null); + when(ipv4GuestSubnetNetworkMapDao.findBySubnetAndDataCenterId(subnet, zoneId)).thenReturn(null); when(routedIpv4Manager.getParentOfNetworkCidr(zoneId, subnet)).thenReturn(subnetVO); when(subnetVO.getDomainId()).thenReturn(domainId); when(subnetVO.getAccountId()).thenReturn(accountId); @@ -530,6 +531,20 @@ public void testGetOrCreateIpv4SubnetForGuestNetworkOrVpcInternalByCidr3() { verify(ipv4GuestSubnetNetworkMapDao).persist(any(Ipv4GuestSubnetNetworkMapVO.class)); } + @Test + public void testGetOrCreateIpv4SubnetForGuestNetworkOrVpcInternalByCidrInAnotherZone() { + Long otherZoneId = zoneId + 1L; + // The same subnet is already allocated in another zone, which must not block this zone + when(ipv4GuestSubnetNetworkMapDao.findBySubnetAndDataCenterId(subnet, otherZoneId)).thenReturn(null); + + routedIpv4Manager.getOrCreateIpv4SubnetForGuestNetworkOrVpcInternal(subnet, domainId, accountId, otherZoneId); + + ArgumentCaptor captor = ArgumentCaptor.forClass(Ipv4GuestSubnetNetworkMapVO.class); + verify(ipv4GuestSubnetNetworkMapDao).persist(captor.capture()); + Assert.assertEquals(otherZoneId, captor.getValue().getDataCenterId()); + Assert.assertEquals(subnet, captor.getValue().getSubnet()); + } + @Test public void testGetOrCreateIpv4SubnetForGuestNetworkOrVpcInternalByCidrSize1() { Ipv4GuestSubnetNetworkMap result = routedIpv4Manager.getOrCreateIpv4SubnetForGuestNetworkOrVpcInternal(cidrSize, domainId, accountId, zoneId); @@ -629,8 +644,8 @@ public void testCreateIpv4SubnetFromParentSubnetByCidrSize() { when(subnetVO.getId()).thenReturn(zoneSubnetId); when(dataCenterIpv4GuestSubnetDao.findById(zoneSubnetId)).thenReturn(subnetVO); - Ipv4GuestSubnetNetworkMapVO ipv4GuestSubnetNetworkMap1 = new Ipv4GuestSubnetNetworkMapVO(zoneSubnetId, "172.16.1.0/28", null, Ipv4GuestSubnetNetworkMap.State.Free); - Ipv4GuestSubnetNetworkMapVO ipv4GuestSubnetNetworkMap2 = new Ipv4GuestSubnetNetworkMapVO(zoneSubnetId, "172.16.1.64/28", null, Ipv4GuestSubnetNetworkMap.State.Free); + Ipv4GuestSubnetNetworkMapVO ipv4GuestSubnetNetworkMap1 = new Ipv4GuestSubnetNetworkMapVO(zoneSubnetId, zoneId, "172.16.1.0/28", null, Ipv4GuestSubnetNetworkMap.State.Free); + Ipv4GuestSubnetNetworkMapVO ipv4GuestSubnetNetworkMap2 = new Ipv4GuestSubnetNetworkMapVO(zoneSubnetId, zoneId, "172.16.1.64/28", null, Ipv4GuestSubnetNetworkMap.State.Free); when(ipv4GuestSubnetNetworkMapDao.listByParent(zoneSubnetId)).thenReturn(Arrays.asList(ipv4GuestSubnetNetworkMap1, ipv4GuestSubnetNetworkMap2)); when(subnetVO.getSubnet()).thenReturn("172.16.1.0/24"); @@ -643,8 +658,8 @@ public void testCreateIpv4SubnetFromParentSubnetByCidrSize() { networkCidr = routedIpv4Manager.createIpv4SubnetStringFromParentSubnet(subnetVO, 25); Assert.assertEquals("172.16.1.128/25", networkCidr); - Ipv4GuestSubnetNetworkMapVO ipv4GuestSubnetNetworkMap3 = new Ipv4GuestSubnetNetworkMapVO(zoneSubnetId, "172.16.1.16/28", null, Ipv4GuestSubnetNetworkMap.State.Free); - Ipv4GuestSubnetNetworkMapVO ipv4GuestSubnetNetworkMap4 = new Ipv4GuestSubnetNetworkMapVO(zoneSubnetId, "172.16.1.128/28", null, Ipv4GuestSubnetNetworkMap.State.Free); + Ipv4GuestSubnetNetworkMapVO ipv4GuestSubnetNetworkMap3 = new Ipv4GuestSubnetNetworkMapVO(zoneSubnetId, zoneId, "172.16.1.16/28", null, Ipv4GuestSubnetNetworkMap.State.Free); + Ipv4GuestSubnetNetworkMapVO ipv4GuestSubnetNetworkMap4 = new Ipv4GuestSubnetNetworkMapVO(zoneSubnetId, zoneId, "172.16.1.128/28", null, Ipv4GuestSubnetNetworkMap.State.Free); when(ipv4GuestSubnetNetworkMapDao.listByParent(zoneSubnetId)).thenReturn(Arrays.asList(ipv4GuestSubnetNetworkMap1, ipv4GuestSubnetNetworkMap2, ipv4GuestSubnetNetworkMap3, ipv4GuestSubnetNetworkMap4)); networkCidr = routedIpv4Manager.createIpv4SubnetStringFromParentSubnet(subnetVO, 28); @@ -665,7 +680,8 @@ public void testCreateIpv4SubnetFromParentSubnetByCidrSize() { public void testAssignIpv4SubnetToNetwork() { when(network.getId()).thenReturn(networkId); when(network.getCidr()).thenReturn(subnet); - when(ipv4GuestSubnetNetworkMapDao.findBySubnet(subnet)).thenReturn(ipv4GuestSubnetNetworkMap); + when(network.getDataCenterId()).thenReturn(zoneId); + when(ipv4GuestSubnetNetworkMapDao.findBySubnetAndDataCenterId(subnet, zoneId)).thenReturn(ipv4GuestSubnetNetworkMap); when(ipv4GuestSubnetNetworkMap.getId()).thenReturn(ipv4GuestSubnetNetworkMapId); routedIpv4Manager.assignIpv4SubnetToNetwork(network); @@ -677,7 +693,8 @@ public void testAssignIpv4SubnetToNetwork() { public void testAssignIpv4SubnetToVpc() { when(vpc.getId()).thenReturn(vpcId); when(vpc.getCidr()).thenReturn(subnet); - when(ipv4GuestSubnetNetworkMapDao.findBySubnet(subnet)).thenReturn(ipv4GuestSubnetNetworkMap); + when(vpc.getZoneId()).thenReturn(zoneId); + when(ipv4GuestSubnetNetworkMapDao.findBySubnetAndDataCenterId(subnet, zoneId)).thenReturn(ipv4GuestSubnetNetworkMap); when(ipv4GuestSubnetNetworkMap.getId()).thenReturn(ipv4GuestSubnetNetworkMapId); routedIpv4Manager.assignIpv4SubnetToVpc(vpc); From d98069c1ae4cbf61e44bd92b5b703613d0ae1fc0 Mon Sep 17 00:00:00 2001 From: Prashanth Reddy Date: Sat, 17 Oct 2026 00:30:57 +0000 Subject: [PATCH 2/2] network: list and report routed subnets by their own zone listIpv4SubnetsForGuestNetwork resolved the zoneid filter through dc_ip4_guest_subnets, so a subnet with no parent never matched, and a zone with no zone-level subnet returned an empty list. The response derived the zone the same way, leaving parentless subnets with no zone at all. Now that the record carries its zone, filter on data_center_id directly and fall back to it when building the response. Addresses review feedback on #14419. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017rnv8vT6LQSKfwkzp8tvg9 --- .../network/Ipv4GuestSubnetNetworkMap.java | 2 + .../network/Ipv4GuestSubnetNetworkMapVO.java | 1 + .../network/RoutedIpv4ManagerImpl.java | 10 +---- .../network/RoutedIpv4ManagerImplTest.java | 39 +++++++++++++++++++ 4 files changed, 44 insertions(+), 8 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/network/Ipv4GuestSubnetNetworkMap.java b/api/src/main/java/org/apache/cloudstack/network/Ipv4GuestSubnetNetworkMap.java index 569ed22c1640..1ab1b8527ec0 100644 --- a/api/src/main/java/org/apache/cloudstack/network/Ipv4GuestSubnetNetworkMap.java +++ b/api/src/main/java/org/apache/cloudstack/network/Ipv4GuestSubnetNetworkMap.java @@ -36,6 +36,8 @@ enum State { Long getParentId(); + Long getDataCenterId(); + String getSubnet(); Long getVpcId(); diff --git a/engine/schema/src/main/java/org/apache/cloudstack/network/Ipv4GuestSubnetNetworkMapVO.java b/engine/schema/src/main/java/org/apache/cloudstack/network/Ipv4GuestSubnetNetworkMapVO.java index 7e2e6f051158..d8f88a13a90e 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/network/Ipv4GuestSubnetNetworkMapVO.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/network/Ipv4GuestSubnetNetworkMapVO.java @@ -99,6 +99,7 @@ public Long getParentId() { return parentId; } + @Override public Long getDataCenterId() { return dataCenterId; } diff --git a/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java b/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java index ec92051e0da7..3befa9d64310 100644 --- a/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/network/RoutedIpv4ManagerImpl.java @@ -109,7 +109,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.stream.Collectors; public class RoutedIpv4ManagerImpl extends ComponentLifecycleBase implements RoutedIpv4Manager { @@ -519,12 +518,7 @@ public List listIpv4GuestSubnetsForGuestNetwork(ListI sc.addAnd("id", SearchCriteria.Op.EQ, id); } if (zoneId != null) { - List subnets = dataCenterIpv4GuestSubnetDao.listByDataCenterId(zoneId); - if (CollectionUtils.isEmpty(subnets)) { - return new ArrayList<>(); - } - List parentIds = subnets.stream().map(DataCenterIpv4GuestSubnetVO::getId).collect(Collectors.toList()); - sc.addAnd("parentId", SearchCriteria.Op.IN, parentIds.toArray()); + sc.addAnd("dataCenterId", SearchCriteria.Op.EQ, zoneId); } if (parentId != null) { sc.addAnd("parentId", SearchCriteria.Op.EQ, parentId); @@ -554,7 +548,7 @@ public Ipv4SubnetForGuestNetworkResponse createIpv4SubnetForGuestNetworkResponse response.setState(subnet.getState().name()); response.setId(subnet.getUuid()); response.setAllocatedTime(subnet.getAllocated()); - Long zoneId = null; + Long zoneId = subnet.getDataCenterId(); if (subnet.getNetworkId() != null) { Network network = ApiDBUtils.findNetworkById(subnet.getNetworkId()); response.setNetworkId(network.getUuid()); diff --git a/server/src/test/java/org/apache/cloudstack/network/RoutedIpv4ManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/network/RoutedIpv4ManagerImplTest.java index 3615bf2552b9..9f2c8eaab972 100644 --- a/server/src/test/java/org/apache/cloudstack/network/RoutedIpv4ManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/network/RoutedIpv4ManagerImplTest.java @@ -52,6 +52,7 @@ import com.cloud.user.AccountVO; import com.cloud.user.User; import com.cloud.user.UserVO; +import com.cloud.utils.db.SearchCriteria; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.net.NetUtils; @@ -60,6 +61,7 @@ import org.apache.cloudstack.api.command.admin.network.DedicateIpv4SubnetForZoneCmd; import org.apache.cloudstack.api.command.admin.network.DeleteIpv4SubnetForGuestNetworkCmd; import org.apache.cloudstack.api.command.admin.network.DeleteIpv4SubnetForZoneCmd; +import org.apache.cloudstack.api.command.admin.network.ListIpv4SubnetsForGuestNetworkCmd; import org.apache.cloudstack.api.command.admin.network.ReleaseDedicatedIpv4SubnetForZoneCmd; import org.apache.cloudstack.api.command.admin.network.UpdateIpv4SubnetForZoneCmd; import org.apache.cloudstack.api.command.admin.network.bgp.ChangeBgpPeersForNetworkCmd; @@ -497,6 +499,43 @@ public void testCreateIpv4SubnetForGuestNetworkResponse() { Assert.assertEquals("ipv4subnetforguestnetwork", response.getObjectName()); } + @Test + public void testCreateIpv4SubnetForGuestNetworkResponseWithoutParentOrOwner() { + when(ipv4GuestSubnetNetworkMap.getSubnet()).thenReturn(subnet); + when(ipv4GuestSubnetNetworkMap.getState()).thenReturn(ipv4GuestSubnetNetworkMapState); + when(ipv4GuestSubnetNetworkMap.getDataCenterId()).thenReturn(zoneId); + when(ipv4GuestSubnetNetworkMap.getNetworkId()).thenReturn(null); + when(ipv4GuestSubnetNetworkMap.getVpcId()).thenReturn(null); + when(ipv4GuestSubnetNetworkMap.getParentId()).thenReturn(null); + apiDBUtilsMocked.when(() -> ApiDBUtils.findZoneById(zoneId)).thenReturn(zone); + when(zone.getName()).thenReturn(zoneName); + when(zone.getUuid()).thenReturn(zoneUuid); + + Ipv4SubnetForGuestNetworkResponse response = routedIpv4Manager.createIpv4SubnetForGuestNetworkResponse(ipv4GuestSubnetNetworkMap); + + // the subnet has no parent, network or VPC, so the zone can only come from the subnet itself + Assert.assertEquals(zoneUuid, response.getZoneId()); + Assert.assertEquals(zoneName, response.getZoneName()); + } + + @Test + @SuppressWarnings("unchecked") + public void testListIpv4GuestSubnetsForGuestNetworkByZone() { + ListIpv4SubnetsForGuestNetworkCmd cmd = Mockito.mock(ListIpv4SubnetsForGuestNetworkCmd.class); + when(cmd.getZoneId()).thenReturn(zoneId); + SearchCriteria sc = Mockito.mock(SearchCriteria.class); + when(ipv4GuestSubnetNetworkMapDao.createSearchCriteria()).thenReturn(sc); + when(ipv4GuestSubnetNetworkMapDao.search(sc, null)).thenReturn(Arrays.asList(ipv4GuestSubnetNetworkMap)); + + List result = routedIpv4Manager.listIpv4GuestSubnetsForGuestNetwork(cmd); + + Assert.assertEquals(1, result.size()); + verify(sc).addAnd("dataCenterId", SearchCriteria.Op.EQ, zoneId); + // the zone is taken from the subnet itself, not resolved through the zone's parent subnets, + // so subnets created without a zone subnet are no longer missing from the result + verify(dataCenterIpv4GuestSubnetDao, Mockito.never()).listByDataCenterId(Mockito.anyLong()); + } + @Test public void testGetOrCreateIpv4SubnetForGuestNetworkOrVpcInternalByCidr1() { when(ipv4GuestSubnetNetworkMapDao.findBySubnetAndDataCenterId(subnet, zoneId)).thenReturn(ipv4GuestSubnetNetworkMap);