| index 39def0f57..dc4757ec5 100644 |
| --- a/internal/host/systemapply.go |
| +++ b/internal/host/systemapply.go |
| @@ -115,6 +115,13 @@ func productionSysOps() sysOps { |
| defer cancel() |
| out, err := exec.CommandContext(ctx, name, args...).Output() |
| if err != nil { |
| + // Output() keeps stderr out of the returned bytes so the |
| + // caller can parse them; a command that failed is the one |
| + // case where what it said matters more than what it printed. |
| + var ee *exec.ExitError |
| + if errors.As(err, &ee) && len(ee.Stderr) > 0 { |
| + return string(out), fmt.Errorf("%s %s: %w: %s", name, strings.Join(args, " "), err, strings.TrimSpace(string(ee.Stderr))) |
| + } |
| return string(out), fmt.Errorf("%s %s: %w", name, strings.Join(args, " "), err) |
| } |
| return string(out), nil |
| @@ -187,12 +194,20 @@ func (a *systemApplier) Run(ctx context.Context, recover bool) error { |
| } |
| defer os.RemoveAll(a.stagingDir()) |
| |
| - if err := a.recoverIfNeeded(ctx); err != nil { |
| + recovered, err := a.recoverIfNeeded(ctx) |
| + if err != nil { |
| return err |
| } |
| if recover { |
| return nil |
| } |
| + if recovered { |
| + // The recovery moved the box and wrote the result this run answers |
| + // with — an apply on top of that would overwrite that result and |
| + // restart the edge a second time. The agent re-touches the marker |
| + // if it still wants the update, and by then it can read the result. |
| + return nil |
| + } |
| req, err := a.readRequest() |
| if errors.Is(err, os.ErrNotExist) { |
| return nil |
| @@ -201,6 +216,15 @@ func (a *systemApplier) Run(ctx context.Context, recover bool) error { |
| a.ops.logf("system apply: request: %v", err) |
| return nil // nothing to record against: the request itself is unreadable |
| } |
| + // A tuple this box has already rolled back is not tried again: the |
| + // build failed its gate here, and re-applying it on every marker touch |
| + // would restart the edge in a loop. Only those two outcomes stick — |
| + // refused, deferred and noted are all worth another go. |
| + if r, _ := readSystemResult(a.optDir); r.matches(a.channel, req.Version, req.PromotedAt) && |
| + (r.Outcome == systemOutcomeRolledBack || r.Outcome == systemOutcomeRollbackFailed) { |
| + a.ops.logf("system apply: %s %s was already %s on this box; not applying it again", a.channel, req.Version, r.Outcome) |
| + return nil |
| + } |
| p, err := a.verifyPointer(ctx) |
| if err != nil { |
| return a.record(req, err) |
| @@ -370,7 +394,9 @@ func (a *systemApplier) prevBuild(ctx context.Context) string { |
| } |
| |
| // stagePlan runs `<binary> bootstrap -root <dir> -plan-json` and returns |
| -// that binary's arming plan and the unit files it staged. |
| +// that binary's arming plan and the unit files it staged. The plan is left |
| +// in <dir>/plan.json next to them, so a later step can pick up the whole |
| +// staging (stagedPlan) without exec'ing the binary a second time. |
| func (a *systemApplier) stagePlan(ctx context.Context, binary, dir string) (bootstrapPlan, []string, error) { |
| os.RemoveAll(dir) |
| if err := os.MkdirAll(dir, 0o700); err != nil { |
| @@ -394,6 +420,9 @@ func (a *systemApplier) stagePlan(ctx context.Context, binary, dir string) (boot |
| units = append(units, e.Name()) |
| } |
| } |
| + if err := writeJSONDurable(filepath.Join(dir, "plan.json"), plan, 0o600); err != nil { |
| + return bootstrapPlan{}, nil, err |
| + } |
| return plan, units, nil |
| } |
| |
| @@ -567,25 +596,29 @@ func (a *systemApplier) arm(plan bootstrapPlan) error { |
| // arm the previous plan. applied.json is untouched — the .prev build is |
| // what passed a gate, and it is what applied.json already names. |
| // |
| -// Every exit closes the journal: what a rollback could do it has done, and |
| -// a journal left open would have the next run (or the next boot) redo the |
| -// same restore against a box a human now has to look at anyway. |
| +// A rollback that cannot finish leaves the journal open: the box is then |
| +// between two builds — the previous binary under the failed build's units, |
| +// say — and only another recovery puts it right. The one exception is a |
| +// complete restore whose GATE failed: the files are all the previous |
| +// build's, restoring them again would change nothing, and the box needs a |
| +// human rather than a loop. |
| func (a *systemApplier) rollback(ctx context.Context, req systemRequest, reason string, j *systemJournal) error { |
| a.ops.logf("system apply: %s %s: rolling back: %s", a.channel, j.Target, reason) |
| failed := func(what string, err error) error { |
| _ = a.writeResult(req, systemOutcomeRollbackFailed, reason+"; "+what+": "+err.Error()) |
| - a.removeJournal() |
| return fmt.Errorf("system apply: rollback of %s failed: %s: %v", j.Target, what, err) |
| } |
| plan, err := a.restoreFiles(ctx, j) |
| if err != nil { |
| - return failed("restore", err) |
| + return failed("restore", err) // journal kept: the restore is half done |
| } |
| if err := a.restartEdge(); err != nil { |
| - return failed("restart", err) |
| + return failed("restart", err) // journal kept: the edge is still to come back |
| } |
| if err := a.gate(ctx, j.PrevBuild); err != nil { |
| - return failed("previous build failed its gate too", err) |
| + err := failed("previous build failed its gate too", err) |
| + a.removeJournal() |
| + return err |
| } |
| if err := a.arm(plan); err != nil { |
| a.ops.logf("system apply: arm previous plan: %v", err) |
| @@ -610,21 +643,48 @@ func (a *systemApplier) restoreFiles(ctx context.Context, j *systemJournal) (boo |
| } |
| // The same quiesce the install did: a root pass that is mid-flight |
| // against the failed build must not run on across the swap back. |
| - a.ops.systemctl(append([]string{"stop"}, systemQuiesceUnits...)...) |
| + if _, err := a.ops.systemctl(append([]string{"stop"}, systemQuiesceUnits...)...); err != nil { |
| + a.ops.logf("system apply: quiesce: %v (continuing)", err) |
| + } |
| if err := copyFileDurable(prev, systemBinaryPath(a.optDir), 0o755); err != nil { |
| return bootstrapPlan{}, err |
| } |
| - // Staged from carlos.prev itself, not from the journal: the unit set to |
| - // put back is whatever the binary being restored ships. |
| + // The unit set to put back is whatever the binary being restored ships, |
| + // so it comes from carlos.prev and never from the journal. The install |
| + // already staged it, and that staging is preferred: re-running a |
| + // bootstrap on a box that is mid-rollback is one more thing that can |
| + // fail, and it would wipe the good units before it tried. |
| prevDir := filepath.Join(a.stagingDir(), "prev") |
| - plan, _, err := a.stagePlan(ctx, prev, prevDir) |
| + plan, err := stagedPlan(prevDir) |
| if err != nil { |
| - return bootstrapPlan{}, fmt.Errorf("carlos.prev bootstrap: %w", err) |
| + if plan, _, err = a.stagePlan(ctx, prev, prevDir); err != nil { |
| + return bootstrapPlan{}, fmt.Errorf("carlos.prev bootstrap: %w", err) |
| + } |
| } |
| if err := a.installUnits(ctx, prevDir, j.UnitsAdded); err != nil { |
| return bootstrapPlan{}, err |
| } |
| - if _, err := a.ops.systemctl(append([]string{"enable"}, plan.Enabled...)...); err != nil { |
| + if len(plan.Enabled) > 0 { |
| + if _, err := a.ops.systemctl(append([]string{"enable"}, plan.Enabled...)...); err != nil { |
| + return bootstrapPlan{}, err |
| + } |
| + } |
| + return plan, nil |
| +} |
| + |
| +// stagedPlan reads back what a stagePlan into dir left there: the plan the |
| +// binary printed and the units it wrote. An empty unit directory or a |
| +// missing plan.json means the staging is not there to be reused. |
| +func stagedPlan(dir string) (bootstrapPlan, error) { |
| + entries, err := os.ReadDir(filepath.Join(dir, "etc", "systemd", "system")) |
| + if err != nil { |
| + return bootstrapPlan{}, err |
| + } |
| + if len(entries) == 0 { |
| + return bootstrapPlan{}, fmt.Errorf("no units staged in %s", dir) |
| + } |
| + var plan bootstrapPlan |
| + if err := readJSONFile(filepath.Join(dir, "plan.json"), &plan); err != nil { |
| return bootstrapPlan{}, err |
| } |
| return plan, nil |
| @@ -667,36 +727,47 @@ func copyFileDurable(src, dst string, mode os.FileMode) error { |
| // that build proved itself, and undoing it would throw away a good apply. |
| // The journal is removed LAST, after the result is written and the restart |
| // issued, so a recovery that dies partway is recovered again. |
| -func (a *systemApplier) recoverIfNeeded(ctx context.Context) error { |
| +// |
| +// It reports whether it moved the box: a run that recovered has written the |
| +// result it answers with, and does not then go on to apply a request. |
| +func (a *systemApplier) recoverIfNeeded(ctx context.Context) (bool, error) { |
| j, err := readSystemJournal(a.optDir) |
| if err != nil || j == nil { |
| - return err |
| + return false, err |
| } |
| - // The request is a hint here too: it may be gone, or name something |
| - // else entirely. The journal is what this recovery is about, so it |
| - // names the result when the request cannot. |
| + // The result belongs to the apply being recovered, not to whatever the |
| + // agent has asked for since. The request is used only when it is that |
| + // same apply — then it carries the real promoted_at. |
| req, _ := a.readRequest() |
| - if req.Version == "" { |
| + if req.Version != j.Target { |
| req = systemRequest{Version: j.Target, PromotedAt: j.StartedAt} |
| } |
| a.ops.logf("system apply: recovering an apply of %s interrupted at %s", j.Target, j.Phase) |
| - switch j.Phase { |
| - case systemPhaseGated: |
| - plan, _, err := a.stagePlan(ctx, systemBinaryPath(a.optDir), filepath.Join(a.stagingDir(), "new")) |
| - if err != nil { |
| - return fmt.Errorf("system apply: recovery: %w", err) |
| - } |
| - return a.finish(req, j.Target, plan) |
| - case systemPhaseBackingUp: |
| + if j.Phase == systemPhaseBackingUp { |
| // Nothing on the box had changed yet: only a half-written .prev |
| - // copy can be left, and it is not the file anything reads. |
| + // copy can be left, and it is not the file anything reads. The run |
| + // carries on to the request it was started for. |
| os.Remove(systemPrevPath(a.optDir) + ".tmp") |
| a.removeJournal() |
| - return nil |
| + return false, nil |
| + } |
| + // Forward from gated — but only while the build that passed the gate is |
| + // still the one on disk. A binary that has moved since means this is a |
| + // pre-gate state whatever the journal says. |
| + if j.Phase == systemPhaseGated { |
| + if got := a.onDiskBuild(ctx); got != j.Target { |
| + a.ops.logf("system apply: recovery: %s passed its gate but the binary on disk reports %q; rolling back instead", j.Target, got) |
| + } else { |
| + plan, _, err := a.stagePlan(ctx, systemBinaryPath(a.optDir), filepath.Join(a.stagingDir(), "new")) |
| + if err != nil { |
| + return false, fmt.Errorf("system apply: recovery: %w", err) |
| + } |
| + return true, a.finish(req, j.Target, plan) |
| + } |
| } |
| plan, err := a.restoreFiles(ctx, j) |
| if err != nil { |
| - return fmt.Errorf("system apply: recovery: %w", err) // journal kept: the next run recovers again |
| + return false, fmt.Errorf("system apply: recovery: %w", err) // journal kept: the next run recovers again |
| } |
| if a.ops.edgeStartedThisBoot() { |
| a.ops.systemctl("reset-failed", systemEdgeUnit) |
| @@ -704,7 +775,7 @@ func (a *systemApplier) recoverIfNeeded(ctx context.Context) error { |
| // handler, and a blocking restart of the unit that triggered it |
| // would wait on the job that is waiting on this one. |
| if _, err := a.ops.systemctl("restart", "--no-block", systemEdgeUnit); err != nil { |
| - return fmt.Errorf("system apply: recovery: restart: %w", err) |
| + return false, fmt.Errorf("system apply: recovery: restart: %w", err) |
| } |
| var armed []string |
| for _, u := range plan.Enabled { |
| @@ -717,10 +788,10 @@ func (a *systemApplier) recoverIfNeeded(ctx context.Context) error { |
| } |
| } |
| if err := a.writeResult(req, systemOutcomeRolledBack, "interrupted at "+j.Phase); err != nil { |
| - return err |
| + return false, err |
| } |
| a.removeJournal() |
| - return nil |
| + return true, nil |
| } |
| |
| // record turns a refusal or deferral into result.json and a nil return. |
amadan