Skip to content

Commit 567abaf

Browse files
winterhazelwinterhazel
committed
Make domainid optional in assignVirtualMachine 
1 parent e1521f1 commit 567abaf

2 files changed

Lines changed: 23 additions & 55 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
@@ -7966,11 +7966,6 @@ public UserVm moveVmToUser(final AssignVMCmd cmd) throws ResourceAllocationExcep
79667966

79677967
checkCallerAccessToAccounts(caller, oldAccount, newAccount);
79687968

7969-
logger.trace("Verifying if the provided domain ID [{}] is valid.", domainId);
7970-
if (projectId != null && domainId == null) {
7971-
throw new InvalidParameterValueException("Please provide a valid domain ID; cannot assign VM to a project if domain ID is NULL.");
7972-
}
7973-
79747969
validateIfVmHasNoRules(vm, vmId);
79757970

79767971
final List<VolumeVO> volumes = _volsDao.findByInstance(vmId);
@@ -7981,10 +7976,6 @@ public UserVm moveVmToUser(final AssignVMCmd cmd) throws ResourceAllocationExcep
79817976

79827977
validateIfNewOwnerHasAccessToTemplate(vm, newAccount, template);
79837978

7984-
DomainVO domain = _domainDao.findById(domainId);
7985-
logger.trace("Verifying if the new account [{}] has access to the specified domain [{}].", newAccount, domain);
7986-
_accountMgr.checkAccess(newAccount, domain);
7987-
79887979
List<Reserver> reservations = new ArrayList<>();
79897980
try {
79907981
verifyResourceLimitsForAccountAndStorage(newAccount, vm, offering, volumes, template, reservations);
@@ -7994,7 +7985,7 @@ public UserVm moveVmToUser(final AssignVMCmd cmd) throws ResourceAllocationExcep
79947985
Transaction.execute(new TransactionCallbackNoReturn() {
79957986
@Override
79967987
public void doInTransactionWithoutResult(TransactionStatus status) {
7997-
executeStepsToChangeOwnershipOfVm(cmd, caller, oldAccount, newAccount, vm, offering, volumes, template, domainId);
7988+
executeStepsToChangeOwnershipOfVm(cmd, caller, oldAccount, newAccount, vm, offering, volumes, template);
79987989
}
79997990
});
80007991
} catch (Exception e) {
@@ -8192,10 +8183,9 @@ protected NetworkOfferingVO getOfferingWithRequiredAvailabilityForNetworkCreatio
81928183
* @param offering The service offering which will be used to decrement and increment resource counts.
81938184
* @param volumes The volumes of the VM which will be assigned to another user.
81948185
* @param template The template of the VM which will be assigned to another user.
8195-
* @param domainId The ID of the domain where the VM which will be assigned to another user is.
81968186
*/
81978187
protected void executeStepsToChangeOwnershipOfVm(AssignVMCmd cmd, Account caller, Account oldAccount, Account newAccount, UserVmVO vm, ServiceOfferingVO offering,
8198-
List<VolumeVO> volumes, VirtualMachineTemplate template, Long domainId) {
8188+
List<VolumeVO> volumes, VirtualMachineTemplate template) {
81998189

82008190
logger.trace("Generating destroy event for VM [{}].", vm);
82018191
UsageEventUtils.publishUsageEvent(EventTypes.EVENT_VM_DESTROY, vm.getAccountId(), vm.getDataCenterId(), vm.getId(), vm.getHostName(), vm.getServiceOfferingId(),
@@ -8208,7 +8198,7 @@ protected void executeStepsToChangeOwnershipOfVm(AssignVMCmd cmd, Account caller
82088198
removeInstanceFromInstanceGroup(vm.getId());
82098199

82108200
Long newAccountId = newAccount.getAccountId();
8211-
updateVmOwner(newAccount, vm, domainId, newAccountId);
8201+
updateVmOwner(newAccount, vm);
82128202

82138203
updateVolumesOwner(volumes, oldAccount, newAccount, newAccountId);
82148204

@@ -8231,11 +8221,11 @@ protected void executeStepsToChangeOwnershipOfVm(AssignVMCmd cmd, Account caller
82318221
vm.getTemplateId(), vm.getHypervisorType().toString(), VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm());
82328222
}
82338223

8234-
protected void updateVmOwner(Account newAccount, UserVmVO vm, Long domainId, Long newAccountId) {
8224+
protected void updateVmOwner(Account newAccount, UserVmVO vm) {
82358225
logger.debug("Updating VM [{}] owner to [{}].", vm, newAccount);
82368226

8237-
vm.setAccountId(newAccountId);
8238-
vm.setDomainId(domainId);
8227+
vm.setAccountId(newAccount.getId());
8228+
vm.setDomainId(newAccount.getDomainId());
82398229

82408230
_vmDao.persist(vm);
82418231
}

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

Lines changed: 17 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,6 @@
128128
import com.cloud.deploy.DeployDestination;
129129
import com.cloud.deploy.DeploymentPlanner;
130130
import com.cloud.deploy.DeploymentPlanningManager;
131-
import com.cloud.domain.Domain;
132131
import com.cloud.domain.DomainVO;
133132
import com.cloud.domain.dao.DomainDao;
134133
import com.cloud.event.ActionEventUtils;
@@ -735,7 +734,7 @@ private void configureDoNothingForMethodsThatWeDoNotWantToTest() throws Resource
735734
Mockito.doNothing().when(userVmManagerImpl).removeInstanceFromInstanceGroup(Mockito.anyLong());
736735
Mockito.doNothing().when(userVmManagerImpl).validateIfNewOwnerHasAccessToTemplate(Mockito.any(), Mockito.any(), Mockito.any());
737736

738-
Mockito.doNothing().when(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any());
737+
Mockito.doNothing().when(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any());
739738
Mockito.doNothing().when(userVmManagerImpl).updateVolumesOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any());
740739
Mockito.doNothing().when(userVmManagerImpl).updateVmNetwork(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any());
741740

@@ -2078,7 +2077,7 @@ public void validateIfNewOwnerHasAccessToTemplateTestCallCheckAccessWhenTemplate
20782077

20792078
@Test
20802079
public void updateVmOwnerTestCallsSetAccountIdSetDomainIdAndPersist() {
2081-
userVmManagerImpl.updateVmOwner(accountMock, userVmVoMock, 1l, 1l);
2080+
userVmManagerImpl.updateVmOwner(accountMock, userVmVoMock);
20822081

20832082
Mockito.verify(userVmVoMock).setAccountId(Mockito.anyLong());
20842083
Mockito.verify(userVmVoMock).setDomainId(Mockito.anyLong());
@@ -3033,23 +3032,22 @@ public void moveVmToUserTestValidateAccountsAndCallerAccessToThemThrowsInvalidPa
30333032
}
30343033

30353034
@Test
3036-
public void moveVmToUserTestProjectIdProvidedAndDomainIdIsNullThrowsInvalidParameterValueException() throws ResourceUnavailableException, InsufficientCapacityException,
3035+
public void moveVmToUserTestMovesVmWhenProjectIdIsProvidedAndDomainIdIsNull() throws ResourceUnavailableException, InsufficientCapacityException,
30373036
ResourceAllocationException {
3038-
3039-
String expectedMessage = "Please provide a valid domain ID; cannot assign VM to a project if domain ID is NULL.";
3040-
30413037
Mockito.doReturn(true).when(accountManager).isRootAdmin(Mockito.anyLong());
30423038
Mockito.doReturn(userVmVoMock).when(userVmDao).findById(Mockito.anyLong());
30433039
Mockito.doReturn(1l).when(assignVmCmdMock).getProjectId();
30443040
Mockito.doReturn(null).when(assignVmCmdMock).getDomainId();
3041+
Mockito.doReturn(null).when(userVmManagerImpl).ensureDestinationNetwork(Mockito.any(), Mockito.any(), Mockito.any());
3042+
Mockito.doNothing().when(userVmManagerImpl).executeStepsToChangeOwnershipOfVm(Mockito.any(), Mockito.any(), Mockito.any(),
3043+
Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any());
30453044

30463045
configureDoNothingForMethodsThatWeDoNotWantToTest();
30473046

3048-
InvalidParameterValueException assertThrows = Assert.assertThrows(expectedInvalidParameterValueException, () -> {
3049-
userVmManagerImpl.moveVmToUser(assignVmCmdMock);
3050-
});
3047+
userVmManagerImpl.moveVmToUser(assignVmCmdMock);
30513048

3052-
Assert.assertEquals(expectedMessage, assertThrows.getMessage());
3049+
Mockito.verify(userVmManagerImpl).executeStepsToChangeOwnershipOfVm(Mockito.any(), Mockito.any(), Mockito.any(),
3050+
Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any());
30533051
}
30543052

30553053
@Test
@@ -3123,26 +3121,6 @@ public void moveVmToUserTestVerifyValidateIfNewOwnerHasAccessToTemplateThrowsInv
31233121
Assert.assertThrows(InvalidParameterValueException.class, () -> userVmManagerImpl.moveVmToUser(assignVmCmdMock));
31243122
}
31253123

3126-
@Test
3127-
public void moveVmToUserTestAccountManagerCheckAccessThrowsPermissionDeniedException() throws ResourceUnavailableException, InsufficientCapacityException,
3128-
ResourceAllocationException {
3129-
3130-
LinkedList<VolumeVO> volumes = new LinkedList<VolumeVO>();
3131-
3132-
Mockito.doReturn(true).when(accountManager).isRootAdmin(Mockito.anyLong());
3133-
Mockito.doReturn(userVmVoMock).when(userVmDao).findById(Mockito.anyLong());
3134-
Mockito.doReturn(null).when(assignVmCmdMock).getProjectId();
3135-
Mockito.doReturn(volumes).when(volumeDaoMock).findByInstance(Mockito.anyLong());
3136-
Mockito.doReturn(accountMock).when(accountManager).finalizeOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any());
3137-
Mockito.doReturn(domainVoMock).when(domainDaoMock).findById(Mockito.anyLong());
3138-
3139-
configureDoNothingForMethodsThatWeDoNotWantToTest();
3140-
3141-
doThrow(PermissionDeniedException.class).when(accountManager).checkAccess(Mockito.any(Account.class), Mockito.any(Domain.class));
3142-
3143-
Assert.assertThrows(PermissionDeniedException.class, () -> userVmManagerImpl.moveVmToUser(assignVmCmdMock));
3144-
}
3145-
31463124
@Test
31473125
public void executeStepsToChangeOwnershipOfVmTestUpdateVmNetworkThrowsInsufficientCapacityException() throws ResourceUnavailableException, InsufficientCapacityException,
31483126
ResourceAllocationException {
@@ -3158,10 +3136,10 @@ public void executeStepsToChangeOwnershipOfVmTestUpdateVmNetworkThrowsInsufficie
31583136
Mockito.any());
31593137

31603138
Assert.assertThrows(CloudRuntimeException.class, () -> userVmManagerImpl.executeStepsToChangeOwnershipOfVm(assignVmCmdMock, callerAccount, accountMock, accountMock,
3161-
userVmVoMock, serviceOfferingVoMock, volumes, virtualMachineTemplateMock, 1l));
3139+
userVmVoMock, serviceOfferingVoMock, volumes, virtualMachineTemplateMock));
31623140

