diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 69d64a4153c..0abab9b149d 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -54,6 +54,7 @@ import javax.naming.ConfigurationException; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.ParserConfigurationException; +import com.cloud.network.NetworkService; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.ControlledEntity.ACLType; import org.apache.cloudstack.acl.SecurityChecker.AccessType; @@ -605,6 +606,9 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir @Inject NsxProviderDao nsxProviderDao; + @Inject + NetworkService networkService; + private ScheduledExecutorService _executor = null; private ScheduledExecutorService _vmIpFetchExecutor = null; @@ -7445,12 +7449,21 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir logger.trace("Verifying if the new account [{}] has access to the specified domain [{}].", 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); + Network newNetwork = ensureDestinationNetwork(cmd, vm, newAccount); + try { + Transaction.execute(new TransactionCallbackNoReturn() { + @Override + public void doInTransactionWithoutResult(TransactionStatus status) { + executeStepsToChangeOwnershipOfVm(cmd, caller, oldAccount, newAccount, vm, offering, volumes, template, domainId); + } + }); + } catch (Exception e) { + if (newNetwork != null) { + logger.debug("Cleaning up the created network."); + networkService.deleteNetwork(newNetwork.getId(), false); } - }); + throw e; + } logger.info("VM [{}] now belongs to account [{}].", vm.getInstanceName(), newAccountName); return vm; @@ -7556,6 +7569,67 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } } + /** + * This method will create an isolated network for the new account to allocate the virtual machine if: + * + * @return the created isolated network, or null if it was not created. + */ + protected Network ensureDestinationNetwork(AssignVMCmd cmd, UserVmVO vm, Account newAccount) throws InsufficientCapacityException, ResourceAllocationException { + DataCenterVO zone = _dcDao.findById(vm.getDataCenterId()); + if (zone.getNetworkType() == NetworkType.Basic) { + logger.debug("No need to ensure an isolated network for the VM because the zone uses basic networks."); + return null; + } + + List networkIdList = cmd.getNetworkIds(); + List securityGroupIdList = cmd.getSecurityGroupIdList(); + if (_networkModel.checkSecurityGroupSupportForNetwork(newAccount, zone, networkIdList, securityGroupIdList)) { + logger.debug("No need to ensure an isolated network for the VM because security groups is enabled for this zone."); + return null; + } + if (CollectionUtils.isNotEmpty(securityGroupIdList)) { + throw new InvalidParameterValueException("Cannot move VM with security groups; security group feature is not enabled in this zone."); + } + + LinkedHashSet applicableNetworks = new LinkedHashSet<>(); + addNetworksToNetworkIdList(vm, newAccount, networkIdList, applicableNetworks, new HashMap<>(), new HashMap<>()); + if (!applicableNetworks.isEmpty()) { + logger.debug("No need to create an isolated network for the VM because there are other applicable networks."); + return null; + } + + List virtualNetworks = _networkModel.listNetworksForAccount(newAccount.getId(), zone.getId(), Network.GuestType.Isolated); + if (!virtualNetworks.isEmpty()) { + logger.debug("No need create a new isolated network for the VM because the owner already has existing isolated networks."); + return null; + } + + return createApplicableNetworkToCreateVm(newAccount, zone); + } + + /** + * @return a network offering with required availability that will be used to create a new isolated network for the VM + * assignment process. + */ + protected NetworkOfferingVO getOfferingWithRequiredAvailabilityForNetworkCreation() { + 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)); + } + 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)); + } + return firstRequiredOffering; + } + /** * Executes all ownership steps necessary to assign a VM to another user: * generating a destroy VM event ({@link EventTypes}), @@ -7649,9 +7723,9 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir /** * 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)}. + * {@link #updateBasicTypeNetworkForVm(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)}. + * {@link #updateAdvancedTypeNetworkForVm(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. @@ -7674,11 +7748,11 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir List securityGroupIdList = cmd.getSecurityGroupIdList(); if (zone.getNetworkType() == NetworkType.Basic) { - updateBasicTypeNetworkForVm(cmd, vm, newAccount, template, vmOldProfile, zone, networkIdList, securityGroupIdList); + updateBasicTypeNetworkForVm(vm, newAccount, template, vmOldProfile, zone, networkIdList, securityGroupIdList); return; } - updateAdvancedTypeNetworkForVm(cmd, caller, vm, newAccount, template, vmOldProfile, zone, networkIdList, securityGroupIdList); + updateAdvancedTypeNetworkForVm(caller, vm, newAccount, template, vmOldProfile, zone, networkIdList, securityGroupIdList); } /** @@ -7731,7 +7805,6 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir * 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. @@ -7741,7 +7814,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir * @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, + protected void updateBasicTypeNetworkForVm(UserVmVO vm, Account newAccount, VirtualMachineTemplate template, VirtualMachineProfileImpl vmOldProfile, DataCenterVO zone, List networkIdList, List securityGroupIdList) throws InsufficientCapacityException { if (networkIdList != null && !networkIdList.isEmpty()) { @@ -7771,11 +7844,10 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir * 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)}), + * or selecting applicable networks otherwise ({@link #selectApplicableNetworkToCreateVm(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. @@ -7788,7 +7860,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir * @throws ResourceAllocationException * @throws InvalidParameterValueException */ - protected void updateAdvancedTypeNetworkForVm(AssignVMCmd cmd, Account caller, UserVmVO vm, Account newAccount, VirtualMachineTemplate template, + protected void updateAdvancedTypeNetworkForVm(Account caller, UserVmVO vm, Account newAccount, VirtualMachineTemplate template, VirtualMachineProfileImpl vmOldProfile, DataCenterVO zone, List networkIdList, List securityGroupIdList) throws InsufficientCapacityException, ResourceAllocationException, InvalidParameterValueException { @@ -7801,7 +7873,8 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir 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(vm); - addNetworksToNetworkIdList(vm, newAccount, vmOldProfile, networkIdList, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); + addNetworksToNetworkIdList(vm, newAccount, networkIdList, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); + cleanupOfOldOwnerNicsForNetwork(vmOldProfile); NetworkVO defaultNetwork = addNicsToApplicableNetworksAndReturnDefaultNetwork(applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics, networks); @@ -7819,10 +7892,11 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir throw new InvalidParameterValueException("Cannot move VM with security groups; security group feature is not enabled in this zone."); } - addNetworksToNetworkIdList(vm, newAccount, vmOldProfile, networkIdList, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); + addNetworksToNetworkIdList(vm, newAccount, networkIdList, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); + cleanupOfOldOwnerNicsForNetwork(vmOldProfile); if (applicableNetworks.isEmpty()) { - selectApplicableNetworkToCreateVm(caller, newAccount, zone, applicableNetworks); + selectApplicableNetworkToCreateVm(newAccount, zone, applicableNetworks); } addNicsToApplicableNetworksAndReturnDefaultNetwork(applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics, networks); @@ -7878,16 +7952,14 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir * 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, + protected void addNetworksToNetworkIdList(UserVmVO vm, Account newAccount, List networkIdList, Set applicableNetworks, Map requestedIPv4ForNics, Map requestedIPv6ForNics) { logger.trace("Adding networks to network list."); @@ -7895,8 +7967,6 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir keepOldSharedNetworkForVm(vm, newAccount, networkIdList, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); addAdditionalNetworksToVm(vm, newAccount, networkIdList, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); - - cleanupOfOldOwnerNicsForNetwork(vmOldProfile); } /** @@ -7937,34 +8007,21 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir * 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) + protected void selectApplicableNetworkToCreateVm(Account newAccount, DataCenterVO zone, Set applicableNetworks) throws InsufficientCapacityException, ResourceAllocationException { logger.trace("Selecting the applicable network to create the VM."); - 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)); - } - - 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)); - } - NetworkVO defaultNetwork; List virtualNetworks = _networkModel.listNetworksForAccount(newAccount.getId(), zone.getId(), Network.GuestType.Isolated); if (virtualNetworks.isEmpty()) { - defaultNetwork = createApplicableNetworkToCreateVm(caller, newAccount, zone, firstRequiredOffering); + throw new CloudRuntimeException(String.format("Could not find an applicable network to create virtual machine for account [%s].", newAccount)); } 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)); @@ -8096,13 +8153,15 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir * @throws InsufficientCapacityException * @throws ResourceAllocationException */ - protected NetworkVO createApplicableNetworkToCreateVm(Account caller, Account newAccount, DataCenterVO zone, NetworkOfferingVO requiredOffering) + protected NetworkVO createApplicableNetworkToCreateVm(Account newAccount, DataCenterVO zone) throws InsufficientCapacityException, ResourceAllocationException { logger.trace("Creating an applicable network to create the VM."); NetworkVO defaultNetwork; Long zoneId = zone.getId(); + Account caller = CallContext.current().getCallingAccount(); + NetworkOfferingVO requiredOffering = getOfferingWithRequiredAvailabilityForNetworkCreation(); String requiredOfferingTags = requiredOffering.getTags(); long physicalNetworkId = _networkModel.findPhysicalNetworkId(zoneId, requiredOfferingTags, requiredOffering.getTrafficType()); @@ -8112,14 +8171,13 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir throw new InvalidParameterValueException(String.format("Unable to find physical network with ID [%s] and tag [%s].", physicalNetworkId, requiredOfferingTags)); } - Long requiredOfferingId = requiredOffering.getId(); + long requiredOfferingId = requiredOffering.getId(); logger.debug("Creating network for account [{}] from the network offering [{}] as a part of VM deployment process.", newAccount, requiredOfferingId); - 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, null); + String networkName = String.format("%s-network", newAccount.getAccountName()); + Network newNetwork = _networkMgr.createGuestNetwork(requiredOfferingId, networkName, networkName, 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, null); if (requiredOffering.isPersistent()) { newNetwork = implementNetwork(caller, zone, newNetwork); diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index f07d2af21af..ac1ecaa456b 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -40,6 +40,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import com.cloud.network.NetworkService; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.SecurityChecker; import org.apache.cloudstack.api.BaseCmd.HTTPMethod; @@ -365,6 +366,9 @@ public class UserVmManagerImplTest { @Mock private VMInstanceVO vmInstanceMock; + @Mock + NetworkService networkServiceMock; + private static final long vmId = 1l; private static final long zoneId = 2L; private static final long accountId = 3L; @@ -1970,12 +1974,12 @@ public class UserVmManagerImplTest { 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.doNothing().when(userVmManagerImpl).updateBasicTypeNetworkForVm(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.verify(userVmManagerImpl).updateBasicTypeNetworkForVm(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); } @@ -1983,12 +1987,12 @@ public class UserVmManagerImplTest { 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.doNothing().when(userVmManagerImpl).updateAdvancedTypeNetworkForVm(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.verify(userVmManagerImpl).updateAdvancedTypeNetworkForVm(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); } @@ -2058,40 +2062,37 @@ public class UserVmManagerImplTest { } @Test - public void addNetworksToNetworkIdListTestCallsKeepOldSharedNetworkForVmAddAdditionalNetworksToVmAndCleanupOfOldOwnerNicsForNetwork() { + public void addNetworksToNetworkIdListTestCallsKeepOldSharedNetworkForVmAndAddAdditionalNetworksToVm() { LinkedList networkIdList = new LinkedList(); HashSet applicableNetworks = new HashSet(); HashMap requestedIPv4ForNics = new HashMap(); HashMap requestedIPv6ForNics = new HashMap(); - userVmManagerImpl.addNetworksToNetworkIdList(userVmVoMock, accountMock, virtualMachineProfileMock, networkIdList, applicableNetworks, requestedIPv4ForNics, + userVmManagerImpl.addNetworksToNetworkIdList(userVmVoMock, accountMock, 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() { + public void getOfferingWithRequiredAvailabilityForNetworkCreationTestRequiredOfferingsListHasNoOfferingsThrowsInvalidParameterValueException() { 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(); + 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); + userVmManagerImpl.getOfferingWithRequiredAvailabilityForNetworkCreation(); }); Assert.assertEquals(expectedMessage, assertThrows.getMessage()); } @Test - public void selectApplicableNetworkToCreateVmTestFirstOfferingIsNotEnabledThrowsInvalidParameterValueException() { + public void getOfferingWithRequiredAvailabilityForNetworkCreationTestFirstOfferingIsNotEnabledThrowsInvalidParameterValueException() { 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); @@ -2101,30 +2102,22 @@ public class UserVmManagerImplTest { Mockito.doReturn(1l).when(networkOfferingVoMock).getId(); InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { - userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks); + userVmManagerImpl.getOfferingWithRequiredAvailabilityForNetworkCreation(); }); Assert.assertEquals(expectedMessage, assertThrows.getMessage()); } - @Test - public void selectApplicableNetworkToCreateVmTestVirtualNetworkIsEmptyCallsCreateApplicableNetworkToCreateVm() throws InsufficientCapacityException, + @Test(expected = CloudRuntimeException.class) + public void selectApplicableNetworkToCreateVmTestVirtualNetworkIsEmptyThrowsException() throws InsufficientCapacityException, ResourceAllocationException { - HashSet applicableNetworks = new HashSet(); + 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); + userVmManagerImpl.selectApplicableNetworkToCreateVm(accountMock, _dcMock, applicableNetworks); } @Test @@ -2133,17 +2126,13 @@ public class UserVmManagerImplTest { 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); + userVmManagerImpl.selectApplicableNetworkToCreateVm(accountMock, _dcMock, applicableNetworks); }); Assert.assertEquals(expectedMessage, assertThrows.getMessage()); @@ -2153,17 +2142,13 @@ public class UserVmManagerImplTest { 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); + userVmManagerImpl.selectApplicableNetworkToCreateVm(accountMock, _dcMock, applicableNetworks); Mockito.verify(_networkDao).findById(Mockito.anyLong()); } @@ -2452,9 +2437,11 @@ public class UserVmManagerImplTest { @Test public void createApplicableNetworkToCreateVmTestPhysicalNetworkIsNullThrowsInvalidParameterValueException() { + Mockito.doReturn(networkOfferingVoMock).when(userVmManagerImpl).getOfferingWithRequiredAvailabilityForNetworkCreation(); + 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); + userVmManagerImpl.createApplicableNetworkToCreateVm(accountMock, _dcMock); }); Assert.assertEquals(expectedMessage, assertThrows.getMessage()); @@ -2466,11 +2453,17 @@ public class UserVmManagerImplTest { Mockito.doReturn(physicalNetworkVo).when(physicalNetworkDaoMock).findById(Mockito.anyLong()); Mockito.doReturn(true).when(networkOfferingVoMock).isPersistent(); + Mockito.doReturn(networkOfferingVoMock).when(userVmManagerImpl).getOfferingWithRequiredAvailabilityForNetworkCreation(); Mockito.doReturn(networkMock).when(userVmManagerImpl).implementNetwork(Mockito.any(), Mockito.any(), Mockito.any()); + 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.any()); - userVmManagerImpl.createApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, networkOfferingVoMock); + userVmManagerImpl.createApplicableNetworkToCreateVm(accountMock, _dcMock); - Mockito.verify(userVmManagerImpl).implementNetwork(callerAccount, _dcMock, null); + Mockito.verify(userVmManagerImpl).implementNetwork(callerAccount, _dcMock, networkMock); } @Test @@ -2480,16 +2473,16 @@ public class UserVmManagerImplTest { 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.any()); + 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.any()); + Mockito.doReturn(networkOfferingVoMock).when(userVmManagerImpl).getOfferingWithRequiredAvailabilityForNetworkCreation(); Mockito.doReturn(1l).when(networkMock).getId(); Mockito.doReturn(networkMock).when(_networkDao).findById(Mockito.anyLong()); - userVmManagerImpl.createApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, networkOfferingVoMock); + userVmManagerImpl.createApplicableNetworkToCreateVm(accountMock, _dcMock); Mockito.verify(userVmManagerImpl, Mockito.never()).implementNetwork(Mockito.any(), Mockito.any(), Mockito.any()); } @@ -2674,7 +2667,7 @@ public class UserVmManagerImplTest { networkIdList.add(1l); InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { - userVmManagerImpl.updateBasicTypeNetworkForVm(assignVmCmdMock, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, networkIdList, + userVmManagerImpl.updateBasicTypeNetworkForVm(userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, networkIdList, securityGroupIdList); }); @@ -2689,7 +2682,7 @@ public class UserVmManagerImplTest { Mockito.doReturn(networkMock).when(networkModel).getExclusiveGuestNetwork(Mockito.anyLong()); - userVmManagerImpl.updateBasicTypeNetworkForVm(assignVmCmdMock, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, null, + userVmManagerImpl.updateBasicTypeNetworkForVm(userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, null, securityGroupIdList); Mockito.verify(userVmManagerImpl).cleanupOfOldOwnerNicsForNetwork(virtualMachineProfileMock); @@ -2707,7 +2700,7 @@ public class UserVmManagerImplTest { Mockito.doReturn(networkMock).when(networkModel).getExclusiveGuestNetwork(Mockito.anyLong()); - userVmManagerImpl.updateBasicTypeNetworkForVm(assignVmCmdMock, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, networkIdList, + userVmManagerImpl.updateBasicTypeNetworkForVm(userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, networkIdList, securityGroupIdList); Mockito.verify(userVmManagerImpl).cleanupOfOldOwnerNicsForNetwork(virtualMachineProfileMock); @@ -2725,7 +2718,7 @@ public class UserVmManagerImplTest { 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, + userVmManagerImpl.updateAdvancedTypeNetworkForVm(callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, networkIdList, securityGroupIdList); }); @@ -2746,7 +2739,7 @@ public class UserVmManagerImplTest { Mockito.doReturn(true).when(networkModel).checkSecurityGroupSupportForNetwork(accountMock, _dcMock, networkIdList, securityGroupIdList); - userVmManagerImpl.updateAdvancedTypeNetworkForVm(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, + userVmManagerImpl.updateAdvancedTypeNetworkForVm(callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, networkIdList, securityGroupIdList); Mockito.verify(securityGroupManagerMock).removeInstanceFromGroups(Mockito.any()); @@ -2765,7 +2758,7 @@ public class UserVmManagerImplTest { Mockito.doReturn(false).when(networkModel).checkSecurityGroupSupportForNetwork(accountMock, _dcMock, networkIdList, securityGroupIdList); InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { - userVmManagerImpl.updateAdvancedTypeNetworkForVm(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, + userVmManagerImpl.updateAdvancedTypeNetworkForVm(callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, networkIdList, securityGroupIdList); }); @@ -2780,16 +2773,17 @@ public class UserVmManagerImplTest { 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.doNothing().when(userVmManagerImpl).selectApplicableNetworkToCreateVm(Mockito.any(), Mockito.any(), Mockito.any()); 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, + userVmManagerImpl.updateAdvancedTypeNetworkForVm(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).addNetworksToNetworkIdList(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyMap(), Mockito.anyMap()); + Mockito.verify(userVmManagerImpl).cleanupOfOldOwnerNicsForNetwork(Mockito.any()); + Mockito.verify(userVmManagerImpl).selectApplicableNetworkToCreateVm(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()); } @@ -2808,11 +2802,12 @@ public class UserVmManagerImplTest { 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, + userVmManagerImpl.updateAdvancedTypeNetworkForVm(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).addNetworksToNetworkIdList(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyMap(), Mockito.anyMap()); + Mockito.verify(userVmManagerImpl).cleanupOfOldOwnerNicsForNetwork(Mockito.any()); + Mockito.verify(userVmManagerImpl, Mockito.never()).selectApplicableNetworkToCreateVm(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()); }