Skip to content

Fix issues in VM Scheduler #7782

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DaanHoogland merged 2 commits into from
Sep 18, 2023
Merged

Fix issues in VM Scheduler #7782

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bdb0247
Fix issues in VM Scheduler
vishesh92 Jul 27, 2023
7c0c89d
increase wait time for test_vm_schedule
vishesh92 Jul 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion server/src/main/java/org/apache/cloudstack/vm/schedule/VMScheduleManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ public VMScheduleResponse createSchedule(CreateVMScheduleCmd cmd) {

return Transaction.execute((TransactionCallback<VMScheduleResponse>) status -> {
VMScheduleVO vmSchedule = vmScheduleDao.persist(new VMScheduleVO(cmd.getVmId(), finalDescription, cronExpression.toString(), timeZoneId, finalAction, finalStartDate, finalEndDate, cmd.getEnabled()));
vmScheduler.scheduleNextJob(vmSchedule);
vmScheduler.scheduleNextJob(vmSchedule, new Date());
CallContext.current().setEventResourceId(vm.getId());
CallContext.current().setEventResourceType(ApiCommandResourceType.VirtualMachine);
return createResponse(vmSchedule);
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/org/apache/cloudstack/vm/schedule/VMScheduler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,5 @@ public interface VMScheduler extends Manager, Scheduler {

void updateScheduledJob(VMScheduleVO vmSchedule);

Date scheduleNextJob(VMScheduleVO vmSchedule);
Date scheduleNextJob(VMScheduleVO vmSchedule, Date timestamp);
}
31 changes: 13 additions & 18 deletions server/src/main/java/org/apache/cloudstack/vm/schedule/VMSchedulerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,20 +96,11 @@ public void removeScheduledJobs(List<Long> vmScheduleIds) {
@Override
public void updateScheduledJob(VMScheduleVO vmSchedule) {
removeScheduledJobs(Longs.asList(vmSchedule.getId()));
scheduleNextJob(vmSchedule);
}

public Date scheduleNextJob(Long vmScheduleId) {
VMScheduleVO vmSchedule = vmScheduleDao.findById(vmScheduleId);
if (vmSchedule != null) {
return scheduleNextJob(vmSchedule);
}
LOGGER.debug(String.format("VM Schedule [id=%s] is removed. Not scheduling next job.", vmScheduleId));
return null;
scheduleNextJob(vmSchedule, new Date());
}

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

ZonedDateTime now = ZonedDateTime.now(vmSchedule.getTimeZoneId());
ZonedDateTime now;
if (timestamp != null) {
now = ZonedDateTime.ofInstant(timestamp.toInstant(), vmSchedule.getTimeZoneId());
} else {
now = ZonedDateTime.now(vmSchedule.getTimeZoneId());
}
ZonedDateTime zonedStartDate = ZonedDateTime.ofInstant(startDate.toInstant(), vmSchedule.getTimeZoneId());
ZonedDateTime zonedEndDate = null;
if (endDate != null) {
Expand Down Expand Up @@ -178,8 +174,7 @@ public boolean start() {
// Adding 1 minute to currentTimestamp to ensure that
// jobs which were to be run at current time, doesn't cause issues
currentTimestamp = DateUtils.addMinutes(new Date(), 1);

scheduleNextJobs();
scheduleNextJobs(currentTimestamp);

final TimerTask schedulerPollTask = new ManagedContextTimerTask() {
@Override
Expand All @@ -193,7 +188,7 @@ protected void runInContext() {
};

vmSchedulerTimer = new Timer("VMSchedulerPollTask");
vmSchedulerTimer.schedule(schedulerPollTask, 5000L, 60 * 1000L);
vmSchedulerTimer.scheduleAtFixedRate(schedulerPollTask, 5000L, 60 * 1000L);
return true;
}

Expand All @@ -207,7 +202,7 @@ public void poll(Date timestamp) {
try {
if (scanLock.lock(30)) {
try {
scheduleNextJobs();
scheduleNextJobs(currentTimestamp);
} finally {
scanLock.unlock();
}
Expand Down Expand Up @@ -236,10 +231,10 @@ public void poll(Date timestamp) {
}
}

private void scheduleNextJobs() {
private void scheduleNextJobs(Date timestamp) {
for (final VMScheduleVO schedule : vmScheduleDao.listAllActiveSchedules()) {
try {
scheduleNextJob(schedule);
scheduleNextJob(schedule, timestamp);
} catch (Exception e) {
LOGGER.warn("Error in scheduling next job for schedule " + schedule.getUuid(), e);
}
Expand Down
73 changes: 31 additions & 42 deletions server/src/test/java/org/apache/cloudstack/vm/schedule/VMSchedulerImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,22 +89,22 @@ public void setUp() throws Exception {
MockitoAnnotations.initMocks(this);
PowerMockito.mockStatic(ActionEventUtils.class);
Mockito.when(ActionEventUtils.onScheduledActionEvent(Mockito.anyLong(), Mockito.anyLong(), Mockito.anyString(),
Mockito.anyString(), Mockito.anyLong(), Mockito.anyString(), Mockito.anyBoolean(), Mockito.anyLong())).thenReturn(1L);
Mockito.anyString(), Mockito.anyLong(), Mockito.anyString(), Mockito.anyBoolean(),
Mockito.anyLong()))
.thenReturn(1L);
Mockito.when(ActionEventUtils.onCompletedActionEvent(Mockito.anyLong(), Mockito.anyLong(), Mockito.any(),
Mockito.anyString(), Mockito.anyBoolean(),
Mockito.anyString(),
Mockito.anyLong(), Mockito.anyString(), Mockito.anyLong())).thenReturn(1L);
}

private void prepareMocksForProcessJob(VirtualMachine vm, VMScheduledJob vmScheduledJob, VirtualMachine.State vmState, VMSchedule.Action action, Long executeJobReturnValue) {
Mockito.when(vm.getState()).thenReturn(vmState);
Mockito.when(vmScheduledJob.getAction()).thenReturn(action);

if (executeJobReturnValue != null) {
Mockito.doReturn(executeJobReturnValue).when(vmScheduler).executeStartVMJob(Mockito.any(VirtualMachine.class), Mockito.anyLong());
Mockito.doReturn(executeJobReturnValue).when(vmScheduler).executeStopVMJob(Mockito.any(VirtualMachine.class), Mockito.anyBoolean(), Mockito.anyLong());
Mockito.doReturn(executeJobReturnValue).when(vmScheduler).executeRebootVMJob(Mockito.any(VirtualMachine.class), Mockito.anyBoolean(), Mockito.anyLong());
}
@Test
public void testProcessJobRunning() {
executeProcessJobWithVMStateAndActionNonSkipped(VirtualMachine.State.Running, VMSchedule.Action.STOP);
executeProcessJobWithVMStateAndActionNonSkipped(VirtualMachine.State.Running, VMSchedule.Action.FORCE_STOP);
executeProcessJobWithVMStateAndActionNonSkipped(VirtualMachine.State.Running, VMSchedule.Action.REBOOT);
executeProcessJobWithVMStateAndActionNonSkipped(VirtualMachine.State.Running, VMSchedule.Action.FORCE_REBOOT);
executeProcessJobWithVMStateAndActionNonSkipped(VirtualMachine.State.Stopped, VMSchedule.Action.START);
}

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

@Test
public void testProcessJobRunning() {
executeProcessJobWithVMStateAndActionNonSkipped(VirtualMachine.State.Running, VMSchedule.Action.STOP);
executeProcessJobWithVMStateAndActionNonSkipped(VirtualMachine.State.Running, VMSchedule.Action.FORCE_STOP);
executeProcessJobWithVMStateAndActionNonSkipped(VirtualMachine.State.Running, VMSchedule.Action.REBOOT);
executeProcessJobWithVMStateAndActionNonSkipped(VirtualMachine.State.Running, VMSchedule.Action.FORCE_REBOOT);
executeProcessJobWithVMStateAndActionNonSkipped(VirtualMachine.State.Stopped, VMSchedule.Action.START);
private void prepareMocksForProcessJob(VirtualMachine vm, VMScheduledJob vmScheduledJob,
VirtualMachine.State vmState, VMSchedule.Action action,
Long executeJobReturnValue) {
Mockito.when(vm.getState()).thenReturn(vmState);
Mockito.when(vmScheduledJob.getAction()).thenReturn(action);

if (executeJobReturnValue != null) {
Mockito.doReturn(executeJobReturnValue).when(vmScheduler).executeStartVMJob(
Mockito.any(VirtualMachine.class), Mockito.anyLong());
Mockito.doReturn(executeJobReturnValue).when(vmScheduler).executeStopVMJob(
Mockito.any(VirtualMachine.class), Mockito.anyBoolean(), Mockito.anyLong());
Mockito.doReturn(executeJobReturnValue).when(vmScheduler).executeRebootVMJob(
Mockito.any(VirtualMachine.class), Mockito.anyBoolean(), Mockito.anyLong());
}
}

@Test
Expand Down Expand Up @@ -172,7 +179,7 @@ public void testScheduleNextJobScheduleScheduleExists() {
Mockito.when(vmSchedule.getStartDate()).thenReturn(startDate);
Mockito.when(userVmManager.getUserVm(Mockito.anyLong())).thenReturn(vm);
Mockito.when(vmScheduledJobDao.persist(Mockito.any())).thenThrow(EntityExistsException.class);
Date actualScheduledTime = vmScheduler.scheduleNextJob(vmSchedule);
Date actualScheduledTime = vmScheduler.scheduleNextJob(vmSchedule, new Date());

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

Assert.assertEquals(expectedScheduledTime, actualScheduledTime);
}
Expand Down Expand Up @@ -226,7 +233,7 @@ public void testScheduleNextJobScheduleFutureScheduleWithTimeZoneChecks() throws
expectedScheduledTime = DateUtils.addDays(expectedScheduledTime, 1);
}

Date actualScheduledTime = vmScheduler.scheduleNextJob(vmSchedule);
Date actualScheduledTime = vmScheduler.scheduleNextJob(vmSchedule, new Date());
Assert.assertEquals(expectedScheduledTime, actualScheduledTime);
}

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

Assert.assertEquals(expectedScheduledTime, actualScheduledTime);
}
Expand Down Expand Up @@ -277,7 +284,7 @@ public void testScheduleNextJobScheduleCurrentScheduleWithTimeZoneChecks() throw
expectedScheduledTime = DateUtils.addDays(expectedScheduledTime, 1);
}

Date actualScheduledTime = vmScheduler.scheduleNextJob(vmSchedule);
Date actualScheduledTime = vmScheduler.scheduleNextJob(vmSchedule, new Date());
Assert.assertEquals(expectedScheduledTime, actualScheduledTime);
}

Expand All @@ -286,36 +293,18 @@ public void testScheduleNextJobScheduleExpired() {
VMScheduleVO vmSchedule = Mockito.mock(VMScheduleVO.class);
Mockito.when(vmSchedule.getEndDate()).thenReturn(DateUtils.addMinutes(new Date(), -5));
Mockito.when(vmSchedule.getEnabled()).thenReturn(true);
Date actualDate = vmScheduler.scheduleNextJob(vmSchedule);
Date actualDate = vmScheduler.scheduleNextJob(vmSchedule, new Date());
Assert.assertNull(actualDate);
}

@Test
public void testScheduleNextJobScheduleDisabled() {
VMScheduleVO vmSchedule = Mockito.mock(VMScheduleVO.class);
Mockito.when(vmSchedule.getEnabled()).thenReturn(false);
Date actualDate = vmScheduler.scheduleNextJob(vmSchedule);
Date actualDate = vmScheduler.scheduleNextJob(vmSchedule, new Date());
Assert.assertNull(actualDate);
}

@Test
public void testScheduleNextJobScheduleIdNotExists() {
long vmScheduleId = 1;
Mockito.when(vmScheduleDao.findById(vmScheduleId)).thenReturn(null);
Date actualDate = vmScheduler.scheduleNextJob(vmScheduleId);
Assert.assertNull(actualDate);
}

@Test
public void testScheduleNextJobScheduleIdExists() {
long vmScheduleId = 1;
VMScheduleVO vmScheduleVO = Mockito.mock(VMScheduleVO.class);
Date date = Mockito.mock(Date.class);
Mockito.when(vmScheduleDao.findById(vmScheduleId)).thenReturn(vmScheduleVO);
Mockito.doReturn(date).when(vmScheduler).scheduleNextJob(vmScheduleVO);
Date actualDate = vmScheduler.scheduleNextJob(vmScheduleId);
Assert.assertEquals(date, actualDate);
}

@Test
public void testExecuteJobs() {
Expand Down
2 changes: 1 addition & 1 deletion test/integration/smoke/test_vm_schedule.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ def test_05_vmschedule_test_e2e(self):
stop_vmschedule.delete(self.apiclient)

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

# Verify VM Schedule is deleted
self.assertEqual(
Expand Down