From 9aee625ab45465a6cfe4d95f60f9be67ce391ffe Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 31 Mar 2023 15:14:09 +0530 Subject: [PATCH] orchestration: fix error on deleted template vm start (#7327) * server: fix error on deleted template vm start When a VM is deployed with start flag as false and the template is deleted before the VM start, NPE is obeserved it is started for the first time. Signed-off-by: Abhishek Kumar * fix Signed-off-by: Abhishek Kumar * fix Signed-off-by: Abhishek Kumar --------- Signed-off-by: Abhishek Kumar --- .../cloud/vm/VirtualMachineManagerImpl.java | 29 ++++++++- .../orchestration/VolumeOrchestrator.java | 12 ++-- .../vm/VirtualMachineManagerImplTest.java | 65 ++++++++++++++++++- 3 files changed, 99 insertions(+), 7 deletions(-) diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index fa1ad061a0b..9bbe35fc97e 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -17,6 +17,8 @@ package com.cloud.vm; +import static com.cloud.configuration.ConfigurationManagerImpl.MIGRATE_VM_ACROSS_CLUSTERS; + import java.net.URI; import java.sql.PreparedStatement; import java.sql.ResultSet; @@ -204,6 +206,7 @@ import com.cloud.storage.Storage.ImageFormat; import com.cloud.storage.StorageManager; import com.cloud.storage.StoragePool; import com.cloud.storage.VMTemplateVO; +import com.cloud.storage.VMTemplateZoneVO; import com.cloud.storage.Volume; import com.cloud.storage.Volume.Type; import com.cloud.storage.VolumeApiService; @@ -213,6 +216,7 @@ import com.cloud.storage.dao.GuestOSCategoryDao; import com.cloud.storage.dao.GuestOSDao; import com.cloud.storage.dao.StoragePoolHostDao; import com.cloud.storage.dao.VMTemplateDao; +import com.cloud.storage.dao.VMTemplateZoneDao; import com.cloud.storage.dao.VolumeDao; import com.cloud.template.VirtualMachineTemplate; import com.cloud.user.Account; @@ -253,8 +257,6 @@ import com.cloud.vm.snapshot.VMSnapshotManager; import com.cloud.vm.snapshot.VMSnapshotVO; import com.cloud.vm.snapshot.dao.VMSnapshotDao; -import static com.cloud.configuration.ConfigurationManagerImpl.MIGRATE_VM_ACROSS_CLUSTERS; - public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMachineManager, VmWorkJobHandler, Listener, Configurable { private static final Logger s_logger = Logger.getLogger(VirtualMachineManagerImpl.class); @@ -281,6 +283,8 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac @Inject private VMTemplateDao _templateDao; @Inject + private VMTemplateZoneDao templateZoneDao; + @Inject private ItWorkDao _workDao; @Inject private UserVmDao _userVmDao; @@ -1005,6 +1009,25 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } } + protected void checkIfTemplateNeededForCreatingVmVolumes(VMInstanceVO vm) { + final List existingRootVolumes = _volsDao.findReadyRootVolumesByInstance(vm.getId()); + if (CollectionUtils.isNotEmpty(existingRootVolumes)) { + return; + } + final VMTemplateVO template = _templateDao.findById(vm.getTemplateId()); + if (template == null) { + String msg = "Template for the VM instance can not be found, VM instance configuration needs to be updated"; + s_logger.error(String.format("%s. Template ID: %d seems to be removed", msg, vm.getTemplateId())); + throw new CloudRuntimeException(msg); + } + final VMTemplateZoneVO templateZoneVO = templateZoneDao.findByZoneTemplate(vm.getDataCenterId(), template.getId()); + if (templateZoneVO == null) { + String msg = "Template for the VM instance can not be found in the zone ID: %s, VM instance configuration needs to be updated"; + s_logger.error(String.format("%s. %s", msg, template)); + throw new CloudRuntimeException(msg); + } + } + @Override public void orchestrateStart(final String vmUuid, final Map params, final DeploymentPlan planToDeploy, final DeploymentPlanner planner) throws InsufficientCapacityException, ConcurrentOperationException, ResourceUnavailableException { @@ -1068,6 +1091,8 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac boolean reuseVolume = true; final DataCenterDeployment originalPlan = plan; + checkIfTemplateNeededForCreatingVmVolumes(vm); + int retry = StartRetry.value(); while (retry-- != 0) { s_logger.debug("VM start attempt #" + (StartRetry.value() - retry)); diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java index 476ecf23cbc..4ff4483e3ba 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java @@ -37,7 +37,6 @@ import java.util.stream.Collectors; import javax.inject.Inject; import javax.naming.ConfigurationException; -import com.cloud.storage.StorageUtil; import org.apache.cloudstack.api.command.admin.vm.MigrateVMCmd; import org.apache.cloudstack.api.command.admin.volume.MigrateVolumeCmdByAdmin; import org.apache.cloudstack.api.command.user.volume.MigrateVolumeCmd; @@ -69,6 +68,7 @@ import org.apache.cloudstack.framework.jobs.AsyncJobManager; import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO; import org.apache.cloudstack.resourcedetail.DiskOfferingDetailVO; import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao; +import org.apache.cloudstack.snapshot.SnapshotHelper; import org.apache.cloudstack.storage.command.CommandResult; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao; @@ -79,6 +79,7 @@ import org.apache.commons.collections.CollectionUtils; import org.apache.commons.collections.MapUtils; import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; +import org.jetbrains.annotations.Nullable; import com.cloud.agent.api.to.DataTO; import com.cloud.agent.api.to.DatadiskTO; @@ -114,6 +115,7 @@ import com.cloud.storage.Storage; import com.cloud.storage.Storage.ImageFormat; import com.cloud.storage.StorageManager; import com.cloud.storage.StoragePool; +import com.cloud.storage.StorageUtil; import com.cloud.storage.VMTemplateStorageResourceAssoc; import com.cloud.storage.Volume; import com.cloud.storage.Volume.Type; @@ -161,9 +163,6 @@ import com.cloud.vm.dao.UserVmCloneSettingDao; import com.cloud.vm.dao.UserVmDao; import com.cloud.vm.dao.UserVmDetailsDao; -import org.apache.cloudstack.snapshot.SnapshotHelper; -import org.jetbrains.annotations.Nullable; - public class VolumeOrchestrator extends ManagerBase implements VolumeOrchestrationService, Configurable { public enum UserVmCloneType { @@ -1546,6 +1545,11 @@ public class VolumeOrchestrator extends ManagerBase implements VolumeOrchestrati future = volService.createVolumeAsync(volume, destPool); } else { + final VirtualMachineTemplate template = _entityMgr.findById(VirtualMachineTemplate.class, templateId); + if (template == null) { + s_logger.error(String.format("Failed to find template: %d for %s", templateId, volume)); + throw new CloudRuntimeException(String.format("Failed to find template for volume ID: %s", volume.getUuid())); + } TemplateInfo templ = tmplFactory.getReadyTemplateOnImageStore(templateId, dest.getDataCenter().getId()); PrimaryDataStore primaryDataStore = (PrimaryDataStore)destPool; diff --git a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java index cea863d1bde..7079eabc4b0 100644 --- a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java +++ b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java @@ -31,7 +31,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import com.cloud.exception.InvalidParameterValueException; import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; @@ -54,6 +53,7 @@ import com.cloud.deploy.DataCenterDeployment; import com.cloud.deploy.DeploymentPlan; import com.cloud.deploy.DeploymentPlanner; import com.cloud.deploy.DeploymentPlanner.ExcludeList; +import com.cloud.exception.InvalidParameterValueException; import com.cloud.host.HostVO; import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.hypervisor.HypervisorGuru; @@ -64,10 +64,14 @@ import com.cloud.storage.DiskOfferingVO; import com.cloud.storage.ScopeType; import com.cloud.storage.StoragePool; import com.cloud.storage.StoragePoolHostVO; +import com.cloud.storage.VMTemplateVO; +import com.cloud.storage.VMTemplateZoneVO; import com.cloud.storage.Volume; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.DiskOfferingDao; import com.cloud.storage.dao.StoragePoolHostDao; +import com.cloud.storage.dao.VMTemplateDao; +import com.cloud.storage.dao.VMTemplateZoneDao; import com.cloud.storage.dao.VolumeDao; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.VirtualMachine.State; @@ -124,6 +128,10 @@ public class VirtualMachineManagerImplTest { @Mock private DiskOfferingDao diskOfferingDaoMock; + @Mock + VMTemplateDao templateDao; + @Mock + VMTemplateZoneDao templateZoneDao; @Before public void setup() { @@ -693,4 +701,59 @@ public class VirtualMachineManagerImplTest { Mockito.doReturn(isOfferingUsingLocal).when(diskOfferingMock).isUseLocalStorage(); virtualMachineManagerImpl.checkIfNewOfferingStorageScopeMatchesStoragePool(vmInstanceMock, diskOfferingMock); } + + @Test + public void checkIfTemplateNeededForCreatingVmVolumesExistingRootVolumes() { + long vmId = 1L; + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(vmId); + Mockito.when(volumeDaoMock.findReadyRootVolumesByInstance(vmId)).thenReturn(List.of(Mockito.mock(VolumeVO.class))); + virtualMachineManagerImpl.checkIfTemplateNeededForCreatingVmVolumes(vm); + } + + @Test(expected = CloudRuntimeException.class) + public void checkIfTemplateNeededForCreatingVmVolumesMissingTemplate() { + long vmId = 1L; + long templateId = 1L; + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(vmId); + Mockito.when(vm.getTemplateId()).thenReturn(templateId); + Mockito.when(volumeDaoMock.findReadyRootVolumesByInstance(vmId)).thenReturn(null); + Mockito.when(templateDao.findById(templateId)).thenReturn(null); + virtualMachineManagerImpl.checkIfTemplateNeededForCreatingVmVolumes(vm); + } + + @Test(expected = CloudRuntimeException.class) + public void checkIfTemplateNeededForCreatingVmVolumesMissingZoneTemplate() { + long vmId = 1L; + long templateId = 1L; + long dcId = 1L; + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(vmId); + Mockito.when(vm.getTemplateId()).thenReturn(templateId); + Mockito.when(vm.getDataCenterId()).thenReturn(dcId); + Mockito.when(volumeDaoMock.findReadyRootVolumesByInstance(vmId)).thenReturn(null); + VMTemplateVO template = Mockito.mock(VMTemplateVO.class); + Mockito.when(vm.getId()).thenReturn(templateId); + Mockito.when(templateDao.findById(templateId)).thenReturn(template); + Mockito.when(templateZoneDao.findByZoneTemplate(dcId, templateId)).thenReturn(null); + virtualMachineManagerImpl.checkIfTemplateNeededForCreatingVmVolumes(vm); + } + + @Test + public void checkIfTemplateNeededForCreatingVmVolumesTemplateAvailable() { + long vmId = 1L; + long templateId = 1L; + long dcId = 1L; + VMInstanceVO vm = Mockito.mock(VMInstanceVO.class); + Mockito.when(vm.getId()).thenReturn(vmId); + Mockito.when(vm.getTemplateId()).thenReturn(templateId); + Mockito.when(vm.getDataCenterId()).thenReturn(dcId); + Mockito.when(volumeDaoMock.findReadyRootVolumesByInstance(vmId)).thenReturn(new ArrayList<>()); + VMTemplateVO template = Mockito.mock(VMTemplateVO.class); + Mockito.when(template.getId()).thenReturn(templateId); + Mockito.when(templateDao.findById(templateId)).thenReturn(template); + Mockito.when(templateZoneDao.findByZoneTemplate(dcId, templateId)).thenReturn(Mockito.mock(VMTemplateZoneVO.class)); + virtualMachineManagerImpl.checkIfTemplateNeededForCreatingVmVolumes(vm); + } }