feat(bootstrap): revive the talos phase, rename the mirror patch #710

2026-08-28T14:37:02Z

claude-bot commented

2026-08-28 14:37:02 +00:00

Zwei Commits, die nach dem Merge von #709 entstanden sind und deshalb dort nicht mehr mitkamen.

1. `bootstrap.sh talos` lebt wieder und installiert das CNI

Der Cilium-Install war in #709 ein Shell-Block im Runbook zum Abtippen. Das ist dieselbe Sorte Verrottung wie eine gepinnte Version: niemand führt ihn zwischen zwei Rebuilds aus, also meldet nichts, wenn er nicht mehr funktioniert. Er gehört in das Skript, das den Rest des Bootstraps ohnehin fährt.

Die Wiederbelebung war fast nur Löscharbeit. Die Phase war seit #581 unbrauchbar, weil sie cilium-cni.yaml ganz übergab und dessen $patch: delete gegen eine frische Config scheitert. Die Datei existiert nicht mehr — damit ist der Fehler weg.

Der Install sitzt zwischen talosctl bootstrap und der Ready-Warteschleife, der einzigen möglichen Stelle: mit cni: none bleibt das kubelet NotReady, die Schleife liefe sonst gegen einen Cluster, der nie konvergiert.

Repo-URL, Version und Werte kommen zur Laufzeit aus der HelmRelease, dazu ein Riegel: die HelmRelease ist eine Zwei-Dokument-Datei, und wer das falsche selektiert, bekommt einen leeren Wertesatz, den Helm stillschweigend als Chart-Defaults rendert. Genau das ist mir heute beim Rekonstruieren passiert — der Guard prüft auf policyMapMax und bricht laut ab, statt das in den Datapath durchzureichen.

--wait, weil der nächste Schritt ein Netz braucht; upgrade --install, damit ein Neulauf nach abgebrochenem Bootstrap konvergiert.

2. `registry-mirror.yaml` → `containerd-layer-retention.yaml`

Der Name behauptet, der Bootstrap hänge an einem Registry-Mirror. Wer die Patch-Liste im Runbook liest, schließt das zu Recht — und es wäre ein ernstes Problem, weil ein Cold Rebuild keinen Mirror haben kann: ein in-cluster Mirror existiert nicht, bevor der Cluster existiert.

Die Datei konfiguriert keinen Mirror. Sie setzt eine containerd-Option, discard_unpacked_layers = false, damit Spegel gezogene Layer später an Peers ausliefern kann. Spegel schreibt die hosts.toml selbst zur Laufzeit; im gesamten talos/patches/ gibt es keinen machine.registries-Block. Beim Bootstrap wird direkt von den echten Registries gezogen.

Der Fehlalarm kommt also allein vom Namen — und kostet im Ernstfall, dass jemand einen Rebuild für blockiert hält, der es nicht ist.

Geprüft

bash -n sauber, keine Verweise mehr auf gelöschte Dateien
talosctl gen config mit exakt der Patch-Liste, die das Skript übergibt: cni=none, proxy.disabled=true, 0 Cilium-Manifeste, validiert für metal mode

Ungetestet bleibt die Apply-und-Bootstrap-Hälfte — dafür müsste ein Cluster plattgemacht werden. Das Runbook sagt es ausdrücklich und behält die manuelle Prozedur als das, was man vorher liest und worauf man zurückfällt.

