Merge branch '4.20' of https://github.com/apache/cloudstack

Author: Pearl1594

plugins/host-allocators/random/src/main/java/com/cloud/agent/manager/allocator/impl/RandomAllocator.java

@@ -94,15 +94,17 @@ private List<Host> findSuitableHosts(VirtualMachineProfile vmProfile, Deployment
             return suitableHosts;
         }
         String offeringHostTag = offering.getHostTag();
+
         VMTemplateVO template = (VMTemplateVO)vmProfile.getTemplate();
         String templateTag = template.getTemplateTag();
         String hostTag = null;
-        if (ObjectUtils.anyNull(offeringHostTag, templateTag)) {
-            hostTag = offeringHostTag;
-            hostTag = hostTag == null ? templateTag : String.format("%s, %s", hostTag, templateTag);
-            logger.debug(String.format("Looking for hosts in dc [%s], pod [%s], cluster [%s] and complying with host tag(s): [%s]", dcId, podId, clusterId, hostTag));
+        if (ObjectUtils.anyNotNull(offeringHostTag, templateTag)) {
+            hostTag = ObjectUtils.allNotNull(offeringHostTag, templateTag) ?
+                    String.format("%s, %s", offeringHostTag, templateTag) :
+                    ObjectUtils.firstNonNull(offeringHostTag, templateTag);
+            logger.debug("Looking for hosts in dc [{}], pod [{}], cluster [{}] and complying with host tag(s): [{}]", dcId, podId, clusterId, hostTag);
         } else {
-            logger.debug("Looking for hosts in dc: " + dcId + "  pod:" + podId + "  cluster:" + clusterId);
+            logger.debug("Looking for hosts in dc: {} pod: {} cluster: {}", dcId , podId, clusterId);
         }
         if (hosts != null) {
             // retain all computing hosts, regardless of whether they support routing...it's random after all

server/src/main/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java

@@ -25,7 +25,6 @@
 import javax.inject.Inject;
 import javax.naming.ConfigurationException;
 
-import com.cloud.utils.exception.CloudRuntimeException;
 import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
 import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
 import org.springframework.stereotype.Component;
@@ -130,8 +129,8 @@ public List<Host> allocateTo(VirtualMachineProfile vmProfile, DeploymentPlan pla
             // FirstFitAllocator should be used for user VMs only since it won't care whether the host is capable of routing or not
             return new ArrayList<>();
         }
-
-        logger.debug("Looking for hosts in zone [{}], pod [{}], cluster [{}]", dcId, podId, clusterId);
+        String paramAsStringToLog = String.format("zone [%s], pod [%s], cluster [%s]", dcId, podId, clusterId);
+        logger.debug("Looking for hosts in {}", paramAsStringToLog);
 
         String hostTagOnOffering = offering.getHostTag();
         String hostTagOnTemplate = template.getTemplateTag();
@@ -203,8 +202,8 @@ public List<Host> allocateTo(VirtualMachineProfile vmProfile, DeploymentPlan pla
 
 
         if (clusterHosts.isEmpty()) {
-            logger.error("No suitable host found for vm [{}] with tags [{}].", vmProfile, hostTagOnOffering);
-            throw new CloudRuntimeException(String.format("No suitable host found for vm [%s].", vmProfile));
+            logger.warn("No suitable host found for VM [{}] with tags {} in {}.", vmProfile, hostTagOnOffering, paramAsStringToLog);
+            return null;
         }
         // add all hosts that we are not considering to the avoid list
         List<HostVO> allhostsInCluster = _hostDao.listAllUpAndEnabledNonHAHosts(type, clusterId, podId, dcId, null);

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

@@ -1038,36 +1038,48 @@ public List<IPAddressVO> listAvailablePublicIps(final long dcId, final Long podI
     @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.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 {
-                                logger.error("Failed to mark public IP as allocated: {}", addr);
+                    final IPAddressVO userIp = _ipAddressDao.lockRow(addr.getId(), true);
+                    if (userIp == null) {
+                        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())) {
+                        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) {
+                        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 {
-                        logger.error("Failed to acquire row lock to mark public IP as allocated: {}", addr);
+
+                        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());
+                        }
                     }
                 }
             });
@@ -1553,27 +1565,31 @@ public IPAddressVO associateIPToGuestNetwork(long ipId, long networkId, boolean
 
         boolean isSourceNat = isSourceNatAvailableForNetwork(owner, ipToAssoc, network);
 
-        logger.debug("Associating ip " + ipToAssoc + " to network " + network);
+        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);
-
-            success = applyIpAssociations(network, false);
+        try {
+            Pair<IPAddressVO, Boolean> updatedIpAddress = Transaction.execute((TransactionCallbackWithException<Pair<IPAddressVO, Boolean>, 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));
+            });
+
+            ip = updatedIpAddress.first();
+            success = updatedIpAddress.second();
             if (success) {
-                logger.debug("Successfully associated ip address " + ip.getAddress().addr() + " to network " + network);
+                logger.debug(String.format("Successfully associated IP address [%s] to network [%s]", ip.getAddress().addr(), network));
             } else {
-                logger.warn("Failed to associate ip address " + ip.getAddress().addr() + " to network " + network);
+                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) {
-            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);
+            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

@@ -50,7 +50,6 @@
 import com.cloud.dc.Vlan;
 import com.cloud.network.dao.NsxProviderDao;
 import com.cloud.network.element.NsxProviderVO;
-import com.cloud.resourcelimit.CheckedReservation;
 import com.google.common.collect.Sets;
 import org.apache.cloudstack.acl.ControlledEntity.ACLType;
 import org.apache.cloudstack.alert.AlertService;
@@ -3206,32 +3205,27 @@ public IpAddress associateIPToVpc(final long ipId, final long vpcId) throws Reso
         // check permissions
         _accountMgr.checkAccess(caller, null, false, owner, vpc);
 
-        logger.debug("Associating ip " + ipToAssoc + " to vpc " + vpc);
+        logger.debug(String.format("Associating IP [%s] to VPC [%s]", ipToAssoc, vpc));
 
         final boolean isSourceNatFinal = isSrcNatIpRequired(vpc.getVpcOfferingId()) && getExistingSourceNatInVpc(vpc.getAccountId(), vpcId, false) == 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<IPAddressVO, CloudRuntimeException>) 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);
             });
+
+            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) {
-            logger.error("Failed to associate ip " + ipToAssoc + " to vpc " + vpc, e);
-            throw new CloudRuntimeException("Failed to associate ip " + ipToAssoc + " to vpc " + vpc, e);
+            String errorMessage = String.format("Failed to associate IP address [%s] to VPC [%s]", ipToAssoc, vpc);
+            logger.error(errorMessage, e);
+            throw new CloudRuntimeException(errorMessage, e);
         }
-
-        logger.debug("Successfully assigned ip " + ipToAssoc + " to vpc " + vpc);
-        CallContext.current().putContextParameter(IpAddress.class, ipToAssoc.getUuid());
-        return _ipAddressDao.findById(ipId);
     }
 
     @Override