feat(foreman): report the Kubernetes node on FleetNode.status, not as identity

[Defilan]

What

Implement the #1640 decision: FleetNode identity is the agent process, and
the Kubernetes node it runs on becomes a reported property —
status.kubernetesNode.

Why

Refs #1640

The chart has set FLEET_NODE_NAME from the downward API (fieldRef: spec.nodeName) since M4c, but no Go code ever read it. The agent falls back to
os.Hostname() — the pod name — so every replica registers its own FleetNode.
The decision on #1640 established that fallback as the correct model:
spec.nodeName's contract was always worker-scoped, and every
FleetNode.status mechanism (one currentTask reservation, heartbeat-driven
claim expiry and the Draining reaper, phase) assumes a single writer. Wiring
the env var into identity would break all of them at once and reintroduce the
two-agents claim race.

What was genuinely missing is the machine as a property: nothing recorded
which physical node an in-cluster agent pod runs on. On a fleet of two GB10s, a
Strix and metal Macs, "which box is this agent on?" was unanswerable from the CR.

How

• status.kubernetesNode, stamped on heartbeat alongside os/arch, fed
  from FLEET_NODE_NAME — the previously-dead env var, repurposed rather than
  deleted. Off-cluster agents leave it empty (covered by a test).
• spec.nodeName godoc corrected to state it is the worker's own name (the
  pod name in-cluster) and that the Kubernetes node lives at
  status.kubernetesNode. The field is not renamed — that is a CRD break,
  deferred per the decision.
