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.

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;

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);
 

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) {
+                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());
             }

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 {
+    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();

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" +
+                        s_logger.info("Portable IP " + ipAddress.getUuid() + " is not associated with the network yet "
-                                "associate IP with the network " + networkId);
+                                + " 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);
                         }

server/src/com/cloud/network/vpc/VpcManagerImpl.java

@@ -1217,11 +1217,20 @@ 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) {
+            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");
                 }
             }
+        } 
         
         if (success) {
             s_logger.debug("Released ip addresses for vpc id=" + vpcId + " as a part of cleanup vpc process");

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);

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
     }
 

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
     }