CLOUDSTACK-2700:on network/vpc delete, portable IP should be still associated with account
Unlike public ip which gets dis-associated (released) with the account on network/VPC delete, portable IP should continue to be associated with the account even when the network/VPC with which it is currently associated in deleted. This fix ensures portable IP are associated to account even after network/vpc is deleted. Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/883333c2 Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/883333c2 Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/883333c2 Branch: refs/heads/vmware-storage-motion Commit: 883333c2143e9f3b14a3bc91f8e486e3574dfa37 Parents: ad48c83 Author: Murali Reddy <muralimmre...@gmail.com> Authored: Mon May 27 18:42:33 2013 +0530 Committer: Murali Reddy <muralimmre...@gmail.com> Committed: Mon May 27 18:44:41 2013 +0530 ---------------------------------------------------------------------- api/src/com/cloud/network/NetworkService.java | 2 +- server/src/com/cloud/network/NetworkManager.java | 9 +++--- .../src/com/cloud/network/NetworkManagerImpl.java | 20 +++++++++++--- .../src/com/cloud/network/NetworkServiceImpl.java | 10 +++++-- .../com/cloud/network/rules/RulesManagerImpl.java | 12 ++++---- .../src/com/cloud/network/vpc/VpcManagerImpl.java | 15 +++++++++-- server/src/com/cloud/user/AccountManagerImpl.java | 8 ++++++ .../com/cloud/network/MockNetworkManagerImpl.java | 2 +- .../test/com/cloud/vpc/MockNetworkManagerImpl.java | 2 +- 9 files changed, 56 insertions(+), 24 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/883333c2/api/src/com/cloud/network/NetworkService.java ---------------------------------------------------------------------- diff --git a/api/src/com/cloud/network/NetworkService.java b/api/src/com/cloud/network/NetworkService.java index 59702a2..405cecd 100755 --- a/api/src/com/cloud/network/NetworkService.java +++ b/api/src/com/cloud/network/NetworkService.java @@ -55,7 +55,7 @@ public interface NetworkService { IpAddress allocatePortableIP(Account ipOwner, int regionId, Long zoneId, Long networkId, Long vpcId) throws ResourceAllocationException, InsufficientAddressCapacityException, ConcurrentOperationException; - boolean releasePortableIpAddress(long ipAddressId) throws InsufficientAddressCapacityException; + boolean releasePortableIpAddress(long ipAddressId); Network createGuestNetwork(CreateNetworkCmd cmd) throws InsufficientCapacityException, ConcurrentOperationException, ResourceAllocationException; http://git-wip-us.apache.org/repos/asf/cloudstack/blob/883333c2/server/src/com/cloud/network/NetworkManager.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/NetworkManager.java b/server/src/com/cloud/network/NetworkManager.java index 05bc26e..19f396a 100755 --- a/server/src/com/cloud/network/NetworkManager.java +++ b/server/src/com/cloud/network/NetworkManager.java @@ -268,6 +268,11 @@ public interface NetworkManager { IPAddressVO associateIPToGuestNetwork(long ipAddrId, long networkId, boolean releaseOnFailure) throws ResourceAllocationException, ResourceUnavailableException, InsufficientAddressCapacityException, ConcurrentOperationException; + IpAddress allocatePortableIp(Account ipOwner, Account caller, long dcId, Long networkId, Long vpcID) + throws ConcurrentOperationException, ResourceAllocationException, InsufficientAddressCapacityException; + + boolean releasePortableIpAddress(long addrId); + IPAddressVO associatePortableIPToGuestNetwork(long ipAddrId, long networkId, boolean releaseOnFailure) throws ResourceAllocationException, ResourceUnavailableException, InsufficientAddressCapacityException, ConcurrentOperationException; @@ -362,10 +367,6 @@ public interface NetworkManager { IpAddress allocateIp(Account ipOwner, boolean isSystem, Account caller, long callerId, DataCenter zone) throws ConcurrentOperationException, ResourceAllocationException, InsufficientAddressCapacityException; - - IpAddress allocatePortableIp(Account ipOwner, Account caller, long dcId, Long networkId, Long vpcID) - throws ConcurrentOperationException, ResourceAllocationException, InsufficientAddressCapacityException; - Map<String, String> finalizeServicesAndProvidersForNetwork(NetworkOffering offering, Long physicalNetworkId); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/883333c2/server/src/com/cloud/network/NetworkManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/NetworkManagerImpl.java b/server/src/com/cloud/network/NetworkManagerImpl.java index d699b2e..b443dca 100755 --- a/server/src/com/cloud/network/NetworkManagerImpl.java +++ b/server/src/com/cloud/network/NetworkManagerImpl.java @@ -1118,7 +1118,8 @@ public class NetworkManagerImpl extends ManagerBase implements NetworkManager, L } @DB - private void releasePortableIpAddress(long addrId) { + @Override + public boolean releasePortableIpAddress(long addrId) { Transaction txn = Transaction.currentTxn(); GlobalLock portableIpLock = GlobalLock.getInternLock("PortablePublicIpRange"); @@ -1133,12 +1134,13 @@ public class NetworkManagerImpl extends ManagerBase implements NetworkManager, L // removed the provisioned vlan VlanVO vlan = _vlanDao.findById(ip.getVlanId()); - _vlanDao.expunge(vlan.getId()); + _vlanDao.remove(vlan.getId()); // remove the provisioned public ip address - _ipAddressDao.expunge(ip.getId()); + _ipAddressDao.remove(ip.getId()); txn.commit(); + return true; } finally { portableIpLock.releaseRef(); } @@ -3537,8 +3539,16 @@ public class NetworkManagerImpl extends ManagerBase implements NetworkManager, L List<IPAddressVO> ipsToRelease = _ipAddressDao.listByAssociatedNetwork(networkId, null); for (IPAddressVO ipToRelease : ipsToRelease) { if (ipToRelease.getVpcId() == null) { - IPAddressVO ip = markIpAsUnavailable(ipToRelease.getId()); - assert (ip != null) : "Unable to mark the ip address id=" + ipToRelease.getId() + " as unavailable."; + if (!ipToRelease.isPortable()) { + IPAddressVO ip = markIpAsUnavailable(ipToRelease.getId()); + assert (ip != null) : "Unable to mark the ip address id=" + ipToRelease.getId() + " as unavailable."; + } else { + // portable IP address are associated with owner, until explicitly requested to be disassociated + // so as part of network clean up just break IP association with guest network + ipToRelease.setAssociatedWithNetworkId(null); + _ipAddressDao.update(ipToRelease.getId(), ipToRelease); + s_logger.debug("Portable IP address " + ipToRelease + " is no longer associated with any network"); + } } else { _vpcMgr.unassignIPFromVpcNetwork(ipToRelease.getId(), network.getId()); } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/883333c2/server/src/com/cloud/network/NetworkServiceImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/NetworkServiceImpl.java b/server/src/com/cloud/network/NetworkServiceImpl.java index 1533ca9..f880bcc 100755 --- a/server/src/com/cloud/network/NetworkServiceImpl.java +++ b/server/src/com/cloud/network/NetworkServiceImpl.java @@ -591,8 +591,12 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService { @Override @ActionEvent(eventType = EventTypes.EVENT_PORTABLE_IP_RELEASE, eventDescription = "disassociating portable Ip", async = true) - public boolean releasePortableIpAddress(long ipAddressId) throws InsufficientAddressCapacityException { - return releaseIpAddressInternal(ipAddressId); + public boolean releasePortableIpAddress(long ipAddressId) { + try { + return releaseIpAddressInternal(ipAddressId); + } catch (Exception e) { + return false; + } } @Override @@ -880,7 +884,7 @@ public class NetworkServiceImpl extends ManagerBase implements NetworkService { boolean success = _networkMgr.disassociatePublicIpAddress(ipAddressId, userId, caller); if (success) { - if (!ipVO.isPortable()) { + if (ipVO.isPortable()) { return success; } Long networkId = ipVO.getAssociatedWithNetworkId(); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/883333c2/server/src/com/cloud/network/rules/RulesManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/rules/RulesManagerImpl.java b/server/src/com/cloud/network/rules/RulesManagerImpl.java index 8834553..3c95cef 100755 --- a/server/src/com/cloud/network/rules/RulesManagerImpl.java +++ b/server/src/com/cloud/network/rules/RulesManagerImpl.java @@ -490,10 +490,9 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules "a part of enable static nat"); return false; } - performedIpAssoc = true; } else if (ipAddress.isPortable()) { - s_logger.info("Portable IP " + ipAddress.getUuid() + " is not associated with the network, so" + - "associate IP with the network " + networkId); + s_logger.info("Portable IP " + ipAddress.getUuid() + " is not associated with the network yet " + + " so associate IP with the network " + networkId); try { // check if StaticNat service is enabled in the network _networkModel.checkIpForService(ipAddress, Service.StaticNat, networkId); @@ -504,13 +503,12 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules } // associate portable IP with guest network - _networkMgr.associatePortableIPToGuestNetwork(ipId, networkId, false); + ipAddress = _networkMgr.associatePortableIPToGuestNetwork(ipId, networkId, false); } catch (Exception e) { s_logger.warn("Failed to associate portable id=" + ipId + " to network id=" + networkId + " as " + "a part of enable static nat"); return false; } - performedIpAssoc = true; } } else if (ipAddress.getAssociatedWithNetworkId() != networkId) { if (ipAddress.isPortable()) { @@ -520,14 +518,16 @@ public class RulesManagerImpl extends ManagerBase implements RulesManager, Rules // check if portable IP can be transferred across the networks if (_networkMgr.isPortableIpTransferableFromNetwork(ipId, ipAddress.getAssociatedWithNetworkId() )) { try { + // transfer the portable IP and refresh IP details _networkMgr.transferPortableIP(ipId, ipAddress.getAssociatedWithNetworkId(), networkId); + ipAddress = _ipAddressDao.findById(ipId); } catch (Exception e) { s_logger.warn("Failed to associate portable id=" + ipId + " to network id=" + networkId + " as " + "a part of enable static nat"); return false; } } else { - throw new InvalidParameterValueException("Portable IP: " + ipId + " has associated services" + + throw new InvalidParameterValueException("Portable IP: " + ipId + " has associated services " + "in network " + ipAddress.getAssociatedWithNetworkId() + " so can not be transferred to " + " network " + networkId); } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/883333c2/server/src/com/cloud/network/vpc/VpcManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/vpc/VpcManagerImpl.java b/server/src/com/cloud/network/vpc/VpcManagerImpl.java index 1aab732..15e1539 100644 --- a/server/src/com/cloud/network/vpc/VpcManagerImpl.java +++ b/server/src/com/cloud/network/vpc/VpcManagerImpl.java @@ -1217,9 +1217,18 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis List<IPAddressVO> ipsToRelease = _ipAddressDao.listByAssociatedVpc(vpcId, null); s_logger.debug("Releasing ips for vpc id=" + vpcId + " as a part of vpc cleanup"); for (IPAddressVO ipToRelease : ipsToRelease) { - success = success && _ntwkMgr.disassociatePublicIpAddress(ipToRelease.getId(), callerUserId, caller); - if (!success) { - s_logger.warn("Failed to cleanup ip " + ipToRelease + " as a part of vpc id=" + vpcId + " cleanup"); + if (ipToRelease.isPortable()) { + // portable IP address are associated with owner, until explicitly requested to be disassociated. + // so as part of VPC clean up just break IP association with VPC + ipToRelease.setVpcId(null); + ipToRelease.setAssociatedWithNetworkId(null); + _ipAddressDao.update(ipToRelease.getId(), ipToRelease); + s_logger.debug("Portable IP address " + ipToRelease + " is no longer associated with any VPC"); + } else { + success = success && _ntwkMgr.disassociatePublicIpAddress(ipToRelease.getId(), callerUserId, caller); + if (!success) { + s_logger.warn("Failed to cleanup ip " + ipToRelease + " as a part of vpc id=" + vpcId + " cleanup"); + } } } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/883333c2/server/src/com/cloud/user/AccountManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/user/AccountManagerImpl.java b/server/src/com/cloud/user/AccountManagerImpl.java index 7421422..e72005e 100755 --- a/server/src/com/cloud/user/AccountManagerImpl.java +++ b/server/src/com/cloud/user/AccountManagerImpl.java @@ -731,6 +731,14 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M _resourceCountDao.removeEntriesByOwner(accountId, ResourceOwnerType.Account); _resourceLimitDao.removeEntriesByOwner(accountId, ResourceOwnerType.Account); + // release account specific acquired portable IP's. Since all the portable IP's must have been already + // disassociated with VPC/guest network (due to deletion), so just mark portable IP as free. + List<? extends IpAddress> portableIpsToRelease = _ipAddressDao.listByAccount(accountId); + for (IpAddress ip : portableIpsToRelease) { + s_logger.debug("Releasing portable ip " + ip + " as a part of account id=" + accountId + " cleanup"); + _networkMgr.releasePortableIpAddress(ip.getId()); + } + return true; } catch (Exception ex) { s_logger.warn("Failed to cleanup account " + account + " due to ", ex); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/883333c2/server/test/com/cloud/network/MockNetworkManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/test/com/cloud/network/MockNetworkManagerImpl.java b/server/test/com/cloud/network/MockNetworkManagerImpl.java index e5d34fb..53c0fa8 100755 --- a/server/test/com/cloud/network/MockNetworkManagerImpl.java +++ b/server/test/com/cloud/network/MockNetworkManagerImpl.java @@ -887,7 +887,7 @@ public class MockNetworkManagerImpl extends ManagerBase implements NetworkManage } @Override - public boolean releasePortableIpAddress(long ipAddressId) throws InsufficientAddressCapacityException { + public boolean releasePortableIpAddress(long ipAddressId) { return false;// TODO Auto-generated method stub } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/883333c2/server/test/com/cloud/vpc/MockNetworkManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/test/com/cloud/vpc/MockNetworkManagerImpl.java b/server/test/com/cloud/vpc/MockNetworkManagerImpl.java index 7129273..a1ece44 100644 --- a/server/test/com/cloud/vpc/MockNetworkManagerImpl.java +++ b/server/test/com/cloud/vpc/MockNetworkManagerImpl.java @@ -210,7 +210,7 @@ public class MockNetworkManagerImpl extends ManagerBase implements NetworkManage } @Override - public boolean releasePortableIpAddress(long ipAddressId) throws InsufficientAddressCapacityException { + public boolean releasePortableIpAddress(long ipAddressId) { return false;// TODO Auto-generated method stub }
