PR multi tags in compute offering [#4398] (#4399)

* [#4398] adapt code to handle multi tag string with commas

* [#4398] remove trailing spaces

* [#4398] add multi host tag support for ingest process

* [#4398] add test for multi tag support in offerings

* [#4398]  update multitag support for DeploymentPlanningManagerImpl

encapsulate multi tag check from Ingest Feature, DepolymentPlanningManager into
HostDaoImpl to prevent code duplicates

* [#4398] move logic to HostVO and add tests

* rename test method

* [#4398] Change string method to apaches StringUtils

* [#4398] modify test for multi tag support

* adapt sql for double tags

Co-authored-by: Dirk Klahre <[email protected]>

Author: DK101010

engine/schema/src/main/java/com/cloud/host/HostVO.java

@@ -40,10 +40,13 @@
 
 import com.cloud.agent.api.VgpuTypesInfo;
 import com.cloud.hypervisor.Hypervisor.HypervisorType;
+import com.cloud.offering.ServiceOffering;
 import com.cloud.resource.ResourceState;
 import com.cloud.storage.Storage.StoragePoolType;
 import com.cloud.utils.NumbersUtil;
 import com.cloud.utils.db.GenericDao;
+import java.util.Arrays;
+import org.apache.commons.lang.StringUtils;
 
 @Entity
 @Table(name = "host")
@@ -740,6 +743,18 @@ public void setUuid(String uuid) {
         this.uuid = uuid;
     }
 
+    public boolean checkHostServiceOfferingTags(ServiceOffering serviceOffering){
+        if (serviceOffering == null) {
+            return false;
+        }
+        if (StringUtils.isEmpty(serviceOffering.getHostTag())) {
+            return true;
+        }
+
+        List<String> serviceOfferingTags = Arrays.asList(serviceOffering.getHostTag().split(","));
+        return this.getHostTags() != null && this.getHostTags().containsAll(serviceOfferingTags);
+    }
+
     @Override
     public PartitionType partitionType() {
         return PartitionType.Host;

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

@@ -70,6 +70,7 @@
 import com.cloud.utils.db.TransactionLegacy;
 import com.cloud.utils.db.UpdateBuilder;
 import com.cloud.utils.exception.CloudRuntimeException;
+import java.util.Arrays;
 
 @DB
 @TableGenerator(name = "host_req_sq", table = "op_host", pkColumnName = "id", valueColumnName = "sequence", allocationSize = 1)
@@ -78,12 +79,19 @@ public class HostDaoImpl extends GenericDaoBase<HostVO, Long> implements HostDao
     private static final Logger status_logger = Logger.getLogger(Status.class);
     private static final Logger state_logger = Logger.getLogger(ResourceState.class);
 
+    private static final String LIST_HOST_IDS_BY_COMPUTETAGS = "SELECT filtered.host_id, COUNT(filtered.tag) AS tag_count "
+                                                             + "FROM (SELECT host_id, tag FROM host_tags GROUP BY host_id,tag) AS filtered "
+                                                             + "WHERE tag IN(%s) "
+                                                             + "GROUP BY host_id "
+                                                             + "HAVING tag_count = %s ";
+    private static final String SEPARATOR = ",";
     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;
 
     protected SearchBuilder<HostVO> IdStatusSearch;
@@ -736,11 +744,6 @@ public void markHostsAsDisconnected(long msId, long lastPing) {
 
     @Override
     public List<HostVO> listByHostTag(Host.Type type, Long clusterId, Long podId, long dcId, String hostTag) {
-
-        SearchBuilder<HostTagVO> hostTagSearch = _hostTagsDao.createSearchBuilder();
-        HostTagVO tagEntity = hostTagSearch.entity();
-        hostTagSearch.and("tag", tagEntity.getTag(), SearchCriteria.Op.EQ);
-
         SearchBuilder<HostVO> hostSearch = createSearchBuilder();
         HostVO entity = hostSearch.entity();
         hostSearch.and("type", entity.getType(), SearchCriteria.Op.EQ);
@@ -749,10 +752,8 @@ public List<HostVO> listByHostTag(Host.Type type, Long clusterId, Long podId, lo
         hostSearch.and("cluster", entity.getClusterId(), SearchCriteria.Op.EQ);
         hostSearch.and("status", entity.getStatus(), SearchCriteria.Op.EQ);
         hostSearch.and("resourceState", entity.getResourceState(), SearchCriteria.Op.EQ);
-        hostSearch.join("hostTagSearch", hostTagSearch, entity.getId(), tagEntity.getHostId(), JoinBuilder.JoinType.INNER);
 
         SearchCriteria<HostVO> sc = hostSearch.create();
-        sc.setJoinParameters("hostTagSearch", "tag", hostTag);
         sc.setParameters("type", type.toString());
         if (podId != null) {
             sc.setParameters("pod", podId);
@@ -764,7 +765,13 @@ public List<HostVO> listByHostTag(Host.Type type, Long clusterId, Long podId, lo
         sc.setParameters("status", Status.Up.toString());
         sc.setParameters("resourceState", ResourceState.Enabled.toString());
 
-        return listBy(sc);
+        List<HostVO> tmpHosts = listBy(sc);
+        List<HostVO> correctHostsByHostTags = new ArrayList();
+        List<Long> hostIdsByComputeOffTags = findHostByComputeOfferings(hostTag);
+
+        tmpHosts.forEach((host) -> { if(hostIdsByComputeOffTags.contains(host.getId())) correctHostsByHostTags.add(host);});
+
+        return correctHostsByHostTags;
     }
 
     @Override
@@ -1179,28 +1186,69 @@ public List<HostVO> listAllHostsByZoneAndHypervisorType(long zoneId, HypervisorT
     }
 
     @Override
-    public List<Long> listClustersByHostTag(String hostTagOnOffering) {
+    public List<Long> listClustersByHostTag(String computeOfferingTags) {
         TransactionLegacy txn = TransactionLegacy.currentTxn();
+        String sql = this.LIST_CLUSTERID_FOR_HOST_TAG;
         PreparedStatement pstmt = null;
-        List<Long> result = new ArrayList<Long>();
-        StringBuilder sql = new StringBuilder(LIST_CLUSTERID_FOR_HOST_TAG);
-        // during listing the clusters that cross the threshold
-        // we need to check with disabled thresholds of each cluster if not defined at cluster consider the global value
+        List<Long> result = new ArrayList();
+        List<String> tags = Arrays.asList(computeOfferingTags.split(this.SEPARATOR));
+        String subselect = getHostIdsByComputeTags(tags);
+        sql = String.format(sql, subselect);
+
         try {
-            pstmt = txn.prepareAutoCloseStatement(sql.toString());
-            pstmt.setString(1, hostTagOnOffering);
+            pstmt = txn.prepareStatement(sql);
+
+            for(int i = 0; i < tags.size(); i++){
+                pstmt.setString(i+1, tags.get(i));
+            }
+
             ResultSet rs = pstmt.executeQuery();
             while (rs.next()) {
                 result.add(rs.getLong(1));
             }
+            pstmt.close();
+            if(result.isEmpty()){
+                throw new CloudRuntimeException("No suitable host found for follow compute offering tags: " + computeOfferingTags);
+            }
             return result;
         } catch (SQLException e) {
             throw new CloudRuntimeException("DB Exception on: " + sql, e);
-        } catch (Throwable e) {
-            throw new CloudRuntimeException("Caught: " + sql, e);
         }
     }
 
+    private List<Long> findHostByComputeOfferings(String computeOfferingTags){
+        TransactionLegacy txn = TransactionLegacy.currentTxn();
+        PreparedStatement pstmt = null;
+        List<Long> result = new ArrayList();
+        List<String> tags = Arrays.asList(computeOfferingTags.split(this.SEPARATOR));
+        String select = getHostIdsByComputeTags(tags);
+        try {
+            pstmt = txn.prepareStatement(select);
+
+            for(int i = 0; i < tags.size(); i++){
+                pstmt.setString(i+1, tags.get(i));
+            }
+
+            ResultSet rs = pstmt.executeQuery();
+            while (rs.next()) {
+                result.add(rs.getLong(1));
+            }
+            pstmt.close();
+            if(result.isEmpty()){
+                throw new CloudRuntimeException("No suitable host found for follow compute offering tags: " + computeOfferingTags);
+            }
+            return result;
+        } catch (SQLException e) {
+            throw new CloudRuntimeException("DB Exception on: " + select, e);
+        }
+    }
+
+    private String getHostIdsByComputeTags(List<String> offeringTags){
+        List<String> questionMarks = new ArrayList();
+        offeringTags.forEach((tag) -> { questionMarks.add("?"); });
+        return String.format(this.LIST_HOST_IDS_BY_COMPUTETAGS, String.join(",", questionMarks),questionMarks.size());
+    }
+
     @Override
     public List<HostVO> listHostsWithActiveVMs(long offeringId) {
         TransactionLegacy txn = TransactionLegacy.currentTxn();

engine/schema/src/test/java/com/cloud/host/HostVOTest.java

@@ -0,0 +1,61 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package com.cloud.host;
+
+import com.cloud.service.ServiceOfferingVO;
+import com.cloud.storage.Storage.ProvisioningType;
+import com.cloud.vm.VirtualMachine;
+import java.util.Arrays;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import org.junit.Test;
+import org.junit.Before;
+
+public class HostVOTest {
+    HostVO host;
+    ServiceOfferingVO offering;
+
+    @Before
+    public void setUp() throws Exception {
+        host = new HostVO();
+        offering = new ServiceOfferingVO("TestSO", 0, 0, 0, 0, 0, false, "TestSO", ProvisioningType.THIN, true, true,"",false,VirtualMachine.Type.User,false);
+    }
+
+    @Test
+    public void testNoSO() {
+        assertFalse(host.checkHostServiceOfferingTags(null));
+    }
+
+    @Test
+    public void testNoTag() {
+        assertTrue(host.checkHostServiceOfferingTags(offering));
+    }
+
+    @Test
+    public void testRightTag() {
+        host.setHostTags(Arrays.asList("tag1","tag2"));
+        offering.setHostTag("tag2,tag1");
+        assertTrue(host.checkHostServiceOfferingTags(offering));
+    }
+
+    @Test
+    public void testWrongTag() {
+        host.setHostTags(Arrays.asList("tag1","tag2"));
+        offering.setHostTag("tag2,tag4");
+        assertFalse(host.checkHostServiceOfferingTags(offering));
+    }
+}

server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java

@@ -678,7 +678,7 @@ private boolean checkVmProfileAndHost(final VirtualMachineProfile vmProfile, fin
         ServiceOffering offering = vmProfile.getServiceOffering();
         if (offering.getHostTag() != null) {
             _hostDao.loadHostTags(host);
-            if (!(host.getHostTags() != null && host.getHostTags().contains(offering.getHostTag()))) {
+            if (!host.checkHostServiceOfferingTags(offering)) {
                 s_logger.debug("Service Offering host tag does not match the last host of this VM");
                 return false;
             }

server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java

@@ -379,17 +379,8 @@ private List<String> getHostManagedVms(Host host) {
     }
 
     private boolean hostSupportsServiceOffering(HostVO host, ServiceOffering serviceOffering) {
-        if (host == null) {
-            return false;
-        }
-        if (serviceOffering == null) {
-            return false;
-        }
-        if (Strings.isNullOrEmpty(serviceOffering.getHostTag())) {
-            return true;
-        }
         hostDao.loadHostTags(host);
-        return host.getHostTags() != null && host.getHostTags().contains(serviceOffering.getHostTag());
+        return host.checkHostServiceOfferingTags(serviceOffering);
     }
 
     private boolean storagePoolSupportsDiskOffering(StoragePool pool, DiskOffering diskOffering) {

server/src/test/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImplTest.java

@@ -82,6 +82,7 @@
 import com.cloud.host.Host;
 import com.cloud.host.HostVO;
 import com.cloud.host.Status;
+import com.cloud.host.dao.HostDao;
 import com.cloud.hypervisor.Hypervisor;
 import com.cloud.network.Network;
 import com.cloud.network.NetworkModel;
@@ -180,6 +181,8 @@ public class UnmanagedVMsManagerImplTest {
     private UserVmDao userVmDao;
     @Mock
     private NicDao nicDao;
+    @Mock
+    private HostDao hostDao;
 
     @Mock
     private VMInstanceVO virtualMachine;
@@ -235,6 +238,7 @@ public void setUp() throws Exception {
         HostVO hostVO = Mockito.mock(HostVO.class);
         when(hostVO.isInMaintenanceStates()).thenReturn(false);
         hosts.add(hostVO);
+        when(hostVO.checkHostServiceOfferingTags(Mockito.any())).thenReturn(true);
         when(resourceManager.listHostsInClusterByStatus(Mockito.anyLong(), Mockito.any(Status.class))).thenReturn(hosts);
         List<VMTemplateStoragePoolVO> templates = new ArrayList<>();
         when(templatePoolDao.listAll()).thenReturn(templates);
@@ -368,6 +372,7 @@ public void importUnmanagedInstanceTest() {
         when(importUnmanageInstanceCmd.getName()).thenReturn("TestInstance");
         when(importUnmanageInstanceCmd.getAccountName()).thenReturn(null);
         when(importUnmanageInstanceCmd.getDomainId()).thenReturn(null);
+        doNothing().when(hostDao).loadHostTags(null);
         PowerMockito.mockStatic(UsageEventUtils.class);
         unmanagedVMsManager.importUnmanagedInstance(importUnmanageInstanceCmd);
     }