diff --git a/api/src/main/java/com/cloud/network/NetworkService.java b/api/src/main/java/com/cloud/network/NetworkService.java index d959e3abce0..82d229da459 100644 --- a/api/src/main/java/com/cloud/network/NetworkService.java +++ b/api/src/main/java/com/cloud/network/NetworkService.java @@ -114,6 +114,8 @@ public interface NetworkService { IpAddress getIp(long id); + IpAddress getIp(String ipAddress); + Network updateGuestNetwork(final UpdateNetworkCmd cmd); /** diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/address/DisassociateIPAddrCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/address/DisassociateIPAddrCmd.java index b31520e1b88..f9bfcb253b4 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/address/DisassociateIPAddrCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/address/DisassociateIPAddrCmd.java @@ -46,10 +46,14 @@ public class DisassociateIPAddrCmd extends BaseAsyncCmd { //////////////// API parameters ///////////////////// ///////////////////////////////////////////////////// - @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = IPAddressResponse.class, required = true, description = "the ID of the public IP address" - + " to disassociate") + @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = IPAddressResponse.class, description = "the ID of the public IP address" + + " to disassociate. Mutually exclusive with the ipaddress parameter") private Long id; + @Parameter(name=ApiConstants.IP_ADDRESS, type=CommandType.STRING, since="4.19.0", description="IP Address to be disassociated." + + " Mutually exclusive with the id parameter") + private String ipAddress; + // unexposed parameter needed for events logging @Parameter(name = ApiConstants.ACCOUNT_ID, type = CommandType.UUID, entityType = AccountResponse.class, expose = false) private Long ownerId; @@ -59,7 +63,18 @@ public class DisassociateIPAddrCmd extends BaseAsyncCmd { ///////////////////////////////////////////////////// public Long getIpAddressId() { - return id; + if (id != null & ipAddress != null) { + throw new InvalidParameterValueException("id parameter is mutually exclusive with ipaddress parameter"); + } + + if (id != null) { + return id; + } else if (ipAddress != null) { + IpAddress ip = getIpAddressByIp(ipAddress); + return ip.getId(); + } + + throw new InvalidParameterValueException("Please specify either IP address or IP address ID"); } ///////////////////////////////////////////////////// @@ -68,12 +83,13 @@ public class DisassociateIPAddrCmd extends BaseAsyncCmd { @Override public void execute() throws InsufficientAddressCapacityException { - CallContext.current().setEventDetails("IP ID: " + getIpAddressId()); + Long ipAddressId = getIpAddressId(); + CallContext.current().setEventDetails("IP ID: " + ipAddressId); boolean result = false; - if (!isPortable(id)) { - result = _networkService.releaseIpAddress(getIpAddressId()); + if (!isPortable()) { + result = _networkService.releaseIpAddress(ipAddressId); } else { - result = _networkService.releasePortableIpAddress(getIpAddressId()); + result = _networkService.releasePortableIpAddress(ipAddressId); } if (result) { SuccessResponse response = new SuccessResponse(getCommandName()); @@ -85,7 +101,7 @@ public class DisassociateIPAddrCmd extends BaseAsyncCmd { @Override public String getEventType() { - if (!isPortable(id)) { + if (!isPortable()) { return EventTypes.EVENT_NET_IP_RELEASE; } else { return EventTypes.EVENT_PORTABLE_IP_RELEASE; @@ -100,10 +116,7 @@ public class DisassociateIPAddrCmd extends BaseAsyncCmd { @Override public long getEntityOwnerId() { if (ownerId == null) { - IpAddress ip = getIpAddress(id); - if (ip == null) { - throw new InvalidParameterValueException("Unable to find IP address by ID=" + id); - } + IpAddress ip = getIpAddress(); ownerId = ip.getAccountId(); } @@ -120,11 +133,11 @@ public class DisassociateIPAddrCmd extends BaseAsyncCmd { @Override public Long getSyncObjId() { - IpAddress ip = getIpAddress(id); + IpAddress ip = getIpAddress(); return ip.getAssociatedWithNetworkId(); } - private IpAddress getIpAddress(long id) { + private IpAddress getIpAddressById(Long id) { IpAddress ip = _entityMgr.findById(IpAddress.class, id); if (ip == null) { @@ -134,6 +147,29 @@ public class DisassociateIPAddrCmd extends BaseAsyncCmd { } } + private IpAddress getIpAddressByIp(String ipAddress) { + IpAddress ip = _networkService.getIp(ipAddress); + if (ip == null) { + throw new InvalidParameterValueException("Unable to find IP address by IP address=" + ipAddress); + } else { + return ip; + } + } + + private IpAddress getIpAddress() { + if (id != null & ipAddress != null) { + throw new InvalidParameterValueException("id parameter is mutually exclusive with ipaddress parameter"); + } + + if (id != null) { + return getIpAddressById(id); + } else if (ipAddress != null){ + return getIpAddressByIp(ipAddress); + } + + throw new InvalidParameterValueException("Please specify either IP address or IP address ID"); + } + @Override public ApiCommandResourceType getApiResourceType() { return ApiCommandResourceType.IpAddress; @@ -144,8 +180,8 @@ public class DisassociateIPAddrCmd extends BaseAsyncCmd { return getIpAddressId(); } - private boolean isPortable(long id) { - IpAddress ip = getIpAddress(id); + private boolean isPortable() { + IpAddress ip = getIpAddress(); return ip.isPortable(); } } diff --git a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java index 0c17efaab36..f9f80cb5b76 100644 --- a/server/src/main/java/com/cloud/network/NetworkServiceImpl.java +++ b/server/src/main/java/com/cloud/network/NetworkServiceImpl.java @@ -2798,6 +2798,11 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService, C return _ipAddressDao.findById(ipAddressId); } + @Override + public IpAddress getIp(String ipAddress) { + return _ipAddressDao.findByIp(ipAddress); + } + protected boolean providersConfiguredForExternalNetworking(Collection providers) { for (String providerStr : providers) { Provider provider = Network.Provider.getProvider(providerStr); diff --git a/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java b/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java index 945448f7173..7535841974e 100644 --- a/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java +++ b/server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java @@ -275,6 +275,15 @@ public class MockNetworkManagerImpl extends ManagerBase implements NetworkOrches return null; } + /* (non-Javadoc) + * @see com.cloud.network.NetworkService#getIp(String) + */ + @Override + public IpAddress getIp(String ipAddress) { + // TODO Auto-generated method stub + return null; + } + @Override public Network updateGuestNetwork(final UpdateNetworkCmd cmd) { // TODO Auto-generated method stub diff --git a/test/integration/smoke/test_network.py b/test/integration/smoke/test_network.py index 56b434f3241..8f3f4f533dd 100644 --- a/test/integration/smoke/test_network.py +++ b/test/integration/smoke/test_network.py @@ -871,7 +871,7 @@ class TestReleaseIP(cloudstackTestCase): @attr(tags=["advanced", "advancedns", "smoke", "dvs"], required_hardware="false") def test_releaseIP(self): - """Test for release public IP address""" + """Test for release public IP address using the ID""" logger.debug("Deleting Public IP : %s" % self.ip_addr.id) @@ -930,6 +930,66 @@ class TestReleaseIP(cloudstackTestCase): ) return + @attr(tags=["advanced", "advancedns", "smoke", "dvs"], required_hardware="false") + def test_releaseIP_using_IP(self): + """Test for release public IP address using the address""" + + logger.debug("Deleting Public IP : %s" % self.ip_addr.ipaddress) + self.ip_address.delete_by_ip(self.apiclient) + + retriesCount = 10 + isIpAddressDisassociated = False + while retriesCount > 0: + listResponse = list_publicIP( + self.apiclient, + id=self.ip_addr.id, + state="Allocated" + ) + if listResponse is None: + isIpAddressDisassociated = True + break + retriesCount -= 1 + time.sleep(60) + # End while + + self.assertTrue( + isIpAddressDisassociated, + "Failed to disassociate IP address") + + # ListPortForwardingRules should not list + # associated rules with Public IP address + try: + list_nat_rule = list_nat_rules( + self.apiclient, + id=self.nat_rule.id + ) + logger.debug("List NAT Rule response" + str(list_nat_rule)) + except CloudstackAPIException: + logger.debug("Port Forwarding Rule is deleted") + + # listLoadBalancerRules should not list + # associated rules with Public IP address + try: + list_lb_rule = list_lb_rules( + self.apiclient, + id=self.lb_rule.id + ) + logger.debug("List LB Rule response" + str(list_lb_rule)) + except CloudstackAPIException: + logger.debug("Port Forwarding Rule is deleted") + + # SSH Attempt though public IP should fail + with self.assertRaises(Exception): + SshClient( + self.ip_addr.ipaddress, + self.services["natrule"]["publicport"], + self.virtual_machine.username, + self.virtual_machine.password, + retries=2, + delay=0 + ) + return + class TestDeleteAccount(cloudstackTestCase): diff --git a/tools/marvin/marvin/lib/base.py b/tools/marvin/marvin/lib/base.py index 32b0e980407..a9827dcfaaf 100755 --- a/tools/marvin/marvin/lib/base.py +++ b/tools/marvin/marvin/lib/base.py @@ -1879,12 +1879,19 @@ class PublicIPAddress: return PublicIPAddress(apiclient.associateIpAddress(cmd).__dict__) def delete(self, apiclient): - """Dissociate Public IP address""" + """Dissociate Public IP address using the given ID""" cmd = disassociateIpAddress.disassociateIpAddressCmd() cmd.id = self.ipaddress.id apiclient.disassociateIpAddress(cmd) return + def delete_by_ip(self, apiclient): + """Dissociate Public IP address using the given IP address""" + cmd = disassociateIpAddress.disassociateIpAddressCmd() + cmd.ipaddress = self.ipaddress.ipaddress + apiclient.disassociateIpAddress(cmd) + return + @classmethod def list(cls, apiclient, **kwargs): """List all Public IPs matching criteria"""