You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ro...@apache.org on 2017/12/20 10:22:31 UTC

[cloudstack] 22/22: CLOUDSTACK-9595: Fix regression introduced in #1762

This is an automated email from the ASF dual-hosted git repository.

rohit pushed a commit to branch debian9-systemvmtemplate
in repository https://gitbox.apache.org/repos/asf/cloudstack.git

commit b51f4c61f5379991f5efb040656dbd4d61ee9c91
Author: Rohit Yadav <ro...@shapeblue.com>
AuthorDate: Wed Dec 20 15:20:34 2017 +0530

    CLOUDSTACK-9595: Fix regression introduced in #1762
    
    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 <ro...@shapeblue.com>
---
 .../src/com/cloud/network/IpAddressManagerImpl.java  | 20 +++++++++++++-------
 test/integration/smoke/test_vpc_vpn.py               |  1 +
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/server/src/com/cloud/network/IpAddressManagerImpl.java b/server/src/com/cloud/network/IpAddressManagerImpl.java
index 7961d07..6af6b03 100644
--- a/server/src/com/cloud/network/IpAddressManagerImpl.java
+++ b/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;
 
@@ -835,7 +835,7 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage
     @DB
     @Override
     public void markPublicIpAsAllocated(final IPAddressVO addr) {
-        synchronized (_allocatedLock) {
+        synchronized (allocatedLock) {
             Transaction.execute(new TransactionCallbackNoReturn() {
                 @Override
                 public void doInTransactionWithoutResult(TransactionStatus status) {
@@ -858,6 +858,8 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage
                                         _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());
                             }
                         }
                     }
@@ -868,14 +870,18 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage
 
     @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());
                     }
                 }
             });
@@ -936,8 +942,8 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage
                 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()) {
diff --git a/test/integration/smoke/test_vpc_vpn.py b/test/integration/smoke/test_vpc_vpn.py
index 8e1a73d..8c873d2 100644
--- a/test/integration/smoke/test_vpc_vpn.py
+++ b/test/integration/smoke/test_vpc_vpn.py
@@ -344,6 +344,7 @@ class TestVpcRemoteAccessVpn(cloudstackTestCase):
         finally:
             self.logger.debug("Acquired public ip address: OK")
 
+        vpn = None
         try:
             vpn = Vpn.create(self.apiclient,
                              publicipid=ip.id,

-- 
To stop receiving notification emails like this one, please contact
"commits@cloudstack.apache.org" <co...@cloudstack.apache.org>.