Skip to content

Commit 5dbf822

Browse files
committed
address review comments
1 parent 89db8a2 commit 5dbf822

File tree

9 files changed

+91
-176
lines changed

9 files changed

+91
-176
lines changed

api/src/main/java/org/apache/cloudstack/api/ApiConstants.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -269,8 +269,10 @@ public class ApiConstants {
269269
public static final String INTERNAL_DNS2 = "internaldns2";
270270
public static final String INTERNET_PROTOCOL = "internetprotocol";
271271
public static final String INTERVAL_TYPE = "intervaltype";
272+
public static final String INSTANCE_LEASE_DURATION = "leaseduration";
272273
public static final String INSTANCE_LEASE_ENABLED = "instanceleaseenabled";
273-
public static final String LOCATION_TYPE = "locationtype";
274+
public static final String INSTANCE_LEASE_EXPIRY_ACTION = "leaseexpiryaction";
275+
public static final String INSTANCE_LEASE_EXPIRY_DATE= "leaseexpirydate";
274276
public static final String IOPS_READ_RATE = "iopsreadrate";
275277
public static final String IOPS_READ_RATE_MAX = "iopsreadratemax";
276278
public static final String IOPS_READ_RATE_MAX_LENGTH = "iopsreadratemaxlength";
@@ -318,11 +320,13 @@ public class ApiConstants {
318320
public static final String LAST_BOOT = "lastboottime";
319321
public static final String LAST_SERVER_START = "lastserverstart";
320322
public static final String LAST_SERVER_STOP = "lastserverstop";
323+
public static final String LEASED = "leased";
321324
public static final String LEVEL = "level";
322325
public static final String LENGTH = "length";
323326
public static final String LIMIT = "limit";
324327
public static final String LIMIT_CPU_USE = "limitcpuuse";
325328
public static final String LIST_HOSTS = "listhosts";
329+
public static final String LOCATION_TYPE = "locationtype";
326330
public static final String LOCK = "lock";
327331
public static final String LUN = "lun";
328332
public static final String LBID = "lbruleid";
@@ -521,10 +525,6 @@ public class ApiConstants {
521525
public static final String USED_SUBNETS = "usedsubnets";
522526
public static final String USED_IOPS = "usediops";
523527
public static final String USER_DATA = "userdata";
524-
public static final String INSTANCE_LEASE_DURATION = "leaseduration";
525-
public static final String INSTANCE_LEASE_EXPIRY_DATE= "leaseexpirydate";
526-
public static final String INSTANCE_LEASE_EXPIRY_ACTION = "leaseexpiryaction";
527-
public static final String LEASED = "leased";
528528

529529
public static final String USER_DATA_NAME = "userdataname";
530530
public static final String USER_DATA_ID = "userdataid";

api/src/main/java/org/apache/cloudstack/api/command/user/vm/UpdateVMCmd.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ public class UpdateVMCmd extends BaseCustomIdCmd implements SecurityGroupAction,
152152
private Boolean deleteProtection;
153153

154154
@Parameter(name = ApiConstants.INSTANCE_LEASE_DURATION, type = CommandType.INTEGER, since = "4.21.0",
155-
description = "Number of days instance is leased for.")
155+
description = "Number of days to lease the instance from now onward. Use -1 to remove the existing lease")
156156
private Integer leaseDuration;
157157

158158
@Parameter(name = ApiConstants.INSTANCE_LEASE_EXPIRY_ACTION, type = CommandType.STRING, since = "4.21.0",

api/src/main/java/org/apache/cloudstack/api/response/ServiceOfferingResponse.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ public class ServiceOfferingResponse extends BaseResponseWithAnnotations {
239239
private Boolean purgeResources;
240240

241241
@SerializedName(ApiConstants.INSTANCE_LEASE_DURATION)
242-
@Param(description = "Instance lease duration for service offering", since = "4.21.0")
242+
@Param(description = "Instance lease duration (in days) for service offering", since = "4.21.0")
243243
private Integer leaseDuration;
244244

245245
@SerializedName(ApiConstants.INSTANCE_LEASE_EXPIRY_ACTION)

server/src/main/java/com/cloud/api/query/QueryManagerImpl.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,7 @@
162162
import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao;
163163
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
164164
import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao;
165+
import org.apache.cloudstack.vm.lease.VMLeaseManager;
165166
import org.apache.cloudstack.vm.lease.VMLeaseManagerImpl;
166167
import org.apache.commons.collections.CollectionUtils;
167168
import org.apache.commons.collections.MapUtils;
@@ -1492,7 +1493,7 @@ private Pair<List<Long>, Integer> searchForUserVMIdsAndCount(ListVMsCmd cmd) {
14921493
if (cmd.getOnlyLeasedInstances()) {
14931494
SearchBuilder<UserVmDetailVO> leasedInstancesSearch = userVmDetailsDao.createSearchBuilder();
14941495
leasedInstancesSearch.and(leasedInstancesSearch.entity().getName(), SearchCriteria.Op.EQ).values(VmDetailConstants.INSTANCE_LEASE_EXECUTION);
1495-
leasedInstancesSearch.and(leasedInstancesSearch.entity().getValue(), SearchCriteria.Op.EQ).values("PENDING");
1496+
leasedInstancesSearch.and(leasedInstancesSearch.entity().getValue(), SearchCriteria.Op.EQ).values(VMLeaseManager.LeaseActionExecution.PENDING.name());
14961497
userVmSearchBuilder.join("userVmToLeased", leasedInstancesSearch, leasedInstancesSearch.entity().getResourceId(),
14971498
userVmSearchBuilder.entity().getId(), JoinBuilder.JoinType.INNER);
14981499
}

server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767
import org.apache.cloudstack.context.CallContext;
6868
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
6969
import org.apache.cloudstack.query.QueryService;
70+
import org.apache.cloudstack.vm.lease.VMLeaseManager;
7071
import org.apache.cloudstack.vm.lease.VMLeaseManagerImpl;
7172
import org.apache.commons.collections.CollectionUtils;
7273
import org.apache.commons.lang3.BooleanUtils;
@@ -135,21 +136,22 @@ protected UserVmJoinDaoImpl() {
135136

136137
leaseExpiredInstanceSearch = createSearchBuilder();
137138
leaseExpiredInstanceSearch.selectFields(leaseExpiredInstanceSearch.entity().getId(), leaseExpiredInstanceSearch.entity().getState(),
138-
leaseExpiredInstanceSearch.entity().isDeleteProtection(), leaseExpiredInstanceSearch.entity().getUuid(),
139-
leaseExpiredInstanceSearch.entity().getLeaseExpiryAction());
139+
leaseExpiredInstanceSearch.entity().isDeleteProtection(), leaseExpiredInstanceSearch.entity().getName(),
140+
leaseExpiredInstanceSearch.entity().getUuid(), leaseExpiredInstanceSearch.entity().getLeaseExpiryAction());
140141

141-
leaseExpiredInstanceSearch.and(leaseExpiredInstanceSearch.entity().getLeaseActionExecution(), Op.EQ).values("PENDING");
142+
leaseExpiredInstanceSearch.and(leaseExpiredInstanceSearch.entity().getLeaseActionExecution(), Op.EQ).values(VMLeaseManager.LeaseActionExecution.PENDING.name());
142143
leaseExpiredInstanceSearch.and("leaseExpired", leaseExpiredInstanceSearch.entity().getLeaseExpiryDate(), Op.LT);
143144
leaseExpiredInstanceSearch.and("leaseExpiryActions", leaseExpiredInstanceSearch.entity().getLeaseExpiryAction(), Op.IN);
144145
leaseExpiredInstanceSearch.and("instanceStateNotIn", leaseExpiredInstanceSearch.entity().getState(), Op.NOTIN);
145146
leaseExpiredInstanceSearch.done();
146147

147148
remainingLeaseInDaysSearch = createSearchBuilder();
148-
remainingLeaseInDaysSearch.selectFields(remainingLeaseInDaysSearch.entity().getId(), remainingLeaseInDaysSearch.entity().getUuid(),
149+
remainingLeaseInDaysSearch.selectFields(remainingLeaseInDaysSearch.entity().getId(),
150+
remainingLeaseInDaysSearch.entity().getUuid(), remainingLeaseInDaysSearch.entity().getName(),
149151
remainingLeaseInDaysSearch.entity().getUserId(), remainingLeaseInDaysSearch.entity().getDomainId(),
150152
remainingLeaseInDaysSearch.entity().getAccountId(), remainingLeaseInDaysSearch.entity().getLeaseExpiryAction());
151153

152-
remainingLeaseInDaysSearch.and(remainingLeaseInDaysSearch.entity().getLeaseActionExecution(), Op.EQ).values("PENDING");
154+
remainingLeaseInDaysSearch.and(remainingLeaseInDaysSearch.entity().getLeaseActionExecution(), Op.EQ).values(VMLeaseManager.LeaseActionExecution.PENDING.name());
153155
remainingLeaseInDaysSearch.and("leaseCurrentDate", remainingLeaseInDaysSearch.entity().getLeaseExpiryDate(), Op.GTEQ);
154156
remainingLeaseInDaysSearch.and("leaseExpiryEndDate", remainingLeaseInDaysSearch.entity().getLeaseExpiryDate(), Op.LT);
155157
remainingLeaseInDaysSearch.done();
@@ -476,7 +478,9 @@ public UserVmResponse newUserVmResponse(ResponseView view, String objectName, Us
476478
userVmResponse.setUserDataPolicy(userVm.getUserDataPolicy());
477479
}
478480

479-
if (VMLeaseManagerImpl.InstanceLeaseEnabled.value() && userVm.getLeaseExpiryDate() != null && "PENDING".equals(userVm.getLeaseActionExecution())) {
481+
if (VMLeaseManagerImpl.InstanceLeaseEnabled.value() && userVm.getLeaseExpiryDate() != null &&
482+
VMLeaseManager.LeaseActionExecution.PENDING.name().equals(userVm.getLeaseActionExecution())) {
483+
480484
userVmResponse.setLeaseExpiryAction(userVm.getLeaseExpiryAction());
481485
userVmResponse.setLeaseExpiryDate(userVm.getLeaseExpiryDate());
482486
int leaseDuration = (int) computeLeaseDurationFromExpiryDate(new Date(), userVm.getLeaseExpiryDate());
@@ -773,7 +777,7 @@ public List<UserVmJoinVO> listByAccountServiceOfferingTemplateAndNotInState(long
773777
public List<UserVmJoinVO> listEligibleInstancesWithExpiredLease() {
774778
SearchCriteria<UserVmJoinVO> sc = leaseExpiredInstanceSearch.create();
775779
sc.setParameters("leaseExpired", new Date());
776-
sc.setParameters("leaseExpiryActions", "STOP", "DESTROY");
780+
sc.setParameters("leaseExpiryActions", VMLeaseManager.ExpiryAction.STOP.name(), VMLeaseManager.ExpiryAction.DESTROY.name());
777781
sc.setParameters("instanceStateNotIn", State.Destroyed, State.Expunging, State.Error, State.Unknown, State.Migrating);
778782
return listBy(sc);
779783
}

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

Lines changed: 15 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2873,7 +2873,7 @@ public UserVm updateVirtualMachine(UpdateVMCmd cmd) throws ResourceUnavailableEx
28732873
if (details.containsKey(VmDetailConstants.INSTANCE_LEASE_EXECUTION)
28742874
|| details.containsKey(VmDetailConstants.INSTANCE_LEASE_EXPIRY_DATE)
28752875
|| details.containsKey(VmDetailConstants.INSTANCE_LEASE_EXPIRY_ACTION)) {
2876-
throw new InvalidParameterValueException("'lease*' should not be included in details as key");
2876+
throw new InvalidParameterValueException("lease parameters should not be included in details as key");
28772877
}
28782878

28792879
if (details.containsKey("extraconfig")) {
@@ -2924,10 +2924,9 @@ public UserVm updateVirtualMachine(UpdateVMCmd cmd) throws ResourceUnavailableEx
29242924
}
29252925
}
29262926

2927-
Integer leaseDuration = cmd.getLeaseDuration();
2928-
String leaseExpiryAction = cmd.getLeaseExpiryAction();
2929-
validateLeaseProperties(leaseDuration, leaseExpiryAction);
2930-
applyLeaseOnUpdateInstance(vmInstance, leaseDuration, leaseExpiryAction);
2927+
if (VMLeaseManagerImpl.InstanceLeaseEnabled.value()) {
2928+
applyLeaseOnUpdateInstance(vmInstance, cmd.getLeaseDuration(), cmd.getLeaseExpiryAction());
2929+
}
29312930

29322931
return updateVirtualMachine(id, displayName, group, ha, isDisplayVm,
29332932
cmd.getDeleteProtection(), osTypeId, userData,
@@ -6173,9 +6172,10 @@ public UserVm createVirtualMachine(DeployVMCmd cmd) throws InsufficientCapacityE
61736172
}
61746173
}
61756174

6176-
Integer leaseDuration = cmd.getLeaseDuration();
6177-
String leaseExpiryAction = cmd.getLeaseExpiryAction();
6178-
validateLeaseProperties(leaseDuration, leaseExpiryAction);
6175+
boolean isLeaseFeatureEnabled = VMLeaseManagerImpl.InstanceLeaseEnabled.value();
6176+
if (isLeaseFeatureEnabled) {
6177+
validateLeaseProperties(cmd.getLeaseDuration(), cmd.getLeaseExpiryAction());
6178+
}
61796179

61806180
List<Long> networkIds = cmd.getNetworkIds();
61816181
LinkedHashMap<Integer, Long> userVmNetworkMap = getVmOvfNetworkMapping(zone, owner, template, cmd.getVmNetworkMap());
@@ -6275,14 +6275,14 @@ public UserVm createVirtualMachine(DeployVMCmd cmd) throws InsufficientCapacityE
62756275
}
62766276
}
62776277
}
6278-
6279-
applyLeaseOnCreateInstance(vm, leaseDuration, leaseExpiryAction, svcOffering);
6278+
if (isLeaseFeatureEnabled) {
6279+
applyLeaseOnCreateInstance(vm, cmd.getLeaseDuration(), cmd.getLeaseExpiryAction(), svcOffering);
6280+
}
62806281
return vm;
62816282
}
62826283

62836284
protected void validateLeaseProperties(Integer leaseDuration, String leaseExpiryAction) {
6284-
if (!VMLeaseManagerImpl.InstanceLeaseEnabled.value()
6285-
|| ObjectUtils.allNull(leaseDuration, leaseExpiryAction) // both are null
6285+
if (ObjectUtils.allNull(leaseDuration, leaseExpiryAction) // both are null
62866286
|| (leaseDuration != null && leaseDuration < 1)) { // special condition to disable lease for instance
62876287
return;
62886288
}
@@ -6317,9 +6317,6 @@ protected void validateLeaseProperties(Integer leaseDuration, String leaseExpiry
63176317
* @param serviceOfferingJoinVO
63186318
*/
63196319
void applyLeaseOnCreateInstance(UserVm vm, Integer leaseDuration, String leaseExpiryAction, ServiceOfferingJoinVO serviceOfferingJoinVO) {
6320-
if (!VMLeaseManagerImpl.InstanceLeaseEnabled.value()) {
6321-
return;
6322-
}
63236320
if (leaseDuration == null) {
63246321
leaseDuration = serviceOfferingJoinVO.getLeaseDuration();
63256322
}
@@ -6332,11 +6329,7 @@ void applyLeaseOnCreateInstance(UserVm vm, Integer leaseDuration, String leaseEx
63326329
}
63336330

63346331
protected void applyLeaseOnUpdateInstance(UserVm instance, Integer leaseDuration, String leaseExpiryAction) {
6335-
// lease feature must be enabled
6336-
if (!VMLeaseManagerImpl.InstanceLeaseEnabled.value() || leaseDuration == null) {
6337-
return;
6338-
}
6339-
6332+
validateLeaseProperties(leaseDuration, leaseExpiryAction);
63406333
String instanceUuid = instance.getUuid();
63416334
// vm must have associated lease during deployment
63426335
UserVmDetailVO vmDetail = userVmDetailsDao.findDetail(instance.getId(), VmDetailConstants.INSTANCE_LEASE_EXPIRY_DATE);
@@ -6361,7 +6354,8 @@ protected void applyLeaseOnUpdateInstance(UserVm instance, Integer leaseDuration
63616354
}
63626355

63636356
if (leaseDuration < 1) {
6364-
userVmDetailsDao.addDetail(instance.getId(), VmDetailConstants.INSTANCE_LEASE_EXECUTION, "DISABLED", false);
6357+
userVmDetailsDao.addDetail(instance.getId(), VmDetailConstants.INSTANCE_LEASE_EXECUTION,
6358+
VMLeaseManager.LeaseActionExecution.DISABLED.name(), false);
63656359
ActionEventUtils.onActionEvent(CallContext.current().getCallingUserId(), instance.getAccountId(), instance.getDomainId(),
63666360
EventTypes.VM_LEASE_DISABLED, "Disabling lease on the instance", instance.getId(), ApiCommandResourceType.VirtualMachine.toString());
63676361
return;

server/src/main/java/org/apache/cloudstack/vm/lease/VMLeaseManager.java

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,16 +30,23 @@ enum ExpiryAction {
3030
DESTROY
3131
}
3232

33+
enum LeaseActionExecution {
34+
PENDING,
35+
DISABLED,
36+
DONE,
37+
CANCELLED
38+
}
39+
3340
ConfigKey<Long> InstanceLeaseSchedulerInterval = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, Long.class,
3441
"instance.lease.scheduler.interval", "3600", "VM Lease Scheduler interval in seconds",
3542
false, List.of(ConfigKey.Scope.Global));
3643

37-
ConfigKey<Long> InstanceLeaseAlertSchedule = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, Long.class,
38-
"instance.lease.alertscheduler.interval", "86400", "Lease Alert Scheduler interval in seconds",
44+
ConfigKey<Long> InstanceLeaseExpiryEventSchedulerInterval = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, Long.class,
45+
"instance.lease.eventscheduler.interval", "86400", "Lease expiry event Scheduler interval in seconds",
3946
false, List.of(ConfigKey.Scope.Global));
4047

41-
ConfigKey<Long> InstanceLeaseExpiryAlertDaysBefore = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, Long.class,
42-
"instance.lease.alert.daysbefore", "7", "Indicates how many days in advance the alert will be triggered before expiry.",
48+
ConfigKey<Long> InstanceLeaseExpiryEventDaysBefore = new ConfigKey<>(ConfigKey.CATEGORY_ADVANCED, Long.class,
49+
"instance.lease.expiryevent.daysbefore", "7", "Indicates how many days in advance, expiry events will be created before expiry.",
4350
true, List.of(ConfigKey.Scope.Global));
4451

4552
void onLeaseFeatureToggle();

0 commit comments

Comments
 (0)