feat(talos): Config ohne Nachbearbeitung, plus Drei-VM-Rebuildtest #716
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: konairius/homelab#716
Reference in New Issue
Block a user
bootstrap.shhat die generierte Config mit awk nachbearbeitet, bevor es sie angewandt hat — dasHostnameConfig-Dokument wurde herausgeschnitten. Der Block kam im April dazu, als Talos 1.12 anfing es zu emittieren, und er war seitdem tragend: ohne ihn lehnt apidapply-configdirekt ab.Das Dokument war nie das Problem. Den Hostnamen in zwei Schemata gleichzeitig zu setzen ist es:
machine.network.hostnameundmachine.features.stableHostnamesind die v1alpha1-Schreibweisen dessen, was das generierte Dokument ohnehin sagt. Der Name steht jetzt im Dokument, mitauto: offdaneben — und damit fällt der awk-Block weg.Das ist mehr als Kosmetik. Solange die Config nachbearbeitet werden musste, konnte sie nur dieses eine Skript erzeugen. Alles andere, das
talosctl gen configgegen diese Patches laufen ließ — auchtalosctl cluster create— bekam eine Config, die die Nodes verweigern. Genau deshalb war davon nie etwas außerhalb eines echten Rebuilds erprobt.Drei Ebenen statt zwei
Zwei Werte beschrieben dieses Netz statt dieses Clusters und sind aus
cluster.yamlverschwunden:kubelet.nodeIP.validSubnetssite-homelab.yaml10.2.40.0/24, ob er dort ein Interface hat oder nichtcluster.controlPlane.endpointgen confignimmt den Endpoint als zweites Argument,bootstrap.shübergibt ihn bereits — die Wiederholung überschrieb, wofür ein Aufrufer generiert hatteDamit gibt es
cluster.yaml(jedes Cluster),site-*.yaml(dieser Standort) und die Node-Patches (diese Maschine).Der Test
./bootstrap.sh vmtesterzeugt drei QEMU-VMs und fährt die Talos-Phase unverändert gegen sie. Bewusst keine Kopie der Phase: es setztNODES,NODE_ADDRS,SITEundK8S_API_HOSTund fällt durch — ein bestandener Test sagt sonst nur etwas über sich selbst aus. Talos-Version und Schematic liest die Phase ausinstall.imageincluster.yaml, statt sie ein zweites Mal zu nennen.Er hat im ersten Lauf zwei echte Fehler gefunden, beide hier behoben:
Die Phase benutzte die kubeconfig, sobald
talosctl kubeconfigzurückkam — das ist der Moment, in dem Talos Credentials ausstellen kann, nicht der, in demkube-apiserverauf dem Endpoint lauscht. Helm lief in einEOFgegen einen Port, an dem nichts horchte. Jetzt wartet die Phase aufkubectl get --raw /version;kubectl versionhätte aus dem Client geantwortet und gegen ein leeres Cluster bestanden.echoohne-egab rohe Escape-Sequenzen aus statt Farbe.Isolation
TALOSCONFIG/KUBECONFIGwerden für die Access-Dateien beachtet statt fester$HOME-Pfade; ungesetzt lösen sie auf exakt dieselben zwei Pfade auf wie bisher. Das verhindert, dass ein Testlauf den produktiven Zugang durch den einer VM ersetzt.access-staging/wird untervmtestaus demselben Grund übersprungen, und ein EXIT-Trap räumt die VMs auch bei Abbruch ab.Verifiziert
Drei Nodes
Readyauf Cilium 1.20.1 aus der HelmRelease, jeder mit seinem eigenen Patch, etcd-Quorum gebildet. Alle vier Node-Patches generieren und validieren ohne jede Nachbearbeitung.Nicht abgedeckt
Nur auf Hardware zu beweisen: Ceph (Zusatzplatten gehen laut
--disksnur an Worker), die mgmt- und v30iot-VLANs, der Layer2-VIP, Flux — und das Anwenden der drei geänderten Produktions-Patches auf die echten Nodes. Die sind bisher generiert und validiert, nicht appliziert.bootstrap.sh generated a config and then edited it with awk before applying it, cutting out the HostnameConfig document `talosctl gen config` emits. That was added in April when Talos 1.12 started emitting it, and it has been load-bearing since: without it apid rejects apply-config outright with static hostname is already set in v1alpha1 config The document was never the problem. Naming the host in two schemas at once is: machine.network.hostname and machine.features.stableHostname are the v1alpha1 spellings of what the generated document already says. Setting the name in the document instead — with `auto: off`, which is otherwise mutually exclusive with a static name — removes the conflict, and with it the awk. That matters beyond tidiness. Post-processing the generator's output meant the config could only be produced by this one script: anything else that ran `talosctl gen config` against these patches, including `talosctl cluster create`, got a config the nodes refuse. Which is why none of this had ever been tried outside a real rebuild. Two more values moved out of cluster.yaml for the same reason — they described this network rather than this cluster, so a config generated for anywhere else was wrong before it was applied: kubelet.nodeIP.validSubnets now talos/patches/site-homelab.yaml, a third layer between the cluster and the machine. It pinned every node to 10.2.40.0/24 whether or not it has an interface there. cluster.controlPlane.endpoint removed outright. `gen config` takes the endpoint as its second argument and bootstrap.sh already passes it; restating it here overrode whatever a caller had generated for. With that, `./bootstrap.sh vmtest` provisions three throwaway QEMU VMs and runs the talos phase against them unchanged. It is deliberately not a copy of that phase: it sets NODES, NODE_ADDRS, SITE and K8S_API_HOST and falls through, so a passing test says something about a rebuild rather than about itself. Talos version and Image Factory schematic are read out of install.image in cluster.yaml rather than named again. It found two real defects on its first run, both fixed here: The phase used the kubeconfig the moment `talosctl kubeconfig` returned, which is when Talos can issue credentials — not when kube-apiserver is serving on the endpoint the file points at. Helm hit `EOF` against a port nothing was listening on. It now waits for `kubectl get --raw /version`, which needs the server; `kubectl version` would have answered from the client and passed against nothing. `echo` without -e printed raw escape codes instead of colouring the per-node result. TALOSCONFIG/KUBECONFIG are honoured for the access files instead of hardcoded $HOME paths, unset resolving to the same two paths as before. That is what keeps a test run from replacing the operator's production access with a VM's; access- staging is skipped under vmtest for the same reason. Verified: three VMs Ready on Cilium 1.20.1 from the HelmRelease, and all four node patches generate and validate with no post-processing at all. Not covered, and still only provable on hardware: Ceph (the provisioner gives extra disks to workers only), the mgmt and v30iot VLANs, the Layer2 VIP, Flux, and applying the three changed production patches to the real nodes.🤖 Automated review — model
CLI default(complexity: complex), commit6e8d4cdfbad62e3d8d4f17bcd91e1c0cd46d0bf5.Summary
This PR removes the awk-based
HostnameConfigstripping frombootstrap.shbysetting the hostname directly in that document instead of via
machine.network.hostname, splitscluster.yamlinto cluster/site/nodelayers (moving
kubelet.nodeIP.validSubnetsto a newsite-homelab.yamlanddropping the duplicated
cluster.controlPlane.endpoint), and adds avmtestphase that runs the real Talos bootstrap phase unchanged against three
throwaway QEMU VMs. The script changes themselves are careful and were
apparently exercised end-to-end (3 VMs reaching
Ready); the one real gap isthat the two disaster-recovery runbooks which duplicate this same
talosctl gen config/apply-configinvocation were not updated to match.🔴 Blocking
kubernetes/RUNBOOK-cold-rebuild.md:169-171andkubernetes/RUNBOOK-baremetal-conversion.md:200-202build a node config with--config-patch @talos/patches/cluster.yaml,containerd-layer-retention.yamland
k8s-N.yaml, mirroring the sequencebootstrap.shused to apply. Thatsequence is now missing a patch:
kubelet.nodeIP.validSubnetsmoved out ofcluster.yamlinto the newtalos/patches/site-homelab.yaml(
kubernetes/talos/patches/cluster.yaml:44-48), but neither runbook's commandlist was updated to add
--config-patch @talos/patches/site-homelab.yaml. Anode rebuilt by following either runbook today would come up without the
mgmt-LAN constraint on kubelet's node IP — exactly the failure mode the
comment at
site-homelab.yaml:9-11describes ("kubelet picks the firstnon-loopback IP, which could land on the v30iot NIC"). This is the precise
scenario these runbooks exist for (bare-metal node replacement / cold
rebuild), so a stale command here is a real operational risk, not just
doc drift.
HostnameConfigdocument — it collides with
machine.network.hostnamefrom the per-nodepatch" (
RUNBOOK-cold-rebuild.md:180-181,RUNBOOK-baremetal-conversion.md:207-208),and
RUNBOOK-baremetal-conversion.md:208adds "bootstrap.shdoes thesame." Both statements are now false: the per-node patches
(
k8s-1.yaml/k8s-2.yaml/k8s-3.yaml) no longer setmachine.network.hostnameat all (that's exactly what this PR removed), andbootstrap.shno longer strips anything (that's the awk block this PRdeletes). An operator following this instruction during a real recovery
would manually delete the node patch's own
HostnameConfigdocument (theone with
auto: off, hostname: k8s-N), leaving the node without itsintended static hostname.
🟡 Suggestions
kubernetes/bootstrap.sh:58-60— theKUBECONFIGmulti-path check(
case "$KUBECONFIG_PATH" in *:*) ... exit 1 ;; esac) runs unconditionallyat the top of the script, before
PHASEis even considered.KUBECONFIG_PATHis only actually read inside the
talosphase;fluxandwait-appsuse~/.kube/configdirectly viaload_staged_access(bootstrap.sh:372-378)and never touch this variable. An operator whose shell has a merged
KUBECONFIG=~/.kube/config:~/.kube/other(a common kubectl pattern) wouldnow get
./bootstrap.sh fluxor./bootstrap.sh wait-appsrejectedoutright for a reason unrelated to what those phases do. Worth gating the
check on
phase_runs talos(or moving it inside that block).kubernetes/bootstrap.sh:143,168—VM_STATE=/tmp/bootstrap-vmtestis afixed, predictable path in world-writable
/tmp, and it's used withsudo(
sudo -E "$TALOSCTL_BIN" cluster create ...). On a single-user box this isharmless, but on any shared host a local user could pre-create
/tmp/bootstrap-vmtest(e.g. as a symlink) before this runs.VM_DIRrightnext to it already uses
mktemp -dfor the same reason;VM_STATEcan't usea long mktemp path because of the 108-byte
sun_pathlimit the commentexplains, but something like
mktemp -u -p /tmp -t bootstrap-vmtest.XXXXXX(still short, no longer guessable) would close that gap without giving up
the length constraint. Low priority given this only runs locally/by an
operator today (not wired into any CI config in this repo).
kubernetes/bootstrap.sh:120-197(vmtest) and thetalosphase shareVM_CLUSTER=bootstrap-vmtestas a fixed cluster name alongside the fixedVM_STATE. Two concurrentvmtestruns on the same host would collide.Given the script is explicitly local/manual today, this is only worth fixing
if
vmtestis later wired into CI on a shared runner.🟢 Nits
kubernetes/talos/patches/cluster.yaml:2-7(machine.certSANs) still liststhe three production FQDNs and the production VIP unconditionally; these end
up applied to the throwaway VMs too via
vmtest(cluster.yamlis appliedfor every node regardless of
SITE). Harmless — extra unused SANs — butslightly at odds with the PR's "three layers" framing where
cluster.yamlis meant to hold only facts true of every cluster built from it.
Tests
The change is its own test: the new
vmtestphase runs the unmodifiedtalosphase against three throwaway QEMU VMs generated straight from
cluster.yaml/site-qemu.yaml/vm-{1,2,3}.yaml, which is a meaningfullystronger check than a unit test would be for a bash/Talos-config script like
this (per the PR description, it already caught two real bugs: the kubeconfig
race and the missing
echo -e). It's not wired into CI, so it only runs whensomeone remembers to invoke
./bootstrap.sh vmtestlocally — worth keeping inmind, but not something to block this PR on.
Recommendation
Request changes — 2 blocking issue(s) (both in the disaster-recovery
runbooks, not in the code itself).