You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by da...@apache.org on 2020/10/16 07:23:04 UTC
[cloudstack] branch master updated: enable update tags on disk
offerings (#4194)
This is an automated email from the ASF dual-hosted git repository.
dahn 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 c222d0b enable update tags on disk offerings (#4194)
c222d0b is described below
commit c222d0bf607efd5685f49cc24d3538f5a4722b39
Author: Rodrigo D. Lopez <ro...@hotmail.com>
AuthorDate: Fri Oct 16 04:22:42 2020 -0300
enable update tags on disk offerings (#4194)
---
.../admin/offering/UpdateDiskOfferingCmd.java | 13 +++++
.../java/com/cloud/storage/DiskOfferingVO.java | 2 +-
.../configuration/ConfigurationManagerImpl.java | 55 ++++++++++++----------
.../configuration/ConfigurationManagerTest.java | 36 +++++++++++++-
ui/scripts/configuration.js | 39 ++++++++++++++-
5 files changed, 115 insertions(+), 30 deletions(-)
diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateDiskOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateDiskOfferingCmd.java
index 9036953..e1fd878 100644
--- a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateDiskOfferingCmd.java
+++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateDiskOfferingCmd.java
@@ -19,6 +19,7 @@ package org.apache.cloudstack.api.command.admin.offering;
import java.util.ArrayList;
import java.util.List;
+import org.apache.cloudstack.acl.RoleType;
import org.apache.cloudstack.api.APICommand;
import org.apache.cloudstack.api.ApiConstants;
import org.apache.cloudstack.api.ApiErrorCode;
@@ -77,6 +78,14 @@ public class UpdateDiskOfferingCmd extends BaseCmd {
since = "4.13")
private String zoneIds;
+ @Parameter(name = ApiConstants.TAGS,
+ type = CommandType.STRING,
+ description = "comma-separated list of tags for the disk offering, tags should match with existing storage pool tags",
+ authorized = {RoleType.Admin},
+ since = "4.15")
+ private String tags;
+
+
/////////////////////////////////////////////////////
/////////////////// Accessors ///////////////////////
/////////////////////////////////////////////////////
@@ -161,6 +170,10 @@ public class UpdateDiskOfferingCmd extends BaseCmd {
return validZoneIds;
}
+ public String getTags() {
+ return tags;
+ }
+
/////////////////////////////////////////////////////
/////////////// API Implementation///////////////////
/////////////////////////////////////////////////////
diff --git a/engine/schema/src/main/java/com/cloud/storage/DiskOfferingVO.java b/engine/schema/src/main/java/com/cloud/storage/DiskOfferingVO.java
index 7f91537..952d301 100644
--- a/engine/schema/src/main/java/com/cloud/storage/DiskOfferingVO.java
+++ b/engine/schema/src/main/java/com/cloud/storage/DiskOfferingVO.java
@@ -364,7 +364,7 @@ public class DiskOfferingVO implements DiskOffering {
return created;
}
- protected void setTags(String tags) {
+ public void setTags(String tags) {
this.tags = tags;
}
diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
index 0ad3563..ac2ff07 100755
--- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
+++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
@@ -39,6 +39,7 @@ import java.util.UUID;
import javax.inject.Inject;
import javax.naming.ConfigurationException;
+import com.cloud.utils.StringUtils;
import org.apache.cloudstack.acl.SecurityChecker;
import org.apache.cloudstack.affinity.AffinityGroup;
import org.apache.cloudstack.affinity.AffinityGroupService;
@@ -222,7 +223,6 @@ import com.cloud.user.dao.AccountDao;
import com.cloud.user.dao.UserDao;
import com.cloud.utils.NumbersUtil;
import com.cloud.utils.Pair;
-import com.cloud.utils.StringUtils;
import com.cloud.utils.UriUtils;
import com.cloud.utils.component.ManagerBase;
import com.cloud.utils.db.DB;
@@ -3109,6 +3109,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
final Boolean displayDiskOffering = cmd.getDisplayOffering();
final List<Long> domainIds = cmd.getDomainIds();
final List<Long> zoneIds = cmd.getZoneIds();
+ final String tags = cmd.getTags();
// Check if diskOffering exists
final DiskOffering diskOfferingHandle = _entityMgr.findById(DiskOffering.class, diskOfferingId);
@@ -3190,7 +3191,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
throw new InvalidParameterValueException(String.format("Unable to update disk offering: %s by id user: %s because it is not root-admin or domain-admin", diskOfferingHandle.getUuid(), user.getUuid()));
}
- final boolean updateNeeded = name != null || displayText != null || sortKey != null || displayDiskOffering != null;
+ final boolean updateNeeded = shouldUpdateDiskOffering(name, displayText, sortKey, displayDiskOffering, tags);
final boolean detailsUpdateNeeded = !filteredDomainIds.equals(existingDomainIds) || !filteredZoneIds.equals(existingZoneIds);
if (!updateNeeded && !detailsUpdateNeeded) {
return _diskOfferingDao.findById(diskOfferingId);
@@ -3214,30 +3215,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
diskOffering.setDisplayOffering(displayDiskOffering);
}
- // Note: tag editing commented out for now;keeping the code intact,
- // might need to re-enable in next releases
- // if (tags != null)
- // {
- // if (tags.trim().isEmpty() && diskOfferingHandle.getTags() == null)
- // {
- // //no new tags; no existing tags
- // diskOffering.setTagsArray(csvTagsToList(null));
- // }
- // else if (!tags.trim().isEmpty() && diskOfferingHandle.getTags() !=
- // null)
- // {
- // //new tags + existing tags
- // List<String> oldTags = csvTagsToList(diskOfferingHandle.getTags());
- // List<String> newTags = csvTagsToList(tags);
- // oldTags.addAll(newTags);
- // diskOffering.setTagsArray(oldTags);
- // }
- // else if(!tags.trim().isEmpty())
- // {
- // //new tags; NO existing tags
- // diskOffering.setTagsArray(csvTagsToList(tags));
- // }
- // }
+ updateDiskOfferingTagsIfIsNotNull(tags, diskOffering);
if (updateNeeded && !_diskOfferingDao.update(diskOfferingId, diskOffering)) {
return null;
@@ -3274,6 +3252,31 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
return _diskOfferingDao.findById(diskOfferingId);
}
+ /**
+ * Check the tags parameters to the diskOffering
+ * <ul>
+ * <li>If tags is null, do nothing and return.</li>
+ * <li>If tags is not null, set tag to the diskOffering.</li>
+ * <li>If tags is an blank string, set null on diskOffering tag.</li>
+ * </ul>
+ */
+ protected void updateDiskOfferingTagsIfIsNotNull(String tags, DiskOfferingVO diskOffering) {
+ if (tags == null) { return; }
+ if (StringUtils.isNotBlank(tags)) {
+ diskOffering.setTags(tags);
+ } else {
+ diskOffering.setTags(null);
+ }
+ }
+
+ /**
+ * Check if it needs to update any parameter when updateDiskoffering is called
+ * Verify if name or displayText are not blank, tags is not null, sortkey and displayDiskOffering is not null
+ */
+ protected boolean shouldUpdateDiskOffering(String name, String displayText, Integer sortKey, Boolean displayDiskOffering, String tags) {
+ return StringUtils.isNotBlank(name) || StringUtils.isNotBlank(displayText) || tags != null || sortKey != null || displayDiskOffering != null;
+ }
+
@Override
@ActionEvent(eventType = EventTypes.EVENT_DISK_OFFERING_DELETE, eventDescription = "deleting disk offering")
public boolean deleteDiskOffering(final DeleteDiskOfferingCmd cmd) {
diff --git a/server/src/test/java/com/cloud/configuration/ConfigurationManagerTest.java b/server/src/test/java/com/cloud/configuration/ConfigurationManagerTest.java
index 8c11f77..0f95cb0 100644
--- a/server/src/test/java/com/cloud/configuration/ConfigurationManagerTest.java
+++ b/server/src/test/java/com/cloud/configuration/ConfigurationManagerTest.java
@@ -24,6 +24,7 @@ import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.nullable;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
@@ -56,6 +57,7 @@ import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
+import org.mockito.Spy;
import com.cloud.api.query.dao.NetworkOfferingJoinDao;
import com.cloud.api.query.vo.NetworkOfferingJoinVO;
@@ -87,6 +89,7 @@ import com.cloud.network.dao.IPAddressVO;
import com.cloud.network.dao.PhysicalNetworkDao;
import com.cloud.network.dao.PhysicalNetworkVO;
import com.cloud.projects.ProjectManager;
+import com.cloud.storage.DiskOfferingVO;
import com.cloud.storage.VolumeVO;
import com.cloud.storage.dao.VolumeDao;
import com.cloud.user.Account;
@@ -108,6 +111,7 @@ public class ConfigurationManagerTest {
private static final Logger s_logger = Logger.getLogger(ConfigurationManagerTest.class);
+ @Spy
@InjectMocks
ConfigurationManagerImpl configurationMgr = new ConfigurationManagerImpl();
@@ -163,11 +167,12 @@ public class ConfigurationManagerTest {
ImageStoreDao _imageStoreDao;
@Mock
ConfigurationDao _configDao;
+ @Mock
+ DiskOfferingVO diskOfferingVOMock;
VlanVO vlan = new VlanVO(Vlan.VlanType.VirtualNetwork, "vlantag", "vlangateway", "vlannetmask", 1L, "iprange", 1L, 1L, null, null, null);
private static final String MAXIMUM_DURATION_ALLOWED = "3600";
-
@Mock
Network network;
@Mock
@@ -938,4 +943,33 @@ public class ConfigurationManagerTest {
public void validateMaximumIopsAndBytesLengthTestDefaultLengthConfigs() {
configurationMgr.validateMaximumIopsAndBytesLength(36000l, 36000l, 36000l, 36000l);
}
+
+ @Test
+ public void shouldUpdateDiskOfferingTests(){
+ Assert.assertTrue(configurationMgr.shouldUpdateDiskOffering(Mockito.anyString(), Mockito.anyString(), Mockito.anyInt(), Mockito.anyBoolean(), Mockito.anyString()));
+ Assert.assertTrue(configurationMgr.shouldUpdateDiskOffering(Mockito.anyString(), nullable(String.class), nullable(Integer.class), nullable(Boolean.class), nullable(String.class)));
+ Assert.assertTrue(configurationMgr.shouldUpdateDiskOffering(nullable(String.class), Mockito.anyString(), nullable(Integer.class), nullable(Boolean.class), nullable(String.class)));
+ Assert.assertTrue(configurationMgr.shouldUpdateDiskOffering(nullable(String.class), nullable(String.class), Mockito.anyInt(), nullable(Boolean.class), nullable(String.class)));
+ Assert.assertTrue(configurationMgr.shouldUpdateDiskOffering(nullable(String.class), nullable(String.class), nullable(int.class), Mockito.anyBoolean(), nullable(String.class)));
+ Assert.assertTrue(configurationMgr.shouldUpdateDiskOffering(nullable(String.class), nullable(String.class), nullable(int.class), nullable(Boolean.class), Mockito.anyString()));
+ }
+
+ @Test
+ public void shouldUpdateDiskOfferingTestFalse(){
+ Assert.assertFalse(configurationMgr.shouldUpdateDiskOffering(null, null, null, null, null));
+ }
+
+ @Test
+ public void updateDiskOfferingTagsIfIsNotNullTestWhenTagsIsNull(){
+ Mockito.doNothing().when(configurationMgr).updateDiskOfferingTagsIfIsNotNull(null, diskOfferingVOMock);
+ this.configurationMgr.updateDiskOfferingTagsIfIsNotNull(null, diskOfferingVOMock);
+ Mockito.verify(configurationMgr, Mockito.times(1)).updateDiskOfferingTagsIfIsNotNull(null, diskOfferingVOMock);
+ }
+ @Test
+ public void updateDiskOfferingTagsIfIsNotNullTestWhenTagsIsNotNull(){
+ String tags = "tags";
+ Mockito.doNothing().when(configurationMgr).updateDiskOfferingTagsIfIsNotNull(tags, diskOfferingVOMock);
+ this.configurationMgr.updateDiskOfferingTagsIfIsNotNull(tags, diskOfferingVOMock);
+ Mockito.verify(configurationMgr, Mockito.times(1)).updateDiskOfferingTagsIfIsNotNull(tags, diskOfferingVOMock);
+ }
}
diff --git a/ui/scripts/configuration.js b/ui/scripts/configuration.js
index ae838d9..4ce1f37 100644
--- a/ui/scripts/configuration.js
+++ b/ui/scripts/configuration.js
@@ -2613,7 +2613,8 @@
var data = {
id: args.context.diskOfferings[0].id,
name: args.data.name,
- displaytext: args.data.displaytext
+ displaytext: args.data.displaytext,
+ tags: args.data.tags
};
$.ajax({
url: createURL('updateDiskOffering'),
@@ -2939,7 +2940,41 @@
label: 'label.cache.mode'
},
tags: {
- label: 'label.storage.tags'
+ label: 'label.storage.tags',
+ docID: 'helpPrimaryStorageTags',
+ isEditable: true,
+ isTokenInput: true,
+ dataProvider: function(args) {
+ $.ajax({
+ url: createURL("listStorageTags"),
+ dataType: "json",
+ success: function(json) {
+ var item = json.liststoragetagsresponse.storagetag;
+ var tags = [];
+
+ if (item != null)
+ {
+ tags = $.map(item, function(tag) {
+ return {
+ id: tag.name,
+ name: tag.name
+ };
+ });
+ }
+
+ args.response.success({
+ data: tags,
+ hintText: _l('hint.type.part.storage.tag'),
+ noResultsText: _l('hint.no.storage.tags')
+ });
+ },
+ error: function(XMLHttpResponse) {
+ var errorMsg = parseXMLHttpResponse(XMLHttpResponse);
+
+ args.response.error(errorMsg);
+ }
+ });
+ }
},
domain: {
label: 'label.domain'