From ee51245abb19c81624178c8e5386ab2f286575ca Mon Sep 17 00:00:00 2001 From: Charles Queiroz Date: Wed, 22 Feb 2023 01:11:48 +0000 Subject: [PATCH 1/9] adding count resource domain --- .../com/cloud/vm/VirtualMachineManager.java | 29 +- .../cloud/vm/VirtualMachineManagerImpl.java | 184 +++- .../ConfigurationManagerImpl.java | 39 +- .../network/router/NetworkHelperImpl.java | 40 +- .../ResourceLimitManagerImpl.java | 125 ++- .../component/test_router_resources.py | 824 ++++++++++++++++++ utils/src/main/java/com/cloud/utils/Pair.java | 4 + 7 files changed, 1202 insertions(+), 43 deletions(-) create mode 100644 test/integration/component/test_router_resources.py diff --git a/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java b/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java index ada94cd057c1..aba330f237ce 100644 --- a/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java +++ b/engine/api/src/main/java/com/cloud/vm/VirtualMachineManager.java @@ -22,6 +22,7 @@ import java.util.List; import java.util.Map; +import com.cloud.utils.exception.CloudRuntimeException; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.framework.config.ConfigKey; @@ -83,10 +84,21 @@ public interface VirtualMachineManager extends Manager { ConfigKey AllowExposeDomainInMetadata = new ConfigKey<>("Advanced", Boolean.class, "metadata.allow.expose.domain", "false", "If set to true, it allows the VM's domain to be seen in metadata.", true, ConfigKey.Scope.Domain); + ConfigKey ResourceCountRouters = new ConfigKey<>("Advanced", Boolean.class, "resource.count.routers", + "false","Count the CPU and memory resource count of virtual routers towards domain resource calculation", + true, ConfigKey.Scope.Domain); + + ConfigKey ResourceCountRoutersType = new ConfigKey<>("Advanced", String.class, "resource.count.routers.type", "all", + "Possible values are all and delta. If value is all then entire VR cpu and ram are counted else diff " + + "between current VR offering and default VR offering is considered", true, ConfigKey.Scope.Domain); + interface Topics { String VM_POWER_STATE = "vm.powerstate"; } + static final String COUNT_ALL_VR_RESOURCES = "all"; + static final String COUNT_DELTA_VR_RESOURCES = "delta"; + /** * Allocates a new virtual machine instance in the CloudStack DB. This * orchestrates the creation of all virtual resources needed in CloudStack @@ -97,8 +109,7 @@ interface Topics { * define this VM but it must be unique for all of CloudStack. * @param template The template this VM is based on. * @param serviceOffering The service offering that specifies the offering this VM should provide. - * @param defaultNetwork The default network for the VM. - * @param rootDiskOffering For created VMs not based on templates, root disk offering specifies the root disk. + * @param rootDiskOfferingInfo For created VMs not based on templates, root disk offering specifies the root disk. * @param dataDiskOfferings Data disks to attach to the VM. * @param auxiliaryNetworks additional networks to attach the VMs to. * @param plan How to deploy the VM. @@ -173,9 +184,9 @@ void advanceReboot(String vmUuid, Map param void checkIfCanUpgrade(VirtualMachine vmInstance, ServiceOffering newServiceOffering); /** - * @param vmId - * @param serviceOfferingId - * @return + * @param vmId the vm id + * @param newServiceOffering the new service offering + * @return true if the vm db was upgraded, false otherwise */ boolean upgradeVmDb(long vmId, ServiceOffering newServiceOffering, ServiceOffering currentServiceOffering); @@ -219,9 +230,8 @@ NicProfile addVmToNetwork(VirtualMachine vm, Network network, NicProfile request NicTO toNicTO(NicProfile nic, HypervisorType hypervisorType); /** - * @param profile - * @param hvGuru - * @return + * @param profile the vm profile + * @return the vmTO */ VirtualMachineTO toVmTO(VirtualMachineProfile profile); @@ -285,4 +295,7 @@ static String getHypervisorHostname(String name) { HashMap> getVmNetworkStatistics(long hostId, String hostName, Map vmMap); + void incrementVrResourceCount(ServiceOffering offering, Account owner, boolean isDeployOrDestroy) throws CloudRuntimeException; + + void decrementVrResourceCount(ServiceOffering offering, Account owner, boolean isDeployOrDestroy) throws CloudRuntimeException; } diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index 774950855c45..af69f3f8f3ab 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -47,6 +47,8 @@ import javax.naming.ConfigurationException; import javax.persistence.EntityExistsException; +import com.cloud.exception.ResourceAllocationException; +import com.cloud.network.router.VirtualNetworkApplianceManager; import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao; import org.apache.cloudstack.annotation.AnnotationService; import org.apache.cloudstack.annotation.dao.AnnotationDao; @@ -1072,7 +1074,13 @@ public void orchestrateStart(final String vmUuid, final Map para } } + /** + * Returns the service offering by the given configuration. + * + * @return the service offering found or null if not found + */ + public ServiceOffering getServiceOfferingByConfig() { + ServiceOffering defaultRouterOffering = null; + final String globalRouterOffering = VirtualNetworkApplianceManager.VirtualRouterServiceOffering.value(); + + if (globalRouterOffering != null) { + defaultRouterOffering = _serviceOfferingDao.findByUuid(globalRouterOffering); + } + + if (defaultRouterOffering == null) { + defaultRouterOffering = _serviceOfferingDao.findByName(ServiceOffering.routerDefaultOffUniqueName); + } + + return defaultRouterOffering; + } + + /** + * Counts VR resources for the domain if global setting is true. + * If the value is "all", counts all VR resources, otherwise get the difference between + * current VR offering and default VR offering. + * + * @param offering VR service offering + * @param defaultRouterOffering default VR service offering + * @param owner account + * @return a Pair of CPU and RAM + */ + private Pair resolveCpuAndMemoryCount(ServiceOffering offering, ServiceOffering defaultRouterOffering, Account owner) { + Integer cpuCount = 0; + Integer memoryCount = 0; + if (COUNT_ALL_VR_RESOURCES.equalsIgnoreCase(ResourceCountRoutersType.valueIn(owner.getDomainId()))) { + cpuCount = offering.getCpu(); + memoryCount = offering.getRamSize(); + } else if (COUNT_DELTA_VR_RESOURCES.equalsIgnoreCase(ResourceCountRoutersType.valueIn(owner.getDomainId()))) { + // Default offering value can be greater than current offering value + if (offering.getCpu() >= defaultRouterOffering.getCpu()) { + cpuCount = offering.getCpu() - defaultRouterOffering.getCpu(); + } + if (offering.getRamSize() >= defaultRouterOffering.getRamSize()) { + memoryCount = offering.getRamSize() - defaultRouterOffering.getRamSize(); + } + } + + return Pair.of(cpuCount.longValue(), memoryCount.longValue()); + } + + private void validateResourceCount(Pair cpuMemoryCount, Account owner) { + final Long cpuCount = cpuMemoryCount.first(); + final Long memoryCount = cpuMemoryCount.second(); + try { + if (cpuCount > 0) { + _resourceLimitMgr.checkResourceLimit(owner, ResourceType.cpu, cpuCount); + } + if (memoryCount > 0) { + _resourceLimitMgr.checkResourceLimit(owner, ResourceType.memory, memoryCount); + } + } catch (ResourceAllocationException ex) { + throw new CloudRuntimeException(String.format("Unable to deploy/start routers due to {}.", ex.getMessage())); + } + } + + /** + * Checks if resource count can be allocated to the account/domain. + * + * @param cpuMemoryCount a Pair of cpu and ram + * @param owner the account + */ + private void calculateResourceCount(Pair cpuMemoryCount, Account owner, boolean isIncrement) { + validateResourceCount(cpuMemoryCount, owner); + final Long cpuCount = cpuMemoryCount.first(); + final Long memoryCount = cpuMemoryCount.second(); + + // Increment the resource count + if (s_logger.isDebugEnabled()) { + if (isIncrement) { + s_logger.debug(String.format("Incrementing the CPU count with value %s and RAM value with %s.", cpuCount, memoryCount)); + } else { + s_logger.debug(String.format("Decrementing CPU resource count with value %s and memory resource with value %s.",cpuCount, memoryCount)); + } + } + + if(isIncrement) { + _resourceLimitMgr.incrementResourceCount(owner.getAccountId(), ResourceType.cpu, cpuCount); + _resourceLimitMgr.incrementResourceCount(owner.getAccountId(), ResourceType.memory, memoryCount); + } else { + _resourceLimitMgr.decrementResourceCount(owner.getAccountId(), ResourceType.cpu, cpuCount); + _resourceLimitMgr.decrementResourceCount(owner.getAccountId(), ResourceType.memory, memoryCount); + } + } + + /** + * Increments the VR resource count. + * If the global setting resource.count.router is true then the VR + * resource count will be considered as well. + * If the global setting resource.count.router.type is "all" then + * the total VR resource count will be considered, otherwise the difference between + * the current VR service offering and the default offering will + * be considered. + * During router deployment/destroy, we increment the resource + * count only if resource.count.running.vms is false, otherwise + * we increment it during VR start/stop. Same applies for + * decrementing resource count. + * + * @param offering VR service offering + * @param owner account + * @param isDeployOrDestroy true if router is being deployed/destroyed + */ + @Override + public void incrementVrResourceCount(ServiceOffering offering, Account owner, boolean isDeployOrDestroy) { + if (isDeployOrDestroy == Boolean.TRUE.equals(ResourceCountRunningVMsonly.value())) { + return; + } + + final ServiceOffering defaultRouterOffering = getServiceOfferingByConfig(); + final Pair cpuMemoryCount = resolveCpuAndMemoryCount(offering, defaultRouterOffering, owner); + calculateResourceCount(cpuMemoryCount, owner, true); + } + + /** + * Decrements the VR resource count. + * + * @param offering VR service offering + * @param owner account + * @param isDeployOrDestroy true if router is being deployed/destroyed + */ + @Override + public void decrementVrResourceCount(ServiceOffering offering, Account owner, boolean isDeployOrDestroy) { + if (isDeployOrDestroy == Boolean.TRUE.equals(ResourceCountRunningVMsonly.value())) { + return; + } + + final ServiceOffering defaultRouterOffering = getServiceOfferingByConfig(); + final Pair cpuMemoryCount = resolveCpuAndMemoryCount(offering, defaultRouterOffering, owner); + calculateResourceCount(cpuMemoryCount, owner, false); + } + private void resetVmNicsDeviceId(Long vmId) { final List nics = _nicsDao.listByVmId(vmId); - Collections.sort(nics, new Comparator() { - @Override - public int compare(NicVO nic1, NicVO nic2) { - Long nicDevId1 = Long.valueOf(nic1.getDeviceId()); - Long nicDevId2 = Long.valueOf(nic2.getDeviceId()); - return nicDevId1.compareTo(nicDevId2); - } + nics.sort((nic1, nic2) -> { + Long nicDevId1 = (long) nic1.getDeviceId(); + Long nicDevId2 = (long) nic2.getDeviceId(); + return nicDevId1.compareTo(nicDevId2); }); int deviceId = 0; for (final NicVO nic : nics) { @@ -2126,10 +2273,13 @@ private void advanceStop(final VMInstanceVO vm, final boolean cleanUpEvenIfUnabl boolean result = stateTransitTo(vm, Event.OperationSucceeded, null); if (result) { + ServiceOfferingVO offering = _offeringDao.findById(vm.getId(), vm.getServiceOfferingId()); if (VirtualMachine.Type.User.equals(vm.type) && ResourceCountRunningVMsonly.value()) { - ServiceOfferingVO offering = _offeringDao.findById(vm.getId(), vm.getServiceOfferingId()); - resourceCountDecrement(vm.getAccountId(),new Long(offering.getCpu()), new Long(offering.getRamSize())); + resourceCountDecrement(vm.getAccountId(),Long.valueOf(offering.getCpu()), Long.valueOf(offering.getRamSize())); } + + final Account owner = _entityMgr.findById(Account.class, vm.getAccountId()); + updateVrCountResourceBy(vm.type, vm.getDomainId(), offering, owner, true); } else { throw new CloudRuntimeException("unable to stop " + vm); } @@ -2140,6 +2290,16 @@ private void advanceStop(final VMInstanceVO vm, final boolean cleanUpEvenIfUnabl } } + private void updateVrCountResourceBy(VirtualMachine.Type type, long domainId, ServiceOffering offering, Account owner, boolean decrement) { + if (VirtualMachine.Type.DomainRouter.equals(type) && Boolean.TRUE.equals(ResourceCountRouters.valueIn(domainId))) { + if (decrement) { + decrementVrResourceCount(offering, owner, true); + } else { + incrementVrResourceCount(offering, owner, true); + } + } + } + private void setStateMachine() { _stateMachine = VirtualMachine.State.getStateMachine(); } @@ -4647,7 +4807,7 @@ public ConfigKey[] getConfigKeys() { VmOpLockStateRetry, VmOpWaitInterval, ExecuteInSequence, VmJobCheckInterval, VmJobTimeout, VmJobStateReportInterval, VmConfigDriveLabel, VmConfigDriveOnPrimaryPool, VmConfigDriveForceHostCacheUse, VmConfigDriveUseHostCacheOnUnsupportedPool, HaVmRestartHostUp, ResourceCountRunningVMsonly, AllowExposeHypervisorHostname, AllowExposeHypervisorHostnameAccountLevel, SystemVmRootDiskSize, - AllowExposeDomainInMetadata + AllowExposeDomainInMetadata, ResourceCountRouters, ResourceCountRoutersType }; } diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index c5a489ba8325..81a05fb08e15 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -46,6 +46,7 @@ import javax.naming.ConfigurationException; +import com.cloud.vm.VirtualMachineManager; import org.apache.cloudstack.acl.SecurityChecker; import org.apache.cloudstack.affinity.AffinityGroup; import org.apache.cloudstack.affinity.AffinityGroupService; @@ -293,6 +294,7 @@ import com.googlecode.ipv6.IPv6Address; import com.googlecode.ipv6.IPv6Network; +import static org.apache.commons.lang3.StringUtils.isBlank; public class ConfigurationManagerImpl extends ManagerBase implements ConfigurationManager, ConfigurationService, Configurable { public static final Logger s_logger = Logger.getLogger(ConfigurationManagerImpl.class); @@ -458,6 +460,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati protected Set configValuesForValidation; private Set weightBasedParametersForValidation; private Set overprovisioningFactorsForValidation; + private Set resourceCountRoutersTypeValues; public static final String VM_USERDATA_MAX_LENGTH_STRING = "vm.userdata.max.length"; public static final ConfigKey SystemVMUseLocalStorage = new ConfigKey(Boolean.class, "system.vm.use.local.storage", "Advanced", "false", @@ -517,6 +520,7 @@ public boolean configure(final String name, final Map params) th populateConfigValuesForValidationSet(); weightBasedParametersForValidation(); overProvisioningFactorsForValidation(); + populateConfigValuesForRouterResourceCountValidation(); initMessageBusListener(); return true; } @@ -580,6 +584,12 @@ private void overProvisioningFactorsForValidation() { overprovisioningFactorsForValidation.add(CapacityManager.StorageOverprovisioningFactor.key()); } + private void populateConfigValuesForRouterResourceCountValidation() { + resourceCountRoutersTypeValues = new HashSet<>(); + resourceCountRoutersTypeValues.add(VirtualMachineManager.COUNT_ALL_VR_RESOURCES); + resourceCountRoutersTypeValues.add(VirtualMachineManager.COUNT_DELTA_VR_RESOURCES); + } + private void initMessageBusListener() { messageBus.subscribe(EventTypes.EVENT_CONFIGURATION_VALUE_EDIT, new MessageSubscriber() { @Override @@ -618,12 +628,12 @@ public boolean start() { if (localCidrs != null && localCidrs.length > 0) { s_logger.warn("Management network CIDR is not configured originally. Set it default to " + localCidrs[0]); - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_MANAGMENT_NODE, 0, new Long(0), "Management network CIDR is not configured originally. Set it default to " + _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_MANAGMENT_NODE, 0, 0L, "Management network CIDR is not configured originally. Set it default to " + localCidrs[0], ""); _configDao.update(Config.ManagementNetwork.key(), Config.ManagementNetwork.getCategory(), localCidrs[0]); } else { s_logger.warn("Management network CIDR is not properly configured and we are not able to find a default setting"); - _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_MANAGMENT_NODE, 0, new Long(0), + _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_MANAGMENT_NODE, 0, 0L, "Management network CIDR is not properly configured and we are not able to find a default setting", ""); } } @@ -1148,6 +1158,8 @@ private String validateConfigurationValue(final String name, String value, final return errMsg; } + isValueForUpdateCfgCmdDeltaOrAll(name, value); + if (value == null) { if (type.equals(Boolean.class)) { return "Please enter either 'true' or 'false'."; @@ -1254,6 +1266,15 @@ private String validateConfigurationValue(final String name, String value, final return String.format("Invalid value for configuration [%s].", name); } + private void isValueForUpdateCfgCmdDeltaOrAll(String name, String value) { + if (VirtualMachineManager.ResourceCountRoutersType.key().equalsIgnoreCase(name) + && (!resourceCountRoutersTypeValues.contains(value) || isBlank(value))) { + final String msg = "Possible values are: delta or all."; + s_logger.error(msg); + throw new InvalidParameterValueException(msg); + } + } + /** * A valid value should be an integer between min and max (the values from the range). */ @@ -4900,7 +4921,7 @@ public VlanVO doInTransaction(final TransactionStatus status) { ip.isSourceNat(), vlan.getVlanType().toString(), ip.getSystem(), usageHidden, ip.getClass().getName(), ip.getUuid()); } // increment resource count for dedicated public ip's - _resourceLimitMgr.incrementResourceCount(vlanOwner.getId(), ResourceType.public_ip, new Long(ips.size())); + _resourceLimitMgr.incrementResourceCount(vlanOwner.getId(), ResourceType.public_ip, Long.valueOf(ips.size())); } else if (domain != null && !forSystemVms) { // This VLAN is domain-wide, so create a DomainVlanMapVO entry final DomainVlanMapVO domainVlanMapVO = new DomainVlanMapVO(domain.getId(), vlan.getId()); @@ -5251,7 +5272,7 @@ public boolean deleteVlanAndPublicIpRange(final long userId, final long vlanDbId } finally { _vlanDao.releaseFromLockTable(vlanDbId); if (resourceCountToBeDecrement > 0) { //Making sure to decrement the count of only success operations above. For any reaason if disassociation fails then this number will vary from original range length. - _resourceLimitMgr.decrementResourceCount(acctVln.get(0).getAccountId(), ResourceType.public_ip, new Long(resourceCountToBeDecrement)); + _resourceLimitMgr.decrementResourceCount(acctVln.get(0).getAccountId(), ResourceType.public_ip, Long.valueOf(resourceCountToBeDecrement)); } } } else { // !isAccountSpecific @@ -5405,7 +5426,7 @@ public Vlan dedicatePublicIpRange(final DedicatePublicIpRangeCmd cmd) throws Res // increment resource count for dedicated public ip's if (vlanOwner != null) { - _resourceLimitMgr.incrementResourceCount(vlanOwner.getId(), ResourceType.public_ip, new Long(ips.size())); + _resourceLimitMgr.incrementResourceCount(vlanOwner.getId(), ResourceType.public_ip, Long.valueOf(ips.size())); } return vlan; @@ -5497,7 +5518,7 @@ public boolean releasePublicIpRange(final long vlanDbId, final long userId, fina } } // decrement resource count for dedicated public ip's - _resourceLimitMgr.decrementResourceCount(acctVln.get(0).getAccountId(), ResourceType.public_ip, new Long(ips.size())); + _resourceLimitMgr.decrementResourceCount(acctVln.get(0).getAccountId(), ResourceType.public_ip, Long.valueOf(ips.size())); success = true; } else if (isDomainSpecific && _domainVlanMapDao.remove(domainVlan.get(0).getId())) { s_logger.debug("Remove the vlan from domain_vlan_map successfully."); @@ -5666,7 +5687,7 @@ public void checkPodCidrSubnets(final long dcId, final Long podIdToBeSkipped, fi final List newCidrPair = new ArrayList(); newCidrPair.add(0, getCidrAddress(cidr)); newCidrPair.add(1, (long)getCidrSize(cidr)); - currentPodCidrSubnets.put(new Long(-1), newCidrPair); + currentPodCidrSubnets.put(Long.valueOf(-1), newCidrPair); final DataCenterVO dcVo = _zoneDao.findById(dcId); final String guestNetworkCidr = dcVo.getGuestNetworkCidr(); @@ -5766,7 +5787,7 @@ private boolean validPod(final String podName, final long zoneId) { } private String getPodName(final long podId) { - return _podDao.findById(new Long(podId)).getName(); + return _podDao.findById(Long.valueOf(podId)).getName(); } private boolean validZone(final String zoneName) { @@ -5778,7 +5799,7 @@ private boolean validZone(final long zoneId) { } private String getZoneName(final long zoneId) { - final DataCenterVO zone = _zoneDao.findById(new Long(zoneId)); + final DataCenterVO zone = _zoneDao.findById(Long.valueOf(zoneId)); if (zone != null) { return zone.getName(); } else { diff --git a/server/src/main/java/com/cloud/network/router/NetworkHelperImpl.java b/server/src/main/java/com/cloud/network/router/NetworkHelperImpl.java index 934336066ccf..90f9fda4bac9 100644 --- a/server/src/main/java/com/cloud/network/router/NetworkHelperImpl.java +++ b/server/src/main/java/com/cloud/network/router/NetworkHelperImpl.java @@ -264,6 +264,10 @@ public VirtualRouter destroyRouter(final long routerId, final Account caller, fi _accountMgr.checkAccess(caller, null, true, router); + final Account owner = _accountMgr.getAccount(router.getAccountId()); + ServiceOfferingVO routerOffering = _serviceOfferingDao.findById(router.getServiceOfferingId()); + decrementVrResourceCount(owner, routerOffering); + _itMgr.expunge(router.getUuid()); _routerHealthCheckResultDao.expungeHealthChecks(router.getId()); _routerDao.remove(router.getId()); @@ -492,7 +496,7 @@ protected String retrieveTemplateName(final HypervisorType hType, final long dat @Override public DomainRouterVO deployRouter(final RouterDeploymentDefinition routerDeploymentDefinition, final boolean startRouter) - throws InsufficientAddressCapacityException, InsufficientServerCapacityException, InsufficientCapacityException, StorageUnavailableException, ResourceUnavailableException { + throws InsufficientCapacityException, ResourceUnavailableException { final ServiceOfferingVO routerOffering = _serviceOfferingDao.findById(routerDeploymentDefinition.getServiceOfferingId()); final Account owner = routerDeploymentDefinition.getOwner(); @@ -502,6 +506,8 @@ public DomainRouterVO deployRouter(final RouterDeploymentDefinition routerDeploy // failed both times, throw the exception up final List hypervisors = getHypervisors(routerDeploymentDefinition); + incrementVrResourceCount(owner, routerOffering); + int allocateRetry = 0; int startRetry = 0; DomainRouterVO router = null; @@ -551,6 +557,10 @@ public DomainRouterVO deployRouter(final RouterDeploymentDefinition routerDeploy s_logger.debug("Failed to allocate the VR with hypervisor type " + hType + ", retrying one more time"); continue; } else { + // If VR can't be deployed then decrement the resource count + if (VirtualMachineManager.ResourceCountRouters.valueIn(owner.getDomainId())) { + _itMgr.decrementVrResourceCount(routerOffering, owner,true); + } throw ex; } } finally { @@ -566,8 +576,8 @@ public DomainRouterVO deployRouter(final RouterDeploymentDefinition routerDeploy s_logger.debug("Failed to start the VR " + router + " with hypervisor type " + hType + ", " + "destroying it and recreating one more time"); // destroy the router destroyRouter(router.getId(), _accountMgr.getAccount(Account.ACCOUNT_ID_SYSTEM), User.UID_SYSTEM); - continue; } else { + decrementVrResourceCount(owner, routerOffering); throw ex; } } finally { @@ -582,6 +592,32 @@ public DomainRouterVO deployRouter(final RouterDeploymentDefinition routerDeploy return router; } + /** + * If VR can't be deployed then decrement the resource count + * + * @param owner the owner of the VR + * @param routerOffering the service offering + */ + public void decrementVrResourceCount(Account owner, ServiceOfferingVO routerOffering) { + if (Boolean.TRUE.equals(VirtualMachineManager.ResourceCountRouters.valueIn(owner.getDomainId()))) { + _itMgr.decrementVrResourceCount(routerOffering, owner,true); + } + } + + /** + * Increment the resource count with router offering. + * If router can't be deployed or started, decrement the resources. + * If resource.count.running.vms is false, increment resource count. + * + * @param owner the owner of the VR + * @param routerOffering the service offering + */ + public void incrementVrResourceCount(Account owner, ServiceOfferingVO routerOffering) { + if (Boolean.TRUE.equals(VirtualMachineManager.ResourceCountRouters.valueIn(owner.getDomainId()))) { + _itMgr.incrementVrResourceCount(routerOffering, owner, true); + } + } + protected void filterSupportedHypervisors(final List hypervisors) { // For non vpc we keep them all assuming all types in the list are // supported diff --git a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java index 27c44f4df2f4..257161ec3a6c 100644 --- a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java +++ b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java @@ -29,6 +29,12 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; +import com.cloud.network.router.VirtualNetworkApplianceManager; +import com.cloud.offering.ServiceOffering; +import com.cloud.service.ServiceOfferingVO; +import com.cloud.service.dao.ServiceOfferingDao; +import com.cloud.vm.VMInstanceVO; +import com.cloud.vm.VirtualMachine; import org.apache.cloudstack.acl.SecurityChecker.AccessType; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine; @@ -163,6 +169,9 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim @Inject private VlanDao _vlanDao; + @Inject + private ServiceOfferingDao serviceOfferingDao; + protected GenericSearchBuilder templateSizeSearch; protected GenericSearchBuilder snapshotSizeSearch; @@ -680,7 +689,7 @@ public ResourceLimitVO updateResourceLimit(Long accountId, Long domainId, Intege Account caller = CallContext.current().getCallingAccount(); if (max == null) { - max = new Long(Resource.RESOURCE_UNLIMITED); + max = Long.valueOf(Resource.RESOURCE_UNLIMITED); } else if (max.longValue() < Resource.RESOURCE_UNLIMITED) { throw new InvalidParameterValueException("Please specify either '-1' for an infinite limit, or a limit that is at least '0'."); } @@ -967,7 +976,9 @@ public void doInTransactionWithoutResult(TransactionStatus status) { } public long countCpusForAccount(long accountId) { - long cputotal = 0; + long cputotal = 0L; + Account owner = _accountDao.findById(accountId); + DomainVO domain = _domainDao.findById(owner.getDomainId()); // user vms SearchBuilder userVmSearch = _userVmJoinDao.createSearchBuilder(); userVmSearch.and("accountId", userVmSearch.entity().getAccountId(), Op.EQ); @@ -979,19 +990,66 @@ public long countCpusForAccount(long accountId) { SearchCriteria sc1 = userVmSearch.create(); sc1.setParameters("accountId", accountId); if (VirtualMachineManager.ResourceCountRunningVMsonly.value()) - sc1.setParameters("state", new Object[] {State.Destroyed, State.Error, State.Expunging, State.Stopped}); + sc1.setParameters("state", State.Destroyed, State.Error, State.Expunging, State.Stopped); else - sc1.setParameters("state", new Object[] {State.Destroyed, State.Error, State.Expunging}); + sc1.setParameters("state", State.Destroyed, State.Error, State.Expunging); sc1.setParameters("displayVm", 1); List userVms = _userVmJoinDao.search(sc1,null); for (UserVmJoinVO vm : userVms) { - cputotal += Long.valueOf(vm.getCpu()); + cputotal += vm.getCpu(); + } + ServiceOffering defaultRouterOffering = null; + String globalRouterOffering = VirtualNetworkApplianceManager.VirtualRouterServiceOffering.value(); + if (globalRouterOffering != null) { + defaultRouterOffering = serviceOfferingDao.findByUuid(globalRouterOffering); + } + if (defaultRouterOffering == null) { + defaultRouterOffering = serviceOfferingDao.findByName(ServiceOffering.routerDefaultOffUniqueName); + } + + GenericSearchBuilder cpuSearch = serviceOfferingDao.createSearchBuilder(SumCount.class); + cpuSearch.select("sum", Func.SUM, cpuSearch.entity().getCpu()); + cpuSearch.select("count", Func.COUNT, (Object[])null); + SearchBuilder join1 = _vmDao.createSearchBuilder(); + join1.and("accountId", join1.entity().getAccountId(), Op.EQ); + join1.and("type", join1.entity().getType(), Op.IN); + join1.and("state", join1.entity().getState(), SearchCriteria.Op.NIN); + join1.and("displayVm", join1.entity().isDisplayVm(), Op.EQ); + cpuSearch.join("offerings", join1, cpuSearch.entity().getId(), join1.entity().getServiceOfferingId(), JoinBuilder.JoinType.INNER); + cpuSearch.done(); + + SearchCriteria sc = cpuSearch.create(); + sc.setJoinParameters("offerings", "accountId", accountId); + sc.setJoinParameters("offerings", "type", VirtualMachine.Type.DomainRouter); // domain routers + + if (Boolean.TRUE.equals(VirtualMachineManager.ResourceCountRunningVMsonly.value())) { + sc.setJoinParameters("offerings", "state", State.Destroyed, State.Error, State.Expunging, State.Stopped); + } else { + sc.setJoinParameters("offerings", "state", State.Destroyed, State.Error, State.Expunging); + } + sc.setJoinParameters("offerings", "displayVm", 1); + List cpus = serviceOfferingDao.customSearch(sc, null); + if (cpus != null) { + // Calculate the VR CPU resource count depending on the global setting + if (VirtualMachineManager.ResourceCountRouters.valueIn(domain.getId())) { + cputotal += cpus.get(0).sum; + + // Get the diff of current router offering with default offering + if (VirtualMachineManager.ResourceCountRoutersType.valueIn(domain.getId()) + .equalsIgnoreCase(VirtualMachineManager.COUNT_DELTA_VR_RESOURCES)) { + cputotal = cputotal - cpus.get(0).count * defaultRouterOffering.getCpu(); + } + } + return cputotal; + } else { + return cputotal; } - return cputotal; } public long calculateMemoryForAccount(long accountId) { - long ramtotal = 0; + long ramtotal = 0L; + Account owner = _accountDao.findById(accountId); + DomainVO domain = _domainDao.findById(owner.getDomainId()); // user vms SearchBuilder userVmSearch = _userVmJoinDao.createSearchBuilder(); userVmSearch.and("accountId", userVmSearch.entity().getAccountId(), Op.EQ); @@ -1003,15 +1061,58 @@ public long calculateMemoryForAccount(long accountId) { SearchCriteria sc1 = userVmSearch.create(); sc1.setParameters("accountId", accountId); if (VirtualMachineManager.ResourceCountRunningVMsonly.value()) - sc1.setParameters("state", new Object[] {State.Destroyed, State.Error, State.Expunging, State.Stopped}); + sc1.setParameters("state", State.Destroyed, State.Error, State.Expunging, State.Stopped); else - sc1.setParameters("state", new Object[] {State.Destroyed, State.Error, State.Expunging}); + sc1.setParameters("state", State.Destroyed, State.Error, State.Expunging); sc1.setParameters("displayVm", 1); List userVms = _userVmJoinDao.search(sc1,null); for (UserVmJoinVO vm : userVms) { - ramtotal += Long.valueOf(vm.getRamSize()); + ramtotal += vm.getRamSize(); + } + ServiceOffering defaultRouterOffering = null; + String globalRouterOffering = VirtualNetworkApplianceManager.VirtualRouterServiceOffering.value(); + if (globalRouterOffering != null) { + defaultRouterOffering = serviceOfferingDao.findByUuid(globalRouterOffering); + } + if (defaultRouterOffering == null) { + defaultRouterOffering = serviceOfferingDao.findByName(ServiceOffering.routerDefaultOffUniqueName); + } + GenericSearchBuilder memorySearch = serviceOfferingDao.createSearchBuilder(SumCount.class); + memorySearch.select("sum", Func.SUM, memorySearch.entity().getRamSize()); + memorySearch.select("count", Func.COUNT, (Object[])null); + SearchBuilder join1 = _vmDao.createSearchBuilder(); + join1.and("accountId", join1.entity().getAccountId(), Op.EQ); + join1.and("type", join1.entity().getType(), Op.IN); + join1.and("state", join1.entity().getState(), SearchCriteria.Op.NIN); + join1.and("displayVm", join1.entity().isDisplayVm(), Op.EQ); + memorySearch.join("offerings", join1, memorySearch.entity().getId(), join1.entity().getServiceOfferingId(), JoinBuilder.JoinType.INNER); + memorySearch.done(); + + SearchCriteria sc = memorySearch.create(); + sc.setJoinParameters("offerings", "accountId", accountId); + sc.setJoinParameters("offerings", "type", VirtualMachine.Type.DomainRouter); // domain routers + sc.setJoinParameters("offerings", "displayVm", 1); + + if (Boolean.TRUE.equals(VirtualMachineManager.ResourceCountRunningVMsonly.value())) { + sc.setJoinParameters("offerings", "state", State.Destroyed, State.Error, State.Expunging, State.Stopped); + } else { + sc.setJoinParameters("offerings", "state", State.Destroyed, State.Error, State.Expunging); + } + sc.setJoinParameters("offerings", "displayVm", 1); + List memory = serviceOfferingDao.customSearch(sc, null); + if (memory != null) { + // Calculate VR memory count depending on global setting + if (VirtualMachineManager.ResourceCountRouters.valueIn(domain.getId())) { + ramtotal += memory.get(0).sum; + if (VirtualMachineManager.ResourceCountRoutersType.valueIn(domain.getId()) + .equalsIgnoreCase(VirtualMachineManager.COUNT_DELTA_VR_RESOURCES)) { + ramtotal = ramtotal - memory.get(0).count * defaultRouterOffering.getRamSize(); + } + } + return ramtotal; + } else { + return ramtotal; } - return ramtotal; } public long calculateSecondaryStorageForAccount(long accountId) { @@ -1046,7 +1147,7 @@ private long calculatePublicIpForAccount(long accountId) { List dedicatedVlans = _vlanDao.listDedicatedVlans(accountId); for (VlanVO dedicatedVlan : dedicatedVlans) { List ips = _ipAddressDao.listByVlanId(dedicatedVlan.getId()); - dedicatedCount += new Long(ips.size()); + dedicatedCount += Long.valueOf(ips.size()); } allocatedCount = _ipAddressDao.countAllocatedIPsForAccount(accountId); if (dedicatedCount > allocatedCount) { diff --git a/test/integration/component/test_router_resources.py b/test/integration/component/test_router_resources.py new file mode 100644 index 000000000000..6d138649ef28 --- /dev/null +++ b/test/integration/component/test_router_resources.py @@ -0,0 +1,824 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +""" +Test case for router resources +""" + +# Import local modules + +from nose.plugins.attrib import attr +from marvin.cloudstackTestCase import cloudstackTestCase +from marvin.cloudstackAPI import (scaleSystemVm, + stopRouter, + startRouter, + restartNetwork, + updateConfiguration) +from marvin.lib.utils import (cleanup_resources) +from marvin.lib.base import (NetworkOffering, + ServiceOffering, + VirtualMachine, + Account, + Domain, + Network, + Router, + destroyRouter, + Zone, + updateResourceCount) +from marvin.lib.common import (get_zone, + get_template, + get_domain, + list_virtual_machines, + list_networks, + list_configurations, + list_routers, + list_service_offering) + +import logging + +class TestRouterResources(cloudstackTestCase): + + @classmethod + def setupClass(cls): + cls.testClient = super( + TestRouterResources, cls + ).getClsTestClient() + cls.apiclient = cls.testClient.getApiClient() + + cls.services = cls.testClient.getParsedTestDataConfig() + zone = get_zone(cls.apiclient, cls.testClient.getZoneForTests()) + cls.zone = Zone(zone.__dict__) + cls.template = get_template(cls.apiclient, cls.zone.id) + cls._cleanup = [] + + cls.logger = logging.getLogger("TestRouterResources") + cls.stream_handler = logging.StreamHandler() + cls.logger.setLevel(logging.DEBUG) + cls.logger.addHandler(cls.stream_handler) + + # Get Zone, Domain and templates + cls.domain = get_domain(cls.apiclient) + + cls.template = get_template( + cls.apiclient, + cls.zone.id, + cls.services["ostype"] + ) + + cls.service_offering = ServiceOffering.create( + cls.apiclient, + cls.services["service_offerings"]["big"] + ) + + # Create new domain1 + cls.domain1 = Domain.create( + cls.apiclient, + services=cls.services["acl"]["domain1"], + parentdomainid=cls.domain.id) + + # Create account1 + cls.account1 = Account.create( + cls.apiclient, + cls.services["acl"]["accountD1"], + domainid=cls.domain1.id + ) + + # Create Network Offering with all the services + cls.network_offering = NetworkOffering.create( + cls.apiclient, + cls.services["isolated_network_offering"] + ) + # Enable Network offering + cls.network_offering.update(cls.apiclient, state='Enabled') + + cls.network = Network.create( + cls.apiclient, + cls.services["isolated_network"], + accountid=cls.account1.name, + domainid=cls.account1.domainid, + networkofferingid=cls.network_offering.id, + zoneid=cls.zone.id + ) + + virtualmachine = VirtualMachine.create( + cls.apiclient, + services=cls.services["virtual_machine_userdata"], + accountid=cls.account1.name, + domainid=cls.account1.domainid, + serviceofferingid=cls.service_offering.id, + networkids=cls.network.id, + templateid=cls.template.id, + zoneid=cls.zone.id + ) + + vms = list_virtual_machines( + cls.apiclient, + account=cls.account1.name, + domainid=cls.account1.domainid, + id=virtualmachine.id + ) + vm = vms[0] + + # get vm cpu and memory values + cls.vm_cpu_count = vm.cpunumber + cls.vm_mem_count = vm.memory + + routers = list_routers( + cls.apiclient, + account=cls.account1.name, + domainid=cls.account1.domainid + ) + + router = routers[0] + + router_so_id = router.serviceofferingid + + list_service_response = list_service_offering( + cls.apiclient, + id=router_so_id, + issystem="true", + systemvmtype="domainrouter", + listall="true" + ) + + # Get default router service offering cpu and memory values + cls.default_vr_cpu = list_service_response[0].cpunumber + cls.default_vr_ram = list_service_response[0].memory + + cls._cleanup.append(virtualmachine) + cls._cleanup.append(cls.network) + + # Disable Network offering + cls.network_offering.update(cls.apiclient, state='Disabled') + cls._cleanup.append(cls.network_offering) + cls._cleanup.append(cls.account1) + cls._cleanup.append(cls.domain1) + + @classmethod + def tearDownClass(cls): + try: + cleanup_resources(cls.apiclient, cls._cleanup) + except Exception as e: + raise Exception("Warning: Exception during cleanup : %s" % e) + return + + def setUp(self): + self.apiclient = self.testClient.getApiClient() + self.cleanup = [] + return + + def tearDown(self): + try: + cleanup_resources(self.apiclient, self.cleanup) + except Exception as e: + raise Exception("Warning: Exception during cleanup : %s" % e) + return + + def get_resource_amount(self, resource_type): + """ + Function to update resource count of a domain + for the corresponding resource_type passed as parameter + :param resource_type: + :return: resource count + """ + cmd = updateResourceCount.updateResourceCountCmd() + cmd.account = self.account1.name + cmd.domainid = self.domain1.id + cmd.resourcetype = resource_type + response = self.apiclient.updateResourceCount(cmd) + amount = response[0].resourcecount + return amount + + def update_configuration(self, name, value, domainid): + """ + Function to update the global setting value for the domain + :param name: + :param value: + :param domainid: + :return: + """ + updateConfigurationCmd = updateConfiguration.updateConfigurationCmd() + updateConfigurationCmd.name = name + updateConfigurationCmd.value = value + updateConfigurationCmd.domainid = domainid + return self.apiclient.updateConfiguration(updateConfigurationCmd) + + def get_vr_service_offering(self): + """ + Function to get Virtual Router service offering + :return: + """ + routers = list_routers( + self.apiclient, + account=self.account1.name, + domainid=self.account1.domainid + ) + + router = routers[0] + + router_so_id = router.serviceofferingid + + list_service_response = list_service_offering( + self.apiclient, + id=router_so_id, + issystem="true", + listall="true" + ) + + return list_service_response + + def stop_router(self, routerid): + cmd = stopRouter.stopRouterCmd() + cmd.id = routerid + cmd.forced = "true" + self.apiclient.stopRouter(cmd) + + def destroy_router(self, routerid): + self.stop_router(routerid) + cmd = destroyRouter.destroyRouterCmd() + cmd.id = routerid + self.apiclient.destroyRouter(cmd) + + def restart_network(self): + cmd = restartNetwork.restartNetworkCmd() + cmd.id = self.network.id + cmd.cleanup = True + self.apiclient.restartNetwork(cmd) + + @attr(tags=["advanced", "basic", "sg"], required_hardware="false") + def test_01_count_vm_resources(self): + """ + Test case to just count running vm resources with global setting set to false + + # Steps + 1. Vm is already created + 2. Get the resource count of the running vm + 3. Update the resource count for the domain/account + 4. Make sure that these two values matches + """ + CPU_RESOURCE_ID = 8 + RAM_RESOURCE_ID = 9 + + cores = int(self.get_resource_amount(CPU_RESOURCE_ID)) + ram = int(self.get_resource_amount(RAM_RESOURCE_ID)) + + self.info("Initial resource count of domain/account are") + self.info("cpu is %s and ram is %s" % (cores, ram)) + + self.assertEqual( + self.vm_cpu_count, + cores, + "VM CPU count doesnt not match" + ) + + self.assertEqual( + self.vm_mem_count, + ram, + "VM memory count does not match" + ) + + @attr(tags=["advanced", "basic", "sg"], required_hardware="false") + def test_02_count_vm_resources_with_all_vr_resource(self): + """ + Test vm resource count along with vr resource count when global + setting value is set to "all" + + # Steps + 1. Get the vm resource count from test case 1 + 2. Get the default service offering of VR + 3. Extract cpu and ram size from it + 4. Set the global setting "resource.count.routers" to true + 5. Set the value of "resource.count.routers.type" to "all" + 6. Update the resource count of domain/account + 7. Make sure that the cpu and ram count is equal to (vm + vr) + """ + + CPU_RESOURCE_ID = 8 + RAM_RESOURCE_ID = 9 + + # Step 2 + list_service_offering_response = self.get_vr_service_offering() + + # Step 3 + vr_cpu = list_service_offering_response[0].cpunumber + vr_ram = list_service_offering_response[0].memory + + # Step 4 + self.update_configuration("resource.count.routers", + "true", self.domain1.id) + + # Step 5 + self.update_configuration("resource.count.routers.type", + "all", self.domain1.id) + + # Step 6 + cores = int(self.get_resource_amount(CPU_RESOURCE_ID)) + ram = int(self.get_resource_amount(RAM_RESOURCE_ID)) + self.info("New resource count of domain/account are") + self.info("cpu is %s and ram is %s" % (cores, ram)) + + # Step 7 + self.assertEqual( + cores, + self.vm_cpu_count + vr_cpu, + "Total resource count for cpu does not match VM + VR cpu count" + ) + + self.assertEqual( + ram, + self.vm_mem_count + vr_ram, + "Total resource count for memory does not match VM + VR memory" + ) + + @attr(tags=["advanced", "basic", "sg"], required_hardware="false") + def test_03_count_vm_resources_with_delta_vr_resource(self): + """ + Test vm resource count along with vr resource count when global setting + value is set to "delta" + + # Steps + 1. Get the current service offering of VR + 2. Extract cpu and ram size from it + 3. Set the global setting "resource.count.routers" to true + 4. Set the value of "resource.count.routers.type" to "delta" + 5. Update the resource count of domain/account + 6. Make sure that the cpu and ram count is equal to + VM + (current router offering - default router offering) + """ + + CPU_RESOURCE_ID = 8 + RAM_RESOURCE_ID = 9 + + # Step 1 and 2 + list_service_offering_response = self.get_vr_service_offering() + new_vr_cpu = list_service_offering_response[0].cpunumber + new_vr_ram = list_service_offering_response[0].memory + + # Step 3 + self.update_configuration("resource.count.routers", + "true", self.domain1.id) + + # Step 4 + self.update_configuration("resource.count.routers.type", + "delta", self.domain1.id) + + # Step 5 + cores = int(self.get_resource_amount(CPU_RESOURCE_ID)) + ram = int(self.get_resource_amount(RAM_RESOURCE_ID)) + self.info("New resource count for domain/account are") + self.info("cpu is %s and ram is %s" % (cores, ram)) + + # Step 6 + self.assertEqual( + cores, + self.vm_cpu_count + (new_vr_cpu - self.default_vr_cpu), + "Total resource count of cpu does not match delta for vr cpu" + ) + + self.assertEqual( + ram, + self.vm_mem_count + (new_vr_ram - self.default_vr_ram), + "Total resource count of memory does not match delta for vr memory" + ) + + def test_04_count_vm_resource_with_new_vr_offering_all(self): + """ + Test to count vm resources along with new vr service offering with + global setting set to "all" + + Steps + 1. Create a new router service offering with 2 cores and 2Gb Ram + 2. Stop the router + 3. Update the service offering of the router with new offering + 4. Start the router + 5. Set the global setting value to "all" + 6. Update the resource count of domain/account + 7. Get the new cpu/ram count of VR + 8. Make sure that the resource count is equal to VM + new VR service offering + """ + CPU_RESOURCE_ID = 8 + RAM_RESOURCE_ID = 9 + + # Step 1 + offering_data = { + 'displaytext': 'TestOffering', + 'cpuspeed': 1000, + 'cpunumber': 2, + 'name': 'TestOffering', + 'memory': 2048, + 'issystem': 'true', + 'systemvmtype': 'domainrouter' + } + self.new_network_offering = ServiceOffering.create( + self.apiclient, + offering_data, + domainid=self.domain1.id + ) + + routers = list_routers( + self.apiclient, + account=self.account1.name, + domainid=self.account1.domainid + ) + + router = routers[0] + + # Step 2 + # Stop the router + self.stop_router(router.id) + + # Step 3 + scale_systemvm_cmd = scaleSystemVm.scaleSystemVmCmd() + scale_systemvm_cmd.id=router.id + scale_systemvm_cmd.serviceofferingid = self.new_network_offering.id + self.apiclient.scaleSystemVm(scale_systemvm_cmd) + + # Step 4 + cmd = startRouter.startRouterCmd() + cmd.id = router.id + self.apiclient.startRouter(cmd) + + # Step 5 + self.update_configuration("resource.count.routers.type", + "all", self.domain1.id) + + # Step 6 + cores = int(self.get_resource_amount(CPU_RESOURCE_ID)) + ram = int(self.get_resource_amount(RAM_RESOURCE_ID)) + self.info("New resource count for domain/account are") + self.info("cpu is %s and ram is %s" % (cores,ram)) + + # Step 7 + list_service_offering_response = self.get_vr_service_offering() + updated_vr_cpu = list_service_offering_response[0].cpunumber + updated_vr_ram = list_service_offering_response[0].memory + + # Step 8 + self.assertEqual( + cores, + self.vm_cpu_count + updated_vr_cpu, + "Total resource count of cpu does not match delta for vr cpu" + ) + + self.assertEqual( + ram, + self.vm_mem_count + updated_vr_ram, + "Total resource count of memory does not match delta for vr memory" + ) + + @attr(tags=["advanced", "basic", "sg"], required_hardware="false") + def test_05_count_vm_resource_with_delta_vr_count(self): + """ + Test to count vm resources along with new vr service offering with + global setting set to "delta" + + Steps + 1. Set the global setting "resource.count.routers.type" value to "delta" + 2. Update the resource count of domain/account + 3. Get the new cpu/ram count of VR + 4. Make sure that the resource count is equal to + VM + (new VR service offering - default router offering) + """ + CPU_RESOURCE_ID = 8 + RAM_RESOURCE_ID = 9 + + # Step 1 + self.update_configuration("resource.count.routers.type", + "delta", self.domain1.id) + + # Step 2 + cores = int(self.get_resource_amount(CPU_RESOURCE_ID)) + ram = int(self.get_resource_amount(RAM_RESOURCE_ID)) + self.info("New resource count for domain/account are") + self.info("cpu is %s and ram is %s" % (cores, ram)) + + # Step 3 + list_service_offering_response = self.get_vr_service_offering() + updated_vr_cpu = list_service_offering_response[0].cpunumber + updated_vr_ram = list_service_offering_response[0].memory + + # Step 4 + self.assertEqual( + cores, + self.vm_cpu_count + (updated_vr_cpu - self.default_vr_cpu), + "Total resource count of cpu does not match delta for vr cpu" + ) + + self.assertEqual( + ram, + self.vm_mem_count + (updated_vr_ram - self.default_vr_ram), + "Total resource count of memory does not match delta for vr memory" + ) + + @attr(tags=["advanced", "basic", "sg"], required_hardware="false") + def test_06_count_just_vm_resource(self): + """ + Test to count vm resources when global setting "resource.count.routers" + is set to false + + Steps + 1. Set the global setting "resource.count.routers" value to "false" + 2. Update the resource count of domain/account + 3. Make sure that the resource count is equal to VM resource count + """ + CPU_RESOURCE_ID = 8 + RAM_RESOURCE_ID = 9 + + # Step 1 + # set resource.count.routers to false + self.update_configuration("resource.count.routers", + "false", self.domain1.id) + + # Step 2 + cores = int(self.get_resource_amount(CPU_RESOURCE_ID)) + ram = int(self.get_resource_amount(RAM_RESOURCE_ID)) + self.info("New resource count for domain/account are") + self.info("cpu is %s and ram is %s" % (cores,ram)) + + # Step 3 + self.assertEqual( + cores, + self.vm_cpu_count, + "Total resource count of cpu does not match delta for vr cpu" + ) + + self.assertEqual( + ram, + self.vm_mem_count, + "Total resource count of memory does not match delta for vr memory" + ) + + @attr(tags=["advanced", "basic", "sg"], required_hardware="false") + def test_07_count_vm_resources_with_stopped_router(self): + """ + Test to count vm resources when global setting "resource.count.routers" + is set to true and VR is stopped + + Steps + 1. Set the global setting "resource.count.routers" value to "true" + 2. Stop the VR + 3. Update the resource count of domain/account + 4. Get the value of global setting "resource.count.running.vms.only + 5. If the above value is true then resource count should be equal to + resource count of VM else its resource count of VM + VR + """ + CPU_RESOURCE_ID = 8 + RAM_RESOURCE_ID = 9 + + # Step 1 + # set resource.count.routers to true + self.update_configuration("resource.count.routers", + "true", self.domain1.id) + self.update_configuration("resource.count.routers.type", + "all", self.domain1.id) + + # Step 2 + routers = list_routers( + self.apiclient, + account=self.account1.name, + domainid=self.account1.domainid + ) + + router = routers[0] + + cmd = stopRouter.stopRouterCmd() + cmd.id = router.id + cmd.forced = True + self.apiclient.stopRouter(cmd) + + # Step 3 + cores = int(self.get_resource_amount(CPU_RESOURCE_ID)) + ram = int(self.get_resource_amount(RAM_RESOURCE_ID)) + self.info("New resource count for domain/account are") + self.info("cpu is %s and ram is %s" % (cores,ram)) + + # Step 4 + resource_count_running_vms = list_configurations( + self.apiclient, + name='resource.count.running.vms.only' + ) + + running_vms_only = resource_count_running_vms[0].value + + new_cpu_count = 0 + new_ram_size = 0 + + if running_vms_only == 'true': + new_cpu_count = self.vm_cpu_count + new_ram_size = self.vm_mem_count + else: + list_service_offering_response = self.get_vr_service_offering() + updated_vr_cpu = list_service_offering_response[0].cpunumber + updated_vr_ram = list_service_offering_response[0].memory + new_cpu_count = self.vm_cpu_count + updated_vr_cpu + new_ram_size = self.vm_mem_count + updated_vr_ram + + # Step 5 + self.assertEqual( + cores, + new_cpu_count, + "Total resource count of cpu does not match delta for vr cpu" + ) + + self.assertEqual( + ram, + new_ram_size, + "Total resource count of memory does not match delta for vr memory" + ) + + @attr(tags=["advanced", "basic", "sg"], required_hardware="false") + def test_08_count_resources_restarting_network(self): + """ + Test to count vm resources when global setting "resource.count.routers" + is set to true and network is restarted with cleanup + + Steps + 1. Restart the network with cleanup option so that VR uses default offering + 2. Update the resource count of domain/account + 3. Make sure that the resource count is equal to VM + default VR service offering + 4. Create a new router service offering with 2 cores and 2Gb Ram + 5. Stop the router + 6. Update the service offering of the router with new offering + 7. Start the router + 8. Update the resource count of domain/account + 9. Make sure that the resource count is equal to VM + new VR service offering + 10. Set the global setting "router.service.offering" with new VR service offering + 11. Restart network with cleanup option + 12. Resource count should be equal to VM + (new VR offering) + """ + CPU_RESOURCE_ID = 8 + RAM_RESOURCE_ID = 9 + + # Step 1 + self.restart_network() + + # Step 2 + cores = int(self.get_resource_amount(CPU_RESOURCE_ID)) + ram = int(self.get_resource_amount(RAM_RESOURCE_ID)) + + list_service_offering_response = self.get_vr_service_offering() + default_vr_cpu = list_service_offering_response[0].cpunumber + default_vr_ram = list_service_offering_response[0].memory + + # Step 3 + self.assertEqual( + cores, + self.vm_cpu_count + default_vr_cpu, + "Total resource count of cpu does not match vm + default vr cpu" + ) + + self.assertEqual( + ram, + self.vm_mem_count + default_vr_ram, + "Total resource count of memory does not match vm + default vr ram" + ) + + # Step 4 + offering_data = { + 'displaytext': 'TestOffering2', + 'cpuspeed': 1000, + 'cpunumber': 2, + 'name': 'TestOffering2', + 'memory': 2048, + 'issystem': 'true', + 'systemvmtype': 'domainrouter' + } + + network_offering = ServiceOffering.create( + self.apiclient, + offering_data, + domainid=self.domain1.id + ) + + # Step 5 + routers = list_routers( + self.apiclient, + account=self.account1.name, + domainid=self.account1.domainid + ) + + router = routers[0] + + self.stop_router(router.id) + + # Step 6 + scale_systemvm_cmd = scaleSystemVm.scaleSystemVmCmd() + scale_systemvm_cmd.id=router.id + scale_systemvm_cmd.serviceofferingid = network_offering.id + self.apiclient.scaleSystemVm(scale_systemvm_cmd) + + # Step 7 + cmd = startRouter.startRouterCmd() + cmd.id = router.id + self.apiclient.startRouter(cmd) + + # Step 8 + cores = int(self.get_resource_amount(CPU_RESOURCE_ID)) + ram = int(self.get_resource_amount(RAM_RESOURCE_ID)) + + list_service_offering_response = self.get_vr_service_offering() + updated_vr_cpu = list_service_offering_response[0].cpunumber + updated_vr_ram = list_service_offering_response[0].memory + + # Step 9 + self.assertEqual( + cores, + self.vm_cpu_count + updated_vr_cpu, + "Total resource count of cpu does not match vm + new vr cpu" + ) + + self.assertEqual( + ram, + self.vm_mem_count + updated_vr_ram, + "Total resource count of memory does not match vm + new vr ram" + ) + + # Step 10 + self.update_configuration("router.service.offering", list_service_offering_response[0].id, None) + + # Step 11 + self.restart_network() + + # Step 12 + cores = int(self.get_resource_amount(CPU_RESOURCE_ID)) + ram = int(self.get_resource_amount(RAM_RESOURCE_ID)) + + self.assertEqual( + cores, + self.vm_cpu_count + updated_vr_cpu, + "Total resource count of cpu does not match vm + new vr cpu" + ) + + self.assertEqual( + ram, + self.vm_mem_count + updated_vr_ram, + "Total resource count of memory does not match vm + new vr ram" + ) + + self.update_configuration("router.service.offering", "", None) + + @attr(tags=["advanced", "basic", "sg"], required_hardware="false") + def test_09_count_vm_resources_with_destroyed_router(self): + """ + Test to count vm resources when global setting "resource.count.routers" + is set to true and VR is destroyed + + Steps + 1. Set the global setting "resource.count.routers" value to "true" + 2. Destroy the VR + 3. Update the resource count of domain/account + 4.The new resource count should be equal to VM resource count + """ + CPU_RESOURCE_ID = 8 + RAM_RESOURCE_ID = 9 + + # Step 1 + # set resource.count.routers to true + self.update_configuration("resource.count.routers", + "true", self.domain1.id) + self.update_configuration("resource.count.routers.type", + "all", self.domain1.id) + + # Step 2 + routers = list_routers( + self.apiclient, + account=self.account1.name, + domainid=self.account1.domainid + ) + + router = routers[0] + + self.destroy_router(router.id) + + # Step 3 + cores = int(self.get_resource_amount(CPU_RESOURCE_ID)) + ram = int(self.get_resource_amount(RAM_RESOURCE_ID)) + self.info("New resource count for domain/account are") + self.info("cpu is %s and ram is %s" % (cores,ram)) + + # Step 4 + self.assertEqual( + cores, + self.vm_cpu_count, + "Total resource count of cpu does not match delta for vr cpu" + ) + + self.assertEqual( + ram, + self.vm_mem_count, + "Total resource count of memory does not match delta for vr memory" + ) diff --git a/utils/src/main/java/com/cloud/utils/Pair.java b/utils/src/main/java/com/cloud/utils/Pair.java index 73d35625fa7c..93dcc75a9dbf 100644 --- a/utils/src/main/java/com/cloud/utils/Pair.java +++ b/utils/src/main/java/com/cloud/utils/Pair.java @@ -35,6 +35,10 @@ public Pair(T t, U u) { this.u = u; } + public static Pair of(T t, U u) { + return new Pair<>(t, u); + } + public T first() { return t; } From c94445827c64ee452220bcaf875995d626f5c26a Mon Sep 17 00:00:00 2001 From: Charles Queiroz Date: Wed, 22 Feb 2023 11:35:37 +0000 Subject: [PATCH 2/9] Update test/integration/component/test_router_resources.py Co-authored-by: dahn --- test/integration/component/test_router_resources.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/test/integration/component/test_router_resources.py b/test/integration/component/test_router_resources.py index 6d138649ef28..96b53da167e6 100644 --- a/test/integration/component/test_router_resources.py +++ b/test/integration/component/test_router_resources.py @@ -182,11 +182,7 @@ def setUp(self): return def tearDown(self): - try: - cleanup_resources(self.apiclient, self.cleanup) - except Exception as e: - raise Exception("Warning: Exception during cleanup : %s" % e) - return + super(TestRouterResources, self).tearDown() def get_resource_amount(self, resource_type): """ From 816d477ff782d6a3acb55678f41c85bd35c88178 Mon Sep 17 00:00:00 2001 From: Charles Queiroz Date: Wed, 22 Feb 2023 11:35:51 +0000 Subject: [PATCH 3/9] Update test/integration/component/test_router_resources.py Co-authored-by: dahn --- test/integration/component/test_router_resources.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/test/integration/component/test_router_resources.py b/test/integration/component/test_router_resources.py index 96b53da167e6..c60fcbbbd34e 100644 --- a/test/integration/component/test_router_resources.py +++ b/test/integration/component/test_router_resources.py @@ -170,11 +170,7 @@ def setupClass(cls): @classmethod def tearDownClass(cls): - try: - cleanup_resources(cls.apiclient, cls._cleanup) - except Exception as e: - raise Exception("Warning: Exception during cleanup : %s" % e) - return + super(TestRouterResources, cls).tearDownClass() def setUp(self): self.apiclient = self.testClient.getApiClient() From c231cca1085eaba5389cba9000f1d58ad3e28731 Mon Sep 17 00:00:00 2001 From: Chales Queiroz Date: Wed, 22 Feb 2023 12:52:56 +0000 Subject: [PATCH 4/9] Fixing the cleanup strategy --- .../component/test_router_resources.py | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/test/integration/component/test_router_resources.py b/test/integration/component/test_router_resources.py index c60fcbbbd34e..f962aa9432b7 100644 --- a/test/integration/component/test_router_resources.py +++ b/test/integration/component/test_router_resources.py @@ -84,11 +84,16 @@ def setupClass(cls): cls.services["service_offerings"]["big"] ) + cls._cleanup.append(cls.service_offering) + # Create new domain1 cls.domain1 = Domain.create( cls.apiclient, services=cls.services["acl"]["domain1"], - parentdomainid=cls.domain.id) + parentdomainid=cls.domain.id + ) + + cls._cleanup.append(cls.domain1) # Create account1 cls.account1 = Account.create( @@ -97,11 +102,16 @@ def setupClass(cls): domainid=cls.domain1.id ) + cls._cleanup.append(cls.account1) + # Create Network Offering with all the services cls.network_offering = NetworkOffering.create( cls.apiclient, cls.services["isolated_network_offering"] ) + + cls._cleanup.append(cls.network_offering) + # Enable Network offering cls.network_offering.update(cls.apiclient, state='Enabled') @@ -114,6 +124,8 @@ def setupClass(cls): zoneid=cls.zone.id ) + cls._cleanup.append(cls.network) + virtualmachine = VirtualMachine.create( cls.apiclient, services=cls.services["virtual_machine_userdata"], @@ -125,6 +137,8 @@ def setupClass(cls): zoneid=cls.zone.id ) + cls._cleanup.append(virtualmachine) + vms = list_virtual_machines( cls.apiclient, account=cls.account1.name, @@ -159,14 +173,8 @@ def setupClass(cls): cls.default_vr_cpu = list_service_response[0].cpunumber cls.default_vr_ram = list_service_response[0].memory - cls._cleanup.append(virtualmachine) - cls._cleanup.append(cls.network) - # Disable Network offering cls.network_offering.update(cls.apiclient, state='Disabled') - cls._cleanup.append(cls.network_offering) - cls._cleanup.append(cls.account1) - cls._cleanup.append(cls.domain1) @classmethod def tearDownClass(cls): From 454f0bbb2e6b104eb3b122cd87bc7db816ac5988 Mon Sep 17 00:00:00 2001 From: Charles Queiroz Date: Fri, 3 Mar 2023 10:26:31 +0000 Subject: [PATCH 5/9] Update test/integration/component/test_router_resources.py Co-authored-by: dahn --- test/integration/component/test_router_resources.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/integration/component/test_router_resources.py b/test/integration/component/test_router_resources.py index f962aa9432b7..a291152cd16c 100644 --- a/test/integration/component/test_router_resources.py +++ b/test/integration/component/test_router_resources.py @@ -423,11 +423,12 @@ def test_04_count_vm_resource_with_new_vr_offering_all(self): 'issystem': 'true', 'systemvmtype': 'domainrouter' } - self.new_network_offering = ServiceOffering.create( + network_offering = self.new_network_offering = ServiceOffering.create( self.apiclient, offering_data, domainid=self.domain1.id ) + self.cleanup.append(network_offering) routers = list_routers( self.apiclient, From f962e15904ef2c22b4e4c4602b6d7083bc3c1648 Mon Sep 17 00:00:00 2001 From: Charles Queiroz Date: Fri, 3 Mar 2023 10:26:43 +0000 Subject: [PATCH 6/9] Update test/integration/component/test_router_resources.py Co-authored-by: dahn --- test/integration/component/test_router_resources.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/integration/component/test_router_resources.py b/test/integration/component/test_router_resources.py index a291152cd16c..35d906767fd8 100644 --- a/test/integration/component/test_router_resources.py +++ b/test/integration/component/test_router_resources.py @@ -704,6 +704,7 @@ def test_08_count_resources_restarting_network(self): offering_data, domainid=self.domain1.id ) + self.cleanup.append(network_offering) # Step 5 routers = list_routers( From 5ef2b711723953d5894b0aba4434647d5d59337a Mon Sep 17 00:00:00 2001 From: Chales Queiroz Date: Thu, 13 Apr 2023 14:50:13 +0100 Subject: [PATCH 7/9] Adding tests and refactoring --- .github/workflows/ci.yml | 3 +- .../cloud/vm/VirtualMachineManagerImpl.java | 26 +- .../vm/VirtualMachineManagerImplTest.java | 276 +++++++++++------- .../network/router/NetworkHelperImpl.java | 2 +- .../ResourceLimitManagerImpl.java | 153 ++++++---- .../network/router/NetworkHelperImplTest.java | 68 ++++- utils/src/main/java/com/cloud/utils/Pair.java | 3 +- .../test/java/com/cloud/utils/PairTest.java | 87 ++++++ 8 files changed, 423 insertions(+), 195 deletions(-) create mode 100644 utils/src/test/java/com/cloud/utils/PairTest.java diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fadf33f2043a..b7ded050f376 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -186,7 +186,8 @@ jobs: component/test_vpc_offerings component/test_vpc_routers component/test_vpn_users", - "component/test_vpc_network_lbrules" ] + "component/test_vpc_network_lbrules", + "component/test_router_resources"] steps: - uses: actions/checkout@v3 diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index af69f3f8f3ab..df73cc48161a 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -27,7 +27,6 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; -import java.util.Comparator; import java.util.Date; import java.util.HashMap; import java.util.HashSet; @@ -1471,7 +1470,7 @@ public ServiceOffering getServiceOfferingByConfig() { * @param owner account * @return a Pair of CPU and RAM */ - private Pair resolveCpuAndMemoryCount(ServiceOffering offering, ServiceOffering defaultRouterOffering, Account owner) { + public Pair resolveCpuAndMemoryCount(ServiceOffering offering, ServiceOffering defaultRouterOffering, Account owner) { Integer cpuCount = 0; Integer memoryCount = 0; if (COUNT_ALL_VR_RESOURCES.equalsIgnoreCase(ResourceCountRoutersType.valueIn(owner.getDomainId()))) { @@ -1501,7 +1500,7 @@ private void validateResourceCount(Pair cpuMemoryCount, Account owne _resourceLimitMgr.checkResourceLimit(owner, ResourceType.memory, memoryCount); } } catch (ResourceAllocationException ex) { - throw new CloudRuntimeException(String.format("Unable to deploy/start routers due to {}.", ex.getMessage())); + throw new CloudRuntimeException(String.format("Unable to deploy/start routers due to %s.", ex.getMessage())); } } @@ -1511,7 +1510,7 @@ private void validateResourceCount(Pair cpuMemoryCount, Account owne * @param cpuMemoryCount a Pair of cpu and ram * @param owner the account */ - private void calculateResourceCount(Pair cpuMemoryCount, Account owner, boolean isIncrement) { + public void calculateResourceCount(Pair cpuMemoryCount, Account owner, boolean isIncrement) { validateResourceCount(cpuMemoryCount, owner); final Long cpuCount = cpuMemoryCount.first(); final Long memoryCount = cpuMemoryCount.second(); @@ -1555,7 +1554,7 @@ private void calculateResourceCount(Pair cpuMemoryCount, Account own public void incrementVrResourceCount(ServiceOffering offering, Account owner, boolean isDeployOrDestroy) { if (isDeployOrDestroy == Boolean.TRUE.equals(ResourceCountRunningVMsonly.value())) { return; - } + } // final ServiceOffering defaultRouterOffering = getServiceOfferingByConfig(); final Pair cpuMemoryCount = resolveCpuAndMemoryCount(offering, defaultRouterOffering, owner); @@ -1565,9 +1564,9 @@ public void incrementVrResourceCount(ServiceOffering offering, Account owner, bo /** * Decrements the VR resource count. * - * @param offering VR service offering - * @param owner account - * @param isDeployOrDestroy true if router is being deployed/destroyed + * @param offering the service offering of the VR + * @param owner the account of the VR + * @param isDeployOrDestroy true if the VR is being deployed or destroyed, false if the VR is being started or stopped */ @Override public void decrementVrResourceCount(ServiceOffering offering, Account owner, boolean isDeployOrDestroy) { @@ -3671,13 +3670,10 @@ protected VirtualMachineTO getVmTO(Long vmId) { final VMInstanceVO vm = _vmDao.findById(vmId); final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm); final List nics = _nicsDao.listByVmId(profile.getId()); - Collections.sort(nics, new Comparator() { - @Override - public int compare(NicVO nic1, NicVO nic2) { - Long nicId1 = Long.valueOf(nic1.getDeviceId()); - Long nicId2 = Long.valueOf(nic2.getDeviceId()); - return nicId1.compareTo(nicId2); - } + nics.sort((nic1, nic2) -> { + Long nicDevId1 = (long) nic1.getDeviceId(); + Long nicDevId2 = (long) nic2.getDeviceId(); + return nicDevId1.compareTo(nicDevId2); }); for (final NicVO nic : nics) { diff --git a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java index dd6f51d7577f..ffc6410d2a79 100644 --- a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java +++ b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java @@ -17,14 +17,12 @@ package com.cloud.vm; +import static com.cloud.vm.VirtualMachineManager.ResourceCountRunningVMsonly; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.anyLong; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.when; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; import java.lang.reflect.Field; import java.util.ArrayList; @@ -33,6 +31,9 @@ import java.util.List; import java.util.Map; +import com.cloud.user.Account; +import com.cloud.utils.Pair; +import com.cloud.vm.dao.VMInstanceDao; import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; @@ -46,7 +47,7 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.Spy; -import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.junit.MockitoJUnitRunner; import com.cloud.agent.AgentManager; import com.cloud.agent.api.Command; @@ -79,7 +80,6 @@ import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.VirtualMachine.State; import com.cloud.vm.dao.UserVmDao; -import com.cloud.vm.dao.VMInstanceDao; @RunWith(MockitoJUnitRunner.class) public class VirtualMachineManagerImplTest { @@ -90,8 +90,6 @@ public class VirtualMachineManagerImplTest { @Mock private AgentManager agentManagerMock; @Mock - private VMInstanceDao vmInstanceDaoMock; - @Mock private ServiceOfferingDao serviceOfferingDaoMock; @Mock private VolumeDao volumeDaoMock; @@ -99,7 +97,7 @@ public class VirtualMachineManagerImplTest { private PrimaryDataStoreDao storagePoolDaoMock; @Mock private VMInstanceVO vmInstanceMock; - private long vmInstanceVoMockId = 1L; + private final long vmInstanceVoMockId = 1L; @Mock private ServiceOfferingVO serviceOfferingMock; @@ -107,9 +105,9 @@ public class VirtualMachineManagerImplTest { @Mock private DiskOfferingVO diskOfferingMock; - private long hostMockId = 1L; - private long clusterMockId = 2L; - private long zoneMockId = 3L; + private final long hostMockId = 1L; + private final long clusterMockId = 2L; + private final long zoneMockId = 3L; @Mock private HostVO hostMock; @Mock @@ -119,12 +117,11 @@ public class VirtualMachineManagerImplTest { private VirtualMachineProfile virtualMachineProfileMock; @Mock private StoragePoolVO storagePoolVoMock; - private long storagePoolVoMockId = 11L; - private long storagePoolVoMockClusterId = 234L; + private final long storagePoolVoMockId = 11L; @Mock private VolumeVO volumeVoMock; - private long volumeMockId = 1111L; + private final long volumeMockId = 1111L; @Mock private StoragePoolHostDao storagePoolHostDaoMock; @@ -144,21 +141,25 @@ public class VirtualMachineManagerImplTest { @Mock private UserVmVO userVmMock; + @Mock + private VMInstanceDao vmDaoMock; + @Before public void setup() { virtualMachineManagerImpl.setHostAllocators(new ArrayList<>()); - when(vmInstanceMock.getId()).thenReturn(vmInstanceVoMockId); - when(vmInstanceMock.getServiceOfferingId()).thenReturn(2L); - when(hostMock.getId()).thenReturn(hostMockId); - when(dataCenterDeploymentMock.getHostId()).thenReturn(hostMockId); - when(dataCenterDeploymentMock.getClusterId()).thenReturn(clusterMockId); + Mockito.when(vmInstanceMock.getId()).thenReturn(vmInstanceVoMockId); + Mockito.when(vmInstanceMock.getServiceOfferingId()).thenReturn(2L); + Mockito.when(hostMock.getId()).thenReturn(hostMockId); + Mockito.when(dataCenterDeploymentMock.getHostId()).thenReturn(hostMockId); + Mockito.when(dataCenterDeploymentMock.getClusterId()).thenReturn(clusterMockId); - when(hostMock.getDataCenterId()).thenReturn(zoneMockId); - when(hostDaoMock.findById(any())).thenReturn(hostMock); + Mockito.when(hostMock.getDataCenterId()).thenReturn(zoneMockId); + Mockito.when(hostDaoMock.findById(any())).thenReturn(hostMock); - when(userVmJoinDaoMock.searchByIds(any())).thenReturn(new ArrayList<>()); - when(userVmDaoMock.findById(any())).thenReturn(userVmMock); + Mockito.when(userVmJoinDaoMock.searchByIds(any())).thenReturn(new ArrayList<>()); + Mockito.when(userVmDaoMock.findById(any())).thenReturn(userVmMock); + Mockito.when(vmDaoMock.findById(any())).thenReturn(vmInstanceMock); Mockito.doReturn(vmInstanceVoMockId).when(virtualMachineProfileMock).getId(); @@ -177,8 +178,8 @@ public void setup() { @Test public void testaddHostIpToCertDetailsIfConfigAllows() { - Host vmHost = mock(Host.class); - ConfigKey testConfig = mock(ConfigKey.class); + Host vmHost = Mockito.mock(Host.class); + ConfigKey testConfig = Mockito.mock(ConfigKey.class); Long dataCenterId = 5L; String hostIp = "1.1.1.1"; @@ -186,9 +187,9 @@ public void testaddHostIpToCertDetailsIfConfigAllows() { Map ipAddresses = new HashMap<>(); ipAddresses.put(NetworkElementCommand.ROUTER_IP, routerIp); - when(testConfig.valueIn(dataCenterId)).thenReturn(true); - when(vmHost.getDataCenterId()).thenReturn(dataCenterId); - when(vmHost.getPrivateIpAddress()).thenReturn(hostIp); + Mockito.when(testConfig.valueIn(dataCenterId)).thenReturn(true); + Mockito.when(vmHost.getDataCenterId()).thenReturn(dataCenterId); + Mockito.when(vmHost.getPrivateIpAddress()).thenReturn(hostIp); virtualMachineManagerImpl.addHostIpToCertDetailsIfConfigAllows(vmHost, ipAddresses, testConfig); assertTrue(ipAddresses.containsKey(NetworkElementCommand.HYPERVISOR_HOST_PRIVATE_IP)); @@ -199,17 +200,16 @@ public void testaddHostIpToCertDetailsIfConfigAllows() { @Test public void testaddHostIpToCertDetailsIfConfigAllowsWhenConfigFalse() { - Host vmHost = mock(Host.class); - ConfigKey testConfig = mock(ConfigKey.class); + Host vmHost = Mockito.mock(Host.class); + ConfigKey testConfig = Mockito.mock(ConfigKey.class); Long dataCenterId = 5L; - String hostIp = "1.1.1.1"; String routerIp = "2.2.2.2"; Map ipAddresses = new HashMap<>(); ipAddresses.put(NetworkElementCommand.ROUTER_IP, routerIp); - when(testConfig.valueIn(dataCenterId)).thenReturn(false); - when(vmHost.getDataCenterId()).thenReturn(dataCenterId); + Mockito.when(testConfig.valueIn(dataCenterId)).thenReturn(false); + Mockito.when(vmHost.getDataCenterId()).thenReturn(dataCenterId); virtualMachineManagerImpl.addHostIpToCertDetailsIfConfigAllows(vmHost, ipAddresses, testConfig); assertFalse(ipAddresses.containsKey(NetworkElementCommand.HYPERVISOR_HOST_PRIVATE_IP)); @@ -220,18 +220,18 @@ public void testaddHostIpToCertDetailsIfConfigAllowsWhenConfigFalse() { @Test(expected = CloudRuntimeException.class) public void testScaleVM3() throws Exception { DeploymentPlanner.ExcludeList excludeHostList = new DeploymentPlanner.ExcludeList(); - virtualMachineManagerImpl.findHostAndMigrate(vmInstanceMock.getUuid(), 2l, null, excludeHostList); + virtualMachineManagerImpl.findHostAndMigrate(vmInstanceMock.getUuid(), 2L, null, excludeHostList); } @Test public void testSendStopWithOkAnswer() throws Exception { - VirtualMachineGuru guru = mock(VirtualMachineGuru.class); - VirtualMachine vm = mock(VirtualMachine.class); - VirtualMachineProfile profile = mock(VirtualMachineProfile.class); + VirtualMachineGuru guru = Mockito.mock(VirtualMachineGuru.class); + VirtualMachine vm = Mockito.mock(VirtualMachine.class); + VirtualMachineProfile profile = Mockito.mock(VirtualMachineProfile.class); StopAnswer answer = new StopAnswer(new StopCommand(vm, false, false), "ok", true); - when(profile.getVirtualMachine()).thenReturn(vm); - when(vm.getHostId()).thenReturn(1L); - when(agentManagerMock.send(anyLong(), (Command)any())).thenReturn(answer); + Mockito.when(profile.getVirtualMachine()).thenReturn(vm); + Mockito.when(vm.getHostId()).thenReturn(1L); + Mockito.when(agentManagerMock.send(anyLong(), (Command)any())).thenReturn(answer); boolean actual = virtualMachineManagerImpl.sendStop(guru, profile, false, false); @@ -240,13 +240,13 @@ public void testSendStopWithOkAnswer() throws Exception { @Test public void testSendStopWithFailAnswer() throws Exception { - VirtualMachineGuru guru = mock(VirtualMachineGuru.class); - VirtualMachine vm = mock(VirtualMachine.class); - VirtualMachineProfile profile = mock(VirtualMachineProfile.class); + VirtualMachineGuru guru = Mockito.mock(VirtualMachineGuru.class); + VirtualMachine vm = Mockito.mock(VirtualMachine.class); + VirtualMachineProfile profile = Mockito.mock(VirtualMachineProfile.class); StopAnswer answer = new StopAnswer(new StopCommand(vm, false, false), "fail", false); - when(profile.getVirtualMachine()).thenReturn(vm); - when(vm.getHostId()).thenReturn(1L); - when(agentManagerMock.send(anyLong(), (Command)any())).thenReturn(answer); + Mockito.when(profile.getVirtualMachine()).thenReturn(vm); + Mockito.when(vm.getHostId()).thenReturn(1L); + Mockito.when(agentManagerMock.send(anyLong(), (Command)any())).thenReturn(answer); boolean actual = virtualMachineManagerImpl.sendStop(guru, profile, false, false); @@ -255,12 +255,12 @@ public void testSendStopWithFailAnswer() throws Exception { @Test public void testSendStopWithNullAnswer() throws Exception { - VirtualMachineGuru guru = mock(VirtualMachineGuru.class); - VirtualMachine vm = mock(VirtualMachine.class); - VirtualMachineProfile profile = mock(VirtualMachineProfile.class); - when(profile.getVirtualMachine()).thenReturn(vm); - when(vm.getHostId()).thenReturn(1L); - when(agentManagerMock.send(anyLong(), (Command)any())).thenReturn(null); + VirtualMachineGuru guru = Mockito.mock(VirtualMachineGuru.class); + VirtualMachine vm = Mockito.mock(VirtualMachine.class); + VirtualMachineProfile profile = Mockito.mock(VirtualMachineProfile.class); + Mockito.when(profile.getVirtualMachine()).thenReturn(vm); + Mockito.when(vm.getHostId()).thenReturn(1L); + Mockito.when(agentManagerMock.send(anyLong(), (Command)any())).thenReturn(null); boolean actual = virtualMachineManagerImpl.sendStop(guru, profile, false, false); @@ -269,9 +269,9 @@ public void testSendStopWithNullAnswer() throws Exception { @Test public void testExeceuteInSequence() { - assertTrue(virtualMachineManagerImpl.getExecuteInSequence(HypervisorType.XenServer) == false); - assertTrue(virtualMachineManagerImpl.getExecuteInSequence(HypervisorType.KVM) == false); - assertTrue(virtualMachineManagerImpl.getExecuteInSequence(HypervisorType.Ovm3) == VirtualMachineManager.ExecuteInSequence.value()); + assertFalse(virtualMachineManagerImpl.getExecuteInSequence(HypervisorType.XenServer)); + assertFalse(virtualMachineManagerImpl.getExecuteInSequence(HypervisorType.KVM)); + assertEquals(virtualMachineManagerImpl.getExecuteInSequence(HypervisorType.Ovm3), VirtualMachineManager.ExecuteInSequence.value()); } private void overrideDefaultConfigValue(final ConfigKey configKey, final String value) throws IllegalAccessException, NoSuchFieldException { @@ -297,31 +297,31 @@ public void testExeceuteInSequenceVmware() throws IllegalAccessException, NoSuch } @Test - public void testCheckIfCanUpgrade() throws Exception { - when(vmInstanceMock.getState()).thenReturn(State.Stopped); - when(serviceOfferingMock.isDynamic()).thenReturn(true); - when(vmInstanceMock.getServiceOfferingId()).thenReturn(1l); + public void testCheckIfCanUpgrade() { + Mockito.when(vmInstanceMock.getState()).thenReturn(State.Stopped); + Mockito.when(serviceOfferingMock.isDynamic()).thenReturn(true); + Mockito.when(vmInstanceMock.getServiceOfferingId()).thenReturn(1L); - ServiceOfferingVO mockCurrentServiceOffering = mock(ServiceOfferingVO.class); - DiskOfferingVO mockCurrentDiskOffering = mock(DiskOfferingVO.class); + ServiceOfferingVO mockCurrentServiceOffering = Mockito.mock(ServiceOfferingVO.class); + DiskOfferingVO mockCurrentDiskOffering = Mockito.mock(DiskOfferingVO.class); - when(serviceOfferingDaoMock.findByIdIncludingRemoved(anyLong(), anyLong())).thenReturn(mockCurrentServiceOffering); - when(diskOfferingDaoMock.findByIdIncludingRemoved(anyLong())).thenReturn(mockCurrentDiskOffering); - when(diskOfferingDaoMock.findById(anyLong())).thenReturn(diskOfferingMock); - when(diskOfferingMock.isUseLocalStorage()).thenReturn(false); - when(mockCurrentServiceOffering.isSystemUse()).thenReturn(true); - when(serviceOfferingMock.isSystemUse()).thenReturn(true); + Mockito.when(serviceOfferingDaoMock.findByIdIncludingRemoved(anyLong(), anyLong())).thenReturn(mockCurrentServiceOffering); + Mockito.when(diskOfferingDaoMock.findByIdIncludingRemoved(anyLong())).thenReturn(mockCurrentDiskOffering); + Mockito.when(diskOfferingDaoMock.findById(anyLong())).thenReturn(diskOfferingMock); + Mockito.when(diskOfferingMock.isUseLocalStorage()).thenReturn(false); + Mockito.when(mockCurrentServiceOffering.isSystemUse()).thenReturn(true); + Mockito.when(serviceOfferingMock.isSystemUse()).thenReturn(true); String[] oldDOStorageTags = {"x","y"}; String[] newDOStorageTags = {"z","x","y"}; - when(mockCurrentDiskOffering.getTagsArray()).thenReturn(oldDOStorageTags); - when(diskOfferingMock.getTagsArray()).thenReturn(newDOStorageTags); + Mockito.when(mockCurrentDiskOffering.getTagsArray()).thenReturn(oldDOStorageTags); + Mockito.when(diskOfferingMock.getTagsArray()).thenReturn(newDOStorageTags); virtualMachineManagerImpl.checkIfCanUpgrade(vmInstanceMock, serviceOfferingMock); } @Test(expected = InvalidParameterValueException.class) public void testCheckIfCanUpgradeFail() { - when(serviceOfferingMock.getState()).thenReturn(ServiceOffering.State.Inactive); + Mockito.when(serviceOfferingMock.getState()).thenReturn(ServiceOffering.State.Inactive); virtualMachineManagerImpl.checkIfCanUpgrade(vmInstanceMock, serviceOfferingMock); } @@ -390,7 +390,7 @@ public void buildMapUsingUserInformationTestUserDefinedMigrationMapEmpty() { Assert.assertTrue(volumeToPoolObjectMap.isEmpty()); - Mockito.verify(userDefinedVolumeToStoragePoolMap, times(0)).keySet(); + Mockito.verify(userDefinedVolumeToStoragePoolMap, Mockito.times(0)).keySet(); } @Test(expected = CloudRuntimeException.class) @@ -398,7 +398,7 @@ public void buildMapUsingUserInformationTestTargetHostDoesNotHaveAccessToPool() HashMap userDefinedVolumeToStoragePoolMap = new HashMap<>(); userDefinedVolumeToStoragePoolMap.put(volumeMockId, storagePoolVoMockId); - Mockito.doNothing().when(virtualMachineManagerImpl).executeManagedStorageChecksWhenTargetStoragePoolProvided(Mockito.any(StoragePoolVO.class), Mockito.any(VolumeVO.class), Mockito.any(StoragePoolVO.class)); + Mockito.doNothing().when(virtualMachineManagerImpl).executeManagedStorageChecksWhenTargetStoragePoolProvided(any(StoragePoolVO.class), any(VolumeVO.class), any(StoragePoolVO.class)); Mockito.doReturn(null).when(storagePoolHostDaoMock).findByPoolHost(storagePoolVoMockId, hostMockId); virtualMachineManagerImpl.buildMapUsingUserInformation(virtualMachineProfileMock, hostMock, userDefinedVolumeToStoragePoolMap); @@ -410,8 +410,8 @@ public void buildMapUsingUserInformationTestTargetHostHasAccessToPool() { HashMap userDefinedVolumeToStoragePoolMap = Mockito.spy(new HashMap<>()); userDefinedVolumeToStoragePoolMap.put(volumeMockId, storagePoolVoMockId); - Mockito.doNothing().when(virtualMachineManagerImpl).executeManagedStorageChecksWhenTargetStoragePoolProvided(Mockito.any(StoragePoolVO.class), Mockito.any(VolumeVO.class), - Mockito.any(StoragePoolVO.class)); + Mockito.doNothing().when(virtualMachineManagerImpl).executeManagedStorageChecksWhenTargetStoragePoolProvided(any(StoragePoolVO.class), any(VolumeVO.class), + any(StoragePoolVO.class)); Mockito.doReturn(Mockito.mock(StoragePoolHostVO.class)).when(storagePoolHostDaoMock).findByPoolHost(storagePoolVoMockId, hostMockId); Map volumeToPoolObjectMap = virtualMachineManagerImpl.buildMapUsingUserInformation(virtualMachineProfileMock, hostMock, userDefinedVolumeToStoragePoolMap); @@ -419,7 +419,7 @@ public void buildMapUsingUserInformationTestTargetHostHasAccessToPool() { assertFalse(volumeToPoolObjectMap.isEmpty()); assertEquals(storagePoolVoMock, volumeToPoolObjectMap.get(volumeVoMock)); - Mockito.verify(userDefinedVolumeToStoragePoolMap, times(1)).keySet(); + Mockito.verify(userDefinedVolumeToStoragePoolMap, Mockito.times(1)).keySet(); } @Test @@ -447,7 +447,7 @@ public void executeManagedStorageChecksWhenTargetStoragePoolNotProvidedTestCurre virtualMachineManagerImpl.executeManagedStorageChecksWhenTargetStoragePoolNotProvided(hostMock, storagePoolVoMock, volumeVoMock); Mockito.verify(storagePoolVoMock).isManaged(); - Mockito.verify(storagePoolHostDaoMock, Mockito.times(0)).findByPoolHost(Mockito.anyLong(), Mockito.anyLong()); + Mockito.verify(storagePoolHostDaoMock, Mockito.times(0)).findByPoolHost(anyLong(), anyLong()); } @Test @@ -471,15 +471,15 @@ public void executeManagedStorageChecksWhenTargetStoragePoolNotProvidedTestCurre @Test public void getCandidateStoragePoolsToMigrateLocalVolumeTestLocalVolume() { - Mockito.doReturn(Mockito.mock(DiskOfferingVO.class)).when(diskOfferingDaoMock).findById(Mockito.anyLong()); + Mockito.doReturn(Mockito.mock(DiskOfferingVO.class)).when(diskOfferingDaoMock).findById(anyLong()); Mockito.doReturn(true).when(storagePoolVoMock).isLocal(); List poolListMock = new ArrayList<>(); poolListMock.add(storagePoolVoMock); - Mockito.doReturn(poolListMock).when(storagePoolAllocatorMock).allocateToPool(Mockito.any(DiskProfile.class), Mockito.any(VirtualMachineProfile.class), Mockito.any(DeploymentPlan.class), - Mockito.any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); + Mockito.doReturn(poolListMock).when(storagePoolAllocatorMock).allocateToPool(any(DiskProfile.class), any(VirtualMachineProfile.class), any(DeploymentPlan.class), + any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); List poolList = virtualMachineManagerImpl.getCandidateStoragePoolsToMigrateLocalVolume(virtualMachineProfileMock, dataCenterDeploymentMock, volumeVoMock); @@ -489,15 +489,15 @@ public void getCandidateStoragePoolsToMigrateLocalVolumeTestLocalVolume() { @Test public void getCandidateStoragePoolsToMigrateLocalVolumeTestCrossClusterMigration() { - Mockito.doReturn(Mockito.mock(DiskOfferingVO.class)).when(diskOfferingDaoMock).findById(Mockito.anyLong()); + Mockito.doReturn(Mockito.mock(DiskOfferingVO.class)).when(diskOfferingDaoMock).findById(anyLong()); Mockito.doReturn(false).when(storagePoolVoMock).isLocal(); List poolListMock = new ArrayList<>(); poolListMock.add(storagePoolVoMock); - Mockito.doReturn(poolListMock).when(storagePoolAllocatorMock).allocateToPool(Mockito.any(DiskProfile.class), Mockito.any(VirtualMachineProfile.class), Mockito.any(DeploymentPlan.class), - Mockito.any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); + Mockito.doReturn(poolListMock).when(storagePoolAllocatorMock).allocateToPool(any(DiskProfile.class), any(VirtualMachineProfile.class), any(DeploymentPlan.class), + any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); Mockito.doReturn(true).when(virtualMachineManagerImpl).isStorageCrossClusterMigration(clusterMockId, storagePoolVoMock); List poolList = virtualMachineManagerImpl.getCandidateStoragePoolsToMigrateLocalVolume(virtualMachineProfileMock, dataCenterDeploymentMock, volumeVoMock); @@ -508,15 +508,15 @@ public void getCandidateStoragePoolsToMigrateLocalVolumeTestCrossClusterMigratio @Test public void getCandidateStoragePoolsToMigrateLocalVolumeTestWithinClusterMigration() { - Mockito.doReturn(Mockito.mock(DiskOfferingVO.class)).when(diskOfferingDaoMock).findById(Mockito.anyLong()); + Mockito.doReturn(Mockito.mock(DiskOfferingVO.class)).when(diskOfferingDaoMock).findById(anyLong()); Mockito.doReturn(false).when(storagePoolVoMock).isLocal(); List poolListMock = new ArrayList<>(); poolListMock.add(storagePoolVoMock); - Mockito.doReturn(poolListMock).when(storagePoolAllocatorMock).allocateToPool(Mockito.any(DiskProfile.class), Mockito.any(VirtualMachineProfile.class), Mockito.any(DeploymentPlan.class), - Mockito.any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); + Mockito.doReturn(poolListMock).when(storagePoolAllocatorMock).allocateToPool(any(DiskProfile.class), any(VirtualMachineProfile.class), any(DeploymentPlan.class), + any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); Mockito.doReturn(false).when(virtualMachineManagerImpl).isStorageCrossClusterMigration(clusterMockId, storagePoolVoMock); List poolList = virtualMachineManagerImpl.getCandidateStoragePoolsToMigrateLocalVolume(virtualMachineProfileMock, dataCenterDeploymentMock, volumeVoMock); @@ -536,33 +536,33 @@ public void getCandidateStoragePoolsToMigrateLocalVolumeTestMoreThanOneAllocator virtualMachineManagerImpl.setStoragePoolAllocators(storagePoolAllocatorsMock); - Mockito.doReturn(Mockito.mock(DiskOfferingVO.class)).when(diskOfferingDaoMock).findById(Mockito.anyLong()); + Mockito.doReturn(Mockito.mock(DiskOfferingVO.class)).when(diskOfferingDaoMock).findById(anyLong()); Mockito.doReturn(false).when(storagePoolVoMock).isLocal(); List poolListMock = new ArrayList<>(); poolListMock.add(storagePoolVoMock); - Mockito.doReturn(poolListMock).when(storagePoolAllocatorMock).allocateToPool(Mockito.any(DiskProfile.class), Mockito.any(VirtualMachineProfile.class), Mockito.any(DeploymentPlan.class), - Mockito.any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); + Mockito.doReturn(poolListMock).when(storagePoolAllocatorMock).allocateToPool(any(DiskProfile.class), any(VirtualMachineProfile.class), any(DeploymentPlan.class), + any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); - Mockito.doReturn(null).when(storagePoolAllocatorMock2).allocateToPool(Mockito.any(DiskProfile.class), Mockito.any(VirtualMachineProfile.class), Mockito.any(DeploymentPlan.class), - Mockito.any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); + Mockito.doReturn(null).when(storagePoolAllocatorMock2).allocateToPool(any(DiskProfile.class), any(VirtualMachineProfile.class), any(DeploymentPlan.class), + any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); - Mockito.doReturn(new ArrayList<>()).when(storagePoolAllocatorMock3).allocateToPool(Mockito.any(DiskProfile.class), Mockito.any(VirtualMachineProfile.class), Mockito.any(DeploymentPlan.class), - Mockito.any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); + Mockito.doReturn(new ArrayList<>()).when(storagePoolAllocatorMock3).allocateToPool(any(DiskProfile.class), any(VirtualMachineProfile.class), any(DeploymentPlan.class), + any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); Mockito.doReturn(false).when(virtualMachineManagerImpl).isStorageCrossClusterMigration(clusterMockId, storagePoolVoMock); List poolList = virtualMachineManagerImpl.getCandidateStoragePoolsToMigrateLocalVolume(virtualMachineProfileMock, dataCenterDeploymentMock, volumeVoMock); Assert.assertTrue(poolList.isEmpty()); - Mockito.verify(storagePoolAllocatorMock).allocateToPool(Mockito.any(DiskProfile.class), Mockito.any(VirtualMachineProfile.class), Mockito.any(DeploymentPlan.class), - Mockito.any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); - Mockito.verify(storagePoolAllocatorMock2).allocateToPool(Mockito.any(DiskProfile.class), Mockito.any(VirtualMachineProfile.class), Mockito.any(DeploymentPlan.class), - Mockito.any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); - Mockito.verify(storagePoolAllocatorMock3).allocateToPool(Mockito.any(DiskProfile.class), Mockito.any(VirtualMachineProfile.class), Mockito.any(DeploymentPlan.class), - Mockito.any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); + Mockito.verify(storagePoolAllocatorMock).allocateToPool(any(DiskProfile.class), any(VirtualMachineProfile.class), any(DeploymentPlan.class), + any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); + Mockito.verify(storagePoolAllocatorMock2).allocateToPool(any(DiskProfile.class), any(VirtualMachineProfile.class), any(DeploymentPlan.class), + any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); + Mockito.verify(storagePoolAllocatorMock3).allocateToPool(any(DiskProfile.class), any(VirtualMachineProfile.class), any(DeploymentPlan.class), + any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); } @Test(expected = CloudRuntimeException.class) @@ -651,8 +651,8 @@ public void createStoragePoolMappingsForVolumesTestNotCrossCluterMigrationWithCl HashMap volumeToPoolObjectMap = new HashMap<>(); Mockito.doReturn(ScopeType.CLUSTER).when(storagePoolVoMock).getScope(); - Mockito.doNothing().when(virtualMachineManagerImpl).executeManagedStorageChecksWhenTargetStoragePoolNotProvided(Mockito.any(), Mockito.any(), Mockito.any()); - Mockito.doReturn(false).when(virtualMachineManagerImpl).isStorageCrossClusterMigration(Mockito.anyLong(), Mockito.any()); + Mockito.doNothing().when(virtualMachineManagerImpl).executeManagedStorageChecksWhenTargetStoragePoolNotProvided(any(), any(), any()); + Mockito.doReturn(false).when(virtualMachineManagerImpl).isStorageCrossClusterMigration(anyLong(), any()); virtualMachineManagerImpl.createStoragePoolMappingsForVolumes(virtualMachineProfileMock, dataCenterDeploymentMock, volumeToPoolObjectMap, allVolumes); @@ -675,7 +675,7 @@ public void createMappingVolumeAndStoragePoolTest() { Mockito.doReturn(volumesNotMapped).when(virtualMachineManagerImpl).findVolumesThatWereNotMappedByTheUser(virtualMachineProfileMock, volumeToPoolObjectMap); Mockito.doNothing().when(virtualMachineManagerImpl).createStoragePoolMappingsForVolumes(Mockito.eq(virtualMachineProfileMock), - Mockito.any(DataCenterDeployment.class), Mockito.eq(volumeToPoolObjectMap), Mockito.eq(volumesNotMapped)); + any(DataCenterDeployment.class), Mockito.eq(volumeToPoolObjectMap), Mockito.eq(volumesNotMapped)); Map mappingVolumeAndStoragePool = virtualMachineManagerImpl.createMappingVolumeAndStoragePool(virtualMachineProfileMock, hostMock, new HashMap<>()); @@ -685,14 +685,14 @@ public void createMappingVolumeAndStoragePoolTest() { inOrder.verify(virtualMachineManagerImpl).buildMapUsingUserInformation(Mockito.eq(virtualMachineProfileMock), Mockito.eq(hostMock), Mockito.anyMapOf(Long.class, Long.class)); inOrder.verify(virtualMachineManagerImpl).findVolumesThatWereNotMappedByTheUser(virtualMachineProfileMock, volumeToPoolObjectMap); inOrder.verify(virtualMachineManagerImpl).createStoragePoolMappingsForVolumes(Mockito.eq(virtualMachineProfileMock), - Mockito.any(DataCenterDeployment.class), Mockito.eq(volumeToPoolObjectMap), Mockito.eq(volumesNotMapped)); + any(DataCenterDeployment.class), Mockito.eq(volumeToPoolObjectMap), Mockito.eq(volumesNotMapped)); } @Test public void matchesOfSorts() { List nothing = null; List empty = new ArrayList<>(); - List tag = Arrays.asList("bla"); + List tag = List.of("bla"); List tags = Arrays.asList("bla", "blob"); List others = Arrays.asList("bla", "blieb"); List three = Arrays.asList("bla", "blob", "blieb"); @@ -739,13 +739,13 @@ public void isRootVolumeOnLocalStorageTestZone() { private void prepareAndTestIsRootVolumeOnLocalStorage(ScopeType scope, boolean expected) { StoragePoolVO storagePoolVoMock = Mockito.mock(StoragePoolVO.class); - Mockito.doReturn(storagePoolVoMock).when(storagePoolDaoMock).findById(Mockito.anyLong()); + Mockito.doReturn(storagePoolVoMock).when(storagePoolDaoMock).findById(anyLong()); Mockito.doReturn(scope).when(storagePoolVoMock).getScope(); List mockedVolumes = new ArrayList<>(); mockedVolumes.add(volumeVoMock); - Mockito.doReturn(mockedVolumes).when(volumeDaoMock).findByInstanceAndType(Mockito.anyLong(), Mockito.any()); + Mockito.doReturn(mockedVolumes).when(volumeDaoMock).findByInstanceAndType(anyLong(), any()); - boolean result = virtualMachineManagerImpl.isRootVolumeOnLocalStorage(0l); + boolean result = virtualMachineManagerImpl.isRootVolumeOnLocalStorage(0L); assertEquals(expected, result); } @@ -771,9 +771,67 @@ public void checkIfNewOfferingStorageScopeMatchesStoragePoolTestSharedLocal() { } private void prepareAndRunCheckIfNewOfferingStorageScopeMatchesStoragePool(boolean isRootOnLocal, boolean isOfferingUsingLocal) { - Mockito.doReturn(isRootOnLocal).when(virtualMachineManagerImpl).isRootVolumeOnLocalStorage(Mockito.anyLong()); + Mockito.doReturn(isRootOnLocal).when(virtualMachineManagerImpl).isRootVolumeOnLocalStorage(anyLong()); Mockito.doReturn("vmInstanceMockedToString").when(vmInstanceMock).toString(); Mockito.doReturn(isOfferingUsingLocal).when(diskOfferingMock).isUseLocalStorage(); virtualMachineManagerImpl.checkIfNewOfferingStorageScopeMatchesStoragePool(vmInstanceMock, diskOfferingMock); } + + @Test + public void shouldIncrementVrResourceCountReturnWhenConfigResourceCountRunningVMsonlyIsEnabled() throws NoSuchFieldException, IllegalAccessException { + ServiceOffering offering = Mockito.mock(ServiceOffering.class); + Account owner = Mockito.mock(Account.class); + boolean isDeployOrDestroy = true; + overrideDefaultConfigValue(ResourceCountRunningVMsonly, "true"); + + virtualMachineManagerImpl.incrementVrResourceCount(offering, owner, isDeployOrDestroy); + Mockito.verify(virtualMachineManagerImpl, Mockito.times(0)).getServiceOfferingByConfig(); + Mockito.verify(virtualMachineManagerImpl, Mockito.times(0)).resolveCpuAndMemoryCount(any(), any(), any()); + Mockito.verify(virtualMachineManagerImpl, Mockito.times(0)).calculateResourceCount(any(), any(), Mockito.anyBoolean()); + } + + @Test + public void shouldIncrementVrResourceCountContinueWhenConfigResourceCountRunningVMsonlyIsDisabled() throws NoSuchFieldException, IllegalAccessException { + ServiceOffering offering = Mockito.mock(ServiceOffering.class); + Account owner = Mockito.mock(Account.class); + boolean isDeployOrDestroy = true; + overrideDefaultConfigValue(ResourceCountRunningVMsonly, "false"); + Mockito.doReturn(serviceOfferingMock).when(virtualMachineManagerImpl).getServiceOfferingByConfig(); + Mockito.doReturn(Pair.of("", "")).when(virtualMachineManagerImpl).resolveCpuAndMemoryCount(any(), any(), any()); + Mockito.doNothing().when(virtualMachineManagerImpl).calculateResourceCount(any(), any(), Mockito.anyBoolean()); + + virtualMachineManagerImpl.incrementVrResourceCount(offering, owner, isDeployOrDestroy); + Mockito.verify(virtualMachineManagerImpl, Mockito.times(1)).getServiceOfferingByConfig(); + Mockito.verify(virtualMachineManagerImpl, Mockito.times(1)).resolveCpuAndMemoryCount(any(), any(), any()); + Mockito.verify(virtualMachineManagerImpl, Mockito.times(1)).calculateResourceCount(any(), any(), Mockito.anyBoolean()); + } + + @Test + public void shouldDecrementVrResourceCountReturnWhenConfigResourceCountRunningVMsonlyIsEnabled() throws NoSuchFieldException, IllegalAccessException { + ServiceOffering offering = Mockito.mock(ServiceOffering.class); + Account owner = Mockito.mock(Account.class); + boolean isDeployOrDestroy = true; + overrideDefaultConfigValue(ResourceCountRunningVMsonly, "true"); + + virtualMachineManagerImpl.decrementVrResourceCount(offering, owner, isDeployOrDestroy); + Mockito.verify(virtualMachineManagerImpl, Mockito.times(0)).getServiceOfferingByConfig(); + Mockito.verify(virtualMachineManagerImpl, Mockito.times(0)).resolveCpuAndMemoryCount(any(), any(), any()); + Mockito.verify(virtualMachineManagerImpl, Mockito.times(0)).calculateResourceCount(any(), any(), Mockito.anyBoolean()); + } + + @Test + public void shouldDecrementVrResourceCountContinueWhenConfigResourceCountRunningVMsonlyIsDisabled() throws NoSuchFieldException, IllegalAccessException { + ServiceOffering offering = Mockito.mock(ServiceOffering.class); + Account owner = Mockito.mock(Account.class); + boolean isDeployOrDestroy = true; + overrideDefaultConfigValue(ResourceCountRunningVMsonly, "false"); + Mockito.doReturn(serviceOfferingMock).when(virtualMachineManagerImpl).getServiceOfferingByConfig(); + Mockito.doReturn(Pair.of("", "")).when(virtualMachineManagerImpl).resolveCpuAndMemoryCount(any(), any(), any()); + Mockito.doNothing().when(virtualMachineManagerImpl).calculateResourceCount(any(), any(), Mockito.anyBoolean()); + + virtualMachineManagerImpl.incrementVrResourceCount(offering, owner, isDeployOrDestroy); + Mockito.verify(virtualMachineManagerImpl, Mockito.times(1)).getServiceOfferingByConfig(); + Mockito.verify(virtualMachineManagerImpl, Mockito.times(1)).resolveCpuAndMemoryCount(any(), any(), any()); + Mockito.verify(virtualMachineManagerImpl, Mockito.times(1)).calculateResourceCount(any(), any(), Mockito.anyBoolean()); + } } diff --git a/server/src/main/java/com/cloud/network/router/NetworkHelperImpl.java b/server/src/main/java/com/cloud/network/router/NetworkHelperImpl.java index 90f9fda4bac9..950fddcd56c0 100644 --- a/server/src/main/java/com/cloud/network/router/NetworkHelperImpl.java +++ b/server/src/main/java/com/cloud/network/router/NetworkHelperImpl.java @@ -602,7 +602,7 @@ public void decrementVrResourceCount(Account owner, ServiceOfferingVO routerOffe if (Boolean.TRUE.equals(VirtualMachineManager.ResourceCountRouters.valueIn(owner.getDomainId()))) { _itMgr.decrementVrResourceCount(routerOffering, owner,true); } - } + } // shouldDecrementIfTheDomainExists /** * Increment the resource count with router offering. diff --git a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java index 257161ec3a6c..3741fd23e50d 100644 --- a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java +++ b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java @@ -92,6 +92,7 @@ import com.cloud.user.AccountVO; import com.cloud.user.ResourceLimitService; import com.cloud.user.dao.AccountDao; +import com.cloud.utils.Pair; import com.cloud.utils.component.ManagerBase; import com.cloud.utils.concurrency.NamedThreadFactory; import com.cloud.utils.db.DB; @@ -119,6 +120,7 @@ @Component public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLimitService, Configurable { public static final Logger s_logger = Logger.getLogger(ResourceLimitManagerImpl.class); + public static final String OFFERINGS_NAME = "offerings"; @Inject private AccountManager _accountMgr; @@ -178,9 +180,9 @@ public class ResourceLimitManagerImpl extends ManagerBase implements ResourceLim protected SearchBuilder ResourceCountSearch; ScheduledExecutorService _rcExecutor; long _resourceCountCheckInterval = 0; - Map accountResourceLimitMap = new EnumMap(ResourceType.class); - Map domainResourceLimitMap = new EnumMap(ResourceType.class); - Map projectResourceLimitMap = new EnumMap(ResourceType.class); + Map accountResourceLimitMap = new EnumMap<>(ResourceType.class); + Map domainResourceLimitMap = new EnumMap<>(ResourceType.class); + Map projectResourceLimitMap = new EnumMap<>(ResourceType.class); @Override public boolean start() { @@ -279,7 +281,7 @@ public void incrementResourceCount(long accountId, ResourceType type, Long... de return; } - long numToIncrement = (delta.length == 0) ? 1 : delta[0].longValue(); + long numToIncrement = (delta.length == 0) ? 1 : delta[0]; if (!updateResourceCountForAccount(accountId, type, true, numToIncrement)) { // we should fail the operation (resource creation) when failed to update the resource count @@ -294,7 +296,7 @@ public void decrementResourceCount(long accountId, ResourceType type, Long... de s_logger.trace("Not decrementing resource count for system accounts, returning"); return; } - long numToDecrement = (delta.length == 0) ? 1 : delta[0].longValue(); + long numToDecrement = (delta.length == 0) ? 1 : delta[0]; if (!updateResourceCountForAccount(accountId, type, false, numToDecrement)) { _alertMgr.sendAlert(AlertManager.AlertType.ALERT_TYPE_UPDATE_RESOURCE_COUNT, 0L, 0L, "Failed to decrement resource count of type " + type + " for account id=" + accountId, @@ -316,10 +318,10 @@ public long findCorrectResourceLimitForAccount(Account account, ResourceType typ // Check if limit is configured for account if (limit != null) { - max = limit.getMax().longValue(); + max = limit.getMax(); } else { // If the account has an no limit set, then return global default account limits - Long value = null; + Long value; if (account.getType() == Account.Type.PROJECT) { value = projectResourceLimitMap.get(type); } else { @@ -357,10 +359,10 @@ public long findCorrectResourceLimitForAccount(long accountId, Long limit, Resou // Check if limit is configured for account if (limit != null) { - max = limit.longValue(); + max = limit; } else { // If the account has an no limit set, then return global default account limits - Long value = null; + Long value; if (account.getType() == Account.Type.PROJECT) { value = projectResourceLimitMap.get(type); } else { @@ -392,7 +394,7 @@ public long findCorrectResourceLimitForDomain(Domain domain, ResourceType type) ResourceLimitVO limit = _resourceLimitDao.findByOwnerIdAndType(domain.getId(), ResourceOwnerType.Domain, type); if (limit != null) { - max = limit.getMax().longValue(); + max = limit.getMax(); } else { // check domain hierarchy Long domainId = domain.getParent(); @@ -406,9 +408,9 @@ public long findCorrectResourceLimitForDomain(Domain domain, ResourceType type) } if (limit != null) { - max = limit.getMax().longValue(); + max = limit.getMax(); } else { - Long value = null; + Long value; value = domainResourceLimitMap.get(type); if (value != null) { if (value < 0) { // return unlimit if value is set to negative @@ -427,7 +429,7 @@ public long findCorrectResourceLimitForDomain(Domain domain, ResourceType type) private void checkDomainResourceLimit(final Account account, final Project project, final ResourceType type, long numResources) throws ResourceAllocationException { // check all domains in the account's domain hierarchy - Long domainId = null; + Long domainId; if (project != null) { domainId = project.getDomainId(); } else { @@ -440,8 +442,7 @@ private void checkDomainResourceLimit(final Account account, final Project proje if (domainId != Domain.ROOT_DOMAIN) { long domainResourceLimit = findCorrectResourceLimitForDomain(domain, type); long currentDomainResourceCount = _resourceCountDao.getResourceCount(domainId, ResourceOwnerType.Domain, type); - long currentResourceReservation = reservationDao.getDomainReservation(domainId, type); - long requestedDomainResourceCount = currentDomainResourceCount + currentResourceReservation + numResources; + long requestedDomainResourceCount = currentDomainResourceCount + numResources; String messageSuffix = " domain resource limits of Type '" + type + "'" + " for Domain Id = " + domainId + " is exceeded: Domain Resource Limit = " + toHumanReadableSize(domainResourceLimit) + ", Current Domain Resource Amount = " + toHumanReadableSize(currentDomainResourceCount) + ", Requested Resource Amount = " + toHumanReadableSize(numResources) + "."; @@ -464,8 +465,7 @@ private void checkAccountResourceLimit(final Account account, final Project proj // Check account limits long accountResourceLimit = findCorrectResourceLimitForAccount(account, type); long currentResourceCount = _resourceCountDao.getResourceCount(account.getId(), ResourceOwnerType.Account, type); - long currentResourceReservation = reservationDao.getAccountReservation(account.getId(), type); - long requestedResourceCount = currentResourceCount + currentResourceReservation + numResources; + long requestedResourceCount = currentResourceCount + numResources; String convertedAccountResourceLimit = String.valueOf(accountResourceLimit); String convertedCurrentResourceCount = String.valueOf(currentResourceCount); @@ -509,14 +509,14 @@ private List lockDomainRows(long domainId, final ResourceType t @Override public long findDefaultResourceLimitForDomain(ResourceType resourceType) { - Long resourceLimit = null; + Long resourceLimit; resourceLimit = domainResourceLimitMap.get(resourceType); if (resourceLimit != null && (resourceType == ResourceType.primary_storage || resourceType == ResourceType.secondary_storage)) { if (! Long.valueOf(Resource.RESOURCE_UNLIMITED).equals(resourceLimit)) { resourceLimit = resourceLimit * ResourceType.bytesToGiB; } } else { - resourceLimit = Long.valueOf(Resource.RESOURCE_UNLIMITED); + resourceLimit = (long) Resource.RESOURCE_UNLIMITED; } return resourceLimit; } @@ -553,8 +553,8 @@ public void doInTransactionWithoutResult(TransactionStatus status) throws Resour @Override public List searchForLimits(Long id, Long accountId, Long domainId, ResourceType resourceType, Long startIndex, Long pageSizeVal) { Account caller = CallContext.current().getCallingAccount(); - List limits = new ArrayList(); - boolean isAccount = true; + List limits = new ArrayList<>(); + boolean isAccount; if (!_accountMgr.isAdmin(caller.getId())) { accountId = caller.getId(); @@ -649,8 +649,8 @@ public List searchForLimits(Long id, Long accountId, Long domai // see if any limits are missing from the table, and if yes - get it from the config table and add ResourceType[] resourceTypes = ResourceCount.ResourceType.values(); if (foundLimits.size() != resourceTypes.length) { - List accountLimitStr = new ArrayList(); - List domainLimitStr = new ArrayList(); + List accountLimitStr = new ArrayList<>(); + List domainLimitStr = new ArrayList<>(); for (ResourceLimitVO foundLimit : foundLimits) { if (foundLimit.getAccountId() != null) { accountLimitStr.add(foundLimit.getType().toString()); @@ -698,7 +698,7 @@ public ResourceLimitVO updateResourceLimit(Long accountId, Long domainId, Intege ResourceType resourceType = null; if (typeId != null) { for (ResourceType type : Resource.ResourceType.values()) { - if (type.getOrdinal() == typeId.intValue()) { + if (type.getOrdinal() == typeId) { resourceType = type; } } @@ -729,7 +729,7 @@ public ResourceLimitVO updateResourceLimit(Long accountId, Long domainId, Intege throw new InvalidParameterValueException("Only " + Resource.RESOURCE_UNLIMITED + " limit is supported for Root Admin accounts"); } - if ((caller.getAccountId() == accountId.longValue()) && (_accountMgr.isDomainAdmin(caller.getId()) || caller.getType() == Account.Type.RESOURCE_DOMAIN_ADMIN)) { + if ((caller.getAccountId() == accountId) && (_accountMgr.isDomainAdmin(caller.getId()) || caller.getType() == Account.Type.RESOURCE_DOMAIN_ADMIN)) { // If the admin is trying to update their own account, disallow. throw new PermissionDeniedException("Unable to update resource limit for their own account " + accountId + ", permission denied"); } @@ -747,12 +747,12 @@ public ResourceLimitVO updateResourceLimit(Long accountId, Long domainId, Intege _accountMgr.checkAccess(caller, domain); - if (Domain.ROOT_DOMAIN == domainId.longValue()) { + if (Domain.ROOT_DOMAIN == domainId) { // no one can add limits on ROOT domain, disallow... throw new PermissionDeniedException("Cannot update resource limit for ROOT domain " + domainId + ", permission denied"); } - if ((caller.getDomainId() == domainId.longValue()) && caller.getType() == Account.Type.DOMAIN_ADMIN || caller.getType() == Account.Type.RESOURCE_DOMAIN_ADMIN) { + if ((caller.getDomainId() == domainId) && caller.getType() == Account.Type.DOMAIN_ADMIN || caller.getType() == Account.Type.RESOURCE_DOMAIN_ADMIN) { // if the admin is trying to update their own domain, disallow... throw new PermissionDeniedException("Unable to update resource limit for domain " + domainId + ", permission denied"); } @@ -760,7 +760,7 @@ public ResourceLimitVO updateResourceLimit(Long accountId, Long domainId, Intege if (parentDomainId != null) { DomainVO parentDomain = _domainDao.findById(parentDomainId); long parentMaximum = findCorrectResourceLimitForDomain(parentDomain, resourceType); - if ((parentMaximum >= 0) && (max.longValue() > parentMaximum)) { + if ((parentMaximum >= 0) && (max > parentMaximum)) { throw new InvalidParameterValueException("Domain " + domain.getName() + "(id: " + parentDomain.getId() + ") has maximum allowed resource limit " + parentMaximum + " for " + resourceType + ", please specify a value less that or equal to " + parentMaximum); } @@ -784,17 +784,17 @@ public ResourceLimitVO updateResourceLimit(Long accountId, Long domainId, Intege } @Override - public List recalculateResourceCount(Long accountId, Long domainId, Integer typeId) throws InvalidParameterValueException, CloudRuntimeException, PermissionDeniedException { + public List recalculateResourceCount(Long accountId, Long domainId, Integer typeId) throws CloudRuntimeException { Account callerAccount = CallContext.current().getCallingAccount(); long count = 0; - List counts = new ArrayList(); - List resourceTypes = new ArrayList(); + List counts = new ArrayList<>(); + List resourceTypes = new ArrayList<>(); ResourceType resourceType = null; if (typeId != null) { for (ResourceType type : Resource.ResourceType.values()) { - if (type.getOrdinal() == typeId.intValue()) { + if (type.getOrdinal() == typeId) { resourceType = type; } } @@ -977,8 +977,7 @@ public void doInTransactionWithoutResult(TransactionStatus status) { public long countCpusForAccount(long accountId) { long cputotal = 0L; - Account owner = _accountDao.findById(accountId); - DomainVO domain = _domainDao.findById(owner.getDomainId()); + // user vms SearchBuilder userVmSearch = _userVmJoinDao.createSearchBuilder(); userVmSearch.and("accountId", userVmSearch.entity().getAccountId(), Op.EQ); @@ -989,15 +988,31 @@ public long countCpusForAccount(long accountId) { SearchCriteria sc1 = userVmSearch.create(); sc1.setParameters("accountId", accountId); - if (VirtualMachineManager.ResourceCountRunningVMsonly.value()) + if (VirtualMachineManager.ResourceCountRunningVMsonly.value()) { sc1.setParameters("state", State.Destroyed, State.Error, State.Expunging, State.Stopped); - else + } else { sc1.setParameters("state", State.Destroyed, State.Error, State.Expunging); + } sc1.setParameters("displayVm", 1); List userVms = _userVmJoinDao.search(sc1,null); for (UserVmJoinVO vm : userVms) { cputotal += vm.getCpu(); } + + final Pair totalByAccountId = calculateCpuTotalByAccountId(accountId); + if (totalByAccountId.first()) { + return totalByAccountId.second(); + } else { + return cputotal; + } + } + + private Pair calculateCpuTotalByAccountId(long accountId) { + long cputotal = 0; + boolean hasCpus = false; + final Account owner = _accountDao.findById(accountId); + final DomainVO domain = _domainDao.findById(owner.getDomainId()); + ServiceOffering defaultRouterOffering = null; String globalRouterOffering = VirtualNetworkApplianceManager.VirtualRouterServiceOffering.value(); if (globalRouterOffering != null) { @@ -1015,23 +1030,24 @@ public long countCpusForAccount(long accountId) { join1.and("type", join1.entity().getType(), Op.IN); join1.and("state", join1.entity().getState(), SearchCriteria.Op.NIN); join1.and("displayVm", join1.entity().isDisplayVm(), Op.EQ); - cpuSearch.join("offerings", join1, cpuSearch.entity().getId(), join1.entity().getServiceOfferingId(), JoinBuilder.JoinType.INNER); + cpuSearch.join(OFFERINGS_NAME, join1, cpuSearch.entity().getId(), join1.entity().getServiceOfferingId(), JoinBuilder.JoinType.INNER); cpuSearch.done(); SearchCriteria sc = cpuSearch.create(); - sc.setJoinParameters("offerings", "accountId", accountId); - sc.setJoinParameters("offerings", "type", VirtualMachine.Type.DomainRouter); // domain routers + sc.setJoinParameters(OFFERINGS_NAME, "accountId", accountId); + sc.setJoinParameters(OFFERINGS_NAME, "type", VirtualMachine.Type.DomainRouter); // domain routers if (Boolean.TRUE.equals(VirtualMachineManager.ResourceCountRunningVMsonly.value())) { - sc.setJoinParameters("offerings", "state", State.Destroyed, State.Error, State.Expunging, State.Stopped); + sc.setJoinParameters(OFFERINGS_NAME, "state", State.Destroyed, State.Error, State.Expunging, State.Stopped); } else { - sc.setJoinParameters("offerings", "state", State.Destroyed, State.Error, State.Expunging); + sc.setJoinParameters(OFFERINGS_NAME, "state", State.Destroyed, State.Error, State.Expunging); } - sc.setJoinParameters("offerings", "displayVm", 1); + sc.setJoinParameters(OFFERINGS_NAME, "displayVm", 1); List cpus = serviceOfferingDao.customSearch(sc, null); if (cpus != null) { + hasCpus = true; // Calculate the VR CPU resource count depending on the global setting - if (VirtualMachineManager.ResourceCountRouters.valueIn(domain.getId())) { + if (Boolean.TRUE.equals(VirtualMachineManager.ResourceCountRouters.valueIn(domain.getId()))) { cputotal += cpus.get(0).sum; // Get the diff of current router offering with default offering @@ -1040,16 +1056,14 @@ public long countCpusForAccount(long accountId) { cputotal = cputotal - cpus.get(0).count * defaultRouterOffering.getCpu(); } } - return cputotal; - } else { - return cputotal; } + + return Pair.of(hasCpus, cputotal); } public long calculateMemoryForAccount(long accountId) { long ramtotal = 0L; - Account owner = _accountDao.findById(accountId); - DomainVO domain = _domainDao.findById(owner.getDomainId()); + // user vms SearchBuilder userVmSearch = _userVmJoinDao.createSearchBuilder(); userVmSearch.and("accountId", userVmSearch.entity().getAccountId(), Op.EQ); @@ -1060,15 +1074,30 @@ public long calculateMemoryForAccount(long accountId) { SearchCriteria sc1 = userVmSearch.create(); sc1.setParameters("accountId", accountId); - if (VirtualMachineManager.ResourceCountRunningVMsonly.value()) + if (Boolean.TRUE.equals(VirtualMachineManager.ResourceCountRunningVMsonly.value())) { sc1.setParameters("state", State.Destroyed, State.Error, State.Expunging, State.Stopped); - else + } else { sc1.setParameters("state", State.Destroyed, State.Error, State.Expunging); + } sc1.setParameters("displayVm", 1); List userVms = _userVmJoinDao.search(sc1,null); for (UserVmJoinVO vm : userVms) { ramtotal += vm.getRamSize(); } + + final Pair totalByAccountId = calculateMemoryTotalByAccountId(accountId); + if (Boolean.TRUE.equals(totalByAccountId.first())) { + return totalByAccountId.second(); + } else { + return ramtotal; + } + } + + public Pair calculateMemoryTotalByAccountId(long accountId) { + long ramtotal = 0; + boolean hasMemory = false; + Account owner = _accountDao.findById(accountId); + DomainVO domain = _domainDao.findById(owner.getDomainId()); ServiceOffering defaultRouterOffering = null; String globalRouterOffering = VirtualNetworkApplianceManager.VirtualRouterServiceOffering.value(); if (globalRouterOffering != null) { @@ -1079,40 +1108,40 @@ public long calculateMemoryForAccount(long accountId) { } GenericSearchBuilder memorySearch = serviceOfferingDao.createSearchBuilder(SumCount.class); memorySearch.select("sum", Func.SUM, memorySearch.entity().getRamSize()); - memorySearch.select("count", Func.COUNT, (Object[])null); + memorySearch.select("count", Func.COUNT, null); SearchBuilder join1 = _vmDao.createSearchBuilder(); join1.and("accountId", join1.entity().getAccountId(), Op.EQ); join1.and("type", join1.entity().getType(), Op.IN); join1.and("state", join1.entity().getState(), SearchCriteria.Op.NIN); join1.and("displayVm", join1.entity().isDisplayVm(), Op.EQ); - memorySearch.join("offerings", join1, memorySearch.entity().getId(), join1.entity().getServiceOfferingId(), JoinBuilder.JoinType.INNER); + memorySearch.join(OFFERINGS_NAME, join1, memorySearch.entity().getId(), join1.entity().getServiceOfferingId(), JoinBuilder.JoinType.INNER); memorySearch.done(); SearchCriteria sc = memorySearch.create(); - sc.setJoinParameters("offerings", "accountId", accountId); - sc.setJoinParameters("offerings", "type", VirtualMachine.Type.DomainRouter); // domain routers - sc.setJoinParameters("offerings", "displayVm", 1); + sc.setJoinParameters(OFFERINGS_NAME, "accountId", accountId); + sc.setJoinParameters(OFFERINGS_NAME, "type", VirtualMachine.Type.DomainRouter); // domain routers + sc.setJoinParameters(OFFERINGS_NAME, "displayVm", 1); if (Boolean.TRUE.equals(VirtualMachineManager.ResourceCountRunningVMsonly.value())) { - sc.setJoinParameters("offerings", "state", State.Destroyed, State.Error, State.Expunging, State.Stopped); + sc.setJoinParameters(OFFERINGS_NAME, "state", State.Destroyed, State.Error, State.Expunging, State.Stopped); } else { - sc.setJoinParameters("offerings", "state", State.Destroyed, State.Error, State.Expunging); + sc.setJoinParameters(OFFERINGS_NAME, "state", State.Destroyed, State.Error, State.Expunging); } - sc.setJoinParameters("offerings", "displayVm", 1); + sc.setJoinParameters(OFFERINGS_NAME, "displayVm", 1); List memory = serviceOfferingDao.customSearch(sc, null); if (memory != null) { // Calculate VR memory count depending on global setting - if (VirtualMachineManager.ResourceCountRouters.valueIn(domain.getId())) { + if (Boolean.TRUE.equals(VirtualMachineManager.ResourceCountRouters.valueIn(domain.getId()))) { ramtotal += memory.get(0).sum; if (VirtualMachineManager.ResourceCountRoutersType.valueIn(domain.getId()) .equalsIgnoreCase(VirtualMachineManager.COUNT_DELTA_VR_RESOURCES)) { ramtotal = ramtotal - memory.get(0).count * defaultRouterOffering.getRamSize(); } } - return ramtotal; - } else { - return ramtotal; + + hasMemory = true; } + return Pair.of(hasMemory, ramtotal); } public long calculateSecondaryStorageForAccount(long accountId) { diff --git a/server/src/test/java/com/cloud/network/router/NetworkHelperImplTest.java b/server/src/test/java/com/cloud/network/router/NetworkHelperImplTest.java index 4267b71d24fa..27f1397e88f1 100644 --- a/server/src/test/java/com/cloud/network/router/NetworkHelperImplTest.java +++ b/server/src/test/java/com/cloud/network/router/NetworkHelperImplTest.java @@ -26,12 +26,16 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import com.cloud.service.ServiceOfferingVO; +import com.cloud.user.Account; +import com.cloud.vm.VirtualMachineManager; +import org.apache.cloudstack.framework.config.ConfigKey; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; import org.mockito.Matchers; import org.mockito.Mock; -import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.Mockito; import com.cloud.agent.AgentManager; import com.cloud.agent.api.Answer; @@ -40,7 +44,9 @@ import com.cloud.exception.AgentUnavailableException; import com.cloud.exception.OperationTimedoutException; import com.cloud.exception.ResourceUnavailableException; +import org.mockito.junit.MockitoJUnitRunner; +import java.lang.reflect.Field; @RunWith(MockitoJUnitRunner.class) public class NetworkHelperImplTest { @@ -53,9 +59,12 @@ public class NetworkHelperImplTest { @InjectMocks protected NetworkHelperImpl nwHelper = new NetworkHelperImpl(); + @Mock + protected VirtualMachineManager _itMgr; + @Test(expected=ResourceUnavailableException.class) public void testSendCommandsToRouterWrongRouterVersion() - throws AgentUnavailableException, OperationTimedoutException, ResourceUnavailableException { + throws OperationTimedoutException, ResourceUnavailableException { // Prepare NetworkHelperImpl nwHelperUT = spy(this.nwHelper); VirtualRouter vr = mock(VirtualRouter.class); @@ -70,7 +79,7 @@ public void testSendCommandsToRouterWrongRouterVersion() @Test public void testSendCommandsToRouter() - throws AgentUnavailableException, OperationTimedoutException, ResourceUnavailableException { + throws OperationTimedoutException, ResourceUnavailableException { // Prepare NetworkHelperImpl nwHelperUT = spy(this.nwHelper); VirtualRouter vr = mock(VirtualRouter.class); @@ -108,7 +117,7 @@ public void testSendCommandsToRouter() */ @Test public void testSendCommandsToRouterWithTrueResult() - throws AgentUnavailableException, OperationTimedoutException, ResourceUnavailableException { + throws OperationTimedoutException, ResourceUnavailableException { // Prepare NetworkHelperImpl nwHelperUT = spy(this.nwHelper); VirtualRouter vr = mock(VirtualRouter.class); @@ -146,7 +155,7 @@ public void testSendCommandsToRouterWithTrueResult() */ @Test public void testSendCommandsToRouterWithNoAnswers() - throws AgentUnavailableException, OperationTimedoutException, ResourceUnavailableException { + throws OperationTimedoutException, ResourceUnavailableException { // Prepare NetworkHelperImpl nwHelperUT = spy(this.nwHelper); VirtualRouter vr = mock(VirtualRouter.class); @@ -170,4 +179,53 @@ public void testSendCommandsToRouterWithNoAnswers() assertFalse(result); } + @Test + public void shouldIncrementIfTheDomainExists() throws NoSuchFieldException, IllegalAccessException { + Account owner = Mockito.mock(Account.class); + ServiceOfferingVO offering = mock(ServiceOfferingVO.class); + when(owner.getDomainId()).thenReturn(1L); + + overrideDefaultConfigValue(VirtualMachineManager.ResourceCountRouters, "true"); + nwHelper.incrementVrResourceCount(owner, offering); + verify(_itMgr, times(1)).incrementVrResourceCount(offering, owner, true); + } + + @Test + public void shouldDoNothingIfTheDomainDoNotExistsForIncrementVrResourceCount() throws NoSuchFieldException, IllegalAccessException { + Account owner = Mockito.mock(Account.class); + ServiceOfferingVO offering = mock(ServiceOfferingVO.class); + when(owner.getDomainId()).thenReturn(999L); + + overrideDefaultConfigValue(VirtualMachineManager.ResourceCountRouters, "false"); + nwHelper.incrementVrResourceCount(owner, offering); + verify(_itMgr, times(0)).incrementVrResourceCount(offering, owner, true); + } + + @Test + public void shouldDecrementIfTheDomainExists() throws NoSuchFieldException, IllegalAccessException { + Account owner = Mockito.mock(Account.class); + ServiceOfferingVO offering = mock(ServiceOfferingVO.class); + when(owner.getDomainId()).thenReturn(1L); + + overrideDefaultConfigValue(VirtualMachineManager.ResourceCountRouters, "true"); + nwHelper.decrementVrResourceCount(owner, offering); + verify(_itMgr, times(1)).decrementVrResourceCount(offering, owner, true); + } + + @Test + public void shouldDoNothingIfTheDomainDoNotExistsForDecrementVrResourceCount() throws NoSuchFieldException, IllegalAccessException { + Account owner = Mockito.mock(Account.class); + ServiceOfferingVO offering = mock(ServiceOfferingVO.class); + when(owner.getDomainId()).thenReturn(999L); + + overrideDefaultConfigValue(VirtualMachineManager.ResourceCountRouters, "false"); + nwHelper.decrementVrResourceCount(owner, offering); + verify(_itMgr, times(0)).decrementVrResourceCount(offering, owner, true); + } + + private void overrideDefaultConfigValue(final ConfigKey configKey, final String value) throws IllegalAccessException, NoSuchFieldException { + final Field f = ConfigKey.class.getDeclaredField("_defaultValue"); + f.setAccessible(true); + f.set(configKey, value); + } } diff --git a/utils/src/main/java/com/cloud/utils/Pair.java b/utils/src/main/java/com/cloud/utils/Pair.java index 93dcc75a9dbf..0b530e954006 100644 --- a/utils/src/main/java/com/cloud/utils/Pair.java +++ b/utils/src/main/java/com/cloud/utils/Pair.java @@ -65,8 +65,7 @@ public void set(T t, U u) { @Override // Note: This means any two pairs with null for both values will match each // other but what can I do? This is due to stupid type erasure. - public - int hashCode() { + public int hashCode() { return (t != null ? t.hashCode() : 0) | (u != null ? u.hashCode() : 0); } diff --git a/utils/src/test/java/com/cloud/utils/PairTest.java b/utils/src/test/java/com/cloud/utils/PairTest.java new file mode 100644 index 000000000000..f55dec4fd968 --- /dev/null +++ b/utils/src/test/java/com/cloud/utils/PairTest.java @@ -0,0 +1,87 @@ +package com.cloud.utils; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; + +public class PairTest { + + @Test + public void shouldReturnAValidInstanceFromPairWithStrings() { + final String expetedFirst = "A"; + final String expetedSecond = "B"; + final Pair actual = Pair.of("A", "B"); + + assertNotNull("the pair is null",actual); + assertEquals(expetedFirst, actual.first()); + assertEquals(expetedSecond, actual.second()); + } + + @Test + public void shouldReturnAValidInstanceFromPairWithIntegers() { + final Integer expetedFirst = 1; + final Integer expetedSecond = 2; + final Pair actual = Pair.of(1, 2); + + assertNotNull("the pair is null",actual); + assertEquals(expetedFirst, actual.first()); + assertEquals(expetedSecond, actual.second()); + } + + @Test + public void shouldReturnAValidInstanceFromPairWithLongs() { + final Long expetedFirst = 1L; + final Long expetedSecond = 2L; + final Pair actual = Pair.of(1L, 2L); + + assertNotNull("the pair is null",actual); + assertEquals(expetedFirst, actual.first()); + assertEquals(expetedSecond, actual.second()); + } + + @Test + public void shouldReturnAValidInstanceFromPairWithDiferentsTypes() { + final String expetedFirst = "A"; + final Long expetedSecond = 2L; + final Pair actual = Pair.of("A", 2L); + + assertNotNull("the pair is null",actual); + assertEquals(expetedFirst, actual.first()); + assertEquals(expetedSecond, actual.second()); + } + + @Test + public void shouldReturnAValidInstanceFromPairWithNulls() { + final Pair actual = Pair.of(null,null); + + assertNotNull("the pair is null",actual); + assertNull(actual.first()); + assertNull(actual.second()); + } + + @Test + public void testToString_whenTAndUAreNull() { + final Pair p = new Pair<>(null, null); + final String expected = "P[null:null]"; + final String actual = p.toString(); + assertEquals(expected, actual); + } + + @Test + public void testToString_whenTNotNullAndUIsNull() { + final Pair p = new Pair<>("apple", null); + final String expected = "P[apple:null]"; + final String actual = p.toString(); + assertEquals(expected, actual); + } + + @Test + public void testToString_whenTIsNullAndUNotNull() { + final Pair p = new Pair<>(null, "orange"); + final String expected = "P[null:orange]"; + final String actual = p.toString(); + assertEquals(expected, actual); + } +} From 7fd59a656bd7ab42ea9f165f798a96d210e47692 Mon Sep 17 00:00:00 2001 From: Chales Queiroz Date: Thu, 13 Apr 2023 19:04:11 +0100 Subject: [PATCH 8/9] Fixing conflicts --- .../cloud/vm/VirtualMachineManagerImpl.java | 25 ++ .../vm/VirtualMachineManagerImplTest.java | 275 +++++++++++------- tools/marvin/setup.py | 2 +- 3 files changed, 198 insertions(+), 104 deletions(-) diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index df73cc48161a..a2f0bda58432 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -216,6 +216,7 @@ import com.cloud.storage.StorageManager; import com.cloud.storage.StoragePool; import com.cloud.storage.VMTemplateVO; +import com.cloud.storage.VMTemplateZoneVO; import com.cloud.storage.Volume; import com.cloud.storage.Volume.Type; import com.cloud.storage.VolumeApiService; @@ -226,6 +227,7 @@ import com.cloud.storage.dao.GuestOSDao; import com.cloud.storage.dao.StoragePoolHostDao; import com.cloud.storage.dao.VMTemplateDao; +import com.cloud.storage.dao.VMTemplateZoneDao; import com.cloud.storage.dao.VolumeDao; import com.cloud.template.VirtualMachineTemplate; import com.cloud.user.Account; @@ -292,6 +294,8 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac @Inject private VMTemplateDao _templateDao; @Inject + private VMTemplateZoneDao templateZoneDao; + @Inject private ItWorkDao _workDao; @Inject private UserVmDao _userVmDao; @@ -1032,6 +1036,25 @@ protected void addHostIpToCertDetailsIfConfigAllows(Host vmHost, Map existingRootVolumes = _volsDao.findReadyRootVolumesByInstance(vm.getId()); + if (CollectionUtils.isNotEmpty(existingRootVolumes)) { + return; + } + final VMTemplateVO template = _templateDao.findById(vm.getTemplateId()); + if (template == null) { + String msg = "Template for the VM instance can not be found, VM instance configuration needs to be updated"; + s_logger.error(String.format("%s. Template ID: %d seems to be removed", msg, vm.getTemplateId())); + throw new CloudRuntimeException(msg); + } + final VMTemplateZoneVO templateZoneVO = templateZoneDao.findByZoneTemplate(vm.getDataCenterId(), template.getId()); + if (templateZoneVO == null) { + String msg = "Template for the VM instance can not be found in the zone ID: %s, VM instance configuration needs to be updated"; + s_logger.error(String.format("%s. %s", msg, template)); + throw new CloudRuntimeException(msg); + } + } + @Override public void orchestrateStart(final String vmUuid, final Map params, final DeploymentPlan planToDeploy, final DeploymentPlanner planner) throws InsufficientCapacityException, ConcurrentOperationException, ResourceUnavailableException { @@ -1101,6 +1124,8 @@ public void orchestrateStart(final String vmUuid, final Map()); - Mockito.when(vmInstanceMock.getId()).thenReturn(vmInstanceVoMockId); - Mockito.when(vmInstanceMock.getServiceOfferingId()).thenReturn(2L); - Mockito.when(hostMock.getId()).thenReturn(hostMockId); - Mockito.when(dataCenterDeploymentMock.getHostId()).thenReturn(hostMockId); - Mockito.when(dataCenterDeploymentMock.getClusterId()).thenReturn(clusterMockId); + when(vmInstanceMock.getId()).thenReturn(vmInstanceVoMockId); + when(vmInstanceMock.getServiceOfferingId()).thenReturn(2L); + when(hostMock.getId()).thenReturn(hostMockId); + when(dataCenterDeploymentMock.getHostId()).thenReturn(hostMockId); + when(dataCenterDeploymentMock.getClusterId()).thenReturn(clusterMockId); - Mockito.when(hostMock.getDataCenterId()).thenReturn(zoneMockId); - Mockito.when(hostDaoMock.findById(any())).thenReturn(hostMock); + when(hostMock.getDataCenterId()).thenReturn(zoneMockId); + when(hostDaoMock.findById(any())).thenReturn(hostMock); - Mockito.when(userVmJoinDaoMock.searchByIds(any())).thenReturn(new ArrayList<>()); - Mockito.when(userVmDaoMock.findById(any())).thenReturn(userVmMock); - Mockito.when(vmDaoMock.findById(any())).thenReturn(vmInstanceMock); + when(userVmJoinDaoMock.searchByIds(any())).thenReturn(new ArrayList<>()); + when(userVmDaoMock.findById(any())).thenReturn(userVmMock); + when(vmDaoMock.findById(any())).thenReturn(vmInstanceMock); Mockito.doReturn(vmInstanceVoMockId).when(virtualMachineProfileMock).getId(); @@ -178,8 +192,8 @@ public void setup() { @Test public void testaddHostIpToCertDetailsIfConfigAllows() { - Host vmHost = Mockito.mock(Host.class); - ConfigKey testConfig = Mockito.mock(ConfigKey.class); + Host vmHost = mock(Host.class); + ConfigKey testConfig = mock(ConfigKey.class); Long dataCenterId = 5L; String hostIp = "1.1.1.1"; @@ -187,9 +201,9 @@ public void testaddHostIpToCertDetailsIfConfigAllows() { Map ipAddresses = new HashMap<>(); ipAddresses.put(NetworkElementCommand.ROUTER_IP, routerIp); - Mockito.when(testConfig.valueIn(dataCenterId)).thenReturn(true); - Mockito.when(vmHost.getDataCenterId()).thenReturn(dataCenterId); - Mockito.when(vmHost.getPrivateIpAddress()).thenReturn(hostIp); + when(testConfig.valueIn(dataCenterId)).thenReturn(true); + when(vmHost.getDataCenterId()).thenReturn(dataCenterId); + when(vmHost.getPrivateIpAddress()).thenReturn(hostIp); virtualMachineManagerImpl.addHostIpToCertDetailsIfConfigAllows(vmHost, ipAddresses, testConfig); assertTrue(ipAddresses.containsKey(NetworkElementCommand.HYPERVISOR_HOST_PRIVATE_IP)); @@ -200,16 +214,17 @@ public void testaddHostIpToCertDetailsIfConfigAllows() { @Test public void testaddHostIpToCertDetailsIfConfigAllowsWhenConfigFalse() { - Host vmHost = Mockito.mock(Host.class); - ConfigKey testConfig = Mockito.mock(ConfigKey.class); + Host vmHost = mock(Host.class); + ConfigKey testConfig = mock(ConfigKey.class); Long dataCenterId = 5L; + String hostIp = "1.1.1.1"; String routerIp = "2.2.2.2"; Map ipAddresses = new HashMap<>(); ipAddresses.put(NetworkElementCommand.ROUTER_IP, routerIp); - Mockito.when(testConfig.valueIn(dataCenterId)).thenReturn(false); - Mockito.when(vmHost.getDataCenterId()).thenReturn(dataCenterId); + when(testConfig.valueIn(dataCenterId)).thenReturn(false); + when(vmHost.getDataCenterId()).thenReturn(dataCenterId); virtualMachineManagerImpl.addHostIpToCertDetailsIfConfigAllows(vmHost, ipAddresses, testConfig); assertFalse(ipAddresses.containsKey(NetworkElementCommand.HYPERVISOR_HOST_PRIVATE_IP)); @@ -220,18 +235,18 @@ public void testaddHostIpToCertDetailsIfConfigAllowsWhenConfigFalse() { @Test(expected = CloudRuntimeException.class) public void testScaleVM3() throws Exception { DeploymentPlanner.ExcludeList excludeHostList = new DeploymentPlanner.ExcludeList(); - virtualMachineManagerImpl.findHostAndMigrate(vmInstanceMock.getUuid(), 2L, null, excludeHostList); + virtualMachineManagerImpl.findHostAndMigrate(vmInstanceMock.getUuid(), 2l, null, excludeHostList); } @Test public void testSendStopWithOkAnswer() throws Exception { - VirtualMachineGuru guru = Mockito.mock(VirtualMachineGuru.class); - VirtualMachine vm = Mockito.mock(VirtualMachine.class); - VirtualMachineProfile profile = Mockito.mock(VirtualMachineProfile.class); + VirtualMachineGuru guru = mock(VirtualMachineGuru.class); + VirtualMachine vm = mock(VirtualMachine.class); + VirtualMachineProfile profile = mock(VirtualMachineProfile.class); StopAnswer answer = new StopAnswer(new StopCommand(vm, false, false), "ok", true); - Mockito.when(profile.getVirtualMachine()).thenReturn(vm); - Mockito.when(vm.getHostId()).thenReturn(1L); - Mockito.when(agentManagerMock.send(anyLong(), (Command)any())).thenReturn(answer); + when(profile.getVirtualMachine()).thenReturn(vm); + when(vm.getHostId()).thenReturn(1L); + when(agentManagerMock.send(anyLong(), (Command)any())).thenReturn(answer); boolean actual = virtualMachineManagerImpl.sendStop(guru, profile, false, false); @@ -240,13 +255,13 @@ public void testSendStopWithOkAnswer() throws Exception { @Test public void testSendStopWithFailAnswer() throws Exception { - VirtualMachineGuru guru = Mockito.mock(VirtualMachineGuru.class); - VirtualMachine vm = Mockito.mock(VirtualMachine.class); - VirtualMachineProfile profile = Mockito.mock(VirtualMachineProfile.class); + VirtualMachineGuru guru = mock(VirtualMachineGuru.class); + VirtualMachine vm = mock(VirtualMachine.class); + VirtualMachineProfile profile = mock(VirtualMachineProfile.class); StopAnswer answer = new StopAnswer(new StopCommand(vm, false, false), "fail", false); - Mockito.when(profile.getVirtualMachine()).thenReturn(vm); - Mockito.when(vm.getHostId()).thenReturn(1L); - Mockito.when(agentManagerMock.send(anyLong(), (Command)any())).thenReturn(answer); + when(profile.getVirtualMachine()).thenReturn(vm); + when(vm.getHostId()).thenReturn(1L); + when(agentManagerMock.send(anyLong(), (Command)any())).thenReturn(answer); boolean actual = virtualMachineManagerImpl.sendStop(guru, profile, false, false); @@ -255,12 +270,12 @@ public void testSendStopWithFailAnswer() throws Exception { @Test public void testSendStopWithNullAnswer() throws Exception { - VirtualMachineGuru guru = Mockito.mock(VirtualMachineGuru.class); - VirtualMachine vm = Mockito.mock(VirtualMachine.class); - VirtualMachineProfile profile = Mockito.mock(VirtualMachineProfile.class); - Mockito.when(profile.getVirtualMachine()).thenReturn(vm); - Mockito.when(vm.getHostId()).thenReturn(1L); - Mockito.when(agentManagerMock.send(anyLong(), (Command)any())).thenReturn(null); + VirtualMachineGuru guru = mock(VirtualMachineGuru.class); + VirtualMachine vm = mock(VirtualMachine.class); + VirtualMachineProfile profile = mock(VirtualMachineProfile.class); + when(profile.getVirtualMachine()).thenReturn(vm); + when(vm.getHostId()).thenReturn(1L); + when(agentManagerMock.send(anyLong(), (Command)any())).thenReturn(null); boolean actual = virtualMachineManagerImpl.sendStop(guru, profile, false, false); @@ -269,9 +284,9 @@ public void testSendStopWithNullAnswer() throws Exception { @Test public void testExeceuteInSequence() { - assertFalse(virtualMachineManagerImpl.getExecuteInSequence(HypervisorType.XenServer)); - assertFalse(virtualMachineManagerImpl.getExecuteInSequence(HypervisorType.KVM)); - assertEquals(virtualMachineManagerImpl.getExecuteInSequence(HypervisorType.Ovm3), VirtualMachineManager.ExecuteInSequence.value()); + assertTrue(virtualMachineManagerImpl.getExecuteInSequence(HypervisorType.XenServer) == false); + assertTrue(virtualMachineManagerImpl.getExecuteInSequence(HypervisorType.KVM) == false); + assertTrue(virtualMachineManagerImpl.getExecuteInSequence(HypervisorType.Ovm3) == VirtualMachineManager.ExecuteInSequence.value()); } private void overrideDefaultConfigValue(final ConfigKey configKey, final String value) throws IllegalAccessException, NoSuchFieldException { @@ -297,31 +312,31 @@ public void testExeceuteInSequenceVmware() throws IllegalAccessException, NoSuch } @Test - public void testCheckIfCanUpgrade() { - Mockito.when(vmInstanceMock.getState()).thenReturn(State.Stopped); - Mockito.when(serviceOfferingMock.isDynamic()).thenReturn(true); - Mockito.when(vmInstanceMock.getServiceOfferingId()).thenReturn(1L); + public void testCheckIfCanUpgrade() throws Exception { + when(vmInstanceMock.getState()).thenReturn(State.Stopped); + when(serviceOfferingMock.isDynamic()).thenReturn(true); + when(vmInstanceMock.getServiceOfferingId()).thenReturn(1l); - ServiceOfferingVO mockCurrentServiceOffering = Mockito.mock(ServiceOfferingVO.class); - DiskOfferingVO mockCurrentDiskOffering = Mockito.mock(DiskOfferingVO.class); + ServiceOfferingVO mockCurrentServiceOffering = mock(ServiceOfferingVO.class); + DiskOfferingVO mockCurrentDiskOffering = mock(DiskOfferingVO.class); - Mockito.when(serviceOfferingDaoMock.findByIdIncludingRemoved(anyLong(), anyLong())).thenReturn(mockCurrentServiceOffering); - Mockito.when(diskOfferingDaoMock.findByIdIncludingRemoved(anyLong())).thenReturn(mockCurrentDiskOffering); - Mockito.when(diskOfferingDaoMock.findById(anyLong())).thenReturn(diskOfferingMock); - Mockito.when(diskOfferingMock.isUseLocalStorage()).thenReturn(false); - Mockito.when(mockCurrentServiceOffering.isSystemUse()).thenReturn(true); - Mockito.when(serviceOfferingMock.isSystemUse()).thenReturn(true); + when(serviceOfferingDaoMock.findByIdIncludingRemoved(anyLong(), anyLong())).thenReturn(mockCurrentServiceOffering); + when(diskOfferingDaoMock.findByIdIncludingRemoved(anyLong())).thenReturn(mockCurrentDiskOffering); + when(diskOfferingDaoMock.findById(anyLong())).thenReturn(diskOfferingMock); + when(diskOfferingMock.isUseLocalStorage()).thenReturn(false); + when(mockCurrentServiceOffering.isSystemUse()).thenReturn(true); + when(serviceOfferingMock.isSystemUse()).thenReturn(true); String[] oldDOStorageTags = {"x","y"}; String[] newDOStorageTags = {"z","x","y"}; - Mockito.when(mockCurrentDiskOffering.getTagsArray()).thenReturn(oldDOStorageTags); - Mockito.when(diskOfferingMock.getTagsArray()).thenReturn(newDOStorageTags); + when(mockCurrentDiskOffering.getTagsArray()).thenReturn(oldDOStorageTags); + when(diskOfferingMock.getTagsArray()).thenReturn(newDOStorageTags); virtualMachineManagerImpl.checkIfCanUpgrade(vmInstanceMock, serviceOfferingMock); } @Test(expected = InvalidParameterValueException.class) public void testCheckIfCanUpgradeFail() { - Mockito.when(serviceOfferingMock.getState()).thenReturn(ServiceOffering.State.Inactive); + when(serviceOfferingMock.getState()).thenReturn(ServiceOffering.State.Inactive); virtualMachineManagerImpl.checkIfCanUpgrade(vmInstanceMock, serviceOfferingMock); } @@ -390,7 +405,7 @@ public void buildMapUsingUserInformationTestUserDefinedMigrationMapEmpty() { Assert.assertTrue(volumeToPoolObjectMap.isEmpty()); - Mockito.verify(userDefinedVolumeToStoragePoolMap, Mockito.times(0)).keySet(); + Mockito.verify(userDefinedVolumeToStoragePoolMap, times(0)).keySet(); } @Test(expected = CloudRuntimeException.class) @@ -398,7 +413,7 @@ public void buildMapUsingUserInformationTestTargetHostDoesNotHaveAccessToPool() HashMap userDefinedVolumeToStoragePoolMap = new HashMap<>(); userDefinedVolumeToStoragePoolMap.put(volumeMockId, storagePoolVoMockId); - Mockito.doNothing().when(virtualMachineManagerImpl).executeManagedStorageChecksWhenTargetStoragePoolProvided(any(StoragePoolVO.class), any(VolumeVO.class), any(StoragePoolVO.class)); + Mockito.doNothing().when(virtualMachineManagerImpl).executeManagedStorageChecksWhenTargetStoragePoolProvided(Mockito.any(StoragePoolVO.class), Mockito.any(VolumeVO.class), Mockito.any(StoragePoolVO.class)); Mockito.doReturn(null).when(storagePoolHostDaoMock).findByPoolHost(storagePoolVoMockId, hostMockId); virtualMachineManagerImpl.buildMapUsingUserInformation(virtualMachineProfileMock, hostMock, userDefinedVolumeToStoragePoolMap); @@ -410,8 +425,8 @@ public void buildMapUsingUserInformationTestTargetHostHasAccessToPool() { HashMap userDefinedVolumeToStoragePoolMap = Mockito.spy(new HashMap<>()); userDefinedVolumeToStoragePoolMap.put(volumeMockId, storagePoolVoMockId); - Mockito.doNothing().when(virtualMachineManagerImpl).executeManagedStorageChecksWhenTargetStoragePoolProvided(any(StoragePoolVO.class), any(VolumeVO.class), - any(StoragePoolVO.class)); + Mockito.doNothing().when(virtualMachineManagerImpl).executeManagedStorageChecksWhenTargetStoragePoolProvided(Mockito.any(StoragePoolVO.class), Mockito.any(VolumeVO.class), + Mockito.any(StoragePoolVO.class)); Mockito.doReturn(Mockito.mock(StoragePoolHostVO.class)).when(storagePoolHostDaoMock).findByPoolHost(storagePoolVoMockId, hostMockId); Map volumeToPoolObjectMap = virtualMachineManagerImpl.buildMapUsingUserInformation(virtualMachineProfileMock, hostMock, userDefinedVolumeToStoragePoolMap); @@ -419,7 +434,7 @@ public void buildMapUsingUserInformationTestTargetHostHasAccessToPool() { assertFalse(volumeToPoolObjectMap.isEmpty()); assertEquals(storagePoolVoMock, volumeToPoolObjectMap.get(volumeVoMock)); - Mockito.verify(userDefinedVolumeToStoragePoolMap, Mockito.times(1)).keySet(); + Mockito.verify(userDefinedVolumeToStoragePoolMap, times(1)).keySet(); } @Test @@ -447,7 +462,7 @@ public void executeManagedStorageChecksWhenTargetStoragePoolNotProvidedTestCurre virtualMachineManagerImpl.executeManagedStorageChecksWhenTargetStoragePoolNotProvided(hostMock, storagePoolVoMock, volumeVoMock); Mockito.verify(storagePoolVoMock).isManaged(); - Mockito.verify(storagePoolHostDaoMock, Mockito.times(0)).findByPoolHost(anyLong(), anyLong()); + Mockito.verify(storagePoolHostDaoMock, Mockito.times(0)).findByPoolHost(Mockito.anyLong(), Mockito.anyLong()); } @Test @@ -471,15 +486,15 @@ public void executeManagedStorageChecksWhenTargetStoragePoolNotProvidedTestCurre @Test public void getCandidateStoragePoolsToMigrateLocalVolumeTestLocalVolume() { - Mockito.doReturn(Mockito.mock(DiskOfferingVO.class)).when(diskOfferingDaoMock).findById(anyLong()); + Mockito.doReturn(Mockito.mock(DiskOfferingVO.class)).when(diskOfferingDaoMock).findById(Mockito.anyLong()); Mockito.doReturn(true).when(storagePoolVoMock).isLocal(); List poolListMock = new ArrayList<>(); poolListMock.add(storagePoolVoMock); - Mockito.doReturn(poolListMock).when(storagePoolAllocatorMock).allocateToPool(any(DiskProfile.class), any(VirtualMachineProfile.class), any(DeploymentPlan.class), - any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); + Mockito.doReturn(poolListMock).when(storagePoolAllocatorMock).allocateToPool(Mockito.any(DiskProfile.class), Mockito.any(VirtualMachineProfile.class), Mockito.any(DeploymentPlan.class), + Mockito.any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); List poolList = virtualMachineManagerImpl.getCandidateStoragePoolsToMigrateLocalVolume(virtualMachineProfileMock, dataCenterDeploymentMock, volumeVoMock); @@ -489,15 +504,15 @@ public void getCandidateStoragePoolsToMigrateLocalVolumeTestLocalVolume() { @Test public void getCandidateStoragePoolsToMigrateLocalVolumeTestCrossClusterMigration() { - Mockito.doReturn(Mockito.mock(DiskOfferingVO.class)).when(diskOfferingDaoMock).findById(anyLong()); + Mockito.doReturn(Mockito.mock(DiskOfferingVO.class)).when(diskOfferingDaoMock).findById(Mockito.anyLong()); Mockito.doReturn(false).when(storagePoolVoMock).isLocal(); List poolListMock = new ArrayList<>(); poolListMock.add(storagePoolVoMock); - Mockito.doReturn(poolListMock).when(storagePoolAllocatorMock).allocateToPool(any(DiskProfile.class), any(VirtualMachineProfile.class), any(DeploymentPlan.class), - any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); + Mockito.doReturn(poolListMock).when(storagePoolAllocatorMock).allocateToPool(Mockito.any(DiskProfile.class), Mockito.any(VirtualMachineProfile.class), Mockito.any(DeploymentPlan.class), + Mockito.any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); Mockito.doReturn(true).when(virtualMachineManagerImpl).isStorageCrossClusterMigration(clusterMockId, storagePoolVoMock); List poolList = virtualMachineManagerImpl.getCandidateStoragePoolsToMigrateLocalVolume(virtualMachineProfileMock, dataCenterDeploymentMock, volumeVoMock); @@ -508,15 +523,15 @@ public void getCandidateStoragePoolsToMigrateLocalVolumeTestCrossClusterMigratio @Test public void getCandidateStoragePoolsToMigrateLocalVolumeTestWithinClusterMigration() { - Mockito.doReturn(Mockito.mock(DiskOfferingVO.class)).when(diskOfferingDaoMock).findById(anyLong()); + Mockito.doReturn(Mockito.mock(DiskOfferingVO.class)).when(diskOfferingDaoMock).findById(Mockito.anyLong()); Mockito.doReturn(false).when(storagePoolVoMock).isLocal(); List poolListMock = new ArrayList<>(); poolListMock.add(storagePoolVoMock); - Mockito.doReturn(poolListMock).when(storagePoolAllocatorMock).allocateToPool(any(DiskProfile.class), any(VirtualMachineProfile.class), any(DeploymentPlan.class), - any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); + Mockito.doReturn(poolListMock).when(storagePoolAllocatorMock).allocateToPool(Mockito.any(DiskProfile.class), Mockito.any(VirtualMachineProfile.class), Mockito.any(DeploymentPlan.class), + Mockito.any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); Mockito.doReturn(false).when(virtualMachineManagerImpl).isStorageCrossClusterMigration(clusterMockId, storagePoolVoMock); List poolList = virtualMachineManagerImpl.getCandidateStoragePoolsToMigrateLocalVolume(virtualMachineProfileMock, dataCenterDeploymentMock, volumeVoMock); @@ -536,33 +551,33 @@ public void getCandidateStoragePoolsToMigrateLocalVolumeTestMoreThanOneAllocator virtualMachineManagerImpl.setStoragePoolAllocators(storagePoolAllocatorsMock); - Mockito.doReturn(Mockito.mock(DiskOfferingVO.class)).when(diskOfferingDaoMock).findById(anyLong()); + Mockito.doReturn(Mockito.mock(DiskOfferingVO.class)).when(diskOfferingDaoMock).findById(Mockito.anyLong()); Mockito.doReturn(false).when(storagePoolVoMock).isLocal(); List poolListMock = new ArrayList<>(); poolListMock.add(storagePoolVoMock); - Mockito.doReturn(poolListMock).when(storagePoolAllocatorMock).allocateToPool(any(DiskProfile.class), any(VirtualMachineProfile.class), any(DeploymentPlan.class), - any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); + Mockito.doReturn(poolListMock).when(storagePoolAllocatorMock).allocateToPool(Mockito.any(DiskProfile.class), Mockito.any(VirtualMachineProfile.class), Mockito.any(DeploymentPlan.class), + Mockito.any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); - Mockito.doReturn(null).when(storagePoolAllocatorMock2).allocateToPool(any(DiskProfile.class), any(VirtualMachineProfile.class), any(DeploymentPlan.class), - any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); + Mockito.doReturn(null).when(storagePoolAllocatorMock2).allocateToPool(Mockito.any(DiskProfile.class), Mockito.any(VirtualMachineProfile.class), Mockito.any(DeploymentPlan.class), + Mockito.any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); - Mockito.doReturn(new ArrayList<>()).when(storagePoolAllocatorMock3).allocateToPool(any(DiskProfile.class), any(VirtualMachineProfile.class), any(DeploymentPlan.class), - any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); + Mockito.doReturn(new ArrayList<>()).when(storagePoolAllocatorMock3).allocateToPool(Mockito.any(DiskProfile.class), Mockito.any(VirtualMachineProfile.class), Mockito.any(DeploymentPlan.class), + Mockito.any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); Mockito.doReturn(false).when(virtualMachineManagerImpl).isStorageCrossClusterMigration(clusterMockId, storagePoolVoMock); List poolList = virtualMachineManagerImpl.getCandidateStoragePoolsToMigrateLocalVolume(virtualMachineProfileMock, dataCenterDeploymentMock, volumeVoMock); Assert.assertTrue(poolList.isEmpty()); - Mockito.verify(storagePoolAllocatorMock).allocateToPool(any(DiskProfile.class), any(VirtualMachineProfile.class), any(DeploymentPlan.class), - any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); - Mockito.verify(storagePoolAllocatorMock2).allocateToPool(any(DiskProfile.class), any(VirtualMachineProfile.class), any(DeploymentPlan.class), - any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); - Mockito.verify(storagePoolAllocatorMock3).allocateToPool(any(DiskProfile.class), any(VirtualMachineProfile.class), any(DeploymentPlan.class), - any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); + Mockito.verify(storagePoolAllocatorMock).allocateToPool(Mockito.any(DiskProfile.class), Mockito.any(VirtualMachineProfile.class), Mockito.any(DeploymentPlan.class), + Mockito.any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); + Mockito.verify(storagePoolAllocatorMock2).allocateToPool(Mockito.any(DiskProfile.class), Mockito.any(VirtualMachineProfile.class), Mockito.any(DeploymentPlan.class), + Mockito.any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); + Mockito.verify(storagePoolAllocatorMock3).allocateToPool(Mockito.any(DiskProfile.class), Mockito.any(VirtualMachineProfile.class), Mockito.any(DeploymentPlan.class), + Mockito.any(ExcludeList.class), Mockito.eq(StoragePoolAllocator.RETURN_UPTO_ALL)); } @Test(expected = CloudRuntimeException.class) @@ -651,8 +666,8 @@ public void createStoragePoolMappingsForVolumesTestNotCrossCluterMigrationWithCl HashMap volumeToPoolObjectMap = new HashMap<>(); Mockito.doReturn(ScopeType.CLUSTER).when(storagePoolVoMock).getScope(); - Mockito.doNothing().when(virtualMachineManagerImpl).executeManagedStorageChecksWhenTargetStoragePoolNotProvided(any(), any(), any()); - Mockito.doReturn(false).when(virtualMachineManagerImpl).isStorageCrossClusterMigration(anyLong(), any()); + Mockito.doNothing().when(virtualMachineManagerImpl).executeManagedStorageChecksWhenTargetStoragePoolNotProvided(Mockito.any(), Mockito.any(), Mockito.any()); + Mockito.doReturn(false).when(virtualMachineManagerImpl).isStorageCrossClusterMigration(Mockito.anyLong(), Mockito.any()); virtualMachineManagerImpl.createStoragePoolMappingsForVolumes(virtualMachineProfileMock, dataCenterDeploymentMock, volumeToPoolObjectMap, allVolumes); @@ -675,7 +690,7 @@ public void createMappingVolumeAndStoragePoolTest() { Mockito.doReturn(volumesNotMapped).when(virtualMachineManagerImpl).findVolumesThatWereNotMappedByTheUser(virtualMachineProfileMock, volumeToPoolObjectMap); Mockito.doNothing().when(virtualMachineManagerImpl).createStoragePoolMappingsForVolumes(Mockito.eq(virtualMachineProfileMock), - any(DataCenterDeployment.class), Mockito.eq(volumeToPoolObjectMap), Mockito.eq(volumesNotMapped)); + Mockito.any(DataCenterDeployment.class), Mockito.eq(volumeToPoolObjectMap), Mockito.eq(volumesNotMapped)); Map mappingVolumeAndStoragePool = virtualMachineManagerImpl.createMappingVolumeAndStoragePool(virtualMachineProfileMock, hostMock, new HashMap<>()); @@ -685,14 +700,14 @@ public void createMappingVolumeAndStoragePoolTest() { inOrder.verify(virtualMachineManagerImpl).buildMapUsingUserInformation(Mockito.eq(virtualMachineProfileMock), Mockito.eq(hostMock), Mockito.anyMapOf(Long.class, Long.class)); inOrder.verify(virtualMachineManagerImpl).findVolumesThatWereNotMappedByTheUser(virtualMachineProfileMock, volumeToPoolObjectMap); inOrder.verify(virtualMachineManagerImpl).createStoragePoolMappingsForVolumes(Mockito.eq(virtualMachineProfileMock), - any(DataCenterDeployment.class), Mockito.eq(volumeToPoolObjectMap), Mockito.eq(volumesNotMapped)); + Mockito.any(DataCenterDeployment.class), Mockito.eq(volumeToPoolObjectMap), Mockito.eq(volumesNotMapped)); } @Test public void matchesOfSorts() { List nothing = null; List empty = new ArrayList<>(); - List tag = List.of("bla"); + List tag = Arrays.asList("bla"); List tags = Arrays.asList("bla", "blob"); List others = Arrays.asList("bla", "blieb"); List three = Arrays.asList("bla", "blob", "blieb"); @@ -739,13 +754,13 @@ public void isRootVolumeOnLocalStorageTestZone() { private void prepareAndTestIsRootVolumeOnLocalStorage(ScopeType scope, boolean expected) { StoragePoolVO storagePoolVoMock = Mockito.mock(StoragePoolVO.class); - Mockito.doReturn(storagePoolVoMock).when(storagePoolDaoMock).findById(anyLong()); + Mockito.doReturn(storagePoolVoMock).when(storagePoolDaoMock).findById(Mockito.anyLong()); Mockito.doReturn(scope).when(storagePoolVoMock).getScope(); List mockedVolumes = new ArrayList<>(); mockedVolumes.add(volumeVoMock); - Mockito.doReturn(mockedVolumes).when(volumeDaoMock).findByInstanceAndType(anyLong(), any()); + Mockito.doReturn(mockedVolumes).when(volumeDaoMock).findByInstanceAndType(Mockito.anyLong(), Mockito.any()); - boolean result = virtualMachineManagerImpl.isRootVolumeOnLocalStorage(0L); + boolean result = virtualMachineManagerImpl.isRootVolumeOnLocalStorage(0l); assertEquals(expected, result); } @@ -771,12 +786,66 @@ public void checkIfNewOfferingStorageScopeMatchesStoragePoolTestSharedLocal() { } private void prepareAndRunCheckIfNewOfferingStorageScopeMatchesStoragePool(boolean isRootOnLocal, boolean isOfferingUsingLocal) { - Mockito.doReturn(isRootOnLocal).when(virtualMachineManagerImpl).isRootVolumeOnLocalStorage(anyLong()); + Mockito.doReturn(isRootOnLocal).when(virtualMachineManagerImpl).isRootVolumeOnLocalStorage(Mockito.anyLong()); Mockito.doReturn("vmInstanceMockedToString").when(vmInstanceMock).toString(); Mockito.doReturn(isOfferingUsingLocal).when(diskOfferingMock).isUseLocalStorage(); virtualMachineManagerImpl.checkIfNewOfferingStorageScopeMatchesStoragePool(vmInstanceMock, diskOfferingMock); } + @Test + public void checkIfTemplateNeededForCreatingVmVolumesExistingRootVolumes() { + long vmId = 1L; + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(vmId); + Mockito.when(volumeDaoMock.findReadyRootVolumesByInstance(vmId)).thenReturn(List.of(Mockito.mock(VolumeVO.class))); + virtualMachineManagerImpl.checkIfTemplateNeededForCreatingVmVolumes(vm); + } + + @Test(expected = CloudRuntimeException.class) + public void checkIfTemplateNeededForCreatingVmVolumesMissingTemplate() { + long vmId = 1L; + long templateId = 1L; + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(vmId); + Mockito.when(vm.getTemplateId()).thenReturn(templateId); + Mockito.when(volumeDaoMock.findReadyRootVolumesByInstance(vmId)).thenReturn(null); + Mockito.when(templateDao.findById(templateId)).thenReturn(null); + virtualMachineManagerImpl.checkIfTemplateNeededForCreatingVmVolumes(vm); + } + + @Test(expected = CloudRuntimeException.class) + public void checkIfTemplateNeededForCreatingVmVolumesMissingZoneTemplate() { + long vmId = 1L; + long templateId = 1L; + long dcId = 1L; + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(vmId); + Mockito.when(vm.getTemplateId()).thenReturn(templateId); + Mockito.when(vm.getDataCenterId()).thenReturn(dcId); + Mockito.when(volumeDaoMock.findReadyRootVolumesByInstance(vmId)).thenReturn(null); + VMTemplateVO template = Mockito.mock(VMTemplateVO.class); + Mockito.when(vm.getId()).thenReturn(templateId); + Mockito.when(templateDao.findById(templateId)).thenReturn(template); + virtualMachineManagerImpl.checkIfTemplateNeededForCreatingVmVolumes(vm); + } + + @Test + public void checkIfTemplateNeededForCreatingVmVolumesTemplateAvailable() { + long vmId = 1L; + long templateId = 1L; + long dcId = 1L; + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(vmId); + Mockito.when(vm.getTemplateId()).thenReturn(templateId); + Mockito.when(vm.getDataCenterId()).thenReturn(dcId); + Mockito.when(volumeDaoMock.findReadyRootVolumesByInstance(vmId)).thenReturn(new ArrayList<>()); + VMTemplateVO template = Mockito.mock(VMTemplateVO.class); + Mockito.when(template.getId()).thenReturn(templateId); + Mockito.when(templateDao.findById(templateId)).thenReturn(template); + Mockito.when(templateZoneDao.findByZoneTemplate(dcId, templateId)).thenReturn(Mockito.mock(VMTemplateZoneVO.class)); + virtualMachineManagerImpl.checkIfTemplateNeededForCreatingVmVolumes(vm); + } + @Test public void shouldIncrementVrResourceCountReturnWhenConfigResourceCountRunningVMsonlyIsEnabled() throws NoSuchFieldException, IllegalAccessException { ServiceOffering offering = Mockito.mock(ServiceOffering.class); diff --git a/tools/marvin/setup.py b/tools/marvin/setup.py index 59dbfd2b52a2..33f22ebea6b2 100644 --- a/tools/marvin/setup.py +++ b/tools/marvin/setup.py @@ -27,7 +27,7 @@ raise RuntimeError("python setuptools is required to build Marvin") -VERSION = "4.18.0.0-SNAPSHOT" +VERSION = "4.18.0.0" setup(name="Marvin", version=VERSION, From 0f1b391747d711961e281e23735cc94b73c5641e Mon Sep 17 00:00:00 2001 From: Chales Queiroz Date: Fri, 14 Apr 2023 07:12:38 +0100 Subject: [PATCH 9/9] Fixing conflicts and tests --- .../com/cloud/user/ResourceLimitService.java | 7 +++ .../cloud/vm/VirtualMachineManagerImpl.java | 25 +--------- .../vm/VirtualMachineManagerImplTest.java | 47 ++++++++----------- .../ResourceLimitManagerImpl.java | 44 +++++++++-------- .../vpc/MockResourceLimitManagerImpl.java | 7 ++- 5 files changed, 59 insertions(+), 71 deletions(-) diff --git a/api/src/main/java/com/cloud/user/ResourceLimitService.java b/api/src/main/java/com/cloud/user/ResourceLimitService.java index 41b2c8135cf0..249748632921 100644 --- a/api/src/main/java/com/cloud/user/ResourceLimitService.java +++ b/api/src/main/java/com/cloud/user/ResourceLimitService.java @@ -23,6 +23,7 @@ import com.cloud.configuration.ResourceLimit; import com.cloud.domain.Domain; import com.cloud.exception.ResourceAllocationException; +import com.cloud.offering.ServiceOffering; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.user.ResourceReservation; @@ -210,4 +211,10 @@ public interface ResourceLimitService { */ ResourceReservation getReservation(Account account, Boolean displayResource, ResourceType type, Long delta) throws ResourceAllocationException; + /** + * Returns the service offering by the given configuration. + * + * @return the service offering found or null if not found + */ + ServiceOffering getServiceOfferingByConfig(); } diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index a2f0bda58432..ba9f25c40e04 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -47,7 +47,6 @@ import javax.persistence.EntityExistsException; import com.cloud.exception.ResourceAllocationException; -import com.cloud.network.router.VirtualNetworkApplianceManager; import org.apache.cloudstack.affinity.dao.AffinityGroupVMMapDao; import org.apache.cloudstack.annotation.AnnotationService; import org.apache.cloudstack.annotation.dao.AnnotationDao; @@ -1465,26 +1464,6 @@ private void logBootModeParameters(Map para } } - /** - * Returns the service offering by the given configuration. - * - * @return the service offering found or null if not found - */ - public ServiceOffering getServiceOfferingByConfig() { - ServiceOffering defaultRouterOffering = null; - final String globalRouterOffering = VirtualNetworkApplianceManager.VirtualRouterServiceOffering.value(); - - if (globalRouterOffering != null) { - defaultRouterOffering = _serviceOfferingDao.findByUuid(globalRouterOffering); - } - - if (defaultRouterOffering == null) { - defaultRouterOffering = _serviceOfferingDao.findByName(ServiceOffering.routerDefaultOffUniqueName); - } - - return defaultRouterOffering; - } - /** * Counts VR resources for the domain if global setting is true. * If the value is "all", counts all VR resources, otherwise get the difference between @@ -1581,7 +1560,7 @@ public void incrementVrResourceCount(ServiceOffering offering, Account owner, bo return; } // - final ServiceOffering defaultRouterOffering = getServiceOfferingByConfig(); + final ServiceOffering defaultRouterOffering = _resourceLimitMgr.getServiceOfferingByConfig(); final Pair cpuMemoryCount = resolveCpuAndMemoryCount(offering, defaultRouterOffering, owner); calculateResourceCount(cpuMemoryCount, owner, true); } @@ -1599,7 +1578,7 @@ public void decrementVrResourceCount(ServiceOffering offering, Account owner, bo return; } - final ServiceOffering defaultRouterOffering = getServiceOfferingByConfig(); + final ServiceOffering defaultRouterOffering = _resourceLimitMgr.getServiceOfferingByConfig(); final Pair cpuMemoryCount = resolveCpuAndMemoryCount(offering, defaultRouterOffering, owner); calculateResourceCount(cpuMemoryCount, owner, false); } diff --git a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java index 2bfbda1f16c1..0071adb05a2b 100644 --- a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java +++ b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java @@ -35,7 +35,9 @@ import java.util.Map; import com.cloud.user.Account; +import com.cloud.user.ResourceLimitService; import com.cloud.utils.Pair; +import com.cloud.vm.dao.VMInstanceDao; import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; @@ -49,7 +51,7 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.Spy; -import org.mockito.runners.MockitoJUnitRunner; +import org.mockito.junit.MockitoJUnitRunner; import com.cloud.agent.AgentManager; import com.cloud.agent.api.Command; @@ -86,7 +88,6 @@ import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.VirtualMachine.State; import com.cloud.vm.dao.UserVmDao; -import com.cloud.vm.dao.VMInstanceDao; @RunWith(MockitoJUnitRunner.class) public class VirtualMachineManagerImplTest { @@ -97,8 +98,6 @@ public class VirtualMachineManagerImplTest { @Mock private AgentManager agentManagerMock; @Mock - private VMInstanceDao vmInstanceDaoMock; - @Mock private ServiceOfferingDao serviceOfferingDaoMock; @Mock private VolumeDao volumeDaoMock; @@ -106,17 +105,15 @@ public class VirtualMachineManagerImplTest { private PrimaryDataStoreDao storagePoolDaoMock; @Mock private VMInstanceVO vmInstanceMock; - private long vmInstanceVoMockId = 1L; - + private final long vmInstanceVoMockId = 1L; @Mock private ServiceOfferingVO serviceOfferingMock; - @Mock private DiskOfferingVO diskOfferingMock; - private long hostMockId = 1L; - private long clusterMockId = 2L; - private long zoneMockId = 3L; + private final long hostMockId = 1L; + private final long clusterMockId = 2L; + private final long zoneMockId = 3L; @Mock private HostVO hostMock; @Mock @@ -126,12 +123,11 @@ public class VirtualMachineManagerImplTest { private VirtualMachineProfile virtualMachineProfileMock; @Mock private StoragePoolVO storagePoolVoMock; - private long storagePoolVoMockId = 11L; - private long storagePoolVoMockClusterId = 234L; + private final long storagePoolVoMockId = 11L; @Mock private VolumeVO volumeVoMock; - private long volumeMockId = 1111L; + private final long volumeMockId = 1111L; @Mock private StoragePoolHostDao storagePoolHostDaoMock; @@ -154,9 +150,10 @@ public class VirtualMachineManagerImplTest { private UserVmDao userVmDaoMock; @Mock private UserVmVO userVmMock; - @Mock private VMInstanceDao vmDaoMock; + @Mock + private ResourceLimitService resourceLimitServiceMock; @Before public void setup() { @@ -174,6 +171,7 @@ public void setup() { when(userVmJoinDaoMock.searchByIds(any())).thenReturn(new ArrayList<>()); when(userVmDaoMock.findById(any())).thenReturn(userVmMock); when(vmDaoMock.findById(any())).thenReturn(vmInstanceMock); + when(resourceLimitServiceMock.getServiceOfferingByConfig()).thenReturn(serviceOfferingMock); Mockito.doReturn(vmInstanceVoMockId).when(virtualMachineProfileMock).getId(); @@ -218,7 +216,6 @@ public void testaddHostIpToCertDetailsIfConfigAllowsWhenConfigFalse() { ConfigKey testConfig = mock(ConfigKey.class); Long dataCenterId = 5L; - String hostIp = "1.1.1.1"; String routerIp = "2.2.2.2"; Map ipAddresses = new HashMap<>(); ipAddresses.put(NetworkElementCommand.ROUTER_IP, routerIp); @@ -235,7 +232,7 @@ public void testaddHostIpToCertDetailsIfConfigAllowsWhenConfigFalse() { @Test(expected = CloudRuntimeException.class) public void testScaleVM3() throws Exception { DeploymentPlanner.ExcludeList excludeHostList = new DeploymentPlanner.ExcludeList(); - virtualMachineManagerImpl.findHostAndMigrate(vmInstanceMock.getUuid(), 2l, null, excludeHostList); + virtualMachineManagerImpl.findHostAndMigrate(vmInstanceMock.getUuid(), 2L, null, excludeHostList); } @Test @@ -284,9 +281,9 @@ public void testSendStopWithNullAnswer() throws Exception { @Test public void testExeceuteInSequence() { - assertTrue(virtualMachineManagerImpl.getExecuteInSequence(HypervisorType.XenServer) == false); - assertTrue(virtualMachineManagerImpl.getExecuteInSequence(HypervisorType.KVM) == false); - assertTrue(virtualMachineManagerImpl.getExecuteInSequence(HypervisorType.Ovm3) == VirtualMachineManager.ExecuteInSequence.value()); + assertFalse(virtualMachineManagerImpl.getExecuteInSequence(HypervisorType.XenServer)); + assertFalse(virtualMachineManagerImpl.getExecuteInSequence(HypervisorType.KVM)); + assertEquals(virtualMachineManagerImpl.getExecuteInSequence(HypervisorType.Ovm3), VirtualMachineManager.ExecuteInSequence.value()); } private void overrideDefaultConfigValue(final ConfigKey configKey, final String value) throws IllegalAccessException, NoSuchFieldException { @@ -312,10 +309,10 @@ public void testExeceuteInSequenceVmware() throws IllegalAccessException, NoSuch } @Test - public void testCheckIfCanUpgrade() throws Exception { + public void testCheckIfCanUpgrade() { when(vmInstanceMock.getState()).thenReturn(State.Stopped); when(serviceOfferingMock.isDynamic()).thenReturn(true); - when(vmInstanceMock.getServiceOfferingId()).thenReturn(1l); + when(vmInstanceMock.getServiceOfferingId()).thenReturn(1L); ServiceOfferingVO mockCurrentServiceOffering = mock(ServiceOfferingVO.class); DiskOfferingVO mockCurrentDiskOffering = mock(DiskOfferingVO.class); @@ -760,7 +757,7 @@ private void prepareAndTestIsRootVolumeOnLocalStorage(ScopeType scope, boolean e mockedVolumes.add(volumeVoMock); Mockito.doReturn(mockedVolumes).when(volumeDaoMock).findByInstanceAndType(Mockito.anyLong(), Mockito.any()); - boolean result = virtualMachineManagerImpl.isRootVolumeOnLocalStorage(0l); + boolean result = virtualMachineManagerImpl.isRootVolumeOnLocalStorage(0L); assertEquals(expected, result); } @@ -854,7 +851,6 @@ public void shouldIncrementVrResourceCountReturnWhenConfigResourceCountRunningVM overrideDefaultConfigValue(ResourceCountRunningVMsonly, "true"); virtualMachineManagerImpl.incrementVrResourceCount(offering, owner, isDeployOrDestroy); - Mockito.verify(virtualMachineManagerImpl, Mockito.times(0)).getServiceOfferingByConfig(); Mockito.verify(virtualMachineManagerImpl, Mockito.times(0)).resolveCpuAndMemoryCount(any(), any(), any()); Mockito.verify(virtualMachineManagerImpl, Mockito.times(0)).calculateResourceCount(any(), any(), Mockito.anyBoolean()); } @@ -865,12 +861,10 @@ public void shouldIncrementVrResourceCountContinueWhenConfigResourceCountRunning Account owner = Mockito.mock(Account.class); boolean isDeployOrDestroy = true; overrideDefaultConfigValue(ResourceCountRunningVMsonly, "false"); - Mockito.doReturn(serviceOfferingMock).when(virtualMachineManagerImpl).getServiceOfferingByConfig(); Mockito.doReturn(Pair.of("", "")).when(virtualMachineManagerImpl).resolveCpuAndMemoryCount(any(), any(), any()); Mockito.doNothing().when(virtualMachineManagerImpl).calculateResourceCount(any(), any(), Mockito.anyBoolean()); virtualMachineManagerImpl.incrementVrResourceCount(offering, owner, isDeployOrDestroy); - Mockito.verify(virtualMachineManagerImpl, Mockito.times(1)).getServiceOfferingByConfig(); Mockito.verify(virtualMachineManagerImpl, Mockito.times(1)).resolveCpuAndMemoryCount(any(), any(), any()); Mockito.verify(virtualMachineManagerImpl, Mockito.times(1)).calculateResourceCount(any(), any(), Mockito.anyBoolean()); } @@ -883,7 +877,6 @@ public void shouldDecrementVrResourceCountReturnWhenConfigResourceCountRunningVM overrideDefaultConfigValue(ResourceCountRunningVMsonly, "true"); virtualMachineManagerImpl.decrementVrResourceCount(offering, owner, isDeployOrDestroy); - Mockito.verify(virtualMachineManagerImpl, Mockito.times(0)).getServiceOfferingByConfig(); Mockito.verify(virtualMachineManagerImpl, Mockito.times(0)).resolveCpuAndMemoryCount(any(), any(), any()); Mockito.verify(virtualMachineManagerImpl, Mockito.times(0)).calculateResourceCount(any(), any(), Mockito.anyBoolean()); } @@ -894,12 +887,10 @@ public void shouldDecrementVrResourceCountContinueWhenConfigResourceCountRunning Account owner = Mockito.mock(Account.class); boolean isDeployOrDestroy = true; overrideDefaultConfigValue(ResourceCountRunningVMsonly, "false"); - Mockito.doReturn(serviceOfferingMock).when(virtualMachineManagerImpl).getServiceOfferingByConfig(); Mockito.doReturn(Pair.of("", "")).when(virtualMachineManagerImpl).resolveCpuAndMemoryCount(any(), any(), any()); Mockito.doNothing().when(virtualMachineManagerImpl).calculateResourceCount(any(), any(), Mockito.anyBoolean()); virtualMachineManagerImpl.incrementVrResourceCount(offering, owner, isDeployOrDestroy); - Mockito.verify(virtualMachineManagerImpl, Mockito.times(1)).getServiceOfferingByConfig(); Mockito.verify(virtualMachineManagerImpl, Mockito.times(1)).resolveCpuAndMemoryCount(any(), any(), any()); Mockito.verify(virtualMachineManagerImpl, Mockito.times(1)).calculateResourceCount(any(), any(), Mockito.anyBoolean()); } diff --git a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java index 3741fd23e50d..f9c930e6b27b 100644 --- a/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java +++ b/server/src/main/java/com/cloud/resourcelimit/ResourceLimitManagerImpl.java @@ -1012,15 +1012,7 @@ private Pair calculateCpuTotalByAccountId(long accountId) { boolean hasCpus = false; final Account owner = _accountDao.findById(accountId); final DomainVO domain = _domainDao.findById(owner.getDomainId()); - - ServiceOffering defaultRouterOffering = null; - String globalRouterOffering = VirtualNetworkApplianceManager.VirtualRouterServiceOffering.value(); - if (globalRouterOffering != null) { - defaultRouterOffering = serviceOfferingDao.findByUuid(globalRouterOffering); - } - if (defaultRouterOffering == null) { - defaultRouterOffering = serviceOfferingDao.findByName(ServiceOffering.routerDefaultOffUniqueName); - } + final ServiceOffering defaultRouterOffering = getServiceOfferingByConfig(); GenericSearchBuilder cpuSearch = serviceOfferingDao.createSearchBuilder(SumCount.class); cpuSearch.select("sum", Func.SUM, cpuSearch.entity().getCpu()); @@ -1096,16 +1088,10 @@ public long calculateMemoryForAccount(long accountId) { public Pair calculateMemoryTotalByAccountId(long accountId) { long ramtotal = 0; boolean hasMemory = false; - Account owner = _accountDao.findById(accountId); - DomainVO domain = _domainDao.findById(owner.getDomainId()); - ServiceOffering defaultRouterOffering = null; - String globalRouterOffering = VirtualNetworkApplianceManager.VirtualRouterServiceOffering.value(); - if (globalRouterOffering != null) { - defaultRouterOffering = serviceOfferingDao.findByUuid(globalRouterOffering); - } - if (defaultRouterOffering == null) { - defaultRouterOffering = serviceOfferingDao.findByName(ServiceOffering.routerDefaultOffUniqueName); - } + final Account owner = _accountDao.findById(accountId); + final DomainVO domain = _domainDao.findById(owner.getDomainId()); + final ServiceOffering defaultRouterOffering = getServiceOfferingByConfig(); + GenericSearchBuilder memorySearch = serviceOfferingDao.createSearchBuilder(SumCount.class); memorySearch.select("sum", Func.SUM, memorySearch.entity().getRamSize()); memorySearch.select("count", Func.COUNT, null); @@ -1222,6 +1208,26 @@ public void decrementResourceCount(long accountId, ResourceType type, Boolean di } } + /** + * Returns the service offering by the given configuration. + * + * @return the service offering found or null if not found + */ + public ServiceOffering getServiceOfferingByConfig() { + ServiceOffering defaultRouterOffering = null; + final String globalRouterOffering = VirtualNetworkApplianceManager.VirtualRouterServiceOffering.value(); + + if (globalRouterOffering != null) { + defaultRouterOffering = serviceOfferingDao.findByUuid(globalRouterOffering); + } + + if (defaultRouterOffering == null) { + defaultRouterOffering = serviceOfferingDao.findByName(ServiceOffering.routerDefaultOffUniqueName); + } + + return defaultRouterOffering; + } + @Override public void changeResourceCount(long accountId, ResourceType type, Boolean displayResource, Long... delta) { diff --git a/server/src/test/java/com/cloud/vpc/MockResourceLimitManagerImpl.java b/server/src/test/java/com/cloud/vpc/MockResourceLimitManagerImpl.java index 1d1da5331d92..fde106823b15 100644 --- a/server/src/test/java/com/cloud/vpc/MockResourceLimitManagerImpl.java +++ b/server/src/test/java/com/cloud/vpc/MockResourceLimitManagerImpl.java @@ -21,6 +21,7 @@ import javax.naming.ConfigurationException; +import com.cloud.offering.ServiceOffering; import com.cloud.utils.exception.CloudRuntimeException; import org.apache.cloudstack.user.ResourceReservation; import org.springframework.stereotype.Component; @@ -178,6 +179,11 @@ public ResourceReservation getReservation(Account account, Boolean displayResour throw new CloudRuntimeException("no reservation implemented for mock resource management."); } + @Override + public ServiceOffering getServiceOfferingByConfig() { + return null; + } + /* (non-Javadoc) * @see com.cloud.utils.component.Manager#configure(java.lang.String, java.util.Map) */ @@ -213,5 +219,4 @@ public String getName() { // TODO Auto-generated method stub return null; } - }