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

dahn pushed a commit to branch 
revert-10240-ha-and-non-ha-hosts-listing-issue-with-multiple-tags
in repository https://gitbox.apache.org/repos/asf/cloudstack.git

commit 53573c111134eb7400990896dc8dbb51e8eef6b8
Author: dahn <daan.hoogl...@gmail.com>
AuthorDate: Mon Feb 3 10:34:01 2025 +0100

    Revert "Improve listing of HA and non-HA hosts when ha.tag setting is 
defined…"
    
    This reverts commit 0d5047b8b730758bfabfe183c02d284e4db3260b.
---
 .../main/java/com/cloud/host/dao/HostDaoImpl.java  | 93 +++++++++-------------
 .../com/cloud/server/ManagementServerImpl.java     |  4 +-
 .../java/com/cloud/vm/FirstFitPlannerTest.java     |  2 +
 3 files changed, 42 insertions(+), 57 deletions(-)

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 4f0ae3cce9c..da829a5fe24 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
@@ -60,7 +60,6 @@ import com.cloud.org.Grouping;
 import com.cloud.org.Managed;
 import com.cloud.resource.ResourceState;
 import com.cloud.utils.DateUtil;
-import com.cloud.utils.StringUtils;
 import com.cloud.utils.db.Attribute;
 import com.cloud.utils.db.DB;
 import com.cloud.utils.db.Filter;
@@ -84,13 +83,13 @@ 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_HOST_TAGS = "SELECT 
filtered.host_id, COUNT(filtered.tag) AS tag_count "
-                                                             + "FROM (SELECT 
host_id, tag, is_tag_a_rule FROM host_tags GROUP BY host_id,tag,is_tag_a_rule) 
AS filtered "
-                                                             + "WHERE tag IN 
(%s) AND (is_tag_a_rule = 0 OR is_tag_a_rule IS NULL) "
+    private static final String LIST_HOST_IDS_BY_COMPUTETAGS = "SELECT 
filtered.host_id, COUNT(filtered.tag) AS tag_count "
+                                                             + "FROM (SELECT 
host_id, tag, is_tag_a_rule FROM host_tags GROUP BY host_id,tag) AS filtered "
+                                                             + "WHERE tag 
IN(%s) AND is_tag_a_rule = 0 "
                                                              + "GROUP BY 
host_id "
                                                              + "HAVING 
tag_count = %s ";
     private static final String SEPARATOR = ",";
-    private static final String LIST_CLUSTER_IDS_FOR_HOST_TAGS = "select 
distinct cluster_id from host join ( %s ) AS selected_hosts ON host.id = 
selected_hosts.host_id";
+    private static final String LIST_CLUSTERID_FOR_HOST_TAG = "select distinct 
cluster_id from host join ( %s ) AS selected_hosts ON host.id = 
selected_hosts.host_id";
     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) " +
