daemon/internal/jobs: Preserve repeated reschedules across restarts

A restart after two or more reschedules replayed an occurrence that a
manual run had replaced: startup kept a persisted future occurrence
only when it was the one right after the upcoming occurrence.

Keep any persisted future occurrence when re-arming a schedule.
This also trusts an occurrence persisted while the clock ran ahead,
which cannot be told apart from repeated reschedules.

The test now reschedules through manual runs and a store reload.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>

daemon/internal/jobs/manager.go

@@ -191,14 +191,11 @@ func (m *Manager) Start(ctx context.Context) {
 			!m.firedFor(job.ID, missed)
 
 		if next, ok := schedule.next(now, loc); ok {
-			// A reschedule persisted the occurrence after the upcoming one;
-			// keep it rather than fire the occurrence it skipped. Only that
-			// exact occurrence is kept, so a clock stepped back cannot
-			// postpone the job any further.
+			// Each manual reschedule replaces an upcoming occurrence, so
+			// repeated reschedules can move the cursor several occurrences
+			// ahead. Keep it rather than replay any replaced occurrences.
 			if job.NextFireAtNano > next.UnixNano() {
-				if after, ok := schedule.next(next, loc); ok && after.UnixNano() == job.NextFireAtNano {
-					next = after
-				}
+				next = time.Unix(0, job.NextFireAtNano)
 			}
 
 			m.sched.arm(job.ID, schedule, loc, next)

daemon/internal/jobs/scheduler_test.go

@@ -258,37 +258,42 @@ func TestSchedulerCreateRejectsNeverFiringCron(t *testing.T) {
 	assert.Check(t, cerrdefs.IsInvalidArgument(err), "want invalid-argument, got %v", err)
 }
 
-// TestSchedulerStartKeepsRescheduledOccurrence checks that a restart keeps the
-// occurrence a reschedule persisted instead of firing the one it skipped, and
-// keeps no later one.
+// TestSchedulerStartKeepsRescheduledOccurrence checks that a restart does not
+// replay occurrences replaced by one or more manual runs.
 func TestSchedulerStartKeepsRescheduledOccurrence(t *testing.T) {
-	schedule, err := parseCron("0 3 * * *")
-	assert.NilError(t, err)
-
-	upcoming, _ := schedule.next(time.Now(), time.UTC)
-	rescheduled, _ := schedule.next(upcoming, time.UTC)
-
 	for _, tc := range []struct {
-		doc      string
-		stored   time.Time
-		expected time.Time
+		doc  string
+		runs int
 	}{
-		{doc: "the occurrence after the upcoming one is kept", stored: rescheduled, expected: rescheduled},
-		{doc: "a later occurrence is recomputed", stored: rescheduled.Add(24 * time.Hour), expected: upcoming},
+		{doc: "one reschedule", runs: 1},
+		{doc: "repeated reschedules", runs: 2},
 	} {
 		t.Run(tc.doc, func(t *testing.T) {
-			store, _ := newTestStore(t)
-			job := makeJob("job1", "nightly")
-			job.NextFireAtNano = tc.stored.UnixNano()
-			assert.NilError(t, store.CreateJob(t.Context(), job))
+			synctest.Test(t, func(t *testing.T) {
+				start := time.Now()
+				m, fake, kv := newRestartableManager(t)
+				job, _, err := m.Create(t.Context(), "nightly", scheduleSpec(""))
+				assert.NilError(t, err)
 
-			m := NewManager(store, newFakeBackend(), testNameGenerator)
-			m.Start(t.Context())
-			t.Cleanup(func() { assert.Check(t, m.Shutdown(shutdownCtx(t))) })
+				for range tc.runs {
+					run, err := m.Run(t.Context(), job.ID, true)
+					assert.NilError(t, err)
+					fake.exit(run.ContainerID, 0, nil)
+					waitTerminal(t, m, job.ID, run.ID)
+					waitJobSettled(t, m, job.ID, func(j *jobsv0.Job) bool { return j.State == jobsv0.JobStateIdle })
+				}
 
-			next, armed := m.sched.nextFor(job.ID)
-			assert.Assert(t, armed)
-			assert.Check(t, is.Equal(next.UnixNano(), tc.expected.UnixNano()))
+				expected := start.Add(3*time.Hour + time.Duration(tc.runs)*24*time.Hour).UnixNano()
+				before, err := m.Inspect(t.Context(), job.ID)
+				assert.NilError(t, err)
+				assert.Equal(t, before.NextFireAtNano, expected)
+
+				restarted := restartManager(t, m, kv, fake)
+				restarted.Start(t.Context())
+				after, err := restarted.Inspect(t.Context(), job.ID)
+				assert.NilError(t, err)
+				assert.Equal(t, after.NextFireAtNano, expected)
+			})
 		})
 	}
 }