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) + } +}