From 713c6eb05cbba7f114373fbd0bd71f5796d284b1 Mon Sep 17 00:00:00 2001 From: Jeremy Harrington Date: Wed, 8 Jul 2026 06:09:26 -0700 Subject: [PATCH] fix(k3s): count-up/respec join legacy (pre-K3sNode) clusters Bug 2 from the 2026-07-07 recovery: converging an existing cluster created via the old inline path failed with "resolve refs: K3sNode dev-cp-0 not found" -- the Plan-based count-up pointed each new worker join at $ref(K3sNode/), but a legacy cluster has no K3sNode resource for its control planes, so the worker VM was created but never joined. setJoin now branches: if the surviving CP has a K3sNode resource (Plan/ K3sNode-built clusters) it keeps the $ref so the resolver serializes ordering; if not (legacy inline cluster), it resolves the join token + IP directly -- CP IP from the cluster-state agent endpoints, node-token via SSH (cat /var/lib/rancher/k3s/server/node-token) behind a testable readCPNodeToken seam -- and sets concrete joinFrom/joinURLFrom values, which applyK3sNode already accepts as bare strings. Both the count-up (addNodesViaPlan) and respec (respecNodesViaPlan) paths use it. Tests: setJoin uses a $ref when the CP has K3sNode state, and concrete values (via the faked token reader) when it does not; existing count-up/ respec tests updated to seed the CP K3sNode state they implicitly assume (they exercise the Plan/K3sNode path). --- ROADMAP.md | 25 ++--- .../controller/providers/k3s/countup_plan.go | 61 +++++++++++- .../providers/k3s/countup_plan_test.go | 2 + .../providers/k3s/legacy_join_test.go | 94 +++++++++++++++++++ .../controller/providers/k3s/respec_plan.go | 4 +- .../providers/k3s/respec_plan_test.go | 2 + 6 files changed, 174 insertions(+), 14 deletions(-) create mode 100644 internal/controller/providers/k3s/legacy_join_test.go diff --git a/ROADMAP.md b/ROADMAP.md index a898910..d0c5bfc 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -267,17 +267,20 @@ plan/state); harden the provider contract before the ecosystem widens. capture it; a Cluster re-apply now always reconciles (its apply-existing path is idempotent, so a converged cluster is a fast no-op). Tests: `TestGetDetectsDeletedChildVM`, `TestVerifyingCacheSkippedForComposite`. -- [ ] **Bug: Plan-based count-up breaks on legacy (pre-`K3sNode`) - clusters.** After `applyExisting` was retired, existing-cluster - converge goes solely through the Plan/dispatcher path, which emits - worker `K3sNode` children with `$ref` joins to `K3sNode/` (e.g. - `K3sNode/dev-cp-0`). A cluster created via the old inline path has no - `K3sNode` resources for its control planes, so ref resolution fails: - `resolve refs: ref k3s.openctl.io/v1/K3sNode/dev-cp-0: K3sNode - "dev-cp-0" not found` — the worker VM is created but never joins. - *Fix:* synthesize `K3sNode` resources for existing CPs during Plan, - resolve the join token from cluster state instead of a `K3sNode` ref, - or provide a one-time migration for pre-`K3sNode` clusters. +- [x] **Bug: Plan-based count-up breaks on legacy (pre-`K3sNode`) + clusters — FIXED.** Existing-cluster converge goes through the Plan + path, which pointed each new worker's join at `$ref(K3sNode/)`. A + cluster created via the old inline path has no `K3sNode` resource for + its CPs, so ref resolution failed (`K3sNode "dev-cp-0" not found`) and + the worker VM was created but never joined. *Fix:* `setJoin` now + checks whether the surviving CP has a `K3sNode` resource — if so it + keeps the `$ref` (Plan/K3sNode clusters); if not (legacy), it resolves + the join token + IP directly (CP IP from cluster-state endpoints, token + via SSH `cat .../node-token` behind a testable `readCPNodeToken` seam) + and sets **concrete** `joinFrom`/`joinURLFrom` values, which + `applyK3sNode` already accepts as bare strings. Both the count-up and + respec Plan paths use it. Tests: `TestSetJoin_UsesRefWhenK3sNodeExists`, + `TestSetJoin_ConcreteForLegacyCP`. - [x] Plugin-defined CLI subcommands (`openctl k3s logs/restart/upgrade`). Generic protocol + CLI registration shipped: plugins advertise typed subcommands in capabilities, and the CLI dispatches them with diff --git a/internal/controller/providers/k3s/countup_plan.go b/internal/controller/providers/k3s/countup_plan.go index 111f068..6222ee0 100644 --- a/internal/controller/providers/k3s/countup_plan.go +++ b/internal/controller/providers/k3s/countup_plan.go @@ -5,8 +5,12 @@ import ( "fmt" "sort" "strings" + "time" + "github.com/openctl/openctl/internal/config" "github.com/openctl/openctl/internal/controller/operations" + k3sresources "github.com/openctl/openctl/pkg/k3s/resources" + "github.com/openctl/openctl/pkg/k3s/ssh" "github.com/openctl/openctl/pkg/protocol" ) @@ -62,8 +66,10 @@ func (p *Provider) addNodesViaPlan( // Every added node JOINS the cluster — never initializes it. // Plan strips the join refs off the first CP (index 0); if a // re-added cp-0 lands here that would make it try to bootstrap - // a second cluster, so set the refs unconditionally. - setJoinRef(c, survivingCP) + // a second cluster, so set the join unconditionally. + if err := p.setJoin(c, survivingCP, manifest, name, current, removed); err != nil { + return nil, err + } k3sNodes = append(k3sNodes, c) } case kindAgentInstall: @@ -116,6 +122,57 @@ func survivingControlPlane(clusterName string, current []childRef, removed, excl return cps[0], nil } +// readCPNodeToken reads a control plane's k3s join token over SSH. A package +// var so the legacy-CP join path is unit-testable without a live node. +var readCPNodeToken = func(host, user, keyPath string) (string, error) { + client, err := ssh.WaitForSSH(host, sshPort, user, keyPath, 60*time.Second) + if err != nil { + return "", err + } + defer func() { _ = client.Close() }() + tok, err := client.RunSudo("cat /var/lib/rancher/k3s/server/node-token") + if err != nil { + return "", err + } + return strings.TrimSpace(tok), nil +} + +// setJoin points a new node at the surviving control plane. When that CP has a +// K3sNode resource (clusters built via the Plan/K3sNode path), it uses a $ref +// so the ref resolver serializes install ordering. For a LEGACY cluster whose +// CP was installed inline — no K3sNode resource, so the $ref would fail with +// "K3sNode not found" — it resolves the join token + IP directly from +// cluster state + SSH and sets concrete values instead. This is what lets a +// pre-K3sNode cluster be scaled/converged through the Plan path. +func (p *Provider) setJoin(k3sNode *protocol.Resource, cpName string, manifest *protocol.Resource, clusterName string, current []childRef, removed map[string]bool) error { + if st, err := loadNodeState(cpName); err == nil && st != nil { + setJoinRef(k3sNode, cpName) // K3sNode resource exists → $ref + return nil + } + // Legacy CP: resolve the join token + IP concretely. + cpIP, err := p.survivingCPEndpoint(clusterName, current, removed, nil) + if err != nil || cpIP == "" { + return fmt.Errorf("legacy control plane %q has no K3sNode resource and no known endpoint to resolve its join token: %w", cpName, err) + } + spec, err := k3sresources.ParseClusterSpec(manifest) + if err != nil { + return err + } + keyPath := spec.SSH.PrivateKeyPath + if exp, expErr := config.ExpandPath(keyPath); expErr == nil { + keyPath = exp + } + token, err := readCPNodeToken(cpIP, spec.SSH.User, keyPath) + if err != nil { + return fmt.Errorf("read node-token from legacy CP %q (%s): %w", cpName, cpIP, err) + } + // applyK3sNode accepts a bare-string joinFrom (the token) and joinURLFrom + // (the CP IP), bypassing ref resolution. + k3sNode.Spec["joinFrom"] = token + k3sNode.Spec["joinURLFrom"] = cpIP + return nil +} + // setJoinRef points a K3sNode's join refs at cpName's K3sNode state so the // node joins the existing cluster (resolving status.nodeToken + // status.vmIP) rather than initializing a new one. diff --git a/internal/controller/providers/k3s/countup_plan_test.go b/internal/controller/providers/k3s/countup_plan_test.go index c1dd206..5820528 100644 --- a/internal/controller/providers/k3s/countup_plan_test.go +++ b/internal/controller/providers/k3s/countup_plan_test.go @@ -30,6 +30,7 @@ func joinRefName(t *testing.T, k3sNode *protocol.Resource, field string) string // the surviving control plane, and returns its resolved IP. func TestAddNodesViaPlan_AppliesAddedNodeChildrenWithSurvivingCPJoin(t *testing.T) { t.Setenv("HOME", t.TempDir()) + seedServerNodes(t, "dev-cp-0") // Desired: 1 CP (dev-cp-0, already exists) + 1 worker (dev-w-0, to add). m := clusterManifest("dev", func(r *protocol.Resource) { @@ -84,6 +85,7 @@ func TestAddNodesViaPlan_AppliesAddedNodeChildrenWithSurvivingCPJoin(t *testing. // CP, not the default index-0 target. func TestAddNodesViaPlan_RepointsJoinWhenCP0Removed(t *testing.T) { t.Setenv("HOME", t.TempDir()) + seedServerNodes(t, "dev-cp-0", "dev-cp-1") // Desired: 2 CPs (cp-0, cp-1) + 1 worker to add. cp-1 already exists and // survives; cp-0 is being removed this converge. diff --git a/internal/controller/providers/k3s/legacy_join_test.go b/internal/controller/providers/k3s/legacy_join_test.go new file mode 100644 index 0000000..28611a7 --- /dev/null +++ b/internal/controller/providers/k3s/legacy_join_test.go @@ -0,0 +1,94 @@ +package k3s + +import ( + "testing" + + "github.com/openctl/openctl/pkg/protocol" +) + +// clusterStateWithEndpoint writes a cluster state file whose agent endpoints +// map the CP to an IP, so survivingCPEndpoint can resolve it. +func clusterStateWithEndpoint(t *testing.T, home, cluster, cpName, cpIP string) { + t.Helper() + writeClusterState(t, home, cluster, `apiVersion: k3s.openctl.io/v1 +kind: Cluster +metadata: + name: `+cluster+` +status: + outputs: + agent: + endpoints: + `+cpName+`: `+cpIP+` +children: + - provider: proxmox + kind: VirtualMachine + name: `+cpName+` +`) +} + +// seedServerNodes writes K3sNode state for control planes, representing a +// cluster built via the Plan/K3sNode path (so setJoin uses a $ref). +func seedServerNodes(t *testing.T, names ...string) { + t.Helper() + for _, n := range names { + if err := saveNodeState(&nodeState{Name: n, Role: roleServer, Installed: true, NodeToken: "tok-" + n}); err != nil { + t.Fatal(err) + } + } +} + +// When the CP has a K3sNode resource, setJoin uses a $ref (Plan/K3sNode path). +func TestSetJoin_UsesRefWhenK3sNodeExists(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + clusterStateWithEndpoint(t, home, "dev", "dev-cp-0", "10.0.0.10") + // A K3sNode resource exists for the CP. + if err := saveNodeState(&nodeState{Name: "dev-cp-0", Role: roleServer, Installed: true, NodeToken: "tok"}); err != nil { + t.Fatal(err) + } + + worker := &protocol.Resource{Spec: map[string]any{}} + p := &Provider{} + current := []childRef{{Kind: "VirtualMachine", Name: "dev-cp-0"}} + if err := p.setJoin(worker, "dev-cp-0", clusterManifest("dev"), "dev", current, nil); err != nil { + t.Fatalf("setJoin: %v", err) + } + jf, _ := worker.Spec["joinFrom"].(map[string]any) + if jf == nil || jf["$ref"] == nil { + t.Errorf("joinFrom = %v, want a $ref", worker.Spec["joinFrom"]) + } +} + +// Legacy cluster: the CP has no K3sNode resource, so setJoin resolves the token +// concretely (via the readCPNodeToken seam) instead of an unresolvable $ref. +func TestSetJoin_ConcreteForLegacyCP(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + clusterStateWithEndpoint(t, home, "dev", "dev-cp-0", "10.0.0.10") + // No K3sNode state for dev-cp-0 → legacy. + + // Fake the SSH token read. + orig := readCPNodeToken + defer func() { readCPNodeToken = orig }() + var gotHost, gotUser string + readCPNodeToken = func(host, user, keyPath string) (string, error) { + gotHost, gotUser = host, user + return "K10legacy::server:abc", nil + } + + worker := &protocol.Resource{Spec: map[string]any{}} + p := &Provider{} + current := []childRef{{Kind: "VirtualMachine", Name: "dev-cp-0"}} + if err := p.setJoin(worker, "dev-cp-0", clusterManifest("dev"), "dev", current, nil); err != nil { + t.Fatalf("setJoin: %v", err) + } + if gotHost != "10.0.0.10" || gotUser != "ubuntu" { + t.Errorf("token read from %s@%s, want ubuntu@10.0.0.10", gotUser, gotHost) + } + if worker.Spec["joinFrom"] != "K10legacy::server:abc" { + t.Errorf("joinFrom = %v, want the concrete token", worker.Spec["joinFrom"]) + } + if worker.Spec["joinURLFrom"] != "10.0.0.10" { + t.Errorf("joinURLFrom = %v, want the concrete CP IP", worker.Spec["joinURLFrom"]) + } +} diff --git a/internal/controller/providers/k3s/respec_plan.go b/internal/controller/providers/k3s/respec_plan.go index 4afecb6..0485952 100644 --- a/internal/controller/providers/k3s/respec_plan.go +++ b/internal/controller/providers/k3s/respec_plan.go @@ -77,7 +77,9 @@ func (p *Provider) respecNodesViaPlan( } // Recreate at the desired size, rejoining the surviving CP. - setJoinRef(k3sNode, survivingCP) + if err := p.setJoin(k3sNode, survivingCP, manifest, name, current, removed); err != nil { + return nil, fmt.Errorf("respec %s: resolve join: %w", node, err) + } for _, child := range []*protocol.Resource{vm, k3sNode, agent} { if _, err := cd.ApplyChild(ctx, child); err != nil { return nil, fmt.Errorf("respec %s: recreate: %w", node, err) diff --git a/internal/controller/providers/k3s/respec_plan_test.go b/internal/controller/providers/k3s/respec_plan_test.go index f9709b1..5e63601 100644 --- a/internal/controller/providers/k3s/respec_plan_test.go +++ b/internal/controller/providers/k3s/respec_plan_test.go @@ -15,6 +15,7 @@ import ( // the *other* CP (cp-1) — never itself, which is down during its own respec. func TestRespecNodesViaPlan_DestroyRecreateRejoinSurvivingCP(t *testing.T) { t.Setenv("HOME", t.TempDir()) + seedServerNodes(t, "dev-cp-0", "dev-cp-1") m := clusterManifest("dev", func(r *protocol.Resource) { r.Spec["nodes"].(map[string]any)["controlPlane"] = map[string]any{"count": float64(2)} @@ -70,6 +71,7 @@ func TestRespecNodesViaPlan_DestroyRecreateRejoinSurvivingCP(t *testing.T) { // control plane (cp-0), and the worker's own respec doesn't exclude any CP. func TestRespecNodesViaPlan_WorkerRejoinsCP(t *testing.T) { t.Setenv("HOME", t.TempDir()) + seedServerNodes(t, "dev-cp-0") m := clusterManifest("dev", func(r *protocol.Resource) { r.Spec["nodes"].(map[string]any)["workers"] = []any{