server: fix public IP association/disassociation to new network (#3489)

Fixes #3321

This changes removes exception throwing while associating an IP address to a new isolated network which is in Allocated state. And it allows disassociating an IP address when it is used for source NAT purpose but network is in allocated state.

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

Author: shwstppr

server/src/main/java/com/cloud/network/IpAddressManagerImpl.java

@@ -29,9 +29,6 @@
 
 import javax.inject.Inject;
 
-import com.cloud.dc.DomainVlanMapVO;
-import org.apache.log4j.Logger;
-
 import org.apache.cloudstack.acl.ControlledEntity.ACLType;
 import org.apache.cloudstack.acl.SecurityChecker.AccessType;
 import org.apache.cloudstack.api.response.AcquirePodIpCmdResponse;
@@ -44,6 +41,7 @@
 import org.apache.cloudstack.region.PortableIpDao;
 import org.apache.cloudstack.region.PortableIpVO;
 import org.apache.cloudstack.region.Region;
+import org.apache.log4j.Logger;
 
 import com.cloud.agent.AgentManager;
 import com.cloud.alert.AlertManager;
@@ -54,6 +52,7 @@
 import com.cloud.dc.DataCenter;
 import com.cloud.dc.DataCenter.NetworkType;
 import com.cloud.dc.DataCenterIpAddressVO;
+import com.cloud.dc.DomainVlanMapVO;
 import com.cloud.dc.HostPodVO;
 import com.cloud.dc.Pod;
 import com.cloud.dc.PodVlanMapVO;
@@ -1429,12 +1428,6 @@ protected boolean isSourceNatAvailableForNetwork(Account owner, IPAddressVO ipTo
         if (!sharedSourceNat) {
             if (getExistingSourceNatInNetwork(owner.getId(), network.getId()) == null) {
                 if (network.getGuestType() == GuestType.Isolated && network.getVpcId() == null && !ipToAssoc.isPortable()) {
-                    if (network.getState() == Network.State.Allocated) {
-                        //prevent associating an ip address to an allocated (unimplemented network).
-                        //it will cause the ip to become source nat, and it can't be disassociated later on.
-                        String msg = String.format("Network with UUID:%s is in allocated and needs to be implemented first before acquiring an IP address", network.getUuid());
-                        throw new InvalidParameterValueException(msg);
-                    }
                     isSourceNat = true;
                 }
             }

server/src/main/java/com/cloud/network/NetworkServiceImpl.java

@@ -924,7 +924,12 @@ private boolean releaseIpAddressInternal(long ipAddressId) throws InsufficientAd
             _accountMgr.checkAccess(caller, null, true, ipVO);
         }
 
-        if (ipVO.isSourceNat()) {
+        Network guestNetwork = null;
+        final Long networkId = ipVO.getAssociatedWithNetworkId();
+        if (networkId != null) {
+            guestNetwork = getNetwork(networkId);
+        }
+        if (ipVO.isSourceNat() && guestNetwork != null && guestNetwork.getState() != Network.State.Allocated) {
             throw new IllegalArgumentException("ip address is used for source nat purposes and can not be disassociated.");
         }
 
@@ -941,9 +946,7 @@ private boolean releaseIpAddressInternal(long ipAddressId) throws InsufficientAd
         boolean success = _ipAddrMgr.disassociatePublicIpAddress(ipAddressId, userId, caller);
 
         if (success) {
-            Long networkId = ipVO.getAssociatedWithNetworkId();
-            if (networkId != null) {
-                Network guestNetwork = getNetwork(networkId);
+            if (guestNetwork != null) {
                 NetworkOffering offering = _entityMgr.findById(NetworkOffering.class, guestNetwork.getNetworkOfferingId());
                 Long vmId = ipVO.getAssociatedWithVmId();
                 if (offering.isElasticIp() && vmId != null) {

server/src/test/java/com/cloud/network/IpAddressManagerTest.java

@@ -36,8 +36,8 @@
 import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.Spy;
+import org.mockito.runners.MockitoJUnitRunner;
 
-import com.cloud.exception.InvalidParameterValueException;
 import com.cloud.exception.ResourceUnavailableException;
 import com.cloud.network.Network.Service;
 import com.cloud.network.dao.IPAddressDao;
@@ -50,7 +50,6 @@
 import com.cloud.offerings.dao.NetworkOfferingDao;
 import com.cloud.user.AccountVO;
 import com.cloud.utils.net.Ip;
-import org.mockito.runners.MockitoJUnitRunner;
 
 @RunWith(MockitoJUnitRunner.class)
 public class IpAddressManagerTest {
@@ -170,7 +169,7 @@ public void assertSourceNatImplementedNetwork() {
         assertTrue("Source NAT should be true", isSourceNat);
     }
 
-    @Test(expected = InvalidParameterValueException.class)
+    @Test
     public void assertSourceNatAllocatedNetwork() {
 
         NetworkVO networkAllocated = Mockito.mock(NetworkVO.class);
@@ -184,7 +183,7 @@ public void assertSourceNatAllocatedNetwork() {
         Mockito.when(networkDao.findById(2L)).thenReturn(networkAllocated);
         doReturn(null).when(ipAddressManager).getExistingSourceNatInNetwork(1L, 2L);
 
-        ipAddressManager.isSourceNatAvailableForNetwork(account, ipAddressVO, networkAllocated);
+        assertTrue(ipAddressManager.isSourceNatAvailableForNetwork(account, ipAddressVO, networkAllocated));
     }
 
     @Test