diff --git a/internal/apply/apply.go b/internal/apply/apply.go index 4521d5a..efe8ed0 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -60,6 +60,10 @@ type Outcome struct { kept, keptMode, keptOwner string // stateless is a service whose unit's lifecycle is the machine's (novox/hq ADR 0117). stateless bool + // refused is a run-once process that was started and did not complete (novox/hq issue 467): only + // that clears what its record says it was run as. A step that failed before it was started — its + // bundle not fetched while the store was away — refused nothing, and its last success stands. + refused bool // scope and user are, for a service, whose manager it was applied through (novox/hq ADR 0177). scope, user string // found is, for a service, its unit as the host first found it (novox/hq ADR 0118). @@ -678,9 +682,11 @@ func ApplyMindingWindows( // and let ADR 0136's gate open on what it had just refused. Its record is kept — where its // unit and bundle are is what removes them when it is undeclared — with what it was run as // cleared, so every apply runs it again until it passes, and passing records its success - // as before. A run-once container needs none of this: what it reads is part of what it is + // as before. Only a step that was started and failed: one whose bundle could not be fetched + // refused nothing, and clearing it would run every step on the machine again once the + // artifact store came back from an outage. A run-once container needs none of this: what it reads is part of what it is // run as, so a refused input keeps it from matching its last success. - if p, ok := resource.(*declaration.Process); ok && p.RunOnce { + if outcome.refused { if rec, ok := known.Find(resource.Identity()); ok && rec.Wrote != "" { rec.Wrote = "" known.Record(rec) diff --git a/internal/apply/process.go b/internal/apply/process.go index 917e79b..bc5a3bf 100644 --- a/internal/apply/process.go +++ b/internal/apply/process.go @@ -172,10 +172,12 @@ func applyProcess(ctx context.Context, r *declaration.Process, run Runner, return out, err } if _, err := run(ctx, "systemctl", "start", r.Name+".service"); err != nil { + out.refused = true // started and did not complete: run again until it does (novox/hq issue 467) return out, fmt.Errorf("the %s step did not complete (journalctl -u %s.service says why): %w", r.Name, r.Name, err) } + // Updated whenever it has a record, whose Wrote a failed run cleared (novox/hq issue 467). out.Action = "created" - if previous.Wrote != "" { + if previous.Wrote != "" || previous.ID != "" { out.Action = "updated" } out.Detail = fmt.Sprintf("%d file(s), step completed", written) diff --git a/internal/apply/step_failed_test.go b/internal/apply/step_failed_test.go index 19d7d66..821df39 100644 --- a/internal/apply/step_failed_test.go +++ b/internal/apply/step_failed_test.go @@ -3,6 +3,8 @@ package apply import ( "context" "errors" + "net/http" + "net/http/httptest" "os" "path/filepath" "strings" @@ -25,6 +27,7 @@ type aStepBed struct { dir, body, digest string starts int // how many times the step's unit was started refuse bool // whether the step fails when started + storeGone bool // whether the bundle's store answers } func newStepBed(t *testing.T) *aStepBed { @@ -33,7 +36,18 @@ func newStepBed(t *testing.T) *aStepBed { unitDir, daemonRoot = units, bundles t.Cleanup(func() { unitDir, daemonRoot = wasUnits, wasBundles }) body, digest := anArchive(t, map[string]string{"check": "#!/bin/sh\n"}) - return &aStepBed{t: t, dir: t.TempDir(), body: serving(t, body), digest: digest} + b := &aStepBed{t: t, dir: t.TempDir(), digest: digest} + // The bundle's store, which can be made not to answer. + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if b.storeGone { + http.NotFound(w, nil) + return + } + _, _ = w.Write(body) + })) + t.Cleanup(server.Close) + b.body = server.URL + "/check.tar.gz" + return b } func (b *aStepBed) run(ctx context.Context, name string, args ...string) (string, error) { @@ -195,3 +209,46 @@ func TestAContainerStepThatFailedRunsAgain(t *testing.T) { t.Errorf("apply 5: a step that passed ran again: err %v, runs %d (want 4)", err, runs) } } + +func TestAStepWhoseBundleCouldNotBeFetchedIsNotRunAgainForIt(t *testing.T) { + // A fetch that fails is a failure of the step, and gates its module, but the step was never started: + // it did not refuse anything, and its last success stands. Cleared for it, every step on a machine + // would run again once the artifact store came back from an outage. + b := newStepBed(t) + _, known, err := b.apply(store.State{}, "good") + if err != nil || b.starts != 1 { + t.Fatalf("apply 1: err %v, starts %d", err, b.starts) + } + b.storeGone = true + report, known, err := b.apply(known, "good") + if err == nil || !skippedIn(report, "mod.after") || b.starts != 1 { + t.Fatalf("store gone: err %v, skipped %v, starts %d", err, skippedIn(report, "mod.after"), b.starts) + } + b.storeGone = false + report, _, err = b.apply(known, "good") + if err != nil || b.starts != 1 { + t.Errorf("store back: err %v, a step that passed and was never started since ran again (started %d times, want 1)", err, b.starts) + } + for _, o := range report.Outcomes { + if o.ID == "mod.check" && o.Action != "unchanged" { + t.Errorf("store back: the step reports %s: %s", o.Action, o.Detail) + } + } +} + +func TestAStepPassingAfterAFailureSaysItWasUpdated(t *testing.T) { + b := newStepBed(t) + _, known, _ := b.apply(store.State{}, "good") + b.refuse = true + _, known, _ = b.apply(known, "bad") + b.refuse = false + report, _, err := b.apply(known, "bad") + if err != nil { + t.Fatal(err) + } + for _, o := range report.Outcomes { + if o.ID == "mod.check" && o.Action != "updated" { + t.Errorf("a step with a record that passed after failing says %s, want updated", o.Action) + } + } +}