From 0cff8f7d70143eb4453ae5c5b550a5cda6630e69 Mon Sep 17 00:00:00 2001 From: xujiantop-crypto <265865031+xujiantop-crypto@users.noreply.github.com> Date: Thu, 10 Sep 2026 19:29:56 +0800 Subject: [PATCH 1/2] network: handle missing network when releasing public IP Signed-off-by: xujiantop-crypto <265865031+xujiantop-crypto@users.noreply.github.com> --- .../cloud/network/IpAddressManagerImpl.java | 10 ++++-- .../cloud/network/IpAddressManagerTest.java | 34 +++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java index dc78ac6790f4..a0ad399cca6d 100644 --- a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java +++ b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java @@ -792,11 +792,15 @@ public boolean disassociatePublicIpAddress(IpAddress ipAddress, long userId, Acc logger.debug("Releasing ip {}; sourceNat = {}", ip, ip.isSourceNat()); } + Network associatedNetwork = null; if (ip.getAssociatedWithNetworkId() != null) { - Network network = _networksDao.findById(ip.getAssociatedWithNetworkId()); + associatedNetwork = _networksDao.findById(ip.getAssociatedWithNetworkId()); + } + + if (associatedNetwork != null) { try { - if (!applyIpAssociations(network, rulesContinueOnErrFlag)) { - logger.warn("Unable to apply ip address associations for " + network); + if (!applyIpAssociations(associatedNetwork, rulesContinueOnErrFlag)) { + logger.warn("Unable to apply ip address associations for " + associatedNetwork); success = false; } } catch (ResourceUnavailableException e) { diff --git a/server/src/test/java/com/cloud/network/IpAddressManagerTest.java b/server/src/test/java/com/cloud/network/IpAddressManagerTest.java index cf3a886ce99f..421e82fbde5d 100644 --- a/server/src/test/java/com/cloud/network/IpAddressManagerTest.java +++ b/server/src/test/java/com/cloud/network/IpAddressManagerTest.java @@ -19,10 +19,13 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -36,6 +39,7 @@ import com.cloud.network.vo.PublicIpQuarantineVO; import com.cloud.user.Account; import com.cloud.user.AccountManager; +import org.apache.cloudstack.annotation.dao.AnnotationDao; import org.apache.cloudstack.context.CallContext; import org.junit.Assert; import org.junit.Before; @@ -56,6 +60,7 @@ import com.cloud.network.dao.NetworkVO; import com.cloud.network.rules.StaticNat; import com.cloud.network.rules.StaticNatImpl; +import com.cloud.network.vpc.dao.VpcDao; import com.cloud.offerings.NetworkOfferingVO; import com.cloud.offerings.dao.NetworkOfferingDao; import com.cloud.user.AccountVO; @@ -105,6 +110,12 @@ public class IpAddressManagerTest { @Mock AccountManager accountManagerMock; + @Mock + AnnotationDao annotationDao; + + @Mock + VpcDao vpcDao; + final long dummyID = 1L; final String UUID = "uuid"; @@ -130,6 +141,29 @@ public void setup() throws ResourceUnavailableException { Mockito.when(networkOfferingDao.findById(Mockito.anyLong())).thenReturn(networkOfferingVO); } + @Test + public void disassociatePublicIpAddressUnassignsReleasingIpWhenAssociatedNetworkIsMissing() throws ResourceUnavailableException { + long networkId = 2L; + when(ipAddressMock.getId()).thenReturn(dummyID); + when(ipAddressDao.acquireInLockTable(dummyID)).thenReturn(ipAddressVoMock); + when(ipAddressVoMock.getId()).thenReturn(dummyID); + doReturn(true).when(ipAddressManager).cleanupIpResources(ipAddressMock, dummyID, account); + doReturn(ipAddressVoMock).when(ipAddressManager).markIpAsUnavailable(dummyID); + when(ipAddressVoMock.getAssociatedWithNetworkId()).thenReturn(networkId); + when(ipAddressVoMock.getState()).thenReturn(IpAddress.State.Releasing); + when(ipAddressVoMock.getUuid()).thenReturn(UUID); + when(networkDao.findById(networkId)).thenReturn(null); + doReturn(null).when(ipAddressManager).addPublicIpAddressToQuarantine(ipAddressVoMock, account.getDomainId()); + + boolean result = ipAddressManager.disassociatePublicIpAddress(ipAddressMock, dummyID, account); + + assertTrue(result); + verify(ipAddressManager, never()).applyIpAssociations(any(), anyBoolean()); + verify(ipAddressDao).unassignIpAddress(dummyID); + verify(annotationDao).removeByEntityType("PUBLIC_IP_ADDRESS", UUID); + verify(ipAddressDao).releaseFromLockTable(dummyID); + } + @Test public void testGetStaticNatSourceIps() { String publicIpAddress = "192.168.1.3"; From 89032735858f68cac99ee115fd6cd6eb6f126d25 Mon Sep 17 00:00:00 2001 From: xujiantop-crypto <265865031+xujiantop-crypto@users.noreply.github.com> Date: Fri, 11 Sep 2026 10:18:10 +0800 Subject: [PATCH 2/2] test: cover VPC public IP release paths Signed-off-by: xujiantop-crypto <265865031+xujiantop-crypto@users.noreply.github.com> --- .../cloud/network/IpAddressManagerTest.java | 91 +++++++++++++++++-- 1 file changed, 84 insertions(+), 7 deletions(-) diff --git a/server/src/test/java/com/cloud/network/IpAddressManagerTest.java b/server/src/test/java/com/cloud/network/IpAddressManagerTest.java index 421e82fbde5d..cc277e25854c 100644 --- a/server/src/test/java/com/cloud/network/IpAddressManagerTest.java +++ b/server/src/test/java/com/cloud/network/IpAddressManagerTest.java @@ -58,11 +58,18 @@ import com.cloud.network.dao.IPAddressVO; import com.cloud.network.dao.NetworkDao; import com.cloud.network.dao.NetworkVO; +import com.cloud.network.element.NetworkElement; import com.cloud.network.rules.StaticNat; import com.cloud.network.rules.StaticNatImpl; +import com.cloud.network.vpc.VpcOfferingServiceMapVO; +import com.cloud.network.vpc.VpcOfferingVO; +import com.cloud.network.vpc.VpcVO; import com.cloud.network.vpc.dao.VpcDao; +import com.cloud.network.vpc.dao.VpcOfferingDao; +import com.cloud.network.vpc.dao.VpcOfferingServiceMapDao; import com.cloud.offerings.NetworkOfferingVO; import com.cloud.offerings.dao.NetworkOfferingDao; +import com.cloud.offerings.dao.NetworkOfferingServiceMapDao; import com.cloud.user.AccountVO; import com.cloud.utils.net.Ip; @@ -116,6 +123,15 @@ public class IpAddressManagerTest { @Mock VpcDao vpcDao; + @Mock + VpcOfferingDao vpcOfferingDao; + + @Mock + VpcOfferingServiceMapDao vpcOfferingServiceMapDao; + + @Mock + NetworkOfferingServiceMapDao networkOfferingServiceMapDao; + final long dummyID = 1L; final String UUID = "uuid"; @@ -144,26 +160,87 @@ public void setup() throws ResourceUnavailableException { @Test public void disassociatePublicIpAddressUnassignsReleasingIpWhenAssociatedNetworkIsMissing() throws ResourceUnavailableException { long networkId = 2L; - when(ipAddressMock.getId()).thenReturn(dummyID); - when(ipAddressDao.acquireInLockTable(dummyID)).thenReturn(ipAddressVoMock); + long vpcId = 3L; + long vpcOfferingId = 4L; + prepareIpDisassociation(networkId); when(ipAddressVoMock.getId()).thenReturn(dummyID); - doReturn(true).when(ipAddressManager).cleanupIpResources(ipAddressMock, dummyID, account); - doReturn(ipAddressVoMock).when(ipAddressManager).markIpAsUnavailable(dummyID); - when(ipAddressVoMock.getAssociatedWithNetworkId()).thenReturn(networkId); when(ipAddressVoMock.getState()).thenReturn(IpAddress.State.Releasing); - when(ipAddressVoMock.getUuid()).thenReturn(UUID); + when(ipAddressVoMock.getVpcId()).thenReturn(vpcId); when(networkDao.findById(networkId)).thenReturn(null); - doReturn(null).when(ipAddressManager).addPublicIpAddressToQuarantine(ipAddressVoMock, account.getDomainId()); + doReturn(publicIpQuarantineVOMock).when(ipAddressManager).addPublicIpAddressToQuarantine(ipAddressVoMock, account.getDomainId()); + + VpcVO vpc = mock(VpcVO.class); + VpcOfferingVO offering = mock(VpcOfferingVO.class); + when(vpcDao.findById(vpcId)).thenReturn(vpc); + when(vpc.getVpcOfferingId()).thenReturn(vpcOfferingId); + when(vpcOfferingDao.findById(vpcOfferingId)).thenReturn(offering); + when(offering.getId()).thenReturn(vpcOfferingId); + when(vpcOfferingServiceMapDao.listProvidersForServiceForVpcOffering(vpcOfferingId, Service.NetworkACL)) + .thenReturn(Collections.singletonList(new VpcOfferingServiceMapVO(vpcOfferingId, Service.NetworkACL, Network.Provider.VPCVirtualRouter))); + NetworkElement element = mock(NetworkElement.class); + ipAddressManager._networkModel = mock(NetworkModel.class); + when(ipAddressManager._networkModel.getElementImplementingProvider(Network.Provider.VPCVirtualRouter.getName())).thenReturn(element); boolean result = ipAddressManager.disassociatePublicIpAddress(ipAddressMock, dummyID, account); assertTrue(result); verify(ipAddressManager, never()).applyIpAssociations(any(), anyBoolean()); + verify(ipAddressManager).addPublicIpAddressToQuarantine(ipAddressVoMock, account.getDomainId()); verify(ipAddressDao).unassignIpAddress(dummyID); + verify(element).releaseIp(ipAddressVoMock); verify(annotationDao).removeByEntityType("PUBLIC_IP_ADDRESS", UUID); verify(ipAddressDao).releaseFromLockTable(dummyID); } + @Test + public void disassociatePublicIpAddressAppliesAssociationsWhenNetworkExists() throws ResourceUnavailableException { + verifyDisassociationWithExistingNetwork(true); + } + + @Test + public void disassociatePublicIpAddressReturnsFailureWhenAssociationsFail() throws ResourceUnavailableException { + verifyDisassociationWithExistingNetwork(false); + } + + private void verifyDisassociationWithExistingNetwork(boolean associationsApplied) throws ResourceUnavailableException { + long networkId = 2L; + prepareIpDisassociation(networkId); + NetworkVO network = mock(NetworkVO.class); + when(networkDao.findById(networkId)).thenReturn(network); + doReturn(associationsApplied).when(ipAddressManager).applyIpAssociations(network, IpAddressManagerImpl.rulesContinueOnErrFlag); + + boolean result = ipAddressManager.disassociatePublicIpAddress(ipAddressMock, dummyID, account); + + Assert.assertEquals(associationsApplied, result); + verify(ipAddressManager).applyIpAssociations(network, IpAddressManagerImpl.rulesContinueOnErrFlag); + verify(ipAddressManager, never()).addPublicIpAddressToQuarantine(any(), anyLong()); + verify(ipAddressDao, never()).unassignIpAddress(anyLong()); + verify(ipAddressDao).releaseFromLockTable(dummyID); + } + + @Test + public void disassociatePublicIpAddressDoesNotUnassignFreeIpWhenNetworkIsMissing() throws ResourceUnavailableException { + prepareIpDisassociation(2L); + when(ipAddressVoMock.getState()).thenReturn(IpAddress.State.Free); + + boolean result = ipAddressManager.disassociatePublicIpAddress(ipAddressMock, dummyID, account); + + assertTrue(result); + verify(ipAddressManager, never()).applyIpAssociations(any(), anyBoolean()); + verify(ipAddressManager, never()).addPublicIpAddressToQuarantine(any(), anyLong()); + verify(ipAddressDao, never()).unassignIpAddress(anyLong()); + verify(ipAddressDao).releaseFromLockTable(dummyID); + } + + private void prepareIpDisassociation(long networkId) { + when(ipAddressMock.getId()).thenReturn(dummyID); + when(ipAddressDao.acquireInLockTable(dummyID)).thenReturn(ipAddressVoMock); + doReturn(true).when(ipAddressManager).cleanupIpResources(ipAddressMock, dummyID, account); + doReturn(ipAddressVoMock).when(ipAddressManager).markIpAsUnavailable(dummyID); + when(ipAddressVoMock.getAssociatedWithNetworkId()).thenReturn(networkId); + when(ipAddressVoMock.getUuid()).thenReturn(UUID); + } + @Test public void testGetStaticNatSourceIps() { String publicIpAddress = "192.168.1.3";