From 4cb5198d8479745f8bc5ae2cdea248ee38683123 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Mon, 12 May 2025 09:47:25 +0530 Subject: [PATCH] fix test and variable name --- .../java/com/cloud/vm/UserVmManagerImpl.java | 20 +++---- .../com/cloud/vm/UserVmManagerImplTest.java | 59 ++++++++++++------- 2 files changed, 47 insertions(+), 32 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index b8e44c7816a..3aa7b4646a0 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -3984,7 +3984,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir return defaultNetwork; } - private NetworkVO createDefaultNetworkForAccount(DataCenter zone, Account owner, List requiredOfferings) + protected NetworkVO createDefaultNetworkForAccount(DataCenter zone, Account owner, List requiredOfferings) throws InsufficientCapacityException, ResourceAllocationException { NetworkVO defaultNetwork = null; long physicalNetworkId = _networkModel.findPhysicalNetworkId(zone.getId(), requiredOfferings.get(0).getTags(), requiredOfferings.get(0).getTrafficType()); @@ -7591,12 +7591,12 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir updateVmOwner(newAccount, vm, domainId, newAccountId); updateVolumesOwner(volumes, oldAccount, newAccount, newAccountId); - MutableBoolean isNetworkCreated = new MutableBoolean(false); + MutableBoolean isNetworkAutoCreated = new MutableBoolean(false); try { - updateVmNetwork(cmd, caller, vm, newAccount, template, isNetworkCreated); + updateVmNetwork(cmd, caller, vm, newAccount, template, isNetworkAutoCreated); } catch (InsufficientCapacityException | ResourceAllocationException e) { List networkVOS = _networkDao.listByAccountIdNetworkName(newAccountId, newAccount.getAccountName() + "-network"); - if (networkVOS.size() == 1 && isNetworkCreated.get()) { + if (networkVOS.size() == 1 && isNetworkAutoCreated.get()) { _networkDao.remove(networkVOS.get(0).getId()); } throw new CloudRuntimeException(String.format("Unable to update networks when assigning VM [%s] due to [%s].", vm, e.getMessage()), e); @@ -7662,7 +7662,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir * @throws InsufficientCapacityException * @throws ResourceAllocationException */ - protected void updateVmNetwork(AssignVMCmd cmd, Account caller, UserVmVO vm, Account newAccount, VirtualMachineTemplate template, MutableBoolean isNetworkCreated) + protected void updateVmNetwork(AssignVMCmd cmd, Account caller, UserVmVO vm, Account newAccount, VirtualMachineTemplate template, MutableBoolean isNetworkAutoCreated) throws InsufficientCapacityException, ResourceAllocationException { logger.trace("Updating network for VM [{}].", vm); @@ -7680,7 +7680,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir return; } - updateAdvancedTypeNetworkForVm(cmd, caller, vm, newAccount, template, vmOldProfile, zone, networkIdList, securityGroupIdList, isNetworkCreated); + updateAdvancedTypeNetworkForVm(cmd, caller, vm, newAccount, template, vmOldProfile, zone, networkIdList, securityGroupIdList, isNetworkAutoCreated); } /** @@ -7791,7 +7791,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir * @throws InvalidParameterValueException */ protected void updateAdvancedTypeNetworkForVm(AssignVMCmd cmd, Account caller, UserVmVO vm, Account newAccount, VirtualMachineTemplate template, - VirtualMachineProfileImpl vmOldProfile, DataCenterVO zone, List networkIdList, List securityGroupIdList, MutableBoolean isNetworkCreated) + VirtualMachineProfileImpl vmOldProfile, DataCenterVO zone, List networkIdList, List securityGroupIdList, MutableBoolean isNetworkAutoCreated) throws InsufficientCapacityException, ResourceAllocationException, InvalidParameterValueException { LinkedHashSet applicableNetworks = new LinkedHashSet<>(); @@ -7824,7 +7824,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir addNetworksToNetworkIdList(vm, newAccount, vmOldProfile, networkIdList, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); if (applicableNetworks.isEmpty()) { - selectApplicableNetworkToCreateVm(caller, newAccount, zone, applicableNetworks, isNetworkCreated); + selectApplicableNetworkToCreateVm(caller, newAccount, zone, applicableNetworks, isNetworkAutoCreated); } addNicsToApplicableNetworksAndReturnDefaultNetwork(applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics, networks); @@ -7947,7 +7947,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir * @throws ResourceAllocationException */ protected void selectApplicableNetworkToCreateVm(Account caller, Account newAccount, DataCenterVO zone, - Set applicableNetworks, MutableBoolean isNetworkCreated) + Set applicableNetworks, MutableBoolean isNetworkAutoCreated) throws InsufficientCapacityException, ResourceAllocationException { logger.trace("Selecting the applicable network to create the VM."); @@ -7969,7 +7969,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir if (virtualNetworks.isEmpty()) { try (TransactionLegacy txn = TransactionLegacy.open("CreateNetworkTxn")) { defaultNetwork = createApplicableNetworkToCreateVm(caller, newAccount, zone, firstRequiredOffering); - isNetworkCreated.set(true); + isNetworkAutoCreated.set(true); txn.commit(); } } else if (virtualNetworks.size() > 1) { diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index f07d2af21af..5093d37a2bc 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -36,10 +36,12 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.when; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import com.cloud.network.Networks; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.SecurityChecker; import org.apache.cloudstack.api.BaseCmd.HTTPMethod; @@ -609,7 +611,7 @@ public class UserVmManagerImplTest { 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).updateVmNetwork(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); Mockito.doNothing().when(userVmManagerImpl).resourceCountIncrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any()); } @@ -1973,7 +1975,8 @@ public class UserVmManagerImplTest { 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); + UserVmManagerImpl.MutableBoolean isNetworkCreated = Mockito.mock(UserVmManagerImpl.MutableBoolean.class); + userVmManagerImpl.updateVmNetwork(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, isNetworkCreated); Mockito.verify(userVmManagerImpl).updateBasicTypeNetworkForVm(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); @@ -1984,12 +1987,14 @@ public class UserVmManagerImplTest { 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()); + Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); - userVmManagerImpl.updateVmNetwork(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock); + UserVmManagerImpl.MutableBoolean isNetworkCreated = Mockito.mock(UserVmManagerImpl.MutableBoolean.class); + + userVmManagerImpl.updateVmNetwork(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, isNetworkCreated); Mockito.verify(userVmManagerImpl).updateAdvancedTypeNetworkForVm(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), - Mockito.any(), Mockito.any(), Mockito.any()); + Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); } @Test @@ -2078,11 +2083,12 @@ public class UserVmManagerImplTest { NetworkOffering.Availability.Required); HashSet applicableNetworks = new HashSet(); LinkedList requiredOfferings = new LinkedList(); + UserVmManagerImpl.MutableBoolean isNetworkCreated = new UserVmManagerImpl.MutableBoolean(false); Mockito.doReturn(requiredOfferings).when(networkOfferingDaoMock).listByAvailability(NetworkOffering.Availability.Required, false); InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { - userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks); + userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks, isNetworkCreated); }); Assert.assertEquals(expectedMessage, assertThrows.getMessage()); @@ -2100,8 +2106,9 @@ public class UserVmManagerImplTest { Mockito.doReturn(1l).when(networkOfferingVoMock).getId(); + UserVmManagerImpl.MutableBoolean isNetworkCreated = new UserVmManagerImpl.MutableBoolean(false); InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { - userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks); + userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks, isNetworkCreated); }); Assert.assertEquals(expectedMessage, assertThrows.getMessage()); @@ -2122,7 +2129,8 @@ public class UserVmManagerImplTest { Mockito.doReturn(networkMock).when(userVmManagerImpl).createApplicableNetworkToCreateVm(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); - userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks); + UserVmManagerImpl.MutableBoolean isNetworkCreated = Mockito.mock(UserVmManagerImpl.MutableBoolean.class); + userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks, isNetworkCreated); Mockito.verify(userVmManagerImpl).createApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, networkOfferingVoMock); } @@ -2142,8 +2150,9 @@ public class UserVmManagerImplTest { virtualNetworks.add(networkMock); virtualNetworks.add(networkMock); + UserVmManagerImpl.MutableBoolean isNetworkCreated = Mockito.mock(UserVmManagerImpl.MutableBoolean.class); InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { - userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks); + userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks, isNetworkCreated); }); Assert.assertEquals(expectedMessage, assertThrows.getMessage()); @@ -2163,7 +2172,8 @@ public class UserVmManagerImplTest { Mockito.doReturn(1).when(networkVoListMock).size(); Mockito.doReturn(networkMock).when(networkVoListMock).get(0); - userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks); + UserVmManagerImpl.MutableBoolean isNetworkCreated = Mockito.mock(UserVmManagerImpl.MutableBoolean.class); + userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks, isNetworkCreated); Mockito.verify(_networkDao).findById(Mockito.anyLong()); } @@ -2724,9 +2734,10 @@ public class UserVmManagerImplTest { Mockito.doReturn(true).when(networkModel).checkSecurityGroupSupportForNetwork(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); + UserVmManagerImpl.MutableBoolean isNetworkCreated = Mockito.mock(UserVmManagerImpl.MutableBoolean.class); InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { userVmManagerImpl.updateAdvancedTypeNetworkForVm(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, - _dcMock, networkIdList, securityGroupIdList); + _dcMock, networkIdList, securityGroupIdList, isNetworkCreated); }); Mockito.verify(securityGroupManagerMock).removeInstanceFromGroups(Mockito.any()); @@ -2746,8 +2757,9 @@ public class UserVmManagerImplTest { Mockito.doReturn(true).when(networkModel).checkSecurityGroupSupportForNetwork(accountMock, _dcMock, networkIdList, securityGroupIdList); + UserVmManagerImpl.MutableBoolean isNetworkCreated = Mockito.mock(UserVmManagerImpl.MutableBoolean.class); userVmManagerImpl.updateAdvancedTypeNetworkForVm(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, - networkIdList, securityGroupIdList); + networkIdList, securityGroupIdList, isNetworkCreated); Mockito.verify(securityGroupManagerMock).removeInstanceFromGroups(Mockito.any()); Mockito.verify(userVmManagerImpl).allocateNetworksForVm(Mockito.any(), Mockito.any()); @@ -2764,9 +2776,10 @@ public class UserVmManagerImplTest { Mockito.doReturn(false).when(networkModel).checkSecurityGroupSupportForNetwork(accountMock, _dcMock, networkIdList, securityGroupIdList); + UserVmManagerImpl.MutableBoolean isNetworkCreated = Mockito.mock(UserVmManagerImpl.MutableBoolean.class); InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { userVmManagerImpl.updateAdvancedTypeNetworkForVm(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, - _dcMock, networkIdList, securityGroupIdList); + _dcMock, networkIdList, securityGroupIdList, isNetworkCreated); }); Assert.assertEquals(expectedMessage, assertThrows.getMessage()); @@ -2780,16 +2793,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.any(), Mockito.any()); Mockito.doReturn(false).when(networkModel).checkSecurityGroupSupportForNetwork(accountMock, _dcMock, networkIdList, securityGroupIdList); Mockito.doReturn(true).when(securityGroupIdList).isEmpty(); + UserVmManagerImpl.MutableBoolean isNetworkCreated = Mockito.mock(UserVmManagerImpl.MutableBoolean.class); userVmManagerImpl.updateAdvancedTypeNetworkForVm(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, - networkIdList, securityGroupIdList); + networkIdList, securityGroupIdList, isNetworkCreated); 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).selectApplicableNetworkToCreateVm(Mockito.any(), 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()); } @@ -2808,11 +2822,12 @@ public class UserVmManagerImplTest { Mockito.doReturn(networkMock).when(_networkDao).findById(Mockito.anyLong()); Mockito.doReturn(true).when(userVmManagerImpl).canAccountUseNetwork(accountMock, networkMock); + UserVmManagerImpl.MutableBoolean isNetworkCreated = Mockito.mock(UserVmManagerImpl.MutableBoolean.class); userVmManagerImpl.updateAdvancedTypeNetworkForVm(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, - networkIdList, securityGroupIdList); + networkIdList, securityGroupIdList, isNetworkCreated); 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, Mockito.never()).selectApplicableNetworkToCreateVm(Mockito.any(), 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()); } @@ -3045,7 +3060,7 @@ public class UserVmManagerImplTest { configureDoNothingForMethodsThatWeDoNotWantToTest(); Mockito.doThrow(InsufficientAddressCapacityException.class).when(userVmManagerImpl).updateVmNetwork(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), - Mockito.any()); + Mockito.any(), Mockito.any()); Assert.assertThrows(CloudRuntimeException.class, () -> userVmManagerImpl.executeStepsToChangeOwnershipOfVm(assignVmCmdMock, callerAccount, accountMock, accountMock, userVmVoMock, serviceOfferingVoMock, volumes, virtualMachineTemplateMock, 1l)); @@ -3068,7 +3083,7 @@ public class UserVmManagerImplTest { configureDoNothingForMethodsThatWeDoNotWantToTest(); Mockito.doThrow(ResourceAllocationException.class).when(userVmManagerImpl).updateVmNetwork(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), - Mockito.any()); + Mockito.any(), Mockito.any()); Assert.assertThrows(CloudRuntimeException.class, () -> userVmManagerImpl.executeStepsToChangeOwnershipOfVm(assignVmCmdMock, callerAccount, accountMock, accountMock, userVmVoMock, serviceOfferingVoMock, volumes, virtualMachineTemplateMock, 1l)); @@ -3098,7 +3113,7 @@ public class UserVmManagerImplTest { 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).updateVmNetwork(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); Mockito.verify(userVmManagerImpl).resourceCountIncrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any()); } } @@ -3121,7 +3136,7 @@ public class UserVmManagerImplTest { 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).updateVmNetwork(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); Mockito.verify(userVmManagerImpl, Mockito.never()).resourceCountIncrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any()); } }