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 2021/08/02 13:48:34 UTC

[cloudstack] branch main updated: Allow updating the storage/host tags of service offerings (#5043)

This is an automated email from the ASF dual-hosted git repository.

dahn pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudstack.git


The following commit(s) were added to refs/heads/main by this push:
     new d6a77a7  Allow updating the storage/host tags of service offerings (#5043)
d6a77a7 is described below

commit d6a77a72f00d01e82b536c2b9d43c00690ee96fd
Author: slavkap <51...@users.noreply.github.com>
AuthorDate: Mon Aug 2 16:48:07 2021 +0300

    Allow updating the storage/host tags of service offerings (#5043)
---
 .../org/apache/cloudstack/api/ApiConstants.java    |  1 +
 .../admin/offering/UpdateServiceOfferingCmd.java   | 24 ++++++
 .../api/response/ServiceOfferingResponse.java      |  2 +-
 .../src/main/java/com/cloud/host/dao/HostDao.java  |  2 +
 .../main/java/com/cloud/host/dao/HostDaoImpl.java  | 25 ++++++
 .../storage/datastore/db/PrimaryDataStoreDao.java  |  1 +
 .../datastore/db/PrimaryDataStoreDaoImpl.java      | 26 +++++++
 .../configuration/ConfigurationManagerImpl.java    | 88 +++++++++++++++-------
 .../configuration/ConfigurationManagerTest.java    | 12 +--
 test/integration/smoke/test_service_offerings.py   | 15 ++++
 ui/src/config/section/offering.js                  |  4 +-
 ui/src/views/AutogenView.vue                       |  6 +-
 12 files changed, 165 insertions(+), 41 deletions(-)

diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
index b060b5a..6d64fe9 100644
--- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
+++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java
@@ -357,6 +357,7 @@ public class ApiConstants {
     public static final String SWAP_OWNER = "swapowner";
     public static final String SYSTEM_VM_TYPE = "systemvmtype";
     public static final String TAGS = "tags";
+    public static final String STORAGE_TAGS = "storagetags";
     public static final String TARGET_IQN = "targetiqn";
     public static final String TEMPLATE_FILTER = "templatefilter";
     public static final String TEMPLATE_ID = "templateid";
diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateServiceOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateServiceOfferingCmd.java
index 43a0666..4212a00 100644
--- a/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateServiceOfferingCmd.java
+++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateServiceOfferingCmd.java
@@ -19,12 +19,14 @@ 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;
 import org.apache.cloudstack.api.BaseCmd;
 import org.apache.cloudstack.api.Parameter;
 import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.BaseCmd.CommandType;
 import org.apache.cloudstack.api.response.ServiceOfferingResponse;
 import org.apache.log4j.Logger;
 
@@ -71,6 +73,20 @@ public class UpdateServiceOfferingCmd extends BaseCmd {
             since = "4.13")
     private String zoneIds;
 
+    @Parameter(name = ApiConstants.STORAGE_TAGS,
+            type = CommandType.STRING,
+            description = "comma-separated list of tags for the service offering, tags should match with existing storage pool tags",
+            authorized = {RoleType.Admin},
+            since = "4.16")
+    private String storageTags;
+
+    @Parameter(name = ApiConstants.HOST_TAGS,
+            type = CommandType.STRING,
+            description = "the host tag for this service offering.",
+            authorized = {RoleType.Admin},
+            since = "4.16")
+    private String hostTags;
+
     /////////////////////////////////////////////////////
     /////////////////// Accessors ///////////////////////
     /////////////////////////////////////////////////////
@@ -151,6 +167,14 @@ public class UpdateServiceOfferingCmd extends BaseCmd {
         return validZoneIds;
     }
 
+    public String getStorageTags() {
+        return storageTags;
+    }
+
+    public String getHostTags() {
+        return hostTags;
+    }
+
     /////////////////////////////////////////////////////
     /////////////// API Implementation///////////////////
     /////////////////////////////////////////////////////
diff --git a/api/src/main/java/org/apache/cloudstack/api/response/ServiceOfferingResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/ServiceOfferingResponse.java
index 59bd72c..42f89b1 100644
--- a/api/src/main/java/org/apache/cloudstack/api/response/ServiceOfferingResponse.java
+++ b/api/src/main/java/org/apache/cloudstack/api/response/ServiceOfferingResponse.java
@@ -76,7 +76,7 @@ public class ServiceOfferingResponse extends BaseResponse {
     @Param(description = "true if the vm needs to be volatile, i.e., on every reboot of vm from API root disk is discarded and creates a new root disk")
     private Boolean isVolatile;
 
-    @SerializedName("tags")
+    @SerializedName("storagetags")
     @Param(description = "the tags for the service offering")
     private String tags;
 
diff --git a/engine/schema/src/main/java/com/cloud/host/dao/HostDao.java b/engine/schema/src/main/java/com/cloud/host/dao/HostDao.java
index af06fcc..12207da 100644
--- a/engine/schema/src/main/java/com/cloud/host/dao/HostDao.java
+++ b/engine/schema/src/main/java/com/cloud/host/dao/HostDao.java
@@ -138,4 +138,6 @@ public interface HostDao extends GenericDao<HostVO, Long>, StateDao<Status, Stat
     List<HostVO> listByClusterAndHypervisorType(long clusterId, HypervisorType hypervisorType);
 
     HostVO findByName(String name);
+
+    List<HostVO> listHostsWithActiveVMs(long offeringId);
 }
diff --git a/engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java b/engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java
index e58df1d..b19f717 100644
--- a/engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java
+++ b/engine/schema/src/main/java/com/cloud/host/dao/HostDaoImpl.java
@@ -79,6 +79,10 @@ public class HostDaoImpl extends GenericDaoBase<HostVO, Long> implements HostDao
     private static final Logger state_logger = Logger.getLogger(ResourceState.class);
 
     private static final String LIST_CLUSTERID_FOR_HOST_TAG = "select distinct cluster_id from host join host_tags on host.id = host_tags.host_id and host_tags.tag = ?";
+    private static final String GET_HOSTS_OF_ACTIVE_VMS = "select h.id " +
+            "from vm_instance vm " +
+            "join host h on (vm.host_id=h.id) " +
+            "where vm.service_offering_id= ? and vm.state not in (\"Destroyed\", \"Expunging\", \"Error\") group by h.id";
 
     protected SearchBuilder<HostVO> TypePodDcStatusSearch;
 
@@ -1198,6 +1202,27 @@ public class HostDaoImpl extends GenericDaoBase<HostVO, Long> implements HostDao
     }
 
     @Override
+    public List<HostVO> listHostsWithActiveVMs(long offeringId) {
+        TransactionLegacy txn = TransactionLegacy.currentTxn();
+        PreparedStatement pstmt = null;
+        List<HostVO> result = new ArrayList<>();
+        StringBuilder sql = new StringBuilder(GET_HOSTS_OF_ACTIVE_VMS);
+        try {
+            pstmt = txn.prepareAutoCloseStatement(sql.toString());
+            pstmt.setLong(1, offeringId);
+            ResultSet rs = pstmt.executeQuery();
+            while (rs.next()) {
+                result.add(toEntityBean(rs, false));
+            }
+            return result;
+        } catch (SQLException e) {
+            throw new CloudRuntimeException("DB Exception on: " + sql, e);
+        } catch (Throwable e) {
+            throw new CloudRuntimeException("Caught: " + sql, e);
+        }
+    }
+
+    @Override
     public List<HostVO> listAllHostsByType(Host.Type type) {
         SearchCriteria<HostVO> sc = TypeSearch.create();
         sc.setParameters("type", type);
diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDao.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDao.java
index f56fdb8..80fddc9 100644
--- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDao.java
+++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDao.java
@@ -132,4 +132,5 @@ public interface PrimaryDataStoreDao extends GenericDao<StoragePoolVO, Long> {
 
     List<StoragePoolVO> findPoolsByStorageType(String storageType);
 
+    List<StoragePoolVO> listStoragePoolsWithActiveVolumesByOfferingId(long offeringid);
 }
diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java
index 6b07ef9..2ab95bb 100644
--- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java
+++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java
@@ -74,6 +74,11 @@ public class PrimaryDataStoreDaoImpl extends GenericDaoBase<StoragePoolVO, Long>
     protected final String TagsSqlPrefix = "SELECT storage_pool.* from storage_pool LEFT JOIN storage_pool_tags ON storage_pool.id = storage_pool_tags.pool_id WHERE storage_pool.removed is null and storage_pool.status = 'Up' and storage_pool.data_center_id = ? and (storage_pool.pod_id = ? or storage_pool.pod_id is null) and storage_pool.scope = ? and (";
     protected final String TagsSqlSuffix = ") GROUP BY storage_pool_tags.pool_id HAVING COUNT(storage_pool_tags.tag) >= ?";
 
+    private static final String GET_STORAGE_POOLS_OF_VOLUMES_WITHOUT_OR_NOT_HAVING_TAGS = "select s.id " +
+            "from volumes vol " +
+            "join storage_pool s on vol.pool_id=s.id " +
+            "where vol.disk_offering_id= ? and vol.state not in (\"Destroy\", \"Error\", \"Expunging\") group by s.id";
+
     /**
      * Used in method findPoolsByDetailsOrTagsInternal
      */
@@ -589,4 +594,25 @@ public class PrimaryDataStoreDaoImpl extends GenericDaoBase<StoragePoolVO, Long>
         sc.setParameters("poolType", storageType);
         return listBy(sc);
     }
+
+    @Override
+    public List<StoragePoolVO> listStoragePoolsWithActiveVolumesByOfferingId(long offeringId) {
+        TransactionLegacy txn = TransactionLegacy.currentTxn();
+        PreparedStatement pstmt = null;
+        List<StoragePoolVO> result = new ArrayList<>();
+        StringBuilder sql = new StringBuilder(GET_STORAGE_POOLS_OF_VOLUMES_WITHOUT_OR_NOT_HAVING_TAGS);
+        try {
+            pstmt = txn.prepareAutoCloseStatement(sql.toString());
+            pstmt.setLong(1, offeringId);
+            ResultSet rs = pstmt.executeQuery();
+            while (rs.next()) {
+                result.add(toEntityBean(rs, false));
+            }
+            return result;
+        } catch (SQLException e) {
+            throw new CloudRuntimeException("DB Exception on: " + sql, e);
+        } catch (Throwable e) {
+            throw new CloudRuntimeException("Caught: " + sql, e);
+        }
+    }
 }
diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
index 319f797..d06f748 100755
--- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
+++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
@@ -161,7 +161,9 @@ import com.cloud.exception.PermissionDeniedException;
 import com.cloud.exception.ResourceAllocationException;
 import com.cloud.exception.ResourceUnavailableException;
 import com.cloud.gpu.GPU;
+import com.cloud.host.HostVO;
 import com.cloud.host.dao.HostDao;
+import com.cloud.host.dao.HostTagsDao;
 import com.cloud.hypervisor.Hypervisor.HypervisorType;
 import com.cloud.network.IpAddressManager;
 import com.cloud.network.Network;
@@ -211,6 +213,7 @@ import com.cloud.storage.Storage.ProvisioningType;
 import com.cloud.storage.StorageManager;
 import com.cloud.storage.Volume;
 import com.cloud.storage.dao.DiskOfferingDao;
+import com.cloud.storage.dao.StoragePoolTagsDao;
 import com.cloud.storage.dao.VMTemplateZoneDao;
 import com.cloud.storage.dao.VolumeDao;
 import com.cloud.test.IPRangeConfig;
@@ -392,6 +395,10 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
     private VMTemplateZoneDao templateZoneDao;
     @Inject
     VsphereStoragePolicyDao vsphereStoragePolicyDao;
+    @Inject
+    HostTagsDao hostTagDao;
+    @Inject
+    StoragePoolTagsDao storagePoolTagDao;
 
 
     // FIXME - why don't we have interface for DataCenterLinkLocalIpAddressDao?
@@ -2706,6 +2713,8 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
         Long userId = CallContext.current().getCallingUserId();
         final List<Long> domainIds = cmd.getDomainIds();
         final List<Long> zoneIds = cmd.getZoneIds();
+        String storageTags = cmd.getStorageTags();
+        String hostTags = cmd.getHostTags();
 
         if (userId == null) {
             userId = Long.valueOf(User.UID_SYSTEM);
@@ -2787,7 +2796,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
             throw new InvalidParameterValueException(String.format("Unable to update service offering: %s by id user: %s because it is not root-admin or domain-admin", offeringHandle.getUuid(), user.getUuid()));
         }
 
-        final boolean updateNeeded = name != null || displayText != null || sortKey != null;
+        final boolean updateNeeded = name != null || displayText != null || sortKey != null || storageTags != null || hostTags != null;
         final boolean detailsUpdateNeeded = !filteredDomainIds.equals(existingDomainIds) || !filteredZoneIds.equals(existingZoneIds);
         if (!updateNeeded && !detailsUpdateNeeded) {
             return _serviceOfferingDao.findById(id);
@@ -2807,29 +2816,9 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
             offering.setSortKey(sortKey);
         }
 
-        // 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() && offeringHandle.getTags() == null)
-        // {
-        // //no new tags; no existing tags
-        // offering.setTagsArray(csvTagsToList(null));
-        // }
-        // else if (!tags.trim().isEmpty() && offeringHandle.getTags() != null)
-        // {
-        // //new tags + existing tags
-        // List<String> oldTags = csvTagsToList(offeringHandle.getTags());
-        // List<String> newTags = csvTagsToList(tags);
-        // oldTags.addAll(newTags);
-        // offering.setTagsArray(oldTags);
-        // }
-        // else if(!tags.trim().isEmpty())
-        // {
-        // //new tags; NO existing tags
-        // offering.setTagsArray(csvTagsToList(tags));
-        // }
-        // }
+        updateOfferingTagsIfIsNotNull(storageTags, offering);
+
+        updateServiceOfferingHostTagsIfNotNull(hostTags, offering);
 
         if (updateNeeded && !_serviceOfferingDao.update(id, offering)) {
             return null;
@@ -3284,7 +3273,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
             diskOffering.setDisplayOffering(displayDiskOffering);
         }
 
-        updateDiskOfferingTagsIfIsNotNull(tags, diskOffering);
+        updateOfferingTagsIfIsNotNull(tags, diskOffering);
 
         validateMaxRateEqualsOrGreater(iopsReadRate, iopsReadRateMax, IOPS_READ_RATE);
         validateMaxRateEqualsOrGreater(iopsWriteRate, iopsWriteRateMax, IOPS_WRITE_RATE);
@@ -3336,16 +3325,27 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
     }
 
     /**
-     * Check the tags parameters to the diskOffering
+     * Check the tags parameters to the disk/service offering
      * <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>
+     *     <li>If tags is not null, will set tag to the disk/service offering if the pools with active volumes have the new tags.</li>
+     *     <li>If tags is an blank string, set null on disk/service offering tag.</li>
      * </ul>
      */
-    protected void updateDiskOfferingTagsIfIsNotNull(String tags, DiskOfferingVO diskOffering) {
+    protected void updateOfferingTagsIfIsNotNull(String tags, DiskOfferingVO diskOffering) {
         if (tags == null) { return; }
         if (StringUtils.isNotBlank(tags)) {
+            tags = StringUtils.cleanupTags(tags);
+            List<StoragePoolVO> pools = _storagePoolDao.listStoragePoolsWithActiveVolumesByOfferingId(diskOffering.getId());
+            if (CollectionUtils.isNotEmpty(pools)) {
+                List<String> listOfTags = Arrays.asList(tags.split(","));
+                for (StoragePoolVO storagePoolVO : pools) {
+                    List<String> tagsOnPool = storagePoolTagDao.getStoragePoolTags(storagePoolVO.getId());
+                    if (CollectionUtils.isEmpty(tagsOnPool) || !tagsOnPool.containsAll(listOfTags)) {
+                        throw new InvalidParameterValueException(String.format("There are active volumes using offering [%s], and the pools [%s] don't have the new tags", diskOffering.getId(), pools));
+                    }
+                }
+            }
             diskOffering.setTags(tags);
         } else {
             diskOffering.setTags(null);
@@ -3353,6 +3353,36 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati
     }
 
     /**
+     * Check the host tags parameters to the service offering
+     * <ul>
+     *     <li>If host tags is null, do nothing and return.</li>
+     *     <li>If host tags is not null, will set host tag to the service offering if the hosts with active VMs have the new tags.</li>
+     *     <li>If host tags is an blank string, set null on service offering tag.</li>
+     * </ul>
+     */
+    protected void updateServiceOfferingHostTagsIfNotNull(String hostTags, ServiceOfferingVO offering) {
+        if (hostTags == null) {
+            return;
+        }
+        if (StringUtils.isNotBlank(hostTags)) {
+            hostTags = StringUtils.cleanupTags(hostTags);
+            List<HostVO> hosts = _hostDao.listHostsWithActiveVMs(offering.getId());
+            if (CollectionUtils.isNotEmpty(hosts)) {
+                List<String> listOfHostTags = Arrays.asList(hostTags.split(","));
+                for (HostVO host : hosts) {
+                    List<String> tagsOnHost = hostTagDao.gethostTags(host.getId());
+                    if (CollectionUtils.isEmpty(tagsOnHost) || !tagsOnHost.containsAll(listOfHostTags)) {
+                        throw new InvalidParameterValueException(String.format("There are active VMs using offering [%s], and the hosts [%s] don't have the new tags", offering.getId(), hosts));
+                    }
+                }
+            }
+            offering.setHostTag(hostTags);
+        } else {
+            offering.setHostTag(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
      */
diff --git a/server/src/test/java/com/cloud/configuration/ConfigurationManagerTest.java b/server/src/test/java/com/cloud/configuration/ConfigurationManagerTest.java
index 3d18d1a..f8155f1 100644
--- a/server/src/test/java/com/cloud/configuration/ConfigurationManagerTest.java
+++ b/server/src/test/java/com/cloud/configuration/ConfigurationManagerTest.java
@@ -991,15 +991,15 @@ public class ConfigurationManagerTest {
 
     @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);
+        Mockito.doNothing().when(configurationMgr).updateOfferingTagsIfIsNotNull(null, diskOfferingVOMock);
+        this.configurationMgr.updateOfferingTagsIfIsNotNull(null, diskOfferingVOMock);
+        Mockito.verify(configurationMgr, Mockito.times(1)).updateOfferingTagsIfIsNotNull(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);
+        Mockito.doNothing().when(configurationMgr).updateOfferingTagsIfIsNotNull(tags, diskOfferingVOMock);
+        this.configurationMgr.updateOfferingTagsIfIsNotNull(tags, diskOfferingVOMock);
+        Mockito.verify(configurationMgr, Mockito.times(1)).updateOfferingTagsIfIsNotNull(tags, diskOfferingVOMock);
     }
 }
diff --git a/test/integration/smoke/test_service_offerings.py b/test/integration/smoke/test_service_offerings.py
index 8a7682e..3a942a1 100644
--- a/test/integration/smoke/test_service_offerings.py
+++ b/test/integration/smoke/test_service_offerings.py
@@ -414,6 +414,8 @@ class TestServiceOfferings(cloudstackTestCase):
         # Generate new name & displaytext from random data
         random_displaytext = random_gen()
         random_name = random_gen()
+        random_tag = random_gen()
+        random_hosttag = random_gen()
 
         self.debug("Updating service offering with ID: %s" %
                    self.service_offering_1.id)
@@ -423,6 +425,8 @@ class TestServiceOfferings(cloudstackTestCase):
         cmd.id = self.service_offering_1.id
         cmd.displaytext = random_displaytext
         cmd.name = random_name
+        cmd.storagetags = random_tag
+        cmd.hosttags = random_hosttag
         self.apiclient.updateServiceOffering(cmd)
 
         list_service_response = list_service_offering(
@@ -452,6 +456,17 @@ class TestServiceOfferings(cloudstackTestCase):
             "Check server name in updateServiceOffering"
         )
 
+        self.assertEqual(
+            list_service_response[0].storagetags,
+            random_tag,
+            "Check storage tags in updateServiceOffering"
+        )
+
+        self.assertEqual(
+            list_service_response[0].hosttags,
+            random_hosttag,
+            "Check host tags in updateServiceOffering"
+        )
         return
 
     @attr(
diff --git a/ui/src/config/section/offering.js b/ui/src/config/section/offering.js
index 3bd1ab9..9e22d91 100644
--- a/ui/src/config/section/offering.js
+++ b/ui/src/config/section/offering.js
@@ -31,7 +31,7 @@ export default {
       params: { isrecursive: 'true' },
       columns: ['name', 'displaytext', 'cpunumber', 'cpuspeed', 'memory', 'domain', 'zone', 'order'],
       details: () => {
-        var fields = ['name', 'id', 'displaytext', 'offerha', 'provisioningtype', 'storagetype', 'iscustomized', 'iscustomizediops', 'limitcpuuse', 'cpunumber', 'cpuspeed', 'memory', 'hosttags', 'tags', 'domain', 'zone', 'created', 'dynamicscalingenabled']
+        var fields = ['name', 'id', 'displaytext', 'offerha', 'provisioningtype', 'storagetype', 'iscustomized', 'iscustomizediops', 'limitcpuuse', 'cpunumber', 'cpuspeed', 'memory', 'hosttags', 'storagetags', 'domain', 'zone', 'created', 'dynamicscalingenabled']
         if (store.getters.apis.createServiceOffering &&
           store.getters.apis.createServiceOffering.params.filter(x => x.name === 'storagepolicy').length > 0) {
           fields.splice(6, 0, 'vspherestoragepolicy')
@@ -61,7 +61,7 @@ export default {
         label: 'label.edit',
         docHelp: 'adminguide/service_offerings.html#modifying-or-deleting-a-service-offering',
         dataView: true,
-        args: ['name', 'displaytext']
+        args: ['name', 'displaytext', 'storagetags', 'hosttags']
       }, {
         api: 'updateServiceOffering',
         icon: 'lock',
diff --git a/ui/src/views/AutogenView.vue b/ui/src/views/AutogenView.vue
index 229cd21..24a966d 100644
--- a/ui/src/views/AutogenView.vue
+++ b/ui/src/views/AutogenView.vue
@@ -931,7 +931,7 @@ export default {
 
       this.showAction = true
       for (const param of this.currentAction.paramFields) {
-        if (param.type === 'list' && ['tags', 'hosttags'].includes(param.name)) {
+        if (param.type === 'list' && ['tags', 'hosttags', 'storagetags'].includes(param.name)) {
           param.type = 'string'
         }
         if (param.type === 'uuid' || param.type === 'list' || param.name === 'account' || (this.currentAction.mapping && param.name in this.currentAction.mapping)) {
@@ -1214,13 +1214,13 @@ export default {
               continue
             }
             if (input === undefined || input === null ||
-              (input === '' && !['updateStoragePool', 'updateHost', 'updatePhysicalNetwork', 'updateDiskOffering', 'updateNetworkOffering'].includes(action.api))) {
+              (input === '' && !['updateStoragePool', 'updateHost', 'updatePhysicalNetwork', 'updateDiskOffering', 'updateNetworkOffering', 'updateServiceOffering'].includes(action.api))) {
               if (param.type === 'boolean') {
                 params[key] = false
               }
               break
             }
-            if (input === '' && !['tags'].includes(key)) {
+            if (input === '' && !['tags', 'hosttags', 'storagetags'].includes(key)) {
               break
             }
             if (action.mapping && key in action.mapping && action.mapping[key].options) {