Skip to content

Commit e56e97a

Browse files
winterhazelwinterhazel
committed
Make domainid optional in assignVirtualMachine 
1 parent c267ad3 commit e56e97a

2 files changed

Lines changed: 23 additions & 54 deletions

File tree

‎server/src/main/java/com/cloud/vm/UserVmManagerImpl.java‎

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7455,11 +7455,6 @@ public UserVm moveVmToUser(final AssignVMCmd cmd) throws ResourceAllocationExcep
74557455

74567456
checkCallerAccessToAccounts(caller, oldAccount, newAccount);
74577457

7458-
logger.trace("Verifying if the provided domain ID [{}] is valid.", domainId);
7459-
if (projectId != null && domainId == null) {
7460-
throw new InvalidParameterValueException("Please provide a valid domain ID; cannot assign VM to a project if domain ID is NULL.");
7461-
}
7462-
74637458
validateIfVmHasNoRules(vm, vmId);
74647459

74657460
final List<VolumeVO> volumes = _volsDao.findByInstance(vmId);
@@ -7470,10 +7465,6 @@ public UserVm moveVmToUser(final AssignVMCmd cmd) throws ResourceAllocationExcep
74707465

74717466
validateIfNewOwnerHasAccessToTemplate(vm, newAccount, template);
74727467

