From 6b30c38050e367d0872e4895de7543dbdfeca67c Mon Sep 17 00:00:00 2001 From: Anurag Awasthi Date: Thu, 14 Mar 2019 17:19:18 +0530 Subject: [PATCH 1/7] api: snapshot, snapshotpolicy tag support This change allows adding tags to snapshots and snapshot policy while creating them. createSnapshot, createSnapshotPolicy APIs have been modified to allow passing key-value based tags. UI has been modified to show input controls for allowing addition of tags in both create snapshot and snapshot policy form. Unit test for CreateSnapshotPolicyCmd has been added. Signed-off-by: Abhishek Kumar --- .../java/com/cloud/server/ResourceTag.java | 2 +- .../com/cloud/storage/VolumeApiService.java | 3 +- .../user/snapshot/CreateSnapshotCmd.java | 22 ++- .../snapshot/CreateSnapshotPolicyCmd.java | 23 +++- .../api/response/SnapshotPolicyResponse.java | 16 ++- .../api/response/SnapshotResponse.java | 26 ++-- .../command/test/CreateSnapshotCmdTest.java | 30 ++++- .../java/com/cloud/api/ApiResponseHelper.java | 10 +- .../cloud/storage/VolumeApiServiceImpl.java | 16 ++- .../storage/snapshot/SnapshotManagerImpl.java | 126 ++++++++++-------- .../snapshot/SnapshotSchedulerImpl.java | 13 ++ .../storage/VolumeApiServiceImplTest.java | 10 +- ui/css/cloudstack3.css | 21 ++- ui/index.html | 12 ++ 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 ++++++++- 18 files changed, 347 insertions(+), 112 deletions(-) diff --git a/api/src/main/java/com/cloud/server/ResourceTag.java b/api/src/main/java/com/cloud/server/ResourceTag.java index 0bd5d734e30e..99eb8603d1c3 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 enum ResourceObjectType { 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 7b38a6b1af10..aa6d8a664c8d 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 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 b77b98545900..72437c8d3fe8 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.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 Long getPolicyId() { } } + public Map getTags() { + Map tagsMap = new HashMap<>(); + if (MapUtils.isNotEmpty(tags)) { + for (Map services : (Collection>)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 void execute() { 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 4e2e6bd3bbd0..898bae5919ff 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.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 long getEntityOwnerId() { return volume.getAccountId(); } + public Map getTags() { + Map tagsMap = new HashMap<>(); + if (MapUtils.isNotEmpty(tags)) { + for (Map services : (Collection>)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 10710c64803d..d1e535ee7433 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(); + } + public String getId() { return id; } @@ -111,4 +117,8 @@ public Boolean isForDisplay() { public void setForDisplay(Boolean forDisplay) { this.forDisplay = forDisplay; } + + public void setTags(Set 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 bb2ff7f6d0e9..94bb4d144449 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 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(); + } + @Override public String getObjectId() { return this.getId(); @@ -206,7 +208,7 @@ public void setZoneId(String zoneId) { this.zoneId = zoneId; } - public void setTags(List tags) { + public void setTags(Set 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 ceb63ab6e560..4739082cf644 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 @@ 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.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 void testCreateSuccess() { 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 void testCreateFailure() { 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 void testCreateFailure() { 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 tag1 = new HashMap<>(); + tag1.put("key", "key1"); + tag1.put("value", "value1"); + final Map tag2 = new HashMap<>(); + tag2.put("key", "key2"); + tag2.put("value", "value2"); + final Map expectedTags = new HashMap<>(); + expectedTags.put("key1", "value1"); + expectedTags.put("key2", "value2"); + + final Map> tagsParams = new HashMap<>(); + tagsParams.put("0", tag1); + tagsParams.put("1", tag2); + ReflectionTestUtils.setField(createSnapshotCmd, "tags", tagsParams); + Assert.assertEquals(createSnapshotCmd.getTags(), expectedTags); + } } diff --git a/server/src/main/java/com/cloud/api/ApiResponseHelper.java b/server/src/main/java/com/cloud/api/ApiResponseHelper.java index 9deaa962f2be..154971710e57 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 SnapshotResponse createSnapshotResponse(Snapshot snapshot) { 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 SnapshotPolicyResponse createSnapshotPolicyResponse(SnapshotPolicy policy policyResponse.setForDisplay(policy.isDisplay()); policyResponse.setObjectName("snapshotpolicy"); + List tags = _resourceTagDao.listBy(policy.getId(), ResourceObjectType.SnapshotPolicy); + List tagResponses = new ArrayList(); + 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 ae9af51a71d6..f3f70750c036 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.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.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; @@ -2330,7 +2335,16 @@ protected Volume liveMigrateVolume(Volume volume, StoragePool destPool) throws S @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 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 ee65486a1275..51354b33ce9f 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.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.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.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 SnapshotPolicyVO createPolicy(CreateSnapshotPolicyCmd cmd, Account policy policy.setDisplay(display); _snapshotPolicyDao.update(policy.getId(), policy); _snapSchedMgr.scheduleOrCancelNextSnapshotJobOnDisplayChange(policy, previousDisplay); + taggedResourceService.deleteTags(Collections.singletonList(policy.getUuid()), ResourceObjectType.SnapshotPolicy, null); + } + final Map tags = cmd.getTags(); + if (MapUtils.isNotEmpty(tags)) { + taggedResourceService.createTags(Collections.singletonList(policy.getUuid()), ResourceObjectType.SnapshotPolicy, tags, null); } } finally { createSnapshotPolicyLock.unlock(); @@ -916,10 +928,13 @@ public SnapshotPolicyVO createPolicy(CreateSnapshotPolicyCmd cmd, Account policy } } - protected boolean deletePolicy(long userId, Long policyId) { + protected boolean deletePolicy(Long policyId) { SnapshotPolicyVO snapshotPolicy = _snapshotPolicyDao.findById(policyId); _snapSchedMgr.removeSchedule(snapshotPolicy.getVolumeId(), snapshotPolicy.getId()); - return _snapshotPolicyDao.remove(policyId); + if (taggedResourceService.deleteTags(Collections.singletonList(snapshotPolicy.getUuid()), ResourceObjectType.SnapshotPolicy, null)) { + return _snapshotPolicyDao.remove(policyId); + } + throw new CloudRuntimeException("Unable to delete snapshot policy due to failure to delete tags for the snapshot policy uuid: " + snapshotPolicy.getUuid()); } @Override @@ -963,8 +978,7 @@ private List listSnapsforVolumeTypeNotDestroyed(long volumeId, Type public void deletePoliciesForVolume(Long volumeId) { List 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 +1335,6 @@ public boolean stop() { public boolean deleteSnapshotPolicies(DeleteSnapshotPoliciesCmd cmd) { Long policyId = cmd.getId(); List policyIds = cmd.getIds(); - Long userId = getSnapshotUserId(); if ((policyId == null) && (policyIds == null)) { throw new InvalidParameterValueException("No policy id (or list of ids) specified."); @@ -1335,6 +1348,10 @@ public boolean deleteSnapshotPolicies(DeleteSnapshotPoliciesCmd cmd) { 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 +1365,14 @@ public boolean deleteSnapshotPolicies(DeleteSnapshotPoliciesCmd cmd) { _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 dc71b3653530..bccf8c68b232 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.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 @@ protected void scheduleSnapshots() { params.put("ctxUserId", "1"); params.put("ctxAccountId", "" + volume.getAccountId()); params.put("ctxStartEventId", String.valueOf(eventId)); + List 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/test/java/com/cloud/storage/VolumeApiServiceImplTest.java b/server/src/test/java/com/cloud/storage/VolumeApiServiceImplTest.java index bb5599f0a996..9d1426ce9f15 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 @@ 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.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.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 void testTakeSnapshotF1() throws ResourceAllocationException { 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 void testTakeSnapshotF2() throws ResourceAllocationException { 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 b6877cdaf292..810cc9ca88ba 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; @@ -8510,6 +8510,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 +8527,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 +8669,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 +10706,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 8645fc252c84..4c9adc517e0f 100644 --- a/ui/index.html +++ b/ui/index.html @@ -1583,6 +1583,9 @@

+ + +
@@ -1617,6 +1620,9 @@

+ + +
@@ -1659,6 +1665,9 @@

+ + +
@@ -1701,6 +1710,9 @@

+ + +
diff --git a/ui/scripts/storage.js b/ui/scripts/storage.js index f1d3330f2624..63da51da1797 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 6d0eefd40326..82b348930479 100644 --- a/ui/scripts/ui-custom/recurringSnapshots.js +++ b/ui/scripts/ui-custom/recurringSnapshots.js @@ -61,6 +61,10 @@ } }); + $($snapshots.find('.taggerContainer')).each(function() { + $('
').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 96f2298ff1a4..de2709ef57f0 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 = $('
').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 = $('
').taggerInForm().appendTo($formItem); } else { //text field $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 a77d6c518304..7356dc055db9 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, '=', $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 = $('
    ').addClass('tags'); + var $title = elems.info(_l('label.tags')).addClass('title inside-form'); + var $loading = $('
    ').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)); From 46728dd72b29ae51ac32730f37d14f28cc7d6ba6 Mon Sep 17 00:00:00 2001 From: Anurag Awasthi Date: Fri, 15 Mar 2019 11:52:55 +0530 Subject: [PATCH 2/7] api: added CreateSnapshotPolicyCmd class unit test This unit test class was missing from previous commit changes Signed-off-by: Abhishek Kumar --- .../snapshot/CreateSnapshotPolicyCmdTest.java | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 api/src/test/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotPolicyCmdTest.java 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 000000000000..935db7e705e6 --- /dev/null +++ b/api/src/test/java/org/apache/cloudstack/api/command/user/snapshot/CreateSnapshotPolicyCmdTest.java @@ -0,0 +1,30 @@ +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 tag1 = new HashMap<>(); + tag1.put("key", "key1"); + tag1.put("value", "value1"); + final Map tag2 = new HashMap<>(); + tag2.put("key", "key2"); + tag2.put("value", "value2"); + final Map expectedTags = new HashMap<>(); + expectedTags.put("key1", "value1"); + expectedTags.put("key2", "value2"); + + final Map> 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 From 1a5a5e587e1ab43a1fba55c9f7ede65728b51b0e Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 26 Mar 2019 18:20:15 +0530 Subject: [PATCH 3/7] Added missing license for CreateSnapshotPolicyCmdTest Signed-off-by: Abhishek Kumar --- .../snapshot/CreateSnapshotPolicyCmdTest.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) 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 index 935db7e705e6..860c2c2c3e71 100644 --- 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 @@ -1,3 +1,19 @@ +// 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; From 76434e13a6a9d785262dbc1a13e87d4c26853537 Mon Sep 17 00:00:00 2001 From: Anurag Awasthi Date: Wed, 19 Jun 2019 13:04:02 +0530 Subject: [PATCH 4/7] Dont throw exception for missing tags The interface doesn't support and callers already take care of returned value to decide if tags were deleted or not. --- .../cloud/tags/TaggedResourceManagerImpl.java | 41 ++++++++++--------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/server/src/main/java/com/cloud/tags/TaggedResourceManagerImpl.java b/server/src/main/java/com/cloud/tags/TaggedResourceManagerImpl.java index ae7949863b48..9191e45b2427 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.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.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); @@ -389,7 +390,7 @@ public boolean deleteTags(List resourceIds, ResourceObjectType resourceT } if (tagsToDelete.isEmpty()) { - throw new InvalidParameterValueException("Unable to find any tags which conform to specified delete parameters."); + return false; } //Remove the tags From 53899239e132ea59a8db5f58136793df826d40bf Mon Sep 17 00:00:00 2001 From: Anurag Awasthi Date: Wed, 19 Jun 2019 13:08:52 +0530 Subject: [PATCH 5/7] Bug fix for delete policy - delete even if there were no tags to delete --- .../com/cloud/storage/snapshot/SnapshotManagerImpl.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) 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 51354b33ce9f..5d98c503ab91 100755 --- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -931,10 +931,8 @@ public SnapshotPolicyVO createPolicy(CreateSnapshotPolicyCmd cmd, Account policy protected boolean deletePolicy(Long policyId) { SnapshotPolicyVO snapshotPolicy = _snapshotPolicyDao.findById(policyId); _snapSchedMgr.removeSchedule(snapshotPolicy.getVolumeId(), snapshotPolicy.getId()); - if (taggedResourceService.deleteTags(Collections.singletonList(snapshotPolicy.getUuid()), ResourceObjectType.SnapshotPolicy, null)) { - return _snapshotPolicyDao.remove(policyId); - } - throw new CloudRuntimeException("Unable to delete snapshot policy due to failure to delete tags for the snapshot policy uuid: " + snapshotPolicy.getUuid()); + taggedResourceService.deleteTags(Collections.singletonList(snapshotPolicy.getUuid()), ResourceObjectType.SnapshotPolicy, null); + return _snapshotPolicyDao.remove(policyId); } @Override From eeb2238f5d9492942ff784a5fc4415dc8fc4ce6e Mon Sep 17 00:00:00 2001 From: Anurag Awasthi Date: Thu, 20 Jun 2019 11:22:05 +0530 Subject: [PATCH 6/7] Allow users to tag snapshot policy for the volumes they own --- .../main/java/com/cloud/tags/TaggedResourceManagerImpl.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/server/src/main/java/com/cloud/tags/TaggedResourceManagerImpl.java b/server/src/main/java/com/cloud/tags/TaggedResourceManagerImpl.java index 9191e45b2427..44876d0ddf54 100644 --- a/server/src/main/java/com/cloud/tags/TaggedResourceManagerImpl.java +++ b/server/src/main/java/com/cloud/tags/TaggedResourceManagerImpl.java @@ -221,6 +221,10 @@ private Pair getAccountDomain(long resourceId, ResourceObjectType re 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(); } From ee904ceb68d822c5b2ed4a150970e782344fa4e1 Mon Sep 17 00:00:00 2001 From: Anurag Awasthi Date: Tue, 25 Jun 2019 12:32:06 +0530 Subject: [PATCH 7/7] Increase min height for snapshot dialog forms --- ui/css/cloudstack3.css | 4 ++++ ui/index.html | 8 ++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/ui/css/cloudstack3.css b/ui/css/cloudstack3.css index 810cc9ca88ba..45d60aa97e8d 100644 --- a/ui/css/cloudstack3.css +++ b/ui/css/cloudstack3.css @@ -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; diff --git a/ui/index.html b/ui/index.html index 4c9adc517e0f..1dddbd3cdc8a 100644 --- a/ui/index.html +++ b/ui/index.html @@ -1553,7 +1553,7 @@

-
+
@@ -1590,7 +1590,7 @@

-
+
@@ -1627,7 +1627,7 @@

-
+
@@ -1672,7 +1672,7 @@

-
+