@@ -786,15 +785,6 @@ public class HostDaoImpl extends GenericDaoBase<HostVO, 
Long> implements HostDao
 
     @Override
     public List<HostVO> listByHostTag(Host.Type type, Long clusterId, Long 
podId, long dcId, String hostTag) {
-        return listHostsWithOrWithoutHostTags(type, clusterId, podId, dcId, 
hostTag, true);
-    }
-
-    private List<HostVO> listHostsWithOrWithoutHostTags(Host.Type type, Long 
clusterId, Long podId, long dcId, String hostTags, boolean withHostTags) {
-        if (StringUtils.isEmpty(hostTags)) {
-            s_logger.debug("Host tags not specified, to list hosts");
-            return new ArrayList();
-        }
-
         SearchBuilder<HostVO> hostSearch = createSearchBuilder();
         HostVO entity = hostSearch.entity();
         hostSearch.and("type", entity.getType(), SearchCriteria.Op.EQ);
@@ -805,9 +795,7 @@ public class HostDaoImpl extends GenericDaoBase<HostVO, 
Long> implements HostDao
         hostSearch.and("resourceState", entity.getResourceState(), 
SearchCriteria.Op.EQ);
 
         SearchCriteria<HostVO> sc = hostSearch.create();
-        if (type != null) {
-            sc.setParameters("type", type.toString());
-        }
+        sc.setParameters("type", type.toString());
         if (podId != null) {
             sc.setParameters("pod", podId);
         }
@@ -818,38 +806,27 @@ public class HostDaoImpl extends GenericDaoBase<HostVO, 
Long> implements HostDao
         sc.setParameters("status", Status.Up.toString());
         sc.setParameters("resourceState", ResourceState.Enabled.toString());
 
-        List<HostVO> upAndEnabledHosts = listBy(sc);
-        if (CollectionUtils.isEmpty(upAndEnabledHosts)) {
-            return new ArrayList();
-        }
+        List<HostVO> tmpHosts = listBy(sc);
+        List<HostVO> correctHostsByHostTags = new ArrayList();
+        List<Long> hostIdsByComputeOffTags = 
findHostByComputeOfferings(hostTag);
 
-        List<Long> hostIdsByHostTags = findHostIdsByHostTags(hostTags);
-        if (CollectionUtils.isEmpty(hostIdsByHostTags)) {
-            return withHostTags ? new ArrayList() : upAndEnabledHosts;
-        }
+        tmpHosts.forEach((host) -> { 
if(hostIdsByComputeOffTags.contains(host.getId())) 
correctHostsByHostTags.add(host);});
 
-        if (withHostTags) {
-            List<HostVO> upAndEnabledHostsWithHostTags = new ArrayList();
-            upAndEnabledHosts.forEach((host) -> { if 
(hostIdsByHostTags.contains(host.getId())) 
upAndEnabledHostsWithHostTags.add(host);});
-            return upAndEnabledHostsWithHostTags;
-        } else {
-            List<HostVO> upAndEnabledHostsWithoutHostTags = new ArrayList();
-            upAndEnabledHosts.forEach((host) -> { if 
(!hostIdsByHostTags.contains(host.getId())) 
upAndEnabledHostsWithoutHostTags.add(host);});
-            return upAndEnabledHostsWithoutHostTags;
-        }
+        return correctHostsByHostTags;
     }
 
     @Override
     public List<HostVO> listAllUpAndEnabledNonHAHosts(Type type, Long 
clusterId, Long podId, long dcId, String haTag) {
-        if (StringUtils.isNotEmpty(haTag)) {
-            return listHostsWithOrWithoutHostTags(type, clusterId, podId, 
dcId, haTag, false);
-        }
-
         SearchBuilder<HostTagVO> hostTagSearch = 
_hostTagsDao.createSearchBuilder();
         hostTagSearch.and();
         hostTagSearch.op("isTagARule", hostTagSearch.entity().getIsTagARule(), 
Op.EQ);
         hostTagSearch.or("tagDoesNotExist", 
hostTagSearch.entity().getIsTagARule(), Op.NULL);
         hostTagSearch.cp();
+        if (haTag != null && !haTag.isEmpty()) {
+            hostTagSearch.and().op("tag", hostTagSearch.entity().getTag(), 
SearchCriteria.Op.NEQ);
+            hostTagSearch.or("tagNull", hostTagSearch.entity().getTag(), 
SearchCriteria.Op.NULL);
+            hostTagSearch.cp();
+        }
 
         SearchBuilder<HostVO> hostSearch = createSearchBuilder();
 
@@ -860,12 +837,18 @@ public class HostDaoImpl extends GenericDaoBase<HostVO, 
Long> implements HostDao
         hostSearch.and("status", hostSearch.entity().getStatus(), 
SearchCriteria.Op.EQ);
         hostSearch.and("resourceState", 
hostSearch.entity().getResourceState(), SearchCriteria.Op.EQ);
 
+
         hostSearch.join("hostTagSearch", hostTagSearch, 
hostSearch.entity().getId(), hostTagSearch.entity().getHostId(), 
JoinBuilder.JoinType.LEFTOUTER);
 
+
         SearchCriteria<HostVO> sc = hostSearch.create();
 
         sc.setJoinParameters("hostTagSearch", "isTagARule", false);
 
+        if (haTag != null && !haTag.isEmpty()) {
+            sc.setJoinParameters("hostTagSearch", "tag", haTag);
+        }
+
         if (type != null) {
             sc.setParameters("type", type);
         }
@@ -1317,19 +1300,19 @@ public class HostDaoImpl extends GenericDaoBase<HostVO, 
Long> implements HostDao
     }
 
     @Override
