Skip to content

Commit d25521e

Browse files
vishesh92vishesh92
authored
Fix issues in VM Scheduler (#7782)
1 parent 4eb110a commit d25521e

File tree

5 files changed

+47
-63
lines changed

5 files changed

+47
-63
lines changed

‎server/src/main/java/org/apache/cloudstack/vm/schedule/VMScheduleManagerImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ public VMScheduleResponse createSchedule(CreateVMScheduleCmd cmd) {
127127

128128
return Transaction.execute((TransactionCallback<VMScheduleResponse>) status -> {
129129
VMScheduleVO vmSchedule = vmScheduleDao.persist(new VMScheduleVO(cmd.getVmId(), finalDescription, cronExpression.toString(), timeZoneId, finalAction, finalStartDate, finalEndDate, cmd.getEnabled()));
130-
vmScheduler.scheduleNextJob(vmSchedule);
130+
vmScheduler.scheduleNextJob(vmSchedule, new Date());
131131
CallContext.current().setEventResourceId(vm.getId());
132132
CallContext.current().setEventResourceType(ApiCommandResourceType.VirtualMachine);
133133
return createResponse(vmSchedule);

‎server/src/main/java/org/apache/cloudstack/vm/schedule/VMScheduler.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,5 +32,5 @@ public interface VMScheduler extends Manager, Scheduler {
3232

3333
void updateScheduledJob(VMScheduleVO vmSchedule);
3434

35-
Date scheduleNextJob(VMScheduleVO vmSchedule);
35+
Date scheduleNextJob(VMScheduleVO vmSchedule, Date timestamp);
3636
}

‎server/src/main/java/org/apache/cloudstack/vm/schedule/VMSchedulerImpl.java‎

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -96,20 +96,11 @@ public void removeScheduledJobs(List<Long> vmScheduleIds) {
9696
@Override
9797
public void updateScheduledJob(VMScheduleVO vmSchedule) {
9898
removeScheduledJobs(Longs.asList(vmSchedule.getId()));
99-
scheduleNextJob(vmSchedule);
100-
}
101-
102-
public Date scheduleNextJob(Long vmScheduleId) {
103-
VMScheduleVO vmSchedule = vmScheduleDao.findById(vmScheduleId);
104-
if (vmSchedule != null) {
105-
return scheduleNextJob(vmSchedule);
106-
}
107-
LOGGER.debug(String.format("VM Schedule [id=%s] is removed. Not scheduling next job.", vmScheduleId));
108-
return null;
99+
scheduleNextJob(vmSchedule, new Date());
109100
}
110101

111102
@Override
112-
public Date scheduleNextJob(VMScheduleVO vmSchedule) {
103+
public Date scheduleNextJob(VMScheduleVO vmSchedule, Date timestamp) {
113104
if (!vmSchedule.getEnabled()) {
114105
LOGGER.debug(String.format("VM Schedule [id=%s] for VM [id=%s] is disabled. Not scheduling next job.", vmSchedule.getUuid(), vmSchedule.getVmId()));
115106
return null;
@@ -127,7 +118,12 @@ public Date scheduleNextJob(VMScheduleVO vmSchedule) {
127118
return null;
128119
}
129120

130-
ZonedDateTime now = ZonedDateTime.now(vmSchedule.getTimeZoneId());
121+
ZonedDateTime now;
122+
if (timestamp != null) {
123+
now = ZonedDateTime.ofInstant(timestamp.toInstant(), vmSchedule.getTimeZoneId());
124+
} else {
125+
now = ZonedDateTime.now(vmSchedule.getTimeZoneId());
126+
}
131127
ZonedDateTime zonedStartDate = ZonedDateTime.ofInstant(startDate.toInstant(), vmSchedule.getTimeZoneId());
132128
ZonedDateTime zonedEndDate = null;
133129
if (endDate != null) {
@@ -178,8 +174,7 @@ public boolean start() {
178174
// Adding 1 minute to currentTimestamp to ensure that
179175
// jobs which were to be run at current time, doesn't cause issues
180176
currentTimestamp = DateUtils.addMinutes(new Date(), 1);
181-
182-
scheduleNextJobs();
177+
scheduleNextJobs(currentTimestamp);
183178

184179
final TimerTask schedulerPollTask = new ManagedContextTimerTask() {
185180
@Override
@@ -193,7 +188,7 @@ protected void runInContext() {
193188
};
194189

195190
vmSchedulerTimer = new Timer("VMSchedulerPollTask");
196-
vmSchedulerTimer.schedule(schedulerPollTask, 5000L, 60 * 1000L);
191+
vmSchedulerTimer.scheduleAtFixedRate(schedulerPollTask, 5000L, 60 * 1000L);
197192
return true;
198193
}
199194

@@ -207,7 +202,7 @@ public void poll(Date timestamp) {
207202
try {
208203
if (scanLock.lock(30)) {
209204
try {
210-
scheduleNextJobs();
205+
scheduleNextJobs(currentTimestamp);
211206
} finally {
212207
scanLock.unlock();
213208
}
@@ -236,10 +231,10 @@ public void poll(Date timestamp) {
236231
}
237232
}
238233

239-
private void scheduleNextJobs() {
234+
private void scheduleNextJobs(Date timestamp) {
240235
for (final VMScheduleVO schedule : vmScheduleDao.listAllActiveSchedules()) {
241236
try {
242-
scheduleNextJob(schedule);
237+
scheduleNextJob(schedule, timestamp);
243238
} catch (Exception e) {
244239
LOGGER.warn("Error in scheduling next job for schedule " + schedule.getUuid(), e);
245240
}

‎server/src/test/java/org/apache/cloudstack/vm/schedule/VMSchedulerImplTest.java‎

Lines changed: 31 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -89,22 +89,22 @@ public void setUp() throws Exception {
8989
MockitoAnnotations.initMocks(this);
9090
PowerMockito.mockStatic(ActionEventUtils.class);
9191
Mockito.when(ActionEventUtils.onScheduledActionEvent(Mockito.anyLong(), Mockito.anyLong(), Mockito.anyString(),
92-
Mockito.anyString(), Mockito.anyLong(), Mockito.anyString(), Mockito.anyBoolean(), Mockito.anyLong())).thenReturn(1L);
92+
Mockito.anyString(), Mockito.anyLong(), Mockito.anyString(), Mockito.anyBoolean(),
93+
Mockito.anyLong()))
94+
.thenReturn(1L);
9395
Mockito.when(ActionEventUtils.onCompletedActionEvent(Mockito.anyLong(), Mockito.anyLong(), Mockito.any(),
9496
Mockito.anyString(), Mockito.anyBoolean(),
9597
Mockito.anyString(),
9698
Mockito.anyLong(), Mockito.anyString(), Mockito.anyLong())).thenReturn(1L);
9799
}
98100

99-
private void prepareMocksForProcessJob(VirtualMachine vm, VMScheduledJob vmScheduledJob, VirtualMachine.State vmState, VMSchedule.Action action, Long executeJobReturnValue) {
100-
Mockito.when(vm.getState()).thenReturn(vmState);
101-
Mockito.when(vmScheduledJob.getAction()).thenReturn(action);
102-
103-
if (executeJobReturnValue != null) {
104-
Mockito.doReturn(executeJobReturnValue).when(vmScheduler).executeStartVMJob(Mockito.any(VirtualMachine.class), Mockito.anyLong());
105-
Mockito.doReturn(executeJobReturnValue).when(vmScheduler).executeStopVMJob(Mockito.any(VirtualMachine.class), Mockito.anyBoolean(), Mockito.anyLong());
106-
Mockito.doReturn(executeJobReturnValue).when(vmScheduler).executeRebootVMJob(Mockito.any(VirtualMachine.class), Mockito.anyBoolean(), Mockito.anyLong());
107-
}
101+
@Test
102+
public void testProcessJobRunning() {
103+
executeProcessJobWithVMStateAndActionNonSkipped(VirtualMachine.State.Running, VMSchedule.Action.STOP);
104+
executeProcessJobWithVMStateAndActionNonSkipped(VirtualMachine.State.Running, VMSchedule.Action.FORCE_STOP);
105+
executeProcessJobWithVMStateAndActionNonSkipped(VirtualMachine.State.Running, VMSchedule.Action.REBOOT);
106+
executeProcessJobWithVMStateAndActionNonSkipped(VirtualMachine.State.Running, VMSchedule.Action.FORCE_REBOOT);
107+
executeProcessJobWithVMStateAndActionNonSkipped(VirtualMachine.State.Stopped, VMSchedule.Action.START);
108108
}
109109

110110
private void executeProcessJobWithVMStateAndActionNonSkipped(VirtualMachine.State state, VMSchedule.Action action) {
@@ -125,13 +125,20 @@ private void executeProcessJobWithVMStateAndActionNonSkipped(VirtualMachine.Stat
125125
Assert.assertEquals(expectedValue, jobId);
126126
}
127127

128-
@Test
129-
public void testProcessJobRunning() {
130-
executeProcessJobWithVMStateAndActionNonSkipped(VirtualMachine.State.Running, VMSchedule.Action.STOP);
131-
executeProcessJobWithVMStateAndActionNonSkipped(VirtualMachine.State.Running, VMSchedule.Action.FORCE_STOP);
132-
executeProcessJobWithVMStateAndActionNonSkipped(VirtualMachine.State.Running, VMSchedule.Action.REBOOT);
133-
executeProcessJobWithVMStateAndActionNonSkipped(VirtualMachine.State.Running, VMSchedule.Action.FORCE_REBOOT);
134-
executeProcessJobWithVMStateAndActionNonSkipped(VirtualMachine.State.Stopped, VMSchedule.Action.START);
128+
private void prepareMocksForProcessJob(VirtualMachine vm, VMScheduledJob vmScheduledJob,
129+
VirtualMachine.State vmState, VMSchedule.Action action,
130+
Long executeJobReturnValue) {
131+
Mockito.when(vm.getState()).thenReturn(vmState);
132+
Mockito.when(vmScheduledJob.getAction()).thenReturn(action);
133+
134+
if (executeJobReturnValue != null) {
135+
Mockito.doReturn(executeJobReturnValue).when(vmScheduler).executeStartVMJob(
136+
Mockito.any(VirtualMachine.class), Mockito.anyLong());
137+
Mockito.doReturn(executeJobReturnValue).when(vmScheduler).executeStopVMJob(
138+
Mockito.any(VirtualMachine.class), Mockito.anyBoolean(), Mockito.anyLong());
139+
Mockito.doReturn(executeJobReturnValue).when(vmScheduler).executeRebootVMJob(
140+
Mockito.any(VirtualMachine.class), Mockito.anyBoolean(), Mockito.anyLong());
141+
}
135142
}
136143

137144
@Test
@@ -172,7 +179,7 @@ public void testScheduleNextJobScheduleScheduleExists() {
172179
Mockito.when(vmSchedule.getStartDate()).thenReturn(startDate);
173180
Mockito.when(userVmManager.getUserVm(Mockito.anyLong())).thenReturn(vm);
174181
Mockito.when(vmScheduledJobDao.persist(Mockito.any())).thenThrow(EntityExistsException.class);
175-
Date actualScheduledTime = vmScheduler.scheduleNextJob(vmSchedule);
182+
Date actualScheduledTime = vmScheduler.scheduleNextJob(vmSchedule, new Date());
176183

177184
Assert.assertEquals(expectedScheduledTime, actualScheduledTime);
178185
}
@@ -190,7 +197,7 @@ public void testScheduleNextJobScheduleFutureSchedule() {
190197
Mockito.when(vmSchedule.getTimeZoneId()).thenReturn(TimeZone.getTimeZone("UTC").toZoneId());
191198
Mockito.when(vmSchedule.getStartDate()).thenReturn(startDate);
192199
Mockito.when(userVmManager.getUserVm(Mockito.anyLong())).thenReturn(vm);
193-
Date actualScheduledTime = vmScheduler.scheduleNextJob(vmSchedule);
200+
Date actualScheduledTime = vmScheduler.scheduleNextJob(vmSchedule, new Date());
194201

195202
Assert.assertEquals(expectedScheduledTime, actualScheduledTime);
196203
}
@@ -226,7 +233,7 @@ public void testScheduleNextJobScheduleFutureScheduleWithTimeZoneChecks() throws
226233
expectedScheduledTime = DateUtils.addDays(expectedScheduledTime, 1);
227234
}
228235

229-
Date actualScheduledTime = vmScheduler.scheduleNextJob(vmSchedule);
236+
Date actualScheduledTime = vmScheduler.scheduleNextJob(vmSchedule, new Date());
230237
Assert.assertEquals(expectedScheduledTime, actualScheduledTime);
231238
}
232239

@@ -242,7 +249,7 @@ public void testScheduleNextJobScheduleCurrentSchedule() {
242249
Mockito.when(vmSchedule.getTimeZoneId()).thenReturn(TimeZone.getTimeZone("UTC").toZoneId());
243250
Mockito.when(vmSchedule.getStartDate()).thenReturn(DateUtils.addDays(now, -1));
244251
Mockito.when(userVmManager.getUserVm(Mockito.anyLong())).thenReturn(vm);
245-
Date actualScheduledTime = vmScheduler.scheduleNextJob(vmSchedule);
252+
Date actualScheduledTime = vmScheduler.scheduleNextJob(vmSchedule, new Date());
246253

247254
Assert.assertEquals(expectedScheduledTime, actualScheduledTime);
248255
}
@@ -277,7 +284,7 @@ public void testScheduleNextJobScheduleCurrentScheduleWithTimeZoneChecks() throw
277284
expectedScheduledTime = DateUtils.addDays(expectedScheduledTime, 1);
278285
}
279286

280-
Date actualScheduledTime = vmScheduler.scheduleNextJob(vmSchedule);
287+
Date actualScheduledTime = vmScheduler.scheduleNextJob(vmSchedule, new Date());
281288
Assert.assertEquals(expectedScheduledTime, actualScheduledTime);
282289
}
283290

@@ -286,36 +293,18 @@ public void testScheduleNextJobScheduleExpired() {
286293
VMScheduleVO vmSchedule = Mockito.mock(VMScheduleVO.class);
287294
Mockito.when(vmSchedule.getEndDate()).thenReturn(DateUtils.addMinutes(new Date(), -5));
288295
Mockito.when(vmSchedule.getEnabled()).thenReturn(true);
289-
Date actualDate = vmScheduler.scheduleNextJob(vmSchedule);
296+
Date actualDate = vmScheduler.scheduleNextJob(vmSchedule, new Date());
290297
Assert.assertNull(actualDate);
291298
}
292299

293300
@Test
294301
public void testScheduleNextJobScheduleDisabled() {
295302
VMScheduleVO vmSchedule = Mockito.mock(VMScheduleVO.class);
296303
Mockito.when(vmSchedule.getEnabled()).thenReturn(false);
297-
Date actualDate = vmScheduler.scheduleNextJob(vmSchedule);
304+
Date actualDate = vmScheduler.scheduleNextJob(vmSchedule, new Date());
298305
Assert.assertNull(actualDate);
299306
}
300307

301-
@Test
302-
public void testScheduleNextJobScheduleIdNotExists() {
303-
long vmScheduleId = 1;
304-
Mockito.when(vmScheduleDao.findById(vmScheduleId)).thenReturn(null);
305-
Date actualDate = vmScheduler.scheduleNextJob(vmScheduleId);
306-
Assert.assertNull(actualDate);
307-
}
308-
309-
@Test
310-
public void testScheduleNextJobScheduleIdExists() {
311-
long vmScheduleId = 1;
312-
VMScheduleVO vmScheduleVO = Mockito.mock(VMScheduleVO.class);
313-
Date date = Mockito.mock(Date.class);
314-
Mockito.when(vmScheduleDao.findById(vmScheduleId)).thenReturn(vmScheduleVO);
315-
Mockito.doReturn(date).when(vmScheduler).scheduleNextJob(vmScheduleVO);
316-
Date actualDate = vmScheduler.scheduleNextJob(vmScheduleId);
317-
Assert.assertEquals(date, actualDate);
318-
}
319308

320309
@Test
321310
public void testExecuteJobs() {

‎test/integration/smoke/test_vm_schedule.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,7 @@ def test_05_vmschedule_test_e2e(self):
571571
stop_vmschedule.delete(self.apiclient)
572572

573573
# To ensure that all vm schedules have been deleted and all of their jobs have been completed
574-
time.sleep(15)
574+
time.sleep(60)
575575

576576
# Verify VM Schedule is deleted
577577
self.assertEqual(

0 commit comments

Comments
 (0)