Skip to content

Commit ad62573

Browse files
sureshanapartidhslove
sureshanaparti
authored and
dhslove
committed
Improve listing of HA and non-HA hosts when ha.tag setting is defined and hosts have multiple tags along with ha tag (apache#10240)
1 parent a256800 commit ad62573

File tree

3 files changed

+58
-43
lines changed

3 files changed

+58
-43
lines changed

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

Lines changed: 56 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
import com.cloud.org.Managed;
6060
import com.cloud.resource.ResourceState;
6161
import com.cloud.utils.DateUtil;
62+
import com.cloud.utils.StringUtils;
6263
import com.cloud.utils.db.Attribute;
6364
import com.cloud.utils.db.DB;
6465
import com.cloud.utils.db.Filter;
@@ -80,13 +81,13 @@
8081
@TableGenerator(name = "host_req_sq", table = "op_host", pkColumnName = "id", valueColumnName = "sequence", allocationSize = 1)
8182
public class HostDaoImpl extends GenericDaoBase<HostVO, Long> implements HostDao { //FIXME: , ExternalIdDao {
8283

83-
private static final String LIST_HOST_IDS_BY_COMPUTETAGS = "SELECT filtered.host_id, COUNT(filtered.tag) AS tag_count "
84-
+ "FROM (SELECT host_id, tag, is_tag_a_rule FROM host_tags GROUP BY host_id,tag) AS filtered "
85-
+ "WHERE tag IN(%s) AND is_tag_a_rule = 0 "
84+
private static final String LIST_HOST_IDS_BY_HOST_TAGS = "SELECT filtered.host_id, COUNT(filtered.tag) AS tag_count "
85+
+ "FROM (SELECT host_id, tag, is_tag_a_rule FROM host_tags GROUP BY host_id,tag,is_tag_a_rule) AS filtered "
86+
+ "WHERE tag IN (%s) AND (is_tag_a_rule = 0 OR is_tag_a_rule IS NULL) "
8687
+ "GROUP BY host_id "
8788
+ "HAVING tag_count = %s ";
8889
private static final String SEPARATOR = ",";
89-
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";
90+
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";
9091
private static final String GET_HOSTS_OF_ACTIVE_VMS = "select h.id " +
9192
"from vm_instance vm " +
9293
"join host h on (vm.host_id=h.id) " +
@@ -793,7 +794,16 @@ public void markHostsAsDisconnected(long msId, long lastPing) {
793794
}
794795

795796
@Override
796-
public List<HostVO> listByHostTag(Host.Type type, Long clusterId, Long podId, Long dcId, String hostTag) {
797+
public List<HostVO> listByHostTag(Host.Type type, Long clusterId, Long podId, long dcId, String hostTag) {
798+
return listHostsWithOrWithoutHostTags(type, clusterId, podId, dcId, hostTag, true);
799+
}
800+
801+
private List<HostVO> listHostsWithOrWithoutHostTags(Host.Type type, Long clusterId, Long podId, long dcId, String hostTags, boolean withHostTags) {
802+
if (StringUtils.isEmpty(hostTags)) {
803+
logger.debug("Host tags not specified, to list hosts");
804+
return new ArrayList();
805+
}
806+
797807
SearchBuilder<HostVO> hostSearch = createSearchBuilder();
798808
HostVO entity = hostSearch.entity();
799809
hostSearch.and("type", entity.getType(), SearchCriteria.Op.EQ);
@@ -804,7 +814,9 @@ public List<HostVO> listByHostTag(Host.Type type, Long clusterId, Long podId, Lo
804814
hostSearch.and("resourceState", entity.getResourceState(), SearchCriteria.Op.EQ);
805815

806816
SearchCriteria<HostVO> sc = hostSearch.create();
807-
sc.setParameters("type", type.toString());
817+
if (type != null) {
818+
sc.setParameters("type", type.toString());
819+
}
808820
if (podId != null) {
809821
sc.setParameters("pod", podId);
810822
}
@@ -817,27 +829,38 @@ public List<HostVO> listByHostTag(Host.Type type, Long clusterId, Long podId, Lo
817829
sc.setParameters("status", Status.Up.toString());
818830
sc.setParameters("resourceState", ResourceState.Enabled.toString());
819831

820-
List<HostVO> tmpHosts = listBy(sc);
821-
List<HostVO> correctHostsByHostTags = new ArrayList();
822-
List<Long> hostIdsByComputeOffTags = findHostByComputeOfferings(hostTag);
832+
List<HostVO> upAndEnabledHosts = listBy(sc);
833+
if (CollectionUtils.isEmpty(upAndEnabledHosts)) {
834+
return new ArrayList();
835+
}
823836

824-
tmpHosts.forEach((host) -> { if(hostIdsByComputeOffTags.contains(host.getId())) correctHostsByHostTags.add(host);});
837+
List<Long> hostIdsByHostTags = findHostIdsByHostTags(hostTags);
838+
if (CollectionUtils.isEmpty(hostIdsByHostTags)) {
839+
return withHostTags ? new ArrayList() : upAndEnabledHosts;
840+
}
825841

826-
return correctHostsByHostTags;
842+
if (withHostTags) {
843+
List<HostVO> upAndEnabledHostsWithHostTags = new ArrayList();
844+
upAndEnabledHosts.forEach((host) -> { if (hostIdsByHostTags.contains(host.getId())) upAndEnabledHostsWithHostTags.add(host);});
845+
return upAndEnabledHostsWithHostTags;
846+
} else {
847+
List<HostVO> upAndEnabledHostsWithoutHostTags = new ArrayList();
848+
upAndEnabledHosts.forEach((host) -> { if (!hostIdsByHostTags.contains(host.getId())) upAndEnabledHostsWithoutHostTags.add(host);});
849+
return upAndEnabledHostsWithoutHostTags;
850+
}
827851
}
828852

829853
@Override
830854
public List<HostVO> listAllUpAndEnabledNonHAHosts(Type type, Long clusterId, Long podId, long dcId, String haTag) {
855+
if (StringUtils.isNotEmpty(haTag)) {
856+
return listHostsWithOrWithoutHostTags(type, clusterId, podId, dcId, haTag, false);
857+
}
858+
831859
SearchBuilder<HostTagVO> hostTagSearch = _hostTagsDao.createSearchBuilder();
832860
hostTagSearch.and();
833861
hostTagSearch.op("isTagARule", hostTagSearch.entity().getIsTagARule(), Op.EQ);
834862
hostTagSearch.or("tagDoesNotExist", hostTagSearch.entity().getIsTagARule(), Op.NULL);
835863
hostTagSearch.cp();
836-
if (haTag != null && !haTag.isEmpty()) {
837-
hostTagSearch.and().op("tag", hostTagSearch.entity().getTag(), SearchCriteria.Op.NEQ);
838-
hostTagSearch.or("tagNull", hostTagSearch.entity().getTag(), SearchCriteria.Op.NULL);
839-
hostTagSearch.cp();
840-
}
841864

842865
SearchBuilder<HostVO> hostSearch = createSearchBuilder();
843866

@@ -848,18 +871,12 @@ public List<HostVO> listAllUpAndEnabledNonHAHosts(Type type, Long clusterId, Lon
848871
hostSearch.and("status", hostSearch.entity().getStatus(), SearchCriteria.Op.EQ);
849872
hostSearch.and("resourceState", hostSearch.entity().getResourceState(), SearchCriteria.Op.EQ);
850873

851-
852874
hostSearch.join("hostTagSearch", hostTagSearch, hostSearch.entity().getId(), hostTagSearch.entity().getHostId(), JoinBuilder.JoinType.LEFTOUTER);
853875

854-
855876
SearchCriteria<HostVO> sc = hostSearch.create();
856877

857878
sc.setJoinParameters("hostTagSearch", "isTagARule", false);
858879

859-
if (haTag != null && !haTag.isEmpty()) {
860-
sc.setJoinParameters("hostTagSearch", "tag", haTag);
861-
}
862-
863880
if (type != null) {
864881
sc.setParameters("type", type);
865882
}
@@ -1311,19 +1328,19 @@ public List<HostVO> listAllHostsThatHaveNoRuleTag(Type type, Long clusterId, Lon
13111328
}
13121329

13131330
@Override
1314-
public List<Long> listClustersByHostTag(String computeOfferingTags) {
1331+
public List<Long> listClustersByHostTag(String hostTags) {
13151332
TransactionLegacy txn = TransactionLegacy.currentTxn();
1316-
String sql = this.LIST_CLUSTERID_FOR_HOST_TAG;
1333+
String selectStmtToListClusterIdsByHostTags = this.LIST_CLUSTER_IDS_FOR_HOST_TAGS;
13171334
PreparedStatement pstmt = null;
13181335
List<Long> result = new ArrayList();
1319-
List<String> tags = Arrays.asList(computeOfferingTags.split(this.SEPARATOR));
1320-
String subselect = getHostIdsByComputeTags(tags);
1321-
sql = String.format(sql, subselect);
1336+
List<String> tags = Arrays.asList(hostTags.split(this.SEPARATOR));
1337+
String selectStmtToListHostIdsByHostTags = getSelectStmtToListHostIdsByHostTags(tags);
1338+
selectStmtToListClusterIdsByHostTags = String.format(selectStmtToListClusterIdsByHostTags, selectStmtToListHostIdsByHostTags);
13221339

13231340
try {
1324-
pstmt = txn.prepareStatement(sql);
1341+
pstmt = txn.prepareStatement(selectStmtToListClusterIdsByHostTags);
13251342

1326-
for(int i = 0; i < tags.size(); i++){
1343+
for (int i = 0; i < tags.size(); i++){
13271344
pstmt.setString(i+1, tags.get(i));
13281345
}
13291346

@@ -1334,20 +1351,20 @@ public List<Long> listClustersByHostTag(String computeOfferingTags) {
13341351
pstmt.close();
13351352
return result;
13361353
} catch (SQLException e) {
1337-
throw new CloudRuntimeException("DB Exception on: " + sql, e);
1354+
throw new CloudRuntimeException("DB Exception on: " + selectStmtToListClusterIdsByHostTags, e);
13381355
}
13391356
}
13401357

1341-
private List<Long> findHostByComputeOfferings(String computeOfferingTags){
1358+
private List<Long> findHostIdsByHostTags(String hostTags){
13421359
TransactionLegacy txn = TransactionLegacy.currentTxn();
13431360
PreparedStatement pstmt = null;
13441361
List<Long> result = new ArrayList();
1345-
List<String> tags = Arrays.asList(computeOfferingTags.split(this.SEPARATOR));
1346-
String select = getHostIdsByComputeTags(tags);
1362+
List<String> tags = Arrays.asList(hostTags.split(this.SEPARATOR));
1363+
String selectStmtToListHostIdsByHostTags = getSelectStmtToListHostIdsByHostTags(tags);
13471364
try {
1348-
pstmt = txn.prepareStatement(select);
1365+
pstmt = txn.prepareStatement(selectStmtToListHostIdsByHostTags);
13491366

1350-
for(int i = 0; i < tags.size(); i++){
1367+
for (int i = 0; i < tags.size(); i++){
13511368
pstmt.setString(i+1, tags.get(i));
13521369
}
13531370

@@ -1358,7 +1375,7 @@ private List<Long> findHostByComputeOfferings(String computeOfferingTags){
13581375
pstmt.close();
13591376
return result;
13601377
} catch (SQLException e) {
1361-
throw new CloudRuntimeException("DB Exception on: " + select, e);
1378+
throw new CloudRuntimeException("DB Exception on: " + selectStmtToListHostIdsByHostTags, e);
13621379
}
13631380
}
13641381

@@ -1408,10 +1425,10 @@ public List<Long> listSsvmHostsWithPendingMigrateJobsOrderedByJobCount() {
14081425
return result;
14091426
}
14101427

1411-
private String getHostIdsByComputeTags(List<String> offeringTags){
1428+
private String getSelectStmtToListHostIdsByHostTags(List<String> hostTags){
14121429
List<String> questionMarks = new ArrayList();
1413-
offeringTags.forEach((tag) -> { questionMarks.add("?"); });
1414-
return String.format(this.LIST_HOST_IDS_BY_COMPUTETAGS, String.join(",", questionMarks),questionMarks.size());
1430+
hostTags.forEach((tag) -> { questionMarks.add("?"); });
1431+
return String.format(this.LIST_HOST_IDS_BY_HOST_TAGS, String.join(",", questionMarks), questionMarks.size());
14151432
}
14161433

14171434
@Override

‎server/src/main/java/com/cloud/server/ManagementServerImpl.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1966,7 +1966,7 @@ private Pair<List<HostVO>, Integer> searchForServers(final Long startIndex, fina
19661966

19671967
final String haTag = _haMgr.getHaTag();
19681968
SearchBuilder<HostTagVO> hostTagSearch = null;
1969-
if (haHosts != null && haTag != null && !haTag.isEmpty()) {
1969+
if (haHosts != null && StringUtils.isNotEmpty(haTag)) {
19701970
hostTagSearch = _hostTagsDao.createSearchBuilder();
19711971
if ((Boolean)haHosts) {
19721972
hostTagSearch.and().op("tag", hostTagSearch.entity().getTag(), SearchCriteria.Op.EQ);
@@ -2027,7 +2027,7 @@ private Pair<List<HostVO>, Integer> searchForServers(final Long startIndex, fina
20272027
sc.setParameters("resourceState", resourceState);
20282028
}
20292029

2030-
if (haHosts != null && haTag != null && !haTag.isEmpty()) {
2030+
if (haHosts != null && StringUtils.isNotEmpty(haTag)) {
20312031
sc.setJoinParameters("hostTagSearch", "tag", haTag);
20322032
}
20332033

‎server/src/test/java/com/cloud/vm/FirstFitPlannerTest.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,8 +219,6 @@ public void checkClusterListBasedOnHostTag() throws InsufficientServerCapacityEx
219219
}
220220

221221
private List<Long> initializeForClusterListBasedOnHostTag(ServiceOffering offering) {
222-
223-
224222
when(offering.getHostTag()).thenReturn("hosttag1");
225223
initializeForClusterThresholdDisabled();
226224
List<Long> matchingClusters = new ArrayList<>();

0 commit comments

Comments
 (0)