-    public List<Long> listClustersByHostTag(String hostTags) {
+    public List<Long> listClustersByHostTag(String computeOfferingTags) {
         TransactionLegacy txn = TransactionLegacy.currentTxn();
-        String selectStmtToListClusterIdsByHostTags = 
this.LIST_CLUSTER_IDS_FOR_HOST_TAGS;
+        String sql = this.LIST_CLUSTERID_FOR_HOST_TAG;
         PreparedStatement pstmt = null;
         List<Long> result = new ArrayList();
-        List<String> tags = Arrays.asList(hostTags.split(this.SEPARATOR));
-        String selectStmtToListHostIdsByHostTags = 
getSelectStmtToListHostIdsByHostTags(tags);
-        selectStmtToListClusterIdsByHostTags = 
String.format(selectStmtToListClusterIdsByHostTags, 
selectStmtToListHostIdsByHostTags);
+        List<String> tags = 
Arrays.asList(computeOfferingTags.split(this.SEPARATOR));
+        String subselect = getHostIdsByComputeTags(tags);
+        sql = String.format(sql, subselect);
 
         try {
-            pstmt = txn.prepareStatement(selectStmtToListClusterIdsByHostTags);
+            pstmt = txn.prepareStatement(sql);
 
-            for (int i = 0; i < tags.size(); i++){
+            for(int i = 0; i < tags.size(); i++){
                 pstmt.setString(i+1, tags.get(i));
             }
 
@@ -1340,20 +1323,20 @@ public class HostDaoImpl extends GenericDaoBase<HostVO, 
Long> implements HostDao
             pstmt.close();
             return result;
         } catch (SQLException e) {
-            throw new CloudRuntimeException("DB Exception on: " + 
selectStmtToListClusterIdsByHostTags, e);
+            throw new CloudRuntimeException("DB Exception on: " + sql, e);
         }
     }
 
-    private List<Long> findHostIdsByHostTags(String hostTags){
+    private List<Long> findHostByComputeOfferings(String computeOfferingTags){
         TransactionLegacy txn = TransactionLegacy.currentTxn();
         PreparedStatement pstmt = null;
         List<Long> result = new ArrayList();
-        List<String> tags = Arrays.asList(hostTags.split(this.SEPARATOR));
-        String selectStmtToListHostIdsByHostTags = 
getSelectStmtToListHostIdsByHostTags(tags);
+        List<String> tags = 
Arrays.asList(computeOfferingTags.split(this.SEPARATOR));
+        String select = getHostIdsByComputeTags(tags);
         try {
-            pstmt = txn.prepareStatement(selectStmtToListHostIdsByHostTags);
+            pstmt = txn.prepareStatement(select);
 
-            for (int i = 0; i < tags.size(); i++){
+            for(int i = 0; i < tags.size(); i++){
                 pstmt.setString(i+1, tags.get(i));
             }
 
@@ -1364,7 +1347,7 @@ public class HostDaoImpl extends GenericDaoBase<HostVO, 
Long> implements HostDao
             pstmt.close();
             return result;
         } catch (SQLException e) {
-            throw new CloudRuntimeException("DB Exception on: " + 
selectStmtToListHostIdsByHostTags, e);
+            throw new CloudRuntimeException("DB Exception on: " + select, e);
         }
     }
 
@@ -1389,10 +1372,10 @@ public class HostDaoImpl extends GenericDaoBase<HostVO, 
Long> implements HostDao
         return new ArrayList<>(result);
     }
 
-    private String getSelectStmtToListHostIdsByHostTags(List<String> hostTags){
+    private String getHostIdsByComputeTags(List<String> offeringTags){
         List<String> questionMarks = new ArrayList();
-        hostTags.forEach((tag) -> { questionMarks.add("?"); });
-        return String.format(this.LIST_HOST_IDS_BY_HOST_TAGS, String.join(",", 
questionMarks), questionMarks.size());
+        offeringTags.forEach((tag) -> { questionMarks.add("?"); });
+        return String.format(this.LIST_HOST_IDS_BY_COMPUTETAGS, 
String.join(",", questionMarks),questionMarks.size());
     }
 
     @Override
diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java 
b/server/src/main/java/com/cloud/server/ManagementServerImpl.java
index 7ccd6b5e289..c032019510f 100644
--- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java
+++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java
@@ -1919,7 +1919,7 @@ public class ManagementServerImpl extends ManagerBase 
implements ManagementServe
 
         final String haTag = _haMgr.getHaTag();
         SearchBuilder<HostTagVO> hostTagSearch = null;
-        if (haHosts != null && StringUtils.isNotEmpty(haTag)) {
+        if (haHosts != null && haTag != null && !haTag.isEmpty()) {
             hostTagSearch = _hostTagsDao.createSearchBuilder();
             if ((Boolean)haHosts) {
                 hostTagSearch.and().op("tag", hostTagSearch.entity().getTag(), 
SearchCriteria.Op.EQ);
@@ -1980,7 +1980,7 @@ public class ManagementServerImpl extends ManagerBase 
implements ManagementServe
             sc.setParameters("resourceState", resourceState);
         }
 
-        if (haHosts != null && StringUtils.isNotEmpty(haTag)) {
+        if (haHosts != null && haTag != null && !haTag.isEmpty()) {
             sc.setJoinParameters("hostTagSearch", "tag", haTag);
         }
 
diff --git a/server/src/test/java/com/cloud/vm/FirstFitPlannerTest.java 
b/server/src/test/java/com/cloud/vm/FirstFitPlannerTest.java
index 5b795ac6a34..eecd7cc2d35 100644
--- a/server/src/test/java/com/cloud/vm/FirstFitPlannerTest.java
+++ b/server/src/test/java/com/cloud/vm/FirstFitPlannerTest.java
@@ -217,6 +217,8 @@ public class FirstFitPlannerTest {
     }
 
     private List<Long> initializeForClusterListBasedOnHostTag(ServiceOffering 
offering) {
+
+
         when(offering.getHostTag()).thenReturn("hosttag1");
         initializeForClusterThresholdDisabled();
         List<Long> matchingClusters = new ArrayList<>();

Reply via email to