Skip to content

Commit cc79edf

Browse files
shwstpprshwstppr
committed
address review comments
Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
1 parent e89925c commit cc79edf

10 files changed

Lines changed: 138 additions & 93 deletions

File tree

‎api/src/main/java/com/cloud/event/EventTypes.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -908,7 +908,7 @@ public class EventTypes {
908908
public static final String EVENT_INSTANCE_BOOT_GROUP_REBOOT = "INSTANCE.BOOT.GROUP.REBOOT";
909909
public static final String EVENT_INSTANCE_BOOT_GROUP_MEMBER_ADD = "INSTANCE.BOOT.GROUP.MEMBER.ADD";
910910
public static final String EVENT_INSTANCE_BOOT_GROUP_MEMBER_REMOVE = "INSTANCE.BOOT.GROUP.MEMBER.REMOVE";
911-
public static final String EVENT_INSTANCE_BOOT_GROUP_MEMBER_REORDER = "INSTANCE.BOOT.GROUP.MEMBER.REODER";
911+
public static final String EVENT_INSTANCE_BOOT_GROUP_MEMBER_REORDER = "INSTANCE.BOOT.GROUP.MEMBER.REORDER";
912912
public static final String EVENT_INSTANCE_BOOT_GROUP_READINESS_RULE_CREATE = "INSTANCE.BOOT.GROUP.READINESS.RULE.CREATE";
913913
public static final String EVENT_INSTANCE_BOOT_GROUP_READINESS_RULE_UPDATE = "INSTANCE.BOOT.GROUP.READINESS.RULE.UPDATE";
914914
public static final String EVENT_INSTANCE_BOOT_GROUP_READINESS_RULE_DELETE = "INSTANCE.BOOT.GROUP.READINESS.RULE.DELETE";

‎api/src/main/java/org/apache/cloudstack/api/command/user/bootgroup/CreateInstanceBootGroupReadinessRuleCmd.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ public class CreateInstanceBootGroupReadinessRuleCmd extends BaseCmd implements
6565
private Long instanceGroupId;
6666

6767
@Parameter(name = ApiConstants.RULE_TYPE, type = CommandType.STRING, required = true,
68-
description = "The readiness rule type: GuestAgentLiveness, Ping, PortCheck, InstanceQuorum or CustomScript")
68+
description = "The readiness rule type: GuestAgentLiveness, Ping, PortCheck, MemberQuorum or CustomScript")
6969
private String ruleType;
7070

7171
@Parameter(name = ApiConstants.NAME, type = CommandType.STRING, description = "The name of the readiness rule; auto-generated if omitted")

‎engine/schema/src/main/java/com/cloud/vm/dao/InstanceBootGroupReadinessRuleDao.java‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,21 +21,24 @@
2121

2222
import org.apache.cloudstack.vm.bootgroup.InstanceBootGroupMember;
2323
import org.apache.cloudstack.vm.bootgroup.InstanceBootGroupReadinessRuleVO;
24+
import org.apache.cloudstack.vm.bootgroup.readiness.InstanceBootGroupReadinessRule;
2425

26+
import com.cloud.utils.Pair;
2527
import com.cloud.utils.db.GenericDao;
2628

2729
public interface InstanceBootGroupReadinessRuleDao extends GenericDao<InstanceBootGroupReadinessRuleVO, Long> {
2830

29-
List<InstanceBootGroupReadinessRuleVO> listByBootGroupId(long bootGroupId);
31+
Pair<List<InstanceBootGroupReadinessRuleVO>, Integer> searchAndCountByBootGroupId(long bootGroupId,
32+
Long id,
33+
InstanceBootGroupMember.MemberType itemType,
34+
Long itemId,
35+
InstanceBootGroupReadinessRule.RuleType ruleType,
36+
String keyword,
37+
Long startIndex,
38+
Long pageSize);
3039

3140
List<InstanceBootGroupReadinessRuleVO> listEnabledByItem(long bootGroupId, InstanceBootGroupMember.MemberType itemType, long itemId);
3241

3342
List<InstanceBootGroupReadinessRuleVO> listByItem(long bootGroupId, InstanceBootGroupMember.MemberType itemType, long itemId);
3443

35-
/**
36-
* Cleanup for a member removed from its boot group, or a VM leaving its Instance Group — there's
37-
* no FK path for this (item_id doesn't reference instance_boot_group_member/instance_group_vm_map),
38-
* so it's enforced here in code instead of a DB cascade.
39-
*/
40-
void deleteByItem(InstanceBootGroupMember.MemberType itemType, long itemId);
4144
}

