From 4ff62c5dd28ebd930b55e9b79567493aa1ff53c7 Mon Sep 17 00:00:00 2001
From: Ilker Manap <ilker@Ilkers-MacBook-Pro.local>
Date: Mon, 5 Oct 2026 12:43:25 +0200
Subject: [PATCH] ctl upgrade asks the server what it runs, and waits for a
 node as long as it does

A binary installed by hand made ctl upgrade say this machine already ran
the release and leave the old server running. And a node had 15 seconds
to answer an upgrade the control plane gives five minutes, so a node that
was upgrading stopped the run.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
---
 CHANGELOG.md                         |  9 +++++++
 docs/admin-api.openapi.json          |  4 +++
 internal/cli/ctl.go                  |  8 ++++++
 internal/cli/ctl_upgrade.go          | 37 ++++++++++++++++++++++------
 internal/cli/ctl_upgrade_test.go     | 19 ++++++++++++++
 internal/control/adminapi/summary.go | 10 +++++---
 6 files changed, 77 insertions(+), 10 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5c6a1f69..8aa1258c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -32,6 +32,15 @@ APIs change between minors without a deprecation period.
   the old file beside it as `server.yaml.bak-<time>`. A file that already says
   `guard:`, even empty, is left alone.
 
+- `ctl upgrade` on a control plane whose binary was installed by hand said
+  "this machine already runs" it and left the old server running: it read
+  its own binary's version. It now asks the server what it runs, and
+  restarts it when only the binary is new.
+- `ctl upgrade` gave a node 15 seconds to answer the upgrade, while the
+  control plane waits up to five minutes for the agent to fetch and check
+  the release; a node that was upgrading stopped the run. It now waits as
+  long as the control plane does.
+
 ## v0.11.4 — 2026-10-05
 
 Two control planes, run in anger on the reference cluster.
diff --git a/docs/admin-api.openapi.json b/docs/admin-api.openapi.json
index 4219f346..2e9a5732 100644
--- a/docs/admin-api.openapi.json
+++ b/docs/admin-api.openapi.json
@@ -5416,6 +5416,10 @@
         "type": "object",
         "description": "Summary is the whole fleet in one answer.\n\nA per-node listing is the right tool for four nodes and the wrong one for two hundred: the questions that matter at that size — how full am I, what is broken, what is holding cards it is not using — are aggregates, and reading them off a long table is a chore that gets skipped. Everything here is derived from the same state the tables show, so it can never disagree with them; it is a different question asked of the same facts.",
         "properties": {
+          "version": {
+            "type": "string",
+            "description": "Version is the release the answering control plane runs: not the binary on its disk, which a manual install changes underneath it."
+          },
           "nodes": {
             "$ref": "#/components/schemas/NodeTotals"
           },
diff --git a/internal/cli/ctl.go b/internal/cli/ctl.go
index ebe72cf9..1952db69 100644
--- a/internal/cli/ctl.go
+++ b/internal/cli/ctl.go
@@ -39,6 +39,14 @@ func newCtlClient(addr string, tokenFiles ...string) *ctlClient {
 	}
 }
 
+// patient is the same client waiting up to d for an answer, for a call the
+// server itself may spend minutes on.
+func (c *ctlClient) patient(d time.Duration) *ctlClient {
+	cp := *c
+	cp.http = &http.Client{Timeout: d}
+	return &cp
+}
+
 func (c *ctlClient) get(ctx context.Context, path string, out any) error {
 	return c.do(ctx, http.MethodGet, path, nil, out)
 }
diff --git a/internal/cli/ctl_upgrade.go b/internal/cli/ctl_upgrade.go
index af863640..499c640f 100644
--- a/internal/cli/ctl_upgrade.go
+++ b/internal/cli/ctl_upgrade.go
@@ -152,8 +152,20 @@ func upgradeOtherControlPlanes(ctx context.Context, out io.Writer, client func()
 	return nil
 }
 