7473-
DomainVO domain = _domainDao.findById(domainId);
7474-
logger.trace("Verifying if the new account [{}] has access to the specified domain [{}].", newAccount, domain);
7475-
_accountMgr.checkAccess(newAccount, domain);
7476-
74777468
List<Reserver> reservations = new ArrayList<>();
74787469
try {
74797470
verifyResourceLimitsForAccountAndStorage(newAccount, vm, offering, volumes, template, reservations);
@@ -7483,7 +7474,7 @@ public UserVm moveVmToUser(final AssignVMCmd cmd) throws ResourceAllocationExcep
74837474
Transaction.execute(new TransactionCallbackNoReturn() {
74847475
@Override
74857476
public void doInTransactionWithoutResult(TransactionStatus status) {
7486-
executeStepsToChangeOwnershipOfVm(cmd, caller, oldAccount, newAccount, vm, offering, volumes, template, domainId);
7477+
executeStepsToChangeOwnershipOfVm(cmd, caller, oldAccount, newAccount, vm, offering, volumes, template);
74877478
}
74887479
});
74897480
} catch (Exception e) {
@@ -7681,10 +7672,9 @@ protected NetworkOfferingVO getOfferingWithRequiredAvailabilityForNetworkCreatio
76817672
* @param offering The service offering which will be used to decrement and increment resource counts.
76827673
* @param volumes The volumes of the VM which will be assigned to another user.
76837674
* @param template The template of the VM which will be assigned to another user.
7684-
* @param domainId The ID of the domain where the VM which will be assigned to another user is.
76857675
*/
76867676
protected void executeStepsToChangeOwnershipOfVm(AssignVMCmd cmd, Account caller, Account oldAccount, Account newAccount, UserVmVO vm, ServiceOfferingVO offering,
7687-
List<VolumeVO> volumes, VirtualMachineTemplate template, Long domainId) {
7677+
List<VolumeVO> volumes, VirtualMachineTemplate template) {
76887678

76897679
logger.trace("Generating destroy event for VM [{}].", vm);
76907680
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_DESTROY, vm.getAccountId(), vm.getDataCenterId(), vm.getId(), vm.getHostName(), vm.getServiceOfferingId(),
@@ -7697,7 +7687,7 @@ protected void executeStepsToChangeOwnershipOfVm(AssignVMCmd cmd, Account caller
76977687
removeInstanceFromInstanceGroup(vm.getId());
76987688

76997689
Long newAccountId = newAccount.getAccountId();
7700-
updateVmOwner(newAccount, vm, domainId, newAccountId);
7690+
updateVmOwner(newAccount, vm);
77017691

77027692
updateVolumesOwner(volumes, oldAccount, newAccount, newAccountId);
77037693

@@ -7717,11 +7707,11 @@ protected void executeStepsToChangeOwnershipOfVm(AssignVMCmd cmd, Account caller
77177707
vm.getTemplateId(), vm.getHypervisorType().toString(), VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm());
77187708
}
77197709

7720-
protected void updateVmOwner(Account newAccount, UserVmVO vm, Long domainId, Long newAccountId) {
7710+
protected void updateVmOwner(Account newAccount, UserVmVO vm) {
77217711
logger.debug("Updating VM [{}] owner to [{}].", vm, newAccount);
77227712

7723-
vm.setAccountId(newAccountId);
7724-
vm.setDomainId(domainId);
7713+
vm.setAccountId(newAccount.getId());
7714+
vm.setDomainId(newAccount.getDomainId());
77257715

77267716
_vmDao.persist(vm);
77277717
}

‎server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java‎

Lines changed: 17 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -618,7 +618,7 @@ private void configureDoNothingForMethodsThatWeDoNotWantToTest() throws Resource
618618
Mockito.doNothing().when(userVmManagerImpl).removeInstanceFromInstanceGroup(Mockito.anyLong());
619619
Mockito.doNothing().when(userVmManagerImpl).validateIfNewOwnerHasAccessToTemplate(Mockito.any(), Mockito.any(), Mockito.any());
620620

621-
Mockito.doNothing().when(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any());
621+
Mockito.doNothing().when(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any());
622622
Mockito.doNothing().when(userVmManagerImpl).updateVolumesOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any());
623623
Mockito.doNothing().when(userVmManagerImpl).updateVmNetwork(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any());
624624

@@ -1958,7 +1958,7 @@ public void validateIfNewOwnerHasAccessToTemplateTestCallCheckAccessWhenTemplate
19581958

19591959
@Test
19601960
public void updateVmOwnerTestCallsSetAccountIdSetDomainIdAndPersist() {
1961-
userVmManagerImpl.updateVmOwner(accountMock, userVmVoMock, 1l, 1l);
1961+
userVmManagerImpl.updateVmOwner(accountMock, userVmVoMock);
19621962

19631963
Mockito.verify(userVmVoMock).setAccountId(Mockito.anyLong());
19641964
Mockito.verify(userVmVoMock).setDomainId(Mockito.anyLong());
@@ -2913,23 +2913,22 @@ public void moveVmToUserTestValidateAccountsAndCallerAccessToThemThrowsInvalidPa
29132913
}
29142914

29152915
@Test
2916-
public void moveVmToUserTestProjectIdProvidedAndDomainIdIsNullThrowsInvalidParameterValueException() throws ResourceUnavailableException, InsufficientCapacityException,
2916+
public void moveVmToUserTestMovesVmWhenProjectIdIsProvidedAndDomainIdIsNull() throws ResourceUnavailableException, InsufficientCapacityException,
29172917
ResourceAllocationException {
2918-
2919-
String expectedMessage = "Please provide a valid domain ID; cannot assign VM to a project if domain ID is NULL.";
2920-
29212918
Mockito.doReturn(true).when(accountManager).isRootAdmin(Mockito.anyLong());
29222919
Mockito.doReturn(userVmVoMock).when(userVmDao).findById(Mockito.anyLong());
29232920
Mockito.doReturn(1l).when(assignVmCmdMock).getProjectId();
29242921
Mockito.doReturn(null).when(assignVmCmdMock).getDomainId();
2922+
Mockito.doReturn(null).when(userVmManagerImpl).ensureDestinationNetwork(Mockito.any(), Mockito.any(), Mockito.any());
2923+
Mockito.doNothing().when(userVmManagerImpl).executeStepsToChangeOwnershipOfVm(Mockito.any(), Mockito.any(), Mockito.any(),
2924+
Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any());
29252925

29262926
configureDoNothingForMethodsThatWeDoNotWantToTest();
29272927

2928-
InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> {
2929-
userVmManagerImpl.moveVmToUser(assignVmCmdMock);
2930-
});
2928+
userVmManagerImpl.moveVmToUser(assignVmCmdMock);
29312929

2932-
Assert.assertEquals(expectedMessage, assertThrows.getMessage());
2930+
Mockito.verify(userVmManagerImpl).executeStepsToChangeOwnershipOfVm(Mockito.any(), Mockito.any(), Mockito.any(),
2931+
Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any());
29332932
}
29342933

29352934
@Test
@@ -3003,26 +3002,6 @@ public void moveVmToUserTestVerifyValidateIfNewOwnerHasAccessToTemplateThrowsInv
30033002
Assert.assertThrows(InvalidParameterValueException.class, () -> userVmManagerImpl.moveVmToUser(assignVmCmdMock));
30043003
}
30053004

