From 1d56abcf21f347fcbacb2938e2fe12d8b09d3249 Mon Sep 17 00:00:00 2001 From: Stephan Krug Date: Wed, 4 Jan 2023 18:12:31 -0300 Subject: [PATCH 1/9] Refactor and revert changes with assign VM error --- .../main/java/com/cloud/vm/UserVmService.java | 2 +- .../api/command/admin/vm/AssignVMCmd.java | 2 +- .../java/com/cloud/vm/UserVmManagerImpl.java | 1095 +++++++----- .../com/cloud/vm/UserVmManagerImplTest.java | 1508 ++++++++++++++++- .../java/com/cloud/vm/UserVmManagerTest.java | 4 +- 5 files changed, 2162 insertions(+), 449 deletions(-) diff --git a/api/src/main/java/com/cloud/vm/UserVmService.java b/api/src/main/java/com/cloud/vm/UserVmService.java index 9d8b196a4ff5..f97d167f8b8c 100644 --- a/api/src/main/java/com/cloud/vm/UserVmService.java +++ b/api/src/main/java/com/cloud/vm/UserVmService.java @@ -483,7 +483,7 @@ VirtualMachine migrateVirtualMachine(Long vmId, Host destinationHost) throws Res VirtualMachine migrateVirtualMachineWithVolume(Long vmId, Host destinationHost, Map volumeToPool) throws ResourceUnavailableException, ConcurrentOperationException, ManagementServerException, VirtualMachineMigrationException; - UserVm moveVMToUser(AssignVMCmd moveUserVMCmd) throws ResourceAllocationException, ConcurrentOperationException, ResourceUnavailableException, + UserVm moveVmToUser(AssignVMCmd moveUserVMCmd) throws ResourceAllocationException, ConcurrentOperationException, ResourceUnavailableException, InsufficientCapacityException; VirtualMachine vmStorageMigration(Long vmId, StoragePool destPool); diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java index ac63a5efac37..e7f688eabb58 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java @@ -120,7 +120,7 @@ public List getSecurityGroupIdList() { @Override public void execute() { try { - UserVm userVm = _userVmService.moveVMToUser(this); + UserVm userVm = _userVmService.moveVmToUser(this); if (userVm == null) { throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to move vm"); } diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index dce8a21b4788..e02faab4f042 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -146,6 +146,7 @@ import org.apache.commons.lang3.builder.ToStringBuilder; import org.apache.commons.lang3.builder.ToStringStyle; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.w3c.dom.Document; @@ -2732,7 +2733,7 @@ protected void runInContext() { } } - private void verifyVmLimits(UserVmVO vmInstance, Map details) { + protected void verifyVmLimits(UserVmVO vmInstance, Map details) { Account owner = _accountDao.findById(vmInstance.getAccountId()); if (owner == null) { throw new InvalidParameterValueException("The owner of " + vmInstance + " does not exist: " + vmInstance.getAccountId()); @@ -7276,518 +7277,760 @@ protected void checkVolumesLimits(Account account, List volumes) throw @DB @Override @ActionEvent(eventType = EventTypes.EVENT_VM_MOVE, eventDescription = "move VM to another user", async = false) - public UserVm moveVMToUser(final AssignVMCmd cmd) throws ResourceAllocationException, ConcurrentOperationException, ResourceUnavailableException, InsufficientCapacityException { - // VERIFICATIONS and VALIDATIONS - - // VV 1: verify the two users + public UserVm moveVmToUser(final AssignVMCmd cmd) throws ResourceAllocationException, ConcurrentOperationException, ResourceUnavailableException, InsufficientCapacityException { Account caller = CallContext.current().getCallingAccount(); - if (!_accountMgr.isRootAdmin(caller.getId()) - && !_accountMgr.isDomainAdmin(caller.getId())) { // only - // root - // admin - // can - // assign - // VMs - throw new InvalidParameterValueException("Only domain admins are allowed to assign VMs and not " + caller.getType()); - } - - // get and check the valid VM - final UserVmVO vm = _vmDao.findById(cmd.getVmId()); - if (vm == null) { - throw new InvalidParameterValueException("There is no vm by that id " + cmd.getVmId()); - } else if (vm.getState() == State.Running) { // VV 3: check if vm is - // running - if (logger.isDebugEnabled()) { - logger.debug("VM is Running, unable to move the vm " + vm); - } - InvalidParameterValueException ex = new InvalidParameterValueException("VM is Running, unable to move the vm with specified vmId"); - ex.addProxyObject(vm.getUuid(), "vmId"); - throw ex; + Long callerId = caller.getId(); + logger.trace(String.format("Verifying if caller [%s] is root or domain admin.", caller)); + if (!_accountMgr.isRootAdmin(callerId) && !_accountMgr.isDomainAdmin(callerId)) { + throw new InvalidParameterValueException(String.format("Only root or domain admins are allowed to assign VMs. Caller [%s] is of type [%s].", caller, caller.getType())); } - final Account oldAccount = _accountService.getActiveAccountById(vm.getAccountId()); - if (oldAccount == null) { - throw new InvalidParameterValueException("Invalid account for VM " + vm.getAccountId() + " in domain."); - } - final Account newAccount = _accountMgr.finalizeOwner(caller, cmd.getAccountName(), cmd.getDomainId(), cmd.getProjectId()); - if (newAccount == null) { - throw new InvalidParameterValueException("Invalid accountid=" + cmd.getAccountName() + " in domain " + cmd.getDomainId()); - } + Long vmId = cmd.getVmId(); + final UserVmVO vm = _vmDao.findById(vmId); + validateIfVmExistsAndIsNotRunning(vm, vmId); - if (newAccount.getState() == Account.State.DISABLED) { - throw new InvalidParameterValueException("The new account owner " + cmd.getAccountName() + " is disabled."); - } + Long domainId = cmd.getDomainId(); + Long projectId = cmd.getProjectId(); + Long oldAccountId = vm.getAccountId(); + String newAccountName = cmd.getAccountName(); + final Account oldAccount = _accountService.getActiveAccountById(oldAccountId); + final Account newAccount = _accountMgr.finalizeOwner(caller, newAccountName, domainId, projectId); + validateOldAndNewAccounts(oldAccount, newAccount, oldAccountId, newAccountName, domainId); + + checkCallerAccessToAccounts(caller, oldAccount, newAccount); - if (cmd.getProjectId() != null && cmd.getDomainId() == null) { + logger.trace(String.format("Verifying if the provided domain ID [%s] is valid.", domainId)); + if (projectId != null && domainId == null) { throw new InvalidParameterValueException("Please provide a valid domain ID; cannot assign VM to a project if domain ID is NULL."); } - //check caller has access to both the old and new account - _accountMgr.checkAccess(caller, null, true, oldAccount); - _accountMgr.checkAccess(caller, null, true, newAccount); + validateIfVmHasNoRules(vm, vmId); - // make sure the accounts are not same - if (oldAccount.getAccountId() == newAccount.getAccountId()) { - throw new InvalidParameterValueException("The new account is the same as the old account. Account id =" + oldAccount.getAccountId()); + final List volumes = _volsDao.findByInstance(vmId); + validateIfVolumesHaveNoSnapshots(volumes); + + final ServiceOfferingVO offering = serviceOfferingDao.findByIdIncludingRemoved(vm.getId(), vm.getServiceOfferingId()); + VirtualMachineTemplate template = _templateDao.findByIdIncludingRemoved(vm.getTemplateId()); + + verifyResourceLimitsForAccountAndStorage(newAccount, vm, vmId, offering, volumes, template); + + validateIfNewOwnerHasAccessToTemplate(vm, newAccount, template); + + DomainVO domain = _domainDao.findById(domainId); + logger.trace(String.format("Verifying if the new account [%s] has access to the specified domain [%s].", newAccount, domain)); + _accountMgr.checkAccess(newAccount, domain); + + Transaction.execute(new TransactionCallbackNoReturn() { + @Override + public void doInTransactionWithoutResult(TransactionStatus status) { + executeStepsToChangeOwnershipOfVm(cmd, caller, oldAccount, newAccount, vm, offering, volumes, template, domainId); + } + }); + + logger.info(String.format("VM [%s] now belongs to account [%s].", vm.getInstanceName(), newAccountName)); + return vm; + } + + protected void validateIfVmExistsAndIsNotRunning(UserVmVO vm, Long vmId) { + logger.trace(String.format("Validating if VM [%s] exists and is not in state [%s].", vmId, State.Running)); + + if (vm == null) { + throw new InvalidParameterValueException(String.format("There is no VM by ID [%s].", vmId)); + } else if (vm.getState() == State.Running) { + String errMsg = String.format("Unable to move VM [%s] in [%s] state.", vm, vm.getState()); + logger.warn(errMsg); + throw new InvalidParameterValueException(errMsg); } + } - // don't allow to move the vm if there are existing PF/LB/Static Nat - // rules, or vm is assigned to static Nat ip - List pfrules = _portForwardingDao.listByVm(cmd.getVmId()); - if (pfrules != null && pfrules.size() > 0) { - throw new InvalidParameterValueException("Remove the Port forwarding rules for this VM before assigning to another user."); + /** + * Validates if the provided VM does not have any existing Port Forwarding, Load Balancer, Static Nat, and One to One Nat rules. + * If any rules exist, throws a {@link InvalidParameterValueException}. + * @param vm the VM to be checked for the rules. + * @param vmId the ID of the VM to be checked. + * @throws InvalidParameterValueException + */ + protected void validateIfVmHasNoRules(UserVmVO vm, Long vmId) throws InvalidParameterValueException { + logger.trace(String.format("Validating if VM [%s] has no Port Forwarding, Static Nat, Load Balancing or One to One Nat rules.", vm)); + + List portForwardingRules = _portForwardingDao.listByVm(vmId); + if (CollectionUtils.isNotEmpty(portForwardingRules)) { + throw new InvalidParameterValueException(String.format("Remove any Port Forwarding rules for VM [%s] before assigning it to another user.", vm)); } - List snrules = _rulesDao.listStaticNatByVmId(vm.getId()); - if (snrules != null && snrules.size() > 0) { - throw new InvalidParameterValueException("Remove the StaticNat rules for this VM before assigning to another user."); + + List staticNatRules = _rulesDao.listStaticNatByVmId(vmId); + if (CollectionUtils.isNotEmpty(staticNatRules)) { + throw new InvalidParameterValueException(String.format("Remove the StaticNat rules for VM [%s] before assigning it to another user.", vm)); } - List maps = _loadBalancerVMMapDao.listByInstanceId(vm.getId()); - if (maps != null && maps.size() > 0) { - throw new InvalidParameterValueException("Remove the load balancing rules for this VM before assigning to another user."); + + List loadBalancerVmMaps = _loadBalancerVMMapDao.listByInstanceId(vmId); + if (CollectionUtils.isNotEmpty(loadBalancerVmMaps)) { + throw new InvalidParameterValueException(String.format("Remove the Load Balancing rules for VM [%s] before assigning it to another user.", vm)); } - // check for one on one nat - List ips = _ipAddressDao.findAllByAssociatedVmId(cmd.getVmId()); + + List ips = _ipAddressDao.findAllByAssociatedVmId(vmId); for (IPAddressVO ip : ips) { if (ip.isOneToOneNat()) { - throw new InvalidParameterValueException("Remove the one to one nat rule for this VM for ip " + ip.toString()); + throw new InvalidParameterValueException(String.format("Remove the One to One Nat rule for VM [%s] for IP [%s].", vm, ip)); } } + } - final List volumes = _volsDao.findByInstance(cmd.getVmId()); - + protected void validateIfVolumesHaveNoSnapshots(List volumes) throws InvalidParameterValueException { + logger.trace("Verifying if there are any snapshots for any of the VM volumes."); for (VolumeVO volume : volumes) { - List snapshots = _snapshotDao.listByStatusNotIn(volume.getId(), Snapshot.State.Destroyed,Snapshot.State.Error); - if (snapshots != null && snapshots.size() > 0) { - throw new InvalidParameterValueException( - "Snapshots exists for volume: "+ volume.getName()+ ", Detach volume or remove snapshots for volume before assigning VM to another user."); + logger.trace(String.format("Verifying snapshots for volume [%s].", volume)); + List snapshots = _snapshotDao.listByStatusNotIn(volume.getId(), Snapshot.State.Destroyed, Snapshot.State.Error); + if (CollectionUtils.isNotEmpty(snapshots)) { + throw new InvalidParameterValueException(String.format("Snapshots exist for volume [%s]. Detach volume or remove snapshots for the volume before assigning VM to " + + "another user.", volume.getName())); } } + } - DataCenterVO zone = _dcDao.findById(vm.getDataCenterId()); + /** + * Verifies if the CPU, RAM and volume size do not exceed the account and the primary storage limit. + * If any limit is exceeded, throws a {@link ResourceAllocationException}. + * @param account The account to check if CPU and RAM limit has been exceeded. + * @param vm The VM which can exceed resource limits. + * @param vmId The ID for the VM which can exceed resource limits. + * @param offering The service offering which can exceed resource limits. + * @param volumes The volumes whose total size can exceed resource limits. + * @throws ResourceAllocationException + */ + protected void verifyResourceLimitsForAccountAndStorage(Account account, UserVmVO vm, Long vmId, ServiceOfferingVO offering, List volumes, VirtualMachineTemplate template) + throws ResourceAllocationException { - VirtualMachineTemplate template = _templateDao.findByIdIncludingRemoved(vm.getTemplateId()); - // Get serviceOffering and Volumes for Virtual Machine - final ServiceOfferingVO offering = serviceOfferingDao.findByIdIncludingRemoved(vm.getId(), vm.getServiceOfferingId()); + logger.trace(String.format("Verifying if CPU and RAM for VM [%s] do not exceed account [%s] limit.", vm, account)); - //Remove vm from instance group - removeInstanceFromInstanceGroup(cmd.getVmId()); + resourceCountIncrement(account.getAccountId(), vm.isDisplayVm(), offering, template); - // VV 2: check if account/domain is with in resource limits to create a new vm - if (!VirtualMachineManager.ResourceCountRunningVMsonly.value()) { - resourceLimitService.checkVmResourceLimit(newAccount, vm.isDisplayVm(), offering, template); - } + logger.trace(String.format("Verifying if volume size for VM [%s] does not exceed account [%s] limit.", vm, account)); + + checkVolumesLimits(account, volumes); + } - // VV 3: check if volumes and primary storage space are with in resource limits - checkVolumesLimits(newAccount, volumes); + protected void validateIfNewOwnerHasAccessToTemplate(UserVmVO vm, Account newAccount, VirtualMachineTemplate template) { + logger.trace(String.format("Validating if new owner [%s] has access to the template specified for VM [%s].", newAccount, vm)); - // VV 4: Check if new owner can use the vm template if (template == null) { - throw new InvalidParameterValueException(String.format("Template for VM: %s cannot be found", vm.getUuid())); + throw new InvalidParameterValueException(String.format("Template for VM [%s] cannot be found.", vm.getUuid())); } + if (!template.isPublicTemplate()) { Account templateOwner = _accountMgr.getAccount(template.getAccountId()); _accountMgr.checkAccess(newAccount, null, true, templateOwner); } + } - // VV 5: check the new account can create vm in the domain - DomainVO domain = _domainDao.findById(cmd.getDomainId()); - _accountMgr.checkAccess(newAccount, domain); + /** + * Executes all ownership steps necessary to assign a VM to another user: + * generating a destroy VM event ({@link EventTypes}), + * decrementing the old user resource count ({@link #resourceCountDecrement(long, Boolean, ServiceOffering, VirtualMachineTemplate)}), + * removing the VM from its instance group ({@link #removeInstanceFromInstanceGroup(long)}), + * updating the VM owner to the new account ({@link #updateVmOwner(Account, UserVmVO, Long, Long)}), + * updating the volumes to the new account ({@link #updateVolumesOwner(List, Account, Account, Long)}), + * updating the network for the VM ({@link #updateVmNetwork(AssignVMCmd, Account, UserVmVO, Account, VirtualMachineTemplate)}), + * incrementing the new user resource count ({@link #resourceCountIncrement(long, Boolean, ServiceOffering, VirtualMachineTemplate)}), + * and generating a create VM event ({@link EventTypes}). + * @param cmd The assignVMCmd. + * @param caller The account calling the assignVMCmd. + * @param oldAccount The old account from whom the VM will be moved. + * @param newAccount The new account to whom the VM will move. + * @param vm The VM to be moved between accounts. + * @param offering The service offering which will be used to decrement and increment resource counts. + * @param volumes The volumes of the VM which will be assigned to another user. + * @param template The template of the VM which will be assigned to another user. + * @param domainId The ID of the domain where the VM which will be assigned to another user is. + */ + protected void executeStepsToChangeOwnershipOfVm(AssignVMCmd cmd, Account caller, Account oldAccount, Account newAccount, UserVmVO vm, ServiceOfferingVO offering, + List volumes, VirtualMachineTemplate template, Long domainId) { - Transaction.execute(new TransactionCallbackNoReturn() { - @Override - public void doInTransactionWithoutResult(TransactionStatus status) { - //generate destroy vm event for usage - UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_DESTROY, vm.getAccountId(), vm.getDataCenterId(), - vm.getId(), vm.getHostName(), vm.getServiceOfferingId(), vm.getTemplateId(), - vm.getHypervisorType().toString(), VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm()); - // update resource counts for old account - resourceCountDecrement(oldAccount.getAccountId(), vm.isDisplayVm(), offering, template); - - // OWNERSHIP STEP 1: update the vm owner - vm.setAccountId(newAccount.getAccountId()); - vm.setDomainId(cmd.getDomainId()); - _vmDao.persist(vm); - - // OS 2: update volume - for (VolumeVO volume : volumes) { - UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VOLUME_DELETE, volume.getAccountId(), volume.getDataCenterId(), volume.getId(), volume.getName(), - Volume.class.getName(), volume.getUuid(), volume.isDisplayVolume()); - DiskOfferingVO diskOfferingVO = _diskOfferingDao.findById(volume.getDiskOfferingId()); - _resourceLimitMgr.decrementVolumeResourceCount(oldAccount.getAccountId(), volume.isDisplay(), volume.getSize(), diskOfferingVO); - volume.setAccountId(newAccount.getAccountId()); - volume.setDomainId(newAccount.getDomainId()); - _volsDao.persist(volume); - _resourceLimitMgr.incrementVolumeResourceCount(newAccount.getAccountId(), volume.isDisplay(), volume.getSize(), diskOfferingVO); - UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VOLUME_CREATE, volume.getAccountId(), volume.getDataCenterId(), volume.getId(), volume.getName(), - volume.getDiskOfferingId(), volume.getTemplateId(), volume.getSize(), Volume.class.getName(), - volume.getUuid(), volume.isDisplayVolume()); - } - - //update resource count of new account - resourceCountIncrement(newAccount.getAccountId(), vm.isDisplayVm(), offering, template); - - //generate usage events to account for this change - UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE, vm.getAccountId(), vm.getDataCenterId(), vm.getId(), - vm.getHostName(), vm.getServiceOfferingId(), vm.getTemplateId(), vm.getHypervisorType().toString(), - VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm()); - } - }); + logger.trace(String.format("Generating destroy event for VM [%s].", vm)); + UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_DESTROY, vm.getAccountId(), vm.getDataCenterId(), vm.getId(), vm.getHostName(), vm.getServiceOfferingId(), + vm.getTemplateId(), vm.getHypervisorType().toString(), VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm()); + + logger.trace(String.format("Decrementing old account [%s] resource count.", oldAccount)); + resourceCountDecrement(oldAccount.getAccountId(), vm.isDisplayVm(), offering, template); + + logger.trace(String.format("Removing VM [%s] from its instance group.", vm)); + removeInstanceFromInstanceGroup(vm.getId()); + + Long newAccountId = newAccount.getAccountId(); + updateVmOwner(newAccount, vm, domainId, newAccountId); + + updateVolumesOwner(volumes, oldAccount, newAccount, newAccountId); + + try { + updateVmNetwork(cmd, caller, vm, newAccount, template); + } catch (InsufficientCapacityException | ResourceAllocationException e) { + throw new CloudRuntimeException(String.format("Unable to update networks when assigning VM [%s] due to [%s].", vm, e.getMessage()), e); + } + + logger.trace(String.format("Incrementing new account [%s] resource count.", newAccount)); + if (!isResourceCountRunningVmsOnlyEnabled()) { + resourceCountIncrement(newAccountId, vm.isDisplayVm(), offering, template); + } + + logger.trace(String.format("Generating create event for VM [%s].", vm)); + UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_CREATE, vm.getAccountId(), vm.getDataCenterId(), vm.getId(), vm.getHostName(), vm.getServiceOfferingId(), + vm.getTemplateId(), vm.getHypervisorType().toString(), VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm()); + } + + protected void updateVmOwner(Account newAccount, UserVmVO vm, Long domainId, Long newAccountId) { + logger.debug(String.format("Updating VM [%s] owner to [%s].", vm, newAccount)); + + vm.setAccountId(newAccountId); + vm.setDomainId(domainId); + + _vmDao.persist(vm); + } + + protected void updateVolumesOwner(final List volumes, Account oldAccount, Account newAccount, Long newAccountId) { + logger.debug(String.format("Updating volumes owner from old account [%s] to new account [%s].", oldAccount, newAccount)); + + for (VolumeVO volume : volumes) { + logger.trace(String.format("Generating a delete volume event for volume [%s].", volume)); + UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VOLUME_DELETE, volume.getAccountId(), volume.getDataCenterId(), volume.getId(), volume.getName(), + Volume.class.getName(), volume.getUuid(), volume.isDisplayVolume()); + + logger.trace(String.format("Decrementing volume [%s] and primary storage resource count for the old account [%s].", volume, oldAccount)); + DiskOfferingVO diskOfferingVO = _diskOfferingDao.findById(volume.getDiskOfferingId()); + _resourceLimitMgr.decrementVolumeResourceCount(oldAccount.getAccountId(), volume.isDisplay(), volume.getSize(), diskOfferingVO); + + logger.trace(String.format("Setting the new account [%s] and domain [%s] for volume [%s].", newAccount, newAccount.getDomainId(), volume)); + volume.setAccountId(newAccountId); + volume.setDomainId(newAccount.getDomainId()); + + _volsDao.persist(volume); + + logger.trace(String.format("Incrementing volume [%s] and primary storage resource count for the new account [%s].", volume, newAccount)); + _resourceLimitMgr.incrementVolumeResourceCount(newAccount.getAccountId(), volume.isDisplay(), volume.getSize(), diskOfferingVO); + + logger.trace(String.format("Generating a create volume event for volume [%s].", volume)); + UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VOLUME_CREATE, volume.getAccountId(), volume.getDataCenterId(), volume.getId(), volume.getName(), + volume.getDiskOfferingId(), volume.getTemplateId(), volume.getSize(), Volume.class.getName(), volume.getUuid(), volume.isDisplayVolume()); + } + } + + /** + * Updates the network for a VM being assigned to a new account. + * If the network type for the zone is basic, calls + * {@link #updateBasicTypeNetworkForVm(AssignVMCmd, UserVmVO, Account, VirtualMachineTemplate, VirtualMachineProfileImpl, DataCenterVO, List, List)}. + * If the network type for the zone is advanced, calls + * {@link #updateAdvancedTypeNetworkForVm(AssignVMCmd, Account, UserVmVO, Account, VirtualMachineTemplate, VirtualMachineProfileImpl, DataCenterVO, List, List)}. + * @param cmd The assignVMCmd. + * @param caller The account calling the assignVMCmd. + * @param vm The VM to be assigned to another user, which has to have networks updated. + * @param newAccount The account to whom the VM will be assigned to. + * @param template The template of the VM which will be assigned to another account. + * @throws InsufficientCapacityException + * @throws ResourceAllocationException + */ + protected void updateVmNetwork(AssignVMCmd cmd, Account caller, UserVmVO vm, Account newAccount, VirtualMachineTemplate template) + throws InsufficientCapacityException, ResourceAllocationException { + + logger.trace(String.format("Updating network for VM [%s].", vm)); VirtualMachine vmoi = _itMgr.findById(vm.getId()); VirtualMachineProfileImpl vmOldProfile = new VirtualMachineProfileImpl(vmoi); - // OS 3: update the network + DataCenterVO zone = _dcDao.findById(vm.getDataCenterId()); + List networkIdList = cmd.getNetworkIds(); List securityGroupIdList = cmd.getSecurityGroupIdList(); if (zone.getNetworkType() == NetworkType.Basic) { - if (networkIdList != null && !networkIdList.isEmpty()) { - throw new InvalidParameterValueException("Can't move vm with network Ids; this is a basic zone VM"); - } - // cleanup the old security groups + updateBasicTypeNetworkForVm(cmd, vm, newAccount, template, vmOldProfile, zone, networkIdList, securityGroupIdList); + return; + } + + updateAdvancedTypeNetworkForVm(cmd, caller, vm, newAccount, template, vmOldProfile, zone, networkIdList, securityGroupIdList); + } + + /** + * Validates if the old account exists, the new account exists and is not disabled, and they are different from each other. + * If any of the validations fail, throws a {@link InvalidParameterValueException}. + * @param oldAccount The old account which will be checked if exists, and if it is different from the new account. + * @param newAccount The new account which will be checked if exists, if it is different from the old account, and if it is not disabled. + * @param oldAccountId The ID of the old account to be checked. + * @param newAccountName The name of the new account to be checked. + * @param domainId The domain where to validate the conditions. + * @throws InvalidParameterValueException + */ + protected void validateOldAndNewAccounts(Account oldAccount, Account newAccount, Long oldAccountId, String newAccountName, Long domainId) + throws InvalidParameterValueException { + + if (oldAccount == null) { + throw new InvalidParameterValueException(String.format("Invalid old account [%s] for VM in domain [%s].", oldAccountId, domainId)); + } + + if (newAccount == null) { + throw new InvalidParameterValueException(String.format("Invalid new account [%s] for VM in domain [%s].", newAccountName, domainId)); + } + + if (newAccount.getState() == Account.State.DISABLED) { + throw new InvalidParameterValueException(String.format("The new account owner [%s] is disabled.", newAccount)); + } + + if (oldAccount.getAccountId() == newAccount.getAccountId()) { + throw new InvalidParameterValueException(String.format("The new account [%s] is the same as the old account.", newAccount)); + } + } + + protected void checkCallerAccessToAccounts(Account caller, Account oldAccount, Account newAccount) { + logger.trace(String.format("Verifying if caller [%s] has access to old account [%s].", caller, oldAccount)); + _accountMgr.checkAccess(caller, null, true, oldAccount); + + logger.trace(String.format("Verifying if caller [%s] has access to new account [%s].", caller, newAccount)); + _accountMgr.checkAccess(caller, null, true, newAccount); + } + + protected Boolean isResourceCountRunningVmsOnlyEnabled() { + return VirtualMachineManager.ResourceCountRunningVMsonly.value(); + } + + /** + * Updates a basic type network by: + * cleaning up the old network ({@link #cleanupOfOldOwnerNicsForNetwork(VirtualMachineProfileImpl)}), + * allocating all networks ({@link #allocateNetworksForVm(UserVmVO, LinkedHashMap)}), + * and adding security groups to the VM ({@link #addSecurityGroupsToVm(Account, UserVmVO, VirtualMachineTemplate, List, Network)}). + * If the network has network IDs, throws a {@link InvalidParameterValueException}. + * @param cmd The assignVMCmd which attempts to update a basic network. + * @param vm The VM for which the networks are allocated. + * @param newAccount The new account to which the VM will be assigned to. + * @param template The template of the VM. + * @param vmOldProfile The VM profile. + * @param zone The zone where the network has to be allocated. + * @param networkIdList The list of network IDs provided to the assignVMCmd. + * @param securityGroupIdList The list of security groups provided to the assignVMCmd. + * @throws InsufficientCapacityException + */ + protected void updateBasicTypeNetworkForVm(AssignVMCmd cmd, UserVmVO vm, Account newAccount, VirtualMachineTemplate template, VirtualMachineProfileImpl vmOldProfile, + DataCenterVO zone, List networkIdList, List securityGroupIdList) throws InsufficientCapacityException { + + if (networkIdList != null && !networkIdList.isEmpty()) { + throw new InvalidParameterValueException("Cannot move VM with Network IDs; this is a basic zone VM."); + } + + logger.trace(String.format("Cleanup of old security groups for VM [%s]. They will be recreated for the new account once the VM is started.", vm)); + _securityGroupMgr.removeInstanceFromGroups(cmd.getVmId()); + + cleanupOfOldOwnerNicsForNetwork(vmOldProfile); + + List networkList = new ArrayList(); + Network defaultNetwork = _networkModel.getExclusiveGuestNetwork(zone.getId()); + addDefaultNetworkToNetworkList(networkList, defaultNetwork); + + LinkedHashMap> networks = new LinkedHashMap>(); + NicProfile profile = new NicProfile(); + profile.setDefaultNic(true); + networks.put(networkList.get(0), new ArrayList(Arrays.asList(profile))); + + allocateNetworksForVm(vm, networks); + + addSecurityGroupsToVm(newAccount, vm, template, securityGroupIdList, defaultNetwork); + } + + /** + * Updates an advanced type network by: + * adding NICs to the networks ({@link #addNicsToApplicableNetworksAndReturnDefaultNetwork(LinkedHashSet, Map, Map, LinkedHashMap)}), + * allocating - if security groups are enabled ({@link #allocateNetworksForVm(UserVmVO, LinkedHashMap)}) - + * or selecting applicable networks otherwise ({@link #selectApplicableNetworkToCreateVm(Account, Account, DataCenterVO, Set)}), + * and adding security groups to the VM ({@link #addSecurityGroupsToVm(Account, UserVmVO, VirtualMachineTemplate, List, Network)}) - if enabled in the zone. + * If no applicable network is provided and the zone has security groups enabled, throws a {@link InvalidParameterValueException}. + * If security groups are not enabled, but security groups have been provided, throws a {@link InvalidParameterValueException}. + * @param cmd The assignVMCmd which attempts to update an advanced network. + * @param caller The caller of the assignVMCmd. + * @param vm The VM for which the networks are allocated or selected. + * @param newAccount The new account to which the VM will be assigned to. + * @param template The template of the VM. + * @param vmOldProfile The VM profile. + * @param zone The zone where the network has to be allocated or selected. + * @param networkIdList The list of network IDs provided to the assignVMCmd. + * @param securityGroupIdList The list of security groups provided to the assignVMCmd. + * @throws InsufficientCapacityException + * @throws ResourceAllocationException + * @throws InvalidParameterValueException + */ + protected void updateAdvancedTypeNetworkForVm(AssignVMCmd cmd, Account caller, UserVmVO vm, Account newAccount, VirtualMachineTemplate template, + VirtualMachineProfileImpl vmOldProfile, DataCenterVO zone, List networkIdList, List securityGroupIdList) + throws InsufficientCapacityException, ResourceAllocationException, InvalidParameterValueException { + + LinkedHashSet applicableNetworks = new LinkedHashSet<>(); + Map requestedIPv4ForNics = new HashMap<>(); + Map requestedIPv6ForNics = new HashMap<>(); + LinkedHashMap> networks = new LinkedHashMap>(); + + if (zone.isSecurityGroupEnabled()) { + logger.debug(String.format("Cleanup of old security groups for VM [%s]. They will be recreated for the new account once the VM is started.", vm)); _securityGroupMgr.removeInstanceFromGroups(cmd.getVmId()); - // cleanup the network for the oldOwner - _networkMgr.cleanupNics(vmOldProfile); - _networkMgr.removeNics(vmOldProfile); - // security groups will be recreated for the new account, when the - // VM is started - List networkList = new ArrayList(); - // Get default guest network in Basic zone - Network defaultNetwork = _networkModel.getExclusiveGuestNetwork(zone.getId()); + addNetworksToNetworkIdList(vm, newAccount, vmOldProfile, networkIdList, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); + + NetworkVO defaultNetwork = addNicsToApplicableNetworksAndReturnDefaultNetwork(applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics, networks); - if (defaultNetwork == null) { - throw new InvalidParameterValueException("Unable to find a default network to start a vm"); + if (applicableNetworks.isEmpty()) { + throw new InvalidParameterValueException("No network is specified, please specify one when you move the VM. For now, please add a network to VM on NICs tab."); } else { - networkList.add(_networkDao.findById(defaultNetwork.getId())); + allocateNetworksForVm(vm, networks); } - boolean isVmWare = (template.getHypervisorType() == HypervisorType.VMware); + addSecurityGroupsToVm(newAccount, vm, template, securityGroupIdList, defaultNetwork); + return; + } - if (securityGroupIdList != null && isVmWare) { - throw new InvalidParameterValueException("Security group feature is not supported for vmWare hypervisor"); - } else if (!isVmWare && _networkModel.isSecurityGroupSupportedInNetwork(defaultNetwork) && _networkModel.canAddDefaultSecurityGroup()) { - if (securityGroupIdList == null) { - securityGroupIdList = new ArrayList(); - } - SecurityGroup defaultGroup = _securityGroupMgr.getDefaultSecurityGroup(newAccount.getId()); - if (defaultGroup != null) { - // check if security group id list already contains Default - // security group, and if not - add it - boolean defaultGroupPresent = false; - for (Long securityGroupId : securityGroupIdList) { - if (securityGroupId.longValue() == defaultGroup.getId()) { - defaultGroupPresent = true; - break; - } - } + if (securityGroupIdList != null && !securityGroupIdList.isEmpty()) { + throw new InvalidParameterValueException("Cannot move VM with security groups; security group feature is not enabled in this zone."); + } - if (!defaultGroupPresent) { - securityGroupIdList.add(defaultGroup.getId()); - } + addNetworksToNetworkIdList(vm, newAccount, vmOldProfile, networkIdList, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); - } else { - // create default security group for the account - if (logger.isDebugEnabled()) { - logger.debug("Couldn't find default security group for the account " + newAccount + " so creating a new one"); - } - defaultGroup = _securityGroupMgr.createSecurityGroup(SecurityGroupManager.DEFAULT_GROUP_NAME, SecurityGroupManager.DEFAULT_GROUP_DESCRIPTION, - newAccount.getDomainId(), newAccount.getId(), newAccount.getAccountName()); - securityGroupIdList.add(defaultGroup.getId()); - } + if (applicableNetworks.isEmpty()) { + selectApplicableNetworkToCreateVm(caller, newAccount, zone, applicableNetworks); + } + + addNicsToApplicableNetworksAndReturnDefaultNetwork(applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics, networks); + + allocateNetworksForVm(vm, networks); + logger.debug(String.format("Adding [%s] networks to VM [%s].", networks.size(), vm.getInstanceName())); + } + + protected void cleanupOfOldOwnerNicsForNetwork(VirtualMachineProfileImpl vmOldProfile) { + logger.trace(String.format("Cleanup of old owner network for VM [%s].", vmOldProfile)); + + _networkMgr.cleanupNics(vmOldProfile); + _networkMgr.removeNics(vmOldProfile); + } + + protected void addDefaultNetworkToNetworkList(List networkList, Network defaultNetwork) { + logger.trace("Adding default network to network list."); + + if (defaultNetwork == null) { + throw new InvalidParameterValueException("Unable to find a default network to start a VM."); + } + + networkList.add(_networkDao.findById(defaultNetwork.getId())); + } + + protected void allocateNetworksForVm(UserVmVO vm, LinkedHashMap> networks) throws InsufficientCapacityException { + logger.trace(String.format("Allocating networks for VM [%s].", vm)); + + VirtualMachine vmi = _itMgr.findById(vm.getId()); + VirtualMachineProfileImpl vmProfile = new VirtualMachineProfileImpl(vmi); + _networkMgr.allocate(vmProfile, networks, null); + } + + protected void addSecurityGroupsToVm(Account newAccount, UserVmVO vm, VirtualMachineTemplate template, List securityGroupIdList, Network defaultNetwork) { + int securityIdList = securityGroupIdList != null ? securityGroupIdList.size() : 0; + logger.debug("Adding security groups no " + securityIdList + " to " + vm.getInstanceName()); + + boolean isVmWare = (template.getHypervisorType() == HypervisorType.VMware); + if (securityGroupIdList != null && isVmWare) { + throw new InvalidParameterValueException("Security group feature is not supported for VMWare hypervisor."); + } else if (!isVmWare && (defaultNetwork == null || _networkModel.isSecurityGroupSupportedInNetwork(defaultNetwork)) && _networkModel.canAddDefaultSecurityGroup()) { + if (securityGroupIdList == null) { + securityGroupIdList = new ArrayList(); + } + + addDefaultSecurityGroupToSecurityGroupIdList(newAccount, securityGroupIdList); + } + + _securityGroupMgr.addInstanceToGroups(vm.getId(), securityGroupIdList); + } + + /** + * Adds all networks to the list of network IDs by: + * attempting to keep the shared network for the VM ({@link #keepOldSharedNetworkForVm(UserVmVO, Account, List, Set, Map, Map)}), + * adding any additional applicable networks to the VM ({@link #addAdditionalNetworksToVm(UserVmVO, Account, List, Set, Map, Map)}), + * and cleaning up the network associated to the old owner ({@link #cleanupOfOldOwnerNicsForNetwork(VirtualMachineProfileImpl)}). + * @param vm The VM to add the networks to. + * @param newAccount The account to access the networks. + * @param vmOldProfile The old profile of the VM. + * @param networkIdList The network IDs which have to be added to the VM. + * @param applicableNetworks The applicable networks which have to be added to the VM. + * @param requestedIPv4ForNics All requested IPv4 for NICs. + * @param requestedIPv6ForNics All requested IPv6 for NICs. + */ + protected void addNetworksToNetworkIdList(UserVmVO vm, Account newAccount, VirtualMachineProfileImpl vmOldProfile, List networkIdList, Set applicableNetworks, + Map requestedIPv4ForNics, Map requestedIPv6ForNics) { + + logger.trace("Adding networks to network list."); + + keepOldSharedNetworkForVm(vm, newAccount, networkIdList, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); + + addAdditionalNetworksToVm(vm, newAccount, networkIdList, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); + + cleanupOfOldOwnerNicsForNetwork(vmOldProfile); + } + + /** + * Adds NICs to the applicable networks. The first applicable network is considered the default network, and is associated to the default NIC. + * @param applicableNetworks The applicable networks which will be associated with NICs. + * @param requestedIPv4ForNics All requested IPv4 for NICs. + * @param requestedIPv6ForNics All requested IPv6 for NICs. + * @param networks The networks to which the networks and NICs have to be added. + * @return The default network, if it exists. Otherwise, returns null. + */ + @Nullable + protected NetworkVO addNicsToApplicableNetworksAndReturnDefaultNetwork(LinkedHashSet applicableNetworks, Map requestedIPv4ForNics, + Map requestedIPv6ForNics, LinkedHashMap> networks) { + + logger.trace("Adding NICs to applicable networks."); + + NetworkVO defaultNetwork = null; + + if (!applicableNetworks.isEmpty()) { + NicProfile defaultNic = new NicProfile(); + defaultNic.setDefaultNic(true); + defaultNetwork = applicableNetworks.iterator().next(); + + for (NetworkVO appNet : applicableNetworks) { + defaultNic.setRequestedIPv4(requestedIPv4ForNics.get(appNet.getId())); + defaultNic.setRequestedIPv6(requestedIPv6ForNics.get(appNet.getId())); + networks.put(appNet, new ArrayList(Arrays.asList(defaultNic))); + + defaultNic = new NicProfile(); } + } + return defaultNetwork; + } + + /** + * Selects the default network as the applicable network to be used to create the VM. If none exists, creates a new one. + * If no network offerings are applicable, throws a {@link InvalidParameterValueException}. + * If the network offering applicable is not enabled, throws a {@link InvalidParameterValueException}. + * If more than one default isolated network is related to the account, throws a {@link InvalidParameterValueException}, since the ID of the network to be used has to be + * specified. + * @param caller The account which calls to select the applicable network. + * @param newAccount The new account associated to the selected network. + * @param zone The zone where the network is selected. + * @param applicableNetworks The applicable networks to which the selected network has to be added to. + * @throws InsufficientCapacityException + * @throws ResourceAllocationException + */ + protected void selectApplicableNetworkToCreateVm(Account caller, Account newAccount, DataCenterVO zone, Set applicableNetworks) + throws InsufficientCapacityException, ResourceAllocationException { - LinkedHashMap> networks = new LinkedHashMap>(); - NicProfile profile = new NicProfile(); - profile.setDefaultNic(true); - networks.put(networkList.get(0), new ArrayList(Arrays.asList(profile))); + logger.trace("Selecting the applicable network to create the VM."); - VirtualMachine vmi = _itMgr.findById(vm.getId()); - VirtualMachineProfileImpl vmProfile = new VirtualMachineProfileImpl(vmi); - _networkMgr.allocate(vmProfile, networks, null); + List requiredOfferings = _networkOfferingDao.listByAvailability(Availability.Required, false); + if (CollectionUtils.isEmpty(requiredOfferings)) { + throw new InvalidParameterValueException(String.format("Unable to find network offering with availability [%s] to automatically create the network as a part of VM " + + "creation.", Availability.Required)); + } - _securityGroupMgr.addInstanceToGroups(vm.getId(), securityGroupIdList); + NetworkOfferingVO firstRequiredOffering = requiredOfferings.get(0); + if (firstRequiredOffering.getState() != NetworkOffering.State.Enabled) { + throw new InvalidParameterValueException(String.format("Required network offering ID [%s] is not in [%s] state.", firstRequiredOffering.getId(), + NetworkOffering.State.Enabled)); + } - int securityIdList = securityGroupIdList != null ? securityGroupIdList.size() : 0; - logger.debug("AssignVM: Basic zone, adding security groups no " + securityIdList + " to " + vm.getInstanceName()); + NetworkVO defaultNetwork; + List virtualNetworks = _networkModel.listNetworksForAccount(newAccount.getId(), zone.getId(), Network.GuestType.Isolated); + if (virtualNetworks.isEmpty()) { + defaultNetwork = createApplicableNetworkToCreateVm(caller, newAccount, zone, firstRequiredOffering); + } else if (virtualNetworks.size() > 1) { + throw new InvalidParameterValueException(String.format("More than one default isolated network has been found for account [%s]; please specify networkIDs.", + newAccount)); } else { - Set applicableNetworks = new LinkedHashSet<>(); - Map requestedIPv4ForNics = new HashMap<>(); - Map requestedIPv6ForNics = new HashMap<>(); - if (zone.isSecurityGroupEnabled()) { // advanced zone with security groups - // cleanup the old security groups - _securityGroupMgr.removeInstanceFromGroups(cmd.getVmId()); - // if networkIdList is null and the first network of vm is shared network, then keep it if possible - if (networkIdList == null || networkIdList.isEmpty()) { - NicVO defaultNicOld = _nicDao.findDefaultNicForVM(vm.getId()); - if (defaultNicOld != null) { - NetworkVO defaultNetworkOld = _networkDao.findById(defaultNicOld.getNetworkId()); - if (canAccountUseNetwork(newAccount, defaultNetworkOld)) { - applicableNetworks.add(defaultNetworkOld); - requestedIPv4ForNics.put(defaultNetworkOld.getId(), defaultNicOld.getIPv4Address()); - requestedIPv6ForNics.put(defaultNetworkOld.getId(), defaultNicOld.getIPv6Address()); - logger.debug("AssignVM: use old shared network " + defaultNetworkOld.getName() + " with old ip " + defaultNicOld.getIPv4Address() + " on default nic of vm:" + vm.getInstanceName()); - } - } - } + defaultNetwork = _networkDao.findById(virtualNetworks.get(0).getId()); + } - if (networkIdList != null && !networkIdList.isEmpty()) { - // add any additional networks - for (Long networkId : networkIdList) { - NetworkVO network = _networkDao.findById(networkId); - if (network == null) { - InvalidParameterValueException ex = new InvalidParameterValueException( - "Unable to find specified network id"); - ex.addProxyObject(networkId.toString(), "networkId"); - throw ex; - } + applicableNetworks.add(defaultNetwork); + } - _networkModel.checkNetworkPermissions(newAccount, network); + /** + * Adds the default security group to a security group ID list. If the default security group does not exist, creates a new one. + * @param newAccount The account to be checked for the security groups. + * @param securityGroupIdList The list of security group IDs. + */ + protected void addDefaultSecurityGroupToSecurityGroupIdList(Account newAccount, List securityGroupIdList) { + logger.debug("Adding default security group to security group list if not already in it."); - // don't allow to use system networks - NetworkOffering networkOffering = _entityMgr.findById(NetworkOffering.class, network.getNetworkOfferingId()); - if (networkOffering.isSystemOnly()) { - InvalidParameterValueException ex = new InvalidParameterValueException( - "Specified Network id is system only and can't be used for vm deployment"); - ex.addProxyObject(network.getUuid(), "networkId"); - throw ex; - } + Long newAccountId = newAccount.getId(); + SecurityGroup defaultGroup = _securityGroupMgr.getDefaultSecurityGroup(newAccountId); + boolean defaultGroupPresent = false; - if (network.getGuestType() == Network.GuestType.Shared && network.getAclType() == ACLType.Domain) { - NicVO nicOld = _nicDao.findByNtwkIdAndInstanceId(network.getId(), vm.getId()); - if (nicOld != null) { - requestedIPv4ForNics.put(network.getId(), nicOld.getIPv4Address()); - requestedIPv6ForNics.put(network.getId(), nicOld.getIPv6Address()); - logger.debug("AssignVM: use old shared network " + network.getName() + " with old ip " + nicOld.getIPv4Address() + " on nic of vm:" + vm.getInstanceName()); - } - } - logger.debug("AssignVM: Added network " + network.getName() + " to vm " + vm.getId()); - applicableNetworks.add(network); - } - } + if (defaultGroup != null) { + if (securityGroupIdList.contains(defaultGroup.getId())) { + defaultGroupPresent = true; + } + } else { + logger.debug(String.format("Could not find a default security group for account [%s]. Creating a new one.", newAccount)); + defaultGroup = _securityGroupMgr.createSecurityGroup(SecurityGroupManager.DEFAULT_GROUP_NAME, SecurityGroupManager.DEFAULT_GROUP_DESCRIPTION, newAccount.getDomainId(), + newAccountId, newAccount.getAccountName()); + } - // cleanup the network for the oldOwner - _networkMgr.cleanupNics(vmOldProfile); - _networkMgr.removeNics(vmOldProfile); - - // add the new nics - LinkedHashMap> networks = new LinkedHashMap>(); - int toggle = 0; - NetworkVO defaultNetwork = null; - for (NetworkVO appNet : applicableNetworks) { - NicProfile defaultNic = new NicProfile(); - if (toggle == 0) { - defaultNic.setDefaultNic(true); - defaultNetwork = appNet; - toggle++; - } + if (!defaultGroupPresent) { + securityGroupIdList.add(defaultGroup.getId()); + } + } - defaultNic.setRequestedIPv4(requestedIPv4ForNics.get(appNet.getId())); - defaultNic.setRequestedIPv6(requestedIPv6ForNics.get(appNet.getId())); - networks.put(appNet, new ArrayList(Arrays.asList(defaultNic))); + /** + * Attempts to keep the old shared network for the VM to be assigned to a new account by checking if: + * any old shared network exists, + * and the new account can use the old shared network. + * @param vm The VM to be associated to the network. + * @param newAccount The account which has to be able to access the old shared network. + * @param networkIdList The IDs of the networks to be checked for. + * @param applicableNetworks The applicable networks, which will contain the old shared network if applicable. + * @param requestedIPv4ForNics All requested IPv4 for NICs. + * @param requestedIPv6ForNics All requested IPv6 for NICs. + */ + protected void keepOldSharedNetworkForVm(UserVmVO vm, Account newAccount, List networkIdList, Set applicableNetworks, Map requestedIPv4ForNics, + Map requestedIPv6ForNics) { + logger.trace(String.format("Attempting to keep old shared network for VM [%s].", vm)); - } + if (CollectionUtils.isNotEmpty(networkIdList)) { + return; + } - boolean isVmWare = (template.getHypervisorType() == HypervisorType.VMware); - if (securityGroupIdList != null && isVmWare) { - throw new InvalidParameterValueException("Security group feature is not supported for vmWare hypervisor"); - } else if (!isVmWare && (defaultNetwork == null || _networkModel.isSecurityGroupSupportedInNetwork(defaultNetwork)) && _networkModel.canAddDefaultSecurityGroup()) { - if (securityGroupIdList == null) { - securityGroupIdList = new ArrayList(); - } - SecurityGroup defaultGroup = _securityGroupMgr - .getDefaultSecurityGroup(newAccount.getId()); - if (defaultGroup != null) { - // check if security group id list already contains Default - // security group, and if not - add it - boolean defaultGroupPresent = false; - for (Long securityGroupId : securityGroupIdList) { - if (securityGroupId.longValue() == defaultGroup.getId()) { - defaultGroupPresent = true; - break; - } - } + NicVO defaultNicOld = _nicDao.findDefaultNicForVM(vm.getId()); + if (defaultNicOld == null) { + return; + } - if (!defaultGroupPresent) { - securityGroupIdList.add(defaultGroup.getId()); - } + NetworkVO defaultNetworkOld = _networkDao.findById(defaultNicOld.getNetworkId()); + if (canAccountUseNetwork(newAccount, defaultNetworkOld)) { + applicableNetworks.add(defaultNetworkOld); - } else { - // create default security group for the account - if (logger.isDebugEnabled()) { - logger.debug("Couldn't find default security group for the account " - + newAccount + " so creating a new one"); - } - defaultGroup = _securityGroupMgr.createSecurityGroup( - SecurityGroupManager.DEFAULT_GROUP_NAME, - SecurityGroupManager.DEFAULT_GROUP_DESCRIPTION, - newAccount.getDomainId(), newAccount.getId(), - newAccount.getAccountName()); - securityGroupIdList.add(defaultGroup.getId()); - } - } + Long defaultNetworkOldId = defaultNetworkOld.getId(); + requestedIPv4ForNics.put(defaultNetworkOldId, defaultNicOld.getIPv4Address()); + requestedIPv6ForNics.put(defaultNetworkOldId, defaultNicOld.getIPv6Address()); + + logger.debug(String.format("Using old shared network [%s] with old IP [%s] on default NIC of VM [%s].", defaultNicOld.getIPv4Address(), defaultNetworkOld, + vm)); + } + } - VirtualMachine vmi = _itMgr.findById(vm.getId()); - VirtualMachineProfileImpl vmProfile = new VirtualMachineProfileImpl(vmi); + /** + * Adds any additional networks used by the VM assigned to another user. + * If one of the networks does not exist, throws a {@link InvalidParameterValueException}. + * If any of the network offerings is system only, throws a {@link InvalidParameterValueException}. + * @param vm The VM to which the networks are associated to. + * @param newAccount The new account which will access the VM. + * @param networkIdList The list of network IDs to be checked if they can be added to the VM. + * @param applicableNetworks The list of applicable networks to be added to the VM. + * @param requestedIPv4ForNics All requested IPv4 for NICs. + * @param requestedIPv6ForNics All requested IPv6 for NICs. + */ + protected void addAdditionalNetworksToVm(UserVmVO vm, Account newAccount, List networkIdList, Set applicableNetworks, Map requestedIPv4ForNics, + Map requestedIPv6ForNics) { + logger.trace(String.format("Adding additional networks to VM [%s].", vm)); - if (applicableNetworks.isEmpty()) { - throw new InvalidParameterValueException("No network is specified, please specify one when you move the vm. For now, please add a network to VM on NICs tab."); - } else { - _networkMgr.allocate(vmProfile, networks, null); - } + if (CollectionUtils.isEmpty(networkIdList)) { + return; + } + + for (Long networkId : networkIdList) { + NetworkVO network = _networkDao.findById(networkId); + if (network == null) { + InvalidParameterValueException ex = new InvalidParameterValueException("Unable to find specified Network ID."); + ex.addProxyObject(networkId.toString(), "networkId"); + throw ex; + } - _securityGroupMgr.addInstanceToGroups(vm.getId(), - securityGroupIdList); - logger.debug("AssignVM: Advanced zone, adding security groups no " - + securityGroupIdList.size() + " to " - + vm.getInstanceName()); + _networkModel.checkNetworkPermissions(newAccount, network); - } else { - if (securityGroupIdList != null && !securityGroupIdList.isEmpty()) { - throw new InvalidParameterValueException("Can't move vm with security groups; security group feature is not enabled in this zone"); - } - // if networkIdList is null and the first network of vm is shared network, then keep it if possible - if (networkIdList == null || networkIdList.isEmpty()) { - NicVO defaultNicOld = _nicDao.findDefaultNicForVM(vm.getId()); - if (defaultNicOld != null) { - NetworkVO defaultNetworkOld = _networkDao.findById(defaultNicOld.getNetworkId()); - if (canAccountUseNetwork(newAccount, defaultNetworkOld)) { - applicableNetworks.add(defaultNetworkOld); - requestedIPv4ForNics.put(defaultNetworkOld.getId(), defaultNicOld.getIPv4Address()); - requestedIPv6ForNics.put(defaultNetworkOld.getId(), defaultNicOld.getIPv6Address()); - logger.debug("AssignVM: use old shared network " + defaultNetworkOld.getName() + " with old ip " + defaultNicOld.getIPv4Address() + " on default nic of vm:" + vm.getInstanceName()); - } - } + NetworkOffering networkOffering = _entityMgr.findById(NetworkOffering.class, network.getNetworkOfferingId()); + if (networkOffering.isSystemOnly()) { + throw new InvalidParameterValueException(String.format("Specified network [%s] is system only and cannot be used for VM deployment.", network)); + } + + if (network.getGuestType() == Network.GuestType.Shared && network.getAclType() == ACLType.Domain) { + NicVO nicOld = _nicDao.findByNtwkIdAndInstanceId(networkId, vm.getId()); + if (nicOld != null) { + requestedIPv4ForNics.put(networkId, nicOld.getIPv4Address()); + requestedIPv6ForNics.put(networkId, nicOld.getIPv6Address()); + logger.debug(String.format("Using old shared network [%s] with old IP [%s] on NIC of VM [%s].", network, nicOld.getIPv4Address(), vm)); } + } + logger.debug(String.format("Added network [%s] to VM [%s].", network.getName(), vm.getId())); + applicableNetworks.add(network); + } + } - if (networkIdList != null && !networkIdList.isEmpty()) { - // add any additional networks - for (Long networkId : networkIdList) { - NetworkVO network = _networkDao.findById(networkId); - if (network == null) { - InvalidParameterValueException ex = new InvalidParameterValueException("Unable to find specified network id"); - ex.addProxyObject(networkId.toString(), "networkId"); - throw ex; - } + /** + * Attempts to create a network suitable for the creation of a VM ({@link NetworkOrchestrationService#createGuestNetwork(long, String, String, String, String, String, boolean, + * String, Account, Long, PhysicalNetwork, long, ACLType, Boolean, Long, String, String, Boolean, String, Network.PVlanType, String, String, String, String, String, String, + * String, Pair)}). + * If no physical network is found, throws a {@link InvalidParameterValueException}. + * @param caller The account which calls for the network creation. + * @param newAccount The account to which the network will be created. + * @param zone The zone where the network will be created. + * @param requiredOffering The network offering required to create the network. + * @return The NetworkVO for the network created. + * @throws InsufficientCapacityException + * @throws ResourceAllocationException + */ + protected NetworkVO createApplicableNetworkToCreateVm(Account caller, Account newAccount, DataCenterVO zone, NetworkOfferingVO requiredOffering) + throws InsufficientCapacityException, ResourceAllocationException { - _networkModel.checkNetworkPermissions(newAccount, network); + logger.trace("Creating an applicable network to create the VM."); - // don't allow to use system networks - NetworkOffering networkOffering = _entityMgr.findById(NetworkOffering.class, network.getNetworkOfferingId()); - if (networkOffering.isSystemOnly()) { - InvalidParameterValueException ex = new InvalidParameterValueException("Specified Network id is system only and can't be used for vm deployment"); - ex.addProxyObject(network.getUuid(), "networkId"); - throw ex; - } + NetworkVO defaultNetwork; + Long zoneId = zone.getId(); + String requiredOfferingTags = requiredOffering.getTags(); - if (network.getGuestType() == Network.GuestType.Shared && network.getAclType() == ACLType.Domain) { - NicVO nicOld = _nicDao.findByNtwkIdAndInstanceId(network.getId(), vm.getId()); - if (nicOld != null) { - requestedIPv4ForNics.put(network.getId(), nicOld.getIPv4Address()); - requestedIPv6ForNics.put(network.getId(), nicOld.getIPv6Address()); - logger.debug("AssignVM: use old shared network " + network.getName() + " with old ip " + nicOld.getIPv4Address() + " on nic of vm:" + vm.getInstanceName()); - } - } - logger.debug("AssignVM: Added network " + network.getName() + " to vm " + vm.getId()); - applicableNetworks.add(network); - } - } else if (applicableNetworks.isEmpty()) { - NetworkVO defaultNetwork = null; - List requiredOfferings = _networkOfferingDao.listByAvailability(Availability.Required, false); - if (requiredOfferings.size() < 1) { - throw new InvalidParameterValueException("Unable to find network offering with availability=" + Availability.Required - + " to automatically create the network as a part of vm creation"); - } - if (requiredOfferings.get(0).getState() == NetworkOffering.State.Enabled) { - // get Virtual networks - List virtualNetworks = _networkModel.listNetworksForAccount(newAccount.getId(), zone.getId(), Network.GuestType.Isolated); - if (virtualNetworks.isEmpty()) { - long physicalNetworkId = _networkModel.findPhysicalNetworkId(zone.getId(), requiredOfferings.get(0).getTags(), requiredOfferings.get(0) - .getTrafficType()); - // Validate physical network - PhysicalNetwork physicalNetwork = _physicalNetworkDao.findById(physicalNetworkId); - if (physicalNetwork == null) { - throw new InvalidParameterValueException("Unable to find physical network with id: " + physicalNetworkId + " and tag: " - + requiredOfferings.get(0).getTags()); - } - logger.debug("Creating network for account " + newAccount + " from the network offering id=" + requiredOfferings.get(0).getId() - + " as a part of deployVM process"); - Network newNetwork = _networkMgr.createGuestNetwork(requiredOfferings.get(0).getId(), newAccount.getAccountName() + "-network", - newAccount.getAccountName() + "-network", null, null, null, false, null, newAccount, - null, physicalNetwork, zone.getId(), ACLType.Account, null, null, - null, null, true, null, null, null, null, null, null, null, null, null, null); - // if the network offering has persistent set to true, implement the network - if (requiredOfferings.get(0).isPersistent()) { - DeployDestination dest = new DeployDestination(zone, null, null, null); - UserVO callerUser = _userDao.findById(CallContext.current().getCallingUserId()); - Journal journal = new Journal.LogJournal("Implementing " + newNetwork, logger); - ReservationContext context = new ReservationContextImpl(UUID.randomUUID().toString(), journal, callerUser, caller); - logger.debug("Implementing the network for account" + newNetwork + " as a part of" + " network provision for persistent networks"); - try { - Pair implementedNetwork = _networkMgr.implementNetwork(newNetwork.getId(), dest, context); - if (implementedNetwork == null || implementedNetwork.first() == null) { - logger.warn("Failed to implement the network " + newNetwork); - } - newNetwork = implementedNetwork.second(); - } catch (Exception ex) { - logger.warn("Failed to implement network " + newNetwork + " elements and" - + " resources as a part of network provision for persistent network due to ", ex); - CloudRuntimeException e = new CloudRuntimeException("Failed to implement network" - + " (with specified id) elements and resources as a part of network provision"); - e.addProxyObject(newNetwork.getUuid(), "networkId"); - throw e; - } - } - defaultNetwork = _networkDao.findById(newNetwork.getId()); - } else if (virtualNetworks.size() > 1) { - throw new InvalidParameterValueException("More than 1 default Isolated networks are found " + "for account " + newAccount - + "; please specify networkIds"); - } else { - defaultNetwork = _networkDao.findById(virtualNetworks.get(0).getId()); - } - } else { - throw new InvalidParameterValueException("Required network offering id=" + requiredOfferings.get(0).getId() + " is not in " + NetworkOffering.State.Enabled); - } + long physicalNetworkId = _networkModel.findPhysicalNetworkId(zoneId, requiredOfferingTags, requiredOffering.getTrafficType()); - applicableNetworks.add(defaultNetwork); - } + PhysicalNetwork physicalNetwork = _physicalNetworkDao.findById(physicalNetworkId); + if (physicalNetwork == null) { + throw new InvalidParameterValueException(String.format("Unable to find physical network with ID [%s] and tag [%s].", physicalNetworkId, requiredOfferingTags)); + } - // cleanup the network for the oldOwner - _networkMgr.cleanupNics(vmOldProfile); - _networkMgr.removeNics(vmOldProfile); + Long requiredOfferingId = requiredOffering.getId(); + logger.debug(String.format("Creating network for account [%s] from the network offering [%s] as a part of VM deployment process.", newAccount, requiredOfferingId)); - // add the new nics - LinkedHashMap> networks = new LinkedHashMap>(); - int toggle = 0; - for (NetworkVO appNet : applicableNetworks) { - NicProfile defaultNic = new NicProfile(); - if (toggle == 0) { - defaultNic.setDefaultNic(true); - toggle++; - } - defaultNic.setRequestedIPv4(requestedIPv4ForNics.get(appNet.getId())); - defaultNic.setRequestedIPv6(requestedIPv6ForNics.get(appNet.getId())); - networks.put(appNet, new ArrayList(Arrays.asList(defaultNic))); - } - VirtualMachine vmi = _itMgr.findById(vm.getId()); - VirtualMachineProfileImpl vmProfile = new VirtualMachineProfileImpl(vmi); - _networkMgr.allocate(vmProfile, networks, null); - logger.debug("AssignVM: Advance virtual, adding networks no " + networks.size() + " to " + vm.getInstanceName()); - } // END IF NON SEC GRP ENABLED - } // END IF ADVANCED - logger.info("AssignVM: vm " + vm.getInstanceName() + " now belongs to account " + newAccount.getAccountName()); - return vm; + String newAccountName = newAccount.getAccountName(); + Network newNetwork = _networkMgr.createGuestNetwork(requiredOfferingId, newAccountName + "-network", newAccountName + "-network", null, null, null, false, null, + newAccount, null, physicalNetwork, zoneId, ACLType.Account, null, null, null, null, true, null, null, null, null, null, null, null, null, null, null); + + if (requiredOffering.isPersistent()) { + newNetwork = implementNetwork(caller, zone, newNetwork); + } + + defaultNetwork = _networkDao.findById(newNetwork.getId()); + return defaultNetwork; + } + + protected Network implementNetwork(Account caller, DataCenterVO zone, Network newNetwork) { + logger.trace(String.format("Implementing network [%s].", newNetwork)); + + DeployDestination dest = new DeployDestination(zone, null, null, null); + + Journal journal = new Journal.LogJournal("Implementing " + newNetwork, logger); + + UserVO callerUser = _userDao.findById(CallContext.current().getCallingUserId()); + ReservationContext context = new ReservationContextImpl(UUID.randomUUID().toString(), journal, callerUser, caller); + + logger.debug(String.format("Implementing the network for account [%s] as a part of network provision for persistent networks.", newNetwork)); + + try { + Pair implementedNetwork = _networkMgr.implementNetwork(newNetwork.getId(), dest, context); + + if (implementedNetwork == null || implementedNetwork.first() == null || implementedNetwork.second() == null) { + logger.warn(String.format("Failed to implement network [%s].", newNetwork)); + } else { + newNetwork = implementedNetwork.second(); + } + } catch (Exception ex) { + logger.warn(String.format("Failed to implement network [%s] elements and resources as a part of network provision for persistent network due to [%s].", newNetwork, + ex.getMessage()), ex); + throw new CloudRuntimeException(String.format("Failed to implement network [%s] elements and resources as a part of network provision.", newNetwork)); + } + + return newNetwork; } - private boolean canAccountUseNetwork(Account newAccount, Network network) { - if (network != null && network.getAclType() == ACLType.Domain - && (network.getGuestType() == Network.GuestType.Shared - || network.getGuestType() == Network.GuestType.L2)) { + protected boolean canAccountUseNetwork(Account newAccount, Network network) { + if (network != null && network.getAclType() == ACLType.Domain && (network.getGuestType() == Network.GuestType.Shared || network.getGuestType() == Network.GuestType.L2)) { try { _networkModel.checkNetworkPermissions(newAccount, network); return true; } catch (PermissionDeniedException e) { - logger.debug(String.format("AssignVM: %s network %s can not be used by new account %s", network.getGuestType(), network.getName(), newAccount.getAccountName())); + logger.debug(String.format("[%s] network [%s] cannot be used by new account [%s].", network.getGuestType(), network.getName(), newAccount.getAccountName())); return false; } } diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index 323a1ed9416b..b628053554a7 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -40,8 +40,9 @@ import java.util.List; import java.util.Map; -import com.cloud.offering.DiskOffering; +import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.api.BaseCmd.HTTPMethod; +import org.apache.cloudstack.api.command.admin.vm.AssignVMCmd; import org.apache.cloudstack.api.command.user.vm.DeployVMCmd; import org.apache.cloudstack.api.command.user.vm.DeployVnfApplianceCmd; import org.apache.cloudstack.api.command.user.vm.ResetVMUserDataCmd; @@ -91,6 +92,7 @@ import com.cloud.network.dao.NetworkDao; import com.cloud.network.dao.NetworkVO; import com.cloud.network.security.SecurityGroupVO; +import com.cloud.offering.DiskOffering; import com.cloud.offering.ServiceOffering; import com.cloud.server.ManagementService; import com.cloud.service.ServiceOfferingVO; @@ -98,6 +100,7 @@ import com.cloud.storage.DiskOfferingVO; import com.cloud.storage.GuestOSVO; import com.cloud.storage.ScopeType; +import com.cloud.storage.SnapshotVO; import com.cloud.storage.Storage; import com.cloud.storage.VMTemplateVO; import com.cloud.storage.Volume; @@ -130,6 +133,31 @@ import com.cloud.vm.dao.UserVmDetailsDao; import com.cloud.vm.snapshot.VMSnapshotVO; import com.cloud.vm.snapshot.dao.VMSnapshotDao; +import org.mockito.MockedStatic; + +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.LinkedList; +import com.cloud.domain.DomainVO; +import com.cloud.domain.dao.DomainDao; +import com.cloud.event.UsageEventUtils; +import com.cloud.network.Network; +import com.cloud.network.dao.FirewallRulesDao; +import com.cloud.network.dao.IPAddressDao; +import com.cloud.network.dao.IPAddressVO; +import com.cloud.network.dao.LoadBalancerVMMapDao; +import com.cloud.network.dao.LoadBalancerVMMapVO; +import com.cloud.network.dao.PhysicalNetworkDao; +import com.cloud.network.dao.PhysicalNetworkVO; +import com.cloud.network.guru.NetworkGuru; +import com.cloud.network.rules.FirewallRuleVO; +import com.cloud.network.rules.PortForwardingRule; +import com.cloud.network.rules.dao.PortForwardingRulesDao; +import com.cloud.network.security.SecurityGroupManager; +import com.cloud.offering.NetworkOffering; +import com.cloud.offerings.NetworkOfferingVO; +import com.cloud.offerings.dao.NetworkOfferingDao; @RunWith(MockitoJUnitRunner.class) public class UserVmManagerImplTest { @@ -146,6 +174,7 @@ public class UserVmManagerImplTest { @Mock private DataCenterDao _dcDao; + @Mock private DataCenterVO _dcMock; @@ -159,7 +188,7 @@ public class UserVmManagerImplTest { private NetworkOrchestrationService _networkMgr; @Mock - private NetworkVO _networkMock; + private NetworkVO networkMock; @Mock private GuestOSDao guestOSDao; @@ -248,6 +277,72 @@ public class UserVmManagerImplTest { @Mock ManagementService managementServiceMock; + @Mock + AssignVMCmd assignVmCmdMock; + + @Mock + PortForwardingRulesDao portForwardingRulesDaoMock; + + @Mock + List portForwardingRulesListMock; + + @Mock + FirewallRulesDao firewallRulesDaoMock; + + @Mock + List firewallRuleVoListMock; + + @Mock + LoadBalancerVMMapDao loadBalancerVmMapDaoMock; + + @Mock + List loadBalancerVmMapVoListMock; + + @Mock + IPAddressDao ipAddressDaoMock; + + @Mock + IPAddressVO ipAddressVoMock; + + @Mock + VirtualMachineTemplate virtualMachineTemplateMock; + + @Mock + VirtualMachineProfileImpl virtualMachineProfileMock; + + @Mock + List networkVoListMock; + + @Mock + SecurityGroupManager securityGroupManagerMock; + + @Mock + NetworkOfferingDao networkOfferingDaoMock; + + @Mock + NetworkOfferingVO networkOfferingVoMock; + + @Mock + List networkOfferingVoListMock; + + @Mock + PhysicalNetworkDao physicalNetworkDaoMock; + + @Mock + SecurityGroupVO securityGroupVoMock; + + @Mock + DomainDao domainDaoMock; + + @Mock + DomainVO domainVoMock; + + @Mock + SnapshotVO snapshotVoMock; + + @Mock + ServiceOfferingVO serviceOfferingVoMock; + @Mock private ServiceOfferingVO serviceOffering; @@ -283,9 +378,12 @@ public class UserVmManagerImplTest { private DiskOfferingVO smallerDisdkOffering = prepareDiskOffering(5l * GiB_TO_BYTES, 1l, 1L, 2L); private DiskOfferingVO largerDisdkOffering = prepareDiskOffering(10l * GiB_TO_BYTES, 2l, 10L, 20L); + Class expectedInvalidParameterValueException = InvalidParameterValueException.class; + Class expectedCloudRuntimeException = CloudRuntimeException.class; @Before public void beforeTest() { + userVmManagerImpl.resourceLimitService = resourceLimitMgr; Mockito.when(updateVmCommand.getId()).thenReturn(vmId); @@ -373,7 +471,7 @@ public void validateInputsAndPermissionForUpdateVirtualMachineCommandTest() { } @Test - public void updateVirtualMachineTestDisplayChanged() throws ResourceUnavailableException, InsufficientCapacityException { + public void updateVirtualMachineTestDisplayChanged() throws ResourceUnavailableException, InsufficientCapacityException, ResourceAllocationException { configureDoNothingForMethodsThatWeDoNotWantToTest(); ServiceOffering offering = getSvcoffering(512); Mockito.when(_serviceOfferingDao.findById(Mockito.anyLong(), Mockito.anyLong())).thenReturn((ServiceOfferingVO) offering); @@ -388,7 +486,7 @@ public void updateVirtualMachineTestDisplayChanged() throws ResourceUnavailableE } @Test - public void updateVirtualMachineTestCleanUpTrue() throws ResourceUnavailableException, InsufficientCapacityException { + public void updateVirtualMachineTestCleanUpTrue() throws ResourceUnavailableException, InsufficientCapacityException, ResourceAllocationException { configureDoNothingForMethodsThatWeDoNotWantToTest(); ServiceOffering offering = getSvcoffering(512); Mockito.when(_serviceOfferingDao.findById(Mockito.anyLong(), Mockito.anyLong())).thenReturn((ServiceOfferingVO) offering); @@ -407,23 +505,23 @@ public void updateVirtualMachineTestCleanUpTrue() throws ResourceUnavailableExce } @Test - public void updateVirtualMachineTestCleanUpTrueAndDetailEmpty() throws ResourceUnavailableException, InsufficientCapacityException { + public void updateVirtualMachineTestCleanUpTrueAndDetailEmpty() throws ResourceUnavailableException, InsufficientCapacityException, ResourceAllocationException { prepareAndExecuteMethodDealingWithDetails(true, true); } @Test - public void updateVirtualMachineTestCleanUpTrueAndDetailsNotEmpty() throws ResourceUnavailableException, InsufficientCapacityException { + public void updateVirtualMachineTestCleanUpTrueAndDetailsNotEmpty() throws ResourceUnavailableException, InsufficientCapacityException, ResourceAllocationException { prepareAndExecuteMethodDealingWithDetails(true, false); } @Test - public void updateVirtualMachineTestCleanUpFalseAndDetailsNotEmpty() throws ResourceUnavailableException, InsufficientCapacityException { + public void updateVirtualMachineTestCleanUpFalseAndDetailsNotEmpty() throws ResourceUnavailableException, InsufficientCapacityException, ResourceAllocationException { prepareAndExecuteMethodDealingWithDetails(false, true); } @Test - public void updateVirtualMachineTestCleanUpFalseAndDetailsEmpty() throws ResourceUnavailableException, InsufficientCapacityException { - Mockito.when(accountDao.findById(Mockito.anyLong())).thenReturn(callerAccount); + public void updateVirtualMachineTestCleanUpFalseAndDetailsEmpty() throws ResourceUnavailableException, InsufficientCapacityException, ResourceAllocationException { + Mockito.doNothing().when(userVmManagerImpl).verifyVmLimits(Mockito.any(), Mockito.anyMap()); prepareAndExecuteMethodDealingWithDetails(false, false); } @@ -437,12 +535,11 @@ private List prepareExistingDetails(Long vmId, String... existin return existingDetails; } - private void prepareAndExecuteMethodDealingWithDetails(boolean cleanUpDetails, boolean isDetailsEmpty) throws ResourceUnavailableException, InsufficientCapacityException { + private void prepareAndExecuteMethodDealingWithDetails(boolean cleanUpDetails, boolean isDetailsEmpty) throws ResourceUnavailableException, InsufficientCapacityException, ResourceAllocationException { configureDoNothingForMethodsThatWeDoNotWantToTest(); ServiceOffering offering = getSvcoffering(512); Mockito.when(_serviceOfferingDao.findById(Mockito.anyLong(), Mockito.anyLong())).thenReturn((ServiceOfferingVO) offering); - Mockito.when(_serviceOfferingDao.findByIdIncludingRemoved(Mockito.anyLong(), Mockito.anyLong())).thenReturn((ServiceOfferingVO) offering); ServiceOfferingVO currentServiceOffering = Mockito.mock(ServiceOfferingVO.class); Mockito.lenient().when(currentServiceOffering.getCpu()).thenReturn(1); Mockito.lenient().when(currentServiceOffering.getRamSize()).thenReturn(512); @@ -453,7 +550,7 @@ private void prepareAndExecuteMethodDealingWithDetails(boolean cleanUpDetails, b nics.add(nic1); nics.add(nic2); when(this.nicDao.listByVmId(Mockito.anyLong())).thenReturn(nics); - when(_networkDao.findById(anyLong())).thenReturn(_networkMock); + when(_networkDao.findById(anyLong())).thenReturn(networkMock); lenient().doNothing().when(_networkMgr).saveExtraDhcpOptions(anyString(), anyLong(), anyMap()); HashMap details = new HashMap<>(); if(!isDetailsEmpty) { @@ -494,13 +591,24 @@ private void verifyMethodsThatAreAlwaysExecuted() throws ResourceUnavailableExce } @SuppressWarnings("unchecked") - private void configureDoNothingForMethodsThatWeDoNotWantToTest() throws ResourceUnavailableException, InsufficientCapacityException { + private void configureDoNothingForMethodsThatWeDoNotWantToTest() throws ResourceUnavailableException, InsufficientCapacityException, ResourceAllocationException { Mockito.doNothing().when(userVmManagerImpl).validateInputsAndPermissionForUpdateVirtualMachineCommand(updateVmCommand); Mockito.doReturn(new ArrayList()).when(userVmManagerImpl).getSecurityGroupIdList(updateVmCommand); - Mockito.lenient().doReturn(Mockito.mock(UserVm.class)).when(userVmManagerImpl).updateVirtualMachine(Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), Mockito.anyBoolean(), - Mockito.anyBoolean(), Mockito.anyLong(), - Mockito.anyString(), Mockito.anyLong(), Mockito.anyString(), Mockito.anyBoolean(), Mockito.any(HTTPMethod.class), Mockito.anyString(), Mockito.anyString(), Mockito.anyString(), Mockito.anyList(), - Mockito.anyMap()); + Mockito.lenient().doReturn(Mockito.mock(UserVm.class)).when(userVmManagerImpl).updateVirtualMachine(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.anyBoolean(), + Mockito.anyBoolean(), Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(HTTPMethod.class), Mockito.any(), Mockito.any(), + Mockito.any(), Mockito.anyList(), Mockito.any()); + Mockito.doNothing().when(userVmManagerImpl).validateIfVmExistsAndIsNotRunning(Mockito.any(), Mockito.anyLong()); + Mockito.doNothing().when(userVmManagerImpl).validateOldAndNewAccounts(Mockito.nullable(Account.class), Mockito.nullable(Account.class), Mockito.anyLong(), Mockito.nullable(String.class), Mockito.nullable(Long.class)); + Mockito.doNothing().when(userVmManagerImpl).validateIfVmHasNoRules(Mockito.any(), Mockito.anyLong()); + Mockito.doNothing().when(userVmManagerImpl).removeInstanceFromInstanceGroup(Mockito.anyLong()); + Mockito.doNothing().when(userVmManagerImpl).verifyResourceLimitsForAccountAndStorage(Mockito.any(), Mockito.any(), Mockito.anyLong(), Mockito.any(), Mockito.anyList(), Mockito.any()); + Mockito.doNothing().when(userVmManagerImpl).validateIfNewOwnerHasAccessToTemplate(Mockito.any(), Mockito.any(), Mockito.any()); + + Mockito.doNothing().when(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); + Mockito.doNothing().when(userVmManagerImpl).updateVolumesOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); + Mockito.doNothing().when(userVmManagerImpl).updateVmNetwork(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); + + Mockito.doNothing().when(userVmManagerImpl).resourceCountIncrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any()); } @Test @@ -606,7 +714,7 @@ public void configureCustomRootDiskSizeTestEmptyParameters() { public void configureCustomRootDiskSizeTestEmptyParametersAndOfferingRootSize() { Map customParameters = new HashMap<>(); long expectedRootDiskSize = 10l * GiB_TO_BYTES; - long offeringRootDiskSize = 10l * GiB_TO_BYTES;; + long offeringRootDiskSize = 10l * GiB_TO_BYTES; prepareAndRunConfigureCustomRootDiskSizeTest(customParameters, expectedRootDiskSize, 1, offeringRootDiskSize); } @@ -1509,7 +1617,6 @@ public void updateInstanceDetailsMapWithCurrentValuesForAbsentDetailsTestAllCons @Test public void testCheckVolumesLimits() { - userVmManagerImpl.resourceLimitService = resourceLimitMgr; long diskOffId1 = 1L; DiskOfferingVO diskOfferingVO1 = Mockito.mock(DiskOfferingVO.class); Mockito.when(diskOfferingDao.findById(diskOffId1)).thenReturn(diskOfferingVO1); @@ -1597,4 +1704,1367 @@ public void testGetRootVolumeSizeForVmRestoreNullDiskOfferingAndEmptyDetails() { Long actualSize = userVmManagerImpl.getRootVolumeSizeForVmRestore(null, template, userVm, diskOffering, details, false); Assert.assertEquals(20 * GiB_TO_BYTES, actualSize.longValue()); } + + @Test + public void validateIfVmExistsAndIsNotRunningTestVmIsNullThrowsInvalidParameterValueException() { + String expectedMessage = String.format("There is no VM by ID [%s].", 1l); + + InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { + userVmManagerImpl.validateIfVmExistsAndIsNotRunning(null, 1l); + }); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void validateIfVmExistsAndIsNotRunningTestVmIsRunningThrowsInvalidParameterValueException() { + String expectedMessage = String.format("Unable to move VM [%s] in [%s] state.", userVmVoMock, VirtualMachine.State.Running); + Mockito.doReturn(VirtualMachine.State.Running).when(userVmVoMock).getState(); + + InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { + userVmManagerImpl.validateIfVmExistsAndIsNotRunning(userVmVoMock, 1l); + }); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void validateIfVmExistsAndIsNotRunningTestVmIsNotRunningDoesNotThrowInvalidParameterValueException() { + userVmManagerImpl.validateIfVmExistsAndIsNotRunning(userVmVoMock, 1l); + } + + @Test + public void validateOldAndNewAccountsTestBothAreValidDoNothing() { + Account newAccount = Mockito.mock(Account.class); + Mockito.doReturn(1l).when(newAccount).getAccountId(); + + userVmManagerImpl.validateOldAndNewAccounts(accountMock, newAccount, 1L, "", 1L); + } + + @Test + public void validateOldAndNewAccountsTestOldAccountIsNullThrowsInvalidParameterValueException() { + String expectedMessage = String.format("Invalid old account [%s] for VM in domain [%s].", userVmVoMock.getAccountId(), assignVmCmdMock.getDomainId()); + + InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { + userVmManagerImpl.validateOldAndNewAccounts(null, accountMock, userVmVoMock.getAccountId(), "", assignVmCmdMock.getDomainId()); + }); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void validateOldAndNewAccountsTestNewAccountIsNullThrowsInvalidParameterValueException() { + String expectedMessage = String.format("Invalid new account [%s] for VM in domain [%s].", assignVmCmdMock.getAccountName(), assignVmCmdMock.getDomainId()); + + InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { + userVmManagerImpl.validateOldAndNewAccounts(accountMock, null, 1l, assignVmCmdMock.getAccountName(), assignVmCmdMock.getDomainId()); + }); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void validateOldAndNewAccountsTestNewAccountStateIsDisabledThrowsInvalidParameterValueException() { + String expectedMessage = String.format("The new account owner [%s] is disabled.", accountMock.toString()); + + Mockito.doReturn(Account.State.DISABLED).when(accountMock).getState(); + + InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { + userVmManagerImpl.validateOldAndNewAccounts(accountMock, accountMock, 1l, "", 1l); + }); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void validateOldAndNewAccountsTestOldAccountIsTheSameAsNewAccountThrowsInvalidParameterValueException() { + String expectedMessage = String.format("The new account [%s] is the same as the old account.", accountMock.toString()); + + Mockito.doReturn(Account.State.ENABLED).when(accountMock).getState(); + + InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { + userVmManagerImpl.validateOldAndNewAccounts(accountMock, accountMock, 1l, "", 1l); + }); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void validateOldAndNewAccountsTestOldAccountIsNotTheSameAsNewAccountDoesNotThrowInvalidParameterValueException() { + AccountVO oldAccount = new AccountVO(); + Mockito.doReturn(1l).when(accountMock).getAccountId(); + + userVmManagerImpl.validateOldAndNewAccounts(oldAccount, accountMock, 1l, "", 1l); + } + + @Test + public void checkCallerAccessToAccountsTestCallsCheckAccessToOldAccountAndNewAccount() { + AccountVO oldAccount = new AccountVO(); + + userVmManagerImpl.checkCallerAccessToAccounts(callerAccount, oldAccount, accountMock); + + Mockito.verify(accountManager).checkAccess(callerAccount, null, true, oldAccount); + Mockito.verify(accountManager).checkAccess(callerAccount, null, true, accountMock); + } + + @Test + public void validateIfVmHasNoRulesTestPortForwardingRulesExistThrowsInvalidParameterValueException() { + String expectedMessage = String.format("Remove any Port Forwarding rules for VM [%s] before assigning it to another user.", userVmVoMock); + + Mockito.doReturn(portForwardingRulesListMock).when(portForwardingRulesDaoMock).listByVm(Mockito.anyLong()); + + InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { + userVmManagerImpl.validateIfVmHasNoRules(userVmVoMock, 1l); + }); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void validateIfVmHasNoRulesTestStaticNatRulesExistThrowsInvalidParameterValueException() { + String expectedMessage = String.format("Remove the StaticNat rules for VM [%s] before assigning it to another user.", userVmVoMock); + + Mockito.doReturn(firewallRuleVoListMock).when(firewallRulesDaoMock).listStaticNatByVmId(Mockito.anyLong()); + + InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { + userVmManagerImpl.validateIfVmHasNoRules(userVmVoMock, 1l); + }); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void validateIfVmHasNoRulesTestLoadBalancingRulesExistThrowsInvalidParameterValueException() { + String expectedMessage = String.format("Remove the Load Balancing rules for VM [%s] before assigning it to another user.", userVmVoMock); + + Mockito.doReturn(loadBalancerVmMapVoListMock).when(loadBalancerVmMapDaoMock).listByInstanceId(Mockito.anyLong()); + + InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { + userVmManagerImpl.validateIfVmHasNoRules(userVmVoMock, 1l); + }); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void validateIfVmHasNoRulesTestOneToOneNatRulesExistThrowsInvalidParameterValueException() { + String expectedMessage = String.format("Remove the One to One Nat rule for VM [%s] for IP [%s].", userVmVoMock, ipAddressVoMock.toString()); + + LinkedList ipAddressVoList = new LinkedList(); + + Mockito.doReturn(ipAddressVoList).when(ipAddressDaoMock).findAllByAssociatedVmId(Mockito.anyLong()); + ipAddressVoList.add(ipAddressVoMock); + Mockito.doReturn(true).when(ipAddressVoMock).isOneToOneNat(); + + InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { + userVmManagerImpl.validateIfVmHasNoRules(userVmVoMock, 1l); + }); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void validateIfVmHasNoRulesTestOneToOneNatRulesDoNotExistDoesNotThrowInvalidParameterValueException() { + userVmManagerImpl.validateIfVmHasNoRules(userVmVoMock, 1l); + } + + @Test + public void verifyResourceLimitsForAccountAndStorageTestDoesNotCallResourceLimitCheck() throws ResourceAllocationException { + LinkedList volumeVoList = new LinkedList(); + + userVmManagerImpl.verifyResourceLimitsForAccountAndStorage(accountMock, userVmVoMock, 1l, serviceOfferingVoMock, volumeVoList, virtualMachineTemplateMock); + + Mockito.verify(resourceLimitMgr, Mockito.never()).checkVmResourceLimit(Mockito.any(), Mockito.anyBoolean(), Mockito.any(), Mockito.any()); + + Mockito.verify(resourceLimitMgr).checkResourceLimit(accountMock, Resource.ResourceType.volume, 0l); + Mockito.verify(resourceLimitMgr).checkResourceLimit(accountMock, Resource.ResourceType.primary_storage, 0l); + } + + @Test + public void validateIfNewOwnerHasAccessToTemplateTestTemplateIsNullThrowsInvalidParameterValueException() { + String expectedMessage = String.format("Template for VM [%s] cannot be found.", userVmVoMock.getUuid()); + + InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { + userVmManagerImpl.validateIfNewOwnerHasAccessToTemplate(userVmVoMock, accountMock, null); + }); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void validateIfNewOwnerHasAccessToTemplateTestCallCheckAccessWhenTemplateIsNotPublic() { + Mockito.doReturn(accountMock).when(accountManager).getAccount(Mockito.anyLong()); + + userVmManagerImpl.validateIfNewOwnerHasAccessToTemplate(userVmVoMock, accountMock, virtualMachineTemplateMock); + + Mockito.verify(accountManager).checkAccess(accountMock, null, true, accountMock); + } + + @Test + public void updateVmOwnerTestCallsSetAccountIdSetDomainIdAndPersist() { + userVmManagerImpl.updateVmOwner(accountMock, userVmVoMock, 1l, 1l); + + Mockito.verify(userVmVoMock).setAccountId(Mockito.anyLong()); + Mockito.verify(userVmVoMock).setDomainId(Mockito.anyLong()); + Mockito.verify(userVmDao).persist(userVmVoMock); + } + + @Test + public void updateVmNetworkTestCallsUpdateBasicTypeNetworkForVmIfBasicTypeZone() throws InsufficientCapacityException, ResourceAllocationException { + Mockito.doReturn(_dcMock).when(_dcDao).findById(Mockito.anyLong()); + Mockito.doReturn(DataCenter.NetworkType.Basic).when(_dcMock).getNetworkType(); + Mockito.doNothing().when(userVmManagerImpl).updateBasicTypeNetworkForVm(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), + Mockito.any(), Mockito.any()); + + userVmManagerImpl.updateVmNetwork(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock); + + Mockito.verify(userVmManagerImpl).updateBasicTypeNetworkForVm(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), + Mockito.any(), Mockito.any()); + } + + @Test + public void updateVmNetworkTestCallsUpdateAdvancedTypeNetworkForVmIfNotBasicTypeZone() throws InsufficientCapacityException, ResourceAllocationException { + Mockito.doReturn(_dcMock).when(_dcDao).findById(Mockito.anyLong()); + Mockito.doReturn(DataCenter.NetworkType.Advanced).when(_dcMock).getNetworkType(); + Mockito.doNothing().when(userVmManagerImpl).updateAdvancedTypeNetworkForVm(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), + Mockito.any(), Mockito.any(), Mockito.any()); + + userVmManagerImpl.updateVmNetwork(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock); + + Mockito.verify(userVmManagerImpl).updateAdvancedTypeNetworkForVm(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), + Mockito.any(), Mockito.any(), Mockito.any()); + } + + @Test + public void cleanupOfOldOwnerNicsForNetworkTestCallsCleanupNicsAndRemoveNics() { + userVmManagerImpl.cleanupOfOldOwnerNicsForNetwork(virtualMachineProfileMock); + + Mockito.verify(_networkMgr).cleanupNics(virtualMachineProfileMock); + Mockito.verify(_networkMgr).removeNics(virtualMachineProfileMock); + } + + @Test + public void addDefaultNetworkToNetworkListTestDefaultNetworkIsNullThrowsInvalidParameterValueException() { + String expectedMessage = "Unable to find a default network to start a VM."; + + InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { + userVmManagerImpl.addDefaultNetworkToNetworkList(networkVoListMock, null); + }); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void addDefaultNetworkToNetworkListTestDefaultNetworkIsNotNullAddNetworkToNetworkList() { + userVmManagerImpl.addDefaultNetworkToNetworkList(networkVoListMock, networkMock); + + Mockito.verify(networkVoListMock).add(Mockito.any()); + } + + @Test + public void allocateNetworksForVmTestCallsNetworkManagerAllocate() throws InsufficientCapacityException { + LinkedHashMap> networks = new LinkedHashMap>(); + + Mockito.doReturn(userVmVoMock).when(virtualMachineManager).findById(Mockito.anyLong()); + + userVmManagerImpl.allocateNetworksForVm(userVmVoMock, networks); + + Mockito.verify(_networkMgr).allocate(Mockito.any(), Mockito.any(), Mockito.any()); + } + + @Test + public void addSecurityGroupsToVmTestIsVmWareAndSecurityGroupIdListIsNotNullThrowsInvalidParameterValueException() { + String expectedMessage = "Security group feature is not supported for VMWare hypervisor."; + LinkedList securityGroupIdList = new LinkedList(); + + Mockito.doReturn(Hypervisor.HypervisorType.VMware).when(virtualMachineTemplateMock).getHypervisorType(); + + InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { + userVmManagerImpl.addSecurityGroupsToVm(accountMock, userVmVoMock, virtualMachineTemplateMock, securityGroupIdList, networkMock); + }); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void addSecurityGroupsToVmTestIsNotVmWareDefaultNetworkIsNullAndNetworkModelCanAddDefaultSecurityGroupCallsAddDefaultSecurityGroupToSecurityGroupIdList() { + LinkedList securityGroupIdList = new LinkedList(); + + Mockito.doReturn(Hypervisor.HypervisorType.KVM).when(virtualMachineTemplateMock).getHypervisorType(); + Mockito.doReturn(true).when(networkModel).canAddDefaultSecurityGroup(); + Mockito.doReturn(securityGroupVoMock).when(securityGroupManagerMock).getDefaultSecurityGroup(Mockito.anyLong()); + + userVmManagerImpl.addSecurityGroupsToVm(accountMock, userVmVoMock, virtualMachineTemplateMock, securityGroupIdList, null); + + Mockito.verify(userVmManagerImpl).addDefaultSecurityGroupToSecurityGroupIdList(accountMock, securityGroupIdList); + Mockito.verify(securityGroupManagerMock).addInstanceToGroups(Mockito.anyLong(), Mockito.any()); + } + + @Test + public void addNetworksToNetworkIdListTestCallsKeepOldSharedNetworkForVmAddAdditionalNetworksToVmAndCleanupOfOldOwnerNicsForNetwork() { + LinkedList networkIdList = new LinkedList(); + HashSet applicableNetworks = new HashSet(); + HashMap requestedIPv4ForNics = new HashMap(); + HashMap requestedIPv6ForNics = new HashMap(); + + userVmManagerImpl.addNetworksToNetworkIdList(userVmVoMock, accountMock, virtualMachineProfileMock, networkIdList, applicableNetworks, requestedIPv4ForNics, + requestedIPv6ForNics); + + Mockito.verify(userVmManagerImpl).keepOldSharedNetworkForVm(userVmVoMock, accountMock, networkIdList, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); + Mockito.verify(userVmManagerImpl).addAdditionalNetworksToVm(userVmVoMock, accountMock, networkIdList, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); + Mockito.verify(userVmManagerImpl).cleanupOfOldOwnerNicsForNetwork(virtualMachineProfileMock); + } + + @Test + public void selectApplicableNetworkToCreateVmTestRequiredOfferingsListHasNoOfferingsThrowsInvalidParameterValueException() { + String expectedMessage = String.format("Unable to find network offering with availability [%s] to automatically create the network as a part of VM creation.", + NetworkOffering.Availability.Required); + HashSet applicableNetworks = new HashSet(); + LinkedList requiredOfferings = new LinkedList(); + + Mockito.doReturn(requiredOfferings).when(networkOfferingDaoMock).listByAvailability(NetworkOffering.Availability.Required, false); + + InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { + userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks); + }); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void selectApplicableNetworkToCreateVmTestFirstOfferingIsNotEnabledThrowsInvalidParameterValueException() { + String expectedMessage = String.format("Required network offering ID [%s] is not in [%s] state.", 1l, NetworkOffering.State.Enabled); + HashSet applicableNetworks = new HashSet(); + + Mockito.doReturn(networkOfferingVoListMock).when(networkOfferingDaoMock).listByAvailability(NetworkOffering.Availability.Required, false); + Mockito.doReturn(networkOfferingVoMock).when(networkOfferingVoListMock).get(0); + + Mockito.doReturn(NetworkOffering.State.Disabled).when(networkOfferingVoMock).getState(); + + Mockito.doReturn(1l).when(networkOfferingVoMock).getId(); + + InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { + userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks); + }); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void selectApplicableNetworkToCreateVmTestVirtualNetworkIsEmptyCallsCreateApplicableNetworkToCreateVm() throws InsufficientCapacityException, + ResourceAllocationException { + + HashSet applicableNetworks = new HashSet(); + LinkedList virtualNetworks = new LinkedList<>(); + + Mockito.doReturn(networkOfferingVoListMock).when(networkOfferingDaoMock).listByAvailability(NetworkOffering.Availability.Required, false); + Mockito.doReturn(networkOfferingVoMock).when(networkOfferingVoListMock).get(0); + + Mockito.doReturn(virtualNetworks).when(networkModel).listNetworksForAccount(Mockito.anyLong(), Mockito.anyLong(), Mockito.any()); + Mockito.doReturn(NetworkOffering.State.Enabled).when(networkOfferingVoMock).getState(); + + Mockito.doReturn(networkMock).when(userVmManagerImpl).createApplicableNetworkToCreateVm(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); + + userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks); + + Mockito.verify(userVmManagerImpl).createApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, networkOfferingVoMock); + } + + @Test + public void selectApplicableNetworkToCreateVmTestVirtualNetworkHasMultipleNetworksThrowsInvalidParameterValueException() { + String expectedMessage = String.format("More than one default isolated network has been found for account [%s]; please specify networkIDs.", accountMock.toString()); + HashSet applicableNetworks = new HashSet(); + LinkedList virtualNetworks = new LinkedList(); + + Mockito.doReturn(networkOfferingVoListMock).when(networkOfferingDaoMock).listByAvailability(NetworkOffering.Availability.Required, false); + Mockito.doReturn(networkOfferingVoMock).when(networkOfferingVoListMock).get(0); + + Mockito.doReturn(virtualNetworks).when(networkModel).listNetworksForAccount(Mockito.anyLong(), Mockito.anyLong(), Mockito.any()); + Mockito.doReturn(NetworkOffering.State.Enabled).when(networkOfferingVoMock).getState(); + + virtualNetworks.add(networkMock); + virtualNetworks.add(networkMock); + + InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { + userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks); + }); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void selectApplicableNetworkToCreateVmTestVirtualNetworkHasOneNetworkCallsNetworkDaoFindById() throws InsufficientCapacityException, ResourceAllocationException { + HashSet applicableNetworks = new HashSet(); + + Mockito.doReturn(networkOfferingVoListMock).when(networkOfferingDaoMock).listByAvailability(NetworkOffering.Availability.Required, false); + Mockito.doReturn(networkOfferingVoMock).when(networkOfferingVoListMock).get(0); + + Mockito.doReturn(networkVoListMock).when(networkModel).listNetworksForAccount(Mockito.anyLong(), Mockito.anyLong(), Mockito.any()); + Mockito.doReturn(NetworkOffering.State.Enabled).when(networkOfferingVoMock).getState(); + + Mockito.doReturn(false).when(networkVoListMock).isEmpty(); + Mockito.doReturn(1).when(networkVoListMock).size(); + Mockito.doReturn(networkMock).when(networkVoListMock).get(0); + + userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks); + + Mockito.verify(_networkDao).findById(Mockito.anyLong()); + } + + @Test + public void addDefaultSecurityGroupToSecurityGroupIdListTestDefaultGroupIsNullCallsCreateSecurityGroup() { + String expected = ""; + LinkedList securityGroupIdList = Mockito.spy(new LinkedList()); + + Mockito.doReturn(null).when(securityGroupManagerMock).getDefaultSecurityGroup(Mockito.anyLong()); + Mockito.doReturn(securityGroupVoMock).when(securityGroupManagerMock).createSecurityGroup(SecurityGroupManager.DEFAULT_GROUP_NAME, + SecurityGroupManager.DEFAULT_GROUP_DESCRIPTION, 1l, 1l, expected); + + Mockito.doReturn(1l).when(accountMock).getDomainId(); + Mockito.doReturn(1l).when(accountMock).getId(); + Mockito.doReturn(expected).when(accountMock).getAccountName(); + Mockito.doReturn(1l).when(securityGroupVoMock).getId(); + + userVmManagerImpl.addDefaultSecurityGroupToSecurityGroupIdList(accountMock, securityGroupIdList); + + Mockito.verify(securityGroupManagerMock).createSecurityGroup(SecurityGroupManager.DEFAULT_GROUP_NAME, SecurityGroupManager.DEFAULT_GROUP_DESCRIPTION, 1l, 1l, expected); + Mockito.verify(securityGroupIdList).add(1l); + } + + @Test + public void addDefaultSecurityGroupToSecurityGroupIdListTestDefaultGroupIsPresentDoesNotCallAddIdToSecurityGroupIdList() { + LinkedList securityGroupIdList = Mockito.spy(new LinkedList()); + + securityGroupIdList.addFirst(1l); + Mockito.doReturn(securityGroupVoMock).when(securityGroupManagerMock).getDefaultSecurityGroup(Mockito.anyLong()); + Mockito.doReturn(1l).when(securityGroupVoMock).getId(); + + userVmManagerImpl.addDefaultSecurityGroupToSecurityGroupIdList(accountMock, securityGroupIdList); + + Mockito.verify(securityGroupIdList, Mockito.never()).add(Mockito.anyLong()); + } + + @Test + public void addDefaultSecurityGroupToSecurityGroupIdListTestDefaultGroupIsNotPresentCallsAddIdToSecurityGroupIdList() { + LinkedList securityGroupIdList = Mockito.spy(new LinkedList()); + + Mockito.doReturn(securityGroupVoMock).when(securityGroupManagerMock).getDefaultSecurityGroup(Mockito.anyLong()); + Mockito.doReturn(1l).when(securityGroupVoMock).getId(); + + userVmManagerImpl.addDefaultSecurityGroupToSecurityGroupIdList(accountMock, securityGroupIdList); + + Mockito.verify(securityGroupIdList).add(1l); + } + + @Test + public void keepOldSharedNetworkForVmTestNetworkIdListIsNotNullOrEmptyDoesNotCallFindDefaultNicForVm() { + LinkedList networkIdList = new LinkedList(); + HashSet applicableNetworks = new HashSet(); + HashMap requestedIPv4ForNics = new HashMap(); + HashMap requestedIPv6ForNics = new HashMap(); + + networkIdList.add(1l); + + userVmManagerImpl.keepOldSharedNetworkForVm(userVmVoMock, accountMock, networkIdList, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); + + Mockito.verify(nicDao, Mockito.never()).findDefaultNicForVM(Mockito.anyLong()); + } + + @Test + public void keepOldSharedNetworkForVmTestNetworkIdListIsNullCallsFindDefaultNicForVm() { + HashSet applicableNetworks = new HashSet(); + HashMap requestedIPv4ForNics = new HashMap(); + HashMap requestedIPv6ForNics = new HashMap(); + + userVmManagerImpl.keepOldSharedNetworkForVm(userVmVoMock, accountMock, null, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); + + Mockito.verify(nicDao).findDefaultNicForVM(Mockito.anyLong()); + } + + @Test + public void keepOldSharedNetworkForVmTestNetworkIdListIsEmptyCallsFindDefaultNicForVm() { + LinkedList networkIdList = new LinkedList(); + HashSet applicableNetworks = new HashSet(); + HashMap requestedIPv4ForNics = new HashMap(); + HashMap requestedIPv6ForNics = new HashMap(); + + userVmManagerImpl.keepOldSharedNetworkForVm(userVmVoMock, accountMock, networkIdList, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); + + Mockito.verify(nicDao).findDefaultNicForVM(Mockito.anyLong()); + } + + @Test + public void keepOldSharedNetworkForVmTestDefaultNicOldIsNullDoesNotCallNetworkDaoFindById() { + HashSet applicableNetworks = new HashSet(); + HashMap requestedIPv4ForNics = new HashMap(); + HashMap requestedIPv6ForNics = new HashMap(); + + Mockito.doReturn(null).when(nicDao).findDefaultNicForVM(Mockito.anyLong()); + + userVmManagerImpl.keepOldSharedNetworkForVm(userVmVoMock, accountMock, null, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); + + Mockito.verify(_networkDao, Mockito.never()).findById(Mockito.anyLong()); + } + + @Test + public void keepOldSharedNetworkForVmTestDefaultNicOldIsNotNullCallsNetworkDaoFindById() { + HashSet applicableNetworks = new HashSet(); + HashMap requestedIPv4ForNics = new HashMap(); + HashMap requestedIPv6ForNics = new HashMap(); + + Mockito.doReturn(new NicVO()).when(nicDao).findDefaultNicForVM(Mockito.anyLong()); + + userVmManagerImpl.keepOldSharedNetworkForVm(userVmVoMock, accountMock, null, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); + + Mockito.verify(_networkDao).findById(Mockito.anyLong()); + } + + @Test + public void keepOldSharedNetworkForVmTestAccountCanNotUseNetworkDoesNotAddNetworkToApplicableNetworks() { + HashSet applicableNetworks = Mockito.spy(new HashSet()); + HashMap requestedIPv4ForNics = new HashMap(); + HashMap requestedIPv6ForNics = new HashMap(); + + Mockito.doReturn(new NicVO()).when(nicDao).findDefaultNicForVM(Mockito.anyLong()); + Mockito.doReturn(networkMock).when(_networkDao).findById(Mockito.anyLong()); + Mockito.doReturn(false).when(userVmManagerImpl).canAccountUseNetwork(accountMock, networkMock); + + userVmManagerImpl.keepOldSharedNetworkForVm(userVmVoMock, accountMock, null, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); + + Mockito.verify(applicableNetworks, Mockito.never()).add(Mockito.any()); + } + + @Test + public void keepOldSharedNetworkForVmTestAccountCanUseNetworkAddsNetworkToApplicableNetworks() { + HashSet applicableNetworks = Mockito.spy(new HashSet()); + HashMap requestedIPv4ForNics = new HashMap(); + HashMap requestedIPv6ForNics = new HashMap(); + + Mockito.doReturn(new NicVO()).when(nicDao).findDefaultNicForVM(Mockito.anyLong()); + Mockito.doReturn(networkMock).when(_networkDao).findById(Mockito.anyLong()); + Mockito.doReturn(true).when(userVmManagerImpl).canAccountUseNetwork(accountMock, networkMock); + + userVmManagerImpl.keepOldSharedNetworkForVm(userVmVoMock, accountMock, null, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); + + Mockito.verify(applicableNetworks).add(Mockito.any()); + } + + @Test + public void addAdditionalNetworksToVmTestNetworkIdListIsNullDoesNotCallCheckNetworkPermissions() { + HashSet applicableNetworks = Mockito.spy(new HashSet()); + HashMap requestedIPv4ForNics = new HashMap(); + HashMap requestedIPv6ForNics = new HashMap(); + + userVmManagerImpl.addAdditionalNetworksToVm(userVmVoMock, accountMock, null, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); + + Mockito.verify(networkModel, Mockito.never()).checkNetworkPermissions(Mockito.any(), Mockito.any()); + } + + @Test + public void addAdditionalNetworksToVmTestNetworkIdListIsEmptyDoesNotCallCheckNetworkPermissions() { + LinkedList networkIdList = new LinkedList(); + HashSet applicableNetworks = Mockito.spy(new HashSet()); + HashMap requestedIPv4ForNics = new HashMap(); + HashMap requestedIPv6ForNics = new HashMap(); + + userVmManagerImpl.addAdditionalNetworksToVm(userVmVoMock, accountMock, networkIdList, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); + + Mockito.verify(networkModel, Mockito.never()).checkNetworkPermissions(Mockito.any(), Mockito.any()); + } + + @Test + public void addAdditionalNetworksToVmTestNetworkIsNullThrowsInvalidParameterValueException() { + String expectedMessage = "Unable to find specified Network ID."; + LinkedList networkIdList = new LinkedList(); + HashSet applicableNetworks = Mockito.spy(new HashSet()); + HashMap requestedIPv4ForNics = new HashMap(); + HashMap requestedIPv6ForNics = new HashMap(); + + networkIdList.add(1l); + + InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { + userVmManagerImpl.addAdditionalNetworksToVm(userVmVoMock, accountMock, networkIdList, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); + }); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void addAdditionalNetworksToVmTestNetworkOfferingIsSystemOnlyThrowsInvalidParameterValueException() { + String expectedMessage = String.format("Specified network [%s] is system only and cannot be used for VM deployment.", networkMock); + LinkedList networkIdList = new LinkedList(); + HashSet applicableNetworks = Mockito.spy(new HashSet()); + HashMap requestedIPv4ForNics = new HashMap(); + HashMap requestedIPv6ForNics = new HashMap(); + + networkIdList.add(1l); + + Mockito.doReturn(networkMock).when(_networkDao).findById(Mockito.anyLong()); + Mockito.doReturn(networkOfferingVoMock).when(entityManager).findById(Mockito.any(), Mockito.anyLong()); + Mockito.doReturn(true).when(networkOfferingVoMock).isSystemOnly(); + + InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { + userVmManagerImpl.addAdditionalNetworksToVm(userVmVoMock, accountMock, networkIdList, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); + }); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void addAdditionalNetworksToVmTestNetworkIsNotSharedGuestTypeDoesNotCallNicDaoFindByNtwkIdAndInstanceId() { + LinkedList networkIdList = new LinkedList(); + HashSet applicableNetworks = Mockito.spy(new HashSet()); + HashMap requestedIPv4ForNics = new HashMap(); + HashMap requestedIPv6ForNics = new HashMap(); + + networkIdList.add(1l); + + Mockito.doReturn(networkMock).when(_networkDao).findById(Mockito.anyLong()); + Mockito.doReturn(networkOfferingVoMock).when(entityManager).findById(Mockito.any(), Mockito.anyLong()); + Mockito.doReturn(false).when(networkOfferingVoMock).isSystemOnly(); + Mockito.doReturn(Network.GuestType.L2).when(networkMock).getGuestType(); + + userVmManagerImpl.addAdditionalNetworksToVm(userVmVoMock, accountMock, networkIdList, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); + + Mockito.verify(nicDao, Mockito.never()).findByNtwkIdAndInstanceId(Mockito.anyLong(), Mockito.anyLong()); + } + + @Test + public void addAdditionalNetworksToVmTestNetworkIsNotDomainAclTypeDoesNotCallNicDaoFindByNtwkIdAndInstanceId() { + LinkedList networkIdList = new LinkedList(); + HashSet applicableNetworks = Mockito.spy(new HashSet()); + HashMap requestedIPv4ForNics = new HashMap(); + HashMap requestedIPv6ForNics = new HashMap(); + + networkIdList.add(1l); + + Mockito.doReturn(networkMock).when(_networkDao).findById(Mockito.anyLong()); + Mockito.doReturn(networkOfferingVoMock).when(entityManager).findById(Mockito.any(), Mockito.anyLong()); + Mockito.doReturn(false).when(networkOfferingVoMock).isSystemOnly(); + Mockito.doReturn(Network.GuestType.Shared).when(networkMock).getGuestType(); + Mockito.doReturn(ControlledEntity.ACLType.Account).when(networkMock).getAclType(); + + userVmManagerImpl.addAdditionalNetworksToVm(userVmVoMock, accountMock, networkIdList, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); + + Mockito.verify(nicDao, Mockito.never()).findByNtwkIdAndInstanceId(Mockito.anyLong(), Mockito.anyLong()); + } + + @Test + public void addAdditionalNetworksToVmTestOldNicIsNullDoesNotPutIpv4InRequestIpv4ForNics() { + LinkedList networkIdList = new LinkedList(); + HashSet applicableNetworks = Mockito.spy(new HashSet()); + HashMap requestedIPv4ForNics = Mockito.spy(new HashMap()); + HashMap requestedIPv6ForNics = new HashMap(); + + networkIdList.add(1l); + + Mockito.doReturn(networkMock).when(_networkDao).findById(Mockito.anyLong()); + Mockito.doReturn(networkOfferingVoMock).when(entityManager).findById(Mockito.any(), Mockito.anyLong()); + Mockito.doReturn(false).when(networkOfferingVoMock).isSystemOnly(); + Mockito.doReturn(Network.GuestType.Shared).when(networkMock).getGuestType(); + Mockito.doReturn(ControlledEntity.ACLType.Domain).when(networkMock).getAclType(); + Mockito.doReturn(null).when(nicDao).findByNtwkIdAndInstanceId(Mockito.anyLong(), Mockito.anyLong()); + + userVmManagerImpl.addAdditionalNetworksToVm(userVmVoMock, accountMock, networkIdList, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); + + Mockito.verify(nicDao).findByNtwkIdAndInstanceId(Mockito.anyLong(), Mockito.anyLong()); + Mockito.verify(requestedIPv4ForNics, Mockito.never()).put(Mockito.anyLong(), Mockito.any()); + } + + @Test + public void addAdditionalNetworksToVmTestOldNicIsNotNullPutsIpv4InRequestIpv4ForNics() { + LinkedList networkIdList = new LinkedList(); + HashSet applicableNetworks = Mockito.spy(new HashSet()); + HashMap requestedIPv4ForNics = Mockito.spy(new HashMap()); + HashMap requestedIPv6ForNics = new HashMap(); + + networkIdList.add(1l); + + Mockito.doReturn(networkMock).when(_networkDao).findById(Mockito.anyLong()); + Mockito.doReturn(networkOfferingVoMock).when(entityManager).findById(Mockito.any(), Mockito.anyLong()); + Mockito.doReturn(false).when(networkOfferingVoMock).isSystemOnly(); + Mockito.doReturn(Network.GuestType.Shared).when(networkMock).getGuestType(); + Mockito.doReturn(ControlledEntity.ACLType.Domain).when(networkMock).getAclType(); + Mockito.doReturn(new NicVO()).when(nicDao).findByNtwkIdAndInstanceId(Mockito.anyLong(), Mockito.anyLong()); + + userVmManagerImpl.addAdditionalNetworksToVm(userVmVoMock, accountMock, networkIdList, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); + + Mockito.verify(nicDao).findByNtwkIdAndInstanceId(Mockito.anyLong(), Mockito.anyLong()); + Mockito.verify(requestedIPv4ForNics).put(Mockito.anyLong(), Mockito.any()); + } + + @Test + public void createApplicableNetworkToCreateVmTestPhysicalNetworkIsNullThrowsInvalidParameterValueException() { + String expectedMessage = String.format("Unable to find physical network with ID [%s] and tag [%s].", 0l, null); + InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { + userVmManagerImpl.createApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, networkOfferingVoMock); + }); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void createApplicableNetworkToCreateVmTestFirstNetworkOfferingIsPersistentCallsImplementNetwork() throws InsufficientCapacityException, ResourceAllocationException { + PhysicalNetworkVO physicalNetworkVo = new PhysicalNetworkVO(); + + Mockito.doReturn(physicalNetworkVo).when(physicalNetworkDaoMock).findById(Mockito.anyLong()); + Mockito.doReturn(true).when(networkOfferingVoMock).isPersistent(); + Mockito.doReturn(networkMock).when(userVmManagerImpl).implementNetwork(Mockito.any(), Mockito.any(), Mockito.any()); + + userVmManagerImpl.createApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, networkOfferingVoMock); + + Mockito.verify(userVmManagerImpl).implementNetwork(callerAccount, _dcMock, null); + } + + @Test + public void createApplicableNetworkToCreateVmTestFirstNetworkOfferingIsNotPersistentDoesNotCallImplementNetwork() throws InsufficientCapacityException, + ResourceAllocationException { + + PhysicalNetworkVO physicalNetworkVo = new PhysicalNetworkVO(); + + Mockito.doReturn(physicalNetworkVo).when(physicalNetworkDaoMock).findById(Mockito.anyLong()); + Mockito.doReturn(false).when(networkOfferingVoMock).isPersistent(); + Mockito.doReturn(networkMock).when(_networkMgr).createGuestNetwork(Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), Mockito.any(), Mockito.any(), + Mockito.any(), Mockito.anyBoolean(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyLong(), Mockito.any(), + Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyBoolean(), Mockito.any(), Mockito.any(), Mockito.any(), + Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); + Mockito.doReturn(1l).when(networkMock).getId(); + Mockito.doReturn(networkMock).when(_networkDao).findById(Mockito.anyLong()); + + userVmManagerImpl.createApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, networkOfferingVoMock); + + Mockito.verify(userVmManagerImpl, Mockito.never()).implementNetwork(Mockito.any(), Mockito.any(), Mockito.any()); + } + + @Test + public void canAccountUseNetworkTestNetworkIsNullReturnFalse() { + boolean canAccountUseNetwork = userVmManagerImpl.canAccountUseNetwork(accountMock, null); + + Assert.assertFalse(canAccountUseNetwork); + } + + @Test + public void canAccountUseNetworkTestNetworkAclTypeIsNotDomainReturnFalse() { + Mockito.doReturn(ControlledEntity.ACLType.Account).when(networkMock).getAclType(); + + boolean canAccountUseNetwork = userVmManagerImpl.canAccountUseNetwork(accountMock, networkMock); + + Assert.assertFalse(canAccountUseNetwork); + } + + @Test + public void canAccountUseNetworkTestNetworkGuestTypeIsNotSharedOrL2ReturnFalse() { + Mockito.doReturn(ControlledEntity.ACLType.Domain).when(networkMock).getAclType(); + Mockito.doReturn(Network.GuestType.Isolated).when(networkMock).getGuestType(); + + boolean canAccountUseNetwork = userVmManagerImpl.canAccountUseNetwork(accountMock, networkMock); + + Assert.assertFalse(canAccountUseNetwork); + } + + @Test + public void canAccountUseNetworkTestNetworkGuestTypeIsSharedReturnTrue() { + Mockito.doReturn(ControlledEntity.ACLType.Domain).when(networkMock).getAclType(); + Mockito.doReturn(Network.GuestType.Shared).when(networkMock).getGuestType(); + + boolean canAccountUseNetwork = userVmManagerImpl.canAccountUseNetwork(accountMock, networkMock); + + Mockito.verify(networkModel).checkNetworkPermissions(accountMock, networkMock); + Assert.assertTrue(canAccountUseNetwork); + } + + @Test + public void canAccountUseNetworkTestNetworkGuestTypeIsL2ReturnTrue() { + Mockito.doReturn(ControlledEntity.ACLType.Domain).when(networkMock).getAclType(); + Mockito.doReturn(Network.GuestType.L2).when(networkMock).getGuestType(); + + boolean canAccountUseNetwork = userVmManagerImpl.canAccountUseNetwork(accountMock, networkMock); + + Mockito.verify(networkModel).checkNetworkPermissions(accountMock, networkMock); + Assert.assertTrue(canAccountUseNetwork); + } + + @Test + public void canAccountUseNetworkTestPermissionDeniedExceptionThrownReturnFalse() { + Mockito.doReturn(ControlledEntity.ACLType.Domain).when(networkMock).getAclType(); + Mockito.doReturn(Network.GuestType.L2).when(networkMock).getGuestType(); + + Mockito.doThrow(PermissionDeniedException.class).when(networkModel).checkNetworkPermissions(accountMock, networkMock); + + boolean canAccountUseNetwork = userVmManagerImpl.canAccountUseNetwork(accountMock, networkMock); + + Assert.assertFalse(canAccountUseNetwork); + } + + @Test + public void implementNetworkTestImplementedNetworkIsNullReturnCurrentNewNetwork() throws ResourceUnavailableException, InsufficientCapacityException { + CallContext callContextMock = Mockito.mock(CallContext.class); + NetworkVO currentNetwork = Mockito.mock(NetworkVO.class); + + try (MockedStatic ignored = Mockito.mockStatic(CallContext.class)) { + Mockito.when(CallContext.current()).thenReturn(callContextMock); + + Mockito.doReturn(1l).when(callContextMock).getCallingUserId(); + + Mockito.doReturn(callerUser).when(userDao).findById(Mockito.anyLong()); + Mockito.doReturn(null).when(_networkMgr).implementNetwork(Mockito.anyLong(), Mockito.any(), Mockito.any()); + + Network newNetwork = userVmManagerImpl.implementNetwork(accountMock, _dcMock, currentNetwork); + + Assert.assertEquals(newNetwork, currentNetwork); + } + } + + @Test + public void implementNetworkTestImplementedNetworkFirstIsNullReturnCurrentNewNetwork() throws ResourceUnavailableException, InsufficientCapacityException { + CallContext callContextMock = Mockito.mock(CallContext.class); + NetworkVO currentNetwork = Mockito.mock(NetworkVO.class); + + try (MockedStatic ignored = Mockito.mockStatic(CallContext.class)) { + Mockito.when(CallContext.current()).thenReturn(callContextMock); + + Mockito.doReturn(1l).when(callContextMock).getCallingUserId(); + + Pair implementedNetwork = Mockito.mock(Pair.class); + + Mockito.doReturn(callerUser).when(userDao).findById(Mockito.anyLong()); + Mockito.doReturn(null).when(implementedNetwork).first(); + Mockito.doReturn(implementedNetwork).when(_networkMgr).implementNetwork(Mockito.anyLong(), Mockito.any(), Mockito.any()); + + Network newNetwork = userVmManagerImpl.implementNetwork(accountMock, _dcMock, currentNetwork); + + Assert.assertEquals(newNetwork, currentNetwork); + } + } + + @Test + public void implementNetworkTestImplementedNetworkSecondIsNullReturnCurrentNewNetwork() throws ResourceUnavailableException, InsufficientCapacityException { + CallContext callContextMock = Mockito.mock(CallContext.class); + NetworkVO currentNetwork = Mockito.mock(NetworkVO.class); + + try (MockedStatic ignored = Mockito.mockStatic(CallContext.class)) { + Mockito.when(CallContext.current()).thenReturn(callContextMock); + + Mockito.doReturn(1l).when(callContextMock).getCallingUserId(); + + Pair implementedNetwork = Mockito.mock(Pair.class); + + Mockito.doReturn(callerUser).when(userDao).findById(Mockito.anyLong()); + Mockito.doReturn(networkMock).when(implementedNetwork).first(); + Mockito.doReturn(null).when(implementedNetwork).second(); + Mockito.doReturn(implementedNetwork).when(_networkMgr).implementNetwork(Mockito.anyLong(), Mockito.any(), Mockito.any()); + + Network newNetwork = userVmManagerImpl.implementNetwork(accountMock, _dcMock, currentNetwork); + + Assert.assertEquals(newNetwork, currentNetwork); + } + } + + @Test + public void implementNetworkTestImplementedNetworkSecondIsNotNullReturnImplementedNetworkSecond() throws ResourceUnavailableException, InsufficientCapacityException { + CallContext callContextMock = Mockito.mock(CallContext.class); + NetworkVO currentNetwork = Mockito.mock(NetworkVO.class); + + try (MockedStatic ignored = Mockito.mockStatic(CallContext.class)) { + Mockito.when(CallContext.current()).thenReturn(callContextMock); + + Mockito.doReturn(1l).when(callContextMock).getCallingUserId(); + + Pair implementedNetwork = Mockito.mock(Pair.class); + + Mockito.doReturn(callerUser).when(userDao).findById(Mockito.anyLong()); + Mockito.doReturn(networkMock).when(implementedNetwork).first(); + Mockito.doReturn(networkMock).when(implementedNetwork).second(); + Mockito.doReturn(implementedNetwork).when(_networkMgr).implementNetwork(Mockito.anyLong(), Mockito.any(), Mockito.any()); + + Network newNetwork = userVmManagerImpl.implementNetwork(accountMock, _dcMock, currentNetwork); + + Assert.assertEquals(newNetwork, networkMock); + } + } + + @Test + public void implementNetworkTestImplementedNetworkCatchException() throws ResourceUnavailableException, InsufficientCapacityException { + String expectedMessage = String.format("Failed to implement network [%s] elements and resources as a part of network provision.", networkMock); + + CallContext callContextMock = Mockito.mock(CallContext.class); + + try (MockedStatic ignored = Mockito.mockStatic(CallContext.class)) { + Mockito.when(CallContext.current()).thenReturn(callContextMock); + + Mockito.doReturn(1l).when(callContextMock).getCallingUserId(); + + Pair implementedNetwork = Mockito.mock(Pair.class); + + Mockito.doReturn(callerUser).when(userDao).findById(Mockito.anyLong()); + Mockito.doThrow(InvalidParameterValueException.class).when(_networkMgr).implementNetwork(Mockito.anyLong(), Mockito.any(), Mockito.any()); + + CloudRuntimeException assertThrows = Assert.assertThrows(expectedCloudRuntimeException, () -> { + userVmManagerImpl.implementNetwork(accountMock, _dcMock, networkMock); + }); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + } + + @Test + public void updateBasicTypeNetworkForVmTestNetworkIdListIsNotEmptyThrowsInvalidParameterValueException() { + String expectedMessage = "Cannot move VM with Network IDs; this is a basic zone VM."; + LinkedList networkIdList = new LinkedList(); + LinkedList securityGroupIdList = new LinkedList(); + + networkIdList.add(1l); + + InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { + userVmManagerImpl.updateBasicTypeNetworkForVm(assignVmCmdMock, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, networkIdList, + securityGroupIdList); + }); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void updateBasicTypeNetworkForVmTestNetworkIdListIsNullCallsCleanupOfOldOwnerNicsForNetworkAddDefaultNetworkToNetworkListAllocateNetworksForVmAndAddSecurityGroupsToVm() + throws InsufficientCapacityException { + + LinkedList securityGroupIdList = Mockito.mock(LinkedList.class); + + Mockito.doReturn(networkMock).when(networkModel).getExclusiveGuestNetwork(Mockito.anyLong()); + + userVmManagerImpl.updateBasicTypeNetworkForVm(assignVmCmdMock, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, null, + securityGroupIdList); + + Mockito.verify(userVmManagerImpl).cleanupOfOldOwnerNicsForNetwork(virtualMachineProfileMock); + Mockito.verify(userVmManagerImpl).addDefaultNetworkToNetworkList(Mockito.anyList(), Mockito.any()); + Mockito.verify(userVmManagerImpl).allocateNetworksForVm(Mockito.any(), Mockito.any()); + Mockito.verify(userVmManagerImpl).addSecurityGroupsToVm(accountMock, userVmVoMock,virtualMachineTemplateMock, securityGroupIdList, networkMock); + } + + @Test + public void updateBasicTypeNetworkForVmTestNetworkIdListIsEmptyCallsCleanupOfOldOwnerNicsForNetworkAddDefaultNetworkToNetworkListAllocateNetworksForVmAndAddSecurityGroupsToVm() + throws InsufficientCapacityException { + + LinkedList securityGroupIdList = Mockito.mock(LinkedList.class); + LinkedList networkIdList = new LinkedList(); + + Mockito.doReturn(networkMock).when(networkModel).getExclusiveGuestNetwork(Mockito.anyLong()); + + userVmManagerImpl.updateBasicTypeNetworkForVm(assignVmCmdMock, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, networkIdList, + securityGroupIdList); + + Mockito.verify(userVmManagerImpl).cleanupOfOldOwnerNicsForNetwork(virtualMachineProfileMock); + Mockito.verify(userVmManagerImpl).addDefaultNetworkToNetworkList(Mockito.anyList(), Mockito.any()); + Mockito.verify(userVmManagerImpl).allocateNetworksForVm(Mockito.any(), Mockito.any()); + Mockito.verify(userVmManagerImpl).addSecurityGroupsToVm(accountMock, userVmVoMock,virtualMachineTemplateMock, securityGroupIdList, networkMock); + } + + @Test + public void updateAdvancedTypeNetworkForVmTestSecurityGroupIsEnabledApplicableNetworksIsEmptyThrowsInvalidParameterValueException() { + String expectedMessage = "No network is specified, please specify one when you move the VM. For now, please add a network to VM on NICs tab."; + LinkedList securityGroupIdList = Mockito.mock(LinkedList.class); + LinkedList networkIdList = new LinkedList(); + + Mockito.doReturn(true).when(_dcMock).isSecurityGroupEnabled(); + + InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { + userVmManagerImpl.updateAdvancedTypeNetworkForVm(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, + _dcMock, networkIdList, securityGroupIdList); + }); + + Mockito.verify(securityGroupManagerMock).removeInstanceFromGroups(Mockito.anyLong()); + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void updateAdvancedTypeNetworkForVmTestSecurityGroupIsEnabledApplicableNetworksIsNotEmptyCallsAllocateNetworksForVm() throws InsufficientCapacityException, + ResourceAllocationException { + + LinkedList securityGroupIdList = Mockito.mock(LinkedList.class); + LinkedList networkIdList = new LinkedList(); + + Mockito.doReturn(new NicVO()).when(nicDao).findDefaultNicForVM(Mockito.anyLong()); + Mockito.doReturn(networkMock).when(_networkDao).findById(Mockito.anyLong()); + Mockito.doReturn(true).when(userVmManagerImpl).canAccountUseNetwork(accountMock, networkMock); + + Mockito.doReturn(true).when(_dcMock).isSecurityGroupEnabled(); + + userVmManagerImpl.updateAdvancedTypeNetworkForVm(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, + networkIdList, securityGroupIdList); + + Mockito.verify(securityGroupManagerMock).removeInstanceFromGroups(Mockito.anyLong()); + Mockito.verify(userVmManagerImpl).allocateNetworksForVm(Mockito.any(), Mockito.any()); + Mockito.verify(userVmManagerImpl).addSecurityGroupsToVm(accountMock, userVmVoMock, virtualMachineTemplateMock, securityGroupIdList, networkMock); + } + + @Test + public void updateAdvancedTypeNetworkForVmTestSecurityGroupIsNotEnabledSecurityGroupIdListIsNotEmptyThrowsInvalidParameterValueException() { + String expectedMessage = "Cannot move VM with security groups; security group feature is not enabled in this zone."; + LinkedList securityGroupIdList = Mockito.mock(LinkedList.class); + LinkedList networkIdList = new LinkedList(); + + securityGroupIdList.add(1l); + + Mockito.doReturn(false).when(_dcMock).isSecurityGroupEnabled(); + + InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { + userVmManagerImpl.updateAdvancedTypeNetworkForVm(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, + _dcMock, networkIdList, securityGroupIdList); + }); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void updateAdvancedTypeNetworkForVmTestSecurityGroupIsNotEnabledApplicableNetworksIsEmptyCallsSelectApplicableNetworkToCreateVm() throws InsufficientCapacityException, + ResourceAllocationException { + + LinkedList securityGroupIdList = Mockito.mock(LinkedList.class); + LinkedList networkIdList = new LinkedList(); + + Mockito.doReturn(networkMock).when(userVmManagerImpl).addNicsToApplicableNetworksAndReturnDefaultNetwork(Mockito.any(), Mockito.anyMap(), Mockito.anyMap(), Mockito.any()); + Mockito.doNothing().when(userVmManagerImpl).selectApplicableNetworkToCreateVm(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); + + Mockito.doReturn(false).when(_dcMock).isSecurityGroupEnabled(); + Mockito.doReturn(true).when(securityGroupIdList).isEmpty(); + + userVmManagerImpl.updateAdvancedTypeNetworkForVm(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, + networkIdList, securityGroupIdList); + + Mockito.verify(userVmManagerImpl).addNetworksToNetworkIdList(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyMap(), Mockito.anyMap()); + Mockito.verify(userVmManagerImpl).selectApplicableNetworkToCreateVm(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); + Mockito.verify(userVmManagerImpl).addNicsToApplicableNetworksAndReturnDefaultNetwork(Mockito.any(), Mockito.anyMap(), Mockito.anyMap(), Mockito.any()); + Mockito.verify(userVmManagerImpl).allocateNetworksForVm(Mockito.any(), Mockito.any()); + } + + @Test + public void updateAdvancedTypeNetworkForVmTestSecurityGroupIsNotEnabledApplicableNetworksIsNotEmptyDoesNotCallSelectApplicableNetworkToCreateVm() + throws InsufficientCapacityException, ResourceAllocationException { + + LinkedList securityGroupIdList = Mockito.mock(LinkedList.class); + LinkedList networkIdList = new LinkedList(); + + Mockito.doReturn(false).when(_dcMock).isSecurityGroupEnabled(); + Mockito.doReturn(true).when(securityGroupIdList).isEmpty(); + + Mockito.doReturn(new NicVO()).when(nicDao).findDefaultNicForVM(Mockito.anyLong()); + Mockito.doReturn(networkMock).when(_networkDao).findById(Mockito.anyLong()); + Mockito.doReturn(true).when(userVmManagerImpl).canAccountUseNetwork(accountMock, networkMock); + + userVmManagerImpl.updateAdvancedTypeNetworkForVm(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, + networkIdList, securityGroupIdList); + + Mockito.verify(userVmManagerImpl).addNetworksToNetworkIdList(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyMap(), Mockito.anyMap()); + Mockito.verify(userVmManagerImpl, Mockito.never()).selectApplicableNetworkToCreateVm(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); + Mockito.verify(userVmManagerImpl).addNicsToApplicableNetworksAndReturnDefaultNetwork(Mockito.any(), Mockito.anyMap(), Mockito.anyMap(), Mockito.any()); + Mockito.verify(userVmManagerImpl).allocateNetworksForVm(Mockito.any(), Mockito.any()); + } + + @Test + public void addNicsToApplicableNetworksAndReturnDefaultNetworkTestApplicableNetworkIsEmptyReturnNull() { + HashMap requestedIPv4ForNics = new HashMap(); + HashMap requestedIPv6ForNics = new HashMap(); + LinkedHashSet applicableNetworks = new LinkedHashSet(); + LinkedHashMap> networks = new LinkedHashMap>(); + + NetworkVO defaultNetwork = userVmManagerImpl.addNicsToApplicableNetworksAndReturnDefaultNetwork(applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics, networks); + + Assert.assertNull(defaultNetwork); + } + + @Test + public void addNicsToApplicableNetworksAndReturnDefaultNetworkTestApplicableNetworkIsNotEmptyReturnFirstElement() { + HashMap requestedIPv4ForNics = new HashMap(); + HashMap requestedIPv6ForNics = new HashMap(); + LinkedHashSet applicableNetworks = new LinkedHashSet(); + LinkedHashMap> networks = Mockito.spy(LinkedHashMap.class); + + applicableNetworks.add(networkMock); + + NetworkVO defaultNetwork = userVmManagerImpl.addNicsToApplicableNetworksAndReturnDefaultNetwork(applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics, networks); + + Mockito.verify(networks).put(Mockito.any(), Mockito.any()); + Assert.assertEquals(defaultNetwork, networkMock); + } + + @Test + public void addNicsToApplicableNetworksAndReturnDefaultNetworkTestApplicableNetworkIsNotEmptyPutTwoNetworksInNetworksMapAndReturnFirst() { + HashMap requestedIPv4ForNics = new HashMap(); + HashMap requestedIPv6ForNics = new HashMap(); + LinkedHashSet applicableNetworks = new LinkedHashSet(); + LinkedHashMap> networks = Mockito.spy(LinkedHashMap.class); + + NetworkVO networkVoMock2 = Mockito.mock(NetworkVO.class); + applicableNetworks.add(networkMock); + applicableNetworks.add(networkVoMock2); + + NetworkVO defaultNetwork = userVmManagerImpl.addNicsToApplicableNetworksAndReturnDefaultNetwork(applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics, networks); + + Mockito.verify(networks, Mockito.times(2)).put(Mockito.any(), Mockito.any()); + Assert.assertEquals(defaultNetwork, networkMock); + } + + @Test + public void validateIfVolumesHaveNoSnapshotsTestVolumeHasSnapshotsThrowsInvalidParameterException() { + String expectedMessage = String.format("Snapshots exist for volume [%s]. Detach volume or remove snapshots for the volume before assigning VM to another user.", + volumeVOMock.getName()); + + LinkedList volumes = new LinkedList(); + volumes.add(volumeVOMock); + LinkedList snapshots = new LinkedList(); + snapshots.add(snapshotVoMock); + + Mockito.doReturn(snapshots).when(snapshotDaoMock).listByStatusNotIn(Mockito.anyLong(), Mockito.any(), Mockito.any()); + + InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { + userVmManagerImpl.validateIfVolumesHaveNoSnapshots(volumes); + }); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void validateIfVolumesHaveNoSnapshotsTestVolumeHasNoSnapshotsDoesNotThrowInvalidParameterException() { + LinkedList volumes = new LinkedList(); + volumes.add(volumeVOMock); + LinkedList snapshots = new LinkedList(); + + Mockito.doReturn(snapshots).when(snapshotDaoMock).listByStatusNotIn(Mockito.anyLong(), Mockito.any(), Mockito.any()); + + userVmManagerImpl.validateIfVolumesHaveNoSnapshots(volumes); + } + + @Test + public void moveVmToUserTestCallerIsNotRootAdminAndDomainAdminThrowsInvalidParameterValueException() { + String expectedMessage = String.format("Only root or domain admins are allowed to assign VMs. Caller [%s] is of type [%s].", callerAccount, callerAccount.getType()); + + Mockito.doReturn(false).when(accountManager).isRootAdmin(Mockito.anyLong()); + Mockito.doReturn(false).when(accountManager).isDomainAdmin(Mockito.anyLong()); + + InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { + userVmManagerImpl.moveVmToUser(assignVmCmdMock); + }); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void moveVmToUserTestValidateVmExistsAndIsNotRunningThrowsInvalidParameterValueException() { + Mockito.doReturn(true).when(accountManager).isRootAdmin(Mockito.anyLong()); + + Mockito.doThrow(InvalidParameterValueException.class).when(userVmManagerImpl).validateIfVmExistsAndIsNotRunning(Mockito.any(), Mockito.anyLong()); + + Assert.assertThrows(InvalidParameterValueException.class, () -> userVmManagerImpl.moveVmToUser(assignVmCmdMock)); + } + + @Test + public void moveVmToUserTestValidateAccountsAndCallerAccessToThemThrowsInvalidParameterValueException() { + Mockito.doReturn(true).when(accountManager).isRootAdmin(Mockito.anyLong()); + Mockito.doReturn(userVmVoMock).when(userVmDao).findById(Mockito.anyLong()); + + Assert.assertThrows(InvalidParameterValueException.class, () -> userVmManagerImpl.moveVmToUser(assignVmCmdMock)); + } + + @Test + public void moveVmToUserTestProjectIdProvidedAndDomainIdIsNullThrowsInvalidParameterValueException() throws ResourceUnavailableException, InsufficientCapacityException, + ResourceAllocationException { + + String expectedMessage = "Please provide a valid domain ID; cannot assign VM to a project if domain ID is NULL."; + + Mockito.doReturn(true).when(accountManager).isRootAdmin(Mockito.anyLong()); + Mockito.doReturn(userVmVoMock).when(userVmDao).findById(Mockito.anyLong()); + Mockito.doReturn(1l).when(assignVmCmdMock).getProjectId(); + Mockito.doReturn(null).when(assignVmCmdMock).getDomainId(); + + configureDoNothingForMethodsThatWeDoNotWantToTest(); + + InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { + userVmManagerImpl.moveVmToUser(assignVmCmdMock); + }); + + Assert.assertEquals(expectedMessage, assertThrows.getMessage()); + } + + @Test + public void moveVmToUserTestValidateIfVmHasNoRulesThrowsInvalidParameterValueException() throws ResourceUnavailableException, InsufficientCapacityException, + ResourceAllocationException { + + Mockito.doReturn(true).when(accountManager).isRootAdmin(Mockito.anyLong()); + Mockito.doReturn(userVmVoMock).when(userVmDao).findById(Mockito.anyLong()); + Mockito.doReturn(null).when(assignVmCmdMock).getProjectId(); + + configureDoNothingForMethodsThatWeDoNotWantToTest(); + + Mockito.doThrow(InvalidParameterValueException.class).when(userVmManagerImpl).validateIfVmHasNoRules(Mockito.any(), Mockito.anyLong()); + + Assert.assertThrows(InvalidParameterValueException.class, () -> userVmManagerImpl.moveVmToUser(assignVmCmdMock)); + } + + @Test + public void moveVmToUserTestSnapshotsForVolumeExistThrowsInvalidParameterValueException() throws ResourceUnavailableException, InsufficientCapacityException, + ResourceAllocationException { + + LinkedList volumes = new LinkedList(); + volumes.add(volumeVOMock); + + Mockito.doReturn(true).when(accountManager).isRootAdmin(Mockito.anyLong()); + Mockito.doReturn(userVmVoMock).when(userVmDao).findById(Mockito.anyLong()); + Mockito.doReturn(null).when(assignVmCmdMock).getProjectId(); + Mockito.doReturn(volumes).when(volumeDaoMock).findByInstance(Mockito.anyLong()); + + configureDoNothingForMethodsThatWeDoNotWantToTest(); + + Mockito.doThrow(InvalidParameterValueException.class).when(userVmManagerImpl).validateIfVolumesHaveNoSnapshots(Mockito.any()); + + Assert.assertThrows(InvalidParameterValueException.class, () -> userVmManagerImpl.moveVmToUser(assignVmCmdMock)); + } + + @Test + public void moveVmToUserTestVerifyResourceLimitsForAccountAndStorageThrowsResourceAllocationException() throws ResourceUnavailableException, InsufficientCapacityException, + ResourceAllocationException { + + LinkedList volumes = new LinkedList(); + + Mockito.doReturn(true).when(accountManager).isRootAdmin(Mockito.anyLong()); + Mockito.doReturn(userVmVoMock).when(userVmDao).findById(Mockito.anyLong()); + Mockito.doReturn(null).when(assignVmCmdMock).getProjectId(); + Mockito.doReturn(volumes).when(volumeDaoMock).findByInstance(Mockito.anyLong()); + + configureDoNothingForMethodsThatWeDoNotWantToTest(); + + Mockito.doThrow(ResourceAllocationException.class).when(userVmManagerImpl).verifyResourceLimitsForAccountAndStorage(Mockito.any(), Mockito.any(), Mockito.anyLong(), + Mockito.any(), Mockito.any(), Mockito.any()); + + Assert.assertThrows(ResourceAllocationException.class, () -> userVmManagerImpl.moveVmToUser(assignVmCmdMock)); + } + + @Test + public void moveVmToUserTestVerifyValidateIfNewOwnerHasAccessToTemplateThrowsInvalidParameterValueException() throws ResourceUnavailableException, + InsufficientCapacityException, ResourceAllocationException { + + LinkedList volumes = new LinkedList(); + + Mockito.doReturn(true).when(accountManager).isRootAdmin(Mockito.anyLong()); + Mockito.doReturn(userVmVoMock).when(userVmDao).findById(Mockito.anyLong()); + Mockito.doReturn(null).when(assignVmCmdMock).getProjectId(); + Mockito.doReturn(volumes).when(volumeDaoMock).findByInstance(Mockito.anyLong()); + + configureDoNothingForMethodsThatWeDoNotWantToTest(); + + Mockito.doThrow(InvalidParameterValueException.class).when(userVmManagerImpl).validateIfNewOwnerHasAccessToTemplate(Mockito.any(), Mockito.any(), Mockito.any()); + + Assert.assertThrows(InvalidParameterValueException.class, () -> userVmManagerImpl.moveVmToUser(assignVmCmdMock)); + } + + @Test + public void moveVmToUserTestAccountManagerCheckAccessThrowsPermissionDeniedException() throws ResourceUnavailableException, InsufficientCapacityException, + ResourceAllocationException { + + LinkedList volumes = new LinkedList(); + + Mockito.doReturn(true).when(accountManager).isRootAdmin(Mockito.anyLong()); + Mockito.doReturn(userVmVoMock).when(userVmDao).findById(Mockito.anyLong()); + Mockito.doReturn(null).when(assignVmCmdMock).getProjectId(); + Mockito.doReturn(volumes).when(volumeDaoMock).findByInstance(Mockito.anyLong()); + Mockito.doReturn(accountMock).when(accountManager).finalizeOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); + Mockito.doReturn(domainVoMock).when(domainDaoMock).findById(Mockito.anyLong()); + + configureDoNothingForMethodsThatWeDoNotWantToTest(); + + Mockito.doThrow(PermissionDeniedException.class).when(accountManager).checkAccess(Mockito.any(Account.class), Mockito.any()); + + Assert.assertThrows(PermissionDeniedException.class, () -> userVmManagerImpl.moveVmToUser(assignVmCmdMock)); + } + + @Test + public void executeStepsToChangeOwnershipOfVmTestUpdateVmNetworkThrowsInsufficientCapacityException() throws ResourceUnavailableException, InsufficientCapacityException, + ResourceAllocationException { + + LinkedList volumes = new LinkedList(); + + try (MockedStatic ignored = Mockito.mockStatic(UsageEventUtils.class)) { + Mockito.doReturn(Hypervisor.HypervisorType.KVM).when(userVmVoMock).getHypervisorType(); + + configureDoNothingForMethodsThatWeDoNotWantToTest(); + + Mockito.doThrow(InsufficientAddressCapacityException.class).when(userVmManagerImpl).updateVmNetwork(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), + Mockito.any()); + + Assert.assertThrows(RuntimeException.class, () -> userVmManagerImpl.executeStepsToChangeOwnershipOfVm(assignVmCmdMock, callerAccount, accountMock, accountMock, + userVmVoMock, serviceOfferingVoMock, volumes, virtualMachineTemplateMock, 1l)); + + Mockito.verify(userVmManagerImpl).resourceCountDecrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any()); + Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong()); + Mockito.verify(userVmManagerImpl).updateVolumesOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyLong()); + } + } + + @Test + public void executeStepsToChangeOwnershipOfVmTestUpdateVmNetworkThrowsResourceAllocationException() throws ResourceUnavailableException, InsufficientCapacityException, + ResourceAllocationException { + + LinkedList volumes = new LinkedList(); + + try (MockedStatic ignored = Mockito.mockStatic(UsageEventUtils.class)) { + Mockito.doReturn(Hypervisor.HypervisorType.KVM).when(userVmVoMock).getHypervisorType(); + + configureDoNothingForMethodsThatWeDoNotWantToTest(); + + Mockito.doThrow(ResourceAllocationException.class).when(userVmManagerImpl).updateVmNetwork(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), + Mockito.any()); + + Assert.assertThrows(RuntimeException.class, () -> userVmManagerImpl.executeStepsToChangeOwnershipOfVm(assignVmCmdMock, callerAccount, accountMock, accountMock, + userVmVoMock, serviceOfferingVoMock, volumes, virtualMachineTemplateMock, 1l)); + + Mockito.verify(userVmManagerImpl).resourceCountDecrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any()); + Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong()); + Mockito.verify(userVmManagerImpl).updateVolumesOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyLong()); + } + } + + @Test + public void executeStepsToChangeOwnershipOfVmTestResourceCountRunningVmsOnlyEnabledIsFalseCallsResourceCountIncrement() throws ResourceUnavailableException, + InsufficientCapacityException, ResourceAllocationException { + + LinkedList volumes = new LinkedList(); + + + try (MockedStatic ignored = Mockito.mockStatic(UsageEventUtils.class)) { + Mockito.doReturn(Hypervisor.HypervisorType.KVM).when(userVmVoMock).getHypervisorType(); + Mockito.doReturn(false).when(userVmManagerImpl).isResourceCountRunningVmsOnlyEnabled(); + + configureDoNothingForMethodsThatWeDoNotWantToTest(); + + userVmManagerImpl.executeStepsToChangeOwnershipOfVm(assignVmCmdMock, callerAccount, accountMock, accountMock, userVmVoMock, serviceOfferingVoMock, volumes, + virtualMachineTemplateMock, 1l); + + Mockito.verify(userVmManagerImpl).resourceCountDecrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any()); + Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong()); + Mockito.verify(userVmManagerImpl).updateVolumesOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyLong()); + Mockito.verify(userVmManagerImpl).updateVmNetwork(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); + Mockito.verify(userVmManagerImpl).resourceCountIncrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any()); + } + } + + @Test + public void executeStepsToChangeOwnershipOfVmTestResourceCountRunningVmsOnlyEnabledIsTrueDoesNotCallResourceCountIncrement() throws ResourceUnavailableException, + InsufficientCapacityException, ResourceAllocationException { + + LinkedList volumes = new LinkedList(); + + try (MockedStatic ignored = Mockito.mockStatic(UsageEventUtils.class)) { + Mockito.doReturn(Hypervisor.HypervisorType.KVM).when(userVmVoMock).getHypervisorType(); + Mockito.doReturn(true).when(userVmManagerImpl).isResourceCountRunningVmsOnlyEnabled(); + + configureDoNothingForMethodsThatWeDoNotWantToTest(); + + userVmManagerImpl.executeStepsToChangeOwnershipOfVm(assignVmCmdMock, callerAccount, accountMock, accountMock, userVmVoMock, serviceOfferingVoMock, volumes, + virtualMachineTemplateMock, 1l); + + Mockito.verify(userVmManagerImpl).resourceCountDecrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any()); + Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong()); + Mockito.verify(userVmManagerImpl).updateVolumesOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyLong()); + Mockito.verify(userVmManagerImpl).updateVmNetwork(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); + Mockito.verify(userVmManagerImpl, Mockito.never()).resourceCountIncrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any()); + } + } } diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerTest.java index 8be100d45708..087fa37ae4c2 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerTest.java @@ -477,7 +477,7 @@ public void testMoveVmToUser1() throws Exception { CallContext.register(user, caller); try { - _userVmMgr.moveVMToUser(cmd); + _userVmMgr.moveVmToUser(cmd); } finally { CallContext.unregister(); } @@ -527,7 +527,7 @@ public void testMoveVmToUser2() throws Exception { when(_accountMgr.isRootAdmin(anyLong())).thenReturn(true); try { - _userVmMgr.moveVMToUser(cmd); + _userVmMgr.moveVmToUser(cmd); } finally { CallContext.unregister(); } From dd1fe0f37f19c4f1cdcb4b2d5ba1d75700631329 Mon Sep 17 00:00:00 2001 From: Stephan Krug Date: Mon, 6 Feb 2023 18:31:25 -0300 Subject: [PATCH 2/9] Use CloudRuntimeException --- server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index b628053554a7..5dfa0c7564d5 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -2989,7 +2989,7 @@ public void executeStepsToChangeOwnershipOfVmTestUpdateVmNetworkThrowsInsufficie Mockito.doThrow(InsufficientAddressCapacityException.class).when(userVmManagerImpl).updateVmNetwork(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); - Assert.assertThrows(RuntimeException.class, () -> userVmManagerImpl.executeStepsToChangeOwnershipOfVm(assignVmCmdMock, callerAccount, accountMock, accountMock, + Assert.assertThrows(CloudRuntimeException.class, () -> userVmManagerImpl.executeStepsToChangeOwnershipOfVm(assignVmCmdMock, callerAccount, accountMock, accountMock, userVmVoMock, serviceOfferingVoMock, volumes, virtualMachineTemplateMock, 1l)); Mockito.verify(userVmManagerImpl).resourceCountDecrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any()); @@ -3012,7 +3012,7 @@ public void executeStepsToChangeOwnershipOfVmTestUpdateVmNetworkThrowsResourceAl Mockito.doThrow(ResourceAllocationException.class).when(userVmManagerImpl).updateVmNetwork(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); - Assert.assertThrows(RuntimeException.class, () -> userVmManagerImpl.executeStepsToChangeOwnershipOfVm(assignVmCmdMock, callerAccount, accountMock, accountMock, + Assert.assertThrows(CloudRuntimeException.class, () -> userVmManagerImpl.executeStepsToChangeOwnershipOfVm(assignVmCmdMock, callerAccount, accountMock, accountMock, userVmVoMock, serviceOfferingVoMock, volumes, virtualMachineTemplateMock, 1l)); Mockito.verify(userVmManagerImpl).resourceCountDecrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any()); From 5846f718f8cdcd28bd7ffe21e0b51781157dea72 Mon Sep 17 00:00:00 2001 From: Gabriel Date: Fri, 10 May 2024 09:00:32 -0300 Subject: [PATCH 3/9] fix limit count of user_vm --- server/src/main/java/com/cloud/vm/UserVmManagerImpl.java | 9 +++++---- .../test/java/com/cloud/vm/UserVmManagerImplTest.java | 6 +++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index e02faab4f042..9e535963122c 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -7312,7 +7312,7 @@ public UserVm moveVmToUser(final AssignVMCmd cmd) throws ResourceAllocationExcep final ServiceOfferingVO offering = serviceOfferingDao.findByIdIncludingRemoved(vm.getId(), vm.getServiceOfferingId()); VirtualMachineTemplate template = _templateDao.findByIdIncludingRemoved(vm.getTemplateId()); - verifyResourceLimitsForAccountAndStorage(newAccount, vm, vmId, offering, volumes, template); + verifyResourceLimitsForAccountAndStorage(newAccount, vm, offering, volumes, template); validateIfNewOwnerHasAccessToTemplate(vm, newAccount, template); @@ -7393,17 +7393,18 @@ protected void validateIfVolumesHaveNoSnapshots(List volumes) throws I * If any limit is exceeded, throws a {@link ResourceAllocationException}. * @param account The account to check if CPU and RAM limit has been exceeded. * @param vm The VM which can exceed resource limits. - * @param vmId The ID for the VM which can exceed resource limits. * @param offering The service offering which can exceed resource limits. * @param volumes The volumes whose total size can exceed resource limits. * @throws ResourceAllocationException */ - protected void verifyResourceLimitsForAccountAndStorage(Account account, UserVmVO vm, Long vmId, ServiceOfferingVO offering, List volumes, VirtualMachineTemplate template) + protected void verifyResourceLimitsForAccountAndStorage(Account account, UserVmVO vm, ServiceOfferingVO offering, List volumes, VirtualMachineTemplate template) throws ResourceAllocationException { logger.trace(String.format("Verifying if CPU and RAM for VM [%s] do not exceed account [%s] limit.", vm, account)); - resourceCountIncrement(account.getAccountId(), vm.isDisplayVm(), offering, template); + if (!VirtualMachineManager.ResourceCountRunningVMsonly.value()) { + resourceLimitService.checkVmResourceLimit(account, vm.isDisplayVm(), offering, template); + } logger.trace(String.format("Verifying if volume size for VM [%s] does not exceed account [%s] limit.", vm, account)); diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index 5dfa0c7564d5..ff62cef0bf4c 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -601,7 +601,7 @@ private void configureDoNothingForMethodsThatWeDoNotWantToTest() throws Resource Mockito.doNothing().when(userVmManagerImpl).validateOldAndNewAccounts(Mockito.nullable(Account.class), Mockito.nullable(Account.class), Mockito.anyLong(), Mockito.nullable(String.class), Mockito.nullable(Long.class)); Mockito.doNothing().when(userVmManagerImpl).validateIfVmHasNoRules(Mockito.any(), Mockito.anyLong()); Mockito.doNothing().when(userVmManagerImpl).removeInstanceFromInstanceGroup(Mockito.anyLong()); - Mockito.doNothing().when(userVmManagerImpl).verifyResourceLimitsForAccountAndStorage(Mockito.any(), Mockito.any(), Mockito.anyLong(), Mockito.any(), Mockito.anyList(), Mockito.any()); + Mockito.doNothing().when(userVmManagerImpl).verifyResourceLimitsForAccountAndStorage(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyList(), Mockito.any()); Mockito.doNothing().when(userVmManagerImpl).validateIfNewOwnerHasAccessToTemplate(Mockito.any(), Mockito.any(), Mockito.any()); Mockito.doNothing().when(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); @@ -1872,7 +1872,7 @@ public void validateIfVmHasNoRulesTestOneToOneNatRulesDoNotExistDoesNotThrowInva public void verifyResourceLimitsForAccountAndStorageTestDoesNotCallResourceLimitCheck() throws ResourceAllocationException { LinkedList volumeVoList = new LinkedList(); - userVmManagerImpl.verifyResourceLimitsForAccountAndStorage(accountMock, userVmVoMock, 1l, serviceOfferingVoMock, volumeVoList, virtualMachineTemplateMock); + userVmManagerImpl.verifyResourceLimitsForAccountAndStorage(accountMock, userVmVoMock, serviceOfferingVoMock, volumeVoList, virtualMachineTemplateMock); Mockito.verify(resourceLimitMgr, Mockito.never()).checkVmResourceLimit(Mockito.any(), Mockito.anyBoolean(), Mockito.any(), Mockito.any()); @@ -2931,7 +2931,7 @@ public void moveVmToUserTestVerifyResourceLimitsForAccountAndStorageThrowsResour configureDoNothingForMethodsThatWeDoNotWantToTest(); - Mockito.doThrow(ResourceAllocationException.class).when(userVmManagerImpl).verifyResourceLimitsForAccountAndStorage(Mockito.any(), Mockito.any(), Mockito.anyLong(), + Mockito.doThrow(ResourceAllocationException.class).when(userVmManagerImpl).verifyResourceLimitsForAccountAndStorage(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); Assert.assertThrows(ResourceAllocationException.class, () -> userVmManagerImpl.moveVmToUser(assignVmCmdMock)); From be345c7b25857c417277497b1ed212b202609ef0 Mon Sep 17 00:00:00 2001 From: Fabricio Duarte Date: Thu, 28 Nov 2024 11:24:40 -0300 Subject: [PATCH 4/9] More explicit exception --- .../api/command/admin/vm/AssignVMCmd.java | 19 ++++--------------- .../java/com/cloud/vm/UserVmManagerImpl.java | 16 +++++++++++----- 2 files changed, 15 insertions(+), 20 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java index e7f688eabb58..6f6a5237b3d1 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/AssignVMCmd.java @@ -121,26 +121,15 @@ public List getSecurityGroupIdList() { public void execute() { try { UserVm userVm = _userVmService.moveVmToUser(this); - if (userVm == null) { - throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to move vm"); - } UserVmResponse response = _responseGenerator.createUserVmResponse(ResponseView.Full, "virtualmachine", userVm).get(0); response.setResponseName(getCommandName()); setResponseObject(response); - } catch (InvalidParameterValueException e){ - e.printStackTrace(); - throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, e.getMessage()); } catch (Exception e) { - logger.error("Failed to move vm due to: " + e.getStackTrace()); - if (e.getMessage() != null) { - throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to move vm due to " + e.getMessage()); - } else if (e.getCause() != null) { - throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to move vm due to " + e.getCause()); - } else { - throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Failed to move vm"); - } + ApiErrorCode errorCode = e instanceof InvalidParameterValueException ? ApiErrorCode.PARAM_ERROR : ApiErrorCode.INTERNAL_ERROR; + String msg = String.format("Failed to move VM [%s].", getVmId()); + logger.error(msg, e); + throw new ServerApiException(errorCode, msg); } - } @Override diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 89c1536134a2..17dd416faddd 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -7450,9 +7450,7 @@ protected void validateIfVmSupportsMigration(UserVmVO vm, Long vmId) { if (vm == null) { throw new InvalidParameterValueException(String.format("There is no VM by ID [%s].", vmId)); } else if (vm.getState() == State.Running) { - String errMsg = String.format("Unable to move VM [%s] in [%s] state.", vm, vm.getState()); - logger.warn(errMsg); - throw new InvalidParameterValueException(errMsg); + throw new InvalidParameterValueException(String.format("Unable to move VM [%s] in [%s] state.", vm, vm.getState())); } else if (UserVmManager.SHAREDFSVM.equals(vm.getUserVmType())) { throw new InvalidParameterValueException("Migration is not supported for Shared FileSystem Instances."); } @@ -7527,13 +7525,19 @@ protected void verifyResourceLimitsForAccountAndStorage(Account account, UserVmV } protected void validateIfNewOwnerHasAccessToTemplate(UserVmVO vm, Account newAccount, VirtualMachineTemplate template) { - logger.trace(String.format("Validating if new owner [%s] has access to the template specified for VM [%s].", newAccount, vm)); + logger.trace("Validating if new owner [{}] has access to the template specified for VM [{}].", newAccount, vm); if (template == null) { throw new InvalidParameterValueException(String.format("Template for VM [%s] cannot be found.", vm.getUuid())); } - _accountMgr.checkAccess(newAccount, AccessType.UseEntry, true, template); + logger.debug("Verifying if new owner [{}] has access to the template [{}].", newAccount, template.getUuid()); + try { + _accountMgr.checkAccess(newAccount, AccessType.UseEntry, true, template); + } catch (PermissionDeniedException e) { + String newMsg = String.format("New owner [%s] does not have access to the template specified for VM [%s].", newAccount, vm); + throw new PermissionDeniedException(newMsg, e); + } } /** @@ -7674,6 +7678,8 @@ protected void updateVmNetwork(AssignVMCmd cmd, Account caller, UserVmVO vm, Acc protected void validateOldAndNewAccounts(Account oldAccount, Account newAccount, Long oldAccountId, String newAccountName, Long domainId) throws InvalidParameterValueException { + logger.trace("Validating old [{}] and new accounts [{}].", oldAccount, newAccount); + if (oldAccount == null) { throw new InvalidParameterValueException(String.format("Invalid old account [%s] for VM in domain [%s].", oldAccountId, domainId)); } From 1902eeeb267b27eb77a83729f4707e86758a47ce Mon Sep 17 00:00:00 2001 From: Fabricio Duarte Date: Fri, 29 Nov 2024 15:41:16 -0300 Subject: [PATCH 5/9] Replace String.format() with parametrized messages --- .../java/com/cloud/vm/UserVmManagerImpl.java | 88 +++++++++---------- 1 file changed, 43 insertions(+), 45 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 17dd416faddd..eb3c9a1b8d91 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -7393,7 +7393,7 @@ protected void checkVolumesLimits(Account account, List volumes) throw public UserVm moveVmToUser(final AssignVMCmd cmd) throws ResourceAllocationException, ConcurrentOperationException, ResourceUnavailableException, InsufficientCapacityException { Account caller = CallContext.current().getCallingAccount(); Long callerId = caller.getId(); - logger.trace(String.format("Verifying if caller [%s] is root or domain admin.", caller)); + logger.trace("Verifying if caller [{}] is root or domain admin.", caller); if (!_accountMgr.isRootAdmin(callerId) && !_accountMgr.isDomainAdmin(callerId)) { throw new InvalidParameterValueException(String.format("Only root or domain admins are allowed to assign VMs. Caller [%s] is of type [%s].", caller, caller.getType())); } @@ -7412,7 +7412,7 @@ public UserVm moveVmToUser(final AssignVMCmd cmd) throws ResourceAllocationExcep checkCallerAccessToAccounts(caller, oldAccount, newAccount); - logger.trace(String.format("Verifying if the provided domain ID [%s] is valid.", domainId)); + logger.trace("Verifying if the provided domain ID [{}] is valid.", domainId); if (projectId != null && domainId == null) { throw new InvalidParameterValueException("Please provide a valid domain ID; cannot assign VM to a project if domain ID is NULL."); } @@ -7430,7 +7430,7 @@ public UserVm moveVmToUser(final AssignVMCmd cmd) throws ResourceAllocationExcep validateIfNewOwnerHasAccessToTemplate(vm, newAccount, template); DomainVO domain = _domainDao.findById(domainId); - logger.trace(String.format("Verifying if the new account [%s] has access to the specified domain [%s].", newAccount, domain)); + logger.trace("Verifying if the new account [{}] has access to the specified domain [{}].", newAccount, domain); _accountMgr.checkAccess(newAccount, domain); Transaction.execute(new TransactionCallbackNoReturn() { @@ -7440,12 +7440,12 @@ public void doInTransactionWithoutResult(TransactionStatus status) { } }); - logger.info(String.format("VM [%s] now belongs to account [%s].", vm.getInstanceName(), newAccountName)); + logger.info("VM [{}] now belongs to account [{}].", vm.getInstanceName(), newAccountName); return vm; } protected void validateIfVmSupportsMigration(UserVmVO vm, Long vmId) { - logger.trace(String.format("Validating if VM [%s] exists and is not in state [%s].", vmId, State.Running)); + logger.trace("Validating if VM [{}] exists and is not in state [{}].", vmId, State.Running); if (vm == null) { throw new InvalidParameterValueException(String.format("There is no VM by ID [%s].", vmId)); @@ -7464,7 +7464,7 @@ protected void validateIfVmSupportsMigration(UserVmVO vm, Long vmId) { * @throws InvalidParameterValueException */ protected void validateIfVmHasNoRules(UserVmVO vm, Long vmId) throws InvalidParameterValueException { - logger.trace(String.format("Validating if VM [%s] has no Port Forwarding, Static Nat, Load Balancing or One to One Nat rules.", vm)); + logger.trace("Validating if VM [{}] has no Port Forwarding, Static Nat, Load Balancing or One to One Nat rules.", vm); List portForwardingRules = _portForwardingDao.listByVm(vmId); if (CollectionUtils.isNotEmpty(portForwardingRules)) { @@ -7492,7 +7492,7 @@ protected void validateIfVmHasNoRules(UserVmVO vm, Long vmId) throws InvalidPara protected void validateIfVolumesHaveNoSnapshots(List volumes) throws InvalidParameterValueException { logger.trace("Verifying if there are any snapshots for any of the VM volumes."); for (VolumeVO volume : volumes) { - logger.trace(String.format("Verifying snapshots for volume [%s].", volume)); + logger.trace("Verifying snapshots for volume [{}].", volume); List snapshots = _snapshotDao.listByStatusNotIn(volume.getId(), Snapshot.State.Destroyed, Snapshot.State.Error); if (CollectionUtils.isNotEmpty(snapshots)) { throw new InvalidParameterValueException(String.format("Snapshots exist for volume [%s]. Detach volume or remove snapshots for the volume before assigning VM to " @@ -7513,13 +7513,13 @@ protected void validateIfVolumesHaveNoSnapshots(List volumes) throws I protected void verifyResourceLimitsForAccountAndStorage(Account account, UserVmVO vm, ServiceOfferingVO offering, List volumes, VirtualMachineTemplate template) throws ResourceAllocationException { - logger.trace(String.format("Verifying if CPU and RAM for VM [%s] do not exceed account [%s] limit.", vm, account)); + logger.trace("Verifying if CPU and RAM for VM [{}] do not exceed account [{}] limit.", vm, account); if (!VirtualMachineManager.ResourceCountRunningVMsonly.value()) { resourceLimitService.checkVmResourceLimit(account, vm.isDisplayVm(), offering, template); } - logger.trace(String.format("Verifying if volume size for VM [%s] does not exceed account [%s] limit.", vm, account)); + logger.trace("Verifying if volume size for VM [{}] does not exceed account [{}] limit.", vm, account); checkVolumesLimits(account, volumes); } @@ -7563,14 +7563,14 @@ protected void validateIfNewOwnerHasAccessToTemplate(UserVmVO vm, Account newAcc protected void executeStepsToChangeOwnershipOfVm(AssignVMCmd cmd, Account caller, Account oldAccount, Account newAccount, UserVmVO vm, ServiceOfferingVO offering, List volumes, VirtualMachineTemplate template, Long domainId) { - logger.trace(String.format("Generating destroy event for VM [%s].", vm)); + logger.trace("Generating destroy event for VM [{}].", vm); UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_DESTROY, vm.getAccountId(), vm.getDataCenterId(), vm.getId(), vm.getHostName(), vm.getServiceOfferingId(), vm.getTemplateId(), vm.getHypervisorType().toString(), VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm()); - logger.trace(String.format("Decrementing old account [%s] resource count.", oldAccount)); + logger.trace("Decrementing old account [{}] resource count.", oldAccount); resourceCountDecrement(oldAccount.getAccountId(), vm.isDisplayVm(), offering, template); - logger.trace(String.format("Removing VM [%s] from its instance group.", vm)); + logger.trace("Removing VM [{}] from its instance group.", vm); removeInstanceFromInstanceGroup(vm.getId()); Long newAccountId = newAccount.getAccountId(); @@ -7595,7 +7595,7 @@ protected void executeStepsToChangeOwnershipOfVm(AssignVMCmd cmd, Account caller } protected void updateVmOwner(Account newAccount, UserVmVO vm, Long domainId, Long newAccountId) { - logger.debug(String.format("Updating VM [%s] owner to [%s].", vm, newAccount)); + logger.debug("Updating VM [{}] owner to [{}].", vm, newAccount); vm.setAccountId(newAccountId); vm.setDomainId(domainId); @@ -7604,27 +7604,27 @@ protected void updateVmOwner(Account newAccount, UserVmVO vm, Long domainId, Lon } protected void updateVolumesOwner(final List volumes, Account oldAccount, Account newAccount, Long newAccountId) { - logger.debug(String.format("Updating volumes owner from old account [%s] to new account [%s].", oldAccount, newAccount)); + logger.debug("Updating volumes owner from old account [{}] to new account [{}].", oldAccount, newAccount); for (VolumeVO volume : volumes) { - logger.trace(String.format("Generating a delete volume event for volume [%s].", volume)); + logger.trace("Generating a delete volume event for volume [{}].", volume); UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VOLUME_DELETE, volume.getAccountId(), volume.getDataCenterId(), volume.getId(), volume.getName(), Volume.class.getName(), volume.getUuid(), volume.isDisplayVolume()); - logger.trace(String.format("Decrementing volume [%s] and primary storage resource count for the old account [%s].", volume, oldAccount)); + logger.trace("Decrementing volume [{}] and primary storage resource count for the old account [{}].", volume, oldAccount); DiskOfferingVO diskOfferingVO = _diskOfferingDao.findById(volume.getDiskOfferingId()); _resourceLimitMgr.decrementVolumeResourceCount(oldAccount.getAccountId(), volume.isDisplay(), volume.getSize(), diskOfferingVO); - logger.trace(String.format("Setting the new account [%s] and domain [%s] for volume [%s].", newAccount, newAccount.getDomainId(), volume)); + logger.trace("Setting the new account [{}] and domain [{}] for volume [{}].", newAccount, newAccount.getDomainId(), volume); volume.setAccountId(newAccountId); volume.setDomainId(newAccount.getDomainId()); _volsDao.persist(volume); - logger.trace(String.format("Incrementing volume [%s] and primary storage resource count for the new account [%s].", volume, newAccount)); + logger.trace("Incrementing volume [{}] and primary storage resource count for the new account [{}].", volume, newAccount); _resourceLimitMgr.incrementVolumeResourceCount(newAccount.getAccountId(), volume.isDisplay(), volume.getSize(), diskOfferingVO); - logger.trace(String.format("Generating a create volume event for volume [%s].", volume)); + logger.trace("Generating a create volume event for volume [{}].", volume); UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VOLUME_CREATE, volume.getAccountId(), volume.getDataCenterId(), volume.getId(), volume.getName(), volume.getDiskOfferingId(), volume.getTemplateId(), volume.getSize(), Volume.class.getName(), volume.getUuid(), volume.isDisplayVolume()); } @@ -7647,7 +7647,7 @@ protected void updateVolumesOwner(final List volumes, Account oldAccou protected void updateVmNetwork(AssignVMCmd cmd, Account caller, UserVmVO vm, Account newAccount, VirtualMachineTemplate template) throws InsufficientCapacityException, ResourceAllocationException { - logger.trace(String.format("Updating network for VM [%s].", vm)); + logger.trace("Updating network for VM [{}].", vm); VirtualMachine vmoi = _itMgr.findById(vm.getId()); VirtualMachineProfileImpl vmOldProfile = new VirtualMachineProfileImpl(vmoi); @@ -7698,10 +7698,10 @@ protected void validateOldAndNewAccounts(Account oldAccount, Account newAccount, } protected void checkCallerAccessToAccounts(Account caller, Account oldAccount, Account newAccount) { - logger.trace(String.format("Verifying if caller [%s] has access to old account [%s].", caller, oldAccount)); + logger.trace("Verifying if caller [{}] has access to old account [{}].", caller, oldAccount); _accountMgr.checkAccess(caller, null, true, oldAccount); - logger.trace(String.format("Verifying if caller [%s] has access to new account [%s].", caller, newAccount)); + logger.trace("Verifying if caller [{}] has access to new account [{}].", caller, newAccount); _accountMgr.checkAccess(caller, null, true, newAccount); } @@ -7732,19 +7732,19 @@ protected void updateBasicTypeNetworkForVm(AssignVMCmd cmd, UserVmVO vm, Account throw new InvalidParameterValueException("Cannot move VM with Network IDs; this is a basic zone VM."); } - logger.trace(String.format("Cleanup of old security groups for VM [%s]. They will be recreated for the new account once the VM is started.", vm)); + logger.trace("Cleanup of old security groups for VM [{}]. They will be recreated for the new account once the VM is started.", vm); _securityGroupMgr.removeInstanceFromGroups(cmd.getVmId()); cleanupOfOldOwnerNicsForNetwork(vmOldProfile); - List networkList = new ArrayList(); + List networkList = new ArrayList<>(); Network defaultNetwork = _networkModel.getExclusiveGuestNetwork(zone.getId()); addDefaultNetworkToNetworkList(networkList, defaultNetwork); - LinkedHashMap> networks = new LinkedHashMap>(); + LinkedHashMap> networks = new LinkedHashMap<>(); NicProfile profile = new NicProfile(); profile.setDefaultNic(true); - networks.put(networkList.get(0), new ArrayList(Arrays.asList(profile))); + networks.put(networkList.get(0), new ArrayList<>(Arrays.asList(profile))); allocateNetworksForVm(vm, networks); @@ -7782,7 +7782,7 @@ protected void updateAdvancedTypeNetworkForVm(AssignVMCmd cmd, Account caller, U LinkedHashMap> networks = new LinkedHashMap<>(); if (_networkModel.checkSecurityGroupSupportForNetwork(newAccount, zone, networkIdList, securityGroupIdList)) { - logger.debug(String.format("Cleanup of old security groups for VM [%s]. They will be recreated for the new account once the VM is started.", vm)); + logger.debug("Cleanup of old security groups for VM [{}]. They will be recreated for the new account once the VM is started.", vm); _securityGroupMgr.removeInstanceFromGroups(cmd.getVmId()); addNetworksToNetworkIdList(vm, newAccount, vmOldProfile, networkIdList, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); @@ -7812,11 +7812,11 @@ protected void updateAdvancedTypeNetworkForVm(AssignVMCmd cmd, Account caller, U addNicsToApplicableNetworksAndReturnDefaultNetwork(applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics, networks); allocateNetworksForVm(vm, networks); - logger.debug(String.format("Adding [%s] networks to VM [%s].", networks.size(), vm.getInstanceName())); + logger.debug("Adding [{}] networks to VM [{}].", networks.size(), vm.getInstanceName()); } protected void cleanupOfOldOwnerNicsForNetwork(VirtualMachineProfileImpl vmOldProfile) { - logger.trace(String.format("Cleanup of old owner network for VM [%s].", vmOldProfile)); + logger.trace("Cleanup of old owner network for VM [{}].", vmOldProfile); _networkMgr.cleanupNics(vmOldProfile); _networkMgr.removeNics(vmOldProfile); @@ -7833,7 +7833,7 @@ protected void addDefaultNetworkToNetworkList(List networkList, Netwo } protected void allocateNetworksForVm(UserVmVO vm, LinkedHashMap> networks) throws InsufficientCapacityException { - logger.trace(String.format("Allocating networks for VM [%s].", vm)); + logger.trace("Allocating networks for VM [{}].", vm); VirtualMachine vmi = _itMgr.findById(vm.getId()); VirtualMachineProfileImpl vmProfile = new VirtualMachineProfileImpl(vmi); @@ -7849,7 +7849,7 @@ protected void addSecurityGroupsToVm(Account newAccount, UserVmVO vm, VirtualMac throw new InvalidParameterValueException("Security group feature is not supported for VMWare hypervisor."); } else if (!isVmWare && (defaultNetwork == null || _networkModel.isSecurityGroupSupportedInNetwork(defaultNetwork)) && _networkModel.canAddDefaultSecurityGroup()) { if (securityGroupIdList == null) { - securityGroupIdList = new ArrayList(); + securityGroupIdList = new ArrayList<>(); } addDefaultSecurityGroupToSecurityGroupIdList(newAccount, securityGroupIdList); @@ -7907,7 +7907,7 @@ protected NetworkVO addNicsToApplicableNetworksAndReturnDefaultNetwork(LinkedHas for (NetworkVO appNet : applicableNetworks) { defaultNic.setRequestedIPv4(requestedIPv4ForNics.get(appNet.getId())); defaultNic.setRequestedIPv6(requestedIPv6ForNics.get(appNet.getId())); - networks.put(appNet, new ArrayList(Arrays.asList(defaultNic))); + networks.put(appNet, new ArrayList<>(Arrays.asList(defaultNic))); defaultNic = new NicProfile(); } @@ -7976,7 +7976,7 @@ protected void addDefaultSecurityGroupToSecurityGroupIdList(Account newAccount, defaultGroupPresent = true; } } else { - logger.debug(String.format("Could not find a default security group for account [%s]. Creating a new one.", newAccount)); + logger.debug("Could not find a default security group for account [{}]. Creating a new one.", newAccount); defaultGroup = _securityGroupMgr.createSecurityGroup(SecurityGroupManager.DEFAULT_GROUP_NAME, SecurityGroupManager.DEFAULT_GROUP_DESCRIPTION, newAccount.getDomainId(), newAccountId, newAccount.getAccountName()); } @@ -7999,7 +7999,7 @@ protected void addDefaultSecurityGroupToSecurityGroupIdList(Account newAccount, */ protected void keepOldSharedNetworkForVm(UserVmVO vm, Account newAccount, List networkIdList, Set applicableNetworks, Map requestedIPv4ForNics, Map requestedIPv6ForNics) { - logger.trace(String.format("Attempting to keep old shared network for VM [%s].", vm)); + logger.trace("Attempting to keep old shared network for VM [{}].", vm); if (CollectionUtils.isNotEmpty(networkIdList)) { return; @@ -8018,8 +8018,7 @@ protected void keepOldSharedNetworkForVm(UserVmVO vm, Account newAccount, List networkIdList, Set applicableNetworks, Map requestedIPv4ForNics, Map requestedIPv6ForNics) { - logger.trace(String.format("Adding additional networks to VM [%s].", vm)); + logger.trace("Adding additional networks to VM [{}].", vm); if (CollectionUtils.isEmpty(networkIdList)) { return; @@ -8062,10 +8061,10 @@ protected void addAdditionalNetworksToVm(UserVmVO vm, Account newAccount, List implementedNetwork = _networkMgr.implementNetwork(newNetwork.getId(), dest, context); if (implementedNetwork == null || implementedNetwork.first() == null || implementedNetwork.second() == null) { - logger.warn(String.format("Failed to implement network [%s].", newNetwork)); + logger.warn("Failed to implement network [{}].", newNetwork); } else { newNetwork = implementedNetwork.second(); } } catch (Exception ex) { - logger.warn(String.format("Failed to implement network [%s] elements and resources as a part of network provision for persistent network due to [%s].", newNetwork, - ex.getMessage()), ex); + logger.warn("Failed to implement network [{}] elements and resources as a part of network provision for persistent network due to [{}].", newNetwork, ex.getMessage(), ex); throw new CloudRuntimeException(String.format("Failed to implement network [%s] elements and resources as a part of network provision.", newNetwork)); } From d15a4f5c610fe78d372a6d94ec30884b2e7d3a5c Mon Sep 17 00:00:00 2001 From: Fabricio Duarte Date: Fri, 29 Nov 2024 16:19:25 -0300 Subject: [PATCH 6/9] Add missing import --- server/src/main/java/com/cloud/vm/UserVmManagerImpl.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index eb3c9a1b8d91..c6cb34f6c650 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -143,6 +143,7 @@ import org.apache.commons.lang3.builder.ToStringStyle; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; import org.w3c.dom.Document; From 32ae56ee915939f546dfcef2bdf99a9f498154ab Mon Sep 17 00:00:00 2001 From: Fabricio Duarte Date: Fri, 29 Nov 2024 16:31:33 -0300 Subject: [PATCH 7/9] Fix build --- .../test/java/com/cloud/vm/UserVmManagerImplTest.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index 97d95eb67518..f73a78b3654a 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -2471,10 +2471,12 @@ public void createApplicableNetworkToCreateVmTestFirstNetworkOfferingIsNotPersis Mockito.doReturn(physicalNetworkVo).when(physicalNetworkDaoMock).findById(Mockito.anyLong()); Mockito.doReturn(false).when(networkOfferingVoMock).isPersistent(); - Mockito.doReturn(networkMock).when(_networkMgr).createGuestNetwork(Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), Mockito.any(), Mockito.any(), - Mockito.any(), Mockito.anyBoolean(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyLong(), Mockito.any(), - Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyBoolean(), Mockito.any(), Mockito.any(), Mockito.any(), - Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); + Mockito.doReturn(networkMock).when(_networkMgr).createGuestNetwork(Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), + Mockito.anyString(), Mockito.anyString(), Mockito.anyString(), Mockito.anyBoolean(), Mockito.anyString(), + Mockito.any(), Mockito.anyLong(), Mockito.any(), Mockito.anyLong(), Mockito.any(), Mockito.anyBoolean(), + Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), Mockito.anyBoolean(), Mockito.anyString(), + Mockito.any(), Mockito.anyString(), Mockito.anyString(), Mockito.anyString(), Mockito.anyString(), Mockito.anyString(), + Mockito.anyString(), Mockito.anyString(), Mockito.any(), Mockito.anyInt()); Mockito.doReturn(1l).when(networkMock).getId(); Mockito.doReturn(networkMock).when(_networkDao).findById(Mockito.anyLong()); From 1c79d0d6d4d5d0ac49a0b57ea5a24efb219dedeb Mon Sep 17 00:00:00 2001 From: Fabricio Duarte Date: Fri, 29 Nov 2024 17:37:34 -0300 Subject: [PATCH 8/9] Fix tests --- .../java/com/cloud/vm/UserVmManagerImpl.java | 6 ++- .../com/cloud/vm/UserVmManagerImplTest.java | 40 ++++++++++++------- 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index c6cb34f6c650..141ea75a6455 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -7516,7 +7516,7 @@ protected void verifyResourceLimitsForAccountAndStorage(Account account, UserVmV logger.trace("Verifying if CPU and RAM for VM [{}] do not exceed account [{}] limit.", vm, account); - if (!VirtualMachineManager.ResourceCountRunningVMsonly.value()) { + if (!countOnlyRunningVmsInResourceLimitation()) { resourceLimitService.checkVmResourceLimit(account, vm.isDisplayVm(), offering, template); } @@ -7525,6 +7525,10 @@ protected void verifyResourceLimitsForAccountAndStorage(Account account, UserVmV checkVolumesLimits(account, volumes); } + protected boolean countOnlyRunningVmsInResourceLimitation() { + return VirtualMachineManager.ResourceCountRunningVMsonly.value(); + } + protected void validateIfNewOwnerHasAccessToTemplate(UserVmVO vm, Account newAccount, VirtualMachineTemplate template) { logger.trace("Validating if new owner [{}] has access to the template specified for VM [{}].", newAccount, vm); diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index f73a78b3654a..2db1612755a5 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -41,6 +41,7 @@ import java.util.Map; import org.apache.cloudstack.acl.ControlledEntity; +import org.apache.cloudstack.acl.SecurityChecker; import org.apache.cloudstack.api.BaseCmd.HTTPMethod; import org.apache.cloudstack.api.command.admin.vm.AssignVMCmd; import org.apache.cloudstack.api.command.user.vm.DeployVMCmd; @@ -1916,13 +1917,25 @@ public void validateIfVmHasNoRulesTestOneToOneNatRulesDoNotExistDoesNotThrowInva } @Test - public void verifyResourceLimitsForAccountAndStorageTestDoesNotCallResourceLimitCheck() throws ResourceAllocationException { + public void verifyResourceLimitsForAccountAndStorageTestCountOnlyRunningVmsInResourceLimitationIsTrueDoesNotCallVmResourceLimitCheck() throws ResourceAllocationException { LinkedList volumeVoList = new LinkedList(); + Mockito.doReturn(true).when(userVmManagerImpl).countOnlyRunningVmsInResourceLimitation(); userVmManagerImpl.verifyResourceLimitsForAccountAndStorage(accountMock, userVmVoMock, serviceOfferingVoMock, volumeVoList, virtualMachineTemplateMock); - Mockito.verify(resourceLimitMgr, Mockito.never()).checkVmResourceLimit(Mockito.any(), Mockito.anyBoolean(), Mockito.any(), Mockito.any()); + Mockito.verify(resourceLimitMgr, Mockito.never()).checkVmResourceLimit(Mockito.any(), Mockito.anyBoolean(), Mockito.any(), Mockito.any()); + Mockito.verify(resourceLimitMgr).checkResourceLimit(accountMock, Resource.ResourceType.volume, 0l); + Mockito.verify(resourceLimitMgr).checkResourceLimit(accountMock, Resource.ResourceType.primary_storage, 0l); + } + @Test + public void verifyResourceLimitsForAccountAndStorageTestCountOnlyRunningVmsInResourceLimitationIsFalseCallsVmResourceLimitCheck() throws ResourceAllocationException { + LinkedList volumeVoList = new LinkedList(); + Mockito.doReturn(false).when(userVmManagerImpl).countOnlyRunningVmsInResourceLimitation(); + + userVmManagerImpl.verifyResourceLimitsForAccountAndStorage(accountMock, userVmVoMock, serviceOfferingVoMock, volumeVoList, virtualMachineTemplateMock); + + Mockito.verify(resourceLimitMgr).checkVmResourceLimit(Mockito.any(), Mockito.anyBoolean(), Mockito.any(), Mockito.any()); Mockito.verify(resourceLimitMgr).checkResourceLimit(accountMock, Resource.ResourceType.volume, 0l); Mockito.verify(resourceLimitMgr).checkResourceLimit(accountMock, Resource.ResourceType.primary_storage, 0l); } @@ -1940,11 +1953,9 @@ public void validateIfNewOwnerHasAccessToTemplateTestTemplateIsNullThrowsInvalid @Test public void validateIfNewOwnerHasAccessToTemplateTestCallCheckAccessWhenTemplateIsNotPublic() { - Mockito.doReturn(accountMock).when(accountManager).getAccount(Mockito.anyLong()); - userVmManagerImpl.validateIfNewOwnerHasAccessToTemplate(userVmVoMock, accountMock, virtualMachineTemplateMock); - Mockito.verify(accountManager).checkAccess(accountMock, null, true, accountMock); + Mockito.verify(accountManager).checkAccess(accountMock, SecurityChecker.AccessType.UseEntry, true, virtualMachineTemplateMock); } @Test @@ -2472,11 +2483,10 @@ public void createApplicableNetworkToCreateVmTestFirstNetworkOfferingIsNotPersis Mockito.doReturn(physicalNetworkVo).when(physicalNetworkDaoMock).findById(Mockito.anyLong()); Mockito.doReturn(false).when(networkOfferingVoMock).isPersistent(); Mockito.doReturn(networkMock).when(_networkMgr).createGuestNetwork(Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), - Mockito.anyString(), Mockito.anyString(), Mockito.anyString(), Mockito.anyBoolean(), Mockito.anyString(), - Mockito.any(), Mockito.anyLong(), Mockito.any(), Mockito.anyLong(), Mockito.any(), Mockito.anyBoolean(), - Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), Mockito.anyBoolean(), Mockito.anyString(), - Mockito.any(), Mockito.anyString(), Mockito.anyString(), Mockito.anyString(), Mockito.anyString(), Mockito.anyString(), - Mockito.anyString(), Mockito.anyString(), Mockito.any(), Mockito.anyInt()); + Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyBoolean(), Mockito.any(), Mockito.any(), Mockito.any(), + Mockito.any(), Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), + Mockito.anyBoolean(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), + Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); Mockito.doReturn(1l).when(networkMock).getId(); Mockito.doReturn(networkMock).when(_networkDao).findById(Mockito.anyLong()); @@ -2713,7 +2723,7 @@ public void updateAdvancedTypeNetworkForVmTestSecurityGroupIsEnabledApplicableNe LinkedList securityGroupIdList = Mockito.mock(LinkedList.class); LinkedList networkIdList = new LinkedList(); - Mockito.doReturn(true).when(_dcMock).isSecurityGroupEnabled(); + Mockito.doReturn(true).when(networkModel).checkSecurityGroupSupportForNetwork(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { userVmManagerImpl.updateAdvancedTypeNetworkForVm(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, @@ -2735,7 +2745,7 @@ public void updateAdvancedTypeNetworkForVmTestSecurityGroupIsEnabledApplicableNe Mockito.doReturn(networkMock).when(_networkDao).findById(Mockito.anyLong()); Mockito.doReturn(true).when(userVmManagerImpl).canAccountUseNetwork(accountMock, networkMock); - Mockito.doReturn(true).when(_dcMock).isSecurityGroupEnabled(); + Mockito.doReturn(true).when(networkModel).checkSecurityGroupSupportForNetwork(accountMock, _dcMock, networkIdList, securityGroupIdList); userVmManagerImpl.updateAdvancedTypeNetworkForVm(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, networkIdList, securityGroupIdList); @@ -2753,7 +2763,7 @@ public void updateAdvancedTypeNetworkForVmTestSecurityGroupIsNotEnabledSecurityG securityGroupIdList.add(1l); - Mockito.doReturn(false).when(_dcMock).isSecurityGroupEnabled(); + Mockito.doReturn(false).when(networkModel).checkSecurityGroupSupportForNetwork(accountMock, _dcMock, networkIdList, securityGroupIdList); InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { userVmManagerImpl.updateAdvancedTypeNetworkForVm(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, @@ -2773,7 +2783,7 @@ public void updateAdvancedTypeNetworkForVmTestSecurityGroupIsNotEnabledApplicabl Mockito.doReturn(networkMock).when(userVmManagerImpl).addNicsToApplicableNetworksAndReturnDefaultNetwork(Mockito.any(), Mockito.anyMap(), Mockito.anyMap(), Mockito.any()); Mockito.doNothing().when(userVmManagerImpl).selectApplicableNetworkToCreateVm(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); - Mockito.doReturn(false).when(_dcMock).isSecurityGroupEnabled(); + Mockito.doReturn(false).when(networkModel).checkSecurityGroupSupportForNetwork(accountMock, _dcMock, networkIdList, securityGroupIdList); Mockito.doReturn(true).when(securityGroupIdList).isEmpty(); userVmManagerImpl.updateAdvancedTypeNetworkForVm(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, @@ -2792,7 +2802,7 @@ public void updateAdvancedTypeNetworkForVmTestSecurityGroupIsNotEnabledApplicabl LinkedList securityGroupIdList = Mockito.mock(LinkedList.class); LinkedList networkIdList = new LinkedList(); - Mockito.doReturn(false).when(_dcMock).isSecurityGroupEnabled(); + Mockito.doReturn(false).when(networkModel).checkSecurityGroupSupportForNetwork(accountMock, _dcMock, networkIdList, securityGroupIdList); Mockito.doReturn(true).when(securityGroupIdList).isEmpty(); Mockito.doReturn(new NicVO()).when(nicDao).findDefaultNicForVM(Mockito.anyLong()); From 7d882a99765e7dbfcde08061703d8527076cbc63 Mon Sep 17 00:00:00 2001 From: Fabricio Duarte Date: Wed, 8 Jan 2025 11:18:09 -0300 Subject: [PATCH 9/9] Fix build --- .../src/test/java/com/cloud/vm/UserVmManagerImplTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index 1a673ea66665..f07d2af21af2 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -657,7 +657,7 @@ public void validateOrReplaceMacAddressTestMacAddressNotValidOption4() throws In private void configureValidateOrReplaceMacAddressTest(int times, String macAddress, String expectedMacAddress) throws InsufficientAddressCapacityException { Mockito.when(networkModel.getNextAvailableMacAddressInNetwork(Mockito.anyLong())).thenReturn(expectedMacAddress); - String returnedMacAddress = userVmManagerImpl.validateOrReplaceMacAddress(macAddress, _networkMock); + String returnedMacAddress = userVmManagerImpl.validateOrReplaceMacAddress(macAddress, networkMock); Mockito.verify(networkModel, Mockito.times(times)).getNextAvailableMacAddressInNetwork(Mockito.anyLong()); assertEquals(expectedMacAddress, returnedMacAddress); @@ -2054,7 +2054,7 @@ public void addSecurityGroupsToVmTestIsNotVmWareDefaultNetworkIsNullAndNetworkMo userVmManagerImpl.addSecurityGroupsToVm(accountMock, userVmVoMock, virtualMachineTemplateMock, securityGroupIdList, null); Mockito.verify(userVmManagerImpl).addDefaultSecurityGroupToSecurityGroupIdList(accountMock, securityGroupIdList); - Mockito.verify(securityGroupManagerMock).addInstanceToGroups(Mockito.anyLong(), Mockito.any()); + Mockito.verify(securityGroupManagerMock).addInstanceToGroups(Mockito.any(), Mockito.any()); } @Test @@ -2729,7 +2729,7 @@ public void updateAdvancedTypeNetworkForVmTestSecurityGroupIsEnabledApplicableNe _dcMock, networkIdList, securityGroupIdList); }); - Mockito.verify(securityGroupManagerMock).removeInstanceFromGroups(Mockito.anyLong()); + Mockito.verify(securityGroupManagerMock).removeInstanceFromGroups(Mockito.any()); Assert.assertEquals(expectedMessage, assertThrows.getMessage()); } @@ -2749,7 +2749,7 @@ public void updateAdvancedTypeNetworkForVmTestSecurityGroupIsEnabledApplicableNe userVmManagerImpl.updateAdvancedTypeNetworkForVm(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, networkIdList, securityGroupIdList); - Mockito.verify(securityGroupManagerMock).removeInstanceFromGroups(Mockito.anyLong()); + Mockito.verify(securityGroupManagerMock).removeInstanceFromGroups(Mockito.any()); Mockito.verify(userVmManagerImpl).allocateNetworksForVm(Mockito.any(), Mockito.any()); Mockito.verify(userVmManagerImpl).addSecurityGroupsToVm(accountMock, userVmVoMock, virtualMachineTemplateMock, securityGroupIdList, networkMock); }