A failed action stops what follows; nothing else does

The previous commit continued past every failure, and the lab found the
cost immediately: the bootstrap's store-readiness gate failed, the apply
carried on and started the broker and control plane against a machine
that was not ready, and the database still initialising was shut down.

An action is the only shape whose purpose is to make something true
before the next thing needs it — which is why it is the only one with a
verify. The bootstrap is a row of them. Everything else is independent
state, and stopping there is what made one broken module hold a whole
machine hostage.

The report says which happened: "these things failed" and "these things
failed and the rest was never tried" are different machines.

internal/apply/apply.go

@@ -80,6 +80,11 @@ type Error struct {
 	// because "one thing failed" and "eleven things failed" are different machines and the first
 	// line is what somebody reads.
 	Others int
+
+	// Gated is true when what failed was an action, so nothing after it was attempted. A person
+	// reading a report needs to know the difference between "these things failed" and "these
+	// things failed and the rest was never tried".
+	Gated bool
 }
 
 func (e *Error) Error() string {
@@ -89,9 +94,14 @@ func (e *Error) Error() string {
 	} else if e.Others > 1 {
 		also = fmt.Sprintf(", and %d other resources also failed", e.Others)
 	}
-	return fmt.Sprintf("applying %q: %v%s\n\n%d resource(s) were applied and remain; "+
-		"everything was attempted, so what is not listed as failed was done.",
-		e.Resource, e.Err, also, len(e.Done.Outcomes))
+	rest := "everything was attempted, so what is not listed as failed was done."
+	if e.Gated {
+		// An action is a gate: it exists to make something true before the next thing needs it.
+		rest = "this is an action, so nothing after it was attempted — the machine is in " +
+			"whatever state that left it."
+	}
+	return fmt.Sprintf("applying %q: %v%s\n\n%d resource(s) were applied and remain; %s",
+		e.Resource, e.Err, also, len(e.Done.Outcomes), rest)
 }
 
 func (e *Error) Unwrap() error { return e.Err }
@@ -161,10 +171,30 @@ func Apply(
 		was, _ := known.Find(resource.Identity())
 		outcome, err := applyOne(ctx, sys, resource, run, changed, was, unseal)
 		if err != nil {
-			failures = append(failures, &Error{
-				Resource: resource.Identity(), Err: err, Done: report,
-			})
+			failed := &Error{Resource: resource.Identity(), Err: err, Done: report}
+			failures = append(failures, failed)
 			log(fmt.Sprintf("  failed %s (%s): %v", resource.Identity(), outcome.Target, err))
+
+			// **A failed action stops what follows. Nothing else does.**
+			//
+			// An action is the only shape whose purpose is to make something true *before* the
+			// next thing needs it — which is why it is the only one with a `verify`. The
+			// bootstrap is a row of them: the store answers, then its databases exist, then their
+			// schemas, then the broker. Carrying on past one that did not happen means starting
+			// things against a machine that is not ready, and on a small machine that is how a
+			// database still initialising gets its memory taken away and shuts down. Observed,
+			// in the lab, caused by an earlier version of this loop.
+			//
+			// Everything else is independent state. A package that will not install has nothing
+			// to do with a file on the other side of the declaration, and stopping there is what
+			// made one broken module hold a whole machine hostage
+			// (novox/hq 04-ISSUES/011).
+			if resource.Kind() == declaration.TypeAction {
+				failed.Done = report
+				failed.Others = len(failures) - 1
+				failed.Gated = true
+				return report, known, failed
+			}
 			continue
 		}
 

internal/apply/apply_test.go

@@ -1189,3 +1189,38 @@ func TestAnUntouchedFileIsStillUnchanged(t *testing.T) {
 		t.Errorf("an untouched file was reported as %q", got)
 	}
 }
+
+func TestAFailedActionStopsWhatFollows(t *testing.T) {
+	// An action is the only shape whose purpose is to make something true BEFORE the next thing
+	// needs it, which is why it is the only one with a verify. The bootstrap is a row of them:
+	// the store answers, then its databases exist, then their schemas, then the broker.
+	//
+	// Carrying on past one that did not happen starts things against a machine that is not ready
+	// — and on a small machine that is how a database still initialising has its memory taken
+	// away and shuts down. Observed in the lab, caused by a version of this loop that continued
+	// past everything.
+	dir := t.TempDir()
+	d := parseTrusted(t, `{"declaration":1,"resources":[
+	  {"id":"gate","type":"action","command":["false"],"verify":["false"]},
+	  {"id":"after","type":"file","path":"`+filepath.Join(dir, "after.conf")+`","content":"b\n"}
+	]}`)
+
+	_, _, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginCarried,
+		ExecRunner, nil, nil)
+	if err == nil {
+		t.Fatal("an action that cannot succeed did not fail the apply")
+	}
+	var applyErr *Error
+	if !errors.As(err, &applyErr) {
+		t.Fatalf("got %T", err)
+	}
+	if !applyErr.Gated {
+		t.Error("the failure does not say that nothing after it was attempted")
+	}
+	if _, statErr := os.Stat(filepath.Join(dir, "after.conf")); statErr == nil {
+		t.Error("the apply continued past a failed action, which is a gate")
+	}
+	if !strings.Contains(err.Error(), "nothing after it was attempted") {
+		t.Errorf("the message does not say the rest was not tried: %v", err)
+	}
+}