cloudstack-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (CLOUDSTACK-9607) Preventing template deletion when template is in use.
Date Thu, 04 Jan 2018 05:30:00 GMT

    [ https://issues.apache.org/jira/browse/CLOUDSTACK-9607?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16310743#comment-16310743
] 

ASF GitHub Bot commented on CLOUDSTACK-9607:
--------------------------------------------

rhtyd closed pull request #1773: CLOUDSTACK-9607: Preventing template deletion when template
is in use.
URL: https://github.com/apache/cloudstack/pull/1773
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

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 98d53be836e..95b3eeee059
--- 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 @@
     @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 Long getZoneId() {
         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 3c5024b833e..69efea42df9
--- a/engine/schema/src/com/cloud/vm/dao/VMInstanceDao.java
+++ b/engine/schema/src/com/cloud/vm/dao/VMInstanceDao.java
@@ -53,6 +53,14 @@
      */
     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
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 7065350a57e..6e97d1275a6
--- a/engine/schema/src/com/cloud/vm/dao/VMInstanceDaoImpl.java
+++ b/engine/schema/src/com/cloud/vm/dao/VMInstanceDaoImpl.java
@@ -72,6 +72,7 @@
     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 @@ protected void init() {
         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();
@@ -334,6 +341,15 @@ protected void init() {
         return listBy(sc);
     }
 
+    @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 f6494c3b77e..41f11af68d0
--- a/server/src/com/cloud/template/TemplateManagerImpl.java
+++ b/server/src/com/cloud/template/TemplateManagerImpl.java
@@ -38,6 +38,7 @@
 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 boolean deleteTemplate(DeleteTemplateCmd cmd) {
             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 3039b8bda55..bd21643b4a1
--- a/server/test/com/cloud/template/TemplateManagerImplTest.java
+++ b/server/test/com/cloud/template/TemplateManagerImplTest.java
@@ -30,6 +30,8 @@
 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.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 @@
     @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,
@@ -216,6 +227,50 @@ public void testVerifyTemplateIdOfNonSystemTemplate() {
         templateManager.verifyTemplateId(1L);
     }
 
+    @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);
diff --git a/ui/scripts/templates.js b/ui/scripts/templates.js
index 1ab1b9b09dd..8a7ec6e3388 100755
--- a/ui/scripts/templates.js
+++ b/ui/scripts/templates.js
@@ -1526,14 +1526,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';
                                                      }
@@ -1544,7 +1554,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) {


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


> Preventing template deletion when template is in use.
> -----------------------------------------------------
>
>                 Key: CLOUDSTACK-9607
>                 URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9607
>             Project: CloudStack
>          Issue Type: Bug
>      Security Level: Public(Anyone can view this level - this is the default.) 
>            Reporter: Priyank Parihar
>            Assignee: Priyank Parihar
>
> 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.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Mime
View raw message