Skip to content

Commit 25b63f5

Browse files
priyankpariharyadvr
priyankparihar
authored and
yadvr
committed
CLOUDSTACK-9607: Preventing template deletion when template is in use (#1773)
Consider this scenario: 1. User launches a VM from Template and keep it running 2. Admin logins and deleted that template [CloudPlatform does not check existing / running VM etc. while the deletion is done] 3. User resets the VM 4. CloudPlatform fails to star the VM as it cannot find the corresponding template. It throws error as java.lang.RuntimeException: Job failed due to exception Resource [Host:11] is unreachable: Host 11: Unable to start instance due to can't find ready template: 209 for data center 1 at com.cloud.vm.VmWorkJobDispatcher.runJob(VmWorkJobDispatcher.java:113) at org.apache.cloudstack.framework.jobs.impl.AsyncJobManagerImpl$5.runInContext(AsyncJobManagerImpl.java:495) Client is requesting better handing of this scenario. We need to check existing / running VM's when the template is deleted and warn admin about the possible issue that may occur. REPRO STEPS ================== 1. Launches a VM from Template and keep it running 2. Now delete that template 3. Reset the VM 4. CloudPlatform fails to star the VM as it cannot find the corresponding template. EXPECTED BEHAVIOR ================== Cloud platform should throw some warning message while the template is deleted if that template is being used by existing / running VM's ACTUAL BEHAVIOR ================== Cloud platform does not throw as waring etc.
1 parent 9988c26 commit 25b63f5

6 files changed

Lines changed: 117 additions & 8 deletions

File tree

‎api/src/org/apache/cloudstack/api/command/user/template/DeleteTemplateCmd.java‎

100644100755
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,9 @@ public class DeleteTemplateCmd extends BaseAsyncCmd {
5252
@Parameter(name = ApiConstants.ZONE_ID, type = CommandType.UUID, entityType = ZoneResponse.class, description = "the ID of zone of the template")
5353
private Long zoneId;
5454

55+
@Parameter(name = ApiConstants.FORCED, type = CommandType.BOOLEAN, required = false, description = "Force delete a template.", since = "4.9+")
56+
private Boolean forced;
57+
5558
/////////////////////////////////////////////////////
5659
/////////////////// Accessors ///////////////////////
5760
/////////////////////////////////////////////////////
@@ -64,6 +67,9 @@ public Long getZoneId() {
6467
return zoneId;
6568
}
6669

70+
public boolean isForced() {
71+
return (forced != null) ? forced : true;
72+
}
6773
/////////////////////////////////////////////////////
6874
/////////////// API Implementation///////////////////
6975
/////////////////////////////////////////////////////

‎engine/schema/src/com/cloud/vm/dao/VMInstanceDao.java‎

100644100755
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,14 @@ public interface VMInstanceDao extends GenericDao<VMInstanceVO, Long>, StateDao<
5353
*/
5454
List<VMInstanceVO> listByPodId(long podId);
5555

56+
/**
57+
* Lists non-expunged VMs by templateId
58+
* @param templateId
59+
* @return list of VMInstanceVO deployed from the specified template, that are not expunged
60+
*/
61+
public List<VMInstanceVO> listNonExpungedByTemplate(long templateId);
62+
63+
5664
/**
5765
* Lists non-expunged VMs by zone ID and templateId
5866
* @param zoneId

‎engine/schema/src/com/cloud/vm/dao/VMInstanceDaoImpl.java‎

100644100755
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ public class VMInstanceDaoImpl extends GenericDaoBase<VMInstanceVO, Long> implem
7272
protected SearchBuilder<VMInstanceVO> IdStatesSearch;
7373
protected SearchBuilder<VMInstanceVO> AllFieldsSearch;
7474
protected SearchBuilder<VMInstanceVO> ZoneTemplateNonExpungedSearch;
75+
protected SearchBuilder<VMInstanceVO> TemplateNonExpungedSearch;
7576
protected SearchBuilder<VMInstanceVO> NameLikeSearch;
7677
protected SearchBuilder<VMInstanceVO> StateChangeSearch;
7778
protected SearchBuilder<VMInstanceVO> TransitionSearch;
@@ -165,6 +166,12 @@ protected void init() {
165166
ZoneTemplateNonExpungedSearch.and("state", ZoneTemplateNonExpungedSearch.entity().getState(), Op.NEQ);
166167
ZoneTemplateNonExpungedSearch.done();
167168

169+
170+
TemplateNonExpungedSearch = createSearchBuilder();
171+
TemplateNonExpungedSearch.and("template", TemplateNonExpungedSearch.entity().getTemplateId(), Op.EQ);
172+
TemplateNonExpungedSearch.and("state", TemplateNonExpungedSearch.entity().getState(), Op.NEQ);
173+
TemplateNonExpungedSearch.done();
174+
168175
NameLikeSearch = createSearchBuilder();
169176
NameLikeSearch.and("name", NameLikeSearch.entity().getHostName(), Op.LIKE);
170177
NameLikeSearch.done();
@@ -334,6 +341,15 @@ public List<VMInstanceVO> listByZoneIdAndType(long zoneId, VirtualMachine.Type t
334341
return listBy(sc);
335342
}
336343

344+
@Override
345+
public List<VMInstanceVO> listNonExpungedByTemplate(long templateId) {
346+
SearchCriteria<VMInstanceVO> sc = TemplateNonExpungedSearch.create();
347+
348+
sc.setParameters("template", templateId);
349+
sc.setParameters("state", State.Expunging);
350+
return listBy(sc);
351+
}
352+
337353
@Override
338354
public List<VMInstanceVO> listNonExpungedByZoneAndTemplate(long zoneId, long templateId) {
339355
SearchCriteria<VMInstanceVO> sc = ZoneTemplateNonExpungedSearch.create();

‎server/src/com/cloud/template/TemplateManagerImpl.java‎

100644100755
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import com.cloud.utils.DateUtil;
3939
import com.cloud.utils.Pair;
4040
import com.cloud.utils.EnumUtils;
41+
import com.google.common.base.Joiner;
4142
import com.google.gson.Gson;
4243
import com.google.gson.GsonBuilder;
4344

@@ -1235,6 +1236,19 @@ public boolean deleteTemplate(DeleteTemplateCmd cmd) {
12351236
throw new InvalidParameterValueException("unable to find template with id " + templateId);
12361237
}
12371238

1239+
List<VMInstanceVO> vmInstanceVOList;
1240+
if(cmd.getZoneId() != null) {
1241+
vmInstanceVOList = _vmInstanceDao.listNonExpungedByZoneAndTemplate(cmd.getZoneId(), templateId);
1242+
}
1243+
else {
1244+
vmInstanceVOList = _vmInstanceDao.listNonExpungedByTemplate(templateId);
1245+
}
1246+
if(!cmd.isForced() && CollectionUtils.isNotEmpty(vmInstanceVOList)) {
1247+
final String message = String.format("Unable to delete template with id: %1$s because VM instances: [%2$s] are using it.", templateId, Joiner.on(",").join(vmInstanceVOList));
1248+
s_logger.warn(message);
1249+
throw new InvalidParameterValueException(message);
1250+
}
1251+
12381252
_accountMgr.checkAccess(caller, AccessType.OperateEntry, true, template);
12391253

12401254
if (template.getFormat() == ImageFormat.ISO) {

‎server/test/com/cloud/template/TemplateManagerImplTest.java‎

100644100755
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030
import com.cloud.host.Status;
3131
import com.cloud.host.dao.HostDao;
3232
import com.cloud.hypervisor.Hypervisor;
33+
import com.cloud.storage.Storage;
34+
import com.cloud.storage.TemplateProfile;
3335
import com.cloud.projects.ProjectManager;
3436
import com.cloud.storage.GuestOSVO;
3537
import com.cloud.storage.Snapshot;
@@ -59,9 +61,11 @@
5961
import com.cloud.utils.component.ComponentContext;
6062
import com.cloud.utils.concurrency.NamedThreadFactory;
6163
import com.cloud.utils.exception.CloudRuntimeException;
64+
import com.cloud.vm.VMInstanceVO;
6265
import com.cloud.vm.dao.UserVmDao;
6366
import com.cloud.vm.dao.VMInstanceDao;
6467
import org.apache.cloudstack.api.command.user.template.CreateTemplateCmd;
68+
import org.apache.cloudstack.api.command.user.template.DeleteTemplateCmd;
6569
import org.apache.cloudstack.context.CallContext;
6670
import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService;
6771
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
@@ -169,6 +173,13 @@ public class TemplateManagerImplTest {
169173
@Inject
170174
StorageStrategyFactory storageStrategyFactory;
171175

176+
@Inject
177+
VMInstanceDao _vmInstanceDao;
178+
179+
@Inject
180+
private VMTemplateDao _tmpltDao;
181+
182+
172183
public class CustomThreadPoolExecutor extends ThreadPoolExecutor {
173184
AtomicInteger ai = new AtomicInteger(0);
174185
public CustomThreadPoolExecutor(int corePoolSize, int maximumPoolSize, long keepAliveTime, TimeUnit unit,
@@ -216,6 +227,50 @@ public void testVerifyTemplateIdOfNonSystemTemplate() {
216227
templateManager.verifyTemplateId(1L);
217228
}
218229

230+
@Test
231+
public void testForceDeleteTemplate() {
232+
//In this Unit test all the conditions related to "force template delete flag" are tested.
233+
234+
DeleteTemplateCmd cmd = mock(DeleteTemplateCmd.class);
235+
VMTemplateVO template = mock(VMTemplateVO.class);
236+
TemplateAdapter templateAdapter = mock(TemplateAdapter.class);
237+
TemplateProfile templateProfile = mock(TemplateProfile.class);
238+
239+
240+
List<VMInstanceVO> vmInstanceVOList = new ArrayList<VMInstanceVO>();
241+
List<TemplateAdapter> adapters = new ArrayList<TemplateAdapter>();
242+
adapters.add(templateAdapter);
243+
when(cmd.getId()).thenReturn(0L);
244+
when(_tmpltDao.findById(cmd.getId())).thenReturn(template);
245+
when(cmd.getZoneId()).thenReturn(null);
246+
247+
when(template.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.None);
248+
when(template.getFormat()).thenReturn(Storage.ImageFormat.VMDK);
249+
templateManager.setTemplateAdapters(adapters);
250+
when(templateAdapter.getName()).thenReturn(TemplateAdapter.TemplateAdapterType.Hypervisor.getName().toString());
251+
when(templateAdapter.prepareDelete(any(DeleteTemplateCmd.class))).thenReturn(templateProfile);
252+
when(templateAdapter.delete(templateProfile)).thenReturn(true);
253+
254+
//case 1: When Force delete flag is 'true' but VM instance VO list is empty.
255+
when(cmd.isForced()).thenReturn(true);
256+
templateManager.deleteTemplate(cmd);
257+
258+
//case 2.1: When Force delete flag is 'false' and VM instance VO list is empty.
259+
when(cmd.isForced()).thenReturn(false);
260+
templateManager.deleteTemplate(cmd);
261+
262+
//case 2.2: When Force delete flag is 'false' and VM instance VO list is non empty.
263+
when(cmd.isForced()).thenReturn(false);
264+
VMInstanceVO vmInstanceVO = mock(VMInstanceVO.class);
265+
when(vmInstanceVO.getInstanceName()).thenReturn("mydDummyVM");
266+
vmInstanceVOList.add(vmInstanceVO);
267+
when(_vmInstanceDao.listNonExpungedByTemplate(anyLong())).thenReturn(vmInstanceVOList);
268+
try {
269+
templateManager.deleteTemplate(cmd);
270+
} catch(Exception e) {
271+
assertTrue("Invalid Parameter Value Exception is expected", (e instanceof InvalidParameterValueException));
272+
}
273+
}
219274
@Test
220275
public void testPrepareTemplateIsSeeded() {
221276
VMTemplateVO mockTemplate = mock(VMTemplateVO.class);

‎ui/scripts/templates.js‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1538,14 +1538,24 @@
15381538
actions: {
15391539
remove: {
15401540
label: 'label.action.delete.template',
1541+
createForm: {
1542+
title: 'label.action.delete.template',
1543+
desc: function(args) {
1544+
if(args.context.templates[0].crossZones == true) {
1545+
return 'message.action.delete.template.for.all.zones';
1546+
} else {
1547+
return 'message.action.delete.template';
1548+
}
1549+
},
1550+
fields: {
1551+
forced: {
1552+
label: 'force.delete',
1553+
isBoolean: true,
1554+
isChecked: false
1555+
}
1556+
}
1557+
},
15411558
messages: {
1542-
confirm: function(args) {
1543-
if(args.context.templates[0].crossZones == true) {
1544-
return 'message.action.delete.template.for.all.zones';
1545-
} else {
1546-
return 'message.action.delete.template';
1547-
}
1548-
},
15491559
notification: function(args) {
15501560
return 'label.action.delete.template';
15511561
}
@@ -1556,7 +1566,7 @@
15561566
queryParams += "&zoneid=" + args.context.zones[0].zoneid;
15571567
}
15581568
$.ajax({
1559-
url: createURL(queryParams),
1569+
url: createURL(queryParams + "&forced=" + (args.data.forced == "on")),
15601570
dataType: "json",
15611571
async: true,
15621572
success: function(json) {

0 commit comments

Comments
 (0)