Allow updating the storage/host tags of service offerings (#5043)

This commit is contained in:
slavkap 2021-08-02 16:48:07 +03:00 committed by GitHub
parent 82df04ecc8
commit d6a77a72f0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 165 additions and 41 deletions

View File

@ -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";

View File

@ -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///////////////////
/////////////////////////////////////////////////////

View File

@ -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;

View File

@ -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);
}

View File

@ -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;
@ -1197,6 +1201,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();

View File

@ -132,4 +132,5 @@ public interface PrimaryDataStoreDao extends GenericDao<StoragePoolVO, Long> {
List<StoragePoolVO> findPoolsByStorageType(String storageType);
List<StoragePoolVO> listStoragePoolsWithActiveVolumesByOfferingId(long offeringid);
}

View File

@ -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);
}
}
}

View File

@ -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,22 +3325,63 @@ 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);
}
}
/**
* 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

View File

@ -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);
}
}

View File

@ -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(

View File

@ -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',

View File

@ -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) {