From 0e3ddb2975d352a3af744454b0b1891b79188aee Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Thu, 21 Dec 2017 00:31:51 +0530 Subject: [PATCH] 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 --- .../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 ef6584f9537..4f3fc51319c 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 AssignIpAddressSearch; SearchBuilder 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 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) { @@ -857,6 +857,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()); } } } @@ -867,14 +869,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()); } } }); @@ -935,8 +941,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 ddf76930050..df60686f229 100644 --- a/test/integration/smoke/test_vpc_vpn.py +++ b/test/integration/smoke/test_vpc_vpn.py @@ -390,6 +390,7 @@ class TestVpcRemoteAccessVpn(cloudstackTestCase): finally: self.logger.debug("Acquired public ip address: OK") + vpn = None try: vpn = Vpn.create(self.apiclient, publicipid=ip.id,