From f5eb6963afe70309fd820ff7bbc86fd6f1558df7 Mon Sep 17 00:00:00 2001 From: Ilker Manap Date: Wed, 14 Oct 2026 09:01:09 +0200 Subject: [PATCH] An upgrade that leaves nodes behind says which A site's nodes stayed a release behind after its control plane was upgraded, with nothing printed to say so. ctl upgrade checks every node's version once the waves are done, fails naming any not on the release, and says when the control plane has no nodes registered. llmgw upgrade on a control plane, which looks like the cluster's upgrade and is not, says how many nodes it left behind and the command that upgrades them. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 5 ++++ internal/cli/ctl_upgrade.go | 20 +++++++++++++ internal/cli/ctl_upgrade_test.go | 51 ++++++++++++++++++++++++++++++++ internal/cli/selfupgrade.go | 28 ++++++++++++++++++ 4 files changed, 104 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b377d3..b5ba9dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,11 @@ APIs change between minors without a deprecation period. ### Fixed +- `llmgw ctl upgrade` checks every node's version once the waves are done + and fails naming any not on the release, and says when the control plane + has no nodes registered; `llmgw upgrade` on a control plane says how many + nodes it left behind and the command that upgrades them. A site's nodes + stayed a release behind with nothing to say so. - The cluster and node pages swapped their live part every few seconds, forms and all, so filling one in raced the clock: what was typed, an open choice and the focus were lost. The refresh now waits while a field diff --git a/internal/cli/ctl_upgrade.go b/internal/cli/ctl_upgrade.go index 499c640..a87c9f5 100644 --- a/internal/cli/ctl_upgrade.go +++ b/internal/cli/ctl_upgrade.go @@ -268,6 +268,26 @@ func upgradeNodes(ctx context.Context, out io.Writer, client func() *ctlClient, } fmt.Fprintf(out, "%d node(s) upgraded to %s in %s, %d already on it\n", done, v, time.Since(start).Round(time.Second), skipped) + // Checked at the end, against what the control plane says now: a node + // that is not on the version is said by name, and the command fails. A + // site's nodes stayed a release behind with nothing printed to say so. + if len(nodes) == 0 { + fmt.Fprintln(out, "This control plane has no nodes registered, so none were upgraded.") + } + var now []adminapi.NodeView + if err := client().get(ctx, "/api/v1/nodes", &now); err == nil { + var behind []string + for _, n := range now { + if (len(only) == 0 || slices.Contains(only, n.ID)) && n.AgentVersion != v { + behind = append(behind, fmt.Sprintf("%s (%s, %s)", n.ID, orUnknown(n.AgentVersion), n.State)) + } + } + if len(behind) > 0 { + slices.Sort(behind) + return fmt.Errorf("not on %s: %s\nrun this again to try them, or `llmgw ctl logs NODE` for why", + v, strings.Join(behind, ", ")) + } + } // The one machine this does not reach. Said every time, because it is // exactly the kind of thing that stays three versions behind unnoticed. fmt.Fprintf(out, "The model mirror's host is not upgraded by this; on it, run:\n"+ diff --git a/internal/cli/ctl_upgrade_test.go b/internal/cli/ctl_upgrade_test.go index 6018d58..f65376b 100644 --- a/internal/cli/ctl_upgrade_test.go +++ b/internal/cli/ctl_upgrade_test.go @@ -150,3 +150,54 @@ func TestABinaryInstalledByHandIsNotAServerRunningIt(t *testing.T) { } } } + +func TestAnUpgradeThatFindsNoNodesSaysSo(t *testing.T) { + // A site's nodes stayed a release behind with nothing printed to say + // that none had been reached. + f := &fakeFleet{versions: map[string]string{}} + srv := f.server(t) + var out bytes.Buffer + if err := upgradeNodes(t.Context(), &out, func() *ctlClient { return newCtlClient(srv.URL) }, "v1.0.1", nil, time.Second, 1); err != nil { + t.Fatal(err) + } + if !strings.Contains(out.String(), "no nodes registered") { + t.Errorf("output:\n%s", out.String()) + } +} + +func TestAnUpgradeFailsNamingANodeLeftBehind(t *testing.T) { + // A node that joins, or comes back on its old binary, while the waves + // run is found at the end rather than passed over. + f := &fakeFleet{versions: map[string]string{"a": "v1.0.0"}} + srv := f.server(t) + late := func() *ctlClient { + f.mu.Lock() + if len(f.told) == 1 { + f.versions["late"] = "v1.0.0" + } + f.mu.Unlock() + return newCtlClient(srv.URL) + } + var out bytes.Buffer + err := upgradeNodes(t.Context(), &out, late, "v1.0.1", nil, 5*time.Second, 1) + if err == nil || !strings.Contains(err.Error(), "late (v1.0.0, ready)") { + t.Errorf("err %v", err) + } +} + +func TestUpgradingAControlPlaneAloneSaysItsNodesWereNot(t *testing.T) { + f := &fakeFleet{versions: map[string]string{"p-g1": "v1.0.0", "p-g3": "v1.0.1"}} + srv := f.server(t) + var out bytes.Buffer + nodesLeftBehind(t.Context(), &out, newCtlClient(srv.URL), "v1.0.1") + if !strings.Contains(out.String(), "1 of 2 node(s) are not on v1.0.1") || + !strings.Contains(out.String(), "ctl upgrade v1.0.1 --skip-control-plane") { + t.Errorf("output:\n%s", out.String()) + } + out.Reset() + f.versions["p-g1"] = "v1.0.1" + nodesLeftBehind(t.Context(), &out, newCtlClient(srv.URL), "v1.0.1") + if out.Len() != 0 { + t.Errorf("said something with every node on the version: %s", out.String()) + } +} diff --git a/internal/cli/selfupgrade.go b/internal/cli/selfupgrade.go index 49e7995..17d2df5 100644 --- a/internal/cli/selfupgrade.go +++ b/internal/cli/selfupgrade.go @@ -1,8 +1,11 @@ package cli import ( + "context" "errors" "fmt" + "gitea.mynodes.xyz/ilker/llmgw/internal/control/adminapi" + "io" "os" "os/exec" "runtime" @@ -70,6 +73,9 @@ func newSelfUpgradeCmd() *cobra.Command { if strings.TrimSpace(string(b)) == "active" { if steady++; steady >= 3 { fmt.Fprintf(out, "%s restarted and running\n", service) + if service == "llmgw-server" { + nodesLeftBehind(cmd.Context(), out, newCtlClient("127.0.0.1:9090"), v) + } return nil } } else { @@ -85,3 +91,25 @@ func newSelfUpgradeCmd() *cobra.Command { c.Flags().StringVar(&service, "service", "", "the systemd unit to restart afterwards, e.g. llmgw-mirror") return c } + +// nodesLeftBehind says, after a control plane alone was upgraded, that its +// nodes were not, and how they are: this command looks like the cluster's +// upgrade and is not, and a site's nodes stayed a release behind after it. +func nodesLeftBehind(ctx context.Context, out io.Writer, c *ctlClient, v string) { + var nodes []adminapi.NodeView + _ = c.patient(20*time.Second).get(ctx, "/api/v1/nodes", &nodes) + behind := 0 + for _, n := range nodes { + if n.AgentVersion != v { + behind++ + } + } + switch { + case len(nodes) == 0: + fmt.Fprintf(out, "This upgraded this machine only. The nodes are upgraded from the leading "+ + "control plane with:\n sudo llmgw ctl upgrade %s --skip-control-plane\n", v) + case behind > 0: + fmt.Fprintf(out, "This upgraded this machine only: %d of %d node(s) are not on %s. On the leading "+ + "control plane, run:\n sudo llmgw ctl upgrade %s --skip-control-plane\n", behind, len(nodes), v, v) + } +}