diff --git a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java index 261dc529434..63c55966d5b 100644 --- a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java +++ b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java @@ -1044,36 +1044,48 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage @Override public void markPublicIpAsAllocated(final IPAddressVO addr) { synchronized (allocatedLock) { - Transaction.execute(new TransactionCallbackNoReturn() { + Transaction.execute(new TransactionCallbackWithExceptionNoReturn() { @Override public void doInTransactionWithoutResult(TransactionStatus status) { Account owner = _accountMgr.getAccount(addr.getAllocatedToAccountId()); - if (_ipAddressDao.lockRow(addr.getId(), true) != null) { - final IPAddressVO userIp = _ipAddressDao.findById(addr.getId()); - if (userIp.getState() == IpAddress.State.Allocating || addr.getState() == IpAddress.State.Free || addr.getState() == IpAddress.State.Reserved) { - boolean shouldUpdateIpResourceCount = checkIfIpResourceCountShouldBeUpdated(addr); - addr.setState(IpAddress.State.Allocated); - if (_ipAddressDao.update(addr.getId(), addr)) { - // Save usage event - if (owner.getAccountId() != Account.ACCOUNT_ID_SYSTEM) { - VlanVO vlan = _vlanDao.findById(addr.getVlanId()); - String guestType = vlan.getVlanType().toString(); - if (!isIpDedicated(addr)) { - final boolean usageHidden = isUsageHidden(addr); - UsageEventUtils.publishUsageEvent(EventTypes.EVENT_NET_IP_ASSIGN, owner.getId(), addr.getDataCenterId(), addr.getId(), - addr.getAddress().toString(), addr.isSourceNat(), guestType, addr.getSystem(), usageHidden, - addr.getClass().getName(), addr.getUuid()); - } - if (shouldUpdateIpResourceCount) { - _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()); + final IPAddressVO userIp = _ipAddressDao.lockRow(addr.getId(), true); + if (userIp == null) { + s_logger.error(String.format("Failed to acquire row lock to mark public IP as allocated with ID [%s] and address [%s]", addr.getId(), addr.getAddress())); + return; + } + + List expectedIpAddressStates = List.of(IpAddress.State.Allocating, IpAddress.State.Free, IpAddress.State.Reserved); + if (!expectedIpAddressStates.contains(userIp.getState())) { + s_logger.debug(String.format("Not marking public IP with ID [%s] and address [%s] as allocated, since it is in the [%s] state.", addr.getId(), addr.getAddress(), userIp.getState())); + return; + } + + boolean shouldUpdateIpResourceCount = checkIfIpResourceCountShouldBeUpdated(addr); + addr.setState(IpAddress.State.Allocated); + boolean updatedIpAddress = _ipAddressDao.update(addr.getId(), addr); + if (!updatedIpAddress) { + s_logger.error(String.format("Failed to mark public IP as allocated with ID [%s] and address [%s]", addr.getId(), addr.getAddress())); + return; + } + + if (owner.getAccountId() != Account.ACCOUNT_ID_SYSTEM) { + if (shouldUpdateIpResourceCount) { + try (CheckedReservation publicIpReservation = new CheckedReservation(owner, ResourceType.public_ip, 1L, reservationDao, _resourceLimitMgr)) { + _resourceLimitMgr.incrementResourceCount(owner.getId(), ResourceType.public_ip); + } catch (Exception e) { + _ipAddressDao.unassignIpAddress(addr.getId()); + throw new CloudRuntimeException(e); } } - } else { - s_logger.error("Failed to acquire row lock to mark public IP as allocated with id=" + addr.getId() + " address=" + addr.getAddress()); + + VlanVO vlan = _vlanDao.findById(addr.getVlanId()); + String guestType = vlan.getVlanType().toString(); + if (!isIpDedicated(addr)) { + final boolean usageHidden = isUsageHidden(addr); + UsageEventUtils.publishUsageEvent(EventTypes.EVENT_NET_IP_ASSIGN, owner.getId(), addr.getDataCenterId(), addr.getId(), + addr.getAddress().toString(), addr.isSourceNat(), guestType, addr.getSystem(), usageHidden, + addr.getClass().getName(), addr.getUuid()); + } } } }); @@ -1557,28 +1569,30 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage } boolean isSourceNat = isSourceNatAvailableForNetwork(owner, ipToAssoc, network); - - s_logger.debug("Associating ip " + ipToAssoc + " to network " + network); - + s_logger.debug(String.format("Associating IP [%s] to network [%s].", ipToAssoc, network)); boolean success = false; IPAddressVO ip = null; - try (CheckedReservation publicIpReservation = new CheckedReservation(owner, ResourceType.public_ip, 1l, reservationDao, _resourceLimitMgr)) { - ip = _ipAddressDao.findById(ipId); - //update ip address with networkId - ip.setAssociatedWithNetworkId(networkId); - ip.setSourceNat(isSourceNat); - _ipAddressDao.update(ipId, ip); + try { + Pair updatedIpAddress = Transaction.execute((TransactionCallbackWithException, Exception>) status -> { + IPAddressVO ipAddress = _ipAddressDao.findById(ipId); + ipAddress.setAssociatedWithNetworkId(networkId); + ipAddress.setSourceNat(isSourceNat); + _ipAddressDao.update(ipId, ipAddress); + return new Pair<>(_ipAddressDao.findById(ipId), applyIpAssociations(network, false)); + }); - success = applyIpAssociations(network, false); + ip = updatedIpAddress.first(); + success = updatedIpAddress.second(); if (success) { - s_logger.debug("Successfully associated ip address " + ip.getAddress().addr() + " to network " + network); + s_logger.debug(String.format("Successfully associated IP address [%s] to network [%s]", ip.getAddress().addr(), network)); } else { - s_logger.warn("Failed to associate ip address " + ip.getAddress().addr() + " to network " + network); + s_logger.warn(String.format("Failed to associate IP address [%s] to network [%s]", ip.getAddress().addr(), network)); } - return _ipAddressDao.findById(ipId); + return ip; } catch (Exception e) { - s_logger.error(String.format("Failed to associate ip address %s to network %s", ipToAssoc, network), e); - throw new CloudRuntimeException(String.format("Failed to associate ip address %s to network %s", ipToAssoc, network), e); + String errorMessage = String.format("Failed to associate IP address [%s] to network [%s]", ipToAssoc, network); + s_logger.error(errorMessage, e); + throw new CloudRuntimeException(errorMessage, e); } finally { if (!success && releaseOnFailure) { if (ip != null) { diff --git a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java index 300d6c0109b..4882659f1ce 100644 --- a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java +++ b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java @@ -42,7 +42,6 @@ import javax.annotation.PostConstruct; import javax.inject.Inject; import javax.naming.ConfigurationException; -import com.cloud.resourcelimit.CheckedReservation; import org.apache.cloudstack.acl.ControlledEntity.ACLType; import org.apache.cloudstack.alert.AlertService; import org.apache.cloudstack.annotation.AnnotationService; @@ -2928,32 +2927,27 @@ public class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis // check permissions _accountMgr.checkAccess(caller, null, false, owner, vpc); - s_logger.debug("Associating ip " + ipToAssoc + " to vpc " + vpc); + s_logger.debug(String.format("Associating IP [%s] to VPC [%s]", ipToAssoc, vpc)); final boolean isSourceNatFinal = isSrcNatIpRequired(vpc.getVpcOfferingId()) && getExistingSourceNatInVpc(vpc.getAccountId(), vpcId) == null; - try (CheckedReservation publicIpReservation = new CheckedReservation(owner, ResourceType.public_ip, 1l, reservationDao, _resourceLimitMgr)) { - Transaction.execute(new TransactionCallbackNoReturn() { - @Override - public void doInTransactionWithoutResult(final TransactionStatus status) { + try { + IPAddressVO updatedIpAddress = Transaction.execute((TransactionCallbackWithException) status -> { final IPAddressVO ip = _ipAddressDao.findById(ipId); - // update ip address with networkId ip.setVpcId(vpcId); ip.setSourceNat(isSourceNatFinal); - _ipAddressDao.update(ipId, ip); - - // mark ip as allocated _ipAddrMgr.markPublicIpAsAllocated(ip); - } + return _ipAddressDao.findById(ipId); }); - } catch (Exception e) { - s_logger.error("Failed to associate ip " + ipToAssoc + " to vpc " + vpc, e); - throw new CloudRuntimeException("Failed to associate ip " + ipToAssoc + " to vpc " + vpc, e); - } - s_logger.debug("Successfully assigned ip " + ipToAssoc + " to vpc " + vpc); - CallContext.current().putContextParameter(IpAddress.class, ipToAssoc.getUuid()); - return _ipAddressDao.findById(ipId); + s_logger.debug(String.format("Successfully assigned IP [%s] to VPC [%s]", ipToAssoc, vpc)); + CallContext.current().putContextParameter(IpAddress.class, ipToAssoc.getUuid()); + return updatedIpAddress; + } catch (Exception e) { + String errorMessage = String.format("Failed to associate IP address [%s] to VPC [%s]", ipToAssoc, vpc); + s_logger.error(errorMessage, e); + throw new CloudRuntimeException(errorMessage, e); + } } @Override