From 5a3f8083f920c00123e340a8da56d4f2e3d6ca31 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 8 Oct 2026 21:51:09 -0400 Subject: [PATCH] fix(ansible): a daily schedule holds its time of day instead of creeping later (#5432) Each firing stored last_run_at = the moment the ~60 s loop noticed it, and the next was due a whole interval after that, so every firing's lateness carried into the next: Nightly Docker Prune moved from 05:44:45 to 05:47:05 over six days. A firing now stores its due slot, the latest last_run_at + k * interval not after now (due_slot), for both a launch and a skipped firing. Missed slots after an outage are skipped, not run in a burst. Co-Authored-By: Claude Opus 5.5 --- steward/ansible/scheduler.py | 22 +++++++++++++++++++--- tests/test_ansible_schedule_due.py | 26 +++++++++++++++++++++++++- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/steward/ansible/scheduler.py b/steward/ansible/scheduler.py index c458847..62b3437 100644 --- a/steward/ansible/scheduler.py +++ b/steward/ansible/scheduler.py @@ -3,7 +3,7 @@ from __future__ import annotations import logging -from datetime import datetime, timezone +from datetime import datetime, timedelta, timezone from sqlalchemy import select @@ -26,6 +26,21 @@ def is_due(schedule: AnsibleSchedule, now: datetime) -> bool: return (now - schedule.last_run_at).total_seconds() >= schedule.interval_seconds +def due_slot(schedule: AnsibleSchedule, now: datetime) -> datetime: + """The due time a firing at `now` belongs to, kept as last_run_at. + + The latest last_run_at + k * interval not after now, so the next one is + due a whole interval after this slot rather than after whenever the ~60 s + loop happened to notice it. Counting from `now` added each firing's + lateness to the next, and a daily schedule crept later every day (#5432). + Missed slots (Steward was down) are skipped, not run in a burst. + """ + if schedule.last_run_at is None or not schedule.interval_seconds: + return now + whole = (now - schedule.last_run_at).total_seconds() // schedule.interval_seconds + return schedule.last_run_at + timedelta(seconds=whole * schedule.interval_seconds) + + async def run_due_schedules(app) -> int: """Trigger every due schedule once. Returns the number launched. A due schedule whose previous run hasn't finished skips this firing.""" @@ -39,6 +54,7 @@ async def run_due_schedules(app) -> int: for s in schedules: if not is_due(s, now): continue + slot = due_slot(s, now) async with app.db_sessionmaker() as db: busy = await run_in_flight(db, s.last_run_id) if busy: @@ -49,7 +65,7 @@ async def run_due_schedules(app) -> int: async with db.begin(): fresh = await db.get(AnsibleSchedule, s.id) if fresh: - fresh.last_run_at = now + fresh.last_run_at = slot fresh.last_error = "Skipped: the previous run was still going" logger.info("Schedule %s (%s) skipped: run %s still going", s.id, s.name, s.last_run_id) @@ -75,7 +91,7 @@ async def run_due_schedules(app) -> int: async with db.begin(): fresh = await db.get(AnsibleSchedule, s.id) if fresh: - fresh.last_run_at = now + fresh.last_run_at = slot fresh.last_run_id = run.id if run else None fresh.last_error = err if run: diff --git a/tests/test_ansible_schedule_due.py b/tests/test_ansible_schedule_due.py index 28d6658..92c09fb 100644 --- a/tests/test_ansible_schedule_due.py +++ b/tests/test_ansible_schedule_due.py @@ -2,7 +2,7 @@ from datetime import datetime, timedelta, timezone from types import SimpleNamespace -from steward.ansible.scheduler import is_due +from steward.ansible.scheduler import due_slot, is_due NOW = datetime(2026, 6, 16, 12, 0, 0, tzinfo=timezone.utc) @@ -26,3 +26,27 @@ def test_due_when_interval_elapsed(): def test_not_due_before_interval(): assert is_due(_sched(last=NOW - timedelta(seconds=1800), interval=3600), NOW) is False + + +def test_a_daily_schedule_holds_its_time_of_day(): + """#5432: noticed 40 s late every day, a daily run still belongs to its + 05:44:45 slot, so the lateness never adds up.""" + day = 86400 + last = datetime(2026, 10, 2, 5, 44, 45, tzinfo=timezone.utc) + for _ in range(6): + noticed = last + timedelta(seconds=day + 40) + assert is_due(_sched(last=last, interval=day), noticed) + last = due_slot(_sched(last=last, interval=day), noticed) + assert last == datetime(2026, 10, 8, 5, 44, 45, tzinfo=timezone.utc) + + +def test_missed_slots_are_skipped_not_run_in_a_burst(): + last = NOW - timedelta(hours=5, minutes=10) + slot = due_slot(_sched(last=last, interval=3600), NOW) + assert slot == NOW - timedelta(minutes=10) + assert not is_due(_sched(last=slot, interval=3600), NOW) + + +def test_a_first_run_is_its_own_slot(): + assert due_slot(_sched(last=None), NOW) == NOW +