diff --git a/plugins/host-allocators/random/src/main/java/com/cloud/agent/manager/allocator/impl/RandomAllocator.java b/plugins/host-allocators/random/src/main/java/com/cloud/agent/manager/allocator/impl/RandomAllocator.java index 53e44ab5aab..42129944a19 100644 --- a/plugins/host-allocators/random/src/main/java/com/cloud/agent/manager/allocator/impl/RandomAllocator.java +++ b/plugins/host-allocators/random/src/main/java/com/cloud/agent/manager/allocator/impl/RandomAllocator.java @@ -94,15 +94,17 @@ public class RandomAllocator extends AdapterBase implements HostAllocator { 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 diff --git a/server/src/main/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java b/server/src/main/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java index 4a5f80571ae..590db3406c2 100644 --- a/server/src/main/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java +++ b/server/src/main/java/com/cloud/agent/manager/allocator/impl/FirstFitAllocator.java @@ -25,7 +25,6 @@ import java.util.Map; 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 class FirstFitAllocator extends AdapterBase implements HostAllocator { // 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 class FirstFitAllocator extends AdapterBase implements HostAllocator { 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 allhostsInCluster = _hostDao.listAllUpAndEnabledNonHAHosts(type, clusterId, podId, dcId, null); diff --git a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java index ea8a66ecf9f..ca47393f434 100644 --- a/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java +++ b/server/src/main/java/com/cloud/network/IpAddressManagerImpl.java @@ -1038,36 +1038,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 { - 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 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 class IpAddressManagerImpl extends ManagerBase implements IpAddressManage 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); + 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) { - 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) { 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 83faa2377c5..dc6b6cd3d40 100644 --- a/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java +++ b/server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java @@ -50,7 +50,6 @@ import com.cloud.dc.dao.ASNumberDao; 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 class VpcManagerImpl extends ManagerBase implements VpcManager, VpcProvis // 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) 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) { - logger.error("Failed to associate ip " + ipToAssoc + " to vpc " + vpc, e); - throw new CloudRuntimeException("Failed to associate ip " + ipToAssoc + " to vpc " + vpc, e); - } - logger.debug("Successfully assigned ip " + ipToAssoc + " to vpc " + vpc); - CallContext.current().putContextParameter(IpAddress.class, ipToAssoc.getUuid()); - 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) { + String errorMessage = String.format("Failed to associate IP address [%s] to VPC [%s]", ipToAssoc, vpc); + logger.error(errorMessage, e); + throw new CloudRuntimeException(errorMessage, e); + } } @Override diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 021c6ff6226..167995a6b5f 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -28,7 +28,6 @@ import java.io.UnsupportedEncodingException; import java.net.URLDecoder; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.Date; import java.util.HashMap; import java.util.HashSet; @@ -3105,42 +3104,6 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } } - boolean isVMware = (vm.getHypervisorType() == HypervisorType.VMware); - - if (securityGroupIdList != null && isVMware) { - throw new InvalidParameterValueException("Security group feature is not supported for vmWare hypervisor"); - } else { - // Get default guest network in Basic zone - Network defaultNetwork = null; - try { - DataCenterVO zone = _dcDao.findById(vm.getDataCenterId()); - if (zone.getNetworkType() == NetworkType.Basic) { - // Get default guest network in Basic zone - defaultNetwork = _networkModel.getExclusiveGuestNetwork(zone.getId()); - } else if (_networkModel.checkSecurityGroupSupportForNetwork(_accountMgr.getActiveAccountById(vm.getAccountId()), zone, Collections.emptyList(), securityGroupIdList)) { - NicVO defaultNic = _nicDao.findDefaultNicForVM(vm.getId()); - if (defaultNic != null) { - defaultNetwork = _networkDao.findById(defaultNic.getNetworkId()); - } - } - } catch (InvalidParameterValueException e) { - if(logger.isDebugEnabled()) { - logger.debug(e.getMessage(),e); - } - defaultNetwork = _networkModel.getDefaultNetworkForVm(id); - } - - if (securityGroupIdList != null && _networkModel.isSecurityGroupSupportedInNetwork(defaultNetwork) && _networkModel.canAddDefaultSecurityGroup()) { - if (vm.getState() == State.Stopped) { - // Remove instance from security groups - _securityGroupMgr.removeInstanceFromGroups(vm); - // Add instance in provided groups - _securityGroupMgr.addInstanceToGroups(vm, securityGroupIdList); - } else { - throw new InvalidParameterValueException("Virtual machine must be stopped prior to update security groups "); - } - } - } List nics = _nicDao.listByVmId(vm.getId()); if (hostName != null) { // Check is hostName is RFC compliant @@ -3173,6 +3136,8 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir .getUuid(), nic.getId(), extraDhcpOptionsMap); } + checkAndUpdateSecurityGroupForVM(securityGroupIdList, vm, networks); + _vmDao.updateVM(id, displayName, ha, osTypeId, userData, userDataId, userDataDetails, isDisplayVmEnabled, isDynamicallyScalable, deleteProtection, customId, hostName, instanceName); @@ -3188,6 +3153,48 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir return _vmDao.findById(id); } + private void checkAndUpdateSecurityGroupForVM(List securityGroupIdList, UserVmVO vm, List networks) { + boolean isVMware = (vm.getHypervisorType() == HypervisorType.VMware); + + if (securityGroupIdList != null && isVMware) { + throw new InvalidParameterValueException("Security group feature is not supported for VMware hypervisor"); + } else if (securityGroupIdList != null) { + DataCenterVO zone = _dcDao.findById(vm.getDataCenterId()); + List networkIds = new ArrayList<>(); + try { + if (zone.getNetworkType() == NetworkType.Basic) { + // Get default guest network in Basic zone + Network defaultNetwork = _networkModel.getExclusiveGuestNetwork(zone.getId()); + networkIds.add(defaultNetwork.getId()); + } else { + networkIds = networks.stream().map(Network::getId).collect(Collectors.toList()); + } + } catch (InvalidParameterValueException e) { + if(logger.isDebugEnabled()) { + logger.debug(e.getMessage(),e); + } + } + + if (_networkModel.checkSecurityGroupSupportForNetwork( + _accountMgr.getActiveAccountById(vm.getAccountId()), + zone, networkIds, securityGroupIdList) + ) { + updateSecurityGroup(vm, securityGroupIdList); + } + } + } + + private void updateSecurityGroup(UserVmVO vm, List securityGroupIdList) { + if (vm.getState() == State.Stopped) { + // Remove instance from security groups + _securityGroupMgr.removeInstanceFromGroups(vm); + // Add instance in provided groups + _securityGroupMgr.addInstanceToGroups(vm, securityGroupIdList); + } else { + throw new InvalidParameterValueException(String.format("VM %s must be stopped prior to update security groups", vm.getUuid())); + } + } + protected void updateUserData(UserVm vm) throws ResourceUnavailableException, InsufficientCapacityException { boolean result = updateUserDataInternal(vm); if (result) { @@ -3695,7 +3702,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir boolean isVmWare = (template.getHypervisorType() == HypervisorType.VMware || (hypervisor != null && hypervisor == HypervisorType.VMware)); if (securityGroupIdList != null && isVmWare) { - throw new InvalidParameterValueException("Security group feature is not supported for vmWare hypervisor"); + throw new InvalidParameterValueException("Security group feature is not supported for VMware hypervisor"); } else if (!isVmWare && _networkModel.isSecurityGroupSupportedInNetwork(defaultNetwork) && _networkModel.canAddDefaultSecurityGroup()) { //add the default securityGroup only if no security group is specified if (securityGroupIdList == null || securityGroupIdList.isEmpty()) { @@ -3755,7 +3762,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } else if (securityGroupIdList != null && !securityGroupIdList.isEmpty()) { if (isVmWare) { - throw new InvalidParameterValueException("Security group feature is not supported for vmWare hypervisor"); + throw new InvalidParameterValueException("Security group feature is not supported for VMware hypervisor"); } // Only one network can be specified, and it should be security group enabled if (networkIdList.size() > 1 && template.getHypervisorType() != HypervisorType.KVM && hypervisor != HypervisorType.KVM) { diff --git a/ui/public/locales/en.json b/ui/public/locales/en.json index f000d9f2b49..7bd555b36c3 100644 --- a/ui/public/locales/en.json +++ b/ui/public/locales/en.json @@ -2848,7 +2848,7 @@ "message.attach.volume.progress": "Attaching volume", "message.attach.volume.success": "Successfully attached the volume to the instance", "message.authorization.failed": "Session expired, authorization verification failed.", -"message.autoscale.loadbalancer.update": "The load balancer rule can be updated only when autoscaling group is DISABLED.", +"message.autoscale.loadbalancer.update": "The load balancer rule can be updated. However, instance can be removed only when autoscaling group is DISABLED.", "message.autoscale.policies.update": "The scale up/down policies can be updated only when autoscaling group is DISABLED.", "message.autoscale.vm.networks": "Please choose at least one Network for Instances in the autoscaling group. The default Network must be an Isolated Network or VPC Network Tier which supports Instance AutoScaling and has load balancing rules.", "message.autoscale.vmprofile.update": "The autoscale Instance profile can be updated only when autoscaling group is DISABLED.", diff --git a/ui/src/views/compute/EditVM.vue b/ui/src/views/compute/EditVM.vue index f2d679ee444..75a297cee3e 100644 --- a/ui/src/views/compute/EditVM.vue +++ b/ui/src/views/compute/EditVM.vue @@ -206,7 +206,7 @@ export default { zoneid: this.resource.zoneid }).then(response => { const zone = response?.listzonesresponse?.zone || [] - this.securityGroupsEnabled = zone?.[0]?.securitygroupsenabled + this.securityGroupsEnabled = zone?.[0]?.securitygroupsenabled || this.$store.getters.showSecurityGroups }) }, fetchSecurityGroups () { diff --git a/ui/src/views/compute/InstanceTab.vue b/ui/src/views/compute/InstanceTab.vue index b22f576e70a..925f707591a 100644 --- a/ui/src/views/compute/InstanceTab.vue +++ b/ui/src/views/compute/InstanceTab.vue @@ -179,6 +179,7 @@ export default { vm: {}, totalStorage: 0, currentTab: 'details', + showUpdateSecurityGroupsModal: false, showAddVolumeModal: false, diskOfferings: [], annotations: [],