amadan
system apply restarts carlos-instance-control; readiness refuses a stale control binary (#256 A)
On 2026-10-07 v0.25.1 was installed on the flagship around a control service that had run the 2026-09-16 binary since the first wake. It served the new release's unit templates, which bind /run/carlos-control/exe/<iid>/ sources only the new code writes, so every nested start and the probe failed at namespace setup, and the box reported not ready with the right binary on disk and no reason that named the cause. carlos system apply now restarts the service after the new units are in and daemon-reload has run, before the edge: the new service probes against the old, running edge (a real verdict), and the new agent never meets an old service. A wake dropped by that restart is lost either way, because the edge restart one step later stops every instance (BindsTo); the agent reads the dropped connection as a failed wake and its stop is queued by the socket unit. The rollback and the runtime recovery restart it onto the previous binary the same way; boot recovery leaves it alone (it is not running yet, and a probe before the edge would record a false failure for five minutes). A candidate without the unit (a downgrade below v0.20) has nothing to restart. A restart that fails during install rolls back. The box also checks for itself. The service's /ready compares /proc/self/exe with /opt/carlos/carlos by identity on every call and fails a "binary" line while they differ; and it reports its build in X-Carlos-Build, which the agent compares with its own, refusing nested wakes when the header names another build or is absent (an older service). Both refusals say to restart carlos-instance-control.service.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
8 files changed,
+408
−33
docs/site/cli.md+11 −3internal/host/systemapply.go+71 −2internal/host/systemapply_test.go+117 −3internal/instancectl/client.go+43 −16internal/instancectl/client_test.go+39 −0internal/instancectl/names.go+8 −0internal/instancectl/server.go+60 −9internal/instancectl/server_test.go+59 −0
diff --git a/docs/site/cli.md b/docs/site/cli.md| index 8e1963123..b849f63c0 100644 |
| --- a/docs/site/cli.md |
| +++ b/docs/site/cli.md |
| @@ -1845,9 +1845,9 @@ owns. |
| `update` promotes a public release (or, with `--file`, a binary you built) to |
| `canary` or `stable`; every box on that channel fetches it through its normal |
| -store, verifies it against the signed pointer, installs it, restarts its edge, |
| -and rolls back to the previous binary if the new one does not come up and stay |
| -up. `status` shows each box's platform, channel, running build, the channel's |
| +store, verifies it against the signed pointer, installs it, restarts its |
| +instance-control service and then its edge, and rolls back to the previous |
| +binary if the new one does not come up and stay up. `status` shows each box's platform, channel, running build, the channel's |
| target, and the last apply outcome with its reason. `rollback` points a channel |
| back at the release before this one, and every box on it rolls back the same |
| way it rolled forward. All three talk to your console. You need to be an owner |
| @@ -2030,6 +2030,14 @@ its slice before a start, and runs the isolation probe at start and every |
| five minutes. It accepts a connection only from the edge unit's own main |
| process. Not for operators: there is nothing to type. |
| +It stays up once the first wake has started it, so it has to be restarted |
| +whenever the platform binary changes; `carlos system update` does that, and |
| +a hand roll must too. A box checks both ends: the service refuses to be |
| +ready while the binary it runs is no longer `/opt/carlos/carlos`, and the |
| +agent refuses to wake isolated instances while the service reports another |
| +build than its own, or none. Either way the reason in `carlos logs` ends |
| +with `restart carlos-instance-control.service`. |
| + |
| ### carlos instance-exec |
| The first thing an isolated instance unit runs. It reads the launch file |
diff --git a/internal/host/systemapply.go b/internal/host/systemapply.go| index 0fea54ddd..09330955d 100644 |
| --- a/internal/host/systemapply.go |
| +++ b/internal/host/systemapply.go |
| @@ -44,8 +44,13 @@ import ( |
| // finds is refuse the install. |
| const ( |
| - systemEdgeUnit = "carlos-edge" |
| - systemRelayUnit = "carlos-relay" |
| + systemEdgeUnit = "carlos-edge" |
| + systemRelayUnit = "carlos-relay" |
| + // systemControlUnit is the root instance-control service (isolated-exec |
| + // design §5). It execs /opt/carlos/carlos and stays up for weeks once |
| + // the first wake activated it, so every install that changes the |
| + // binary restarts it (restartInstanceControl; discussion #256). |
| + systemControlUnit = "carlos-instance-control.service" |
| systemLockFile = "lock" |
| systemMaxRequest = 4 << 10 |
| systemGateWait = 20 * time.Second |
| @@ -528,6 +533,11 @@ func (a *systemApplier) install(ctx context.Context, req systemRequest, p *relea |
| if err := a.journal(j); err != nil { |
| return fail("journal", err) |
| } |
| + // The control service before the edge; see restartInstanceControl for |
| + // why that order, and why a restart that may drop a wake is safe here. |
| + if err := a.restartInstanceControl(); err != nil { |
| + return fail("instance-control restart", err) |
| + } |
| if err := a.restartEdge(); err != nil { |
| return fail("restart", err) |
| } |
| @@ -1280,6 +1290,48 @@ func (a *systemApplier) restartEdge() error { |
| return err |
| } |
| +// restartInstanceControl restarts carlos-instance-control.service, the |
| +// root service that starts every nested instance (design §5). It is |
| +// socket-activated and then stays up: on the flagship it had run the |
| +// 2026-09-16 binary for three weeks when v0.25.1 was installed around it |
| +// (discussion #256). A control service of the previous release serves the |
| +// new release's unit templates, which bind sources only the new code |
| +// writes (/run/carlos-control/exe/<iid>/), so every nested start failed |
| +// at namespace setup and the probe unit, run by the same stale service, |
| +// failed the same way: the box reported not ready with the right binary |
| +// on disk and nothing to say why. |
| +// |
| +// Order: after the new units are installed and daemon-reload has run, and |
| +// BEFORE the edge restarts. The new service then runs its start-up probe |
| +// against the old edge, which is up, so the verdict it records is real |
| +// (a probe with no edge cgroup fails on cgroup-ro); and by the time the |
| +// new agent starts, the service it calls is already the new build, so |
| +// there is no window in which a new agent asks an old service to start |
| +// units it cannot. A wake in flight at this moment is lost, and that is |
| +// safe: the edge restart one step later stops every instance anyway |
| +// (BindsTo=carlos-edge.service), so the wake fails either way; the |
| +// systemctl start the dropped request was running is a job systemd |
| +// finishes on its own; the agent reads the dropped connection as a failed |
| +// wake and sends the stop, which the socket unit, up throughout, queues |
| +// for the restarted service. Flat hosts never pass through this service. |
| +// |
| +// A build that ships no control service (a downgrade below v0.20, whose |
| +// unit installUnits has just disabled and removed) has nothing to |
| +// restart; that is the one case this returns nil without calling |
| +// systemctl. reset-failed first, as restartEdge does, so a unit that hit |
| +// its start limit can be restarted at all. |
| +func (a *systemApplier) restartInstanceControl() error { |
| + if _, err := os.Stat(filepath.Join(a.etcDir, "systemd", "system", systemControlUnit)); err != nil { |
| + if errors.Is(err, os.ErrNotExist) { |
| + return nil |
| + } |
| + return err |
| + } |
| + a.ops.systemctl("reset-failed", systemControlUnit) |
| + _, err := a.ops.systemctl("restart", systemControlUnit) |
| + return err |
| +} |
| + |
| // restartRelay mirrors restartEdge for the relay (relay spec §3), with one |
| // difference that decides its shape: the relay is optional. A box with no |
| // /etc/carlos/relay-masters must not be handed a relay it never asked for. |
| @@ -1464,6 +1516,14 @@ func (a *systemApplier) rollback(ctx context.Context, req systemRequest, reason |
| // carlos.prev, now on disk: what this rollback's journal finish makes |
| // current, checked again under activationGuard at that finish. |
| restored, _ := readLine(prevVersionPath(a.optDir)) |
| + // The previous binary is on disk, so the control service goes back |
| + // too, before the edge, as on the way forward. Logged, not fatal: a |
| + // rollback's job is a good edge, and a control service that would |
| + // not restart is caught by the agent's readiness check (it refuses |
| + // nested wakes until the service runs the installed binary). |
| + if err := a.restartInstanceControl(); err != nil { |
| + a.ops.logf("system apply: rollback: instance-control restart: %v (nested wakes refuse until it is restarted)", err) |
| + } |
| if err := a.restartEdge(); err != nil { |
| return failed("restart", err) // journal kept: the edge is still to come back |
| } |
| @@ -1712,6 +1772,15 @@ func (a *systemApplier) recoverIfNeeded(ctx context.Context) (bool, error) { |
| // carlos.prev, now on disk: what the finish below makes current. |
| restored, _ := readLine(prevVersionPath(a.optDir)) |
| if a.ops.edgeStartedThisBoot() { |
| + // The control service first, as in install and rollback. Not at |
| + // boot: it is socket-activated and not yet running, and started |
| + // before the edge it would run its probe against an edge whose |
| + // cgroup does not exist yet and record a failing verdict for the |
| + // next five minutes. Blocking is safe: nothing orders this unit |
| + // against carlos-system-recover.service. |
| + if err := a.restartInstanceControl(); err != nil { |
| + a.ops.logf("system apply: recovery: instance-control restart: %v (nested wakes refuse until it is restarted)", err) |
| + } |
| a.ops.systemctl("reset-failed", systemEdgeUnit) |
| // --no-block: this can be running as the edge's own OnFailure= |
| // handler, and a blocking restart of the unit that triggered it |
diff --git a/internal/host/systemapply_test.go b/internal/host/systemapply_test.go| index a37a18ad6..ddca8e505 100644 |
| --- a/internal/host/systemapply_test.go |
| +++ b/internal/host/systemapply_test.go |
| @@ -131,7 +131,7 @@ func newApplyFixture(t *testing.T, channel, build string) *applyFixture { |
| os.WriteFile(systemBinaryPath(f.a.optDir), []byte("#!/bin/sh\necho carlos "+build+"\n"), 0o755) |
| f.answer["version:"+systemBinaryPath(f.a.optDir)] = "carlos " + build + "\n" |
| f.answer["plan:"+systemBinaryPath(f.a.optDir)] = `{"enabled":["carlos-edge.service","carlos-adopt.timer","carlos-adopt.path","carlos-stage.path","carlos-restart.path","carlos-system-update.path","carlos-system-recover.service"],"inert":["carlos-restore-verify.timer"]}` |
| - f.answer["units:"+systemBinaryPath(f.a.optDir)] = "carlos-edge.service carlos-adopt.timer carlos-adopt.path carlos-stage.path carlos-restart.path carlos-system-update.path carlos-system-update.service carlos-system-recover.service carlos-old-only.timer" |
| + f.answer["units:"+systemBinaryPath(f.a.optDir)] = "carlos-edge.service carlos-adopt.timer carlos-adopt.path carlos-stage.path carlos-restart.path carlos-system-update.path carlos-system-update.service carlos-system-recover.service carlos-instance-control.service carlos-old-only.timer" |
| return f |
| } |
| @@ -416,7 +416,7 @@ func applyHappyVersion(t *testing.T, version string) (*applyFixture, systemReque |
| staged := filepath.Join(f.a.stagingDir(), "carlos") |
| f.answer["version:"+staged] = "carlos " + version + "\n" |
| f.answer["plan:"+staged] = `{"enabled":["carlos-edge.service","carlos-adopt.timer","carlos-adopt.path","carlos-stage.path","carlos-restart.path","carlos-system-update.path","carlos-system-recover.service","carlos-new-only.path"],"inert":["carlos-restore-verify.timer"]}` |
| - f.answer["units:"+staged] = "carlos-edge.service carlos-adopt.timer carlos-adopt.path carlos-stage.path carlos-restart.path carlos-system-update.path carlos-system-update.service carlos-system-recover.service carlos-new-only.path" |
| + f.answer["units:"+staged] = "carlos-edge.service carlos-adopt.timer carlos-adopt.path carlos-stage.path carlos-restart.path carlos-system-update.path carlos-system-update.service carlos-system-recover.service carlos-instance-control.service carlos-new-only.path" |
| // The unit v1 owns and v2 drops is on the box, so removing it is a |
| // thing that can be seen to happen — or not to. |
| os.WriteFile(filepath.Join(f.a.etcDir, "systemd", "system", "carlos-old-only.timer"), []byte("[Unit]\n# v1\n"), 0o644) |
| @@ -740,6 +740,113 @@ func TestApplyRestartsTheRelayAfterTheEdge(t *testing.T) { |
| } |
| } |
| +// Discussion #256: the root instance-control service execs the platform |
| +// binary and stays up for weeks, so an install that leaves it running the |
| +// previous build leaves every nested wake failing at namespace setup. It |
| +// is restarted after the new units are in place (daemon-reload) and before |
| +// the edge, so the new agent never meets an old control service. |
| +func TestApplyRestartsTheInstanceControlServiceBeforeTheEdge(t *testing.T) { |
| + f, _ := applyHappy(t) |
| + if err := f.a.Run(context.Background(), false); err != nil { |
| + t.Fatal(err) |
| + } |
| + joined := strings.Join(f.calls, "\n") |
| + reload := strings.Index(joined, "systemctl daemon-reload") |
| + reset := strings.Index(joined, "systemctl reset-failed carlos-instance-control.service") |
| + restart := strings.Index(joined, "systemctl restart carlos-instance-control.service") |
| + edge := strings.Index(joined, "systemctl restart carlos-edge") |
| + switch { |
| + case reset < 0 || restart < 0: |
| + t.Fatalf("instance-control never restarted:\n%s", joined) |
| + case !(reload < reset && reset < restart && restart < edge): |
| + t.Fatalf("want daemon-reload, then reset-failed and restart carlos-instance-control.service, then restart carlos-edge:\n%s", joined) |
| + } |
| + if strings.Contains(joined, "systemctl try-restart carlos-instance-control.service") { |
| + t.Errorf("try-restart cannot start an inactive (socket-activated, never connected) service:\n%s", joined) |
| + } |
| +} |
| + |
| +// A control service that will not restart fails the install, and the |
| +// rollback restarts it again onto the previous binary (so the restart is |
| +// attempted twice over the run), rather than leaving a new edge beside a |
| +// control service of unknown vintage. |
| +func TestApplyInstanceControlRestartFailureRollsBack(t *testing.T) { |
| + f, req := applyHappy(t) |
| + orig := f.a.ops.systemctl |
| + failed := false |
| + f.a.ops.systemctl = func(args ...string) (string, error) { |
| + out, err := orig(args...) |
| + if args[0] == "restart" && args[1] == systemControlUnit && !failed { |
| + failed = true |
| + return "", errors.New("Job for carlos-instance-control.service failed") |
| + } |
| + return out, err |
| + } |
| + if err := f.a.Run(context.Background(), false); err == nil { |
| + t.Fatal("a control service that would not restart passed the install") |
| + } |
| + r := f.result() |
| + if r == nil || r.Outcome != systemOutcomeRolledBack || !r.matches("stable", "v2", req.PromotedAt) || !strings.Contains(r.Reason, "instance-control restart") { |
| + t.Fatalf("result = %+v", r) |
| + } |
| + if n := strings.Count(strings.Join(f.calls, "\n"), "systemctl restart carlos-instance-control.service"); n != 2 { |
| + t.Errorf("control service restarted %d times, want 2 (install then rollback):\n%s", n, strings.Join(f.calls, "\n")) |
| + } |
| + if bin, _ := os.ReadFile(systemBinaryPath(f.a.optDir)); !strings.Contains(string(bin), "carlos v1") { |
| + t.Fatal("binary not rolled back") |
| + } |
| +} |
| + |
| +// The rollback of a failed gate restarts the control service onto the |
| +// previous binary too, before the edge, for the same reason the relay is. |
| +func TestApplyRollbackRestartsTheInstanceControlService(t *testing.T) { |
| + f, _ := applyHappy(t) |
| + f.edgeUp = false |
| + orig := f.a.ops.systemctl |
| + f.a.ops.systemctl = func(args ...string) (string, error) { |
| + if args[0] == "restart" && args[1] == systemEdgeUnit { |
| + if b, _ := os.ReadFile(systemBinaryPath(f.a.optDir)); strings.Contains(string(b), "carlos v1") { |
| + f.edgeUp = true |
| + } |
| + } |
| + return orig(args...) |
| + } |
| + if err := f.a.Run(context.Background(), false); err == nil { |
| + t.Fatal("a failed gate must exit non-zero") |
| + } |
| + joined := strings.Join(f.calls, "\n") |
| + edge := strings.LastIndex(joined, "systemctl restart carlos-edge") |
| + ctl := strings.LastIndex(joined, "systemctl restart carlos-instance-control.service") |
| + if ctl < 0 || ctl > edge { |
| + t.Fatalf("the rollback left the control service on the withdrawn build, or restarted it after the edge:\n%s", joined) |
| + } |
| + if n := strings.Count(joined, "systemctl restart carlos-instance-control.service"); n != 2 { |
| + t.Errorf("control service restarted %d times, want 2 (apply then rollback):\n%s", n, joined) |
| + } |
| +} |
| + |
| +// A candidate that ships no control service (a downgrade below the |
| +// isolated-exec releases) has nothing to restart: installUnits has just |
| +// disabled and removed the unit, and a restart would fail the install. |
| +func TestApplyDoesNotRestartAControlServiceTheCandidateLacks(t *testing.T) { |
| + f, req := applyHappy(t) |
| + staged := filepath.Join(f.a.stagingDir(), "carlos") |
| + f.answer["units:"+staged] = strings.ReplaceAll(f.answer["units:"+staged], " carlos-instance-control.service", "") |
| + if err := f.a.Run(context.Background(), false); err != nil { |
| + t.Fatal(err) |
| + } |
| + if r := f.result(); r == nil || r.Outcome != systemOutcomeApplied || !r.matches("stable", "v2", req.PromotedAt) { |
| + t.Fatalf("result = %+v", r) |
| + } |
| + joined := strings.Join(f.calls, "\n") |
| + if !strings.Contains(joined, "systemctl disable --now carlos-instance-control.service") { |
| + t.Fatalf("the unit the candidate lacks was not disabled:\n%s", joined) |
| + } |
| + if strings.Contains(joined, "systemctl restart carlos-instance-control.service") { |
| + t.Fatalf("restarted a unit the candidate removed:\n%s", joined) |
| + } |
| +} |
| + |
| // A relay that cannot be restarted is not a failed update. The commonest |
| // case is the ordinary one — a box with no masters file, whose unit systemd |
| // skips on its condition and may well answer about — and the edge, the |
| @@ -860,7 +967,7 @@ func TestRecoveryRollsBackFilesOnlyAtBootAndRestartsAtRuntime(t *testing.T) { |
| t.Error("removed unit not restored") |
| } |
| joined := strings.Join(f.calls, "\n") |
| - if strings.Contains(joined, "systemctl restart carlos-edge") || strings.Contains(joined, "systemctl start ") { |
| + if strings.Contains(joined, "systemctl restart carlos-edge") || strings.Contains(joined, "systemctl start ") || strings.Contains(joined, "systemctl restart carlos-instance-control") { |
| t.Fatalf("boot recovery restarted or started something:\n%s", joined) |
| } |
| if !strings.Contains(joined, "systemctl enable carlos-edge.service") { |
| @@ -890,6 +997,13 @@ func TestRecoveryRollsBackFilesOnlyAtBootAndRestartsAtRuntime(t *testing.T) { |
| t.Errorf("runtime recovery lacks %q:\n%s", want, joined) |
| } |
| } |
| + // The control service goes back to the previous binary with the edge |
| + // (#256), and before it; at boot (above) neither is touched. |
| + ctl := strings.Index(joined, "systemctl restart carlos-instance-control.service") |
| + edge := strings.Index(joined, "systemctl restart --no-block carlos-edge") |
| + if ctl < 0 || ctl > edge { |
| + t.Errorf("runtime recovery does not restart the control service before the edge:\n%s", joined) |
| + } |
| } |
| func TestRecoveryFinishesForwardFromGated(t *testing.T) { |
diff --git a/internal/instancectl/client.go b/internal/instancectl/client.go| index 33dfa32cc..cae0ca4cb 100644 |
| --- a/internal/instancectl/client.go |
| +++ b/internal/instancectl/client.go |
| @@ -12,6 +12,8 @@ import ( |
| "strconv" |
| "strings" |
| "time" |
| + |
| + "github.com/carlosframework/platform/internal/version" |
| ) |
| // Deadlines per verb (design §5.3). A start or stop that exceeds its |
| @@ -34,9 +36,25 @@ type Client struct { |
| // Dial is the dialer; nil means net.Dialer. Tests do not need to override |
| // it, but a caller composing several clients may. |
| Dial func(ctx context.Context, path string) (net.Conn, error) |
| + // Build is the build this agent runs. Ready refuses a service whose |
| + // BuildHeader does not name it (#256); empty skips the comparison, |
| + // for tests that build a Client literal. NewClient sets version.Build. |
| + Build string |
| } |
| -func NewClient(path string) *Client { return &Client{Path: path} } |
| +func NewClient(path string) *Client { return &Client{Path: path, Build: version.Build} } |
| + |
| +// StaleBuildError is Ready's answer to a control service running a build |
| +// other than the agent's own: the service outlived a binary change and |
| +// serves this release's units with the previous release's code (#256). |
| +type StaleBuildError struct{ Running, Want string } |
| + |
| +func (e *StaleBuildError) Error() string { |
| + if e.Running == "" { |
| + return fmt.Sprintf("instance control: carlos-instance-control.service is running an older build than this agent (%s); restart carlos-instance-control.service", e.Want) |
| + } |
| + return fmt.Sprintf("instance control: carlos-instance-control.service is running build %s, this agent is %s; restart carlos-instance-control.service", e.Running, e.Want) |
| +} |
| // NotReadyError carries the probe verdict a 503 /ready returned. |
| type NotReadyError struct{ Report ProbeReport } |
| @@ -76,7 +94,7 @@ func (c *Client) httpClient() *http.Client { |
| // there is one. A non-2xx answer becomes an error carrying the server's |
| // message; a dial failure names the socket so a forgotten enable reads as |
| // what it is from carlos logs. |
| -func (c *Client) do(ctx context.Context, deadline time.Duration, method, path string, q url.Values, out any) (int, error) { |
| +func (c *Client) do(ctx context.Context, deadline time.Duration, method, path string, q url.Values, out any) (int, http.Header, error) { |
| ctx, cancel := context.WithTimeout(ctx, deadline) |
| defer cancel() |
| u := "http://control" + path |
| @@ -85,18 +103,18 @@ func (c *Client) do(ctx context.Context, deadline time.Duration, method, path st |
| } |
| req, err := http.NewRequestWithContext(ctx, method, u, nil) |
| if err != nil { |
| - return 0, err |
| + return 0, nil, err |
| } |
| resp, err := c.httpClient().Do(req) |
| if err != nil { |
| - return 0, fmt.Errorf("instance control socket %s: %w", c.Path, err) |
| + return 0, nil, fmt.Errorf("instance control socket %s: %w", c.Path, err) |
| } |
| defer resp.Body.Close() |
| body, _ := io.ReadAll(io.LimitReader(resp.Body, 64<<10)) |
| if resp.StatusCode == http.StatusServiceUnavailable && path == "/ready" { |
| var r ProbeReport |
| _ = json.Unmarshal(body, &r) |
| - return resp.StatusCode, &NotReadyError{Report: r} |
| + return resp.StatusCode, resp.Header, &NotReadyError{Report: r} |
| } |
| // A 502 from /sweep still carries the units it did and didn't manage to |
| // stop, so it is decoded into out here even though it is about to be |
| @@ -104,13 +122,13 @@ func (c *Client) do(ctx context.Context, deadline time.Duration, method, path st |
| // the failure, not instead of it. |
| if out != nil && len(body) > 0 && (resp.StatusCode/100 == 2 || (resp.StatusCode == http.StatusBadGateway && path == "/sweep")) { |
| if err := json.Unmarshal(body, out); err != nil { |
| - return resp.StatusCode, fmt.Errorf("instance control %s: %v", path, err) |
| + return resp.StatusCode, resp.Header, fmt.Errorf("instance control %s: %v", path, err) |
| } |
| } |
| if resp.StatusCode/100 != 2 { |
| - return resp.StatusCode, fmt.Errorf("instance control %s: %d: %s", path, resp.StatusCode, strings.TrimSpace(string(body))) |
| + return resp.StatusCode, resp.Header, fmt.Errorf("instance control %s: %d: %s", path, resp.StatusCode, strings.TrimSpace(string(body))) |
| } |
| - return resp.StatusCode, nil |
| + return resp.StatusCode, resp.Header, nil |
| } |
| func hostRole(host string, role Role) url.Values { |
| @@ -127,7 +145,7 @@ func (c *Client) Start(ctx context.Context, host string, role Role, memoryMax in |
| q := hostRole(host, role) |
| q.Set("memory_max", strconv.FormatInt(memoryMax, 10)) |
| var out StartResponse |
| - if _, err := c.do(ctx, DeadlineControl, http.MethodPost, "/start", q, &out); err != nil { |
| + if _, _, err := c.do(ctx, DeadlineControl, http.MethodPost, "/start", q, &out); err != nil { |
| return "", err |
| } |
| return out.InvocationID, nil |
| @@ -138,17 +156,17 @@ func (c *Client) Seed(ctx context.Context, host string, memoryMax int64) (SeedRe |
| q := hostRole(host, "") |
| q.Set("memory_max", strconv.FormatInt(memoryMax, 10)) |
| var out SeedResponse |
| - _, err := c.do(ctx, DeadlineSeed, http.MethodPost, "/seed", q, &out) |
| + _, _, err := c.do(ctx, DeadlineSeed, http.MethodPost, "/seed", q, &out) |
| return out, err |
| } |
| func (c *Client) Stop(ctx context.Context, host string, role Role) error { |
| - _, err := c.do(ctx, DeadlineControl, http.MethodPost, "/stop", hostRole(host, role), nil) |
| + _, _, err := c.do(ctx, DeadlineControl, http.MethodPost, "/stop", hostRole(host, role), nil) |
| return err |
| } |
| func (c *Client) Reset(ctx context.Context, host string, role Role) error { |
| - _, err := c.do(ctx, DeadlineControl, http.MethodPost, "/reset", hostRole(host, role), nil) |
| + _, _, err := c.do(ctx, DeadlineControl, http.MethodPost, "/reset", hostRole(host, role), nil) |
| return err |
| } |
| @@ -157,15 +175,24 @@ func (c *Client) Reset(ctx context.Context, host string, role Role) error { |
| // returned with the error. |
| func (c *Client) Sweep(ctx context.Context) (SweepResponse, error) { |
| var out SweepResponse |
| - status, err := c.do(ctx, DeadlineSweep, http.MethodPost, "/sweep", nil, &out) |
| + status, _, err := c.do(ctx, DeadlineSweep, http.MethodPost, "/sweep", nil, &out) |
| if err != nil && status == http.StatusBadGateway { |
| return out, errors.Join(err, errors.New("some instance units would not stop")) |
| } |
| return out, err |
| } |
| -// Ready is nil on 204 and a *NotReadyError on 503. |
| +// Ready is nil on 204 from a service of this agent's own build, a |
| +// *NotReadyError on 503, and a *StaleBuildError on a 204 whose BuildHeader |
| +// is missing or names another build (#256). The verdict is asked first: |
| +// a service that is both stale and not ready says why it is not ready. |
| func (c *Client) Ready(ctx context.Context) error { |
| - _, err := c.do(ctx, DeadlineReady, http.MethodGet, "/ready", nil, nil) |
| - return err |
| + _, hdr, err := c.do(ctx, DeadlineReady, http.MethodGet, "/ready", nil, nil) |
| + if err != nil { |
| + return err |
| + } |
| + if got := hdr.Get(BuildHeader); c.Build != "" && got != c.Build { |
| + return &StaleBuildError{Running: got, Want: c.Build} |
| + } |
| + return nil |
| } |
diff --git a/internal/instancectl/client_test.go b/internal/instancectl/client_test.go| index dc84c5837..a02f4e140 100644 |
| --- a/internal/instancectl/client_test.go |
| +++ b/internal/instancectl/client_test.go |
| @@ -119,3 +119,42 @@ func TestClientRefusesWhenTheSocketIsAbsent(t *testing.T) { |
| t.Fatalf("absent socket: %v", err) |
| } |
| } |
| + |
| +// The agent side of #256: a 204 from a service that reports another build, |
| +// or none at all (a release before the header), is not ready. Both are |
| +// the service carlos system apply or the roll script left running across |
| +// a binary change, and the error names the restart. |
| +func TestClientReadyRefusesAControlServiceOfAnotherBuild(t *testing.T) { |
| + ctl := &fakeCtl{} |
| + s, c := newClient(t, ctl, func(s *Server) { s.Build = "" }) |
| + b, _ := json.Marshal(ProbeReport{Ready: true, Lines: []VerdictLine{{Check: "pid1", Verdict: "ok"}}}) |
| + if err := os.WriteFile(s.ProbeOut, b, 0o644); err != nil { |
| + t.Fatal(err) |
| + } |
| + s.RunProbe(context.Background()) |
| + c.Build = "new1234" |
| + err := c.Ready(context.Background()) |
| + var stale *StaleBuildError |
| + if !errors.As(err, &stale) || stale.Running != "" || !strings.Contains(err.Error(), "older build") || !strings.Contains(err.Error(), "restart carlos-instance-control.service") { |
| + t.Fatalf("no header: Ready = %v", err) |
| + } |
| + s.Build = "old5678" |
| + err = c.Ready(context.Background()) |
| + if !errors.As(err, &stale) || stale.Running != "old5678" || !strings.Contains(err.Error(), "running build old5678, this agent is new1234") { |
| + t.Fatalf("other build: Ready = %v", err) |
| + } |
| + s.Build = "new1234" |
| + if err := c.Ready(context.Background()); err != nil { |
| + t.Fatalf("same build: Ready = %v", err) |
| + } |
| + // A client with no build of its own (a test's literal) does not judge. |
| + c.Build = "" |
| + s.Build = "" |
| + if err := c.Ready(context.Background()); err != nil { |
| + t.Fatalf("unjudged: Ready = %v", err) |
| + } |
| + // NewClient judges with this process's own build. |
| + if NewClient(c.Path).Build == "" { |
| + t.Fatal("NewClient left Build empty; production would never check") |
| + } |
| +} |
diff --git a/internal/instancectl/names.go b/internal/instancectl/names.go| index 7de0f994c..f07a94624 100644 |
| --- a/internal/instancectl/names.go |
| +++ b/internal/instancectl/names.go |
| @@ -168,6 +168,14 @@ type ProbeReport struct { |
| Lines []VerdictLine `json:"lines"` |
| } |
| +// BuildHeader carries the control service's version.Build on every /ready |
| +// answer (#256). The agent compares it with its own: both are meant to be |
| +// /opt/carlos/carlos, and a service that reports another build, or none |
| +// (a release before this header), is one carlos system apply or the roll |
| +// script left running across a binary change. Nested wakes refuse until |
| +// it is restarted, with that as the reason. |
| +const BuildHeader = "X-Carlos-Build" |
| + |
| // Responses of the control verbs (design §5.3). |
| type StartResponse struct { |
| InvocationID string `json:"invocation_id"` |
diff --git a/internal/instancectl/server.go b/internal/instancectl/server.go| index 58ef6c0d8..64b96d55c 100644 |
| --- a/internal/instancectl/server.go |
| +++ b/internal/instancectl/server.go |
| @@ -23,6 +23,7 @@ import ( |
| "github.com/carlosframework/platform/internal/cliflag" |
| "github.com/carlosframework/platform/internal/controlsock" |
| "github.com/carlosframework/platform/internal/provision" |
| + "github.com/carlosframework/platform/internal/version" |
| ) |
| // Server is the root instance-control service (design §5). It does four |
| @@ -78,6 +79,19 @@ type Server struct { |
| // constant: nothing from a request reaches it. |
| Run func(ctx context.Context, args ...string) (string, error) |
| + // Build is the build this process runs (version.Build), answered on |
| + // every /ready as the BuildHeader so the agent can see it talks to |
| + // its own release (#256). Empty sends no header, which a client of |
| + // this release reads as an older service. |
| + Build string |
| + // ExeCurrent reports whether this process still runs the installed |
| + // platform binary; nil skips the check. Production compares |
| + // /proc/self/exe with /opt/carlos/carlos by identity |
| + // (installedBinaryCurrent). Asked on every /ready, not once at probe |
| + // time, because the binary can change at any moment after the probe |
| + // ran: that is exactly the stale-service case of #256. |
| + ExeCurrent func() error |
| + |
| // cgroupWaitBudget bounds a wait for an empty cgroup; tests shorten it. |
| cgroupWaitBudget time.Duration |
| @@ -130,6 +144,8 @@ func runWithClaim(args []string, claim http.Handler) { |
| DataRoot: "/data/instances", |
| Statfs: statfsBytes, |
| DiskEvery: 30 * time.Second, |
| + Build: version.Build, |
| + ExeCurrent: installedBinaryCurrent, |
| } |
| ln, activated, err := activatedListener() |
| if err != nil { |
| @@ -578,6 +594,9 @@ func (s *Server) waitInactiveOrFailed(ctx context.Context, unit string, every ti |
| } |
| func (s *Server) handleReady(w http.ResponseWriter, r *http.Request) { |
| + if s.Build != "" { |
| + w.Header().Set(BuildHeader, s.Build) |
| + } |
| rep, ready := s.Ready() |
| if ready { |
| w.WriteHeader(http.StatusNoContent) |
| @@ -586,18 +605,50 @@ func (s *Server) handleReady(w http.ResponseWriter, r *http.Request) { |
| writeJSON(w, http.StatusServiceUnavailable, rep) |
| } |
| -// Ready returns the last probe verdict. Before the first probe has ever |
| -// run, there is no verdict to report, so a synthetic line names that |
| -// reason explicitly rather than answering with an empty Lines (final |
| -// review, plan 2, item 6): a caller reading /ready during startup should |
| -// see why, not a report that looks like every check silently passed. |
| +// Ready returns the last probe verdict, plus the one check that cannot |
| +// wait for the next probe: that this process still runs the installed |
| +// binary (ExeCurrent). Before the first probe has ever run, there is no |
| +// verdict to report, so a synthetic line names that reason explicitly |
| +// rather than answering with an empty Lines (final review, plan 2, item |
| +// 6): a caller reading /ready during startup should see why, not a report |
| +// that looks like every check silently passed. |
| func (s *Server) Ready() (ProbeReport, bool) { |
| s.mu.Lock() |
| - defer s.mu.Unlock() |
| - if !s.probed { |
| - return ProbeReport{Lines: []VerdictLine{{Check: "probe", Verdict: "fail", Detail: "not yet run"}}}, false |
| + rep, probed := s.report, s.probed |
| + s.mu.Unlock() |
| + if !probed { |
| + rep = ProbeReport{Lines: []VerdictLine{{Check: "probe", Verdict: "fail", Detail: "not yet run"}}} |
| + } |
| + if s.ExeCurrent != nil { |
| + if err := s.ExeCurrent(); err != nil { |
| + // A copy: the stored report must not grow a line per call. |
| + rep = ProbeReport{Ready: false, Lines: append(append([]VerdictLine(nil), rep.Lines...), |
| + VerdictLine{Check: "binary", Verdict: "fail", Detail: err.Error()})} |
| + } |
| + } |
| + return rep, rep.Ready |
| +} |
| + |
| +// installedBinaryCurrent is ExeCurrent in production: /proc/self/exe |
| +// resolves to the file this process was started from, even after that |
| +// file was renamed over (the kernel reports it as deleted), and |
| +// os.SameFile compares device and inode, so a service started from the |
| +// previous release's binary answers a mismatch the moment carlos system |
| +// apply renames the new one into place, and keeps answering it until it |
| +// is restarted (#256). |
| +func installedBinaryCurrent() error { |
| + running, err := os.Stat("/proc/self/exe") |
| + if err != nil { |
| + return fmt.Errorf("stat /proc/self/exe: %v", err) |
| + } |
| + installed, err := os.Stat(SystemBinary) |
| + if err != nil { |
| + return fmt.Errorf("stat %s: %v", SystemBinary, err) |
| + } |
| + if !os.SameFile(running, installed) { |
| + return fmt.Errorf("instance control is running a binary that is no longer %s; restart carlos-instance-control.service", SystemBinary) |
| } |
| - return s.report, s.report.Ready |
| + return nil |
| } |
| func (s *Server) waitBudget() time.Duration { |
diff --git a/internal/instancectl/server_test.go b/internal/instancectl/server_test.go| index aaf822f57..5ef76bc3c 100644 |
| --- a/internal/instancectl/server_test.go |
| +++ b/internal/instancectl/server_test.go |
| @@ -13,6 +13,8 @@ import ( |
| "sync" |
| "testing" |
| "time" |
| + |
| + "github.com/carlosframework/platform/internal/version" |
| ) |
| type fakeCtl struct { |
| @@ -96,6 +98,9 @@ func serveOnSocket(t *testing.T, ctl *fakeCtl, configure ...func(*Server)) (*Ser |
| FixtureOK: func() error { return nil }, |
| ProbeEvery: time.Hour, |
| Logf: t.Logf, |
| + // The same binary as the Client this test process is, as in |
| + // production, where both are /opt/carlos/carlos. |
| + Build: version.Build, |
| } |
| for _, configure := range configure { |
| configure(s) |
| @@ -630,3 +635,57 @@ func TestMountpointInspectionUnverifiedDoesNotBlock(t *testing.T) { |
| t.Error("a failing probe read as ready") |
| } |
| } |
| + |
| +// Discussion #256: carlos-instance-control.service ran the previous |
| +// release's binary for three weeks and served the new release's units. |
| +// /ready now asks, on every call, whether this process still runs the |
| +// installed binary, and a mismatch is a failing line whatever the probe |
| +// said; it names the restart that clears it. |
| +func TestReadyRefusesWhenTheRunningBinaryIsNotTheInstalledOne(t *testing.T) { |
| + ctl := &fakeCtl{} |
| + stale := errors.New("instance control is running a binary that is no longer /opt/carlos/carlos; restart carlos-instance-control.service") |
| + var exeErr error |
| + s, c := newServer(t, ctl, func(s *Server) { |
| + s.ExeCurrent = func() error { return exeErr } |
| + s.Build = "abc1234" |
| + }) |
| + b, _ := json.Marshal(ProbeReport{Ready: true, Lines: []VerdictLine{{Check: "pid1", Verdict: "ok"}}}) |
| + if err := os.WriteFile(s.ProbeOut, b, 0o644); err != nil { |
| + t.Fatal(err) |
| + } |
| + s.RunProbe(context.Background()) |
| + resp, err := c.Get("http://unix/ready") |
| + if err != nil || resp.StatusCode != 204 { |
| + t.Fatalf("current binary: %v %v", resp, err) |
| + } |
| + if got := resp.Header.Get(BuildHeader); got != "abc1234" { |
| + t.Fatalf("%s = %q, want the service's build", BuildHeader, got) |
| + } |
| + exeErr = stale |
| + resp, err = c.Get("http://unix/ready") |
| + if err != nil || resp.StatusCode != 503 { |
| + t.Fatalf("stale binary: %v %v", resp, err) |
| + } |
| + if got := resp.Header.Get(BuildHeader); got != "abc1234" { |
| + t.Fatalf("%s on a 503 = %q, want the service's build", BuildHeader, got) |
| + } |
| + var got ProbeReport |
| + if err := json.NewDecoder(resp.Body).Decode(&got); err != nil { |
| + t.Fatal(err) |
| + } |
| + if got.Ready || len(got.Lines) != 2 || got.Lines[1].Check != "binary" || got.Lines[1].Verdict != "fail" || !strings.Contains(got.Lines[1].Detail, "restart carlos-instance-control.service") { |
| + t.Fatalf("body = %+v", got) |
| + } |
| + // The stored report is not what grew: the next call carries one |
| + // binary line, not two. |
| + resp, _ = c.Get("http://unix/ready") |
| + got = ProbeReport{} |
| + _ = json.NewDecoder(resp.Body).Decode(&got) |
| + if len(got.Lines) != 2 { |
| + t.Fatalf("second call = %+v, want the probe line and one binary line", got) |
| + } |
| + exeErr = nil |
| + if resp, err := c.Get("http://unix/ready"); err != nil || resp.StatusCode != 204 { |
| + t.Fatalf("after the restart: %v %v", resp, err) |
| + } |
| +} |