Fix listing service offerings with different host tags

Author: nvazquez

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

@@ -45,4 +45,6 @@ public interface HostTagsDao extends GenericDao<HostTagVO, Long> {
     HostTagResponse newHostTagResponse(HostTagVO hostTag);
 
     List<HostTagVO> searchByIds(Long... hostTagIds);
+
+    List<String> listByClusterId(Long clusterId);
 }

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

@@ -24,6 +24,7 @@
 import org.apache.cloudstack.framework.config.Configurable;
 import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
 import org.apache.commons.lang3.StringUtils;
+import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
 
 import com.cloud.host.HostTagVO;
@@ -43,9 +44,12 @@ public class HostTagsDaoImpl extends GenericDaoBase<HostTagVO, Long> implements
     private final SearchBuilder<HostTagVO> stSearch;
     private final SearchBuilder<HostTagVO> tagIdsearch;
     private final SearchBuilder<HostTagVO> ImplicitTagsSearch;
+    private final GenericSearchBuilder<HostTagVO, String> tagSearch;
 
     @Inject
     private ConfigurationDao _configDao;
+    @Autowired
+    private HostDao hostDao;
 
     public HostTagsDaoImpl() {
         HostSearch = createSearchBuilder();
@@ -72,6 +76,11 @@ public HostTagsDaoImpl() {
         ImplicitTagsSearch.and("hostId", ImplicitTagsSearch.entity().getHostId(), SearchCriteria.Op.EQ);
         ImplicitTagsSearch.and("isImplicit", ImplicitTagsSearch.entity().getIsImplicit(), SearchCriteria.Op.EQ);
         ImplicitTagsSearch.done();
+
+        tagSearch = createSearchBuilder(String.class);
+        tagSearch.selectFields(tagSearch.entity().getTag());
+        tagSearch.and("idIN", tagSearch.entity().getId(), SearchCriteria.Op.IN);
+        tagSearch.done();
     }
 
     @Override
@@ -235,4 +244,12 @@ public List<HostTagVO> searchByIds(Long... tagIds) {
 
         return tagList;
     }
+
+    @Override
+    public List<String> listByClusterId(Long clusterId) {
+        List<Long> hostIds = hostDao.listIdsByClusterId(clusterId);
+        SearchCriteria<String> sc = tagSearch.create();
+        sc.setParameters("idIN", hostIds.toArray());
+        return customSearch(sc, null);
+    }
 }

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

@@ -4330,6 +4330,7 @@ private Pair<List<Long>, Integer> searchForServiceOfferingIdsAndCount(ListServic
         List<String> hostTags = new ArrayList<>();
         if (currentVmOffering != null) {
             hostTags.addAll(com.cloud.utils.StringUtils.csvTagsToList(currentVmOffering.getHostTag()));
+            addVmCurrentClusterHostTags(vmInstance, hostTags);
         }
 
         if (!hostTags.isEmpty()) {
@@ -4341,7 +4342,7 @@ private Pair<List<Long>, Integer> searchForServiceOfferingIdsAndCount(ListServic
                     flag = false;
                     serviceOfferingSearch.op("hostTag" + tag, serviceOfferingSearch.entity().getHostTag(), Op.FIND_IN_SET);
                 } else {
-                    serviceOfferingSearch.and("hostTag" + tag, serviceOfferingSearch.entity().getHostTag(), Op.FIND_IN_SET);
+                    serviceOfferingSearch.or("hostTag" + tag, serviceOfferingSearch.entity().getHostTag(), Op.FIND_IN_SET);
                 }
             }
             serviceOfferingSearch.cp().cp();
@@ -4486,6 +4487,30 @@ private Pair<List<Long>, Integer> searchForServiceOfferingIdsAndCount(ListServic
         return new Pair<>(offeringIds, count);
     }
 
+    protected void addVmCurrentClusterHostTags(VMInstanceVO vmInstance, List<String> hostTags) {
+        if (vmInstance == null) {
+            return;
+        }
+        Long hostId = vmInstance.getHostId() == null ? vmInstance.getLastHostId() : vmInstance.getHostId();
+        if (hostId == null) {
+            return;
+        }
+        HostVO host = hostDao.findById(hostId);
+        if (host == null) {
+            logger.warn("Unable to find host with id " + hostId);
+            return;
+        }
+        List<String> clusterTags = _hostTagDao.listByClusterId(host.getClusterId());
+        if (CollectionUtils.isEmpty(clusterTags)) {
+            logger.warn("No host tags defined for hosts in the cluster " + host.getClusterId());
+            return;
+        }
+        Set<String> existingTagsSet = new HashSet<>(hostTags);
+        clusterTags.stream()
+                .filter(tag -> !existingTagsSet.contains(tag))
+                .forEach(hostTags::add);
+    }
+
     @Override
     public ListResponse<ZoneResponse> listDataCenters(ListZonesCmd cmd) {
         Pair<List<DataCenterJoinVO>, Integer> result = listDataCentersInternal(cmd);

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

@@ -18,6 +18,7 @@
 package com.cloud.api.query;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
@@ -33,6 +34,7 @@
 import java.util.UUID;
 import java.util.stream.Collectors;
 
+import com.cloud.host.dao.HostTagsDao;
 import org.apache.cloudstack.acl.SecurityChecker;
 import org.apache.cloudstack.api.ApiCommandResourceType;
 import org.apache.cloudstack.api.ResponseObject;
@@ -156,6 +158,9 @@ public class QueryManagerImplTest {
     @Mock
     HostDao hostDao;
 
+    @Mock
+    HostTagsDao hostTagsDao;
+
     @Mock
     ClusterDao clusterDao;
 
@@ -622,4 +627,39 @@ public void updateHostsExtensions_withHostResponses_setsExtension() {
         verify(host1).setExtensionId("a");
         verify(host2).setExtensionId("b");
     }
+
+    @Test
+    public void testAddVmCurrentClusterHostTags() {
+        String tag1 = "tag1";
+        String tag2 = "tag2";
+        VMInstanceVO vmInstance = mock(VMInstanceVO.class);
+        HostVO host = mock(HostVO.class);
+        when(vmInstance.getHostId()).thenReturn(null);
+        when(vmInstance.getLastHostId()).thenReturn(1L);
+        when(hostDao.findById(1L)).thenReturn(host);
+        when(host.getClusterId()).thenReturn(1L);
+        when(hostTagsDao.listByClusterId(1L)).thenReturn(Arrays.asList(tag1, tag2));
+
+        List<String> hostTags = new ArrayList<>(Collections.singleton(tag1));
+        queryManagerImplSpy.addVmCurrentClusterHostTags(vmInstance, hostTags);
+        assertEquals(2, hostTags.size());
+        assertTrue(hostTags.contains(tag2));
+    }
+
+    @Test
+    public void testAddVmCurrentClusterHostTagsEmptyHostTagsInCluster() {
+        String tag1 = "tag1";
+        VMInstanceVO vmInstance = mock(VMInstanceVO.class);
+        HostVO host = mock(HostVO.class);
+        when(vmInstance.getHostId()).thenReturn(null);
+        when(vmInstance.getLastHostId()).thenReturn(1L);
+        when(hostDao.findById(1L)).thenReturn(host);
+        when(host.getClusterId()).thenReturn(1L);
+        when(hostTagsDao.listByClusterId(1L)).thenReturn(null);
+
+        List<String> hostTags = new ArrayList<>(Collections.singleton(tag1));
+        queryManagerImplSpy.addVmCurrentClusterHostTags(vmInstance, hostTags);
+        assertEquals(1, hostTags.size());
+        assertTrue(hostTags.contains(tag1));
+    }
 }