Zwei Commits, die nach dem Merge von #709 entstanden sind und deshalb dort nicht mehr mitkamen. ## 1. `bootstrap.sh talos` lebt wieder und installiert das CNI Der Cilium-Install war in #709 ein Shell-Block im Runbook zum Abtippen. Das ist dieselbe Sorte Verrottung wie eine gepinnte Version: niemand führt ihn zwischen zwei Rebuilds aus, also meldet nichts, wenn er nicht mehr funktioniert. Er gehört in das Skript, das den Rest des Bootstraps ohnehin fährt. Die Wiederbelebung war fast nur Löscharbeit. Die Phase war seit #581 unbrauchbar, weil sie `cilium-cni.yaml` **ganz** übergab und dessen `$patch: delete` gegen eine frische Config scheitert. Die Datei existiert nicht mehr — damit ist der Fehler weg. Der Install sitzt zwischen `talosctl bootstrap` und der Ready-Warteschleife, der einzigen möglichen Stelle: mit `cni: none` bleibt das kubelet `NotReady`, die Schleife liefe sonst gegen einen Cluster, der nie konvergiert. Repo-URL, Version und Werte kommen zur Laufzeit aus der HelmRelease, dazu ein Riegel: die HelmRelease ist eine **Zwei-Dokument-Datei**, und wer das falsche selektiert, bekommt einen leeren Wertesatz, den Helm stillschweigend als Chart-Defaults rendert. Genau das ist mir heute beim Rekonstruieren passiert — der Guard prüft auf `policyMapMax` und bricht laut ab, statt das in den Datapath durchzureichen. `--wait`, weil der nächste Schritt ein Netz braucht; `upgrade --install`, damit ein Neulauf nach abgebrochenem Bootstrap konvergiert. ## 2. `registry-mirror.yaml` → `containerd-layer-retention.yaml` Der Name behauptet, der Bootstrap hänge an einem Registry-Mirror. Wer die Patch-Liste im Runbook liest, schließt das zu Recht — und es wäre ein ernstes Problem, weil ein Cold Rebuild keinen Mirror haben kann: ein in-cluster Mirror existiert nicht, bevor der Cluster existiert. **Die Datei konfiguriert keinen Mirror.** Sie setzt eine containerd-Option, `discard_unpacked_layers = false`, damit Spegel gezogene Layer später an Peers ausliefern kann. Spegel schreibt die `hosts.toml` selbst zur Laufzeit; im gesamten `talos/patches/` gibt es keinen `machine.registries`-Block. Beim Bootstrap wird direkt von den echten Registries gezogen. Der Fehlalarm kommt also allein vom Namen — und kostet im Ernstfall, dass jemand einen Rebuild für blockiert hält, der es nicht ist. ## Geprüft - `bash -n` sauber, keine Verweise mehr auf gelöschte Dateien - `talosctl gen config` mit exakt der Patch-Liste, die das Skript übergibt: `cni=none`, `proxy.disabled=true`, 0 Cilium-Manifeste, validiert für metal mode **Ungetestet bleibt die Apply-und-Bootstrap-Hälfte** — dafür müsste ein Cluster plattgemacht werden. Das Runbook sagt es ausdrücklich und behält die manuelle Prozedur als das, was man vorher liest und worauf man zurückfällt.

claude-bot added 2 commits 2026-08-28 14:37:03 +00:00

feat(bootstrap): revive the talos phase and let it install the CNI cfba60afd0

The Cilium install had become a block of shell in a runbook for a human to
paste. That is the same rot as a pinned version — nobody runs it between
rebuilds, so nothing tells you when it stopped working. It belongs in the script
that already drives the rest of the bootstrap.

The talos phase was unusable since #581: it passed cilium-cni.yaml whole, and
that file's `$patch: delete` document fails against a config with no
inlineManifests. The file is gone, so the failure is gone with it — reviving the
phase was mostly deleting two lines that referenced it.

It now installs Cilium between `talosctl bootstrap` and the node-Ready wait,
which is the only place it can go: with `cni: none` the kubelet stays NotReady
until a CNI exists, so the wait below would otherwise time out against a cluster
that was never going to converge.

Chart repo, version constraint and values are read out of helmrelease.yaml at
run time, so the script cannot install something different from what Flux
reconciles minutes later. helm takes the semver range verbatim. There is a guard
on the extracted values — the HelmRelease is a two-document file and selecting
the wrong one yields an empty values set that helm silently renders as chart
defaults, which is a failure mode worth catching loudly rather than discovering
in a datapath.