3006-
@Test
3007-
public void moveVmToUserTestAccountManagerCheckAccessThrowsPermissionDeniedException() throws ResourceUnavailableException, InsufficientCapacityException,
3008-
ResourceAllocationException {
3009-
3010-
LinkedList<VolumeVO> volumes = new LinkedList<VolumeVO>();
3011-
3012-
Mockito.doReturn(true).when(accountManager).isRootAdmin(Mockito.anyLong());
3013-
Mockito.doReturn(userVmVoMock).when(userVmDao).findById(Mockito.anyLong());
3014-
Mockito.doReturn(null).when(assignVmCmdMock).getProjectId();
3015-
Mockito.doReturn(volumes).when(volumeDaoMock).findByInstance(Mockito.anyLong());
3016-
Mockito.doReturn(accountMock).when(accountManager).finalizeOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any());
3017-
Mockito.doReturn(domainVoMock).when(domainDaoMock).findById(Mockito.anyLong());
3018-
3019-
configureDoNothingForMethodsThatWeDoNotWantToTest();
3020-
3021-
Mockito.doThrow(PermissionDeniedException.class).when(accountManager).checkAccess(Mockito.any(Account.class), Mockito.any());
3022-
3023-
Assert.assertThrows(PermissionDeniedException.class, () -> userVmManagerImpl.moveVmToUser(assignVmCmdMock));
3024-
}
3025-
30263005
@Test
30273006
public void executeStepsToChangeOwnershipOfVmTestUpdateVmNetworkThrowsInsufficientCapacityException() throws ResourceUnavailableException, InsufficientCapacityException,
30283007
ResourceAllocationException {
@@ -3038,10 +3017,10 @@ public void executeStepsToChangeOwnershipOfVmTestUpdateVmNetworkThrowsInsufficie
30383017
Mockito.any());
30393018

30403019
Assert.assertThrows(CloudRuntimeException.class, () -> userVmManagerImpl.executeStepsToChangeOwnershipOfVm(assignVmCmdMock, callerAccount, accountMock, accountMock,
3041-
userVmVoMock, serviceOfferingVoMock, volumes, virtualMachineTemplateMock, 1l));
3020+
userVmVoMock, serviceOfferingVoMock, volumes, virtualMachineTemplateMock));
30423021

30433022
Mockito.verify(userVmManagerImpl).resourceCountDecrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any());
3044-
Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong());
3023+
Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any());
30453024
Mockito.verify(userVmManagerImpl).updateVolumesOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyLong());
30463025
}
30473026
}
@@ -3061,10 +3040,10 @@ public void executeStepsToChangeOwnershipOfVmTestUpdateVmNetworkThrowsResourceAl
30613040
Mockito.any());
30623041

30633042
Assert.assertThrows(CloudRuntimeException.class, () -> userVmManagerImpl.executeStepsToChangeOwnershipOfVm(assignVmCmdMock, callerAccount, accountMock, accountMock,
3064-
userVmVoMock, serviceOfferingVoMock, volumes, virtualMachineTemplateMock, 1l));
3043+
userVmVoMock, serviceOfferingVoMock, volumes, virtualMachineTemplateMock));
30653044

30663045
Mockito.verify(userVmManagerImpl).resourceCountDecrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any());
3067-
Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong());
3046+
Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any());
30683047
Mockito.verify(userVmManagerImpl).updateVolumesOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyLong());
30693048
}
30703049
}
@@ -3083,10 +3062,10 @@ public void executeStepsToChangeOwnershipOfVmTestResourceCountRunningVmsOnlyEnab
30833062
configureDoNothingForMethodsThatWeDoNotWantToTest();
30843063

30853064
userVmManagerImpl.executeStepsToChangeOwnershipOfVm(assignVmCmdMock, callerAccount, accountMock, accountMock, userVmVoMock, serviceOfferingVoMock, volumes,
3086-
virtualMachineTemplateMock, 1l);
3065+
virtualMachineTemplateMock);
30873066

30883067
Mockito.verify(userVmManagerImpl).resourceCountDecrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any());
3089-
Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong());
3068+
Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any());
30903069
Mockito.verify(userVmManagerImpl).updateVolumesOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyLong());
30913070
Mockito.verify(userVmManagerImpl).updateVmNetwork(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any());
30923071
Mockito.verify(userVmManagerImpl).resourceCountIncrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any());
@@ -3106,10 +3085,10 @@ public void executeStepsToChangeOwnershipOfVmTestResourceCountRunningVmsOnlyEnab
31063085
configureDoNothingForMethodsThatWeDoNotWantToTest();
31073086

31083087
userVmManagerImpl.executeStepsToChangeOwnershipOfVm(assignVmCmdMock, callerAccount, accountMock, accountMock, userVmVoMock, serviceOfferingVoMock, volumes,
3109-
virtualMachineTemplateMock, 1l);
3088+
virtualMachineTemplateMock);
31103089

31113090
Mockito.verify(userVmManagerImpl).resourceCountDecrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any());
3112-
Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong());
3091+
Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any());
31133092
Mockito.verify(userVmManagerImpl).updateVolumesOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyLong());
31143093
Mockito.verify(userVmManagerImpl).updateVmNetwork(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any());
31153094
Mockito.verify(userVmManagerImpl, Mockito.never()).resourceCountIncrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any());

0 commit comments

Comments
 (0)