31633141
Mockito.verify(userVmManagerImpl).resourceCountDecrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any());
3164-
Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong());
3142+
Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any());
31653143
Mockito.verify(userVmManagerImpl).updateVolumesOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyLong());
31663144
}
31673145
}
@@ -3181,10 +3159,10 @@ public void executeStepsToChangeOwnershipOfVmTestUpdateVmNetworkThrowsResourceAl
31813159
Mockito.any());
31823160

31833161
Assert.assertThrows(CloudRuntimeException.class, () -> userVmManagerImpl.executeStepsToChangeOwnershipOfVm(assignVmCmdMock, callerAccount, accountMock, accountMock,
3184-
userVmVoMock, serviceOfferingVoMock, volumes, virtualMachineTemplateMock, 1l));
3162+
userVmVoMock, serviceOfferingVoMock, volumes, virtualMachineTemplateMock));
31853163

31863164
Mockito.verify(userVmManagerImpl).resourceCountDecrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any());
3187-
Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong());
3165+
Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any());
31883166
Mockito.verify(userVmManagerImpl).updateVolumesOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyLong());
31893167
}
31903168
}
@@ -3203,10 +3181,10 @@ public void executeStepsToChangeOwnershipOfVmTestResourceCountRunningVmsOnlyEnab
32033181
configureDoNothingForMethodsThatWeDoNotWantToTest();
32043182

