You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ro...@apache.org on 2018/01/04 05:29:42 UTC
[cloudstack] branch master updated: CLOUDSTACK-9607: Preventing
template deletion when template is in use (#1773)
This is an automated email from the ASF dual-hosted git repository.
rohit pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/master by this push:
new 25b63f5 CLOUDSTACK-9607: Preventing template deletion when template is in use (#1773)
25b63f5 is described below
commit 25b63f5e737aba00067e39ee0003f4b52713d6d6
Author: Mowgli <pr...@users.noreply.github.com>
AuthorDate: Thu Jan 4 10:59:39 2018 +0530
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.
---
.../command/user/template/DeleteTemplateCmd.java | 6 +++
.../schema/src/com/cloud/vm/dao/VMInstanceDao.java | 8 ++++
.../src/com/cloud/vm/dao/VMInstanceDaoImpl.java | 16 +++++++
.../com/cloud/template/TemplateManagerImpl.java | 14 ++++++
.../cloud/template/TemplateManagerImplTest.java | 55 ++++++++++++++++++++++
ui/scripts/templates.js | 26 ++++++----
6 files changed, 117 insertions(+), 8 deletions(-)
diff --git a/api/src/org/apache/cloudstack/api/command/user/template/DeleteTemplateCmd.java b/api/src/org/apache/cloudstack/api/command/user/template/DeleteTemplateCmd.java
old mode 100644
new mode 100755
index 98d53be..95b3eee
--- a/api/src/org/apache/cloudstack/api/command/user/template/DeleteTemplateCmd.java
+++ b/api/src/org/apache/cloudstack/api/command/user/template/DeleteTemplateCmd.java
@@ -52,6 +52,9 @@ public class DeleteTemplateCmd extends BaseAsyncCmd {
@Parameter(name = ApiConstants.ZONE_ID, type = CommandType.UUID, entityType = ZoneResponse.class, description = "the ID of zone of the template")
private Long zoneId;
+ @Parameter(name = ApiConstants.FORCED, type = CommandType.BOOLEAN, required = false, description = "Force delete a template.", since = "4.9+")
+ private Boolean forced;
+
/////////////////////////////////////////////////////
/////////////////// Accessors ///////////////////////
/////////////////////////////////////////////////////
@@ -64,6 +67,9 @@ public class DeleteTemplateCmd extends BaseAsyncCmd {
return zoneId;
}
+ public boolean isForced() {
+ return (forced != null) ? forced : true;
+ }
/////////////////////////////////////////////////////
/////////////// API Implementation///////////////////
/////////////////////////////////////////////////////
diff --git a/engine/schema/src/com/cloud/vm/dao/VMInstanceDao.java b/engine/schema/src/com/cloud/vm/dao/VMInstanceDao.java
old mode 100644
new mode 100755
index 3c5024b..69efea4
--- a/engine/schema/src/com/cloud/vm/dao/VMInstanceDao.java
+++ b/engine/schema/src/com/cloud/vm/dao/VMInstanceDao.java
@@ -54,6 +54,14 @@ public interface VMInstanceDao extends GenericDao<VMInstanceVO, Long>, StateDao<
List<VMInstanceVO> listByPodId(long podId);
/**
+ * Lists non-expunged VMs by templateId
+ * @param templateId
+ * @return list of VMInstanceVO deployed from the specified template, that are not expunged
+ */
+ public List<VMInstanceVO> listNonExpungedByTemplate(long templateId);
+
+
+ /**
* Lists non-expunged VMs by zone ID and templateId
* @param zoneId
* @return list of VMInstanceVO in the specified zone, deployed from the specified template, that are not expunged
diff --git a/engine/schema/src/com/cloud/vm/dao/VMInstanceDaoImpl.java b/engine/schema/src/com/cloud/vm/dao/VMInstanceDaoImpl.java
old mode 100644
new mode 100755
index 7065350..6e97d12
--- a/engine/schema/src/com/cloud/vm/dao/VMInstanceDaoImpl.java
+++ b/engine/schema/src/com/cloud/vm/dao/VMInstanceDaoImpl.java
@@ -72,6 +72,7 @@ public class VMInstanceDaoImpl extends GenericDaoBase<VMInstanceVO, Long> implem
protected SearchBuilder<VMInstanceVO> IdStatesSearch;
protected SearchBuilder<VMInstanceVO> AllFieldsSearch;
protected SearchBuilder<VMInstanceVO> ZoneTemplateNonExpungedSearch;
+ protected SearchBuilder<VMInstanceVO> TemplateNonExpungedSearch;
protected SearchBuilder<VMInstanceVO> NameLikeSearch;
protected SearchBuilder<VMInstanceVO> StateChangeSearch;
protected SearchBuilder<VMInstanceVO> TransitionSearch;
@@ -165,6 +166,12 @@ public class VMInstanceDaoImpl extends GenericDaoBase<VMInstanceVO, Long> implem
ZoneTemplateNonExpungedSearch.and("state", ZoneTemplateNonExpungedSearch.entity().getState(), Op.NEQ);
ZoneTemplateNonExpungedSearch.done();
+
+ TemplateNonExpungedSearch = createSearchBuilder();
+ TemplateNonExpungedSearch.and("template", TemplateNonExpungedSearch.entity().getTemplateId(), Op.EQ);
+ TemplateNonExpungedSearch.and("state", TemplateNonExpungedSearch.entity().getState(), Op.NEQ);
+ TemplateNonExpungedSearch.done();
+
NameLikeSearch = createSearchBuilder();
NameLikeSearch.and("name", NameLikeSearch.entity().getHostName(), Op.LIKE);
NameLikeSearch.done();
@@ -335,6 +342,15 @@ public class VMInstanceDaoImpl extends GenericDaoBase<VMInstanceVO, Long> implem
}
@Override
+ public List<VMInstanceVO> listNonExpungedByTemplate(long templateId) {
+ SearchCriteria<VMInstanceVO> sc = TemplateNonExpungedSearch.create();
+
+ sc.setParameters("template", templateId);
+ sc.setParameters("state", State.Expunging);
+ return listBy(sc);
+ }
+
+ @Override
public List<VMInstanceVO> listNonExpungedByZoneAndTemplate(long zoneId, long templateId) {
SearchCriteria<VMInstanceVO> sc = ZoneTemplateNonExpungedSearch.create();
diff --git a/server/src/com/cloud/template/TemplateManagerImpl.java b/server/src/com/cloud/template/TemplateManagerImpl.java
old mode 100644
new mode 100755
index f6494c3..41f11af
--- a/server/src/com/cloud/template/TemplateManagerImpl.java
+++ b/server/src/com/cloud/template/TemplateManagerImpl.java
@@ -38,6 +38,7 @@ import com.cloud.utils.EncryptionUtil;
import com.cloud.utils.DateUtil;
import com.cloud.utils.Pair;
import com.cloud.utils.EnumUtils;
+import com.google.common.base.Joiner;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
@@ -1235,6 +1236,19 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager,
throw new InvalidParameterValueException("unable to find template with id " + templateId);
}
+ List<VMInstanceVO> vmInstanceVOList;
+ if(cmd.getZoneId() != null) {
+ vmInstanceVOList = _vmInstanceDao.listNonExpungedByZoneAndTemplate(cmd.getZoneId(), templateId);
+ }
+ else {
+ vmInstanceVOList = _vmInstanceDao.listNonExpungedByTemplate(templateId);
+ }
+ if(!cmd.isForced() && CollectionUtils.isNotEmpty(vmInstanceVOList)) {
+ 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));
+ s_logger.warn(message);
+ throw new InvalidParameterValueException(message);
+ }
+
_accountMgr.checkAccess(caller, AccessType.OperateEntry, true, template);
if (template.getFormat() == ImageFormat.ISO) {
diff --git a/server/test/com/cloud/template/TemplateManagerImplTest.java b/server/test/com/cloud/template/TemplateManagerImplTest.java
old mode 100644
new mode 100755
index 3039b8b..bd21643
--- a/server/test/com/cloud/template/TemplateManagerImplTest.java
+++ b/server/test/com/cloud/template/TemplateManagerImplTest.java
@@ -30,6 +30,8 @@ import com.cloud.exception.ResourceAllocationException;
import com.cloud.host.Status;
import com.cloud.host.dao.HostDao;
import com.cloud.hypervisor.Hypervisor;
+import com.cloud.storage.Storage;
+import com.cloud.storage.TemplateProfile;
import com.cloud.projects.ProjectManager;
import com.cloud.storage.GuestOSVO;
import com.cloud.storage.Snapshot;
@@ -59,9 +61,11 @@ import com.cloud.user.dao.AccountDao;
import com.cloud.utils.component.ComponentContext;
import com.cloud.utils.concurrency.NamedThreadFactory;
import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.vm.VMInstanceVO;
import com.cloud.vm.dao.UserVmDao;
import com.cloud.vm.dao.VMInstanceDao;
import org.apache.cloudstack.api.command.user.template.CreateTemplateCmd;
+import org.apache.cloudstack.api.command.user.template.DeleteTemplateCmd;
import org.apache.cloudstack.context.CallContext;
import org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
@@ -169,6 +173,13 @@ public class TemplateManagerImplTest {
@Inject
StorageStrategyFactory storageStrategyFactory;
+ @Inject
+ VMInstanceDao _vmInstanceDao;
+
+ @Inject
+ private VMTemplateDao _tmpltDao;
+
+
public class CustomThreadPoolExecutor extends ThreadPoolExecutor {
AtomicInteger ai = new AtomicInteger(0);
public CustomThreadPoolExecutor(int corePoolSize, int maximumPoolSize, long keepAliveTime, TimeUnit unit,
@@ -217,6 +228,50 @@ public class TemplateManagerImplTest {
}
@Test
+ public void testForceDeleteTemplate() {
+ //In this Unit test all the conditions related to "force template delete flag" are tested.
+
+ DeleteTemplateCmd cmd = mock(DeleteTemplateCmd.class);
+ VMTemplateVO template = mock(VMTemplateVO.class);
+ TemplateAdapter templateAdapter = mock(TemplateAdapter.class);
+ TemplateProfile templateProfile = mock(TemplateProfile.class);
+
+
+ List<VMInstanceVO> vmInstanceVOList = new ArrayList<VMInstanceVO>();
+ List<TemplateAdapter> adapters = new ArrayList<TemplateAdapter>();
+ adapters.add(templateAdapter);
+ when(cmd.getId()).thenReturn(0L);
+ when(_tmpltDao.findById(cmd.getId())).thenReturn(template);
+ when(cmd.getZoneId()).thenReturn(null);
+
+ when(template.getHypervisorType()).thenReturn(Hypervisor.HypervisorType.None);
+ when(template.getFormat()).thenReturn(Storage.ImageFormat.VMDK);
+ templateManager.setTemplateAdapters(adapters);
+ when(templateAdapter.getName()).thenReturn(TemplateAdapter.TemplateAdapterType.Hypervisor.getName().toString());
+ when(templateAdapter.prepareDelete(any(DeleteTemplateCmd.class))).thenReturn(templateProfile);
+ when(templateAdapter.delete(templateProfile)).thenReturn(true);
+
+ //case 1: When Force delete flag is 'true' but VM instance VO list is empty.
+ when(cmd.isForced()).thenReturn(true);
+ templateManager.deleteTemplate(cmd);
+
+ //case 2.1: When Force delete flag is 'false' and VM instance VO list is empty.
+ when(cmd.isForced()).thenReturn(false);
+ templateManager.deleteTemplate(cmd);
+
+ //case 2.2: When Force delete flag is 'false' and VM instance VO list is non empty.
+ when(cmd.isForced()).thenReturn(false);
+ VMInstanceVO vmInstanceVO = mock(VMInstanceVO.class);
+ when(vmInstanceVO.getInstanceName()).thenReturn("mydDummyVM");
+ vmInstanceVOList.add(vmInstanceVO);
+ when(_vmInstanceDao.listNonExpungedByTemplate(anyLong())).thenReturn(vmInstanceVOList);
+ try {
+ templateManager.deleteTemplate(cmd);
+ } catch(Exception e) {
+ assertTrue("Invalid Parameter Value Exception is expected", (e instanceof InvalidParameterValueException));
+ }
+ }
+ @Test
public void testPrepareTemplateIsSeeded() {
VMTemplateVO mockTemplate = mock(VMTemplateVO.class);
when(mockTemplate.getId()).thenReturn(202l);
diff --git a/ui/scripts/templates.js b/ui/scripts/templates.js
index b2df73e..c40ff29 100755
--- a/ui/scripts/templates.js
+++ b/ui/scripts/templates.js
@@ -1538,14 +1538,24 @@
actions: {
remove: {
label: 'label.action.delete.template',
+ createForm: {
+ title: 'label.action.delete.template',
+ desc: function(args) {
+ if(args.context.templates[0].crossZones == true) {
+ return 'message.action.delete.template.for.all.zones';
+ } else {
+ return 'message.action.delete.template';
+ }
+ },
+ fields: {
+ forced: {
+ label: 'force.delete',
+ isBoolean: true,
+ isChecked: false
+ }
+ }
+ },
messages: {
- confirm: function(args) {
- if(args.context.templates[0].crossZones == true) {
- return 'message.action.delete.template.for.all.zones';
- } else {
- return 'message.action.delete.template';
- }
- },
notification: function(args) {
return 'label.action.delete.template';
}
@@ -1556,7 +1566,7 @@
queryParams += "&zoneid=" + args.context.zones[0].zoneid;
}
$.ajax({
- url: createURL(queryParams),
+ url: createURL(queryParams + "&forced=" + (args.data.forced == "on")),
dataType: "json",
async: true,
success: function(json) {
--
To stop receiving notification emails like this one, please contact
['"commits@cloudstack.apache.org" <co...@cloudstack.apache.org>'].