Remove the validation of the amount of acquired public IPs when enabling static NAT, adding PF and LB rules on VPC public IPs (#10568)

* fix inconsistency on public IP limits

* fix log message

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

@@ -1044,18 +1044,40 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage
     @Override
     public void markPublicIpAsAllocated(final IPAddressVO addr) {
         synchronized (allocatedLock) {
-            Transaction.execute(new TransactionCallbackNoReturn() {
+            Transaction.execute(new TransactionCallbackWithExceptionNoReturn<CloudRuntimeException>() {
                 @Override
                 public void doInTransactionWithoutResult(TransactionStatus status) {
                     Account owner = _accountMgr.getAccount(addr.getAllocatedToAccountId());
-                    if (_ipAddressDao.lockRow(addr.getId(), true) != null) {
+                    final IPAddressVO userIp = _ipAddressDao.lockRow(addr.getId(), true);
-                        final IPAddressVO userIp = _ipAddressDao.findById(addr.getId());
+                    if (userIp == null) {
-                        if (userIp.getState() == IpAddress.State.Allocating || addr.getState() == IpAddress.State.Free || addr.getState() == IpAddress.State.Reserved) {
+                        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<IpAddress.State> 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);
-                            if (_ipAddressDao.update(addr.getId(), addr)) {
+                    boolean updatedIpAddress = _ipAddressDao.update(addr.getId(), addr);
-                                // Save usage event
+                    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);
+                            }
+                        }
+
                         VlanVO vlan = _vlanDao.findById(addr.getVlanId());
                         String guestType = vlan.getVlanType().toString();
                         if (!isIpDedicated(addr)) {
@@ -1064,16 +1086,6 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage
                                     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());
-                            }
-                        }
-                    } else {
-                        s_logger.error("Failed to acquire row lock to mark public IP as allocated with id=" + addr.getId() + " address=" + addr.getAddress());
                     }
                 }
             });
@@ -1557,28 +1569,30 @@ public class IpAddressManagerImpl extends ManagerBase implements IpAddressManage
         }
 
         boolean isSourceNat = isSourceNatAvailableForNetwork(owner, ipToAssoc, network);
-
+        s_logger.debug(String.format("Associating IP [%s] to network [%s].", ipToAssoc, network));
-        s_logger.debug("Associating ip " + ipToAssoc + " to network " + network);
-
         boolean success = false;
         IPAddressVO ip = null;
-        try (CheckedReservation publicIpReservation = new CheckedReservation(owner, ResourceType.public_ip, 1l, reservationDao, _resourceLimitMgr)) {
+        try {
-            ip = _ipAddressDao.findById(ipId);
+            Pair<IPAddressVO, Boolean> updatedIpAddress = Transaction.execute((TransactionCallbackWithException<Pair<IPAddressVO, Boolean>, Exception>) status -> {
-            //update ip address with networkId
+                IPAddressVO ipAddress = _ipAddressDao.findById(ipId);
-            ip.setAssociatedWithNetworkId(networkId);
+                ipAddress.setAssociatedWithNetworkId(networkId);
-            ip.setSourceNat(isSourceNat);
+                ipAddress.setSourceNat(isSourceNat);
-            _ipAddressDao.update(ipId, ip);
+                _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);
+            String errorMessage = String.format("Failed to associate IP address [%s] to network [%s]", ipToAssoc, network);
-            throw new CloudRuntimeException(String.format("Failed to associate ip address %s to network %s", ipToAssoc, network), e);
+            s_logger.error(errorMessage, e);
+            throw new CloudRuntimeException(errorMessage, e);
         } finally {
             if (!success && releaseOnFailure) {
                 if (ip != null) {

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)) {
+        try {
-            Transaction.execute(new TransactionCallbackNoReturn() {
+            IPAddressVO updatedIpAddress = Transaction.execute((TransactionCallbackWithException<IPAddressVO, CloudRuntimeException>) status -> {
-                @Override
-                public void doInTransactionWithoutResult(final TransactionStatus 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);
-                }
-            });
-        } 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