You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by sh...@apache.org on 2023/11/19 06:21:05 UTC
(cloudstack) branch main updated: api: add ipaddress argument to disassociateIPAddress (#8222)
This is an automated email from the ASF dual-hosted git repository.
shwstppr pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/main by this push:
new 5c7e4b7edc8 api: add ipaddress argument to disassociateIPAddress (#8222)
5c7e4b7edc8 is described below
commit 5c7e4b7edc88a588b94db64998e83b6fcc76048a
Author: Abhisar Sinha <63...@users.noreply.github.com>
AuthorDate: Sun Nov 19 11:50:57 2023 +0530
api: add ipaddress argument to disassociateIPAddress (#8222)
This PR adds argument 'ipadress' to the disassociateIpAddress api. IP address can be disassociated by directly giving the address instead of ID.
Fixes: #8125
---
.../java/com/cloud/network/NetworkService.java | 2 +
.../user/address/DisassociateIPAddrCmd.java | 68 +++++++++++++++++-----
.../java/com/cloud/network/NetworkServiceImpl.java | 5 ++
.../java/com/cloud/vpc/MockNetworkManagerImpl.java | 9 +++
test/integration/smoke/test_network.py | 62 +++++++++++++++++++-
tools/marvin/marvin/lib/base.py | 9 ++-
6 files changed, 137 insertions(+), 18 deletions(-)
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<String> 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"""