fix(k3s): count-up/respec join legacy (pre-K3sNode) clusters (bug 2)

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/<cp>` (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/<cp>)`. 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

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 <cp> 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.

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.

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

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)

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{