+// controlPlaneStep is what this machine needs to run v: the binary installed,
+// the service restarted, both, or neither. The binary is this command's own;
+// running is what the server says it runs, "" from one too old to say. A
+// binary put in place by hand is not a server running it.
+func controlPlaneStep(binary, running, v string) (install, restart bool) {
+	install = binary != v
+	return install, install || running != v
+}
+
 func upgradeControlPlane(ctx context.Context, out io.Writer, client func() *ctlClient, v, releases, svc string) error {
-	if version.Version == v {
+	var sum adminapi.Summary
+	_ = client().get(ctx, "/api/v1/summary", &sum)
+	install, restart := controlPlaneStep(version.Version, sum.Version, v)
+	if !restart {
 		fmt.Fprintf(out, "this machine already runs %s\n", v)
 		return nil
 	}
@@ -165,12 +177,20 @@ func upgradeControlPlane(ctx context.Context, out io.Writer, client func() *ctlC
 	if err != nil {
 		return err
 	}
-	fmt.Fprintf(out, "== this machine: %s -> %s\n", version.Version, v)
-	backup, err := release.Install(ctx, nil, releases, v, runtime.GOOS, runtime.GOARCH, self)
-	if err != nil {
-		return err
+	backup := "the binary installed by hand before it"
+	if install {
+		fmt.Fprintf(out, "== this machine: %s -> %s\n", version.Version, v)
+		if backup, err = release.Install(ctx, nil, releases, v, runtime.GOOS, runtime.GOARCH, self); err != nil {
+			return err
+		}
+		fmt.Fprintf(out, "   installed, checksum verified; the old binary is %s\n", backup)
+	} else {
+		running := sum.Version
+		if running == "" {
+			running = "an older release"
+		}
+		fmt.Fprintf(out, "== this machine: %s is installed, and %s still runs it; restarting %s\n", v, running, svc)
 	}
-	fmt.Fprintf(out, "   installed, checksum verified; the old binary is %s\n", backup)
 	var before []adminapi.NodeView
 	_ = client().get(ctx, "/api/v1/nodes", &before)
 	if b, err := exec.CommandContext(ctx, "systemctl", "restart", svc).CombinedOutput(); err != nil { //nolint:gosec // fixed command
@@ -270,7 +290,10 @@ func upgradeWave(ctx context.Context, out io.Writer, client func() *ctlClient, v
 	body, _ := json.Marshal(adminapi.UpgradeRequest{Version: v})
 	var failed []string
 	for _, n := range wave {
-		if err := client().post(ctx, "/api/v1/nodes/"+n.ID+"/upgrade", body, nil); err != nil {
+		// The control plane waits for the agent to fetch and check the
+		// release, up to five minutes; fifteen seconds gave up on a node
+		// that was upgrading.
+		if err := client().patient(6*time.Minute).post(ctx, "/api/v1/nodes/"+n.ID+"/upgrade", body, nil); err != nil {
 			failed = append(failed, fmt.Sprintf("%s: %v", n.ID, err))
 		}
 	}
diff --git a/internal/cli/ctl_upgrade_test.go b/internal/cli/ctl_upgrade_test.go
index bcfda526..6018d58f 100644
--- a/internal/cli/ctl_upgrade_test.go
+++ b/internal/cli/ctl_upgrade_test.go
@@ -131,3 +131,22 @@ func TestAnUpgradeKnowsWhetherItRunsOnTheLeader(t *testing.T) {
 		t.Error("the leader took itself for a standby")
 	}
 }
+
+func TestABinaryInstalledByHandIsNotAServerRunningIt(t *testing.T) {
+	// The binary put in place with install, llmgw-server not restarted: the
+	// upgrade said "already runs" and left the old server running.
+	for _, c := range []struct {
+		binary, running  string
+		install, restart bool
+	}{
+		{"v1.0.0", "v1.0.0", true, true},  // the usual: fetch it, restart
+		{"v1.0.1", "v1.0.0", false, true}, // installed by hand: restart only
+		{"v1.0.1", "", false, true},       // a server too old to say
+		{"v1.0.1", "v1.0.1", false, false},
+	} {
+		install, restart := controlPlaneStep(c.binary, c.running, "v1.0.1")
+		if install != c.install || restart != c.restart {
+			t.Errorf("binary %s, running %q: install %v restart %v", c.binary, c.running, install, restart)
+		}
+	}
+}
diff --git a/internal/control/adminapi/summary.go b/internal/control/adminapi/summary.go
index b9c106b7..9c0d9e3d 100644
--- a/internal/control/adminapi/summary.go
+++ b/internal/control/adminapi/summary.go
@@ -1,6 +1,7 @@
 package adminapi
 
 import (
+	"gitea.mynodes.xyz/ilker/llmgw/internal/version"
 	"sort"
 	"time"
 )
@@ -14,8 +15,11 @@ import (
 // derived from the same state the tables show, so it can never disagree with
 // them; it is a different question asked of the same facts.
 type Summary struct {
-	Nodes NodeTotals `json:"nodes"`
-	GPUs  []ClassUse `json:"gpus"`
+	// Version is the release the answering control plane runs: not the
+	// binary on its disk, which a manual install changes underneath it.
+	Version string     `json:"version,omitempty"`
+	Nodes   NodeTotals `json:"nodes"`
+	GPUs    []ClassUse `json:"gpus"`
 	// Fabrics matter because a multi-node replica may never span two of them,
 	// so free capacity in the wrong fabric is not capacity for the model that
 	// needs it.
@@ -144,7 +148,7 @@ const idleThreshold = 10 * time.Minute
 
 // Summary builds the fleet summary.
 func (a *API) Summary() Summary {
-	var s Summary
+	s := Summary{Version: version.Version}
 	if a.storeStatus != nil {
 		members, err := a.storeStatus()
 		s.Store = members