Skip to content

Commit a267c73

Browse files
shwstpprkishankavala
authored andcommitted
orchestration: fix error on deleted template vm start (apache#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 <abhishek.mrt22@gmail.com> * fix Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com> * fix Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com> --------- Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
1 parent 25a41ee commit a267c73

File tree

3 files changed

+96
-3
lines changed

3 files changed

+96
-3
lines changed

engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,7 @@
215215
import com.cloud.storage.StorageManager;
216216
import com.cloud.storage.StoragePool;
217217
import com.cloud.storage.VMTemplateVO;
218+
import com.cloud.storage.VMTemplateZoneVO;
218219
import com.cloud.storage.Volume;
219220
import com.cloud.storage.Volume.Type;
220221
import com.cloud.storage.VolumeApiService;
@@ -225,6 +226,7 @@
225226
import com.cloud.storage.dao.GuestOSDao;
226227
import com.cloud.storage.dao.StoragePoolHostDao;
227228
import com.cloud.storage.dao.VMTemplateDao;
229+
import com.cloud.storage.dao.VMTemplateZoneDao;
228230
import com.cloud.storage.dao.VolumeDao;
229231
import com.cloud.template.VirtualMachineTemplate;
230232
import com.cloud.user.Account;
@@ -291,6 +293,8 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
291293
@Inject
292294
private VMTemplateDao _templateDao;
293295
@Inject
296+
private VMTemplateZoneDao templateZoneDao;
297+
@Inject
294298
private ItWorkDao _workDao;
295299
@Inject
296300
private UserVmDao _userVmDao;
@@ -1031,6 +1035,25 @@ protected void addHostIpToCertDetailsIfConfigAllows(Host vmHost, Map<String, Str
10311035
}
10321036
}
10331037

