From 23482e4a28efd9bbc90cfce2eae5004073fcfc6d Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Thu, 8 May 2025 14:26:06 +0530 Subject: [PATCH 1/7] Fix issue with Assign VM operation --- .../src/main/java/com/cloud/vm/UserVmManagerImpl.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index bc21a3a92821..40ba5ba1ed3f 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.xml.parsers.DocumentBuilder; import javax.xml.parsers.ParserConfigurationException; +import com.cloud.utils.db.TransactionLegacy; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.ControlledEntity.ACLType; import org.apache.cloudstack.acl.SecurityChecker.AccessType; @@ -7594,6 +7595,11 @@ protected void executeStepsToChangeOwnershipOfVm(AssignVMCmd cmd, Account caller try { updateVmNetwork(cmd, caller, vm, newAccount, template); } catch (InsufficientCapacityException | ResourceAllocationException e) { + List networkVOS = _networkDao.listByAccountIdNetworkName(newAccountId, newAccount.getAccountName() + "-network"); + if (networkVOS.size() == 1) { + _networkDao.remove(networkVOS.get(0).getId()); + } + _accountMgr.getActiveAccountByName(newAccount.getAccountName() + "-network", newAccount.getDomainId()); throw new CloudRuntimeException(String.format("Unable to update networks when assigning VM [%s] due to [%s].", vm, e.getMessage()), e); } @@ -7961,7 +7967,10 @@ protected void selectApplicableNetworkToCreateVm(Account caller, Account newAcco NetworkVO defaultNetwork; List virtualNetworks = _networkModel.listNetworksForAccount(newAccount.getId(), zone.getId(), Network.GuestType.Isolated); if (virtualNetworks.isEmpty()) { - defaultNetwork = createApplicableNetworkToCreateVm(caller, newAccount, zone, firstRequiredOffering); + try (TransactionLegacy txn = TransactionLegacy.open("CreateNetworkTxn")) { + defaultNetwork = createApplicableNetworkToCreateVm(caller, newAccount, zone, firstRequiredOffering); + txn.commit(); + } } 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)); From 646ccd817e229251947a5236569d9f7eb1ab332f Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Fri, 9 May 2025 17:05:28 +0530 Subject: [PATCH 2/7] fix assign vm issue --- .../java/com/cloud/vm/UserVmManagerImpl.java | 27 ++++++++++++------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 40ba5ba1ed3f..b8e44c7816ae 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -7591,15 +7591,14 @@ protected void executeStepsToChangeOwnershipOfVm(AssignVMCmd cmd, Account caller updateVmOwner(newAccount, vm, domainId, newAccountId); updateVolumesOwner(volumes, oldAccount, newAccount, newAccountId); - + MutableBoolean isNetworkCreated = new MutableBoolean(false); try { - updateVmNetwork(cmd, caller, vm, newAccount, template); + updateVmNetwork(cmd, caller, vm, newAccount, template, isNetworkCreated); } catch (InsufficientCapacityException | ResourceAllocationException e) { List networkVOS = _networkDao.listByAccountIdNetworkName(newAccountId, newAccount.getAccountName() + "-network"); - if (networkVOS.size() == 1) { + if (networkVOS.size() == 1 && isNetworkCreated.get()) { _networkDao.remove(networkVOS.get(0).getId()); } - _accountMgr.getActiveAccountByName(newAccount.getAccountName() + "-network", newAccount.getDomainId()); throw new CloudRuntimeException(String.format("Unable to update networks when assigning VM [%s] due to [%s].", vm, e.getMessage()), e); } @@ -7663,7 +7662,7 @@ protected void updateVolumesOwner(final List volumes, Account oldAccou * @throws InsufficientCapacityException * @throws ResourceAllocationException */ - protected void updateVmNetwork(AssignVMCmd cmd, Account caller, UserVmVO vm, Account newAccount, VirtualMachineTemplate template) + protected void updateVmNetwork(AssignVMCmd cmd, Account caller, UserVmVO vm, Account newAccount, VirtualMachineTemplate template, MutableBoolean isNetworkCreated) throws InsufficientCapacityException, ResourceAllocationException { logger.trace("Updating network for VM [{}].", vm); @@ -7681,7 +7680,7 @@ protected void updateVmNetwork(AssignVMCmd cmd, Account caller, UserVmVO vm, Acc return; } - updateAdvancedTypeNetworkForVm(cmd, caller, vm, newAccount, template, vmOldProfile, zone, networkIdList, securityGroupIdList); + updateAdvancedTypeNetworkForVm(cmd, caller, vm, newAccount, template, vmOldProfile, zone, networkIdList, securityGroupIdList, isNetworkCreated); } /** @@ -7792,7 +7791,7 @@ protected void updateBasicTypeNetworkForVm(AssignVMCmd cmd, UserVmVO vm, Account * @throws InvalidParameterValueException */ protected void updateAdvancedTypeNetworkForVm(AssignVMCmd cmd, Account caller, UserVmVO vm, Account newAccount, VirtualMachineTemplate template, - VirtualMachineProfileImpl vmOldProfile, DataCenterVO zone, List networkIdList, List securityGroupIdList) + VirtualMachineProfileImpl vmOldProfile, DataCenterVO zone, List networkIdList, List securityGroupIdList, MutableBoolean isNetworkCreated) throws InsufficientCapacityException, ResourceAllocationException, InvalidParameterValueException { LinkedHashSet applicableNetworks = new LinkedHashSet<>(); @@ -7825,7 +7824,7 @@ protected void updateAdvancedTypeNetworkForVm(AssignVMCmd cmd, Account caller, U addNetworksToNetworkIdList(vm, newAccount, vmOldProfile, networkIdList, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); if (applicableNetworks.isEmpty()) { - selectApplicableNetworkToCreateVm(caller, newAccount, zone, applicableNetworks); + selectApplicableNetworkToCreateVm(caller, newAccount, zone, applicableNetworks, isNetworkCreated); } addNicsToApplicableNetworksAndReturnDefaultNetwork(applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics, networks); @@ -7947,7 +7946,8 @@ protected NetworkVO addNicsToApplicableNetworksAndReturnDefaultNetwork(LinkedHas * @throws InsufficientCapacityException * @throws ResourceAllocationException */ - protected void selectApplicableNetworkToCreateVm(Account caller, Account newAccount, DataCenterVO zone, Set applicableNetworks) + protected void selectApplicableNetworkToCreateVm(Account caller, Account newAccount, DataCenterVO zone, + Set applicableNetworks, MutableBoolean isNetworkCreated) throws InsufficientCapacityException, ResourceAllocationException { logger.trace("Selecting the applicable network to create the VM."); @@ -7969,6 +7969,7 @@ protected void selectApplicableNetworkToCreateVm(Account caller, Account newAcco if (virtualNetworks.isEmpty()) { try (TransactionLegacy txn = TransactionLegacy.open("CreateNetworkTxn")) { defaultNetwork = createApplicableNetworkToCreateVm(caller, newAccount, zone, firstRequiredOffering); + isNetworkCreated.set(true); txn.commit(); } } else if (virtualNetworks.size() > 1) { @@ -9166,4 +9167,12 @@ private void setVncPasswordForKvmIfAvailable(Map customParameter vm.setVncPassword(customParameters.get(VmDetailConstants.KVM_VNC_PASSWORD)); } } + + public static class MutableBoolean { + private boolean value; + public MutableBoolean(boolean val) { this.value = val; } + public void set(boolean val) { this.value = val; } + public boolean get() { return this.value; } + } + } From 4cb5198d8479745f8bc5ae2cdea248ee38683123 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Mon, 12 May 2025 09:47:25 +0530 Subject: [PATCH 3/7] 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 b8e44c7816ae..3aa7b4646a05 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 @@ private NetworkVO getDefaultNetwork(DataCenter zone, Account owner, boolean sele 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 @@ protected void executeStepsToChangeOwnershipOfVm(AssignVMCmd cmd, Account caller 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 @@ protected void updateVolumesOwner(final List volumes, Account oldAccou * @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 @@ protected void updateVmNetwork(AssignVMCmd cmd, Account caller, UserVmVO vm, Acc 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 @@ protected void updateBasicTypeNetworkForVm(AssignVMCmd cmd, UserVmVO vm, Account * @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 @@ protected void updateAdvancedTypeNetworkForVm(AssignVMCmd cmd, Account caller, U 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 @@ protected NetworkVO addNicsToApplicableNetworksAndReturnDefaultNetwork(LinkedHas * @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 @@ protected void selectApplicableNetworkToCreateVm(Account caller, Account newAcco 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 f07d2af21af2..5093d37a2bc6 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.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 @@ private void configureDoNothingForMethodsThatWeDoNotWantToTest() throws Resource 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 void updateVmNetworkTestCallsUpdateBasicTypeNetworkForVmIfBasicTypeZone() 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 void updateVmNetworkTestCallsUpdateAdvancedTypeNetworkForVmIfNotBasicType 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.MutableBoolean isNetworkCreated = Mockito.mock(UserVmManagerImpl.MutableBoolean.class); - userVmManagerImpl.updateVmNetwork(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock); + 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 void selectApplicableNetworkToCreateVmTestRequiredOfferingsListHasNoOffer 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 void selectApplicableNetworkToCreateVmTestFirstOfferingIsNotEnabledThrows 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 void selectApplicableNetworkToCreateVmTestVirtualNetworkIsEmptyCallsCreat 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 void selectApplicableNetworkToCreateVmTestVirtualNetworkHasMultipleNetwor 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 void selectApplicableNetworkToCreateVmTestVirtualNetworkHasOneNetworkCall 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 void updateAdvancedTypeNetworkForVmTestSecurityGroupIsEnabledApplicableNe 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 void updateAdvancedTypeNetworkForVmTestSecurityGroupIsEnabledApplicableNe 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 void updateAdvancedTypeNetworkForVmTestSecurityGroupIsNotEnabledSecurityG 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 void updateAdvancedTypeNetworkForVmTestSecurityGroupIsNotEnabledApplicabl 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 void updateAdvancedTypeNetworkForVmTestSecurityGroupIsNotEnabledApplicabl 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 void executeStepsToChangeOwnershipOfVmTestUpdateVmNetworkThrowsInsufficie 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 void executeStepsToChangeOwnershipOfVmTestUpdateVmNetworkThrowsResourceAl 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 void executeStepsToChangeOwnershipOfVmTestResourceCountRunningVmsOnlyEnab 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 void executeStepsToChangeOwnershipOfVmTestResourceCountRunningVmsOnlyEnab 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()); } } From 277fee17ea3ceed3d48963517ed2b35c8ac610b0 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Mon, 12 May 2025 10:53:14 +0530 Subject: [PATCH 4/7] use AtomicBoolean --- .../java/com/cloud/vm/UserVmManagerImpl.java | 19 ++++--------- .../com/cloud/vm/UserVmManagerImplTest.java | 28 +++++++++---------- 2 files changed, 20 insertions(+), 27 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 3aa7b4646a05..28ca164b0a5b 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -44,6 +44,7 @@ import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -3984,7 +3985,7 @@ private NetworkVO getDefaultNetwork(DataCenter zone, Account owner, boolean sele return defaultNetwork; } - protected NetworkVO createDefaultNetworkForAccount(DataCenter zone, Account owner, List requiredOfferings) + private 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,7 +7592,7 @@ protected void executeStepsToChangeOwnershipOfVm(AssignVMCmd cmd, Account caller updateVmOwner(newAccount, vm, domainId, newAccountId); updateVolumesOwner(volumes, oldAccount, newAccount, newAccountId); - MutableBoolean isNetworkAutoCreated = new MutableBoolean(false); + AtomicBoolean isNetworkAutoCreated = new AtomicBoolean(false); try { updateVmNetwork(cmd, caller, vm, newAccount, template, isNetworkAutoCreated); } catch (InsufficientCapacityException | ResourceAllocationException e) { @@ -7662,7 +7663,7 @@ protected void updateVolumesOwner(final List volumes, Account oldAccou * @throws InsufficientCapacityException * @throws ResourceAllocationException */ - protected void updateVmNetwork(AssignVMCmd cmd, Account caller, UserVmVO vm, Account newAccount, VirtualMachineTemplate template, MutableBoolean isNetworkAutoCreated) + protected void updateVmNetwork(AssignVMCmd cmd, Account caller, UserVmVO vm, Account newAccount, VirtualMachineTemplate template, AtomicBoolean isNetworkAutoCreated) throws InsufficientCapacityException, ResourceAllocationException { logger.trace("Updating network for VM [{}].", vm); @@ -7791,7 +7792,7 @@ protected void updateBasicTypeNetworkForVm(AssignVMCmd cmd, UserVmVO vm, Account * @throws InvalidParameterValueException */ protected void updateAdvancedTypeNetworkForVm(AssignVMCmd cmd, Account caller, UserVmVO vm, Account newAccount, VirtualMachineTemplate template, - VirtualMachineProfileImpl vmOldProfile, DataCenterVO zone, List networkIdList, List securityGroupIdList, MutableBoolean isNetworkAutoCreated) + VirtualMachineProfileImpl vmOldProfile, DataCenterVO zone, List networkIdList, List securityGroupIdList, AtomicBoolean isNetworkAutoCreated) throws InsufficientCapacityException, ResourceAllocationException, InvalidParameterValueException { LinkedHashSet applicableNetworks = new LinkedHashSet<>(); @@ -7947,7 +7948,7 @@ protected NetworkVO addNicsToApplicableNetworksAndReturnDefaultNetwork(LinkedHas * @throws ResourceAllocationException */ protected void selectApplicableNetworkToCreateVm(Account caller, Account newAccount, DataCenterVO zone, - Set applicableNetworks, MutableBoolean isNetworkAutoCreated) + Set applicableNetworks, AtomicBoolean isNetworkAutoCreated) throws InsufficientCapacityException, ResourceAllocationException { logger.trace("Selecting the applicable network to create the VM."); @@ -9167,12 +9168,4 @@ private void setVncPasswordForKvmIfAvailable(Map customParameter vm.setVncPassword(customParameters.get(VmDetailConstants.KVM_VNC_PASSWORD)); } } - - public static class MutableBoolean { - private boolean value; - public MutableBoolean(boolean val) { this.value = val; } - public void set(boolean val) { this.value = val; } - public boolean get() { return this.value; } - } - } diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index 5093d37a2bc6..591bad37096e 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -36,12 +36,10 @@ 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; @@ -142,6 +140,8 @@ import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.LinkedList; +import java.util.concurrent.atomic.AtomicBoolean; + import com.cloud.domain.DomainVO; import com.cloud.domain.dao.DomainDao; import com.cloud.event.UsageEventUtils; @@ -1975,7 +1975,7 @@ public void updateVmNetworkTestCallsUpdateBasicTypeNetworkForVmIfBasicTypeZone() Mockito.doNothing().when(userVmManagerImpl).updateBasicTypeNetworkForVm(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); - UserVmManagerImpl.MutableBoolean isNetworkCreated = Mockito.mock(UserVmManagerImpl.MutableBoolean.class); + AtomicBoolean isNetworkCreated = Mockito.mock(AtomicBoolean.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(), @@ -1989,7 +1989,7 @@ public void updateVmNetworkTestCallsUpdateAdvancedTypeNetworkForVmIfNotBasicType 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()); - UserVmManagerImpl.MutableBoolean isNetworkCreated = Mockito.mock(UserVmManagerImpl.MutableBoolean.class); + AtomicBoolean isNetworkCreated = Mockito.mock(AtomicBoolean.class); userVmManagerImpl.updateVmNetwork(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, isNetworkCreated); @@ -2083,7 +2083,7 @@ public void selectApplicableNetworkToCreateVmTestRequiredOfferingsListHasNoOffer NetworkOffering.Availability.Required); HashSet applicableNetworks = new HashSet(); LinkedList requiredOfferings = new LinkedList(); - UserVmManagerImpl.MutableBoolean isNetworkCreated = new UserVmManagerImpl.MutableBoolean(false); + AtomicBoolean isNetworkCreated = Mockito.mock(AtomicBoolean.class); Mockito.doReturn(requiredOfferings).when(networkOfferingDaoMock).listByAvailability(NetworkOffering.Availability.Required, false); @@ -2106,7 +2106,7 @@ public void selectApplicableNetworkToCreateVmTestFirstOfferingIsNotEnabledThrows Mockito.doReturn(1l).when(networkOfferingVoMock).getId(); - UserVmManagerImpl.MutableBoolean isNetworkCreated = new UserVmManagerImpl.MutableBoolean(false); + AtomicBoolean isNetworkCreated = Mockito.mock(AtomicBoolean.class); InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks, isNetworkCreated); }); @@ -2129,7 +2129,7 @@ public void selectApplicableNetworkToCreateVmTestVirtualNetworkIsEmptyCallsCreat Mockito.doReturn(networkMock).when(userVmManagerImpl).createApplicableNetworkToCreateVm(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); - UserVmManagerImpl.MutableBoolean isNetworkCreated = Mockito.mock(UserVmManagerImpl.MutableBoolean.class); + AtomicBoolean isNetworkCreated = Mockito.mock(AtomicBoolean.class); userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks, isNetworkCreated); Mockito.verify(userVmManagerImpl).createApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, networkOfferingVoMock); @@ -2150,7 +2150,7 @@ public void selectApplicableNetworkToCreateVmTestVirtualNetworkHasMultipleNetwor virtualNetworks.add(networkMock); virtualNetworks.add(networkMock); - UserVmManagerImpl.MutableBoolean isNetworkCreated = Mockito.mock(UserVmManagerImpl.MutableBoolean.class); + AtomicBoolean isNetworkCreated = Mockito.mock(AtomicBoolean.class); InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks, isNetworkCreated); }); @@ -2172,7 +2172,7 @@ public void selectApplicableNetworkToCreateVmTestVirtualNetworkHasOneNetworkCall Mockito.doReturn(1).when(networkVoListMock).size(); Mockito.doReturn(networkMock).when(networkVoListMock).get(0); - UserVmManagerImpl.MutableBoolean isNetworkCreated = Mockito.mock(UserVmManagerImpl.MutableBoolean.class); + AtomicBoolean isNetworkCreated = Mockito.mock(AtomicBoolean.class); userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks, isNetworkCreated); Mockito.verify(_networkDao).findById(Mockito.anyLong()); @@ -2734,7 +2734,7 @@ public void updateAdvancedTypeNetworkForVmTestSecurityGroupIsEnabledApplicableNe Mockito.doReturn(true).when(networkModel).checkSecurityGroupSupportForNetwork(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); - UserVmManagerImpl.MutableBoolean isNetworkCreated = Mockito.mock(UserVmManagerImpl.MutableBoolean.class); + AtomicBoolean isNetworkCreated = Mockito.mock(AtomicBoolean.class); InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { userVmManagerImpl.updateAdvancedTypeNetworkForVm(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, networkIdList, securityGroupIdList, isNetworkCreated); @@ -2757,7 +2757,7 @@ public void updateAdvancedTypeNetworkForVmTestSecurityGroupIsEnabledApplicableNe Mockito.doReturn(true).when(networkModel).checkSecurityGroupSupportForNetwork(accountMock, _dcMock, networkIdList, securityGroupIdList); - UserVmManagerImpl.MutableBoolean isNetworkCreated = Mockito.mock(UserVmManagerImpl.MutableBoolean.class); + AtomicBoolean isNetworkCreated = Mockito.mock(AtomicBoolean.class); userVmManagerImpl.updateAdvancedTypeNetworkForVm(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, networkIdList, securityGroupIdList, isNetworkCreated); @@ -2776,7 +2776,7 @@ public void updateAdvancedTypeNetworkForVmTestSecurityGroupIsNotEnabledSecurityG Mockito.doReturn(false).when(networkModel).checkSecurityGroupSupportForNetwork(accountMock, _dcMock, networkIdList, securityGroupIdList); - UserVmManagerImpl.MutableBoolean isNetworkCreated = Mockito.mock(UserVmManagerImpl.MutableBoolean.class); + AtomicBoolean isNetworkCreated = Mockito.mock(AtomicBoolean.class); InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { userVmManagerImpl.updateAdvancedTypeNetworkForVm(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, networkIdList, securityGroupIdList, isNetworkCreated); @@ -2798,7 +2798,7 @@ public void updateAdvancedTypeNetworkForVmTestSecurityGroupIsNotEnabledApplicabl Mockito.doReturn(false).when(networkModel).checkSecurityGroupSupportForNetwork(accountMock, _dcMock, networkIdList, securityGroupIdList); Mockito.doReturn(true).when(securityGroupIdList).isEmpty(); - UserVmManagerImpl.MutableBoolean isNetworkCreated = Mockito.mock(UserVmManagerImpl.MutableBoolean.class); + AtomicBoolean isNetworkCreated = Mockito.mock(AtomicBoolean.class); userVmManagerImpl.updateAdvancedTypeNetworkForVm(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, networkIdList, securityGroupIdList, isNetworkCreated); @@ -2822,7 +2822,7 @@ public void updateAdvancedTypeNetworkForVmTestSecurityGroupIsNotEnabledApplicabl Mockito.doReturn(networkMock).when(_networkDao).findById(Mockito.anyLong()); Mockito.doReturn(true).when(userVmManagerImpl).canAccountUseNetwork(accountMock, networkMock); - UserVmManagerImpl.MutableBoolean isNetworkCreated = Mockito.mock(UserVmManagerImpl.MutableBoolean.class); + AtomicBoolean isNetworkCreated = Mockito.mock(AtomicBoolean.class); userVmManagerImpl.updateAdvancedTypeNetworkForVm(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, networkIdList, securityGroupIdList, isNetworkCreated); From caa8c8183b500f438f08160537473cd4c4b83385 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Mon, 12 May 2025 17:32:43 +0530 Subject: [PATCH 5/7] move change ownership logic --- server/src/main/java/com/cloud/vm/UserVmManagerImpl.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index c5cf24a505fc..ef62718a9e18 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -7592,9 +7592,6 @@ protected void executeStepsToChangeOwnershipOfVm(AssignVMCmd cmd, Account caller removeInstanceFromInstanceGroup(vm.getId()); Long newAccountId = newAccount.getAccountId(); - updateVmOwner(newAccount, vm, domainId, newAccountId); - - updateVolumesOwner(volumes, oldAccount, newAccount, newAccountId); AtomicBoolean isNetworkAutoCreated = new AtomicBoolean(false); try { updateVmNetwork(cmd, caller, vm, newAccount, template, isNetworkAutoCreated); @@ -7606,6 +7603,10 @@ protected void executeStepsToChangeOwnershipOfVm(AssignVMCmd cmd, Account caller throw new CloudRuntimeException(String.format("Unable to update networks when assigning VM [%s] due to [%s].", vm, e.getMessage()), e); } + updateVmOwner(newAccount, vm, domainId, newAccountId); + + updateVolumesOwner(volumes, oldAccount, newAccount, newAccountId); + logger.trace(String.format("Incrementing new account [%s] resource count.", newAccount)); if (!isResourceCountRunningVmsOnlyEnabled()) { resourceCountIncrement(newAccountId, vm.isDisplayVm(), offering, template); From fd7159cd5a93318e19e954c513b59eeaf9d4cf50 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Tue, 13 May 2025 09:51:02 +0530 Subject: [PATCH 6/7] rearrange logic for change of ownership to avoid any resources being left behind in the DB should there be an exception --- .../java/com/cloud/vm/UserVmManagerImpl.java | 46 +++++++++---------- .../com/cloud/vm/UserVmManagerImplTest.java | 11 ----- 2 files changed, 23 insertions(+), 34 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index ef62718a9e18..bdbe34a2af33 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -7581,20 +7581,33 @@ 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("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("Decrementing old account [{}] resource count.", oldAccount); - resourceCountDecrement(oldAccount.getAccountId(), vm.isDisplayVm(), offering, template); - - logger.trace("Removing VM [{}] from its instance group.", vm); - removeInstanceFromInstanceGroup(vm.getId()); - Long newAccountId = newAccount.getAccountId(); AtomicBoolean isNetworkAutoCreated = new AtomicBoolean(false); try { updateVmNetwork(cmd, caller, vm, newAccount, template, isNetworkAutoCreated); + + 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("Decrementing old account [{}] resource count.", oldAccount); + resourceCountDecrement(oldAccount.getAccountId(), vm.isDisplayVm(), offering, template); + + logger.trace("Removing VM [{}] from its instance group.", vm); + removeInstanceFromInstanceGroup(vm.getId()); + + updateVmOwner(newAccount, vm, domainId, newAccountId); + + updateVolumesOwner(volumes, oldAccount, newAccount, newAccountId); + + 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()); } catch (InsufficientCapacityException | ResourceAllocationException e) { List networkVOS = _networkDao.listByAccountIdNetworkName(newAccountId, newAccount.getAccountName() + "-network"); if (networkVOS.size() == 1 && isNetworkAutoCreated.get()) { @@ -7602,19 +7615,6 @@ protected void executeStepsToChangeOwnershipOfVm(AssignVMCmd cmd, Account caller } throw new CloudRuntimeException(String.format("Unable to update networks when assigning VM [%s] due to [%s].", vm, e.getMessage()), e); } - - updateVmOwner(newAccount, vm, domainId, newAccountId); - - updateVolumesOwner(volumes, oldAccount, newAccount, newAccountId); - - 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) { diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index 591bad37096e..ccf4ba1ff4e5 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -3055,8 +3055,6 @@ public void executeStepsToChangeOwnershipOfVmTestUpdateVmNetworkThrowsInsufficie 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(), @@ -3064,10 +3062,6 @@ public void executeStepsToChangeOwnershipOfVmTestUpdateVmNetworkThrowsInsufficie 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()); - Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong()); - Mockito.verify(userVmManagerImpl).updateVolumesOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyLong()); } } @@ -3078,8 +3072,6 @@ public void executeStepsToChangeOwnershipOfVmTestUpdateVmNetworkThrowsResourceAl 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(), @@ -3088,9 +3080,6 @@ public void executeStepsToChangeOwnershipOfVmTestUpdateVmNetworkThrowsResourceAl 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()); - Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong()); - Mockito.verify(userVmManagerImpl).updateVolumesOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyLong()); } } From 49b76c9d26a26b400208cd2533cbaad44052b510 Mon Sep 17 00:00:00 2001 From: Pearl Dsilva Date: Tue, 13 May 2025 14:00:50 +0530 Subject: [PATCH 7/7] revert changes made --- .../java/com/cloud/vm/UserVmManagerImpl.java | 72 ++++++++----------- .../com/cloud/vm/UserVmManagerImplTest.java | 70 +++++++++--------- 2 files changed, 61 insertions(+), 81 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index bdbe34a2af33..81a7296e862d 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -44,7 +44,6 @@ import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -55,7 +54,6 @@ import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.ParserConfigurationException; -import com.cloud.utils.db.TransactionLegacy; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.acl.ControlledEntity.ACLType; import org.apache.cloudstack.acl.SecurityChecker.AccessType; @@ -7447,12 +7445,7 @@ public UserVm moveVmToUser(final AssignVMCmd cmd) throws ResourceAllocationExcep 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); - } - }); + executeStepsToChangeOwnershipOfVm(cmd, caller, oldAccount, newAccount, vm, offering, volumes, template, domainId); logger.info("VM [{}] now belongs to account [{}].", vm.getInstanceName(), newAccountName); return vm; @@ -7581,40 +7574,35 @@ 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) { - Long newAccountId = newAccount.getAccountId(); - AtomicBoolean isNetworkAutoCreated = new AtomicBoolean(false); - try { - updateVmNetwork(cmd, caller, vm, newAccount, template, isNetworkAutoCreated); - - 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("Decrementing old account [{}] resource count.", oldAccount); - resourceCountDecrement(oldAccount.getAccountId(), vm.isDisplayVm(), offering, template); + 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("Removing VM [{}] from its instance group.", vm); - removeInstanceFromInstanceGroup(vm.getId()); + logger.trace("Decrementing old account [{}] resource count.", oldAccount); + resourceCountDecrement(oldAccount.getAccountId(), vm.isDisplayVm(), offering, template); - updateVmOwner(newAccount, vm, domainId, newAccountId); + logger.trace("Removing VM [{}] from its instance group.", vm); + removeInstanceFromInstanceGroup(vm.getId()); - updateVolumesOwner(volumes, oldAccount, newAccount, newAccountId); + Long newAccountId = newAccount.getAccountId(); + updateVmOwner(newAccount, vm, domainId, newAccountId); - logger.trace(String.format("Incrementing new account [%s] resource count.", newAccount)); - if (!isResourceCountRunningVmsOnlyEnabled()) { - resourceCountIncrement(newAccountId, vm.isDisplayVm(), offering, template); - } + updateVolumesOwner(volumes, oldAccount, newAccount, newAccountId); - 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()); + try { + updateVmNetwork(cmd, caller, vm, newAccount, template); } catch (InsufficientCapacityException | ResourceAllocationException e) { - List networkVOS = _networkDao.listByAccountIdNetworkName(newAccountId, newAccount.getAccountName() + "-network"); - 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); } + + 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) { @@ -7667,7 +7655,7 @@ protected void updateVolumesOwner(final List volumes, Account oldAccou * @throws InsufficientCapacityException * @throws ResourceAllocationException */ - protected void updateVmNetwork(AssignVMCmd cmd, Account caller, UserVmVO vm, Account newAccount, VirtualMachineTemplate template, AtomicBoolean isNetworkAutoCreated) + protected void updateVmNetwork(AssignVMCmd cmd, Account caller, UserVmVO vm, Account newAccount, VirtualMachineTemplate template) throws InsufficientCapacityException, ResourceAllocationException { logger.trace("Updating network for VM [{}].", vm); @@ -7685,7 +7673,7 @@ protected void updateVmNetwork(AssignVMCmd cmd, Account caller, UserVmVO vm, Acc return; } - updateAdvancedTypeNetworkForVm(cmd, caller, vm, newAccount, template, vmOldProfile, zone, networkIdList, securityGroupIdList, isNetworkAutoCreated); + updateAdvancedTypeNetworkForVm(cmd, caller, vm, newAccount, template, vmOldProfile, zone, networkIdList, securityGroupIdList); } /** @@ -7796,7 +7784,7 @@ protected void updateBasicTypeNetworkForVm(AssignVMCmd cmd, UserVmVO vm, Account * @throws InvalidParameterValueException */ protected void updateAdvancedTypeNetworkForVm(AssignVMCmd cmd, Account caller, UserVmVO vm, Account newAccount, VirtualMachineTemplate template, - VirtualMachineProfileImpl vmOldProfile, DataCenterVO zone, List networkIdList, List securityGroupIdList, AtomicBoolean isNetworkAutoCreated) + VirtualMachineProfileImpl vmOldProfile, DataCenterVO zone, List networkIdList, List securityGroupIdList) throws InsufficientCapacityException, ResourceAllocationException, InvalidParameterValueException { LinkedHashSet applicableNetworks = new LinkedHashSet<>(); @@ -7829,7 +7817,7 @@ protected void updateAdvancedTypeNetworkForVm(AssignVMCmd cmd, Account caller, U addNetworksToNetworkIdList(vm, newAccount, vmOldProfile, networkIdList, applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics); if (applicableNetworks.isEmpty()) { - selectApplicableNetworkToCreateVm(caller, newAccount, zone, applicableNetworks, isNetworkAutoCreated); + selectApplicableNetworkToCreateVm(caller, newAccount, zone, applicableNetworks); } addNicsToApplicableNetworksAndReturnDefaultNetwork(applicableNetworks, requestedIPv4ForNics, requestedIPv6ForNics, networks); @@ -7952,7 +7940,7 @@ protected NetworkVO addNicsToApplicableNetworksAndReturnDefaultNetwork(LinkedHas * @throws ResourceAllocationException */ protected void selectApplicableNetworkToCreateVm(Account caller, Account newAccount, DataCenterVO zone, - Set applicableNetworks, AtomicBoolean isNetworkAutoCreated) + Set applicableNetworks) throws InsufficientCapacityException, ResourceAllocationException { logger.trace("Selecting the applicable network to create the VM."); @@ -7972,11 +7960,7 @@ protected void selectApplicableNetworkToCreateVm(Account caller, Account newAcco NetworkVO defaultNetwork; List virtualNetworks = _networkModel.listNetworksForAccount(newAccount.getId(), zone.getId(), Network.GuestType.Isolated); if (virtualNetworks.isEmpty()) { - try (TransactionLegacy txn = TransactionLegacy.open("CreateNetworkTxn")) { - defaultNetwork = createApplicableNetworkToCreateVm(caller, newAccount, zone, firstRequiredOffering); - isNetworkAutoCreated.set(true); - txn.commit(); - } + 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)); diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index ccf4ba1ff4e5..f07d2af21af2 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -140,8 +140,6 @@ import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.LinkedList; -import java.util.concurrent.atomic.AtomicBoolean; - import com.cloud.domain.DomainVO; import com.cloud.domain.dao.DomainDao; import com.cloud.event.UsageEventUtils; @@ -611,7 +609,7 @@ private void configureDoNothingForMethodsThatWeDoNotWantToTest() throws Resource 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.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()); } @@ -1975,8 +1973,7 @@ public void updateVmNetworkTestCallsUpdateBasicTypeNetworkForVmIfBasicTypeZone() Mockito.doNothing().when(userVmManagerImpl).updateBasicTypeNetworkForVm(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); - AtomicBoolean isNetworkCreated = Mockito.mock(AtomicBoolean.class); - userVmManagerImpl.updateVmNetwork(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, isNetworkCreated); + 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()); @@ -1987,14 +1984,12 @@ public void updateVmNetworkTestCallsUpdateAdvancedTypeNetworkForVmIfNotBasicType 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()); - - AtomicBoolean isNetworkCreated = Mockito.mock(AtomicBoolean.class); + Mockito.any(), Mockito.any(), Mockito.any()); - userVmManagerImpl.updateVmNetwork(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, isNetworkCreated); + 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(), Mockito.any()); + Mockito.any(), Mockito.any(), Mockito.any()); } @Test @@ -2083,12 +2078,11 @@ public void selectApplicableNetworkToCreateVmTestRequiredOfferingsListHasNoOffer NetworkOffering.Availability.Required); HashSet applicableNetworks = new HashSet(); LinkedList requiredOfferings = new LinkedList(); - AtomicBoolean isNetworkCreated = Mockito.mock(AtomicBoolean.class); Mockito.doReturn(requiredOfferings).when(networkOfferingDaoMock).listByAvailability(NetworkOffering.Availability.Required, false); InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { - userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks, isNetworkCreated); + userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks); }); Assert.assertEquals(expectedMessage, assertThrows.getMessage()); @@ -2106,9 +2100,8 @@ public void selectApplicableNetworkToCreateVmTestFirstOfferingIsNotEnabledThrows Mockito.doReturn(1l).when(networkOfferingVoMock).getId(); - AtomicBoolean isNetworkCreated = Mockito.mock(AtomicBoolean.class); InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { - userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks, isNetworkCreated); + userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks); }); Assert.assertEquals(expectedMessage, assertThrows.getMessage()); @@ -2129,8 +2122,7 @@ public void selectApplicableNetworkToCreateVmTestVirtualNetworkIsEmptyCallsCreat Mockito.doReturn(networkMock).when(userVmManagerImpl).createApplicableNetworkToCreateVm(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); - AtomicBoolean isNetworkCreated = Mockito.mock(AtomicBoolean.class); - userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks, isNetworkCreated); + userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks); Mockito.verify(userVmManagerImpl).createApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, networkOfferingVoMock); } @@ -2150,9 +2142,8 @@ public void selectApplicableNetworkToCreateVmTestVirtualNetworkHasMultipleNetwor virtualNetworks.add(networkMock); virtualNetworks.add(networkMock); - AtomicBoolean isNetworkCreated = Mockito.mock(AtomicBoolean.class); InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { - userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks, isNetworkCreated); + userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks); }); Assert.assertEquals(expectedMessage, assertThrows.getMessage()); @@ -2172,8 +2163,7 @@ public void selectApplicableNetworkToCreateVmTestVirtualNetworkHasOneNetworkCall Mockito.doReturn(1).when(networkVoListMock).size(); Mockito.doReturn(networkMock).when(networkVoListMock).get(0); - AtomicBoolean isNetworkCreated = Mockito.mock(AtomicBoolean.class); - userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks, isNetworkCreated); + userVmManagerImpl.selectApplicableNetworkToCreateVm(callerAccount, accountMock, _dcMock, applicableNetworks); Mockito.verify(_networkDao).findById(Mockito.anyLong()); } @@ -2734,10 +2724,9 @@ public void updateAdvancedTypeNetworkForVmTestSecurityGroupIsEnabledApplicableNe Mockito.doReturn(true).when(networkModel).checkSecurityGroupSupportForNetwork(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); - AtomicBoolean isNetworkCreated = Mockito.mock(AtomicBoolean.class); InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { userVmManagerImpl.updateAdvancedTypeNetworkForVm(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, - _dcMock, networkIdList, securityGroupIdList, isNetworkCreated); + _dcMock, networkIdList, securityGroupIdList); }); Mockito.verify(securityGroupManagerMock).removeInstanceFromGroups(Mockito.any()); @@ -2757,9 +2746,8 @@ public void updateAdvancedTypeNetworkForVmTestSecurityGroupIsEnabledApplicableNe Mockito.doReturn(true).when(networkModel).checkSecurityGroupSupportForNetwork(accountMock, _dcMock, networkIdList, securityGroupIdList); - AtomicBoolean isNetworkCreated = Mockito.mock(AtomicBoolean.class); userVmManagerImpl.updateAdvancedTypeNetworkForVm(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, - networkIdList, securityGroupIdList, isNetworkCreated); + networkIdList, securityGroupIdList); Mockito.verify(securityGroupManagerMock).removeInstanceFromGroups(Mockito.any()); Mockito.verify(userVmManagerImpl).allocateNetworksForVm(Mockito.any(), Mockito.any()); @@ -2776,10 +2764,9 @@ public void updateAdvancedTypeNetworkForVmTestSecurityGroupIsNotEnabledSecurityG Mockito.doReturn(false).when(networkModel).checkSecurityGroupSupportForNetwork(accountMock, _dcMock, networkIdList, securityGroupIdList); - AtomicBoolean isNetworkCreated = Mockito.mock(AtomicBoolean.class); InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> { userVmManagerImpl.updateAdvancedTypeNetworkForVm(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, - _dcMock, networkIdList, securityGroupIdList, isNetworkCreated); + _dcMock, networkIdList, securityGroupIdList); }); Assert.assertEquals(expectedMessage, assertThrows.getMessage()); @@ -2793,17 +2780,16 @@ public void updateAdvancedTypeNetworkForVmTestSecurityGroupIsNotEnabledApplicabl 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.any()); + Mockito.doNothing().when(userVmManagerImpl).selectApplicableNetworkToCreateVm(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); Mockito.doReturn(false).when(networkModel).checkSecurityGroupSupportForNetwork(accountMock, _dcMock, networkIdList, securityGroupIdList); Mockito.doReturn(true).when(securityGroupIdList).isEmpty(); - AtomicBoolean isNetworkCreated = Mockito.mock(AtomicBoolean.class); userVmManagerImpl.updateAdvancedTypeNetworkForVm(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, - networkIdList, securityGroupIdList, isNetworkCreated); + 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.any()); + 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()); } @@ -2822,12 +2808,11 @@ public void updateAdvancedTypeNetworkForVmTestSecurityGroupIsNotEnabledApplicabl Mockito.doReturn(networkMock).when(_networkDao).findById(Mockito.anyLong()); Mockito.doReturn(true).when(userVmManagerImpl).canAccountUseNetwork(accountMock, networkMock); - AtomicBoolean isNetworkCreated = Mockito.mock(AtomicBoolean.class); userVmManagerImpl.updateAdvancedTypeNetworkForVm(assignVmCmdMock, callerAccount, userVmVoMock, accountMock, virtualMachineTemplateMock, virtualMachineProfileMock, _dcMock, - networkIdList, securityGroupIdList, isNetworkCreated); + 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.any()); + 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()); } @@ -3055,13 +3040,19 @@ public void executeStepsToChangeOwnershipOfVmTestUpdateVmNetworkThrowsInsufficie 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(), Mockito.any()); + Mockito.any()); 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()); + Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong()); + Mockito.verify(userVmManagerImpl).updateVolumesOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyLong()); } } @@ -3072,14 +3063,19 @@ public void executeStepsToChangeOwnershipOfVmTestUpdateVmNetworkThrowsResourceAl 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(), Mockito.any()); + Mockito.any()); 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()); + Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong()); + Mockito.verify(userVmManagerImpl).updateVolumesOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyLong()); } } @@ -3102,7 +3098,7 @@ public void executeStepsToChangeOwnershipOfVmTestResourceCountRunningVmsOnlyEnab 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.any()); + 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()); } } @@ -3125,7 +3121,7 @@ public void executeStepsToChangeOwnershipOfVmTestResourceCountRunningVmsOnlyEnab 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.any()); + 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()); } }