32053183
userVmManagerImpl.executeStepsToChangeOwnershipOfVm(assignVmCmdMock, callerAccount, accountMock, accountMock, userVmVoMock, serviceOfferingVoMock, volumes,
3206-
virtualMachineTemplateMock, 1L);
3184+
virtualMachineTemplateMock);
32073185

32083186
Mockito.verify(userVmManagerImpl).resourceCountDecrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any());
3209-
Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong());
3187+
Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any());
32103188
Mockito.verify(userVmManagerImpl).updateVolumesOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyLong());
32113189
Mockito.verify(userVmManagerImpl).updateVmNetwork(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any());
32123190
Mockito.verify(userVmManagerImpl).resourceCountIncrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any());
@@ -3226,10 +3204,10 @@ public void executeStepsToChangeOwnershipOfVmTestResourceCountRunningVmsOnlyEnab
32263204
configureDoNothingForMethodsThatWeDoNotWantToTest();
32273205

32283206
userVmManagerImpl.executeStepsToChangeOwnershipOfVm(assignVmCmdMock, callerAccount, accountMock, accountMock, userVmVoMock, serviceOfferingVoMock, volumes,
3229-
virtualMachineTemplateMock, 1l);
3207+
virtualMachineTemplateMock);
32303208

32313209
Mockito.verify(userVmManagerImpl).resourceCountDecrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any());
3232-
Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any(), Mockito.anyLong(), Mockito.anyLong());
3210+
Mockito.verify(userVmManagerImpl).updateVmOwner(Mockito.any(), Mockito.any());
32333211
Mockito.verify(userVmManagerImpl).updateVolumesOwner(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.anyLong());
32343212
Mockito.verify(userVmManagerImpl).updateVmNetwork(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any());
32353213
Mockito.verify(userVmManagerImpl, Mockito.never()).resourceCountIncrement(Mockito.anyLong(), Mockito.any(), Mockito.any(), Mockito.any());

0 commit comments

Comments
 (0)