Skip to content

Commit b585802

Browse files
authored
Fix listing service offerings with different host tags (#12919)
1 parent 1ff9eec commit b585802

File tree

7 files changed

+120
-3
lines changed

7 files changed

+120
-3
lines changed

engine/schema/src/main/java/com/cloud/host/dao/HostTagsDao.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,4 +45,9 @@ public interface HostTagsDao extends GenericDao<HostTagVO, Long> {
4545
HostTagResponse newHostTagResponse(HostTagVO hostTag);
4646

4747
List<HostTagVO> searchByIds(Long... hostTagIds);
48+
49+
/**
50+
* List all host tags defined on hosts within a cluster
51+
*/
52+
List<String> listByClusterId(Long clusterId);
4853
}

engine/schema/src/main/java/com/cloud/host/dao/HostTagsDaoImpl.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.apache.cloudstack.framework.config.ConfigKey;
2424
import org.apache.cloudstack.framework.config.Configurable;
2525
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
26+
import org.apache.commons.collections4.CollectionUtils;
2627
import org.apache.commons.lang3.StringUtils;
2728
import org.springframework.stereotype.Component;
2829

@@ -43,9 +44,12 @@ public class HostTagsDaoImpl extends GenericDaoBase<HostTagVO, Long> implements
4344
private final SearchBuilder<HostTagVO> stSearch;
4445
private final SearchBuilder<HostTagVO> tagIdsearch;
4546
private final SearchBuilder<HostTagVO> ImplicitTagsSearch;
47+
private final GenericSearchBuilder<HostTagVO, String> tagSearch;
4648

4749
@Inject
4850
private ConfigurationDao _configDao;
51+
@Inject
52+
private HostDao hostDao;
4953

5054
public HostTagsDaoImpl() {
5155
HostSearch = createSearchBuilder();
@@ -72,6 +76,11 @@ public HostTagsDaoImpl() {
7276
ImplicitTagsSearch.and("hostId", ImplicitTagsSearch.entity().getHostId(), SearchCriteria.Op.EQ);
7377
ImplicitTagsSearch.and("isImplicit", ImplicitTagsSearch.entity().getIsImplicit(), SearchCriteria.Op.EQ);
7478
ImplicitTagsSearch.done();
79+
80+
tagSearch = createSearchBuilder(String.class);
81+
tagSearch.selectFields(tagSearch.entity().getTag());
82+
tagSearch.and("hostIdIN", tagSearch.entity().getHostId(), SearchCriteria.Op.IN);
83+
tagSearch.done();
7584
}
7685

7786
@Override
@@ -235,4 +244,15 @@ public List<HostTagVO> searchByIds(Long... tagIds) {
235244

236245
return tagList;
237246
}
247+
248+
@Override
249+
public List<String> listByClusterId(Long clusterId) {
250+
List<Long> hostIds = hostDao.listIdsByClusterId(clusterId);
251+
if (CollectionUtils.isEmpty(hostIds)) {
252+
return new ArrayList<>();
253+
}
254+
SearchCriteria<String> sc = tagSearch.create();
255+
sc.setParameters("hostIdIN", hostIds.toArray());
256+
return customSearch(sc, null);
257+
}
238258
}

server/src/main/java/com/cloud/api/query/QueryManagerImpl.java

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import com.cloud.storage.dao.StoragePoolAndAccessGroupMapDao;
4646
import com.cloud.cluster.ManagementServerHostPeerJoinVO;
4747

48+
import com.cloud.vm.UserVmManager;
4849
import org.apache.cloudstack.acl.ControlledEntity;
4950
import org.apache.cloudstack.acl.ControlledEntity.ACLType;
5051
import org.apache.cloudstack.acl.SecurityChecker;
@@ -4330,6 +4331,9 @@ private Pair<List<Long>, Integer> searchForServiceOfferingIdsAndCount(ListServic
43304331
List<String> hostTags = new ArrayList<>();
43314332
if (currentVmOffering != null) {
43324333
hostTags.addAll(com.cloud.utils.StringUtils.csvTagsToList(currentVmOffering.getHostTag()));
4334+
if (UserVmManager.AllowDifferentHostTagsOfferingsForVmScale.value()) {
4335+
addVmCurrentClusterHostTags(vmInstance, hostTags);
4336+
}
43334337
}
43344338

43354339
if (!hostTags.isEmpty()) {
@@ -4341,7 +4345,7 @@ private Pair<List<Long>, Integer> searchForServiceOfferingIdsAndCount(ListServic
43414345
flag = false;
43424346
serviceOfferingSearch.op("hostTag" + tag, serviceOfferingSearch.entity().getHostTag(), Op.FIND_IN_SET);
43434347
} else {
4344-
serviceOfferingSearch.and("hostTag" + tag, serviceOfferingSearch.entity().getHostTag(), Op.FIND_IN_SET);
4348+
serviceOfferingSearch.or("hostTag" + tag, serviceOfferingSearch.entity().getHostTag(), Op.FIND_IN_SET);
43454349
}
43464350
}
43474351
serviceOfferingSearch.cp().cp();
@@ -4486,6 +4490,30 @@ private Pair<List<Long>, Integer> searchForServiceOfferingIdsAndCount(ListServic
44864490
return new Pair<>(offeringIds, count);
44874491
}
44884492

4493+
protected void addVmCurrentClusterHostTags(VMInstanceVO vmInstance, List<String> hostTags) {
4494+
if (vmInstance == null) {
4495+
return;
4496+
}
4497+
Long hostId = vmInstance.getHostId() == null ? vmInstance.getLastHostId() : vmInstance.getHostId();
4498+
if (hostId == null) {
4499+
return;
4500+
}
4501+
HostVO host = hostDao.findById(hostId);
4502+
if (host == null) {
4503+
logger.warn("Unable to find host with id " + hostId);
4504+
return;
4505+
}
4506+
List<String> clusterTags = _hostTagDao.listByClusterId(host.getClusterId());
4507+
if (CollectionUtils.isEmpty(clusterTags)) {
4508+
logger.debug("No host tags defined for hosts in the cluster " + host.getClusterId());
4509+
return;
4510+
}
4511+
Set<String> existingTagsSet = new HashSet<>(hostTags);
4512+
clusterTags.stream()
4513+
.filter(tag -> !existingTagsSet.contains(tag))
4514+
.forEach(hostTags::add);
4515+
}
4516+
44894517
@Override
44904518
public ListResponse<ZoneResponse> listDataCenters(ListZonesCmd cmd) {
44914519
Pair<List<DataCenterJoinVO>, Integer> result = listDataCentersInternal(cmd);

server/src/main/java/com/cloud/vm/UserVmManager.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,9 @@ public interface UserVmManager extends UserVmService {
108108
"Comma separated list of allowed additional VM settings if VM instance settings are read from OVA.",
109109
true, ConfigKey.Scope.Zone, null, null, null, null, null, ConfigKey.Kind.CSV, null);
110110

111+
ConfigKey<Boolean> AllowDifferentHostTagsOfferingsForVmScale = new ConfigKey<>("Advanced", Boolean.class, "allow.different.host.tags.offerings.for.vm.scale", "false",
112+
"Enables/Disable allowing to change a VM offering to offerings with different host tags", true);
113+
111114
static final int MAX_USER_DATA_LENGTH_BYTES = 2048;
112115

113116
public static final String CKS_NODE = "cksnode";

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9412,7 +9412,7 @@ public ConfigKey<?>[] getConfigKeys() {
94129412
VmIpFetchThreadPoolMax, VmIpFetchTaskWorkers, AllowDeployVmIfGivenHostFails, EnableAdditionalVmConfig, DisplayVMOVFProperties,
94139413
KvmAdditionalConfigAllowList, XenServerAdditionalConfigAllowList, VmwareAdditionalConfigAllowList, DestroyRootVolumeOnVmDestruction,
94149414
EnforceStrictResourceLimitHostTagCheck, StrictHostTags, AllowUserForceStopVm, VmDistinctHostNameScope,
9415-
VmwareAdditionalDetailsFromOvaEnabled, VmwareAllowedAdditionalDetailsFromOva};
9415+
VmwareAdditionalDetailsFromOvaEnabled, VmwareAllowedAdditionalDetailsFromOva, AllowDifferentHostTagsOfferingsForVmScale};
94169416
}
94179417

94189418
@Override

server/src/test/java/com/cloud/api/query/QueryManagerImplTest.java

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
package com.cloud.api.query;
1919

2020
import static org.junit.Assert.assertEquals;
21+
import static org.junit.Assert.assertTrue;
2122
import static org.mockito.ArgumentMatchers.any;
2223
import static org.mockito.Mockito.mock;
2324
import static org.mockito.Mockito.verify;
@@ -33,6 +34,7 @@
3334
import java.util.UUID;
3435
import java.util.stream.Collectors;
3536

37+
import com.cloud.host.dao.HostTagsDao;
3638
import org.apache.cloudstack.acl.SecurityChecker;
3739
import org.apache.cloudstack.api.ApiCommandResourceType;
3840
import org.apache.cloudstack.api.ResponseObject;
@@ -156,6 +158,9 @@ public class QueryManagerImplTest {
156158
@Mock
157159
HostDao hostDao;
158160

161+
@Mock
162+
HostTagsDao hostTagsDao;
163+
159164
@Mock
160165
ClusterDao clusterDao;
161166

@@ -622,4 +627,39 @@ public void updateHostsExtensions_withHostResponses_setsExtension() {
622627
verify(host1).setExtensionId("a");
623628
verify(host2).setExtensionId("b");
624629
}
630+
631+
@Test
632+
public void testAddVmCurrentClusterHostTags() {
633+
String tag1 = "tag1";
634+
String tag2 = "tag2";
635+
VMInstanceVO vmInstance = mock(VMInstanceVO.class);
636+
HostVO host = mock(HostVO.class);
637+
when(vmInstance.getHostId()).thenReturn(null);
638+
when(vmInstance.getLastHostId()).thenReturn(1L);
639+
when(hostDao.findById(1L)).thenReturn(host);
640+
when(host.getClusterId()).thenReturn(1L);
641+
when(hostTagsDao.listByClusterId(1L)).thenReturn(Arrays.asList(tag1, tag2));
642+
643+
List<String> hostTags = new ArrayList<>(Collections.singleton(tag1));
644+
queryManagerImplSpy.addVmCurrentClusterHostTags(vmInstance, hostTags);
645+
assertEquals(2, hostTags.size());
646+
assertTrue(hostTags.contains(tag2));
647+
}
648+
649+
@Test
650+
public void testAddVmCurrentClusterHostTagsEmptyHostTagsInCluster() {
651+
String tag1 = "tag1";
652+
VMInstanceVO vmInstance = mock(VMInstanceVO.class);
653+
HostVO host = mock(HostVO.class);
654+
when(vmInstance.getHostId()).thenReturn(null);
655+
when(vmInstance.getLastHostId()).thenReturn(1L);
656+
when(hostDao.findById(1L)).thenReturn(host);
657+
when(host.getClusterId()).thenReturn(1L);
658+
when(hostTagsDao.listByClusterId(1L)).thenReturn(null);
659+
660+
List<String> hostTags = new ArrayList<>(Collections.singleton(tag1));
661+
queryManagerImplSpy.addVmCurrentClusterHostTags(vmInstance, hostTags);
662+
assertEquals(1, hostTags.size());
663+
assertTrue(hostTags.contains(tag1));
664+
}
625665
}

ui/src/views/compute/wizard/ComputeOfferingSelection.vue

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@
4141
<template #headerCell="{ column }">
4242
<template v-if="column.key === 'cpu'"><appstore-outlined /> {{ $t('label.cpu') }}</template>
4343
<template v-if="column.key === 'ram'"><bulb-outlined /> {{ $t('label.memory') }}</template>
44+
<template v-if="column.key === 'hosttags'"><tag-outlined /> {{ $t('label.hosttags') }}</template>
45+
<template v-if="column.key === 'storagetags'"><tag-outlined /> {{ $t('label.storagetags') }}</template>
4446
<template v-if="column.key === 'gpu'"><font-awesome-icon
4547
:icon="['fa-solid', 'fa-microchip']"
4648
class="anticon"
@@ -197,6 +199,22 @@ export default {
197199
})
198200
}
199201
202+
if (this.computeItems.some(item => item.hosttags !== undefined && item.hosttags !== null)) {
203+
baseColumns.push({
204+
key: 'hosttags',
205+
dataIndex: 'hosttags',
206+
width: '30%'
207+
})
208+
}
209+
210+
if (this.computeItems.some(item => item.storagetags !== undefined && item.storagetags !== null)) {
211+
baseColumns.push({
212+
key: 'storagetags',
213+
dataIndex: 'storagetags',
214+
width: '30%'
215+
})
216+
}
217+
200218
return baseColumns
201219
},
202220
tableSource () {
@@ -256,6 +274,7 @@ export default {
256274
}
257275
gpuValue = gpuCount + ' x ' + gpuType
258276
}
277+
259278
return {
260279
key: item.id,
261280
name: item.name,
@@ -267,7 +286,9 @@ export default {
267286
gpuCount: gpuCount,
268287
gpuType: gpuType,
269288
gpu: gpuValue,
270-
gpuDetails: this.getGpuDetails(item)
289+
gpuDetails: this.getGpuDetails(item),
290+
hosttags: item.hosttags !== undefined && item.hosttags !== null ? item.hosttags : undefined,
291+
storagetags: item.storagetags !== undefined && item.storagetags !== null ? item.storagetags : undefined
271292
}
272293
})
273294
},

0 commit comments

Comments
 (0)