This is an automated email from the ASF dual-hosted git repository.
DaanHoogland pushed a commit to branch 4.20
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.20 by this push:
new 3a797993f3a Make `domainid` optional in `assignVirtualMachine` (#13213)
3a797993f3a is described below
commit 3a797993f3a117940c23d821af533b7b726c6b8b
Author: Fabricio Duarte <[email protected]>
AuthorDate: Mon Sep 7 07:40:43 2026 -0300
Make `domainid` optional in `assignVirtualMachine` (#13213)
---
.../main/java/com/cloud/vm/UserVmManagerImpl.java | 22 +++------
.../java/com/cloud/vm/UserVmManagerImplTest.java | 55 +++++++---------------
2 files changed, 23 insertions(+), 54 deletions(-)
diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
index 8617a2137c9..2d2cf80c830 100644
--- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
+++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
@@ -7463,11 +7463,6 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
checkCallerAccessToAccounts(caller, oldAccount, newAccount);
- logger.trace("Verifying if the provided domain ID [{}] is valid.",
domainId);
- if (projectId != null && domainId == null) {
- throw new InvalidParameterValueException("Please provide a valid
domain ID; cannot assign VM to a project if domain ID is NULL.");
- }
-
validateIfVmHasNoRules(vm, vmId);
final List<VolumeVO> volumes = _volsDao.findByInstance(vmId);
@@ -7478,10 +7473,6 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
validateIfNewOwnerHasAccessToTemplate(vm, newAccount, template);
- DomainVO domain = _domainDao.findById(domainId);
- logger.trace("Verifying if the new account [{}] has access to the
specified domain [{}].", newAccount, domain);
- _accountMgr.checkAccess(newAccount, domain);
-
List<Reserver> reservations = new ArrayList<>();
try {
verifyResourceLimitsForAccountAndStorage(newAccount, vm, offering,
volumes, template, reservations);
@@ -7491,7 +7482,7 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
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);
}
});
} catch (Exception e) {
@@ -7689,10 +7680,9 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
* @param offering The service offering which will be used to decrement
and increment resource counts.
* @param volumes The volumes of the VM which will be assigned to another
user.
* @param template The template of the VM which will be assigned to
another user.
- * @param domainId The ID of the domain where the VM which will be
assigned to another user is.
*/
protected void executeStepsToChangeOwnershipOfVm(AssignVMCmd cmd, Account
caller, Account oldAccount, Account newAccount, UserVmVO vm, ServiceOfferingVO
offering,
- List<VolumeVO> volumes,
VirtualMachineTemplate template, Long domainId) {
+ List<VolumeVO> volumes,
VirtualMachineTemplate 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(),
@@ -7705,7 +7695,7 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
removeInstanceFromInstanceGroup(vm.getId());
Long newAccountId = newAccount.getAccountId();
- updateVmOwner(newAccount, vm, domainId, newAccountId);
+ updateVmOwner(newAccount, vm);
updateVolumesOwner(volumes, oldAccount, newAccount, newAccountId);
@@ -7725,11 +7715,11 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
vm.getTemplateId(), vm.getHypervisorType().toString(),
VirtualMachine.class.getName(), vm.getUuid(), vm.isDisplayVm());
}
- protected void updateVmOwner(Account newAccount, UserVmVO vm, Long
domainId, Long newAccountId) {
+ protected void updateVmOwner(Account newAccount, UserVmVO vm) {
logger.debug("Updating VM [{}] owner to [{}].", vm, newAccount);
- vm.setAccountId(newAccountId);
- vm.setDomainId(domainId);
+ vm.setAccountId(newAccount.getId());
+ vm.setDomainId(newAccount.getDomainId());
_vmDao.persist(vm);
}
diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
index 15defe25dee..96f25307d9b 100644
--- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
+++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
@@ -618,7 +618,7 @@ public class UserVmManagerImplTest {
Mockito.doNothing().when(userVmManagerImpl).removeInstanceFromInstanceGroup(Mockito.anyLong());
Mockito.doNothing().when(userVmManagerImpl).validateIfNewOwnerHasAccessToTemplate(Mockito.any(),
Mockito.any(), Mockito.any());
-
Mockito.doNothing().when(userVmManagerImpl).updateVmOwner(Mockito.any(),
Mockito.any(), Mockito.any(), Mockito.any());
+
Mockito.doNothing().when(userVmManagerImpl).updateVmOwner(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());
@@ -1958,7 +1958,7 @@ public class UserVmManagerImplTest {
@Test
public void updateVmOwnerTestCallsSetAccountIdSetDomainIdAndPersist() {
- userVmManagerImpl.updateVmOwner(accountMock, userVmVoMock, 1l, 1l);
+ userVmManagerImpl.updateVmOwner(accountMock, userVmVoMock);
Mockito.verify(userVmVoMock).setAccountId(Mockito.anyLong());
Mockito.verify(userVmVoMock).setDomainId(Mockito.anyLong());
@@ -2913,23 +2913,22 @@ public class UserVmManagerImplTest {
}
@Test
- public void
moveVmToUserTestProjectIdProvidedAndDomainIdIsNullThrowsInvalidParameterValueException()
throws ResourceUnavailableException, InsufficientCapacityException,
+ public void
moveVmToUserTestMovesVmWhenProjectIdIsProvidedAndDomainIdIsNull() throws
ResourceUnavailableException, InsufficientCapacityException,
ResourceAllocationException {
-
- String expectedMessage = "Please provide a valid domain ID; cannot
assign VM to a project if domain ID is NULL.";
-
Mockito.doReturn(true).when(accountManager).isRootAdmin(Mockito.anyLong());
Mockito.doReturn(userVmVoMock).when(userVmDao).findById(Mockito.anyLong());
Mockito.doReturn(1l).when(assignVmCmdMock).getProjectId();
Mockito.doReturn(null).when(assignVmCmdMock).getDomainId();
+
Mockito.doReturn(null).when(userVmManagerImpl).ensureDestinationNetwork(Mockito.any(),
Mockito.any(), Mockito.any());
+
Mockito.doNothing().when(userVmManagerImpl).executeStepsToChangeOwnershipOfVm(Mockito.any(),
Mockito.any(), Mockito.any(),
+ Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(),
Mockito.any());
configureDoNothingForMethodsThatWeDoNotWantToTest();
- InvalidParameterValueException assertThrows =
Assert.assertThrows(expectedInvalidParameterValueException, () -> {
- userVmManagerImpl.moveVmToUser(assignVmCmdMock);
- });
+ userVmManagerImpl.moveVmToUser(assignVmCmdMock);
- Assert.assertEquals(expectedMessage, assertThrows.getMessage());
+
Mockito.verify(userVmManagerImpl).executeStepsToChangeOwnershipOfVm(Mockito.any(),
Mockito.any(), Mockito.any(),
+ Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any(),
Mockito.any());
}
@Test
@@ -3003,26 +3002,6 @@ public class UserVmManagerImplTest {
Assert.assertThrows(InvalidParameterValueException.class, () ->
userVmManagerImpl.moveVmToUser(assignVmCmdMock));
}
- @Test
- public void
moveVmToUserTestAccountManagerCheckAccessThrowsPermissionDeniedException()
throws ResourceUnavailableException, InsufficientCapacityException,
- ResourceAllocationException {
-
- LinkedList<VolumeVO> volumes = new LinkedList<VolumeVO>();
-
-
Mockito.doReturn(true).when(accountManager).isRootAdmin(Mockito.anyLong());
-
Mockito.doReturn(userVmVoMock).when(userVmDao).findById(Mockito.anyLong());
- Mockito.doReturn(null).when(assignVmCmdMock).getProjectId();
-
Mockito.doReturn(volumes).when(volumeDaoMock).findByInstance(Mockito.anyLong());
-
Mockito.doReturn(accountMock).when(accountManager).finalizeOwner(Mockito.any(),
Mockito.any(), Mockito.any(), Mockito.any());
-
Mockito.doReturn(domainVoMock).when(domainDaoMock).findById(Mockito.anyLong());
-
- configureDoNothingForMethodsThatWeDoNotWantToTest();
-
-
Mockito.doThrow(PermissionDeniedException.class).when(accountManager).checkAccess(Mockito.any(Account.class),
Mockito.any());
-
- Assert.assertThrows(PermissionDeniedException.class, () ->
userVmManagerImpl.moveVmToUser(assignVmCmdMock));
- }
-
@Test
public void
executeStepsToChangeOwnershipOfVmTestUpdateVmNetworkThrowsInsufficientCapacityException()
throws ResourceUnavailableException, InsufficientCapacityException,
ResourceAllocationException {
@@ -3038,10 +3017,10 @@ public class UserVmManagerImplTest {
Mockito.any());
Assert.assertThrows(CloudRuntimeException.class, () ->
userVmManagerImpl.executeStepsToChangeOwnershipOfVm(assignVmCmdMock,
callerAccount, accountMock, accountMock,
- userVmVoMock, serviceOfferingVoMock, volumes,
virtualMachineTemplateMock, 1l));
+ userVmVoMock, serviceOfferingVoMock, volumes,
virtualMachineTemplateMock));
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).updateVmOwner(Mockito.any(),
Mockito.any());
Mockito.verify(userVmManagerImpl).updateVolumesOwner(Mockito.any(),
Mockito.any(), Mockito.any(), Mockito.anyLong());
}
}
@@ -3061,10 +3040,10 @@ public class UserVmManagerImplTest {
Mockito.any());
Assert.assertThrows(CloudRuntimeException.class, () ->
userVmManagerImpl.executeStepsToChangeOwnershipOfVm(assignVmCmdMock,
callerAccount, accountMock, accountMock,
- userVmVoMock, serviceOfferingVoMock, volumes,
virtualMachineTemplateMock, 1l));
+ userVmVoMock, serviceOfferingVoMock, volumes,
virtualMachineTemplateMock));
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).updateVmOwner(Mockito.any(),
Mockito.any());
Mockito.verify(userVmManagerImpl).updateVolumesOwner(Mockito.any(),
Mockito.any(), Mockito.any(), Mockito.anyLong());
}
}
@@ -3083,10 +3062,10 @@ public class UserVmManagerImplTest {
configureDoNothingForMethodsThatWeDoNotWantToTest();
userVmManagerImpl.executeStepsToChangeOwnershipOfVm(assignVmCmdMock,
callerAccount, accountMock, accountMock, userVmVoMock, serviceOfferingVoMock,
volumes,
- virtualMachineTemplateMock, 1l);
+ virtualMachineTemplateMock);
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).updateVmOwner(Mockito.any(),
Mockito.any());
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).resourceCountIncrement(Mockito.anyLong(),
Mockito.any(), Mockito.any(), Mockito.any());
@@ -3106,10 +3085,10 @@ public class UserVmManagerImplTest {
configureDoNothingForMethodsThatWeDoNotWantToTest();
userVmManagerImpl.executeStepsToChangeOwnershipOfVm(assignVmCmdMock,
callerAccount, accountMock, accountMock, userVmVoMock, serviceOfferingVoMock,
volumes,
- virtualMachineTemplateMock, 1l);
+ virtualMachineTemplateMock);
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).updateVmOwner(Mockito.any(),
Mockito.any());
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,
Mockito.never()).resourceCountIncrement(Mockito.anyLong(), Mockito.any(),
Mockito.any(), Mockito.any());
