This is an automated email from the ASF dual-hosted git repository. DaanHoogland pushed a commit to branch ghi13454-recurringFailingSnapshots in repository https://gitbox.apache.org/repos/asf/cloudstack.git
commit de2aee26a8b3a05121b86c0c85ec3dcaf8d6a68c Author: Daan Hoogland <[email protected]> AuthorDate: Wed Jul 29 11:44:37 2026 +0200 deal with excesive attempts on failing snapshots --- api/src/main/java/com/cloud/event/EventTypes.java | 2 + .../main/java/com/cloud/event/dao/EventDao.java | 6 + .../java/com/cloud/event/dao/EventDaoImpl.java | 17 ++ .../cloud/storage/snapshot/SnapshotManager.java | 14 ++ .../storage/snapshot/SnapshotManagerImpl.java | 3 +- .../storage/snapshot/SnapshotSchedulerImpl.java | 208 +++++++++++++++++- .../snapshot/SnapshotSchedulerImplTest.java | 232 +++++++++++++++++++++ 7 files changed, 480 insertions(+), 2 deletions(-) diff --git a/api/src/main/java/com/cloud/event/EventTypes.java b/api/src/main/java/com/cloud/event/EventTypes.java index f7d13343d46..5323f71c52b 100644 --- a/api/src/main/java/com/cloud/event/EventTypes.java +++ b/api/src/main/java/com/cloud/event/EventTypes.java @@ -370,6 +370,7 @@ public class EventTypes { public static final String EVENT_SNAPSHOT_DELETE = "SNAPSHOT.DELETE"; public static final String EVENT_SNAPSHOT_REVERT = "SNAPSHOT.REVERT"; public static final String EVENT_SNAPSHOT_EXTRACT = "SNAPSHOT.EXTRACT"; + public static final String EVENT_SNAPSHOT_SKIPPED = "SNAPSHOT.SKIPPED"; public static final String EVENT_SNAPSHOT_POLICY_CREATE = "SNAPSHOTPOLICY.CREATE"; public static final String EVENT_SNAPSHOT_POLICY_UPDATE = "SNAPSHOTPOLICY.UPDATE"; public static final String EVENT_SNAPSHOT_POLICY_DELETE = "SNAPSHOTPOLICY.DELETE"; @@ -1074,6 +1075,7 @@ public class EventTypes { // Snapshots entityEventDetails.put(EVENT_SNAPSHOT_CREATE, Snapshot.class); + entityEventDetails.put(EVENT_SNAPSHOT_SKIPPED, Snapshot.class); entityEventDetails.put(EVENT_SNAPSHOT_DELETE, Snapshot.class); entityEventDetails.put(EVENT_SNAPSHOT_EXTRACT, Snapshot.class); entityEventDetails.put(EVENT_SNAPSHOT_ON_PRIMARY, Snapshot.class); diff --git a/engine/schema/src/main/java/com/cloud/event/dao/EventDao.java b/engine/schema/src/main/java/com/cloud/event/dao/EventDao.java index c50451b03e4..87e9fcb61bc 100644 --- a/engine/schema/src/main/java/com/cloud/event/dao/EventDao.java +++ b/engine/schema/src/main/java/com/cloud/event/dao/EventDao.java @@ -35,4 +35,10 @@ public interface EventDao extends GenericDao<EventVO, Long> { public void archiveEvents(List<EventVO> events); + /** + * Returns the most recent events of the given type for a resource, newest first. + * Used to derive how many consecutive attempts have failed for that resource without a dedicated counter column. + */ + List<EventVO> listLatestEventsByResource(long resourceId, String resourceType, String type, int limit); + } diff --git a/engine/schema/src/main/java/com/cloud/event/dao/EventDaoImpl.java b/engine/schema/src/main/java/com/cloud/event/dao/EventDaoImpl.java index 9417ddd1259..da03ca1482d 100644 --- a/engine/schema/src/main/java/com/cloud/event/dao/EventDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/event/dao/EventDaoImpl.java @@ -38,6 +38,7 @@ public class EventDaoImpl extends GenericDaoBase<EventVO, Long> implements Event protected final SearchBuilder<EventVO> CompletedEventSearch; protected final SearchBuilder<EventVO> ToArchiveOrDeleteEventSearch; protected final SearchBuilder<EventVO> ArchiveByIdsSearch; + protected final SearchBuilder<EventVO> LatestEventsByResourceSearch; public EventDaoImpl() { CompletedEventSearch = createSearchBuilder(); @@ -46,6 +47,12 @@ public class EventDaoImpl extends GenericDaoBase<EventVO, Long> implements Event CompletedEventSearch.and("archived", CompletedEventSearch.entity().getArchived(), Op.EQ); CompletedEventSearch.done(); + LatestEventsByResourceSearch = createSearchBuilder(); + LatestEventsByResourceSearch.and("resourceId", LatestEventsByResourceSearch.entity().getResourceId(), Op.EQ); + LatestEventsByResourceSearch.and("resourceType", LatestEventsByResourceSearch.entity().getResourceType(), Op.EQ); + LatestEventsByResourceSearch.and("type", LatestEventsByResourceSearch.entity().getType(), Op.EQ); + LatestEventsByResourceSearch.done(); + ToArchiveOrDeleteEventSearch = createSearchBuilder(); ToArchiveOrDeleteEventSearch.and("id", ToArchiveOrDeleteEventSearch.entity().getId(), Op.IN); ToArchiveOrDeleteEventSearch.and("type", ToArchiveOrDeleteEventSearch.entity().getType(), Op.EQ); @@ -105,6 +112,16 @@ public class EventDaoImpl extends GenericDaoBase<EventVO, Long> implements Event return search(sc, null); } + @Override + public List<EventVO> listLatestEventsByResource(long resourceId, String resourceType, String type, int limit) { + SearchCriteria<EventVO> sc = LatestEventsByResourceSearch.create(); + sc.setParameters("resourceId", resourceId); + sc.setParameters("resourceType", resourceType); + sc.setParameters("type", type); + Filter filter = new Filter(EventVO.class, "createDate", false, 0L, (long) limit); + return listBy(sc, filter); + } + @Override public void archiveEvents(List<EventVO> events) { if (CollectionUtils.isEmpty(events)) { diff --git a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManager.java b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManager.java index 10dcc2683de..6ad50e3df4b 100644 --- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManager.java +++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManager.java @@ -16,6 +16,8 @@ // under the License. package com.cloud.storage.snapshot; +import java.util.List; + import com.cloud.user.Account; import com.cloud.hypervisor.Hypervisor; import com.cloud.storage.StoragePool; @@ -53,6 +55,18 @@ public interface SnapshotManager extends Configurable { public static final ConfigKey<Integer> BackupRetryInterval = new ConfigKey<Integer>(Integer.class, "backup.retry.interval", "Advanced", "300", "Time in seconds between retries in backing up snapshot to secondary", false, ConfigKey.Scope.Global, null); + ConfigKey<Integer> SnapshotRecurringMaxFailures = new ConfigKey<>(Integer.class, "snapshot.recurring.max.failures", "Snapshots", "3", + "Maximum number of consecutive failed attempts allowed for a recurring snapshot before it is left until its next regularly scheduled run and a failure event is logged. Set to 0 to retry indefinitely.", + true, List.of(ConfigKey.Scope.Account, ConfigKey.Scope.Domain, ConfigKey.Scope.Zone, ConfigKey.Scope.Global), null); + + ConfigKey<Integer> SnapshotRecurringRetryInterval = new ConfigKey<>(Integer.class, "snapshot.recurring.retry.interval", "Snapshots", "300", + "Time in seconds to wait before retrying a failed recurring snapshot attempt.", + true, List.of(ConfigKey.Scope.Account, ConfigKey.Scope.Domain, ConfigKey.Scope.Zone, ConfigKey.Scope.Global), null); + + ConfigKey<Boolean> SnapshotSkipIfVmNotRunning = new ConfigKey<>(Boolean.class, "snapshot.skip.if.vm.not.running", "Snapshots", "false", + "For recurring snapshots, skip taking a new snapshot of a volume (and log a skipped event instead) when the VM it is attached to has not been running since the last snapshot was taken, since nothing on the volume could have changed.", + true, List.of(ConfigKey.Scope.Account, ConfigKey.Scope.Domain, ConfigKey.Scope.Zone, ConfigKey.Scope.Global), null); + public static final ConfigKey<Boolean> VmStorageSnapshotKvm = new ConfigKey<>(Boolean.class, "kvm.vmstoragesnapshot.enabled", "Snapshots", "true", "For live snapshot of virtual machine instance on KVM hypervisor without memory. Requires qemu version 1.6+ (on NFS or Local file system) and qemu-guest-agent installed on guest VM", true, ConfigKey.Scope.Global, null); ConfigKey<Boolean> KVMSnapshotEnabled = new ConfigKey<>(Boolean.class, "kvm.snapshot.enabled", "Snapshots", "true", "Whether volume snapshot is enabled on running instances " + diff --git a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java index dc33a4442a3..250f85ca523 100755 --- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -310,7 +310,8 @@ public class SnapshotManagerImpl extends MutualExclusiveIdsManagerBase implement @Override public ConfigKey<?>[] getConfigKeys() { return new ConfigKey<?>[] {BackupRetryAttempts, BackupRetryInterval, SnapshotHourlyMax, SnapshotDailyMax, SnapshotMonthlyMax, SnapshotWeeklyMax, usageSnapshotSelection, - SnapshotInfo.BackupSnapshotAfterTakingSnapshot, VmStorageSnapshotKvm, kvmIncrementalSnapshot, snapshotDeltaMax, snapshotShowChainSize, UseStorageReplication, KVMSnapshotEnabled}; + SnapshotInfo.BackupSnapshotAfterTakingSnapshot, VmStorageSnapshotKvm, kvmIncrementalSnapshot, snapshotDeltaMax, snapshotShowChainSize, UseStorageReplication, KVMSnapshotEnabled, + SnapshotRecurringMaxFailures, SnapshotRecurringRetryInterval, SnapshotSkipIfVmNotRunning}; } @Override diff --git a/server/src/main/java/com/cloud/storage/snapshot/SnapshotSchedulerImpl.java b/server/src/main/java/com/cloud/storage/snapshot/SnapshotSchedulerImpl.java index ddaa96100bd..7fcfe76f1c7 100644 --- a/server/src/main/java/com/cloud/storage/snapshot/SnapshotSchedulerImpl.java +++ b/server/src/main/java/com/cloud/storage/snapshot/SnapshotSchedulerImpl.java @@ -30,6 +30,7 @@ import javax.naming.ConfigurationException; import org.apache.cloudstack.api.ApiCommandResourceType; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.command.user.snapshot.CreateSnapshotCmd; +import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.framework.jobs.AsyncJobDispatcher; import org.apache.cloudstack.framework.jobs.AsyncJobManager; @@ -43,11 +44,14 @@ import com.cloud.api.ApiDispatcher; import com.cloud.api.ApiGsonHelper; import com.cloud.event.ActionEventUtils; import com.cloud.event.EventTypes; +import com.cloud.event.EventVO; +import com.cloud.event.dao.EventDao; import com.cloud.server.ResourceTag; import com.cloud.server.TaggedResourceService; import com.cloud.storage.Snapshot; import com.cloud.storage.SnapshotPolicyVO; import com.cloud.storage.SnapshotScheduleVO; +import com.cloud.storage.SnapshotVO; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.SnapshotDao; import com.cloud.storage.dao.SnapshotPolicyDao; @@ -63,8 +67,12 @@ import com.cloud.utils.component.ComponentContext; import com.cloud.utils.component.ManagerBase; import com.cloud.utils.concurrency.TestClock; import com.cloud.utils.db.DB; +import com.cloud.utils.db.Filter; import com.cloud.utils.db.GlobalLock; import com.cloud.utils.db.TransactionLegacy; +import com.cloud.vm.VirtualMachine; +import com.cloud.vm.VMInstanceVO; +import com.cloud.vm.dao.VMInstanceDao; import com.cloud.vm.snapshot.VMSnapshotManager; import com.cloud.vm.snapshot.VMSnapshotVO; import com.cloud.vm.snapshot.dao.VMSnapshotDao; @@ -98,6 +106,10 @@ public class SnapshotSchedulerImpl extends ManagerBase implements SnapshotSchedu protected VMSnapshotManager _vmSnaphostManager; @Inject public TaggedResourceService taggedResourceService; + @Inject + protected EventDao _eventDao; + @Inject + protected VMInstanceDao _vmInstanceDao; protected AsyncJobDispatcher _asyncDispatcher; @@ -196,8 +208,10 @@ public class SnapshotSchedulerImpl extends ManagerBase implements SnapshotSchedu if (JobInfo.Status.SUCCEEDED.equals(status)) { logger.debug("Last job of schedule [{}] succeeded; scheduling the next snapshot job.", snapshotSchedule); + recordSnapshotAttemptOutcome(snapshotSchedule, true, null); } else if (JobInfo.Status.FAILED.equals(status)) { logger.debug("Last job of schedule [{}] failed with [{}]; scheduling a new snapshot job.", snapshotSchedule, asyncJob.getResult()); + recordSnapshotAttemptOutcome(snapshotSchedule, false, asyncJob.getResult()); } else { logger.debug("Schedule [{}] is still in progress, skipping next job scheduling.", snapshotSchedule); return; @@ -206,6 +220,40 @@ public class SnapshotSchedulerImpl extends ManagerBase implements SnapshotSchedu scheduleNextSnapshotJob(snapshotSchedule); } + /** + * Logs an event for the outcome of a recurring snapshot job (keyed by the volume, since a fresh snapshot entity + * ID is minted on every attempt) so that consecutive failures can be counted from event history, and raises a + * WARN notification once {@link SnapshotManager#SnapshotRecurringMaxFailures} consecutive failures are reached. + */ + protected void recordSnapshotAttemptOutcome(final SnapshotScheduleVO snapshotSchedule, final boolean succeeded, final String failureResult) { + final VolumeVO volume = _volsDao.findByIdIncludingRemoved(snapshotSchedule.getVolumeId()); + if (volume == null) { + return; + } + + if (succeeded) { + ActionEventUtils.onCreatedActionEvent(User.UID_SYSTEM, volume.getAccountId(), EventVO.LEVEL_INFO, EventTypes.EVENT_SNAPSHOT_CREATE, true, + String.format("Scheduled snapshot creation job for volume [%s] succeeded.", volume), + volume.getId(), ApiCommandResourceType.Volume.toString()); + return; + } + + final Account account = _acctDao.findById(volume.getAccountId()); + final int maxFailures = getScopedConfigValue(SnapshotManager.SnapshotRecurringMaxFailures, volume, account); + final int totalFailures = countConsecutiveFailedAttempts(volume.getId(), maxFailures) + 1; + + ActionEventUtils.onCreatedActionEvent(User.UID_SYSTEM, volume.getAccountId(), EventVO.LEVEL_ERROR, EventTypes.EVENT_SNAPSHOT_CREATE, true, + String.format("Scheduled snapshot creation job for volume [%s] failed: %s", volume, failureResult), + volume.getId(), ApiCommandResourceType.Volume.toString()); + + if (maxFailures > 0 && totalFailures >= maxFailures) { + logger.warn("Snapshot schedule [{}] for volume [{}] has failed [{}] consecutive times.", snapshotSchedule, volume, totalFailures); + ActionEventUtils.onCreatedActionEvent(User.UID_SYSTEM, volume.getAccountId(), EventVO.LEVEL_WARN, EventTypes.EVENT_SNAPSHOT_CREATE, true, + String.format("Recurring snapshot for volume [%s] has failed %d consecutive times.", volume, totalFailures), + volume.getId(), ApiCommandResourceType.Volume.toString()); + } + } + @DB protected void deleteExpiredVMSnapshots() { Date now = new Date(); @@ -237,6 +285,7 @@ public class SnapshotSchedulerImpl extends ManagerBase implements SnapshotSchedu for (final SnapshotScheduleVO snapshotToBeExecuted : snapshotsToBeExecuted) { SnapshotScheduleVO tmpSnapshotScheduleVO = null; + Long eventId = null; final long snapshotScheId = snapshotToBeExecuted.getId(); final long policyId = snapshotToBeExecuted.getPolicyId(); final long volumeId = snapshotToBeExecuted.getVolumeId(); @@ -246,8 +295,13 @@ public class SnapshotSchedulerImpl extends ManagerBase implements SnapshotSchedu continue; } + if (shouldSkipUnchangedVolumeSnapshot(volume)) { + skipAndRescheduleSnapshot(snapshotToBeExecuted, volume); + continue; + } + tmpSnapshotScheduleVO = _snapshotScheduleDao.acquireInLockTable(snapshotScheId); - final Long eventId = + eventId = ActionEventUtils.onScheduledActionEvent(User.UID_SYSTEM, volume.getAccountId(), EventTypes.EVENT_SNAPSHOT_CREATE, "creating snapshot for volume Id:" + volume.getUuid(), volumeId, ApiCommandResourceType.Volume.toString(), true, 0); @@ -289,6 +343,9 @@ public class SnapshotSchedulerImpl extends ManagerBase implements SnapshotSchedu _snapshotScheduleDao.update(snapshotScheId, tmpSnapshotScheduleVO); } catch (final Exception e) { logger.error("The scheduling of snapshot [{}] for volume [{}] failed due to [{}].", snapshotToBeExecuted, volume, e.toString(), e); + if (tmpSnapshotScheduleVO != null) { + handleFailedSnapshotDispatch(snapshotToBeExecuted, volume, tmpSnapshotScheduleVO, eventId, e); + } } finally { if (tmpSnapshotScheduleVO != null) { _snapshotScheduleDao.releaseFromLockTable(snapshotScheId); @@ -297,6 +354,155 @@ public class SnapshotSchedulerImpl extends ManagerBase implements SnapshotSchedu } } + /** + * Handles a synchronous failure to dispatch the CreateSnapshotCmd (e.g. an allocation error) for a recurring + * snapshot. Without this, the schedule's {@code scheduledTimestamp} is never advanced, so it gets retried on + * every poll (every {@code snapshot.poll.interval} seconds) forever. Instead: log a failure event keyed by the + * volume (so consecutive failures can be counted from event history), and either back off by the configured + * retry interval, or - once the configured maximum consecutive failures is reached - give up until the next + * regularly scheduled run and raise a WARN notification event. + */ + protected void handleFailedSnapshotDispatch(final SnapshotScheduleVO snapshotToBeExecuted, final VolumeVO volume, + final SnapshotScheduleVO lockedSchedule, final Long eventId, final Exception cause) { + final long volumeId = volume.getId(); + final Account account = _acctDao.findById(volume.getAccountId()); + final int maxFailures = getScopedConfigValue(SnapshotManager.SnapshotRecurringMaxFailures, volume, account); + final int retryInterval = getScopedConfigValue(SnapshotManager.SnapshotRecurringRetryInterval, volume, account); + final int totalFailures = countConsecutiveFailedAttempts(volumeId, maxFailures) + 1; + + final String failureMessage = String.format("Failed to create scheduled snapshot for volume [%s]: %s", volume, cause.getMessage()); + if (eventId != null) { + ActionEventUtils.onCompletedActionEvent(User.UID_SYSTEM, volume.getAccountId(), EventVO.LEVEL_ERROR, + EventTypes.EVENT_SNAPSHOT_CREATE, failureMessage, volumeId, ApiCommandResourceType.Volume.toString(), eventId); + } else { + ActionEventUtils.onCreatedActionEvent(User.UID_SYSTEM, volume.getAccountId(), EventVO.LEVEL_ERROR, + EventTypes.EVENT_SNAPSHOT_CREATE, true, failureMessage, volumeId, ApiCommandResourceType.Volume.toString()); + } + + if (maxFailures > 0 && totalFailures >= maxFailures) { + final Date nextRegularRun = getNextScheduledTime(snapshotToBeExecuted.getPolicyId(), _currentTimestamp); + lockedSchedule.setScheduledTimestamp(nextRegularRun); + logger.warn("Snapshot schedule [{}] for volume [{}] has failed [{}] consecutive times; it will not be retried until its next regularly scheduled run at [{}].", + snapshotToBeExecuted, volume, totalFailures, nextRegularRun); + ActionEventUtils.onCreatedActionEvent(User.UID_SYSTEM, volume.getAccountId(), EventVO.LEVEL_WARN, EventTypes.EVENT_SNAPSHOT_CREATE, true, + String.format("Recurring snapshot for volume [%s] has failed %d consecutive times and will not be retried until its next regularly scheduled run.", volume, totalFailures), + volumeId, ApiCommandResourceType.Volume.toString()); + } else { + final Date nextRetry = new Date(_currentTimestamp.getTime() + retryInterval * 1000L); + lockedSchedule.setScheduledTimestamp(nextRetry); + logger.debug("Snapshot schedule [{}] for volume [{}] failed [{}] time(s); retrying at [{}].", + snapshotToBeExecuted, volume, totalFailures, nextRetry); + } + _snapshotScheduleDao.update(lockedSchedule.getId(), lockedSchedule); + } + + /** + * Counts how many of the most recent {@code EVENT_SNAPSHOT_CREATE} events logged for this volume are + * consecutive failures (level ERROR), starting from the most recent event and stopping at the first + * non-failure (or absent) event. This derives the "number of failed attempts" from event history instead of a + * dedicated counter column. + */ + protected int countConsecutiveFailedAttempts(final long volumeId, final int limit) { + if (limit <= 0) { + return 0; + } + final List<EventVO> recentEvents = _eventDao.listLatestEventsByResource(volumeId, ApiCommandResourceType.Volume.toString(), + EventTypes.EVENT_SNAPSHOT_CREATE, limit); + int count = 0; + for (final EventVO event : recentEvents) { + if (!EventVO.LEVEL_ERROR.equals(event.getLevel())) { + break; + } + count++; + } + return count; + } + + /** + * Resolves a config value in order of most to least specific scope: account, domain, zone, then global. A + * {@link ConfigKey} can only walk a single scope-parent chain automatically (Account->Domain->Global, or + * Zone->Global), so the four scopes are resolved manually here. + */ + protected <T> T getScopedConfigValue(final ConfigKey<T> key, final VolumeVO volume, final Account account) { + T value = key.valueInScope(ConfigKey.Scope.Account, volume.getAccountId(), true); + if (value == null && account != null) { + value = key.valueInScope(ConfigKey.Scope.Domain, account.getDomainId(), true); + } + if (value == null) { + value = key.valueInScope(ConfigKey.Scope.Zone, volume.getDataCenterId(), true); + } + if (value == null) { + value = key.value(); + } + return value; + } + + /** + * Implements https://github.com/apache/cloudstack/issues/6827: a recurring snapshot is redundant when nothing + * could have changed on the volume since the last one was taken, i.e. when the attached VM has not been running + * at any point since then. Volumes with no snapshot yet, or that are not attached to a VM, are never skipped. + */ + protected boolean shouldSkipUnchangedVolumeSnapshot(final VolumeVO volume) { + final Account account = _acctDao.findById(volume.getAccountId()); + if (!getScopedConfigValue(SnapshotManager.SnapshotSkipIfVmNotRunning, volume, account)) { + return false; + } + + final Long instanceId = volume.getInstanceId(); + if (instanceId == null) { + return false; + } + + final SnapshotVO lastSnapshot = findLastSnapshot(volume.getId()); + if (lastSnapshot == null) { + return false; + } + + final VMInstanceVO vm = _vmInstanceDao.findById(instanceId); + if (vm == null || vm.getPowerState() == VirtualMachine.PowerState.PowerOn) { + return false; + } + + final Date poweredOffSince = vm.getPowerStateUpdateTime(); + // If the VM's power state changed at or after the last snapshot, it may have been running (and the volume + // may have changed) at some point since; only skip when it has been off since strictly before that snapshot. + return poweredOffSince != null && poweredOffSince.before(lastSnapshot.getCreated()); + } + + protected SnapshotVO findLastSnapshot(final long volumeId) { + final Filter filter = new Filter(SnapshotVO.class, "created", false, 0L, 1L); + final List<SnapshotVO> snapshots = _snapshotDao.listByVolumeId(filter, volumeId); + return (snapshots == null || snapshots.isEmpty()) ? null : snapshots.get(0); + } + + /** + * Advances a schedule that was skipped (see {@link #shouldSkipUnchangedVolumeSnapshot}) to its next regularly + * scheduled run, and logs an informational event so the skip is visible and not mistaken for a missed snapshot. + */ + @DB + protected void skipAndRescheduleSnapshot(final SnapshotScheduleVO snapshotToBeExecuted, final VolumeVO volume) { + SnapshotScheduleVO lockedSchedule = null; + final long snapshotScheId = snapshotToBeExecuted.getId(); + try { + lockedSchedule = _snapshotScheduleDao.acquireInLockTable(snapshotScheId); + if (lockedSchedule == null) { + return; + } + final Date nextRegularRun = getNextScheduledTime(snapshotToBeExecuted.getPolicyId(), _currentTimestamp); + lockedSchedule.setScheduledTimestamp(nextRegularRun); + _snapshotScheduleDao.update(snapshotScheId, lockedSchedule); + logger.info("Skipped scheduled snapshot [{}] for volume [{}] because its instance has not been running since the last snapshot; next run at [{}].", + snapshotToBeExecuted, volume, nextRegularRun); + ActionEventUtils.onCreatedActionEvent(User.UID_SYSTEM, volume.getAccountId(), EventVO.LEVEL_INFO, EventTypes.EVENT_SNAPSHOT_SKIPPED, true, + String.format("Skipped scheduled snapshot for volume [%s] because its instance has not been running since the last snapshot.", volume), + volume.getId(), ApiCommandResourceType.Volume.toString()); + } finally { + if (lockedSchedule != null) { + _snapshotScheduleDao.releaseFromLockTable(snapshotScheId); + } + } + } + /** * Verifies if a snapshot for a volume can be scheduled or not based on volume and account status, and removes it from the snapshot scheduler if its policy was removed. * diff --git a/server/src/test/java/com/cloud/storage/snapshot/SnapshotSchedulerImplTest.java b/server/src/test/java/com/cloud/storage/snapshot/SnapshotSchedulerImplTest.java index 3827531891f..4177a23553e 100644 --- a/server/src/test/java/com/cloud/storage/snapshot/SnapshotSchedulerImplTest.java +++ b/server/src/test/java/com/cloud/storage/snapshot/SnapshotSchedulerImplTest.java @@ -16,9 +16,13 @@ // under the License. package com.cloud.storage.snapshot; +import com.cloud.event.ActionEventUtils; +import com.cloud.event.EventVO; +import com.cloud.event.dao.EventDao; import com.cloud.storage.Snapshot; import com.cloud.storage.SnapshotPolicyVO; import com.cloud.storage.SnapshotScheduleVO; +import com.cloud.storage.SnapshotVO; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.SnapshotPolicyDao; import com.cloud.storage.dao.SnapshotScheduleDao; @@ -26,6 +30,9 @@ import com.cloud.storage.dao.VolumeDao; import com.cloud.user.Account; import com.cloud.user.AccountVO; import com.cloud.user.dao.AccountDao; +import com.cloud.vm.VMInstanceVO; +import com.cloud.vm.VirtualMachine; +import com.cloud.vm.dao.VMInstanceDao; import org.apache.cloudstack.framework.jobs.dao.AsyncJobDao; import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO; import org.apache.cloudstack.jobs.JobInfo; @@ -34,11 +41,15 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; import org.mockito.Mock; +import org.mockito.MockedStatic; import org.mockito.Mockito; import org.mockito.Spy; import org.mockito.junit.MockitoJUnitRunner; +import org.springframework.test.util.ReflectionTestUtils; +import java.util.Collections; import java.util.Date; +import java.util.List; @RunWith(MockitoJUnitRunner.class) public class SnapshotSchedulerImplTest { @@ -77,6 +88,18 @@ public class SnapshotSchedulerImplTest { @Mock private AsyncJobVO asyncJobVoMock; + @Mock + private EventDao eventDaoMock; + + @Mock + private VMInstanceDao vmInstanceDaoMock; + + @Mock + private VMInstanceVO vmInstanceVoMock; + + @Mock + private SnapshotVO snapshotVoMock; + @Test public void scheduleNextSnapshotJobTestParameterIsNullReturnNull() { @@ -274,4 +297,213 @@ public class SnapshotSchedulerImplTest { Mockito.verify(snapshotSchedulerImplSpy, Mockito.never()).scheduleNextSnapshotJob(Mockito.any(SnapshotScheduleVO.class)); } + + // --- countConsecutiveFailedAttempts (#13454) --- + + @Test + public void countConsecutiveFailedAttemptsTestLimitZeroReturnsZeroWithoutQuerying() { + int result = snapshotSchedulerImplSpy.countConsecutiveFailedAttempts(1L, 0); + + Assert.assertEquals(0, result); + Mockito.verify(eventDaoMock, Mockito.never()).listLatestEventsByResource(Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), Mockito.anyInt()); + } + + @Test + public void countConsecutiveFailedAttemptsTestStopsAtFirstNonErrorEvent() { + EventVO error1 = Mockito.mock(EventVO.class); + Mockito.doReturn(EventVO.LEVEL_ERROR).when(error1).getLevel(); + EventVO error2 = Mockito.mock(EventVO.class); + Mockito.doReturn(EventVO.LEVEL_ERROR).when(error2).getLevel(); + EventVO success = Mockito.mock(EventVO.class); + Mockito.doReturn(EventVO.LEVEL_INFO).when(success).getLevel(); + + Mockito.doReturn(List.of(error1, error2, success)).when(eventDaoMock).listLatestEventsByResource(Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), Mockito.anyInt()); + + int result = snapshotSchedulerImplSpy.countConsecutiveFailedAttempts(1L, 3); + + Assert.assertEquals(2, result); + } + + @Test + public void countConsecutiveFailedAttemptsTestNoEventsReturnsZero() { + Mockito.doReturn(Collections.emptyList()).when(eventDaoMock).listLatestEventsByResource(Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), Mockito.anyInt()); + + int result = snapshotSchedulerImplSpy.countConsecutiveFailedAttempts(1L, 3); + + Assert.assertEquals(0, result); + } + + // --- getScopedConfigValue (#13454) --- + + @Test + public void getScopedConfigValueTestFallsBackToGlobalDefaultWhenNoDepotConfigured() { + Mockito.doReturn(1L).when(volumeVoMock).getAccountId(); + Mockito.doReturn(1L).when(volumeVoMock).getDataCenterId(); + Mockito.doReturn(1L).when(accountVoMock).getDomainId(); + + Integer result = snapshotSchedulerImplSpy.getScopedConfigValue(SnapshotManager.SnapshotRecurringMaxFailures, volumeVoMock, accountVoMock); + + Assert.assertEquals(Integer.valueOf(3), result); + } + + // --- handleFailedSnapshotDispatch (#13454) --- + + @Test + public void handleFailedSnapshotDispatchTestUnderMaxReschedulesWithRetryIntervalOnly() { + Mockito.doReturn(1L).when(volumeVoMock).getId(); + Mockito.doReturn(1L).when(volumeVoMock).getAccountId(); + Mockito.doReturn(1L).when(volumeVoMock).getDataCenterId(); + Mockito.doReturn(accountVoMock).when(accountDaoMock).findById(Mockito.anyLong()); + Mockito.doReturn(Collections.emptyList()).when(eventDaoMock).listLatestEventsByResource(Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), Mockito.anyInt()); + ReflectionTestUtils.setField(snapshotSchedulerImplSpy, "_currentTimestamp", new Date()); + + try (MockedStatic<ActionEventUtils> actionEventUtilsMocked = Mockito.mockStatic(ActionEventUtils.class)) { + snapshotSchedulerImplSpy.handleFailedSnapshotDispatch(snapshotScheduleVoMock, volumeVoMock, snapshotScheduleVoMock, 5L, new Exception("boom")); + + actionEventUtilsMocked.verify(() -> ActionEventUtils.onCompletedActionEvent( + Mockito.anyLong(), Mockito.anyLong(), Mockito.eq(EventVO.LEVEL_ERROR), Mockito.anyString(), Mockito.anyString(), Mockito.anyLong(), Mockito.anyString(), Mockito.eq(5L))); + actionEventUtilsMocked.verify(() -> ActionEventUtils.onCreatedActionEvent( + Mockito.anyLong(), Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), Mockito.anyBoolean(), Mockito.anyString(), Mockito.anyLong(), Mockito.anyString()), Mockito.never()); + } + + Mockito.verify(snapshotScheduleVoMock).setScheduledTimestamp(Mockito.any(Date.class)); + Mockito.verify(snapshotScheduleDaoMock).update(Mockito.anyLong(), Mockito.eq(snapshotScheduleVoMock)); + } + + @Test + public void handleFailedSnapshotDispatchTestAtMaxGivesUpAndNotifies() { + Mockito.doReturn(1L).when(volumeVoMock).getId(); + Mockito.doReturn(1L).when(volumeVoMock).getAccountId(); + Mockito.doReturn(1L).when(volumeVoMock).getDataCenterId(); + Mockito.doReturn(accountVoMock).when(accountDaoMock).findById(Mockito.anyLong()); + + EventVO error1 = Mockito.mock(EventVO.class); + Mockito.doReturn(EventVO.LEVEL_ERROR).when(error1).getLevel(); + EventVO error2 = Mockito.mock(EventVO.class); + Mockito.doReturn(EventVO.LEVEL_ERROR).when(error2).getLevel(); + Mockito.doReturn(List.of(error1, error2)).when(eventDaoMock).listLatestEventsByResource(Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), Mockito.anyInt()); + + Mockito.doReturn(1L).when(snapshotScheduleVoMock).getPolicyId(); + Mockito.doReturn(null).when(snapshotPolicyDaoMock).findById(Mockito.anyLong()); + + try (MockedStatic<ActionEventUtils> actionEventUtilsMocked = Mockito.mockStatic(ActionEventUtils.class)) { + snapshotSchedulerImplSpy.handleFailedSnapshotDispatch(snapshotScheduleVoMock, volumeVoMock, snapshotScheduleVoMock, null, new Exception("boom")); + + actionEventUtilsMocked.verify(() -> ActionEventUtils.onCreatedActionEvent( + Mockito.anyLong(), Mockito.anyLong(), Mockito.eq(EventVO.LEVEL_ERROR), Mockito.anyString(), Mockito.anyBoolean(), Mockito.anyString(), Mockito.anyLong(), Mockito.anyString())); + actionEventUtilsMocked.verify(() -> ActionEventUtils.onCreatedActionEvent( + Mockito.anyLong(), Mockito.anyLong(), Mockito.eq(EventVO.LEVEL_WARN), Mockito.anyString(), Mockito.anyBoolean(), Mockito.anyString(), Mockito.anyLong(), Mockito.anyString())); + } + + Mockito.verify(snapshotScheduleDaoMock).update(Mockito.anyLong(), Mockito.eq(snapshotScheduleVoMock)); + } + + // --- shouldSkipUnchangedVolumeSnapshot (#6827) --- + + @Test + public void shouldSkipUnchangedVolumeSnapshotTestConfigDisabledByDefaultReturnsFalse() { + Mockito.doReturn(1L).when(volumeVoMock).getAccountId(); + Mockito.doReturn(1L).when(volumeVoMock).getDataCenterId(); + Mockito.doReturn(accountVoMock).when(accountDaoMock).findById(Mockito.anyLong()); + + boolean result = snapshotSchedulerImplSpy.shouldSkipUnchangedVolumeSnapshot(volumeVoMock); + + Assert.assertFalse(result); + Mockito.verifyNoInteractions(vmInstanceDaoMock); + } + + @Test + public void shouldSkipUnchangedVolumeSnapshotTestNoInstanceReturnsFalse() { + stubSkipConfigEnabled(); + Mockito.doReturn(null).when(volumeVoMock).getInstanceId(); + + boolean result = snapshotSchedulerImplSpy.shouldSkipUnchangedVolumeSnapshot(volumeVoMock); + + Assert.assertFalse(result); + } + + @Test + public void shouldSkipUnchangedVolumeSnapshotTestNoPriorSnapshotReturnsFalse() { + stubSkipConfigEnabled(); + Mockito.doReturn(5L).when(volumeVoMock).getInstanceId(); + Mockito.doReturn(1L).when(volumeVoMock).getId(); + Mockito.doReturn(null).when(snapshotSchedulerImplSpy).findLastSnapshot(Mockito.anyLong()); + + boolean result = snapshotSchedulerImplSpy.shouldSkipUnchangedVolumeSnapshot(volumeVoMock); + + Assert.assertFalse(result); + } + + @Test + public void shouldSkipUnchangedVolumeSnapshotTestVmRunningReturnsFalse() { + stubSkipConfigEnabled(); + Mockito.doReturn(5L).when(volumeVoMock).getInstanceId(); + Mockito.doReturn(1L).when(volumeVoMock).getId(); + Mockito.doReturn(snapshotVoMock).when(snapshotSchedulerImplSpy).findLastSnapshot(Mockito.anyLong()); + Mockito.doReturn(vmInstanceVoMock).when(vmInstanceDaoMock).findById(5L); + Mockito.doReturn(VirtualMachine.PowerState.PowerOn).when(vmInstanceVoMock).getPowerState(); + + boolean result = snapshotSchedulerImplSpy.shouldSkipUnchangedVolumeSnapshot(volumeVoMock); + + Assert.assertFalse(result); + } + + @Test + public void shouldSkipUnchangedVolumeSnapshotTestVmOffSinceBeforeLastSnapshotReturnsTrue() { + stubSkipConfigEnabled(); + Mockito.doReturn(5L).when(volumeVoMock).getInstanceId(); + Mockito.doReturn(1L).when(volumeVoMock).getId(); + Mockito.doReturn(new Date(2000L)).when(snapshotVoMock).getCreated(); + Mockito.doReturn(snapshotVoMock).when(snapshotSchedulerImplSpy).findLastSnapshot(Mockito.anyLong()); + Mockito.doReturn(vmInstanceVoMock).when(vmInstanceDaoMock).findById(5L); + Mockito.doReturn(VirtualMachine.PowerState.PowerOff).when(vmInstanceVoMock).getPowerState(); + Mockito.doReturn(new Date(1000L)).when(vmInstanceVoMock).getPowerStateUpdateTime(); + + boolean result = snapshotSchedulerImplSpy.shouldSkipUnchangedVolumeSnapshot(volumeVoMock); + + Assert.assertTrue(result); + } + + @Test + public void shouldSkipUnchangedVolumeSnapshotTestVmOffSinceAfterLastSnapshotReturnsFalse() { + stubSkipConfigEnabled(); + Mockito.doReturn(5L).when(volumeVoMock).getInstanceId(); + Mockito.doReturn(1L).when(volumeVoMock).getId(); + Mockito.doReturn(new Date(1000L)).when(snapshotVoMock).getCreated(); + Mockito.doReturn(snapshotVoMock).when(snapshotSchedulerImplSpy).findLastSnapshot(Mockito.anyLong()); + Mockito.doReturn(vmInstanceVoMock).when(vmInstanceDaoMock).findById(5L); + Mockito.doReturn(VirtualMachine.PowerState.PowerOff).when(vmInstanceVoMock).getPowerState(); + Mockito.doReturn(new Date(2000L)).when(vmInstanceVoMock).getPowerStateUpdateTime(); + + boolean result = snapshotSchedulerImplSpy.shouldSkipUnchangedVolumeSnapshot(volumeVoMock); + + Assert.assertFalse(result); + } + + private void stubSkipConfigEnabled() { + Mockito.doReturn(true).when(snapshotSchedulerImplSpy).getScopedConfigValue(Mockito.eq(SnapshotManager.SnapshotSkipIfVmNotRunning), Mockito.any(), Mockito.any()); + Mockito.doReturn(1L).when(volumeVoMock).getAccountId(); + Mockito.doReturn(accountVoMock).when(accountDaoMock).findById(Mockito.anyLong()); + } + + @Test + public void skipAndRescheduleSnapshotTestUpdatesScheduleAndLogsEvent() { + Mockito.doReturn(snapshotScheduleVoMock).when(snapshotScheduleDaoMock).acquireInLockTable(Mockito.anyLong()); + Mockito.doReturn(1L).when(snapshotScheduleVoMock).getId(); + Mockito.doReturn(1L).when(snapshotScheduleVoMock).getPolicyId(); + Mockito.doReturn(null).when(snapshotPolicyDaoMock).findById(Mockito.anyLong()); + Mockito.doReturn(1L).when(volumeVoMock).getAccountId(); + + try (MockedStatic<ActionEventUtils> actionEventUtilsMocked = Mockito.mockStatic(ActionEventUtils.class)) { + snapshotSchedulerImplSpy.skipAndRescheduleSnapshot(snapshotScheduleVoMock, volumeVoMock); + + actionEventUtilsMocked.verify(() -> ActionEventUtils.onCreatedActionEvent( + Mockito.anyLong(), Mockito.anyLong(), Mockito.eq(EventVO.LEVEL_INFO), Mockito.eq(com.cloud.event.EventTypes.EVENT_SNAPSHOT_SKIPPED), + Mockito.anyBoolean(), Mockito.anyString(), Mockito.anyLong(), Mockito.anyString())); + } + + Mockito.verify(snapshotScheduleVoMock).setScheduledTimestamp(Mockito.any()); + Mockito.verify(snapshotScheduleDaoMock).update(Mockito.eq(1L), Mockito.eq(snapshotScheduleVoMock)); + Mockito.verify(snapshotScheduleDaoMock).releaseFromLockTable(1L); + } }