`--wait` because the next step needs a working network, and `upgrade --install`
so a re-run after a partial bootstrap converges instead of erroring.

Verified as far as it can be without wiping a cluster: `talosctl gen config`
with exactly the patch list the script now passes produces a config carrying
cni: none and proxy: disabled, no Cilium manifests, and it validates for metal
mode. The apply-and-bootstrap half is untested since the rewrite, and the
runbook says so — it keeps the manual procedure as the thing to read first and
to fall back on.

docs(talos): rename registry-mirror.yaml to what it actually does

ci/woodpecker/pr/ci-scripts-test Pipeline was successful

Details

ci/woodpecker/pr/ci-manifests-test Pipeline was successful

Details

ci/woodpecker/pr/pr-review Pipeline was successful

Details

c92aff9e75

The name says the bootstrap depends on a registry mirror. Reading the patch list
in the runbook, that is the obvious conclusion — and it would be a serious
problem, because a cold rebuild has no mirror: an in-cluster one cannot exist
before the cluster does.

The file configures no mirror. It writes one containerd setting,
discard_unpacked_layers = false, so that pulled layers stay in the content store
and Spegel can advertise them to peers later. Spegel writes the per-registry
hosts.toml itself at runtime; the file's own comment says a
machine.registries.mirrors block is not needed, and there is none anywhere in
talos/patches. Image pulls at bootstrap go straight to the real registries.

So the alarm the name raises is false, but it is the name doing it, and the cost
of that is someone deciding a rebuild is blocked when it is not.
containerd-layer-retention.yaml describes the single thing inside.

Renamed with git mv and the three references updated. Verified `talosctl gen
config` still produces a config with the renamed patch in the list.

review-bot reviewed

2026-08-28 14:39:20 +00:00

review-bot left a comment

🤖 Automated review — model CLI default (complexity: complex), commit c92aff9e75b2d9b6405c21b62729ea0fbbcd7d17.

Advisory review by the review-bot pipeline. It never approves; a human merges. Push new commits to get a fresh review.

Summary

Two independent changes: (1) revives the talos phase of bootstrap.sh by installing Cilium via helm using values/version/repo read straight out of kubernetes/infrastructure/cilium/helmrelease.yaml, and (2) renames talos/patches/registry-mirror.yaml → containerd-layer-retention.yaml to match what the patch actually does, updating all runbook references. I verified both against the checkout: cilium-cni.yaml is indeed gone (the old failure mode is moot), containerd-layer-retention.yaml only sets discard_unpacked_layers = false (no machine.registries block, confirming the old name was misleading), the yq paths (spec.values, spec.chart.spec.url, spec.chart.spec.version) match helmrelease.yaml's actual structure, and no stray references to either old filename remain. No blocking issues found.

🟡 Suggestions

kubernetes/bootstrap.sh:198-211 — CILIUM_VALUES=$(mktemp) is only rm -f'd after a successful helm upgrade --install (line 211). If the policyMapMax guard (line 200-201) or the helm invocation itself fails, fail() calls exit 1 and the temp file is left behind. Low-impact (OS temp cleanup eventually handles it, and the script already exits on failure), but a trap 'rm -f "$CILIUM_VALUES"' EXIT right after the mktemp would make it robust to any exit path.
kubernetes/bootstrap.sh:1-20 — the header usage comment lists Requires: talosctl, kubectl, flux, sops age key at ~/.config/sops/age/keys.txt but doesn't mention helm and yq, which are now hard requirements for the talos phase (checked at bootstrap.sh:92-93). Worth adding for anyone reading the header before running the script.
kubernetes/bootstrap.sh:203-204 — helm repo add cilium <url> followed by helm repo update cilium: if a differently-configured cilium repo entry already exists locally (e.g., from an unrelated prior helm repo add), recent Helm versions no-op repo add for a matching name+URL but this hasn't been exercised per the PR body ("apply-and-bootstrap half... not tested"). Not a defect in the diff itself, just worth keeping in mind the first time this runs for real.

Tests