‎engine/schema/src/main/java/com/cloud/vm/dao/InstanceBootGroupReadinessRuleDaoImpl.java‎

Lines changed: 39 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,24 +21,23 @@
2121

2222
import org.apache.cloudstack.vm.bootgroup.InstanceBootGroupMember;
2323
import org.apache.cloudstack.vm.bootgroup.InstanceBootGroupReadinessRuleVO;
24+
import org.apache.cloudstack.vm.bootgroup.readiness.InstanceBootGroupReadinessRule;
2425
import org.springframework.stereotype.Component;
2526

27+
import com.cloud.utils.Pair;
28+
import com.cloud.utils.db.Filter;
2629
import com.cloud.utils.db.GenericDaoBase;
2730
import com.cloud.utils.db.SearchBuilder;
2831
import com.cloud.utils.db.SearchCriteria;
2932

3033
@Component
3134
public class InstanceBootGroupReadinessRuleDaoImpl extends GenericDaoBase<InstanceBootGroupReadinessRuleVO, Long> implements InstanceBootGroupReadinessRuleDao {
3235

33-
private final SearchBuilder<InstanceBootGroupReadinessRuleVO> bootGroupSearch;
3436
private final SearchBuilder<InstanceBootGroupReadinessRuleVO> itemSearch;
3537
private final SearchBuilder<InstanceBootGroupReadinessRuleVO> enabledByItemSearch;
3638
private final SearchBuilder<InstanceBootGroupReadinessRuleVO> byItemSearch;
3739

3840
public InstanceBootGroupReadinessRuleDaoImpl() {
39-
bootGroupSearch = createSearchBuilder();
40-
bootGroupSearch.and("bootGroupId", bootGroupSearch.entity().getBootGroupId(), SearchCriteria.Op.EQ);
41-
bootGroupSearch.done();
4241

4342
itemSearch = createSearchBuilder();
4443
itemSearch.and("itemType", itemSearch.entity().getItemType(), SearchCriteria.Op.EQ);
@@ -60,10 +59,43 @@ public InstanceBootGroupReadinessRuleDaoImpl() {
6059
}
6160

6261
@Override
63-
public List<InstanceBootGroupReadinessRuleVO> listByBootGroupId(long bootGroupId) {
64-
SearchCriteria<InstanceBootGroupReadinessRuleVO> sc = bootGroupSearch.create();
62+
public Pair<List<InstanceBootGroupReadinessRuleVO>, Integer> searchAndCountByBootGroupId(long bootGroupId,
63+
Long id,
64+
InstanceBootGroupMember.MemberType itemType,
65+
Long itemId,
66+
InstanceBootGroupReadinessRule.RuleType ruleType,
67+
String keyword,
68+
Long startIndex,
69+
Long pageSize) {
70+
SearchBuilder<InstanceBootGroupReadinessRuleVO> sb = createSearchBuilder();
71+
sb.and("bootGroupId", sb.entity().getBootGroupId(), SearchCriteria.Op.EQ);
72+
sb.and("id", sb.entity().getId(), SearchCriteria.Op.EQ);
73+
sb.and("itemType", sb.entity().getItemType(), SearchCriteria.Op.EQ);
74+
sb.and("itemId", sb.entity().getItemId(), SearchCriteria.Op.EQ);
75+
sb.and("ruleType", sb.entity().getRuleType(), SearchCriteria.Op.EQ);
76+
sb.and("keyword", sb.entity().getName(), SearchCriteria.Op.LIKE);
77+
sb.done();
78+
79+
SearchCriteria<InstanceBootGroupReadinessRuleVO> sc = sb.create();
6580
sc.setParameters("bootGroupId", bootGroupId);
66-
return listBy(sc);
81+
if (id != null) {
82+
sc.setParameters("id", id);
83+
}
84+
if (itemType != null) {
85+
sc.setParameters("itemType", itemType);
86+
}
87+
if (itemId != null) {
88+
sc.setParameters("itemId", itemId);
89+
}
90+
if (ruleType != null) {
91+
sc.setParameters("ruleType", ruleType);
92+
}
93+
if (keyword != null) {
94+
sc.setParameters("keyword", "%" + keyword + "%");
95+
}
96+
97+
Filter searchFilter = new Filter(InstanceBootGroupReadinessRuleVO.class, "id", true, startIndex, pageSize);
98+
return searchAndCount(sc, searchFilter);
6799
}
68100

69101
@Override
@@ -84,12 +116,4 @@ public List<InstanceBootGroupReadinessRuleVO> listByItem(long bootGroupId, Insta
84116
sc.setParameters("itemId", itemId);
85117
return listBy(sc);
86118
}
87-
88-
@Override
89-
public void deleteByItem(InstanceBootGroupMember.MemberType itemType, long itemId) {
90-
SearchCriteria<InstanceBootGroupReadinessRuleVO> sc = itemSearch.create();
91-
sc.setParameters("itemType", itemType);
92-
sc.setParameters("itemId", itemId);
93-
expunge(sc);
94-
}
95119
}

‎server/src/main/java/org/apache/cloudstack/vm/bootgroup/InstanceBootGroupApiServiceImpl.java‎

Lines changed: 65 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@
7171
import com.cloud.utils.db.Filter;
7272
import com.cloud.utils.db.SearchBuilder;
7373
import com.cloud.utils.db.SearchCriteria;
74+
import com.cloud.utils.db.Transaction;
75+
import com.cloud.utils.db.TransactionCallback;
7476
import com.cloud.vm.InstanceGroupVMMapVO;
7577
import com.cloud.vm.InstanceGroupVO;
7678
import com.cloud.vm.UserVmVO;
@@ -200,9 +202,11 @@ public InstanceBootGroup createInstanceBootGroup(CreateInstanceBootGroupCmd cmd)
200202
@ActionEvent(eventType = EventTypes.EVENT_INSTANCE_BOOT_GROUP_DELETE, eventDescription = "deleting Instance Boot Group")
201203
public boolean deleteInstanceBootGroup(DeleteInstanceBootGroupCmd cmd) {
202204
InstanceBootGroupVO group = getGroupAndCheckAccess(cmd.getId());
203-
instanceBootGroupMemberDao.deleteByBootGroupId(group.getId());
204-
instanceBootGroupDao.remove(group.getId());
205-
return true;
205+
return Transaction.execute((TransactionCallback<Boolean>) status -> {
206+
instanceBootGroupMemberDao.deleteByBootGroupId(group.getId());
207+
instanceBootGroupDao.remove(group.getId());
208+
return true;
209+
});
206210
}
207211

208212
@Override
@@ -301,29 +305,14 @@ public InstanceBootGroupMember addMemberToInstanceBootGroup(AddMemberToInstanceB
301305
InstanceBootGroupMember.MemberType memberType;
302306
long memberId;
303307

304-
if (cmd.getVirtualMachineId() != null && cmd.getInstanceGroupId() != null) {
305-
throw new InvalidParameterValueException("Only one of virtualmachineid or instancegroupid may be specified");
306-
}
307-
if (cmd.getVirtualMachineId() == null && cmd.getInstanceGroupId() == null) {
308-
throw new InvalidParameterValueException("Either virtualmachineid or instancegroupid must be specified");
309-
}
308+
validateEitherVirtualMachineIdOrInstanceGroupIdParam(cmd.getVirtualMachineId(), cmd.getInstanceGroupId());
310309

311310
if (cmd.getVirtualMachineId() != null) {
312-
UserVm vm = userVmDao.findById(cmd.getVirtualMachineId());
313-
if (vm == null) {
314-
throw new InvalidParameterValueException("Unable to find virtual machine with ID: " + cmd.getVirtualMachineId());
315-
}
316-
validateMemberAccount(vm.getAccountId(), group.getAccountId());
317-
instanceBootGroupMembershipGuard.validateVmEligibleForGroupMembership(vm.getId());
311+
UserVm vm = getValidatedVmForAddMember(group, cmd.getVirtualMachineId());
318312
memberType = InstanceBootGroupMember.MemberType.VirtualMachine;
319313
memberId = vm.getId();
320314
} else {
321-
InstanceGroupVO instanceGroup = instanceGroupDao.findById(cmd.getInstanceGroupId());
322-
if (instanceGroup == null || instanceGroup.getRemoved() != null) {
323-
throw new InvalidParameterValueException("Unable to find instance group with ID: " + cmd.getInstanceGroupId());
324-
}
325-
validateMemberAccount(instanceGroup.getAccountId(), group.getAccountId());
326-
instanceBootGroupMembershipGuard.validateInstanceGroupEligibleForBootGroupMembership(instanceGroup.getId());
315+
InstanceGroupVO instanceGroup = getValidatedInstanceGroupAddMember(group, cmd.getInstanceGroupId());
327316
memberType = InstanceBootGroupMember.MemberType.InstanceGroup;
328317
memberId = instanceGroup.getId();
329318
}
@@ -336,6 +325,37 @@ public InstanceBootGroupMember addMemberToInstanceBootGroup(AddMemberToInstanceB
336325
return instanceBootGroupMemberDao.persist(member);
337326
}
338327

328+
@NotNull
329+
private UserVm getValidatedVmForAddMember(InstanceBootGroupVO group, long virtualMachineId) {
330+
UserVm vm = userVmDao.findById(virtualMachineId);
331+
if (vm == null) {
332+
throw new InvalidParameterValueException("Unable to find virtual machine with ID: " + virtualMachineId);
333+
}
334+
validateMemberAccount(vm.getAccountId(), group.getAccountId());
335+
instanceBootGroupMembershipGuard.validateVmEligibleForGroupMembership(vm.getId());
336+
return vm;
337+
}
338+
339+
@NotNull
340+
private InstanceGroupVO getValidatedInstanceGroupAddMember(InstanceBootGroupVO group, long instanceGroupId) {
341+
InstanceGroupVO instanceGroup = instanceGroupDao.findById(instanceGroupId);
342+
if (instanceGroup == null || instanceGroup.getRemoved() != null) {
343+
throw new InvalidParameterValueException("Unable to find instance group with ID: " + instanceGroupId);
344+
}
345+
validateMemberAccount(instanceGroup.getAccountId(), group.getAccountId());
346+
instanceBootGroupMembershipGuard.validateInstanceGroupEligibleForBootGroupMembership(instanceGroup.getId());
347+
return instanceGroup;
348+
}
349+
350+
protected static void validateEitherVirtualMachineIdOrInstanceGroupIdParam(Long virtualMachineId, Long instanceGroupId) {
351+
if (virtualMachineId != null && instanceGroupId != null) {
352+
throw new InvalidParameterValueException("Only one of virtualmachineid or instancegroupid may be specified");
353+
}
354+
if (virtualMachineId == null && instanceGroupId == null) {
355+
throw new InvalidParameterValueException("Either virtualmachineid or instancegroupid must be specified");
356+
}
357+
}
358+
339359
@Override
340360
@ActionEvent(eventType = EventTypes.EVENT_INSTANCE_BOOT_GROUP_MEMBER_REMOVE, eventDescription = "removing Instance Boot Group member")
341361
public boolean removeInstanceBootGroupMember(RemoveInstanceBootGroupMemberCmd cmd) {
@@ -458,7 +478,7 @@ private InstanceBootGroupMemberResponse createInstanceBootGroupMemberResponse(In
458478
response.setOrder(member.getOrder());
459479
response.setCreated(member.getCreated());
460480

461-
List<Long> childVmIds = null;
481+
List<Long> childVmIds = new ArrayList<>();
462482
if (member.getMemberType() == InstanceBootGroupMember.MemberType.VirtualMachine) {
463483
UserVmVO vm = userVmDao.findById(member.getMemberId());
464484
if (vm != null) {
@@ -659,12 +679,7 @@ private void validateMemberAccount(long memberAccountId, long groupAccountId) {
659679
* on the resolved item (in addition to the boot group, already checked via getGroupAndCheckAccess).
660680
*/
661681
private Pair<InstanceBootGroupMember.MemberType, Long> resolveAndCheckAccessToItem(Long virtualMachineId, Long instanceGroupId) {
662-
if (virtualMachineId != null && instanceGroupId != null) {
663-
throw new InvalidParameterValueException("Only one of virtualmachineid or instancegroupid may be specified");
664-
}
665-
if (virtualMachineId == null && instanceGroupId == null) {
666-
throw new InvalidParameterValueException("Either virtualmachineid or instancegroupid must be specified");
667-
}
682+
validateEitherVirtualMachineIdOrInstanceGroupIdParam(virtualMachineId, instanceGroupId);
668683

669684
Account caller = CallContext.current().getCallingAccount();
670685
if (virtualMachineId != null) {
@@ -730,44 +745,35 @@ public boolean deleteInstanceBootGroupReadinessRule(DeleteInstanceBootGroupReadi
730745
public ListResponse<InstanceBootGroupReadinessRuleResponse> listInstanceBootGroupReadinessRules(ListInstanceBootGroupReadinessRulesCmd cmd) {
731746
getGroupAndCheckAccess(cmd.getBootGroupId());
732747

733-
SearchBuilder<InstanceBootGroupReadinessRuleVO> sb = instanceBootGroupReadinessRuleDao.createSearchBuilder();
734-
sb.and("bootGroupId", sb.entity().getBootGroupId(), SearchCriteria.Op.EQ);
735-
sb.and("id", sb.entity().getId(), SearchCriteria.Op.EQ);
736-
sb.and("itemType", sb.entity().getItemType(), SearchCriteria.Op.EQ);
737-
sb.and("itemId", sb.entity().getItemId(), SearchCriteria.Op.EQ);
738-
sb.and("ruleType", sb.entity().getRuleType(), SearchCriteria.Op.EQ);
739-
sb.and("keyword", sb.entity().getName(), SearchCriteria.Op.LIKE);
740-
sb.done();
741-
742-
SearchCriteria<InstanceBootGroupReadinessRuleVO> sc = sb.create();
743-
sc.setParameters("bootGroupId", cmd.getBootGroupId());
744-
if (cmd.getId() != null) {
745-
sc.setParameters("id", cmd.getId());
746-
}
747748
if (cmd.getVirtualMachineId() != null && cmd.getInstanceGroupId() != null) {
748749
throw new InvalidParameterValueException("Only one of virtualmachineid or instancegroupid may be specified");
749750
}
750-
if (cmd.getVirtualMachineId() != null) {
751-
sc.setParameters("itemType", InstanceBootGroupMember.MemberType.VirtualMachine);
752-
sc.setParameters("itemId", cmd.getVirtualMachineId());
753-
} else if (cmd.getInstanceGroupId() != null) {
754-
sc.setParameters("itemType", InstanceBootGroupMember.MemberType.InstanceGroup);
755-
sc.setParameters("itemId", cmd.getInstanceGroupId());
756-
}
757-
InstanceBootGroupReadinessRule.RuleType ruleTypeFilter = null;
751+
InstanceBootGroupReadinessRule.RuleType ruleType = null;
758752
if (cmd.getRuleType() != null) {
759-
ruleTypeFilter = EnumUtils.getEnumIgnoreCase(InstanceBootGroupReadinessRule.RuleType.class, cmd.getRuleType());
760-
if (ruleTypeFilter == null) {
753+
ruleType = EnumUtils.getEnumIgnoreCase(InstanceBootGroupReadinessRule.RuleType.class, cmd.getRuleType());
754+
if (ruleType == null) {
761755
throw new InvalidParameterValueException("Invalid rule type: " + cmd.getRuleType());
762756
}
763-
sc.setParameters("ruleType", ruleTypeFilter);
764-
}
765-
if (cmd.getKeyword() != null) {
766-
sc.setParameters("keyword", "%" + cmd.getKeyword() + "%");
767757
}
768758

769-
Filter searchFilter = new Filter(InstanceBootGroupReadinessRuleVO.class, "id", true, cmd.getStartIndex(), cmd.getPageSizeVal());
770-
Pair<List<InstanceBootGroupReadinessRuleVO>, Integer> rulesAndCount = instanceBootGroupReadinessRuleDao.searchAndCount(sc, searchFilter);
759+
InstanceBootGroupMember.MemberType memberType = null;
760+
Long memberId = null;
761+
if (cmd.getVirtualMachineId() != null) {
762+
memberType = InstanceBootGroupMember.MemberType.VirtualMachine;
763+
memberId = cmd.getVirtualMachineId();
764+
} else if (cmd.getInstanceGroupId() != null) {
765+
memberType = InstanceBootGroupMember.MemberType.InstanceGroup;
766+
memberId = cmd.getInstanceGroupId();
767+
}
768+
Pair<List<InstanceBootGroupReadinessRuleVO>, Integer> rulesAndCount = instanceBootGroupReadinessRuleDao.searchAndCountByBootGroupId(
769+
cmd.getBootGroupId(),
770+
cmd.getId(),
771+
memberType,
772+
memberId,
773+
ruleType,
774+
cmd.getKeyword(),
775+
cmd.getStartIndex(),
776+
cmd.getPageSizeVal());
771777

772778
List<InstanceBootGroupReadinessRuleResponse> responsesList = rulesAndCount.first().stream()
773779
.map(rule -> createInstanceBootGroupReadinessRuleResponse(rule, false, 0))
@@ -776,7 +782,7 @@ public ListResponse<InstanceBootGroupReadinessRuleResponse> listInstanceBootGrou
776782

777783
if (cmd.getId() == null && cmd.getVirtualMachineId() != null) {
778784
for (InstanceBootGroupReadinessRule rule : instanceBootGroupReadinessRuleService.findInheritedGroupRules(cmd.getBootGroupId(), cmd.getVirtualMachineId())) {
779-
if (ruleTypeFilter != null && rule.getRuleType() != ruleTypeFilter) {
785+
if (ruleType != null && rule.getRuleType() != ruleType) {
780786
continue;
781787
}
782788
if (cmd.getKeyword() != null && (rule.getName() == null || !rule.getName().toLowerCase().contains(cmd.getKeyword().toLowerCase()))) {

0 commit comments

Comments
 (0)