From 28d219158bcb85e110989072d4364225b3aa1f89 Mon Sep 17 00:00:00 2001 From: Vishal Rana Date: Sun, 6 Sep 2026 08:12:06 -0700 Subject: [PATCH] fix(status): do not go quiet on a host whose units predate run records MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Run records are written by the unit, so a host still running units from before this change has none. `ob status` had stopped reading systemd's own result entirely, which meant upgrading `ob` — with no deploy, no change on the host — turned a failing scheduled job into a clean report and an exit 0. That is the one direction this feature exists to close. With no records, systemd's retained result is the only evidence there is, and status uses it and says so: the issue names the failure and points at `ob schedule apply`, which is what starts the records. Once records exist they remain the verdict, and a stale systemd result raises nothing. Claude-Session: https://claude.ai/code/session_013MqwrF5NA179khDtEQdib5 --- internal/engine/schedule_status.go | 18 ++++- internal/engine/schedule_test.go | 79 +++++++++++++++++++ .../content/docs/guides/schedule-a-job.mdx | 5 ++ 3 files changed, 101 insertions(+), 1 deletion(-) diff --git a/internal/engine/schedule_status.go b/internal/engine/schedule_status.go index dac4667..84c3ff8 100644 --- a/internal/engine/schedule_status.go +++ b/internal/engine/schedule_status.go @@ -48,6 +48,8 @@ const skipStreakIssue = 3 type scheduleUnitObservation struct { loadState string activeState string + result string + exitStatus int release string startedAt string attempt string @@ -72,7 +74,9 @@ func (e *Engine) scheduleStatuses(ctx context.Context) ([]StatusSchedule, error) unit := e.names().ScheduledJobUnit(job.Name) commands = append(commands, "printf '%s\\n' "+q("@@"+job.Name+":service"), - "systemctl show "+q(unit+".service")+" --no-pager --property=LoadState --property=ActiveState", + // Result and ExecMainStatus are the only evidence a host still + // running pre-record units has. See the fallback below. + "systemctl show "+q(unit+".service")+" --no-pager --property=LoadState --property=ActiveState --property=Result --property=ExecMainStatus", "printf '%s\\n' "+q("@@"+job.Name+":timer"), "systemctl show "+q(unit+".timer")+" --no-pager --property=LoadState --property=ActiveState --property=NextElapseUSecRealtime", "printf '%s\\n' "+q("@@"+job.Name+":run"), @@ -104,8 +108,10 @@ func (e *Engine) scheduleStatuses(ctx context.Context) ([]StatusSchedule, error) if observed[name] == nil { observed[name] = map[string]scheduleUnitObservation{} } + exit, _ := strconv.Atoi(values["ExecMainStatus"]) observed[name][kind] = scheduleUnitObservation{ loadState: values["LoadState"], activeState: values["ActiveState"], + result: values["Result"], exitStatus: exit, release: values["release"], startedAt: values["started_at"], attempt: values["attempt"], next: values["NextElapseUSecRealtime"], history: parseScheduleRunRecords(strings.Join(raw, "\n"), name), @@ -213,6 +219,16 @@ func (e *Engine) scheduleStatuses(ctx context.Context) ([]StatusSchedule, error) } status.Issues = append(status.Issues, issue) } + // No records at all means the host is still running units written + // before this runner: they do not write records, so the only thing + // that knows how the last run ended is systemd. Upgrading `ob` must + // not turn a failing job silent, and the issue says what closes the + // gap. + if len(records) == 0 && service.result != "" && service.result != "success" { + status.Issues = append(status.Issues, fmt.Sprintf( + "last run failed: %s (exit %d); no run record — run `ob schedule apply` so runs are recorded", + service.result, service.exitStatus)) + } if status.ConsecutiveSkips >= skipStreakIssue { status.Issues = append(status.Issues, fmt.Sprintf("skipped %d firings in a row: %s", status.ConsecutiveSkips, status.LastReason)) } diff --git a/internal/engine/schedule_test.go b/internal/engine/schedule_test.go index 82be9a1..3f0aeca 100644 --- a/internal/engine/schedule_test.go +++ b/internal/engine/schedule_test.go @@ -1604,3 +1604,82 @@ func TestScheduleSkipMarkerIsNamedIdenticallyOnBothSides(t *testing.T) { t.Fatalf("runner names the note %q and the notifier %q", inRunner, inNotifier) } } + +// Upgrading ob must not make a failing job go quiet. A host still running +// units written before this runner records nothing, so systemd's own result +// is the only evidence there is, and status has to use it. +func TestScheduleStatusFallsBackToSystemdWhenNoRecordsExist(t *testing.T) { + cfg := testConfig() + cfg.Workloads["nightly"] = app.Workload{ + Role: app.RoleJob, When: "manual", DataEffect: "none", + Schedule: &app.JobSchedule{Cron: "0 2 * * *", Timezone: "UTC", Timeout: "1h", CatchUp: true}, + } + f := &transport.Fake{Dynamic: func(cmd string) (transport.Result, bool) { + if strings.Contains(cmd, "systemctl show") { + return transport.Result{Stdout: `@@journal +persistent +@@nightly:service +LoadState=loaded +ActiveState=failed +Result=timeout +ExecMainStatus=143 +@@nightly:timer +LoadState=loaded +ActiveState=active +@@nightly:run +@@nightly:history +`}, true + } + return transport.Result{}, false + }} + e := New(cfg, testProject(t), f, Options{Out: &bytes.Buffer{}, Sleep: noSleep}) + statuses, err := e.scheduleStatuses(context.Background()) + if err != nil { + t.Fatal(err) + } + got := statuses[0] + if !got.Diverged { + t.Fatalf("a failing job on a pre-record host reads as clean: %#v", got) + } + issues := strings.Join(got.Issues, "; ") + if !strings.Contains(issues, "last run failed: timeout (exit 143)") || !strings.Contains(issues, "ob schedule apply") { + t.Fatalf("issue does not name the failure or the remedy: %#v", got.Issues) + } +} + +// Once records exist they are the verdict, and systemd's retained result must +// not raise a second, contradictory issue. +func TestScheduleStatusPrefersRecordsOverSystemdResult(t *testing.T) { + cfg := testConfig() + cfg.Workloads["nightly"] = app.Workload{ + Role: app.RoleJob, When: "manual", DataEffect: "none", + Schedule: &app.JobSchedule{Cron: "0 2 * * *", Timezone: "UTC", Timeout: "1h", CatchUp: true}, + } + f := &transport.Fake{Dynamic: func(cmd string) (transport.Result, bool) { + if strings.Contains(cmd, "systemctl show") { + return transport.Result{Stdout: `@@journal +persistent +@@nightly:service +LoadState=loaded +ActiveState=inactive +Result=timeout +ExecMainStatus=143 +@@nightly:timer +LoadState=loaded +ActiveState=active +@@nightly:run +@@nightly:history +{"run":"a1b2c3d4e5f60718293a4b5c6d7e8f90","job":"nightly","trigger":"timer","started_at":"2026-09-05T02:00:01Z","finished_at":"2026-09-05T02:01:02Z","duration_s":61,"attempts":1,"exit_status":0,"outcome":"success","inputs":{}} +`}, true + } + return transport.Result{}, false + }} + e := New(cfg, testProject(t), f, Options{Out: &bytes.Buffer{}, Sleep: noSleep}) + statuses, err := e.scheduleStatuses(context.Background()) + if err != nil { + t.Fatal(err) + } + if got := statuses[0]; got.Diverged || got.LastOutcome != "success" { + t.Fatalf("a stale systemd result outvoted the record: %#v", got) + } +} diff --git a/site/src/content/docs/guides/schedule-a-job.mdx b/site/src/content/docs/guides/schedule-a-job.mdx index 79087c4..23b7814 100644 --- a/site/src/content/docs/guides/schedule-a-job.mdx +++ b/site/src/content/docs/guides/schedule-a-job.mdx @@ -132,6 +132,11 @@ under the same application lock, schedule mutex, fence, and journal boundary as a deploy, but it does not stage or activate a release. Run it after upgrading Onebox when an application may not be deployed again soon. +Run records are written by the unit, so a host whose units predate them has +none until this command or a deploy rewrites them. Until then `ob status` falls +back to what systemd retained, reports a failure as it always did, and says +that no run record exists yet. + ## Every run leaves a record When a run ends, for any reason, the unit's `ExecStopPost` writes one record