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 5847453  api: snapshot, snapshotpolicy tag support (#3228)
5847453 is described below

commit 58474530f69825f5108d249ac3f5e4a153d0c8d6
Author: Abhishek Kumar <abhishek.mr...@gmail.com>
AuthorDate: Thu Jun 27 09:21:09 2019 +0530

    api: snapshot, snapshotpolicy tag support (#3228)
    
    Problem: Currently tags cannot be applied to snapshot when it is being 
created but through separate “create tags” API calls. For snapshot policies 
tags cannot be set either at creation or through “create tags” API.
    
    Root Cause: The “create snapshots” API does not support adding tags during 
creation and it can only be done through “create tags” API. Snapshot policy as 
a resource does not support tags and no tags can be set for them through any 
API.
    
    Solution: Tag support for snapshot policy has been added. Snapshot policy 
with tags when executed will produce snapshots containing the same tags from 
snapshot policy.
    
    Following APIs have been updated:
    
    Both “create snapshotpolicy” and “create snapshot” now accepts “tags” as a 
new parameter. The expected format for “tags” parameter is similar to parameter 
“tags” in “create tags“ API.
    Deletion support for tags associated with snapshots policy has been added 
to “delete snapshotpolicies” API.
    Tags set for snapshot policies are added to the Response of “list 
snapshotpolicies“ API.
    UI support for setting tags to snapshots and snapshot policy is provided 
through the corresponding menus with a new section in each form to set tags.
    
    Signed-off-by: Abhishek Kumar <abhishek.mr...@gmail.com>
---
 .../main/java/com/cloud/server/ResourceTag.java    |   2 +-
 .../java/com/cloud/storage/VolumeApiService.java   |   3 +-
 .../command/user/snapshot/CreateSnapshotCmd.java   |  22 +++-
 .../user/snapshot/CreateSnapshotPolicyCmd.java     |  23 +++-
 .../api/response/SnapshotPolicyResponse.java       |  16 ++-
 .../cloudstack/api/response/SnapshotResponse.java  |  26 +++--
 .../api/command/test/CreateSnapshotCmdTest.java    |  30 ++++-
 .../user/snapshot/CreateSnapshotPolicyCmdTest.java |  46 ++++++++
 .../main/java/com/cloud/api/ApiResponseHelper.java |  10 +-
 .../com/cloud/storage/VolumeApiServiceImpl.java    |  16 ++-
 .../storage/snapshot/SnapshotManagerImpl.java      | 122 +++++++++++----------
 .../storage/snapshot/SnapshotSchedulerImpl.java    |  13 +++
 .../com/cloud/tags/TaggedResourceManagerImpl.java  |  45 ++++----
 .../cloud/storage/VolumeApiServiceImplTest.java    |  10 +-
 ui/css/cloudstack3.css                             |  25 ++++-
 ui/index.html                                      |  20 +++-
 ui/scripts/storage.js                              |  34 ++++--
 ui/scripts/ui-custom/recurringSnapshots.js         |   6 +-
 ui/scripts/ui/dialog.js                            |  24 ++--
 ui/scripts/ui/widgets/tagger.js                    |  65 +++++++++--
 20 files changed, 423 insertions(+), 135 deletions(-)

diff --git a/api/src/main/java/com/cloud/server/ResourceTag.java 
b/api/src/main/java/com/cloud/server/ResourceTag.java
index 0bd5d73..99eb860 100644
--- a/api/src/main/java/com/cloud/server/ResourceTag.java
+++ b/api/src/main/java/com/cloud/server/ResourceTag.java
@@ -58,7 +58,7 @@ public interface ResourceTag extends ControlledEntity, 
Identity, InternalIdentit
         AutoScaleVmGroup(false, true),
         LBStickinessPolicy(false, true),
         LBHealthCheckPolicy(false, true),
-        SnapshotPolicy(false, true),
+        SnapshotPolicy(true, true),
         GuestOs(false, true),
         NetworkOffering(false, true),
         VpcOffering(true, false);
diff --git a/api/src/main/java/com/cloud/storage/VolumeApiService.java 
b/api/src/main/java/com/cloud/storage/VolumeApiService.java
index 7b38a6b..aa6d8a6 100644
--- a/api/src/main/java/com/cloud/storage/VolumeApiService.java
+++ b/api/src/main/java/com/cloud/storage/VolumeApiService.java
@@ -19,6 +19,7 @@
 package com.cloud.storage;
 
 import java.net.MalformedURLException;
+import java.util.Map;
 
 import org.apache.cloudstack.api.command.user.volume.AttachVolumeCmd;
 import org.apache.cloudstack.api.command.user.volume.CreateVolumeCmd;
@@ -93,7 +94,7 @@ public interface VolumeApiService {
 
     Volume detachVolumeFromVM(DetachVolumeCmd cmd);
 
-    Snapshot takeSnapshot(Long volumeId, Long policyId, Long snapshotId, 
Account account, boolean quiescevm, Snapshot.LocationType locationType, boolean 
asyncBackup)
+    Snapshot takeSnapshot(Long volumeId, Long policyId, Long snapshotId, 
Account account, boolean quiescevm, Snapshot.LocationType locationType, boolean 
asyncBackup, Map<String, String> tags)
             throws ResourceAllocationException;
 
     Snapshot allocSnapshot(Long volumeId, Long policyId, String snapshotName, 
Snapshot.LocationType locationType) throws ResourceAllocationException;
diff --git 
a/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotCmd.java
 
b/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotCmd.java
index b77b985..72437c8 100644
--- 
a/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotCmd.java
+++ 
b/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotCmd.java
@@ -16,6 +16,10 @@
 // under the License.
 package org.apache.cloudstack.api.command.user.snapshot;
 
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+
 import org.apache.cloudstack.api.APICommand;
 import org.apache.cloudstack.api.ApiCommandJobType;
 import org.apache.cloudstack.api.ApiConstants;
@@ -28,6 +32,7 @@ import org.apache.cloudstack.api.response.DomainResponse;
 import org.apache.cloudstack.api.response.SnapshotPolicyResponse;
 import org.apache.cloudstack.api.response.SnapshotResponse;
 import org.apache.cloudstack.api.response.VolumeResponse;
+import org.apache.commons.collections.MapUtils;
 import org.apache.log4j.Logger;
 
 import com.cloud.event.EventTypes;
@@ -83,6 +88,9 @@ public class CreateSnapshotCmd extends BaseAsyncCreateCmd {
     @Parameter(name = ApiConstants.ASYNC_BACKUP, type = CommandType.BOOLEAN, 
required = false, description = "asynchronous backup if true")
     private Boolean asyncBackup;
 
+    @Parameter(name = ApiConstants.TAGS, type = CommandType.MAP, description = 
"Map of tags (key/value pairs)")
+    private Map tags;
+
     private String syncObjectType = BaseAsyncCmd.snapshotHostSyncObject;
 
     // ///////////////////////////////////////////////////
@@ -121,6 +129,18 @@ public class CreateSnapshotCmd extends BaseAsyncCreateCmd {
         }
     }
 
+    public Map<String, String> getTags() {
+        Map<String, String> tagsMap = new HashMap<>();
+        if (MapUtils.isNotEmpty(tags)) {
+            for (Map<String, String> services : (Collection<Map<String, 
String>>)tags.values()) {
+                String key = services.get("key");
+                String value = services.get("value");
+                tagsMap.put(key, value);
+            }
+        }
+        return tagsMap;
+    }
+
     private Long getHostId() {
         Volume volume = _entityMgr.findById(Volume.class, getVolumeId());
         if (volume == null) {
@@ -196,7 +216,7 @@ public class CreateSnapshotCmd extends BaseAsyncCreateCmd {
         Snapshot snapshot;
         try {
             snapshot =
-                _volumeService.takeSnapshot(getVolumeId(), getPolicyId(), 
getEntityId(), _accountService.getAccount(getEntityOwnerId()), getQuiescevm(), 
getLocationType(), getAsyncBackup());
+                _volumeService.takeSnapshot(getVolumeId(), getPolicyId(), 
getEntityId(), _accountService.getAccount(getEntityOwnerId()), getQuiescevm(), 
getLocationType(), getAsyncBackup(), getTags());
 
             if (snapshot != null) {
                 SnapshotResponse response = 
_responseGenerator.createSnapshotResponse(snapshot);
diff --git 
a/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotPolicyCmd.java
 
b/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotPolicyCmd.java
index 4e2e6bd..898bae5 100644
--- 
a/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotPolicyCmd.java
+++ 
b/api/src/main/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotPolicyCmd.java
@@ -16,9 +16,11 @@
 // under the License.
 package org.apache.cloudstack.api.command.user.snapshot;
 
-import org.apache.cloudstack.acl.RoleType;
-import org.apache.log4j.Logger;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
 
+import org.apache.cloudstack.acl.RoleType;
 import org.apache.cloudstack.api.APICommand;
 import org.apache.cloudstack.api.ApiConstants;
 import org.apache.cloudstack.api.ApiErrorCode;
@@ -27,6 +29,8 @@ import org.apache.cloudstack.api.Parameter;
 import org.apache.cloudstack.api.ServerApiException;
 import org.apache.cloudstack.api.response.SnapshotPolicyResponse;
 import org.apache.cloudstack.api.response.VolumeResponse;
+import org.apache.commons.collections.MapUtils;
+import org.apache.log4j.Logger;
 
 import com.cloud.exception.InvalidParameterValueException;
 import com.cloud.exception.PermissionDeniedException;
@@ -68,6 +72,9 @@ public class CreateSnapshotPolicyCmd extends BaseCmd {
     @Parameter(name = ApiConstants.FOR_DISPLAY, type = CommandType.BOOLEAN, 
description = "an optional field, whether to the display the policy to the end 
user or not", since = "4.4", authorized = {RoleType.Admin})
     private Boolean display;
 
+    @Parameter(name = ApiConstants.TAGS, type = CommandType.MAP, description = 
"Map of tags (key/value pairs)")
+    private Map tags;
+
     /////////////////////////////////////////////////////
     /////////////////// Accessors ///////////////////////
     /////////////////////////////////////////////////////
@@ -133,6 +140,18 @@ public class CreateSnapshotPolicyCmd extends BaseCmd {
         return volume.getAccountId();
     }
 
+    public Map<String, String> getTags() {
+        Map<String, String> tagsMap = new HashMap<>();
+        if (MapUtils.isNotEmpty(tags)) {
+            for (Map<String, String> services : (Collection<Map<String, 
String>>)tags.values()) {
+                String key = services.get("key");
+                String value = services.get("value");
+                tagsMap.put(key, value);
+            }
+        }
+        return tagsMap;
+    }
+
     @Override
     public void execute() {
         SnapshotPolicy result = _snapshotService.createPolicy(this, 
_accountService.getAccount(getEntityOwnerId()));
diff --git 
a/api/src/main/java/org/apache/cloudstack/api/response/SnapshotPolicyResponse.java
 
b/api/src/main/java/org/apache/cloudstack/api/response/SnapshotPolicyResponse.java
index 10710c6..d1e535e 100644
--- 
a/api/src/main/java/org/apache/cloudstack/api/response/SnapshotPolicyResponse.java
+++ 
b/api/src/main/java/org/apache/cloudstack/api/response/SnapshotPolicyResponse.java
@@ -16,18 +16,20 @@
 // under the License.
 package org.apache.cloudstack.api.response;
 
-import com.google.gson.annotations.SerializedName;
+import java.util.LinkedHashSet;
+import java.util.Set;
 
 import org.apache.cloudstack.acl.RoleType;
 import org.apache.cloudstack.api.ApiConstants;
-import org.apache.cloudstack.api.BaseResponse;
+import org.apache.cloudstack.api.BaseResponseWithTagInformation;
 import org.apache.cloudstack.api.EntityReference;
 
 import com.cloud.serializer.Param;
 import com.cloud.storage.snapshot.SnapshotPolicy;
+import com.google.gson.annotations.SerializedName;
 
 @EntityReference(value = SnapshotPolicy.class)
-public class SnapshotPolicyResponse extends BaseResponse {
+public class SnapshotPolicyResponse extends BaseResponseWithTagInformation {
     @SerializedName("id")
     @Param(description = "the ID of the snapshot policy")
     private String id;
@@ -56,6 +58,10 @@ public class SnapshotPolicyResponse extends BaseResponse {
     @Param(description = "is this policy for display to the regular user", 
since = "4.4", authorized = {RoleType.Admin})
     private Boolean forDisplay;
 
+    public SnapshotPolicyResponse() {
+        tags = new LinkedHashSet<ResourceTagResponse>();
+    }
+
     public String getId() {
         return id;
     }
@@ -111,4 +117,8 @@ public class SnapshotPolicyResponse extends BaseResponse {
     public void setForDisplay(Boolean forDisplay) {
         this.forDisplay = forDisplay;
     }
+
+    public void setTags(Set<ResourceTagResponse> tags) {
+        this.tags = tags;
+    }
 }
diff --git 
a/api/src/main/java/org/apache/cloudstack/api/response/SnapshotResponse.java 
b/api/src/main/java/org/apache/cloudstack/api/response/SnapshotResponse.java
index bb2ff7f..94bb4d1 100644
--- a/api/src/main/java/org/apache/cloudstack/api/response/SnapshotResponse.java
+++ b/api/src/main/java/org/apache/cloudstack/api/response/SnapshotResponse.java
@@ -16,18 +16,20 @@
 // under the License.
 package org.apache.cloudstack.api.response;
 
-import com.cloud.serializer.Param;
-import com.cloud.storage.Snapshot;
-import com.google.gson.annotations.SerializedName;
+import java.util.Date;
+import java.util.LinkedHashSet;
+import java.util.Set;
+
 import org.apache.cloudstack.api.ApiConstants;
-import org.apache.cloudstack.api.BaseResponse;
+import org.apache.cloudstack.api.BaseResponseWithTagInformation;
 import org.apache.cloudstack.api.EntityReference;
 
-import java.util.Date;
-import java.util.List;
+import com.cloud.serializer.Param;
+import com.cloud.storage.Snapshot;
+import com.google.gson.annotations.SerializedName;
 
 @EntityReference(value = Snapshot.class)
-public class SnapshotResponse extends BaseResponse implements 
ControlledEntityResponse {
+public class SnapshotResponse extends BaseResponseWithTagInformation 
implements ControlledEntityResponse {
     @SerializedName(ApiConstants.ID)
     @Param(description = "ID of the snapshot")
     private String id;
@@ -96,10 +98,6 @@ public class SnapshotResponse extends BaseResponse 
implements ControlledEntityRe
     @Param(description = "id of the availability zone")
     private String zoneId;
 
-    @SerializedName(ApiConstants.TAGS)
-    @Param(description = "the list of resource tags associated with snapshot", 
responseObject = ResourceTagResponse.class)
-    private List<ResourceTagResponse> tags;
-
     @SerializedName(ApiConstants.REVERTABLE)
     @Param(description = "indicates whether the underlying storage supports 
reverting the volume to this snapshot")
     private boolean revertable;
@@ -116,6 +114,10 @@ public class SnapshotResponse extends BaseResponse 
implements ControlledEntityRe
     @Param(description = "virtual size of backedup snapshot on image store")
     private long virtualSize;
 
+    public SnapshotResponse() {
+        tags = new LinkedHashSet<ResourceTagResponse>();
+    }
+
     @Override
     public String getObjectId() {
         return this.getId();
@@ -206,7 +208,7 @@ public class SnapshotResponse extends BaseResponse 
implements ControlledEntityRe
         this.zoneId = zoneId;
     }
 
-    public void setTags(List<ResourceTagResponse> tags) {
+    public void setTags(Set<ResourceTagResponse> tags) {
         this.tags = tags;
     }
 
diff --git 
a/api/src/test/java/org/apache/cloudstack/api/command/test/CreateSnapshotCmdTest.java
 
b/api/src/test/java/org/apache/cloudstack/api/command/test/CreateSnapshotCmdTest.java
index ceb63ab..4739082 100644
--- 
a/api/src/test/java/org/apache/cloudstack/api/command/test/CreateSnapshotCmdTest.java
+++ 
b/api/src/test/java/org/apache/cloudstack/api/command/test/CreateSnapshotCmdTest.java
@@ -19,9 +19,13 @@ package org.apache.cloudstack.api.command.test;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyBoolean;
 import static org.mockito.Matchers.anyLong;
+import static org.mockito.Matchers.anyObject;
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Matchers.isNull;
 
+import java.util.HashMap;
+import java.util.Map;
+
 import org.apache.cloudstack.api.ResponseGenerator;
 import org.apache.cloudstack.api.ServerApiException;
 import org.apache.cloudstack.api.command.user.snapshot.CreateSnapshotCmd;
@@ -32,6 +36,7 @@ import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 import org.mockito.Mockito;
+import org.springframework.test.util.ReflectionTestUtils;
 
 import com.cloud.storage.Snapshot;
 import com.cloud.storage.VolumeApiService;
@@ -87,9 +92,8 @@ public class CreateSnapshotCmdTest extends TestCase {
         VolumeApiService volumeApiService = 
Mockito.mock(VolumeApiService.class);
         Snapshot snapshot = Mockito.mock(Snapshot.class);
         try {
-
             Mockito.when(volumeApiService.takeSnapshot(anyLong(), anyLong(), 
anyLong(),
-                    any(Account.class), anyBoolean(), 
isNull(Snapshot.LocationType.class), anyBoolean())).thenReturn(snapshot);
+                    any(Account.class), anyBoolean(), 
isNull(Snapshot.LocationType.class), anyBoolean(), 
anyObject())).thenReturn(snapshot);
 
         } catch (Exception e) {
             Assert.fail("Received exception when success expected " + 
e.getMessage());
@@ -122,7 +126,7 @@ public class CreateSnapshotCmdTest extends TestCase {
 
         try {
                 Mockito.when(volumeApiService.takeSnapshot(anyLong(), 
anyLong(), anyLong(),
-                        any(Account.class), anyBoolean(), 
isNull(Snapshot.LocationType.class), anyBoolean())).thenReturn(null);
+                        any(Account.class), anyBoolean(), 
isNull(Snapshot.LocationType.class), anyBoolean(), 
anyObject())).thenReturn(null);
         } catch (Exception e) {
             Assert.fail("Received exception when success expected " + 
e.getMessage());
         }
@@ -136,4 +140,24 @@ public class CreateSnapshotCmdTest extends TestCase {
             Assert.assertEquals("Failed to create snapshot due to an internal 
error creating snapshot for volume 123", exception.getDescription());
         }
     }
+
+    @Test
+    public void testParsingTags() {
+        final CreateSnapshotCmd createSnapshotCmd = new CreateSnapshotCmd();
+        final Map<String, String> tag1 = new HashMap<>();
+        tag1.put("key", "key1");
+        tag1.put("value", "value1");
+        final Map<String, String> tag2 = new HashMap<>();
+        tag2.put("key", "key2");
+        tag2.put("value", "value2");
+        final Map<String, String> expectedTags = new HashMap<>();
+        expectedTags.put("key1", "value1");
+        expectedTags.put("key2", "value2");
+
+        final Map<String, Map<String, String>> tagsParams = new HashMap<>();
+        tagsParams.put("0", tag1);
+        tagsParams.put("1", tag2);
+        ReflectionTestUtils.setField(createSnapshotCmd, "tags", tagsParams);
+        Assert.assertEquals(createSnapshotCmd.getTags(), expectedTags);
+    }
 }
diff --git 
a/api/src/test/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotPolicyCmdTest.java
 
b/api/src/test/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotPolicyCmdTest.java
new file mode 100644
index 0000000..860c2c2
--- /dev/null
+++ 
b/api/src/test/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotPolicyCmdTest.java
@@ -0,0 +1,46 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.user.snapshot;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.springframework.test.util.ReflectionTestUtils;
+
+public class CreateSnapshotPolicyCmdTest {
+    @Test
+    public void testParsingTags() {
+        final CreateSnapshotPolicyCmd createSnapshotPolicyCmd = new 
CreateSnapshotPolicyCmd();
+        final Map<String, String> tag1 = new HashMap<>();
+        tag1.put("key", "key1");
+        tag1.put("value", "value1");
+        final Map<String, String> tag2 = new HashMap<>();
+        tag2.put("key", "key2");
+        tag2.put("value", "value2");
+        final Map<String, String> expectedTags = new HashMap<>();
+        expectedTags.put("key1", "value1");
+        expectedTags.put("key2", "value2");
+
+        final Map<String, Map<String, String>> tagsParams = new HashMap<>();
+        tagsParams.put("0", tag1);
+        tagsParams.put("1", tag2);
+        ReflectionTestUtils.setField(createSnapshotPolicyCmd, "tags", 
tagsParams);
+        Assert.assertEquals(createSnapshotPolicyCmd.getTags(), expectedTags);
+    }
+}
\ No newline at end of file
diff --git a/server/src/main/java/com/cloud/api/ApiResponseHelper.java 
b/server/src/main/java/com/cloud/api/ApiResponseHelper.java
index 9bea30a..4e9a2bc 100644
--- a/server/src/main/java/com/cloud/api/ApiResponseHelper.java
+++ b/server/src/main/java/com/cloud/api/ApiResponseHelper.java
@@ -552,7 +552,7 @@ public class ApiResponseHelper implements ResponseGenerator 
{
             ResourceTagResponse tagResponse = createResourceTagResponse(tag, 
true);
             CollectionUtils.addIgnoreNull(tagResponses, tagResponse);
         }
-        snapshotResponse.setTags(tagResponses);
+        snapshotResponse.setTags(new HashSet<>(tagResponses));
 
         snapshotResponse.setObjectName("snapshot");
         return snapshotResponse;
@@ -654,6 +654,14 @@ public class ApiResponseHelper implements 
ResponseGenerator {
         policyResponse.setForDisplay(policy.isDisplay());
         policyResponse.setObjectName("snapshotpolicy");
 
+        List<? extends ResourceTag> tags = 
_resourceTagDao.listBy(policy.getId(), ResourceObjectType.SnapshotPolicy);
+        List<ResourceTagResponse> tagResponses = new 
ArrayList<ResourceTagResponse>();
+        for (ResourceTag tag : tags) {
+            ResourceTagResponse tagResponse = createResourceTagResponse(tag, 
false);
+            CollectionUtils.addIgnoreNull(tagResponses, tagResponse);
+        }
+        policyResponse.setTags(new HashSet<>(tagResponses));
+
         return policyResponse;
     }
 
diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java 
b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
index 182379a..2022d5b 100644
--- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
+++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java
@@ -80,6 +80,7 @@ import 
org.apache.cloudstack.utils.identity.ManagementServerNode;
 import org.apache.cloudstack.utils.imagestore.ImageStoreUtil;
 import org.apache.cloudstack.utils.volume.VirtualMachineDiskInfo;
 import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.collections.MapUtils;
 import org.apache.log4j.Logger;
 import org.joda.time.DateTime;
 import org.joda.time.DateTimeZone;
@@ -114,6 +115,8 @@ import com.cloud.hypervisor.HypervisorCapabilitiesVO;
 import com.cloud.hypervisor.dao.HypervisorCapabilitiesDao;
 import com.cloud.org.Grouping;
 import com.cloud.serializer.GsonHelper;
+import com.cloud.server.ResourceTag;
+import com.cloud.server.TaggedResourceService;
 import com.cloud.service.dao.ServiceOfferingDetailsDao;
 import com.cloud.storage.Storage.ImageFormat;
 import com.cloud.storage.dao.DiskOfferingDao;
@@ -256,6 +259,8 @@ public class VolumeApiServiceImpl extends ManagerBase 
implements VolumeApiServic
     private StoragePoolTagsDao storagePoolTagsDao;
     @Inject
     private StorageUtil storageUtil;
+    @Inject
+    public TaggedResourceService taggedResourceService;
 
     protected Gson _gson;
 
@@ -2343,7 +2348,16 @@ public class VolumeApiServiceImpl extends ManagerBase 
implements VolumeApiServic
 
     @Override
     @ActionEvent(eventType = EventTypes.EVENT_SNAPSHOT_CREATE, 
eventDescription = "taking snapshot", async = true)
-    public Snapshot takeSnapshot(Long volumeId, Long policyId, Long 
snapshotId, Account account, boolean quiescevm, Snapshot.LocationType 
locationType, boolean asyncBackup)
+    public Snapshot takeSnapshot(Long volumeId, Long policyId, Long 
snapshotId, Account account, boolean quiescevm, Snapshot.LocationType 
locationType, boolean asyncBackup, Map<String, String> tags)
+            throws ResourceAllocationException {
+        final Snapshot snapshot = takeSnapshotInternal(volumeId, policyId, 
snapshotId, account, quiescevm, locationType, asyncBackup);
+        if (snapshot != null && MapUtils.isNotEmpty(tags)) {
+            
taggedResourceService.createTags(Collections.singletonList(snapshot.getUuid()), 
ResourceTag.ResourceObjectType.Snapshot, tags, null);
+        }
+        return snapshot;
+    }
+
+    private Snapshot takeSnapshotInternal(Long volumeId, Long policyId, Long 
snapshotId, Account account, boolean quiescevm, Snapshot.LocationType 
locationType, boolean asyncBackup)
             throws ResourceAllocationException {
         VolumeInfo volume = volFactory.getVolume(volumeId);
         if (volume == null) {
diff --git 
a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java 
b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java
index ee65486..5d98c50 100755
--- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java
+++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java
@@ -16,8 +16,53 @@
 // under the License.
 package com.cloud.storage.snapshot;
 
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Date;
+import java.util.List;
+import java.util.Map;
+import java.util.TimeZone;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+
+import javax.inject.Inject;
+import javax.naming.ConfigurationException;
+
+import org.apache.cloudstack.api.command.user.snapshot.CreateSnapshotPolicyCmd;
+import 
org.apache.cloudstack.api.command.user.snapshot.DeleteSnapshotPoliciesCmd;
+import org.apache.cloudstack.api.command.user.snapshot.ListSnapshotPoliciesCmd;
+import org.apache.cloudstack.api.command.user.snapshot.ListSnapshotsCmd;
+import org.apache.cloudstack.api.command.user.snapshot.UpdateSnapshotPolicyCmd;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
+import 
org.apache.cloudstack.engine.subsystem.api.storage.DataStoreCapabilities;
+import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
+import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint;
+import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector;
+import 
org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine;
+import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory;
+import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotService;
+import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy;
+import 
org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy.SnapshotOperation;
+import 
org.apache.cloudstack.engine.subsystem.api.storage.StorageStrategyFactory;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
+import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
+import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope;
+import org.apache.cloudstack.framework.config.ConfigKey;
+import org.apache.cloudstack.framework.config.Configurable;
+import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
+import org.apache.cloudstack.managed.context.ManagedContextRunnable;
+import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.commons.collections.MapUtils;
+import org.apache.log4j.Logger;
+import org.springframework.stereotype.Component;
+
 import com.cloud.agent.api.Answer;
-import com.cloud.utils.db.GlobalLock;
 import com.cloud.agent.api.Command;
 import com.cloud.agent.api.DeleteSnapshotsDirCommand;
 import com.cloud.alert.AlertManager;
@@ -42,6 +87,7 @@ import com.cloud.hypervisor.Hypervisor.HypervisorType;
 import com.cloud.projects.Project.ListProjectResourcesCriteria;
 import com.cloud.resource.ResourceManager;
 import com.cloud.server.ResourceTag.ResourceObjectType;
+import com.cloud.server.TaggedResourceService;
 import com.cloud.storage.CreateSnapshotPayload;
 import com.cloud.storage.DataStoreRole;
 import com.cloud.storage.ScopeType;
@@ -80,6 +126,7 @@ import com.cloud.utils.Ternary;
 import com.cloud.utils.concurrency.NamedThreadFactory;
 import com.cloud.utils.db.DB;
 import com.cloud.utils.db.Filter;
+import com.cloud.utils.db.GlobalLock;
 import com.cloud.utils.db.JoinBuilder;
 import com.cloud.utils.db.SearchBuilder;
 import com.cloud.utils.db.SearchCriteria;
@@ -92,48 +139,6 @@ import com.cloud.vm.dao.UserVmDao;
 import com.cloud.vm.snapshot.VMSnapshot;
 import com.cloud.vm.snapshot.VMSnapshotVO;
 import com.cloud.vm.snapshot.dao.VMSnapshotDao;
-import org.apache.cloudstack.api.command.user.snapshot.CreateSnapshotPolicyCmd;
-import 
org.apache.cloudstack.api.command.user.snapshot.DeleteSnapshotPoliciesCmd;
-import org.apache.cloudstack.api.command.user.snapshot.ListSnapshotPoliciesCmd;
-import org.apache.cloudstack.api.command.user.snapshot.ListSnapshotsCmd;
-import org.apache.cloudstack.api.command.user.snapshot.UpdateSnapshotPolicyCmd;
-import org.apache.cloudstack.context.CallContext;
-import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
-import 
org.apache.cloudstack.engine.subsystem.api.storage.DataStoreCapabilities;
-import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
-import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint;
-import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector;
-import 
org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine;
-import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory;
-import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo;
-import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotService;
-import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy;
-import 
org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy.SnapshotOperation;
-import 
org.apache.cloudstack.engine.subsystem.api.storage.StorageStrategyFactory;
-import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
-import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
-import org.apache.cloudstack.engine.subsystem.api.storage.ZoneScope;
-import org.apache.cloudstack.framework.config.ConfigKey;
-import org.apache.cloudstack.framework.config.Configurable;
-import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
-import org.apache.cloudstack.managed.context.ManagedContextRunnable;
-import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
-import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
-import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO;
-import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
-import org.apache.log4j.Logger;
-import org.springframework.stereotype.Component;
-
-import javax.inject.Inject;
-import javax.naming.ConfigurationException;
-import java.util.ArrayList;
-import java.util.Date;
-import java.util.List;
-import java.util.Map;
-import java.util.TimeZone;
-import java.util.concurrent.Executors;
-import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.TimeUnit;
 
 @Component
 public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase 
implements SnapshotManager, SnapshotApiService, Configurable {
@@ -192,6 +197,8 @@ public class SnapshotManagerImpl extends 
MutualExclusiveIdsManagerBase implement
     ResourceManager _resourceMgr;
     @Inject
     StorageStrategyFactory _storageStrategyFactory;
+    @Inject
+    public TaggedResourceService taggedResourceService;
 
     private int _totalRetries;
     private int _pauseInterval;
@@ -902,6 +909,11 @@ public class SnapshotManagerImpl extends 
MutualExclusiveIdsManagerBase implement
                     policy.setDisplay(display);
                     _snapshotPolicyDao.update(policy.getId(), policy);
                     
_snapSchedMgr.scheduleOrCancelNextSnapshotJobOnDisplayChange(policy, 
previousDisplay);
+                    
taggedResourceService.deleteTags(Collections.singletonList(policy.getUuid()), 
ResourceObjectType.SnapshotPolicy, null);
+                }
+                final Map<String, String> tags = cmd.getTags();
+                if (MapUtils.isNotEmpty(tags)) {
+                    
taggedResourceService.createTags(Collections.singletonList(policy.getUuid()), 
ResourceObjectType.SnapshotPolicy, tags, null);
                 }
             } finally {
                 createSnapshotPolicyLock.unlock();
@@ -916,9 +928,10 @@ public class SnapshotManagerImpl extends 
MutualExclusiveIdsManagerBase implement
         }
     }
 
-    protected boolean deletePolicy(long userId, Long policyId) {
+    protected boolean deletePolicy(Long policyId) {
         SnapshotPolicyVO snapshotPolicy = 
_snapshotPolicyDao.findById(policyId);
         _snapSchedMgr.removeSchedule(snapshotPolicy.getVolumeId(), 
snapshotPolicy.getId());
+        
taggedResourceService.deleteTags(Collections.singletonList(snapshotPolicy.getUuid()),
 ResourceObjectType.SnapshotPolicy, null);
         return _snapshotPolicyDao.remove(policyId);
     }
 
@@ -963,8 +976,7 @@ public class SnapshotManagerImpl extends 
MutualExclusiveIdsManagerBase implement
     public void deletePoliciesForVolume(Long volumeId) {
         List<SnapshotPolicyVO> policyInstances = 
listPoliciesforVolume(volumeId);
         for (SnapshotPolicyVO policyInstance : policyInstances) {
-            Long policyId = policyInstance.getId();
-            deletePolicy(1L, policyId);
+            deletePolicy(policyInstance.getId());
         }
         // We also want to delete the manual snapshots scheduled for this 
volume
         // We can only delete the schedules in the future, not the ones which 
are already executing.
@@ -1321,7 +1333,6 @@ public class SnapshotManagerImpl extends 
MutualExclusiveIdsManagerBase implement
     public boolean deleteSnapshotPolicies(DeleteSnapshotPoliciesCmd cmd) {
         Long policyId = cmd.getId();
         List<Long> policyIds = cmd.getIds();
-        Long userId = getSnapshotUserId();
 
         if ((policyId == null) && (policyIds == null)) {
             throw new InvalidParameterValueException("No policy id (or list of 
ids) specified.");
@@ -1335,6 +1346,10 @@ public class SnapshotManagerImpl extends 
MutualExclusiveIdsManagerBase implement
             throw new InvalidParameterValueException("There are no policy 
ids");
         }
 
+        if (policyIds.contains(Snapshot.MANUAL_POLICY_ID)) {
+            throw new InvalidParameterValueException("Invalid Policy id given: 
" + Snapshot.MANUAL_POLICY_ID);
+        }
+
         for (Long policy : policyIds) {
             SnapshotPolicyVO snapshotPolicyVO = 
_snapshotPolicyDao.findById(policy);
             if (snapshotPolicyVO == null) {
@@ -1348,21 +1363,14 @@ public class SnapshotManagerImpl extends 
MutualExclusiveIdsManagerBase implement
             _accountMgr.checkAccess(CallContext.current().getCallingAccount(), 
null, true, volume);
         }
 
-        boolean success = true;
-
-        if (policyIds.contains(Snapshot.MANUAL_POLICY_ID)) {
-            throw new InvalidParameterValueException("Invalid Policy id given: 
" + Snapshot.MANUAL_POLICY_ID);
-        }
-
         for (Long pId : policyIds) {
-            if (!deletePolicy(userId, pId)) {
-                success = false;
+            if (!deletePolicy(pId)) {
                 s_logger.warn("Failed to delete snapshot policy with Id: " + 
policyId);
-                return success;
+                return false;
             }
         }
 
-        return success;
+        return true;
     }
 
     @Override
diff --git 
a/server/src/main/java/com/cloud/storage/snapshot/SnapshotSchedulerImpl.java 
b/server/src/main/java/com/cloud/storage/snapshot/SnapshotSchedulerImpl.java
index dc71b36..bccf8c6 100644
--- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotSchedulerImpl.java
+++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotSchedulerImpl.java
@@ -43,6 +43,8 @@ import com.cloud.api.ApiDispatcher;
 import com.cloud.api.ApiGsonHelper;
 import com.cloud.event.ActionEventUtils;
 import com.cloud.event.EventTypes;
+import com.cloud.server.ResourceTag;
+import com.cloud.server.TaggedResourceService;
 import com.cloud.storage.Snapshot;
 import com.cloud.storage.SnapshotPolicyVO;
 import com.cloud.storage.SnapshotScheduleVO;
@@ -97,6 +99,8 @@ public class SnapshotSchedulerImpl extends ManagerBase 
implements SnapshotSchedu
     protected VMSnapshotDao _vmSnapshotDao;
     @Inject
     protected VMSnapshotManager _vmSnaphostManager;
+    @Inject
+    public TaggedResourceService taggedResourceService;
 
     protected AsyncJobDispatcher _asyncDispatcher;
 
@@ -305,6 +309,15 @@ public class SnapshotSchedulerImpl extends ManagerBase 
implements SnapshotSchedu
                 params.put("ctxUserId", "1");
                 params.put("ctxAccountId", "" + volume.getAccountId());
                 params.put("ctxStartEventId", String.valueOf(eventId));
+                List<? extends ResourceTag> resourceTags = 
taggedResourceService.listByResourceTypeAndId(ResourceTag.ResourceObjectType.SnapshotPolicy,
 policyId);
+                if (resourceTags != null && !resourceTags.isEmpty()) {
+                    int tagNumber = 0;
+                    for (ResourceTag resourceTag : resourceTags) {
+                        params.put("tags[" + tagNumber + "].key", 
resourceTag.getKey());
+                        params.put("tags[" + tagNumber + "].value", 
resourceTag.getValue());
+                        tagNumber++;
+                    }
+                }
 
                 final CreateSnapshotCmd cmd = new CreateSnapshotCmd();
                 ComponentContext.inject(cmd);
diff --git a/server/src/main/java/com/cloud/tags/TaggedResourceManagerImpl.java 
b/server/src/main/java/com/cloud/tags/TaggedResourceManagerImpl.java
index ae79498..44876d0 100644
--- a/server/src/main/java/com/cloud/tags/TaggedResourceManagerImpl.java
+++ b/server/src/main/java/com/cloud/tags/TaggedResourceManagerImpl.java
@@ -16,13 +16,31 @@
 // under the License.
 package com.cloud.tags;
 
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.stream.Collectors;
+
+import javax.inject.Inject;
+import javax.naming.ConfigurationException;
+import javax.persistence.EntityExistsException;
+
+import org.apache.cloudstack.api.Identity;
+import org.apache.cloudstack.api.InternalIdentity;
+import org.apache.cloudstack.context.CallContext;
+import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.commons.collections.MapUtils;
+import org.apache.commons.lang.StringUtils;
+import org.apache.log4j.Logger;
+
 import com.cloud.dc.DataCenterVO;
 import com.cloud.domain.PartOf;
 import com.cloud.event.ActionEvent;
 import com.cloud.event.EventTypes;
 import com.cloud.exception.InvalidParameterValueException;
 import com.cloud.exception.PermissionDeniedException;
-import com.cloud.offerings.NetworkOfferingVO;
 import com.cloud.network.LBHealthCheckPolicyVO;
 import com.cloud.network.as.AutoScaleVmGroupVO;
 import com.cloud.network.as.AutoScaleVmProfileVO;
@@ -43,6 +61,7 @@ import com.cloud.network.vpc.NetworkACLVO;
 import com.cloud.network.vpc.StaticRouteVO;
 import com.cloud.network.vpc.VpcOfferingVO;
 import com.cloud.network.vpc.VpcVO;
+import com.cloud.offerings.NetworkOfferingVO;
 import com.cloud.projects.ProjectVO;
 import com.cloud.server.ResourceTag;
 import com.cloud.server.ResourceTag.ResourceObjectType;
@@ -74,24 +93,6 @@ import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.vm.NicVO;
 import com.cloud.vm.UserVmVO;
 import com.cloud.vm.snapshot.VMSnapshotVO;
-import org.apache.cloudstack.api.Identity;
-import org.apache.cloudstack.api.InternalIdentity;
-import org.apache.cloudstack.context.CallContext;
-import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
-import org.apache.commons.lang.StringUtils;
-import org.apache.log4j.Logger;
-
-import org.apache.commons.collections.MapUtils;
-
-import javax.inject.Inject;
-import javax.naming.ConfigurationException;
-import javax.persistence.EntityExistsException;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Objects;
-import java.util.stream.Collectors;
 
 public class TaggedResourceManagerImpl extends ManagerBase implements 
TaggedResourceService {
     public static final Logger s_logger = 
Logger.getLogger(TaggedResourceManagerImpl.class);
@@ -220,6 +221,10 @@ public class TaggedResourceManagerImpl extends ManagerBase 
implements TaggedReso
             accountId = ((ProjectVO)entity).getProjectAccountId();
         }
 
+        if (resourceType == ResourceObjectType.SnapshotPolicy) {
+            accountId = _entityMgr.findById(VolumeVO.class, 
((SnapshotPolicyVO)entity).getVolumeId()).getAccountId();
+        }
+
         if (entity instanceof OwnedBy) {
             accountId = ((OwnedBy)entity).getAccountId();
         }
@@ -389,7 +394,7 @@ public class TaggedResourceManagerImpl extends ManagerBase 
implements TaggedReso
         }
 
         if (tagsToDelete.isEmpty()) {
-            throw new InvalidParameterValueException("Unable to find any tags 
which conform to specified delete parameters.");
+            return false;
         }
 
         //Remove the tags
diff --git 
a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java 
b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java
index bb5599f..9d1426c 100644
--- a/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java
+++ b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java
@@ -18,6 +18,7 @@ package com.cloud.storage;
 
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyLong;
+import static org.mockito.Matchers.anyObject;
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.doNothing;
@@ -65,6 +66,7 @@ import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.Spy;
 import org.mockito.runners.MockitoJUnitRunner;
+import org.springframework.test.util.ReflectionTestUtils;
 
 import com.cloud.configuration.Resource;
 import com.cloud.configuration.Resource.ResourceType;
@@ -76,6 +78,7 @@ import com.cloud.host.dao.HostDao;
 import com.cloud.hypervisor.Hypervisor.HypervisorType;
 import com.cloud.org.Grouping;
 import com.cloud.serializer.GsonHelper;
+import com.cloud.server.TaggedResourceService;
 import com.cloud.storage.Volume.Type;
 import com.cloud.storage.dao.StoragePoolTagsDao;
 import com.cloud.storage.dao.VolumeDao;
@@ -409,7 +412,7 @@ public class VolumeApiServiceImplTest {
         
when(volumeDataFactoryMock.getVolume(anyLong())).thenReturn(volumeInfoMock);
         when(volumeInfoMock.getState()).thenReturn(Volume.State.Allocated);
         when(volumeInfoMock.getPoolId()).thenReturn(1L);
-        volumeApiServiceImpl.takeSnapshot(5L, Snapshot.MANUAL_POLICY_ID, 3L, 
null, false, null, false);
+        volumeApiServiceImpl.takeSnapshot(5L, Snapshot.MANUAL_POLICY_ID, 3L, 
null, false, null, false, null);
     }
 
     @Test
@@ -419,7 +422,10 @@ public class VolumeApiServiceImplTest {
         when(volumeInfoMock.getInstanceId()).thenReturn(null);
         when(volumeInfoMock.getPoolId()).thenReturn(1L);
         
when(volumeServiceMock.takeSnapshot(Mockito.any(VolumeInfo.class))).thenReturn(snapshotInfoMock);
-        volumeApiServiceImpl.takeSnapshot(5L, Snapshot.MANUAL_POLICY_ID, 3L, 
null, false, null, false);
+        final TaggedResourceService taggedResourceService = 
Mockito.mock(TaggedResourceService.class);
+        Mockito.when(taggedResourceService.createTags(anyObject(), 
anyObject(), anyObject(), anyObject())).thenReturn(null);
+        ReflectionTestUtils.setField(volumeApiServiceImpl, 
"taggedResourceService", taggedResourceService);
+        volumeApiServiceImpl.takeSnapshot(5L, Snapshot.MANUAL_POLICY_ID, 3L, 
null, false, null, false, null);
     }
 
     @Test
diff --git a/ui/css/cloudstack3.css b/ui/css/cloudstack3.css
index 0d2aca6..c3d8061 100644
--- a/ui/css/cloudstack3.css
+++ b/ui/css/cloudstack3.css
@@ -4027,7 +4027,7 @@ textarea {
   font-size: 14px;
 }
 
-.ui-dialog div.form-container div.value label {
+#label_delete_volumes label {
   display: block;
   width: 119px;
   margin-top: 2px;
@@ -8447,6 +8447,10 @@ div#details-tab-aclRules td.cidrlist span {
   display: inline-block;
 }
 
+.recurring-snapshots .schedule .forms .formContainer {
+  min-height: 250px;
+}
+
 .recurring-snapshots .schedule .add-snapshot-actions {
   float: left;
   clear: both;
@@ -8510,6 +8514,11 @@ div#details-tab-aclRules td.cidrlist span {
   left: 5px;
 }
 
+.recurring-snapshots .schedule .forms .tagger form div.value label {
+  width: 25px;
+  top: 10px;
+}
+
 .recurring-snapshots .schedule .forms form label.error {
   float: left;
   width: 100%;
@@ -8522,6 +8531,10 @@ div#details-tab-aclRules td.cidrlist span {
   margin: 8px 0 0;
 }
 
+.recurring-snapshots .schedule .forms .tagger form .field {
+  margin: 0;
+}
+
 .recurring-snapshots .schedule .forms form .name {
   float: left;
   width: 72px;
@@ -8660,6 +8673,11 @@ div#details-tab-aclRules td.cidrlist span {
   padding: 0;
 }
 
+.recurring-snapshots .ui-tabs .tagger ul {
+    margin: 16px auto auto;
+    padding-bottom: 10px;
+}
+
 .recurring-snapshots .ui-tabs ul li a {
   width: 76px;
   background: url("../images/sprites.png") no-repeat -521px -533px;
@@ -10692,6 +10710,11 @@ div#details-tab-aclRules td.cidrlist span {
   display: none;
 }
 
+.ui-dialog .tagger .tag-info.inside-form {
+  display: block;
+  text-align: left;
+}
+
 .ui-dialog.editTags .ui-button {
   float: right;
 }
diff --git a/ui/index.html b/ui/index.html
index 8645fc2..1dddbd3 100644
--- a/ui/index.html
+++ b/ui/index.html
@@ -1553,7 +1553,7 @@
                         </ul>
 
                         <!-- Hourly -->
-                        <div id="recurring-snapshots-hourly">
+                        <div id="recurring-snapshots-hourly" 
class="formContainer">
                             <form>
                                 <input type="hidden" name="snapshot-type" 
value="hourly" />
 
@@ -1583,11 +1583,14 @@
                                         <label for="maxsnaps"><translate 
key="label.snapshots" /></label>
                                     </div>
                                 </div>
+
+                                <!-- Tags -->
+                                <div class="field taggerContainer"></div>
                             </form>
                         </div>
 
                         <!-- Daily -->
-                        <div id="recurring-snapshots-daily">
+                        <div id="recurring-snapshots-daily" 
class="formContainer">
                             <form>
                                 <input type="hidden" name="snapshot-type" 
value="daily" />
 
@@ -1617,11 +1620,14 @@
                                         <label for="maxsnaps"><translate 
key="label.snapshots" /></label>
                                     </div>
                                 </div>
+
+                                <!-- Tags -->
+                                <div class="field taggerContainer"></div>
                             </form>
                         </div>
 
                         <!-- Weekly -->
-                        <div id="recurring-snapshots-weekly">
+                        <div id="recurring-snapshots-weekly" 
class="formContainer">
                             <form>
                                 <input type="hidden" name="snapshot-type" 
value="weekly" />
 
@@ -1659,11 +1665,14 @@
                                         <label for="maxsnaps"><translate 
key="label.snapshots" /></label>
                                     </div>
                                 </div>
+
+                                <!-- Tags -->
+                                <div class="field taggerContainer"></div>
                             </form>
                         </div>
 
                         <!-- Monthly -->
-                        <div id="recurring-snapshots-monthly">
+                        <div id="recurring-snapshots-monthly" 
class="formContainer">
                             <form>
                                 <input type="hidden" name="snapshot-type" 
value="monthly" />
 
@@ -1701,6 +1710,9 @@
                                         <label for="maxsnaps"><translate 
key="label.snapshots" /></label>
                                     </div>
                                 </div>
+
+                                <!-- Tags -->
+                                <div class="field taggerContainer"></div>
                             </form>
                         </div>
                     </div>
diff --git a/ui/scripts/storage.js b/ui/scripts/storage.js
index 789af42..e6eaacf 100644
--- a/ui/scripts/storage.js
+++ b/ui/scripts/storage.js
@@ -921,6 +921,10 @@
                                         asyncBackup: {
                                             label: 'label.async.backup',
                                             isBoolean: true
+                                        },
+                                        tags: {
+                                            label: 'label.tags',
+                                            tagger: true
                                         }
                                     }
                                 },
@@ -935,6 +939,15 @@
                                             name: args.data.name
                                         });
                                     }
+                                    if (!$.isEmptyObject(args.data.tags)) {
+                                        $(args.data.tags).each(function(idx, 
tagData) {
+                                            var formattedTagData = {};
+                                            formattedTagData["tags[" + _s(idx) 
+ "].key"] = _s(tagData.key);
+                                            formattedTagData["tags[" + _s(idx) 
+ "].value"] = _s(tagData.value);
+                                            $.extend(data, formattedTagData);
+                                        });
+                                    }
+
                                     $.ajax({
                                         url: createURL("createSnapshot"),
                                         data: data,
@@ -1000,7 +1013,9 @@
                                                 var snap = args.snapshot;
 
                                                 var data = {
-                                                    keep: snap.maxsnaps,
+                                                    volumeid: 
args.context.volumes[0].id,
+                                                    intervaltype: 
snap['snapshot-type'],
+                                                    maxsnaps: snap.maxsnaps,
                                                     timezone: snap.timezone
                                                 };
 
@@ -1053,15 +1068,18 @@
                                                         break;
                                                 }
 
+                                                if 
(!$.isEmptyObject(snap.tags)) {
+                                                    
$(snap.tags).each(function(idx, tagData) {
+                                                        var formattedTagData = 
{};
+                                                        
formattedTagData["tags[" + _s(idx) + "].key"] = _s(tagData.key);
+                                                        
formattedTagData["tags[" + _s(idx) + "].value"] = _s(tagData.value);
+                                                        $.extend(data, 
formattedTagData);
+                                                    });
+                                                }
+
                                                 $.ajax({
                                                     url: 
createURL('createSnapshotPolicy'),
-                                                    data: {
-                                                        volumeid: 
args.context.volumes[0].id,
-                                                        intervaltype: 
snap['snapshot-type'],
-                                                        maxsnaps: 
snap.maxsnaps,
-                                                        schedule: 
data.schedule,
-                                                        timezone: snap.timezone
-                                                    },
+                                                    data: data,
                                                     dataType: 'json',
                                                     async: true,
                                                     success: 
function(successData) {
diff --git a/ui/scripts/ui-custom/recurringSnapshots.js 
b/ui/scripts/ui-custom/recurringSnapshots.js
index 6d0eefd..82b3489 100644
--- a/ui/scripts/ui-custom/recurringSnapshots.js
+++ b/ui/scripts/ui-custom/recurringSnapshots.js
@@ -61,6 +61,10 @@
                 }
             });
 
+            $($snapshots.find('.taggerContainer')).each(function() {
+                $('<div>').taggerInForm().appendTo(this);
+            });
+
             // Form validation
             $snapshots.find('form').validate();
 
@@ -70,7 +74,7 @@
 
                 if (!$form.valid()) return false;
 
-                var formData = cloudStack.serializeForm($form);
+                var formData = $.extend(cloudStack.serializeForm($form), 
{'tags' : cloudStack.getTagsFromForm($form)});
 
                 actions.add({
                     context: context,
diff --git a/ui/scripts/ui/dialog.js b/ui/scripts/ui/dialog.js
index 96f2298..de2709e 100644
--- a/ui/scripts/ui/dialog.js
+++ b/ui/scripts/ui/dialog.js
@@ -149,7 +149,6 @@
 
             var isAsync = false;
             var isNoDialog = args.noDialog ? args.noDialog : false;
-
             $(fields).each(function(idx, element) {
                 var key = this;
                 var field = args.form.fields[key];
@@ -190,7 +189,6 @@
                  closeOnEscape: false
                  }); */
                 // Label field
-
                 var $name = $('<div>').addClass('name')
                         .appendTo($formItem)
                         .append(
@@ -619,9 +617,9 @@
                     }
                     $input.addClass("disallowSpecialCharacters");
                     $input.datepicker({
-                       dateFormat: 'yy-mm-dd',
-                       maxDate: field.maxDate,
-                       minDate: field.minDate
+                        dateFormat: 'yy-mm-dd',
+                        maxDate: field.maxDate,
+                        minDate: field.minDate
                     });
 
                 } else if (field.range) { //2 text fields on the same line 
(e.g. port range: startPort - endPort)
@@ -702,6 +700,10 @@
 
                         this.oldUnit = newUnit;
                     })
+                } else if (field.tagger) {
+                    $name.hide();
+                    $value.hide();
+                    $input = $('<div>').taggerInForm().appendTo($formItem);
                 } else { //text field
                     $input = $('<input>').attr({
                         name: key,
@@ -717,10 +719,12 @@
                     $input.addClass("disallowSpecialCharacters");
                 }
 
-                if (field.validation != null)
-                    $input.data('validation-rules', field.validation);
-                else
-                    $input.data('validation-rules', {});
+                if (!field.tagger) { // Tagger has it's own validation
+                    if (field.validation != null)
+                        $input.data('validation-rules', field.validation);
+                    else
+                        $input.data('validation-rules', {});
+                }
 
                 var fieldLabel = field.label;
 
@@ -774,7 +778,7 @@
 
             var complete = function($formContainer) {
                 var $form = $formContainer.find('form');
-                var data = cloudStack.serializeForm($form);
+                var data = $.extend(cloudStack.serializeForm($form), {'tags' : 
cloudStack.getTagsFromForm($form)});
 
                 if (!$formContainer.find('form').valid()) {
                     // Ignore hidden field validation
diff --git a/ui/scripts/ui/widgets/tagger.js b/ui/scripts/ui/widgets/tagger.js
index a77d6c5..7356dc0 100644
--- a/ui/scripts/ui/widgets/tagger.js
+++ b/ui/scripts/ui/widgets/tagger.js
@@ -46,10 +46,7 @@
 
             $keyField.append($keyLabel, $key);
             $valueField.append($valueLabel, $value);
-            $form.append(
-                $keyField, $valueField,
-                $submit
-            );
+            $form.append($keyField, $valueField, $submit);
 
             $form.validate();
 
@@ -80,9 +77,11 @@
                         }
                     });
 
-                    // Prevent input during submission
-                    $key.attr('disabled', 'disabled');
-                    $value.attr('disabled', 'disabled');
+                    if (args.isAsyncSubmit) {
+                        // Prevent input during submission
+                        $key.attr('disabled', 'disabled');
+                        $value.attr('disabled', 'disabled');
+                    }
 
                     return false;
                 } :
@@ -102,6 +101,8 @@
 
             $label.append($key, '<span>=</span>', $value);
             $label.attr('title', title);
+            $label.attr('cloudstack_tag_key', _s(data.key));
+            $label.attr('cloudstack_tag_value', _s(data.value));
             $remove.click(function() {
                 if (onRemove) onRemove($li, data);
             });
@@ -173,6 +174,7 @@
             };
 
             var $inputArea = elems.inputArea({
+                isAsyncSubmit: true,
                 onSubmit: function(args) {
                     var data = args.data;
                     var success = args.response.success;
@@ -252,4 +254,53 @@
             });
         }
     });
+
+    $.widget('cloudStack.taggerInForm', {
+        _init: function(args) {
+            var $container = this.element.addClass('tagger');
+            var $tagArea = $('<ul>').addClass('tags');
+            var $title = elems.info(_l('label.tags')).addClass('title 
inside-form');
+            var $loading = $('<div>').addClass('loading-overlay');
+            var $tags = {};
+
+            var onRemoveItem = function($item, data) {
+                $item.remove();
+                if ($tags[data.key]) delete $tags[data.key];
+                else {
+                    cloudStack.dialog.notice({
+                        message: "Unexpected error occured in attempting 
deletion"
+                    });
+                }
+            };
+
+            var $inputArea = elems.inputArea({
+                isAsyncSubmit: false,
+                onSubmit: function(args) {
+                    var data = args.data;
+                    if ($tags[data.key]) {
+                        cloudStack.dialog.notice({
+                            message: "Key already present. Please delete 
previous and add again."
+                        });
+                    } else {
+                        var success = args.response.success;
+                        var title = data.key + ' = ' + data.value;
+                        elems.tagItem(title, onRemoveItem, 
data).appendTo($tagArea);
+                        success();
+                        $tags[data.key] = data.value;
+                    }
+                }
+            });
+
+            $container.append($title, $inputArea, $tagArea);
+        }
+    });
+
+    cloudStack.getTagsFromForm = function($form) {
+        var tagLabels = $($form).find('.tagger .tags .label');
+        var tags = [];
+        $(tagLabels).each(function() {
+            tags.push({'key' : $(this).attr('cloudstack_tag_key'), 'value' : 
$(this).attr('cloudstack_tag_value')});
+        });
+        return tags;
+    };
 }(jQuery, cloudStack));

Reply via email to