amadan
adopt: a rolled-back restart starts the service again
The rollback repointed the symlink and stopped, leaving a correct symlink and no process. And because try-restart is a no-op on a unit that is not running — the property that lets a deliberately-stopped service stay stopped — the NEXT pass exited 0 and stamped SetVersions over a service that was not running at all: the exact lying header the mechanism exists to prevent, now with nothing behind it. Every unit-backed route has this. It stayed hidden because the pinfra roll script's explicit post-pass restart papers over it on the only unit-stamped route in the fleet.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
4 files changed,
+469
−6
internal/host/agent.go+92 −4internal/host/agent_test.go+124 −2internal/release/adopt.go+75 −0internal/release/adopt_test.go+178 −0
diff --git a/internal/host/agent.go b/internal/host/agent.go| index b7498b778..1747f5053 100644 |
| --- a/internal/host/agent.go |
| +++ b/internal/host/agent.go |
| @@ -456,9 +456,18 @@ func adoptLoop(reg *registry.Registry, svc *activator.Service) { |
| // instance would go on serving the binary its symlink no longer names |
| // until it happened to go idle. Safe because makeRestartRoute cannot |
| // start a process on either of its paths; see release.RestartHibernating. |
| + // |
| + // WithStartUnit rides wherever the named-unit restart arm can fire: |
| + // a failed named-unit restart that this pass rolled back must not |
| + // leave the service down (release.WithStartUnit). It asks for no |
| + // authority the restart above did not already need — both are one |
| + // systemctl verb on the same unit — and it cannot wake anything: it |
| + // fires only after a try-restart RETURNED AN ERROR, which a stopped |
| + // unit's exit-0 no-op never does. |
| if err := release.Pass(ctx, reg, store, pub, optDir, restart, |
| release.BakeDir(dataDir()), release.OnStageBlocked(stageRequester(dataDir())), |
| - release.RestartHibernating(restart)); err != nil { |
| + release.RestartHibernating(restart), |
| + release.WithStartUnit(makeStartRoute(reg, systemctlStartUnit))); err != nil { |
| log.Printf("agent: adopt pass: %v", err) |
| } |
| } |
| @@ -963,11 +972,22 @@ type fastDeps struct { |
| // are the warm ones somebody is watching. Pinned by |
| // TestFastSweepBouncesAWarmHibernatingCanary: dropping it would restore |
| // a dead-branch production bug without failing any other test. |
| -func fastPassOpts(dd string, stageReq func(host, ver string, err error), restart func(string) error) []release.PassOption { |
| +// |
| +// WithStartUnit IS wired here too (start — makeStartRoute in production, |
| +// nil in tests that never reach a rollback): any pass whose restart |
| +// callback carries the named-unit arm must also carry the start that |
| +// undoes that arm's failed restart, or its rollback leaves the service |
| +// down and the next pass stamps a never-run build |
| +// (release.WithStartUnit). It asks for no authority the try-restart arm |
| +// did not already need, and it cannot fire for a sleeper: only an |
| +// ERRORED restart reaches it, and every arm that can meet a sleeping |
| +// route swallows its failure. |
| +func fastPassOpts(dd string, stageReq func(host, ver string, err error), restart, start func(string) error) []release.PassOption { |
| return []release.PassOption{ |
| release.BakeDir(dd), |
| release.OnStageBlocked(stageReq), |
| release.RestartHibernating(restart), |
| + release.WithStartUnit(start), |
| } |
| } |
| @@ -1159,7 +1179,7 @@ func fastConvergeLoop(reg *registry.Registry, svc *activator.Service) { |
| restart := makeRestartRoute(reg, systemctlTryRestart, systemctlTryRestartUnit, bounceFn(svc)) |
| // Built once, used by both pass shapes below — see fastPassOpts for the |
| // DO-NOT list and why the stage requester is per-loop, not per-tick. |
| - opts := fastPassOpts(dd, stageRequester(dd), restart) |
| + opts := fastPassOpts(dd, stageRequester(dd), restart, makeStartRoute(reg, systemctlStartUnit)) |
| // The restart check's memory (issue #113), seeded ONCE here — one read per |
| // app at arming and never again except on a serial move, which is what |
| // keeps an idle box at exactly one serial GET per tick. See restartSeed for |
| @@ -2058,6 +2078,40 @@ func makeRestartRoute(reg *registry.Registry, tryRestart func(host string), tryR |
| } |
| } |
| +// makeStartRoute builds the adopt pass's rollback-start callback |
| +// (release.WithStartUnit): after a restart that FAILED has had its symlink |
| +// rolled back, start the unit the route NAMES on the restored version — |
| +// `systemctl start`, not try-restart, because the premise of that path is |
| +// that the unit is down (a try-restart there is a no-op and the box stays |
| +// serving nothing). |
| +// |
| +// ONE arm, deliberately, where makeRestartRoute has three: only a route that |
| +// names its own unit has anything for this callback to start. The other two |
| +// process owners never route here at all — their restart arms swallow their |
| +// failures and return nil, so the rollback path this callback belongs to is |
| +// unreachable for them — and a start of carlos-app@<host> or an activator |
| +// child from here could WAKE a sleeper, which no adopt path may do. A route |
| +// with an empty Unit, an unknown host, or a registry that will not answer |
| +// therefore starts NOTHING and returns nil: for those shapes "nothing was |
| +// stopped by the failed restart" is the only answer this callback can stand |
| +// over. (The converged-daemon rollback is caps.Restore's business, not |
| +// this callback's — see release.WithStartUnit.) |
| +// |
| +// This cannot wake anything: it fires only after a try-restart of the named |
| +// unit returned non-zero, and try-restart exits 0 on a loaded-but-stopped |
| +// unit — so a deliberately stopped service never produces the error that |
| +// reaches this, and a unit whose name resolves to nothing gives systemctl |
| +// start nothing to bring up either. |
| +func makeStartRoute(reg *registry.Registry, startUnit func(unit string) error) func(host string) error { |
| + return func(host string) error { |
| + rt, err := reg.Get(host) |
| + if err != nil || rt == nil || rt.Unit == "" { |
| + return nil |
| + } |
| + return startUnit(rt.Unit) |
| + } |
| +} |
| + |
| // systemctlTryRestart is makeRestartRoute's production unit path: cycle the |
| // route's carlos-app@ unit iff it's running, best-effort. A missing or |
| // stopped unit is not this pass's problem — try-restart is by definition a |
| @@ -2101,6 +2155,29 @@ func systemctlTryRestartUnit(unit string) error { |
| return nil |
| } |
| +// systemctlStartUnit is makeStartRoute's production path: bring the named |
| +// unit up after a rolled-back restart left it down. START, not try-restart — |
| +// try-restart is a no-op on a stopped unit, which on this path would leave |
| +// the box with a correct symlink and no process, and hand the NEXT pass's |
| +// try-restart an exit 0 to stamp a never-run build with. Idempotent where it |
| +// matters: `systemctl start` of an already-running unit exits 0 and does |
| +// nothing, so a service something else restored first is not cycled twice. |
| +// |
| +// LOGS AND RETURNS like systemctlTryRestartUnit, and for the same reason: |
| +// this name came from an operator, its failure means the box is serving |
| +// nothing, and the caller's one summary line per host must say so while the |
| +// exit status and systemd's own words stay in the journal under the unit |
| +// name an operator would grep for. Same no-shell shape too: the unit reaches |
| +// exec.Command as ONE argv element. |
| +func systemctlStartUnit(unit string) error { |
| + out, err := exec.Command("systemctl", "start", unit).CombinedOutput() |
| + if err != nil { |
| + log.Printf("adopt: start %s: %v: %s", unit, err, strings.TrimSpace(string(out))) |
| + return fmt.Errorf("start %s: %w: %s", unit, err, strings.TrimSpace(string(out))) |
| + } |
| + return nil |
| +} |
| + |
| // rootRestartRoute is the root adopt pass's restart callback (RunAdopt, out of |
| // carlos-adopt.service): no bounce. That process has no activator and no |
| // children of its own — the exec instances belong to the agent, and another |
| @@ -2441,7 +2518,18 @@ func RunAdopt(args []string) { |
| // config sync stay: both are jobs only this root pass can do, and a |
| // newly staged version wants its unit shape and env file converged with |
| // it, not an hour later. |
| - passOpts := []release.PassOption{release.WithCapabilities(caps.Ops{}), release.BakeDir(dataDir())} |
| + // |
| + // WithStartUnit is wired on THIS pass above all: on a hardened box the |
| + // sandboxed agent's stage is blocked and relayed here, so this is the |
| + // pass that actually try-restarts a console's named unit — and therefore |
| + // the pass whose failed restart, rolled back, must start that unit again |
| + // on the restored version rather than leave a correct symlink over no |
| + // process (release.WithStartUnit; without it the NEXT pass's try-restart |
| + // no-ops on the stopped unit, exits 0, and stamps a build that has never |
| + // run). -stage-only keeps it for the same reason it keeps the converge: |
| + // a relayed stage is exactly the request that performs such a restart. |
| + passOpts := []release.PassOption{release.WithCapabilities(caps.Ops{}), release.BakeDir(dataDir()), |
| + release.WithStartUnit(makeStartRoute(reg, systemctlStartUnit))} |
| if !*stageOnly { |
| passOpts = append(passOpts, release.NagExpiredCanaries(os.Stdout)) |
| } |
diff --git a/internal/host/agent_test.go b/internal/host/agent_test.go| index 07758c854..01e5816c8 100644 |
| --- a/internal/host/agent_test.go |
| +++ b/internal/host/agent_test.go |
| @@ -847,6 +847,128 @@ func TestAdoptPassRefusesToStampAVersionAWrongUnitDidNotStart(t *testing.T) { |
| } |
| } |
| +// TestMakeStartRouteStartsOnlyANamedUnit pins the one-arm shape of the |
| +// rollback-start dispatch: only a route that NAMES its unit has anything for |
| +// it to start. An exec route, a unit-backed route with no name, and a host |
| +// the registry cannot answer for all start NOTHING and return nil — starting |
| +// carlos-app@<host> or an activator child from the rollback path could wake |
| +// a sleeper, which no adopt path may do. |
| +func TestMakeStartRouteStartsOnlyANamedUnit(t *testing.T) { |
| + reg := restartTestRegistry(t) |
| + if err := reg.Upsert(registry.Route{ |
| + Host: "console.test", Kind: "instance", App: "console", |
| + Addr: "127.0.0.1:8090", Status: "ready", Unit: "console.service", |
| + }); err != nil { |
| + t.Fatalf("upsert console.test: %v", err) |
| + } |
| + if err := reg.Upsert(registry.Route{ |
| + Host: "exec.test", Kind: "instance", App: "a", Status: "ready", |
| + }); err != nil { |
| + t.Fatalf("upsert exec.test: %v", err) |
| + } |
| + if err := reg.Upsert(registry.Route{ |
| + Host: "unit.test", Kind: "instance", App: "a", Status: "ready", |
| + Backing: registry.BackingUnit, |
| + }); err != nil { |
| + t.Fatalf("upsert unit.test: %v", err) |
| + } |
| + var units []string |
| + start := makeStartRoute(reg, func(u string) error { units = append(units, u); return nil }) |
| + for _, h := range []string{"console.test", "exec.test", "unit.test", "ghost.test"} { |
| + if err := start(h); err != nil { |
| + t.Fatalf("start %s: %v", h, err) |
| + } |
| + } |
| + if len(units) != 1 || units[0] != "console.service" { |
| + t.Fatalf("started %v, want [console.service] alone", units) |
| + } |
| +} |
| + |
| +// TestAdoptPassStartsTheNamedUnitAgainAfterAFailedRestart is the end-to-end |
| +// for the rollback-start, through the real release.Pass, the real root |
| +// dispatch, and the production systemctl path: a named unit whose |
| +// try-restart fails (the new build would not come back) is rolled back to |
| +// the version it was running — and STARTED on it, because a failed |
| +// try-restart leaves the unit down and try-restart is a no-op on a stopped |
| +// unit. Without the start the box keeps a correct symlink over no process, |
| +// and the NEXT pass's exit-0 no-op stamps the never-run build. |
| +func TestAdoptPassStartsTheNamedUnitAgainAfterAFailedRestart(t *testing.T) { |
| + dir := t.TempDir() |
| + optDir := t.TempDir() |
| + s := release.NewDirStore(dir) |
| + ctx := context.Background() |
| + plat := runtime.GOOS + "-" + runtime.GOARCH |
| + |
| + pub, priv, err := ed25519.GenerateKey(nil) |
| + if err != nil { |
| + t.Fatalf("generate key: %v", err) |
| + } |
| + reg := restartTestRegistry(t) |
| + if err := reg.Upsert(registry.Route{ |
| + Host: "console.test", Kind: "instance", App: "console", Channel: "canary/console", |
| + Addr: "127.0.0.1:8090", Status: "ready", Unit: "console.service", |
| + }); err != nil { |
| + t.Fatalf("upsert console.test: %v", err) |
| + } |
| + for _, v := range []string{"old", "new"} { |
| + m := release.Manifest{ |
| + App: "console", Version: v, Kind: "binary", |
| + Artifacts: map[string]release.Artifact{plat: {}}, |
| + } |
| + if err := release.Ship(ctx, s, "bab", m, map[string][]byte{plat: []byte("build " + v)}); err != nil { |
| + t.Fatalf("Ship %s: %v", v, err) |
| + } |
| + } |
| + if err := release.StageArtifact(optDir, "bab", "old", "console", []byte("build old")); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := release.PointLive(optDir, "console.test", "bab", "old", "console"); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := release.Promote(ctx, s, priv, "bab", "console", "canary/console", "new"); err != nil { |
| + t.Fatalf("Promote new: %v", err) |
| + } |
| + |
| + // A fake systemctl whose try-restart fails the way a service that will |
| + // not come back does, and whose start succeeds — recording the argv of |
| + // both, so this covers systemctlStartUnit's production exec.Command and |
| + // the ordering, not just the constructors. |
| + bin := t.TempDir() |
| + callLog := filepath.Join(t.TempDir(), "systemctl-calls") |
| + if err := os.WriteFile(filepath.Join(bin, "systemctl"), |
| + []byte("#!/bin/sh\necho \"$@\" >> "+callLog+"\ncase \"$1\" in\ntry-restart) echo \"Job for $2 failed.\"; exit 1;;\nesac\nexit 0\n"), 0o755); err != nil { |
| + t.Fatal(err) |
| + } |
| + t.Setenv("PATH", bin+string(os.PathListSeparator)+os.Getenv("PATH")) |
| + |
| + var journal bytes.Buffer |
| + log.SetOutput(&journal) |
| + t.Cleanup(func() { log.SetOutput(os.Stderr) }) |
| + |
| + if err := release.Pass(ctx, reg, s, pub, optDir, rootRestartRoute(reg), |
| + release.WithStartUnit(makeStartRoute(reg, systemctlStartUnit))); err != nil { |
| + t.Fatalf("Pass: %v", err) |
| + } |
| + |
| + got, err := os.ReadFile(callLog) |
| + if err != nil { |
| + t.Fatalf("no systemctl calls at all: %v", err) |
| + } |
| + if want := "try-restart console.service\nstart console.service\n"; string(got) != want { |
| + t.Fatalf("systemctl calls = %q, want %q — the rolled-back unit must be STARTED (try-restart would no-op on it)", got, want) |
| + } |
| + if v, err := release.LiveVersion(optDir, "console.test"); err != nil || v != "old" { |
| + t.Fatalf("live = %q, %v; want old", v, err) |
| + } |
| + rt, err := reg.Get("console.test") |
| + if err != nil || rt == nil { |
| + t.Fatalf("get console.test: %+v, %v", rt, err) |
| + } |
| + if rt.Version == "new" { |
| + t.Fatal("a build whose restart failed was stamped") |
| + } |
| +} |
| + |
| // TestAdoptPassBouncesAWarmHibernatingInstance is the end-to-end this task |
| // exists for, through the real release.Pass with the real closure: an exec |
| // instance minted the way provision does it (Hibernate: true, Backing "") is |
| @@ -969,7 +1091,7 @@ func (f *fastFixture) deps(fullPass func()) *fastDeps { |
| pub: f.pub, |
| optDir: f.optDir, |
| restart: restart, |
| - passOpts: fastPassOpts(f.dataDir, func(host, ver string, err error) {}, restart), |
| + passOpts: fastPassOpts(f.dataDir, func(host, ver string, err error) {}, restart, nil), |
| fullPass: func(context.Context) {}, |
| } |
| if fullPass != nil { |
| @@ -1211,7 +1333,7 @@ func TestFastSweepBouncesAWarmHibernatingCanary(t *testing.T) { |
| func(h string) bool { bounced = append(bounced, h); return true }) |
| d := env.deps(nil) |
| d.restart = restart |
| - d.passOpts = fastPassOpts(env.dataDir, func(host, ver string, err error) {}, restart) |
| + d.passOpts = fastPassOpts(env.dataDir, func(host, ver string, err error) {}, restart, nil) |
| fastTick(ctx, d, time.Unix(1754640000, 0)) |
diff --git a/internal/release/adopt.go b/internal/release/adopt.go| index 46f6925ff..8ce506205 100644 |
| --- a/internal/release/adopt.go |
| +++ b/internal/release/adopt.go |
| @@ -57,6 +57,13 @@ type passConfig struct { |
| // original split: sleepers are repointed with restart=nil and no callback |
| // is consulted for them at all. |
| restartHibernating func(host string) error |
| + |
| + // startUnit is consulted ONLY on adoptHost's restart-failure rollback |
| + // path, after the symlink has gone back (WithStartUnit). nil — every |
| + // call site that does not pass the option, and every existing test — |
| + // keeps today's behaviour byte-identically: the rollback repoints and |
| + // stops. |
| + startUnit func(host string) error |
| } |
| // NagExpiredCanaries opts a Pass/PassFilter call into spec §5's nag: one |
| @@ -190,6 +197,47 @@ func RestartHibernating(restart func(host string) error) PassOption { |
| return func(c *passConfig) { c.restartHibernating = restart } |
| } |
| +// WithStartUnit gives the pass a way to START a route's process after a |
| +// FAILED restart has been rolled back — the one moment adoption knows the |
| +// symlink is correct and the service behind it is down. Without it the |
| +// rollback repoints the symlink and stops there: the unit systemd left |
| +// failed stays failed, and because `systemctl try-restart` is by definition |
| +// a no-op on a unit that is not running (the same property that lets a |
| +// deliberately stopped service stay stopped), the NEXT pass's restart exits |
| +// 0 and SetVersions stamps a build that has never run — the exact lying |
| +// header the named-unit arm's returned error exists to prevent, now over a |
| +// service that is not running at all. |
| +// |
| +// start fires ONLY on that rollback path, and only when there WAS a |
| +// previous version to roll back to: a first adoption whose restart fails |
| +// has nothing restored to start — the symlink still names the refused |
| +// build, and starting it would run that build. A start that also fails |
| +// changes nothing about the refusal (the version stays poisoned and |
| +// unstamped) but is printed, because at that point the box is genuinely |
| +// broken and an operator must not read the rollback line as "recovered". |
| +// |
| +// This cannot WAKE a hibernating route, and the reasoning is the contract: |
| +// the path is reached only after a restart that was ATTEMPTED and returned |
| +// an error. Of the agent's dispatch arms (internal/host's makeRestartRoute) |
| +// only the NAMED-unit arm returns one, and its try-restart exits 0 on a |
| +// loaded-but-stopped unit — so an error means the unit was running and |
| +// would not come back, or the name resolves to nothing (in which case the |
| +// start reaches nothing either). The bounce and carlos-app@ arms swallow |
| +// their failures and return nil, so a sleeping exec child can never route |
| +// here. The converged-daemon arm (caps.Restart) can, but its rollback's |
| +// caps.Restore owns re-binding what it changed, and the callback below |
| +// starts nothing for such a route anyway: |
| +// |
| +// start must start ONLY a route that NAMES its unit (registry Unit column) |
| +// and return nil for everything else — internal/host's makeStartRoute is |
| +// the intended shape. It is `systemctl start`, not try-restart, because the |
| +// entire premise is that the unit is down; on a unit something else already |
| +// brought up (caps.Restore's socket sequence) start is an exit-0 no-op, so |
| +// the two cannot fight. |
| +func WithStartUnit(start func(host string) error) PassOption { |
| + return func(c *passConfig) { c.startUnit = start } |
| +} |
| + |
| // stageBlocked reports whether err is the version store refusing a write to |
| // THIS uid, rather than any of the many other ways staging fails. errors.Is |
| // unwraps the *os.PathError that mkdir/openat return. |
| @@ -812,6 +860,33 @@ func adoptHost(ctx context.Context, reg *registry.Registry, s Store, optDir stri |
| rolled = "; there is no previous version to roll back to, so its capability drop-ins stand as converged" |
| } |
| fmt.Printf(" %s: update to %s failed: %v — rolled the symlink back%s\n", rt.Host, ver, rerr, rolled) |
| + // Start the service again on the version the symlink NOW names. |
| + // The rollback above put the bytes back; without this the box is |
| + // left with a correct symlink and no process — and because |
| + // try-restart is a no-op on a unit that is not running (which is |
| + // what lets a deliberately-stopped service stay stopped), the |
| + // NEXT pass's restart would exit 0, SetVersions would stamp the |
| + // build that has never run, and the header would claim it over a |
| + // service that is down. Start, not try-restart: the whole premise |
| + // here is that the unit is not running. Gated on cur != "" — with |
| + // no previous version nothing was rolled back, the symlink still |
| + // names ver, and starting it would run the build whose restart |
| + // just failed. AFTER caps.Restore above, deliberately: the drop-in |
| + // shape must match the binary before anything execs it, and on a |
| + // unit Restore's socket sequence already brought up, start is an |
| + // exit-0 no-op. |
| + // |
| + // A start that also fails changes nothing about the refusal — the |
| + // version stays poisoned and unstamped — but it is printed, |
| + // because at that point the box is genuinely broken and the |
| + // operator needs to know that rather than reading the rollback |
| + // line as "recovered". |
| + if cfg.startUnit != nil && cur != "" { |
| + if serr := cfg.startUnit(rt.Host); serr != nil { |
| + fmt.Printf(" %s: rolled back to %s but could not start it: %v — this host is serving nothing\n", |
| + rt.Host, cur, serr) |
| + } |
| + } |
| return |
| } |
| } |
diff --git a/internal/release/adopt_test.go b/internal/release/adopt_test.go| index 032852440..998a50032 100644 |
| --- a/internal/release/adopt_test.go |
| +++ b/internal/release/adopt_test.go |
| @@ -2743,3 +2743,181 @@ func TestPassProductionOffStillRatchets(t *testing.T) { |
| t.Fatalf("production memory = %+v, want a latched app with an off-sighting recorded", mem) |
| } |
| } |
| + |
| +// ---- WithStartUnit: the rollback must not leave the service down ---- |
| + |
| +// startUnitFixture is the shared setup of the WithStartUnit tests: one |
| +// named-unit-shaped route live on "old" (staged locally, symlink pointed), |
| +// with "new" promoted at the channel head. canary/* so no bake window |
| +// applies. |
| +func startUnitFixture(t *testing.T, host string) (context.Context, *registry.Registry, Store, ed25519.PublicKey, string) { |
| + t.Helper() |
| + s := NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + priv, pub := testKeys(t) |
| + opt := t.TempDir() |
| + reg := testRegistry(t) |
| + |
| + shipRelease(t, s, "bab", "console", "old", adoptPlatform, []byte("good build")) |
| + shipRelease(t, s, "bab", "console", "new", adoptPlatform, []byte("bad build")) |
| + if err := StageArtifact(opt, "bab", "old", "console", []byte("good build")); err != nil { |
| + t.Fatal(err) |
| + } |
| + addRoute(t, reg, host, "console", "canary/x", false) |
| + if err := PointLive(opt, host, "bab", "old", "console"); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := Promote(ctx, s, priv, "bab", "console", "canary/x", "new"); err != nil { |
| + t.Fatalf("Promote new: %v", err) |
| + } |
| + return ctx, reg, s, pub, opt |
| +} |
| + |
| +// TestFailedRestartLeavesTheUnitRUNNINGOnTheOldVersion. |
| +// |
| +// The rollback repointed the symlink and stopped there, which left the box |
| +// with a correct symlink and no process. Worse, try-restart is a NO-OP on a |
| +// unit that is not running — so the NEXT pass exited 0, stamped SetVersions, |
| +// and produced exactly the lying header the whole mechanism exists to |
| +// prevent, over a service that was not running at all. |
| +func TestFailedRestartLeavesTheUnitRUNNINGOnTheOldVersion(t *testing.T) { |
| + host := "console.example.com" |
| + ctx, reg, s, pub, opt := startUnitFixture(t, host) |
| + |
| + var started []string |
| + restart := func(string) error { return errors.New("Job for console.service failed") } |
| + start := func(h string) error { started = append(started, h); return nil } |
| + |
| + if err := Pass(ctx, reg, s, pub, opt, restart, WithStartUnit(start)); err != nil { |
| + t.Fatalf("Pass: %v", err) |
| + } |
| + |
| + if v, _ := LiveVersion(opt, host); v != "old" { |
| + t.Fatalf("symlink = %q, want it rolled back to old", v) |
| + } |
| + if len(started) != 1 || started[0] != host { |
| + t.Fatalf("started = %v, want the unit started once on the restored version — a rollback that leaves the service down is not a rollback", started) |
| + } |
| + rt, err := reg.Get(host) |
| + if err != nil || rt == nil { |
| + t.Fatalf("get %s: %+v, %v", host, rt, err) |
| + } |
| + if rt.Version == "new" { |
| + t.Fatal("SetVersions stamped the failed build") |
| + } |
| +} |
| + |
| +// TestSecondPassOverAStillBrokenBuildDoesNotStamp: the second-order failure. |
| +// With the unit running again after pass 1, pass 2's try-restart is a real |
| +// restart that fails again — so it rolls back again and still does not stamp. |
| +func TestSecondPassOverAStillBrokenBuildDoesNotStamp(t *testing.T) { |
| + host := "console.example.com" |
| + ctx, reg, s, pub, opt := startUnitFixture(t, host) |
| + |
| + restart := func(string) error { return errors.New("still broken") } |
| + start := func(string) error { return nil } |
| + for i := 0; i < 2; i++ { |
| + if err := Pass(ctx, reg, s, pub, opt, restart, WithStartUnit(start)); err != nil { |
| + t.Fatalf("pass %d: %v", i, err) |
| + } |
| + } |
| + rt, err := reg.Get(host) |
| + if err != nil || rt == nil { |
| + t.Fatalf("get %s: %+v, %v", host, rt, err) |
| + } |
| + if rt.Version == "new" { |
| + t.Fatal("the second pass stamped a build that has never run") |
| + } |
| + if v, _ := LiveVersion(opt, host); v != "old" { |
| + t.Fatalf("symlink after two passes = %q, want old", v) |
| + } |
| +} |
| + |
| +// TestStartFailureStaysPoisonedAndSaysSo: when the start ALSO fails the |
| +// refusal stands exactly as before — poisoned, unstamped, rolled back — and |
| +// the failure is printed, because at that point the box is genuinely broken |
| +// and an operator reading only "rolled the symlink back" would take a host |
| +// that is serving nothing for a recovered one. |
| +func TestStartFailureStaysPoisonedAndSaysSo(t *testing.T) { |
| + host := "console.example.com" |
| + ctx, reg, s, pub, opt := startUnitFixture(t, host) |
| + |
| + restart := func(string) error { return errors.New("Job for console.service failed") } |
| + start := func(string) error { return errors.New("Job for console.service failed again") } |
| + |
| + var out bytes.Buffer |
| + withCapturedStdout(t, &out, func() { |
| + if err := Pass(ctx, reg, s, pub, opt, restart, WithStartUnit(start)); err != nil { |
| + t.Fatalf("Pass: %v", err) |
| + } |
| + }) |
| + |
| + if v, _ := LiveVersion(opt, host); v != "old" { |
| + t.Fatalf("symlink = %q, want old", v) |
| + } |
| + rt, err := reg.Get(host) |
| + if err != nil || rt == nil { |
| + t.Fatalf("get %s: %+v, %v", host, rt, err) |
| + } |
| + if rt.Version == "new" { |
| + t.Fatal("a build whose restart AND start both failed was stamped") |
| + } |
| + if !strings.Contains(out.String(), "could not start it") { |
| + t.Fatalf("printed %q, want the failed start reported", out.String()) |
| + } |
| +} |
| + |
| +// TestSuccessfulRestartNeverConsultsStart: the start capability belongs to |
| +// the ROLLBACK path alone. A restart that succeeds — the every-deploy case — |
| +// must not touch it, or every ordinary adoption would grow a systemctl start. |
| +func TestSuccessfulRestartNeverConsultsStart(t *testing.T) { |
| + host := "console.example.com" |
| + ctx, reg, s, pub, opt := startUnitFixture(t, host) |
| + |
| + restart := func(string) error { return nil } |
| + start := func(h string) error { |
| + t.Fatalf("start consulted for %q after a SUCCESSFUL restart", h) |
| + return nil |
| + } |
| + if err := Pass(ctx, reg, s, pub, opt, restart, WithStartUnit(start)); err != nil { |
| + t.Fatalf("Pass: %v", err) |
| + } |
| + if v, _ := LiveVersion(opt, host); v != "new" { |
| + t.Fatalf("live = %q, want new", v) |
| + } |
| +} |
| + |
| +// TestFirstAdoptionRestartFailureStartsNothing: with no previous version |
| +// (cur == "") the symlink could not be rolled back — it still names the |
| +// version whose restart just failed — so there is no RESTORED version to |
| +// start. Starting there would run the exact build the failure refused. |
| +func TestFirstAdoptionRestartFailureStartsNothing(t *testing.T) { |
| + s := NewDirStore(t.TempDir()) |
| + ctx := context.Background() |
| + priv, pub := testKeys(t) |
| + opt := t.TempDir() |
| + reg := testRegistry(t) |
| + host := "console.example.com" |
| + |
| + shipRelease(t, s, "bab", "console", "new", adoptPlatform, []byte("bad build")) |
| + addRoute(t, reg, host, "console", "canary/x", false) |
| + if err := Promote(ctx, s, priv, "bab", "console", "canary/x", "new"); err != nil { |
| + t.Fatalf("Promote: %v", err) |
| + } |
| + |
| + restart := func(string) error { return errors.New("Job for console.service failed") } |
| + start := func(h string) error { |
| + t.Fatalf("start consulted for %q on a first adoption — nothing was rolled back, so there is nothing restored to start", h) |
| + return nil |
| + } |
| + if err := Pass(ctx, reg, s, pub, opt, restart, WithStartUnit(start)); err != nil { |
| + t.Fatalf("Pass: %v", err) |
| + } |
| + rt, err := reg.Get(host) |
| + if err != nil || rt == nil { |
| + t.Fatalf("get %s: %+v, %v", host, rt, err) |
| + } |
| + if rt.Version == "new" { |
| + t.Fatal("a build whose restart failed was stamped") |
| + } |
| +} |