No automated tests exist for bootstrap.sh (shell orchestration script), consistent with the rest of the file. The PR body documents what was checked: bash -n, and a talosctl gen config + talosctl validate --mode metal dry run with the new patch list — both reasonable substitutes for what can be verified without destroying a cluster. The apply-and-bootstrap path (the new helm upgrade --install cilium step, in particular) is explicitly called out as unverified since it requires wiping a real cluster; that's an honest and appropriately-scoped limitation for a homelab bootstrap script, not a gap I'd block on.

Recommendation

Looks reasonable — no blocking issues; comments above are optional.

🤖 **Automated review** — model `CLI default` (complexity: complex), commit `c92aff9e75b2d9b6405c21b62729ea0fbbcd7d17`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary Two independent changes: (1) revives the `talos` phase of `bootstrap.sh` by installing Cilium via `helm` using values/version/repo read straight out of `kubernetes/infrastructure/cilium/helmrelease.yaml`, and (2) renames `talos/patches/registry-mirror.yaml` → `containerd-layer-retention.yaml` to match what the patch actually does, updating all runbook references. I verified both against the checkout: `cilium-cni.yaml` is indeed gone (the old failure mode is moot), `containerd-layer-retention.yaml` only sets `discard_unpacked_layers = false` (no `machine.registries` block, confirming the old name was misleading), the yq paths (`spec.values`, `spec.chart.spec.url`, `spec.chart.spec.version`) match `helmrelease.yaml`'s actual structure, and no stray references to either old filename remain. No blocking issues found. ## 🟡 Suggestions - `kubernetes/bootstrap.sh:198-211` — `CILIUM_VALUES=$(mktemp)` is only `rm -f`'d after a successful `helm upgrade --install` (line 211). If the `policyMapMax` guard (line 200-201) or the `helm` invocation itself fails, `fail()` calls `exit 1` and the temp file is left behind. Low-impact (OS temp cleanup eventually handles it, and the script already exits on failure), but a `trap 'rm -f "$CILIUM_VALUES"' EXIT` right after the `mktemp` would make it robust to any exit path. - `kubernetes/bootstrap.sh:1-20` — the header usage comment lists `Requires: talosctl, kubectl, flux, sops age key at ~/.config/sops/age/keys.txt` but doesn't mention `helm` and `yq`, which are now hard requirements for the `talos` phase (checked at bootstrap.sh:92-93). Worth adding for anyone reading the header before running the script. - `kubernetes/bootstrap.sh:203-204` — `helm repo add cilium <url>` followed by `helm repo update cilium`: if a differently-configured `cilium` repo entry already exists locally (e.g., from an unrelated prior `helm repo add`), recent Helm versions no-op `repo add` for a matching name+URL but this hasn't been exercised per the PR body ("apply-and-bootstrap half... not tested"). Not a defect in the diff itself, just worth keeping in mind the first time this runs for real. ## Tests No automated tests exist for `bootstrap.sh` (shell orchestration script), consistent with the rest of the file. The PR body documents what was checked: `bash -n`, and a `talosctl gen config` + `talosctl validate --mode metal` dry run with the new patch list — both reasonable substitutes for what can be verified without destroying a cluster. The apply-and-bootstrap path (the new `helm upgrade --install cilium` step, in particular) is explicitly called out as unverified since it requires wiping a real cluster; that's an honest and appropriately-scoped limitation for a homelab bootstrap script, not a gap I'd block on. ## Recommendation Looks reasonable — no blocking issues; comments above are optional.

k.renner merged commit 5c6d59b0b4 into master

2026-08-28 14:41:13 +00:00

k.renner referenced this issue from a commit

2026-08-28 14:41:13 +00:00

Merge pull request 'feat(bootstrap): revive the talos phase, rename the mirror patch' (#710) from cilium-flux-only into master

claude-bot referenced this pull request

2026-08-28 19:27:02 +00:00

fix(bootstrap): der Serial-Lookup hat im Maintenance Mode nie funktioniert #711

Sign in to join this conversation.