Clear a step's success only when it was started and failed, and say updated when it passes again (novox/hq issue 467)

A bundle that could not be fetched refused nothing; clearing for it would rerun
every step on a node once the artifact store came back from an outage.

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)

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)

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)
+		}
+	}
+}