CLOUDSTACK-9595: Fix regression introduced in #1762 (#2370)

The `assignDedicateIpAddress` previously had marked the newly fetched
IP as allocated but now it does not do that. This fails for VPCs
where SNATs IP are retained as allocating and not allocated after
creation.

Signed-off-by: Rohit Yadav <[email protected]>

Author: yadvr

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

@@ -291,8 +291,8 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage
 
     SearchBuilder<IPAddressVO> AssignIpAddressSearch;
     SearchBuilder<IPAddressVO> AssignIpAddressFromPodVlanSearch;
-    private final Object _allocatedLock = new Object();
-    private final Object _allocatingLock = new Object();
+    private static final Object allocatedLock = new Object();
+    private static final Object allocatingLock = new Object();
 
     static Boolean rulesContinueOnErrFlag = true;
 
@@ -834,7 +834,7 @@ public IPAddressVO doInTransaction(TransactionStatus status) throws Insufficient
     @DB
     @Override
     public void markPublicIpAsAllocated(final IPAddressVO addr) {
-        synchronized (_allocatedLock) {
+        synchronized (allocatedLock) {
             Transaction.execute(new TransactionCallbackNoReturn() {
                 @Override
                 public void doInTransactionWithoutResult(TransactionStatus status) {
@@ -857,6 +857,8 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
                                         _resourceLimitMgr.incrementResourceCount(owner.getId(), ResourceType.public_ip);
                                     }
                                 }
+                            } else {
+                                s_logger.error("Failed to mark public IP as allocated with id=" + addr.getId() + " address=" + addr.getAddress());
                             }
                         }
                     }
@@ -867,14 +869,18 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
 
     @DB
     private void markPublicIpAsAllocating(final IPAddressVO addr) {
-        synchronized (_allocatingLock) {
+        synchronized (allocatingLock) {
             Transaction.execute(new TransactionCallbackNoReturn() {
                 @Override
                 public void doInTransactionWithoutResult(TransactionStatus status) {
 
                     if (_ipAddressDao.lockRow(addr.getId(), true) != null) {
                         addr.setState(IpAddress.State.Allocating);
-                        _ipAddressDao.update(addr.getId(), addr);
+                        if (!_ipAddressDao.update(addr.getId(), addr)) {
+                            s_logger.error("Failed to update public IP as allocating with id=" + addr.getId() + " and address=" + addr.getAddress());
+                        }
+                    } else {
+                        s_logger.error("Failed to lock row to mark public IP as allocating with id=" + addr.getId() + " and address=" + addr.getAddress());
                     }
                 }
             });
@@ -935,8 +941,8 @@ public PublicIp assignDedicateIpAddress(Account owner, final Long guestNtwkId, f
                 displayIp = vpc.isDisplay();
             }
 
-            return fetchNewPublicIp(dcId, null, null, owner, VlanType.VirtualNetwork, guestNtwkId, isSourceNat, false, null, false, vpcId, displayIp);
-
+            ip = fetchNewPublicIp(dcId, null, null, owner, VlanType.VirtualNetwork, guestNtwkId, isSourceNat, true, null, false, vpcId, displayIp);
+            return ip;
         } finally {
             if (owner != null) {
                 if (s_logger.isDebugEnabled()) {

test/integration/smoke/test_vpc_vpn.py

@@ -390,6 +390,7 @@ def test_01_vpc_remote_access_vpn(self):
         finally:
             self.logger.debug("Acquired public ip address: OK")
 
+        vpn = None
         try:
             vpn = Vpn.create(self.apiclient,
                              publicipid=ip.id,