1038+
protected void checkIfTemplateNeededForCreatingVmVolumes(VMInstanceVO vm) {
1039+
final List<VolumeVO> existingRootVolumes = _volsDao.findReadyRootVolumesByInstance(vm.getId());
1040+
if (CollectionUtils.isNotEmpty(existingRootVolumes)) {
1041+
return;
1042+
}
1043+
final VMTemplateVO template = _templateDao.findById(vm.getTemplateId());
1044+
if (template == null) {
1045+
String msg = "Template for the VM instance can not be found, VM instance configuration needs to be updated";
1046+
s_logger.error(String.format("%s. Template ID: %d seems to be removed", msg, vm.getTemplateId()));
1047+
throw new CloudRuntimeException(msg);
1048+
}
1049+
final VMTemplateZoneVO templateZoneVO = templateZoneDao.findByZoneTemplate(vm.getDataCenterId(), template.getId());
1050+
if (templateZoneVO == null) {
1051+
String msg = "Template for the VM instance can not be found in the zone ID: %s, VM instance configuration needs to be updated";
1052+
s_logger.error(String.format("%s. %s", msg, template));
1053+
throw new CloudRuntimeException(msg);
1054+
}
1055+
}
1056+
10341057
@Override
10351058
public void orchestrateStart(final String vmUuid, final Map<VirtualMachineProfile.Param, Object> params, final DeploymentPlan planToDeploy, final DeploymentPlanner planner)
10361059
throws InsufficientCapacityException, ConcurrentOperationException, ResourceUnavailableException {
@@ -1094,6 +1117,8 @@ public void orchestrateStart(final String vmUuid, final Map<VirtualMachineProfil
10941117
boolean reuseVolume = true;
10951118
final DataCenterDeployment originalPlan = plan;
10961119

1120+
checkIfTemplateNeededForCreatingVmVolumes(vm);
1121+
10971122
int retry = StartRetry.value();
10981123
while (retry-- != 0) {
10991124
s_logger.debug("VM start attempt #" + (StartRetry.value() - retry));

engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@
7676
import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO;
7777
import org.apache.cloudstack.resourcedetail.DiskOfferingDetailVO;
7878
import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao;
79+
import org.apache.cloudstack.snapshot.SnapshotHelper;
7980
import org.apache.cloudstack.storage.command.CommandResult;
8081
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
8182
import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
@@ -87,6 +88,7 @@
8788
import org.apache.commons.collections.MapUtils;
8889
import org.apache.commons.lang3.StringUtils;
8990
import org.apache.log4j.Logger;
91+
import org.jetbrains.annotations.Nullable;
9092

9193
import com.cloud.agent.api.to.DataTO;
9294
import com.cloud.agent.api.to.DatadiskTO;
@@ -121,6 +123,7 @@
121123
import com.cloud.storage.Storage.ImageFormat;
122124
import com.cloud.storage.StorageManager;
123125
import com.cloud.storage.StoragePool;
126+
import com.cloud.storage.StorageUtil;
124127
import com.cloud.storage.VMTemplateStorageResourceAssoc;
125128
import com.cloud.storage.Volume;
126129
import com.cloud.storage.Volume.Type;
@@ -168,9 +171,6 @@
168171
import com.cloud.vm.dao.UserVmDao;
169172
import com.cloud.vm.dao.UserVmDetailsDao;
170173

171-
import org.apache.cloudstack.snapshot.SnapshotHelper;
172-
import org.jetbrains.annotations.Nullable;
173-
174174
public class VolumeOrchestrator extends ManagerBase implements VolumeOrchestrationService, Configurable {
175175

176176
public enum UserVmCloneType {
@@ -1710,6 +1710,11 @@ private Pair<VolumeVO, DataStore> recreateVolume(VolumeVO vol, VirtualMachinePro
17101710

17111711
future = volService.createVolumeAsync(volume, destPool);
17121712
} else {
1713+
final VirtualMachineTemplate template = _entityMgr.findById(VirtualMachineTemplate.class, templateId);
1714+
if (template == null) {
1715+
s_logger.error(String.format("Failed to find template: %d for %s", templateId, volume));
1716+
throw new CloudRuntimeException(String.format("Failed to find template for volume ID: %s", volume.getUuid()));
1717+
}
17131718
TemplateInfo templ = tmplFactory.getReadyTemplateOnImageStore(templateId, dest.getDataCenter().getId());
17141719
PrimaryDataStore primaryDataStore = (PrimaryDataStore)destPool;
17151720

engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,14 @@
7171
import com.cloud.storage.StorageManager;
7272
import com.cloud.storage.StoragePool;
7373
import com.cloud.storage.StoragePoolHostVO;
74+
import com.cloud.storage.VMTemplateVO;
75+
import com.cloud.storage.VMTemplateZoneVO;
7476
import com.cloud.storage.Volume;
7577
import com.cloud.storage.VolumeVO;
7678
import com.cloud.storage.dao.DiskOfferingDao;
7779
import com.cloud.storage.dao.StoragePoolHostDao;
80+
import com.cloud.storage.dao.VMTemplateDao;
81+
import com.cloud.storage.dao.VMTemplateZoneDao;
7882
import com.cloud.storage.dao.VolumeDao;
7983
import com.cloud.utils.exception.CloudRuntimeException;
8084
import com.cloud.vm.VirtualMachine.State;
@@ -134,6 +138,10 @@ public class VirtualMachineManagerImplTest {
134138

135139
@Mock
136140
private DiskOfferingDao diskOfferingDaoMock;
141+
@Mock
142+
VMTemplateDao templateDao;
143+
@Mock
144+
VMTemplateZoneDao templateZoneDao;
137145

138146
@Mock
139147
private HostDao hostDaoMock;
@@ -776,4 +784,59 @@ private void prepareAndRunCheckIfNewOfferingStorageScopeMatchesStoragePool(boole
776784
Mockito.doReturn(isOfferingUsingLocal).when(diskOfferingMock).isUseLocalStorage();
777785
virtualMachineManagerImpl.checkIfNewOfferingStorageScopeMatchesStoragePool(vmInstanceMock, diskOfferingMock);
778786
}
787+
788+
@Test
789+
public void checkIfTemplateNeededForCreatingVmVolumesExistingRootVolumes() {
790+
long vmId = 1L;
791+
VMInstanceVO vm = Mockito.mock(VMInstanceVO.class);
792+
Mockito.when(vm.getId()).thenReturn(vmId);
793+
Mockito.when(volumeDaoMock.findReadyRootVolumesByInstance(vmId)).thenReturn(List.of(Mockito.mock(VolumeVO.class)));
794+
virtualMachineManagerImpl.checkIfTemplateNeededForCreatingVmVolumes(vm);
795+
}
796+
797+
@Test(expected = CloudRuntimeException.class)
798+
public void checkIfTemplateNeededForCreatingVmVolumesMissingTemplate() {
799+
long vmId = 1L;
800+
long templateId = 1L;
801+
VMInstanceVO vm = Mockito.mock(VMInstanceVO.class);
802+
Mockito.when(vm.getId()).thenReturn(vmId);
803+
Mockito.when(vm.getTemplateId()).thenReturn(templateId);
804+
Mockito.when(volumeDaoMock.findReadyRootVolumesByInstance(vmId)).thenReturn(null);
805+
Mockito.when(templateDao.findById(templateId)).thenReturn(null);
806+
virtualMachineManagerImpl.checkIfTemplateNeededForCreatingVmVolumes(vm);
807+
}
808+
809+
@Test(expected = CloudRuntimeException.class)
810+
public void checkIfTemplateNeededForCreatingVmVolumesMissingZoneTemplate() {
811+
long vmId = 1L;
812+
long templateId = 1L;
813+
long dcId = 1L;
814+
VMInstanceVO vm = Mockito.mock(VMInstanceVO.class);
815+
Mockito.when(vm.getId()).thenReturn(vmId);
816+
Mockito.when(vm.getTemplateId()).thenReturn(templateId);
817+
Mockito.when(vm.getDataCenterId()).thenReturn(dcId);
818+
Mockito.when(volumeDaoMock.findReadyRootVolumesByInstance(vmId)).thenReturn(null);
819+
VMTemplateVO template = Mockito.mock(VMTemplateVO.class);
820+
Mockito.when(vm.getId()).thenReturn(templateId);
821+
Mockito.when(templateDao.findById(templateId)).thenReturn(template);
822+
Mockito.when(templateZoneDao.findByZoneTemplate(dcId, templateId)).thenReturn(null);
823+
virtualMachineManagerImpl.checkIfTemplateNeededForCreatingVmVolumes(vm);
824+
}
825+
826+
@Test
827+
public void checkIfTemplateNeededForCreatingVmVolumesTemplateAvailable() {
828+
long vmId = 1L;
829+
long templateId = 1L;
830+
long dcId = 1L;
831+
VMInstanceVO vm = Mockito.mock(VMInstanceVO.class);
832+
Mockito.when(vm.getId()).thenReturn(vmId);
833+
Mockito.when(vm.getTemplateId()).thenReturn(templateId);
834+
Mockito.when(vm.getDataCenterId()).thenReturn(dcId);
835+
Mockito.when(volumeDaoMock.findReadyRootVolumesByInstance(vmId)).thenReturn(new ArrayList<>());
836+
VMTemplateVO template = Mockito.mock(VMTemplateVO.class);
837+
Mockito.when(template.getId()).thenReturn(templateId);
838+
Mockito.when(templateDao.findById(templateId)).thenReturn(template);
839+
Mockito.when(templateZoneDao.findByZoneTemplate(dcId, templateId)).thenReturn(Mockito.mock(VMTemplateZoneVO.class));
840+
virtualMachineManagerImpl.checkIfTemplateNeededForCreatingVmVolumes(vm);
841+
}
779842
}

0 commit comments

Comments
 (0)