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<>();