• resolveFleetNodeName extracted from main() and tested, pinning the
  identity-source contract ([BUG] FLEET_NODE_NAME is set from spec.nodeName but never read, so FleetNode identity is the pod, not the node #1640 ask 3): the --fleet-node-name flag wins,
  otherwise the sanitized hostname; FLEET_NODE_NAME is deliberately not an
  input.
• CRDs regenerated: make manifests, chart-crds, foreman-chart-crds;
  make check-helm-rbac green.

The kube-state-metrics CustomResourceState label for the new field rides with
the #1643 CRS fix, where that config file is already being rewritten — adding it
here would guarantee a conflict between the two branches.

Follow-up

#1648 (StatefulSet stable identity) is the named follow-up for the churn cost
this decision accepts.

Verification

• Watched both registrar tests and the resolver test fail first (missing field /
  missing function), then pass
• go test ./pkg/foreman/agent/ ./cmd/foreman-agent/ ./api/foreman/... green
• make lint, GOOS=linux cross-arch lint, make lint-deadcode: 0 issues
• make check-helm-rbac: green; helm unittest charts/foreman: 39/39

Checklist

• Tests added/updated
• make test passes locally
• make lint passes locally
• Commit messages follow conventional commits
• All commits are signed off (git commit -s) per DCO
• AI assistance (if any) is disclosed above, per CONTRIBUTING.md
• Documentation updated (if user-facing change) — CRD godoc is the user-facing surface here

Assisted-by: Claude Code (implemented under TDD from the #1640 decision I made
after its architecture study; I reviewed the change and ran the tests, both
lints, the RBAC check and the chart suite before submitting).

[codecov]

Codecov Report

❌ Patch coverage is 33.33333% with 14 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
cmd/foreman-agent/main.go	26.31%	14 Missing ⚠️

📢 Thoughts on this report? Let us know!

[joryirving]

This is right, and it is the right shape: identity stays the agent process, the machine becomes a property. I checked the premise rather than taking it, and it holds. charts/foreman/templates/agent-deployment.yaml:112 does set FLEET_NODE_NAME from spec.nodeName, and before this PR nothing in Go read it, so the pod-name fallback at the old main.go:279 really was the effective behaviour. The resolveFleetNodeName extraction is behaviour-preserving (same flag-wins ordering, same error on empty or failed hostname, same sanitize-only-when-user-supplied log line), and sanitizeName already trims, truncates to 63 and falls back, so the "DNS-1123 label" claim in the new godoc is honest. Both CRD copies match the Go comments verbatim and there are only two copies, so the regen story is complete. Off-cluster is real too, not aspirational: deployment/macos/com.llmkube.foreman-agent.plist:23 and deployment/linux/llmkube-foreman-agent.service:32 both pass --fleet-node-name.

One test does not test what its comment says. TestRegistrar_PatchHeartbeat_OmitsKubernetesNodeWhenUnset (pkg/foreman/agent/fleetnode_test.go:602) claims "an empty value must not be stamped as anything", but it cannot distinguish the guard from its absence: I deleted if r.KubernetesNode != "" at pkg/foreman/agent/fleetnode.go:253 and made the assignment unconditional, and both new tests still pass. On a freshly created node an unconditional "" writes "" either way. The case that separates them is the one that actually matters: a FleetNode whose status already carries a non-empty kubernetesNode, patched by a Registrar with the field empty. With the guard the old value sticks forever; without it, MergeFrom plus omitempty clears it. I would add that case (Upsert, PatchHeartbeat with KubernetesNode set, then a second Registrar with it empty, assert the value you actually want) so the sticky semantics are a decision on the record rather than an accident. Keeping the guard for consistency with OS and Arch is fine by me; it just needs to be the tested behaviour.

Second, the field is currently write-only. Nothing in the repo reads status.kubernetesNode, and it is not a printer column, so the motivating question ("which box is this agent on?") is still only answerable with -o yaml. Adding a +kubebuilder:printcolumn for it next to api/foreman/v1alpha1/fleetnode_types.go:222 costs nothing and is not a CRD break, and it is the fix that matches the stated motivation. The kube-state-metrics label deferral checks out (#1650 does carry kubernetes_node: [status, kubernetesNode] plus a chart test asserting that pattern), so worth noting the ordering: if #1650 lands first its config labels a field the CRD does not have yet. Harmless, just less confusing merged the other way round.

Third, the docs the issue was about are still wrong. docs/site/foreman/README.md:58 says the foreman-agent "on each host self-registers", and :116 describes the chart as registering "a FleetNode for the gate-runner role on the Linux/K8s host". Under this decision it is one FleetNode per agent process, so a pool with replicaCount: 2 gets two on one host, which is exactly what #1640 showed live. That is a two-line fix and it belongs in this PR rather than behind the CRD-godoc justification in the checklist, because the whole point here is that a documented invariant was inverted.

On #1635: no conflict, and this helps it. I test-merged fix/agent-jobmode-slot into this branch and all four shared files (fleetnode_types.go, both CRD YAMLs, main.go) auto-merged clean, since #1635 touches currentTask's godoc and the watcher wiring while this touches the Arch neighbourhood and the resolve block. More usefully, #1635's Recreate comment and its new README section both assume identity is per agent process, so nailing that down as design here removes the risk that #1640 gets closed the other way and breaks them.

[Defilan]

All three applied, pushed fd09bd709.

The test finding was the useful one, and I reproduced your method: deleting the guard leaves both original tests green, because on a freshly created node an unconditional empty write is indistinguishable from no write. The new case is the one you described, a node already carrying a non-empty kubernetesNode patched by a Registrar with the field empty, and it fails when the guard is removed. Sticky is now the tested decision rather than an accident, consistent with OS and Arch.

Added the K8s Node printcolumn. You are right that a write-only field does not answer the question the issue was about. Worth noting for anyone else touching a foreman.llmkube.dev CRD: make chart-crds is the wrong target and quietly no-ops, it needs make foreman-chart-crds.

Fixed both README lines. That one stings a bit, since the whole point of the issue was that a documented invariant had been inverted, and I left the inverted documentation in place.

On ordering with #1650: agreed, this should land first so the CRD has the field before the kube-state-metrics config labels it.