From 863f14a9068c86370e3dda590bdd50544cb8e253 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Wed, 20 Mar 2024 16:34:43 +0530 Subject: [PATCH 1/5] Fix VM import & VM delete with custom offering --- .../java/com/cloud/vm/VirtualMachineProfile.java | 2 ++ .../com/cloud/vm/VirtualMachineProfileImpl.java | 4 ++-- .../cloudstack/vm/UnmanagedVMsManagerImpl.java | 15 ++++++++++----- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/api/src/main/java/com/cloud/vm/VirtualMachineProfile.java b/api/src/main/java/com/cloud/vm/VirtualMachineProfile.java index 62519412fdb5..f2ff3da8449e 100644 --- a/api/src/main/java/com/cloud/vm/VirtualMachineProfile.java +++ b/api/src/main/java/com/cloud/vm/VirtualMachineProfile.java @@ -60,6 +60,8 @@ public interface VirtualMachineProfile { void setConfigDriveLocation(NetworkElement.Location location); + void setServiceOffering(ServiceOffering offering); + public static class Param { public static final Param VmPassword = new Param("VmPassword"); diff --git a/engine/components-api/src/main/java/com/cloud/vm/VirtualMachineProfileImpl.java b/engine/components-api/src/main/java/com/cloud/vm/VirtualMachineProfileImpl.java index bc689fef8736..2d51c3c08703 100644 --- a/engine/components-api/src/main/java/com/cloud/vm/VirtualMachineProfileImpl.java +++ b/engine/components-api/src/main/java/com/cloud/vm/VirtualMachineProfileImpl.java @@ -26,7 +26,6 @@ import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.network.element.NetworkElement; import com.cloud.offering.ServiceOffering; -import com.cloud.service.ServiceOfferingVO; import com.cloud.template.VirtualMachineTemplate; import com.cloud.template.VirtualMachineTemplate.BootloaderType; import com.cloud.user.Account; @@ -260,7 +259,8 @@ public Map getParameters() { return _params; } - public void setServiceOffering(ServiceOfferingVO offering) { + @Override + public void setServiceOffering(ServiceOffering offering) { _offering = offering; } diff --git a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java index 1ed5a8f1648a..879f44f9d6d5 100644 --- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java @@ -896,6 +896,7 @@ private UserVm migrateImportedVM(HostVO sourceHost, VirtualMachineTemplate templ if (!hostSupportsServiceOffering(sourceHost, serviceOffering)) { LOGGER.debug(String.format("VM %s needs to be migrated", vm.getUuid())); final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm, template, serviceOffering, owner, null); + profile.setServiceOffering(serviceOfferingDao.findByIdIncludingRemoved(vm.getId(), serviceOffering.getId())); DeploymentPlanner.ExcludeList excludeList = new DeploymentPlanner.ExcludeList(); excludeList.addHost(sourceHost.getId()); final DataCenterDeployment plan = new DataCenterDeployment(sourceHost.getDataCenterId(), sourceHost.getPodId(), sourceHost.getClusterId(), null, null, null); @@ -1083,7 +1084,7 @@ private UserVm importVirtualMachineInternal(final UnmanagedInstanceTO unmanagedI internalCSName = instanceName; } Map allDetails = new HashMap<>(details); - if (validatedServiceOffering.isDynamic()) { + if (!validatedServiceOffering.isDynamic()) { allDetails.put(VmDetailConstants.CPU_NUMBER, String.valueOf(validatedServiceOffering.getCpu())); allDetails.put(VmDetailConstants.MEMORY, String.valueOf(validatedServiceOffering.getRamSize())); if (serviceOffering.getSpeed() == null) { @@ -2131,7 +2132,7 @@ private UserVm importExternalKvmVirtualMachine(final UnmanagedInstanceTO unmanag UserVm userVm = null; Map allDetails = new HashMap<>(details); - if (serviceOffering.isDynamic()) { + if (!serviceOffering.isDynamic()) { allDetails.put(VmDetailConstants.CPU_NUMBER, String.valueOf(serviceOffering.getCpu())); allDetails.put(VmDetailConstants.MEMORY, String.valueOf(serviceOffering.getRamSize())); allDetails.put(VmDetailConstants.CPU_SPEED, String.valueOf(serviceOffering.getSpeed())); @@ -2189,6 +2190,8 @@ private UserVm importExternalKvmVirtualMachine(final UnmanagedInstanceTO unmanag } final VirtualMachineProfile profile = new VirtualMachineProfileImpl(userVm, template, serviceOffering, owner, null); + ServiceOfferingVO dummyOffering = serviceOfferingDao.findByIdIncludingRemoved(userVm.getId(), serviceOffering.getId()); + profile.setServiceOffering(dummyOffering); DeploymentPlanner.ExcludeList excludeList = new DeploymentPlanner.ExcludeList(); final DataCenterDeployment plan = new DataCenterDeployment(zone.getId(), null, null, null, null, null); DeployDestination dest = null; @@ -2240,7 +2243,7 @@ private UserVm importExternalKvmVirtualMachine(final UnmanagedInstanceTO unmanag cleanupFailedImportVM(userVm); throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Failed to import NICs while importing vm: %s. %s", instanceName, StringUtils.defaultString(e.getMessage()))); } - publishVMUsageUpdateResourceCount(userVm, serviceOffering); + publishVMUsageUpdateResourceCount(userVm, dummyOffering); return userVm; } @@ -2252,7 +2255,7 @@ private UserVm importKvmVirtualMachineFromDisk(final ImportSource importSource, UserVm userVm = null; Map allDetails = new HashMap<>(details); - if (serviceOffering.isDynamic()) { + if (!serviceOffering.isDynamic()) { allDetails.put(VmDetailConstants.CPU_NUMBER, String.valueOf(serviceOffering.getCpu())); allDetails.put(VmDetailConstants.MEMORY, String.valueOf(serviceOffering.getRamSize())); allDetails.put(VmDetailConstants.CPU_SPEED, String.valueOf(serviceOffering.getSpeed())); @@ -2323,6 +2326,8 @@ private UserVm importKvmVirtualMachineFromDisk(final ImportSource importSource, DiskProfile diskProfile = volumeManager.allocateRawVolume(Volume.Type.ROOT, rootVolumeName, diskOffering, null, null, null, userVm, template, owner, null); final VirtualMachineProfile profile = new VirtualMachineProfileImpl(userVm, template, serviceOffering, owner, null); + ServiceOfferingVO dummyOffering = serviceOfferingDao.findByIdIncludingRemoved(userVm.getId(), serviceOffering.getId()); + profile.setServiceOffering(dummyOffering); DeploymentPlanner.ExcludeList excludeList = new DeploymentPlanner.ExcludeList(); final DataCenterDeployment plan = new DataCenterDeployment(zone.getId(), null, null, hostId, poolId, null); DeployDestination dest = null; @@ -2374,7 +2379,7 @@ private UserVm importKvmVirtualMachineFromDisk(final ImportSource importSource, throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("Failed to import volumes while importing vm: %s. %s", instanceName, StringUtils.defaultString(e.getMessage()))); } networkOrchestrationService.importNic(macAddress,0,network, true, userVm, requestedIpPair, zone, true); - publishVMUsageUpdateResourceCount(userVm, serviceOffering); + publishVMUsageUpdateResourceCount(userVm, dummyOffering); return userVm; } From a0a7adf8a886c34927b27bc129fb7cdf92a76f37 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Wed, 20 Mar 2024 21:50:49 +0530 Subject: [PATCH 2/5] Fix test failures --- .../org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java | 1 + 1 file changed, 1 insertion(+) diff --git a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java index 229e59b1a7a3..a1429355ecd7 100644 --- a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java @@ -332,6 +332,7 @@ public void setUp() throws Exception { when(serviceOffering.getRamSize()).thenReturn(instance.getMemory()); when(serviceOffering.getSpeed()).thenReturn(instance.getCpuSpeed()); when(serviceOfferingDao.findById(Mockito.anyLong())).thenReturn(serviceOffering); + when(serviceOfferingDao.findByIdIncludingRemoved(anyLong(), anyLong())).thenReturn(Mockito.mock(ServiceOfferingVO.class)); DiskOfferingVO diskOfferingVO = Mockito.mock(DiskOfferingVO.class); when(diskOfferingDao.findById(Mockito.anyLong())).thenReturn(diskOfferingVO); UserVmVO userVm = Mockito.mock(UserVmVO.class); From ae79f022e7a2a402dff822c753b8513a1bdafd58 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Tue, 26 Mar 2024 17:38:43 +0530 Subject: [PATCH 3/5] Resolve comments --- .../cloudstack/vm/UnmanagedVMsManagerImpl.java | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java index 879f44f9d6d5..c349e5f5a839 100644 --- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java @@ -1084,7 +1084,7 @@ private UserVm importVirtualMachineInternal(final UnmanagedInstanceTO unmanagedI internalCSName = instanceName; } Map allDetails = new HashMap<>(details); - if (!validatedServiceOffering.isDynamic()) { + if (validatedServiceOffering.isDynamic()) { allDetails.put(VmDetailConstants.CPU_NUMBER, String.valueOf(validatedServiceOffering.getCpu())); allDetails.put(VmDetailConstants.MEMORY, String.valueOf(validatedServiceOffering.getRamSize())); if (serviceOffering.getSpeed() == null) { @@ -2132,11 +2132,6 @@ private UserVm importExternalKvmVirtualMachine(final UnmanagedInstanceTO unmanag UserVm userVm = null; Map allDetails = new HashMap<>(details); - if (!serviceOffering.isDynamic()) { - allDetails.put(VmDetailConstants.CPU_NUMBER, String.valueOf(serviceOffering.getCpu())); - allDetails.put(VmDetailConstants.MEMORY, String.valueOf(serviceOffering.getRamSize())); - allDetails.put(VmDetailConstants.CPU_SPEED, String.valueOf(serviceOffering.getSpeed())); - } // Check disks and supplied disk offerings List unmanagedInstanceDisks = unmanagedInstance.getDisks(); @@ -2255,11 +2250,6 @@ private UserVm importKvmVirtualMachineFromDisk(final ImportSource importSource, UserVm userVm = null; Map allDetails = new HashMap<>(details); - if (!serviceOffering.isDynamic()) { - allDetails.put(VmDetailConstants.CPU_NUMBER, String.valueOf(serviceOffering.getCpu())); - allDetails.put(VmDetailConstants.MEMORY, String.valueOf(serviceOffering.getRamSize())); - allDetails.put(VmDetailConstants.CPU_SPEED, String.valueOf(serviceOffering.getSpeed())); - } VirtualMachine.PowerState powerState = VirtualMachine.PowerState.PowerOff; From 74ed2720ad4e9ad2ecc891857e9095b010713b6f Mon Sep 17 00:00:00 2001 From: Vishesh Date: Wed, 27 Mar 2024 13:10:31 +0530 Subject: [PATCH 4/5] fixup --- .../org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java index c349e5f5a839..069f749e3599 100644 --- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java @@ -896,7 +896,7 @@ private UserVm migrateImportedVM(HostVO sourceHost, VirtualMachineTemplate templ if (!hostSupportsServiceOffering(sourceHost, serviceOffering)) { LOGGER.debug(String.format("VM %s needs to be migrated", vm.getUuid())); final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm, template, serviceOffering, owner, null); - profile.setServiceOffering(serviceOfferingDao.findByIdIncludingRemoved(vm.getId(), serviceOffering.getId())); + profile.setServiceOffering(serviceOfferingDao.findById(vm.getId(), serviceOffering.getId())); DeploymentPlanner.ExcludeList excludeList = new DeploymentPlanner.ExcludeList(); excludeList.addHost(sourceHost.getId()); final DataCenterDeployment plan = new DataCenterDeployment(sourceHost.getDataCenterId(), sourceHost.getPodId(), sourceHost.getClusterId(), null, null, null); @@ -2185,7 +2185,7 @@ private UserVm importExternalKvmVirtualMachine(final UnmanagedInstanceTO unmanag } final VirtualMachineProfile profile = new VirtualMachineProfileImpl(userVm, template, serviceOffering, owner, null); - ServiceOfferingVO dummyOffering = serviceOfferingDao.findByIdIncludingRemoved(userVm.getId(), serviceOffering.getId()); + ServiceOfferingVO dummyOffering = serviceOfferingDao.findById(userVm.getId(), serviceOffering.getId()); profile.setServiceOffering(dummyOffering); DeploymentPlanner.ExcludeList excludeList = new DeploymentPlanner.ExcludeList(); final DataCenterDeployment plan = new DataCenterDeployment(zone.getId(), null, null, null, null, null); @@ -2316,7 +2316,7 @@ private UserVm importKvmVirtualMachineFromDisk(final ImportSource importSource, DiskProfile diskProfile = volumeManager.allocateRawVolume(Volume.Type.ROOT, rootVolumeName, diskOffering, null, null, null, userVm, template, owner, null); final VirtualMachineProfile profile = new VirtualMachineProfileImpl(userVm, template, serviceOffering, owner, null); - ServiceOfferingVO dummyOffering = serviceOfferingDao.findByIdIncludingRemoved(userVm.getId(), serviceOffering.getId()); + ServiceOfferingVO dummyOffering = serviceOfferingDao.findById(userVm.getId(), serviceOffering.getId()); profile.setServiceOffering(dummyOffering); DeploymentPlanner.ExcludeList excludeList = new DeploymentPlanner.ExcludeList(); final DataCenterDeployment plan = new DataCenterDeployment(zone.getId(), null, null, hostId, poolId, null); From 654379abd963af643a2e15b5dfeac42531b00a54 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Wed, 27 Mar 2024 13:30:21 +0530 Subject: [PATCH 5/5] fixup --- .../org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java index a1429355ecd7..92131e4f75c9 100644 --- a/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java +++ b/server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java @@ -332,7 +332,7 @@ public void setUp() throws Exception { when(serviceOffering.getRamSize()).thenReturn(instance.getMemory()); when(serviceOffering.getSpeed()).thenReturn(instance.getCpuSpeed()); when(serviceOfferingDao.findById(Mockito.anyLong())).thenReturn(serviceOffering); - when(serviceOfferingDao.findByIdIncludingRemoved(anyLong(), anyLong())).thenReturn(Mockito.mock(ServiceOfferingVO.class)); + when(serviceOfferingDao.findById(anyLong(), anyLong())).thenReturn(Mockito.mock(ServiceOfferingVO.class)); DiskOfferingVO diskOfferingVO = Mockito.mock(DiskOfferingVO.class); when(diskOfferingDao.findById(Mockito.anyLong())).thenReturn(diskOfferingVO); UserVmVO userVm = Mockito.mock(UserVmVO.class);