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

2026-08-28T19:27:02Z

claude-bot commented

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

Die drei Commits aus #710, die es nicht mehr in den Merge geschafft haben, plus ein Fehler, den ein QEMU-Testlauf danach aufgedeckt hat.

Der Fehler

--insecure ist ein Flag von get, kein globales. Es stand vor dem Subcommand:

talosctl -n "$addr" --insecure get systeminformation
→ unknown command "systeminformation" for "talosctl"

talosctl bricht ab, ohne den Node je zu kontaktieren. Das ist der Maintenance-Mode-Zweig — und Maintenance Mode ist der einzige Zustand, in dem eine Maschine beim Cold Rebuild ist. Er schlug immer fehl und fiel auf den PKI-Zweig zurück, der auf einer Maschine ohne Config per Definition nicht funktionieren kann. Die Erkennung konnte also ausschließlich bereits konfigurierte Nodes identifizieren.

Warum das durch den Review kam: verifiziert wurde gegen die drei laufenden Nodes. Dort antwortet der Fallback und verdeckt, dass der erste Zweig tot ist.

Gegenprobe am laufenden Node:

falsche Reihenfolge → unknown command "systeminformation"
richtige Reihenfolge → tls: certificate required   (erreicht den Node, korrekt abgewiesen)

Was den Fehler unsichtbar gehalten hat

2>/dev/null auf beiden Versuchen hat genau die Zeile weggeworfen, die sagt was los ist. Der stderr wird jetzt gesammelt und nur ausgegeben, wenn keiner der beiden Zweige eine Seriennummer liefert — einer von beiden soll ja fehlschlagen. serialnumber is not found statt eines pauschalen „could not read a chassis serial from 10.0.1.5" ist der Unterschied zwischen Diagnose und Raten.

patch=$(grep ...) hatte kein || true. grep endet mit 1, wenn kein Patch die Seriennummer deklariert, und unter set -e beendet eine fehlschlagende Kommandosubstitution das Skript — die Meldung, die genau für diesen Fall geschrieben wurde, war unerreichbar.

Dieselbe Form steckte in der ersten Selftest-Assertion: kubectl … | grep -q ' Ready' liest ein kaputtes kubectl als „kein Ready-Node gefunden" und meldet „confirmed NotReady", ohne irgendetwas geprüft zu haben. Die Node-Liste wird jetzt zuerst geholt, und ihr Fehlen ist ein Fehler.

Was der QEMU-Lauf sonst gezeigt hat

Das echte Factory-Image (unser Schematic, v1.13.9) bootet unter dem QEMU-Provisioner durch, und install_cilium bringt daraus einen Ready-Node mit KubeProxyReplacement: True und Routing: Tunnel [vxlan] — derselbe Datapath wie in Produktion, Kubernetes 1.36.3.

Zwei harte Grenzen dieser Umgebung, damit sie niemand mitten in einer Probe entdeckt:

VMs melden keine Chassis-Seriennummer. systeminformation hat auf QEMU kein serialnumber-Feld. patch_for_machine ist dort prinzipiell nicht testbar.
Zusatzplatten gehen nur an Worker. Laut --disks-Hilfe: disks after the first one are added only to worker machines. Ein reiner Control-Plane-Cluster bekommt keine OSD-Disks, Ceph braucht also zusätzliche Worker-VMs.

Die drei Commits aus #710, die es nicht mehr in den Merge geschafft haben, plus ein Fehler, den ein QEMU-Testlauf danach aufgedeckt hat. ## Der Fehler `--insecure` ist ein Flag von `get`, kein globales. Es stand **vor** dem Subcommand: ``` talosctl -n "$addr" --insecure get systeminformation → unknown command "systeminformation" for "talosctl" ``` talosctl bricht ab, ohne den Node je zu kontaktieren. Das ist der Maintenance-Mode-Zweig — und Maintenance Mode ist der einzige Zustand, in dem eine Maschine beim Cold Rebuild ist. Er schlug immer fehl und fiel auf den PKI-Zweig zurück, der auf einer Maschine ohne Config per Definition nicht funktionieren kann. Die Erkennung konnte also ausschließlich bereits konfigurierte Nodes identifizieren. Warum das durch den Review kam: verifiziert wurde gegen die drei laufenden Nodes. Dort antwortet der Fallback und verdeckt, dass der erste Zweig tot ist. Gegenprobe am laufenden Node: ``` falsche Reihenfolge → unknown command "systeminformation" richtige Reihenfolge → tls: certificate required (erreicht den Node, korrekt abgewiesen) ``` ## Was den Fehler unsichtbar gehalten hat `2>/dev/null` auf beiden Versuchen hat genau die Zeile weggeworfen, die sagt was los ist. Der stderr wird jetzt gesammelt und nur ausgegeben, wenn *keiner* der beiden Zweige eine Seriennummer liefert — einer von beiden soll ja fehlschlagen. `serialnumber is not found` statt eines pauschalen „could not read a chassis serial from 10.0.1.5" ist der Unterschied zwischen Diagnose und Raten. `patch=$(grep ...)` hatte kein `|| true`. grep endet mit 1, wenn kein Patch die Seriennummer deklariert, und unter `set -e` beendet eine fehlschlagende Kommandosubstitution das Skript — die Meldung, die genau für diesen Fall geschrieben wurde, war unerreichbar. Dieselbe Form steckte in der ersten Selftest-Assertion: `kubectl … | grep -q ' Ready'` liest ein kaputtes kubectl als „kein Ready-Node gefunden" und meldet „confirmed NotReady", ohne irgendetwas geprüft zu haben. Die Node-Liste wird jetzt zuerst geholt, und ihr Fehlen ist ein Fehler. ## Was der QEMU-Lauf sonst gezeigt hat Das echte Factory-Image (unser Schematic, v1.13.9) bootet unter dem QEMU-Provisioner durch, und `install_cilium` bringt daraus einen `Ready`-Node mit `KubeProxyReplacement: True` und `Routing: Tunnel [vxlan]` — derselbe Datapath wie in Produktion, Kubernetes 1.36.3. Zwei harte Grenzen dieser Umgebung, damit sie niemand mitten in einer Probe entdeckt: - **VMs melden keine Chassis-Seriennummer.** `systeminformation` hat auf QEMU kein `serialnumber`-Feld. `patch_for_machine` ist dort prinzipiell nicht testbar. - **Zusatzplatten gehen nur an Worker.** Laut `--disks`-Hilfe: *disks after the first one are added only to worker machines*. Ein reiner Control-Plane-Cluster bekommt keine OSD-Disks, Ceph braucht also zusätzliche Worker-VMs.

claude-bot added 5 commits 2026-08-28 19:27:02 +00:00

feat(bootstrap): selftest phase that proves the CNI bootstrap path 82c1e5eaba

The cold-rebuild path had no test, which is why it rotted: `bootstrap.sh talos`
was broken for months and nothing said so, and #709 deleted the only file
setting `cni: none` while every check against the live cluster still passed —
because that cluster has had the setting since it was built.

`bootstrap.sh selftest` builds a throwaway single-node Talos cluster in Docker
and runs the real thing against it:

  - derives the CNI settings and pod/service subnets from cluster.yaml, so
    dropping `cni: none` there fails the test rather than a rebuild
  - asserts the node is NotReady first, otherwise the test proves nothing
  - calls install_cilium — the same function the talos phase uses, not a copy
  - asserts the node reaches Ready AND the agent reports 1/1

Both assertions, because a node can go Ready while the agent is still 0/1;
checking only the node would pass on a half-initialised datapath.

The pod subnets have to come along. The HelmRelease sets ipv6.enabled, and
without a v6 PodCIDR the agent sits forever on "required IPv6 PodCIDR not
available" — a red test caused by the scratch cluster rather than the code. That
is exactly what happened on the first run.

No flag is needed to stop `cluster create` waiting for readiness: Talos sees
`cni: none` and skips the node, kube-proxy and coredns checks itself. The SKIP
lines it prints are themselves evidence the patch landed.

Isolated with TALOSCONFIG, KUBECONFIG and --state pointed at a temp dir, and
torn down from an EXIT trap. Verified afterwards that no container or network
survived and that the production context is still admin@homelab — a test that
can reach the real cluster is worse than no test.

Ran end to end: exit 0, node Ready, agent 1/1, Cilium 1.20.1 from the
HelmRelease, cleanup complete.

What it does not cover, and cannot: the per-node patches (disk serials, VLAN
parents, the VIP), Ceph, OpenBao, the full Flux reconcile. Those need the real
machines. It covers the part that changed and that nothing else exercises.

feat(talos): bind the per-node patches to the chassis serial 5157aec9a5

A per-node patch carries that node's addresses, VLAN parents and disk serials.
Putting one on the wrong machine is the expensive mistake in a rebuild, and the
current defence against it is an operator matching DHCP leases to hostnames by
hand — at the one moment when the machines have no hostnames, because a node in
maintenance mode sits on whatever untagged lease it picked up.

Each patch now declares the chassis it belongs to:

    # talos-chassis-serial: MC016US129EDMHE00039

read from `talosctl get systeminformation`, which answers in maintenance mode
over --insecure, before any config exists. bootstrap.sh asks the machine which
patch is its own instead of trusting that a name resolved to the box we meant,
and refuses to apply a patch whose serial does not match what the machine
reports.

The disk selectors were already a partial defence: they name drive serials, so
the wrong patch fails to match a disk rather than installing somewhere
unintended. That turns a mistake into a failed install. This turns it into a
refusal before anything is written, and — more usefully — removes the guess
entirely, because the machine identifies itself.

Serials, verified against all three:

    MC016US129EDMHE00039  k8s-1
    MF116VS129EDMHE00013  k8s-2
    MC016US129EDMHE00006  k8s-3

Tested by running the lookup against each live node: every one resolves to its
own file. The markers are comments, so `talosctl gen config` is unaffected —
confirmed by regenerating with k8s-2.yaml.

Not addressed here, but this is what makes it fixable: apply_node_config still
finds machines by resolving k8s-N.srv.renner.uno, which cannot work during a
cold rebuild — the runbook says to read the leases off the UDM for exactly that
reason. With identity coming from the hardware, that loop can take addresses
instead of names. Left as its own change so this one stays reviewable.

feat(bootstrap): let each machine choose its own config ac938e89d5

The apply loop iterated over node names, resolved k8s-N.srv.renner.uno, and
applied k8s-N.yaml to whatever answered. That is backwards for the case it
exists to serve: during a cold rebuild the machines have no hostnames and no DNS
records — they sit on whatever untagged lease they picked up, which is why the
runbook says to read the leases off the UDM. Resolving those names at that point
resolves nothing, so the phase could not have worked no matter what else was
fixed.

Now each address is asked who it is, and the patch declaring that chassis serial
is what gets applied. A machine whose serial no patch declares is an error
naming the serial, not something to improvise around.

Addresses come from NODE_ADDRS for a rebuild:

    NODE_ADDRS="10.0.1.5 10.0.1.9 10.0.1.14" ./bootstrap.sh talos

and default to the FQDNs, which is the right input when the cluster already
exists and a node is being re-applied. The default prints a warning that it
cannot work on a cold rebuild, rather than failing obscurely later.

Two things this rework broke and that are fixed here rather than found in
production:

The serial lookup ran before the retry loop, so a machine still booting into
maintenance mode failed immediately where the old code retried for ten minutes.
Identification is now retried, and only a machine that answers with an unknown
serial is a real error. patch_for_machine runs in a subshell there, so its
internal fail cannot end the script — the loop re-runs it bare on the last
attempt to surface the actual message, and there is an explicit guard so
apply-config can never be reached with an empty --config-patch.

Dropping the loop over WORKERS would have silently applied controlplane.yaml to
any future worker. WORKERS has been empty since k8s-edge was retired, so the
loop was dead, but the assumption was load-bearing: the serial says which
machine a patch belongs to, not which base config it wants. The script now
refuses to run if WORKERS is ever non-empty.

resolve_node_ip and DNS_SERVER go with the old approach; nothing else used them.

Verified: lookup resolves each of the three live nodes to its own patch, the
selftest still passes end to end (node Ready, agent 1/1), and nothing was left
running or pointed at the production context afterwards.

feat(bootstrap): make the flux branch configurable so the phase can be rehearsed 1d01688fd8

Both bootstrap calls hardcoded --branch=master, which is why the flux phase
could never be tried before it was needed: `flux bootstrap` commits its own sync
manifests into the branch it reconciles, so running it wrote to the source a
rebuild depends on.

FLUX_BRANCH defaults to master and can be pointed at a throwaway branch:

    FLUX_BRANCH=rehearsal-$(date +%s) ./bootstrap.sh flux

Nothing else about the phase changes, which is the point — a rehearsal that
takes a different code path proves less than one that takes the same one.

This is the only part of the break-glass path that can be exercised in advance.
The waits after it — infrastructure, Ceph, the object store — need real disks,
and no branch makes them work on a scratch cluster. The comment says so rather
than leaving someone to discover it mid-rehearsal.

Also moves install_cilium's doc comment back above install_cilium. Inserting
patch_for_machine ahead of it left the comment describing the wrong function.

Selftest re-run after the change: still green, node Ready and agent 1/1.

fix(bootstrap): the serial lookup never worked in maintenance mode

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

2a40ba61a7

--insecure is a flag of `get`, not a global one. It sat before the subcommand,
so talosctl exited with `unknown command "systeminformation"` without ever
contacting the node.

That is the maintenance-mode branch, and maintenance mode is the only state a
machine is in during a cold rebuild. It failed every time and fell through to
the PKI branch, which by definition cannot work on a machine that has no config
yet. The lookup could only ever have identified an already-configured node.

It passed review because it was verified against the three running nodes, where
the fallback answers and hides that the first branch is dead.

Two things kept that invisible and are fixed with it:

2>/dev/null on both attempts threw away the one line that says what went wrong.
The stderr is now collected and printed only if neither branch produces a
serial: exactly one of them is meant to fail on any given machine, so the noise
is held back rather than discarded. Reading "serialnumber is not found" instead
of a generic "could not read a chassis serial from 10.0.1.5" is the difference
between a diagnosis and a guess.

patch=$(grep ...) had no `|| true`. grep exits 1 when no patch declares the
serial, and under set -e a failing command substitution ends the script — so the
message written for that exact case was unreachable.

Also fixes the selftest's first assertion, which had the same shape: piping
kubectl into `grep -q ' Ready'` reads a broken kubectl as "no Ready node found"
and prints "confirmed NotReady" having checked nothing. The node list is now
fetched first and its absence is a failure.

Found by running the QEMU provisioner against the real factory image. Virtual
machines report no chassis serial at all, which is why patch_for_machine cannot
be exercised there — but reaching that error is what surfaced the flag order.
Verified against the running node: the old order returns `unknown command`, the
new one reaches the node and is refused for the right reason.

review-bot reviewed

2026-08-28 19:32:30 +00:00

review-bot left a comment

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

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

Summary

This PR fixes the maintenance-mode chassis-serial lookup in kubernetes/bootstrap.sh (the --insecure flag was placed before the get subcommand instead of after, so talosctl rejected the command before ever contacting the node), adds three related robustness fixes (unswallowed grep/command-substitution failures under set -e, a docker-based selftest phase that exercises the Cilium CNI bootstrap path end-to-end, and a FLUX_BRANCH override for rehearsing the Flux phase), and tags the three per-node Talos patches with their chassis serials. The core fix is correct and well-verified (the PR body documents the QEMU repro), and the surrounding comments are unusually thorough. No blocking issues in the changed code; a couple of edge cases in the new retry loop and a stale doc are worth a look.

🟡 Suggestions

kubernetes/bootstrap.sh:290-303 (apply_node_config retry loop) — a late-succeeding "surface the real error" retry can produce a false failure. On the 60th iteration, if the silent probe (node_patch=$(patch_for_machine "$addr" 2>/dev/null), line 290) fails but the machine becomes reachable in the 10s before the follow-up bare call patch_for_machine "$addr" at line 297 runs, that bare call succeeds and prints identifies as ... — but its result is never captured into node_patch (the call isn't assigned). The loop then falls through to [[ -n "$node_patch" ]] || fail "never identified the machine at $addr" (line 302), which still fails because node_patch holds the previous (empty) value. The script aborts right after printing what looks like a success line, even though the node just answered correctly. This is a narrow timing window (it only exists on the last iteration, coinciding with the 10s sleep), and re-running the script recovers, but it directly contradicts the "belt and braces" comment's intent above it.
Same loop — a reachable machine with an unrecognized serial is indistinguishable from an unreachable one for up to 10 minutes. patch_for_machine returns non-zero for both "could not read a chassis serial from $addr" (unreachable) and "$addr reports chassis serial '$serial', which no patch ... declares" (reachable, but no matching patch) — line 137 vs. 146. Since the silent probe discards stderr and only checks the exit status, apply_node_config treats both as "no answer yet" (line 295) and keeps retrying for the full 60 attempts (~10 minutes) before the final loud call finally surfaces the real "no patch declares this serial" message. The comment above apply_node_config (lines 265-269) states the goal is exactly to not conflate these two cases ("an unreachable node must not be mistaken for one whose serial is unknown"), but the implementation doesn't actually distinguish them until the last retry.
kubernetes/RUNBOOK-cold-rebuild.md:133 still shows the exact flag order this PR identifies as broken: talosctl -n <maintenance-ip> --insecure get disks (--insecure before the subcommand). Per this PR's own diagnosis, that ordering fails with unknown command "disks" without ever contacting the node. The manual procedure in this runbook is explicitly meant to be read "before running the script" and used as a fallback ("can take over at any step if it stops") for the very cold-rebuild scenario this PR is about, so an operator following it verbatim would hit the same dead branch. Contrast with kubernetes/RUNBOOK-baremetal-conversion.md:185 (talosctl -n <maintenance-ip> get disks --insecure), which already has the correct order — the cold-rebuild runbook is just stale relative to it.
install_cilium (kubernetes/bootstrap.sh:151-183) dropped the original comment's note that the Cilium install "must run before the node-Ready wait below" (it timed out otherwise, since kubelet stays NotReady with no CNI). The call order is unchanged in both the talos and selftest phases, so there's no functional regression, but the rationale for why the ordering matters no longer lives next to either call site.
Selftest preflight (kubernetes/bootstrap.sh:202-204) checks for docker, helm, yq but not talosctl/kubectl, and doesn't check [[ -f "$CILIUM_HR" ]] the way the talos phase does (line 298). Missing tools/files will still fail loudly via set -e and the EXIT trap still cleans up, so this is cosmetic, but the talos phase's more complete preflight would be a nice match.

🟢 Nits

kubernetes/bootstrap.sh:144 — grep -l "^# talos-chassis-serial: ${serial}\$" ...: $serial is interpolated unescaped into a grep pattern. Not exploitable and not a real risk given real chassis serials are alphanumeric, but a serial containing a regex metacharacter would make the match looser than intended.

Tests

No automated test framework covers this file; the new selftest phase (lines 200-288) is itself the testing mechanism, and it's a genuine improvement — it exercises install_cilium against a throwaway Docker Talos cluster instead of relying on production nodes that already have a CNI (which is exactly how the --insecure ordering bug and the historical cni: none regression, #709, went unnoticed). It's opt-in (needs docker, not part of all), which is reasonable for a bootstrap script. The PR body documents manual QEMU verification of the actual fix, which is the appropriate level of rigor for this kind of infrastructure script.

Recommendation

Looks reasonable — no blocking issues; comments above are optional (though the stale RUNBOOK-cold-rebuild.md:133 command is worth fixing while this is fresh).

🤖 **Automated review** — model `CLI default` (complexity: complex), commit `2a40ba61a72f57914048367b378c33abf025c2f9`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary This PR fixes the maintenance-mode chassis-serial lookup in `kubernetes/bootstrap.sh` (the `--insecure` flag was placed before the `get` subcommand instead of after, so `talosctl` rejected the command before ever contacting the node), adds three related robustness fixes (unswallowed `grep`/command-substitution failures under `set -e`, a `docker`-based `selftest` phase that exercises the Cilium CNI bootstrap path end-to-end, and a `FLUX_BRANCH` override for rehearsing the Flux phase), and tags the three per-node Talos patches with their chassis serials. The core fix is correct and well-verified (the PR body documents the QEMU repro), and the surrounding comments are unusually thorough. No blocking issues in the changed code; a couple of edge cases in the new retry loop and a stale doc are worth a look. ## 🟡 Suggestions - **`kubernetes/bootstrap.sh:290-303` (`apply_node_config` retry loop) — a late-succeeding "surface the real error" retry can produce a false failure.** On the 60th iteration, if the silent probe (`node_patch=$(patch_for_machine "$addr" 2>/dev/null)`, line 290) fails but the machine becomes reachable in the 10s before the follow-up bare call `patch_for_machine "$addr"` at line 297 runs, that bare call succeeds and prints `identifies as ...` — but its result is never captured into `node_patch` (the call isn't assigned). The loop then falls through to `[[ -n "$node_patch" ]] || fail "never identified the machine at $addr"` (line 302), which still fails because `node_patch` holds the previous (empty) value. The script aborts right after printing what looks like a success line, even though the node just answered correctly. This is a narrow timing window (it only exists on the last iteration, coinciding with the 10s sleep), and re-running the script recovers, but it directly contradicts the "belt and braces" comment's intent above it. - **Same loop — a reachable machine with an unrecognized serial is indistinguishable from an unreachable one for up to 10 minutes.** `patch_for_machine` returns non-zero for both "could not read a chassis serial from $addr" (unreachable) and "$addr reports chassis serial '$serial', which no patch ... declares" (reachable, but no matching patch) — line 137 vs. 146. Since the silent probe discards stderr and only checks the exit status, `apply_node_config` treats both as "no answer yet" (line 295) and keeps retrying for the full 60 attempts (~10 minutes) before the final loud call finally surfaces the real "no patch declares this serial" message. The comment above `apply_node_config` (lines 265-269) states the goal is exactly to not conflate these two cases ("an unreachable node must not be mistaken for one whose serial is unknown"), but the implementation doesn't actually distinguish them until the last retry. - **`kubernetes/RUNBOOK-cold-rebuild.md:133`** still shows the exact flag order this PR identifies as broken: `` talosctl -n <maintenance-ip> --insecure get disks `` (`--insecure` before the subcommand). Per this PR's own diagnosis, that ordering fails with `unknown command "disks"` without ever contacting the node. The manual procedure in this runbook is explicitly meant to be read "before running the script" and used as a fallback ("can take over at any step if it stops") for the very cold-rebuild scenario this PR is about, so an operator following it verbatim would hit the same dead branch. Contrast with `kubernetes/RUNBOOK-baremetal-conversion.md:185` (`talosctl -n <maintenance-ip> get disks --insecure`), which already has the correct order — the cold-rebuild runbook is just stale relative to it. - **`install_cilium` (`kubernetes/bootstrap.sh:151-183`)** dropped the original comment's note that the Cilium install "must run before the node-Ready wait below" (it timed out otherwise, since kubelet stays NotReady with no CNI). The call order is unchanged in both the `talos` and `selftest` phases, so there's no functional regression, but the rationale for *why* the ordering matters no longer lives next to either call site. - **Selftest preflight (`kubernetes/bootstrap.sh:202-204`)** checks for `docker`, `helm`, `yq` but not `talosctl`/`kubectl`, and doesn't check `[[ -f "$CILIUM_HR" ]]` the way the `talos` phase does (line 298). Missing tools/files will still fail loudly via `set -e` and the `EXIT` trap still cleans up, so this is cosmetic, but the `talos` phase's more complete preflight would be a nice match. ## 🟢 Nits - `kubernetes/bootstrap.sh:144` — `grep -l "^# talos-chassis-serial: ${serial}\$" ...`: `$serial` is interpolated unescaped into a grep pattern. Not exploitable and not a real risk given real chassis serials are alphanumeric, but a serial containing a regex metacharacter would make the match looser than intended. ## Tests No automated test framework covers this file; the new `selftest` phase (lines 200-288) is itself the testing mechanism, and it's a genuine improvement — it exercises `install_cilium` against a throwaway Docker Talos cluster instead of relying on production nodes that already have a CNI (which is exactly how the `--insecure` ordering bug and the historical `cni: none` regression, #709, went unnoticed). It's opt-in (needs `docker`, not part of `all`), which is reasonable for a bootstrap script. The PR body documents manual QEMU verification of the actual fix, which is the appropriate level of rigor for this kind of infrastructure script. ## Recommendation Looks reasonable — no blocking issues; comments above are optional (though the stale `RUNBOOK-cold-rebuild.md:133` command is worth fixing while this is fresh).

claude-bot commented

2026-08-28 19:33:06 +00:00

Nachgereicht: der Fix ist jetzt gegen echten Maintenance Mode verifiziert, nicht nur gegen einen konfigurierten Node.

Dafür einen QEMU-Cluster mit --presets iso,maintenance erzeugt — die Maschine bekommt keine Config, also genau der Zustand beim Cold Rebuild. Beide Reihenfolgen direkt nebeneinander gegen diesen Node:

talosctl -n 10.5.0.2 get systeminformation --insecure   → INSECURE_GET_EXIT=0, Resource kommt zurück
talosctl -n 10.5.0.2 --insecure get systeminformation   → unknown command "systeminformation", Exit 1

Der Maintenance-Dienst beantwortet --insecure also. Der alte Code konnte ihn nicht erreichen, und zwar nicht aus Netzwerkgründen, sondern weil talosctl schon beim Parsen abbrach — der Fehler war unabhängig von der Erreichbarkeit des Nodes.

Damit ist die Behauptung im Commit belegt: der Zweig, auf den ein Cold Rebuild ausschließlich angewiesen ist, war tot.

Bestätigt sich dabei ein zweites Mal: die Ausgabe enthält kein serialnumber-Feld. VMs melden keine Chassis-Seriennummer, patch_for_machine bleibt auf QEMU als Ganzes nicht testbar — geprüft wurde hier ausschließlich der Transportweg.

Nachgereicht: der Fix ist jetzt gegen echten Maintenance Mode verifiziert, nicht nur gegen einen konfigurierten Node. Dafür einen QEMU-Cluster mit `--presets iso,maintenance` erzeugt — die Maschine bekommt keine Config, also genau der Zustand beim Cold Rebuild. Beide Reihenfolgen direkt nebeneinander gegen diesen Node: ``` talosctl -n 10.5.0.2 get systeminformation --insecure → INSECURE_GET_EXIT=0, Resource kommt zurück talosctl -n 10.5.0.2 --insecure get systeminformation → unknown command "systeminformation", Exit 1 ``` Der Maintenance-Dienst beantwortet `--insecure` also. Der alte Code konnte ihn nicht erreichen, und zwar nicht aus Netzwerkgründen, sondern weil talosctl schon beim Parsen abbrach — der Fehler war unabhängig von der Erreichbarkeit des Nodes. Damit ist die Behauptung im Commit belegt: der Zweig, auf den ein Cold Rebuild ausschließlich angewiesen ist, war tot. Bestätigt sich dabei ein zweites Mal: die Ausgabe enthält kein `serialnumber`-Feld. VMs melden keine Chassis-Seriennummer, `patch_for_machine` bleibt auf QEMU als Ganzes nicht testbar — geprüft wurde hier ausschließlich der Transportweg.

claude-bot added 1 commit 2026-08-28 19:38:54 +00:00

feat(bootstrap): let a rehearsal keep its hands out of $HOME

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

524c26c804

FLUX_BRANCH was supposed to make the flux phase rehearsable, and it only got
half way. It made the git side safe -- a throwaway branch instead of master --
while the phase still ran load_staged_access, which copies the staged files over
~/.talos/config and ~/.kube/config unconditionally. Rehearsing against a scratch
cluster would have replaced the operator's production access with a QEMU VM's,
which is a worse outcome than the one FLUX_BRANCH was written to prevent.

The six places that wrote into $HOME now write to TALOSCONFIG_PATH and
KUBECONFIG_PATH, derived from the env vars talosctl and kubectl already read.
Unset, they resolve to the same two paths as before, so a real rebuild takes an
unchanged path -- verified that neither var is set in the normal environment.
Set, the whole phase runs against throwaway files:

    TALOSCONFIG=/tmp/rehearsal/talosconfig \
    KUBECONFIG=/tmp/rehearsal/kubeconfig \
    FLUX_BRANCH=rehearsal-1 ./bootstrap.sh flux

This is deliberately the same shape as FLUX_BRANCH: one variable, identical code
path. A rehearsal that runs different code proves less than one that does not.

KUBECONFIG is allowed to hold a colon-separated list, and this script writes to
it. There is no sensible way to write to a list, so it is rejected with a message
rather than silently clobbering the first entry.

Verified by extracting load_staged_access from this file and running it with both
vars pointed at scratch paths: the staged markers land in the scratch files, and
~/.kube/config and ~/.talos/config are byte-identical by sha256 before and after.

The rehearsal itself has not been run yet -- it additionally needs GITEA_TOKEN,
and `flux bootstrap` writes a branch and a deploy key into the real repository,
which is not something to do as a side effect of a code change.

review-bot reviewed

2026-08-28 19:44:20 +00:00

review-bot left a comment

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

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

Summary

This PR fixes the maintenance-mode serial lookup in kubernetes/bootstrap.sh (a misplaced --insecure flag that made the cold-rebuild node-identification path always fail and silently fall back to a path that can't work on an unconfigured machine), plus two related set -e/pipeline swallowing bugs uncovered by an actual QEMU test run, and adds a selftest phase that proves the Cilium CNI install path against a throwaway Docker cluster. It also makes the talosconfig/kubeconfig write locations overridable so the flux phase can be rehearsed without touching $HOME. The fix for the headline bug is correct and well-reasoned; I found no blocking defects, only a few small follow-ups worth a look.

🟡 Suggestions

kubernetes/RUNBOOK-cold-rebuild.md:133 — talosctl -n <maintenance-ip> --insecure get disks places --insecure before the get subcommand, the exact ordering this PR identifies as broken for get systeminformation (--insecure is a flag of get, not global). If that diagnosis is right, this runbook line has the same bug and silently fails the same way on a real cold rebuild. Worth a quick check/fix while the failure mode is fresh, even though this file isn't part of the diff.
kubernetes/bootstrap.sh:131-167 (patch_for_machine) docstring says the lookup "works equally on a configured node, which is why the same path serves a rebuild and a single node being re-added." That's true for the identify step (it has a non---insecure fallback), but the subsequent apply step at kubernetes/bootstrap.sh:376 still hardcodes talosctl apply-config --insecure. --insecure talks to Talos's maintenance-mode listener, which an already-configured node doesn't expose (consistent with the runbook's own stage-2 example at line 202, which drops --insecure and uses --mode=try instead). If so, re-adding an already-configured single node via apply_node_config would identify correctly but then spin through all 60 apply attempts and fail with "Timeout applying to $addr". This predates the diff (the old DNS-based path had the same hardcoded --insecure), so it's not a regression, but the new docstring's claim is broader than what the code actually delivers — worth either scoping the comment down or handling the configured-node apply case for real.
kubernetes/bootstrap.sh:112 — fail() writes its message with a plain echo (stdout), not >&2. In patch_for_machine, calls like node_patch=$(patch_for_machine "$addr" 2>/dev/null) only redirect stderr, so a fail() invoked inside is captured into node_patch as text instead of being discarded. It's harmless today only because fail() always exits non-zero, which the caller's if node_patch=$(...) && [[ -n "$node_patch" ]] checks before ever using $node_patch's content — but it's a fragile invariant to lean on silently. Sending fail()'s output to stderr would remove the dependency.

🟢 Nits

kubernetes/bootstrap.sh:337-339 — mkdir -p ~/.talos is left over from before TALOSCONFIG_PATH was introduced; the very next line already does mkdir -p "$(dirname "$TALOSCONFIG_PATH")", which is the directory that's actually used. The old line is now dead weight and, worse, hardcodes $HOME in a section whose whole point is to stop touching it unconditionally.

Tests

The PR's own test is the new selftest phase, and it's a solid regression guard for the CNI bootstrap contract (asserts NotReady pre-Cilium, Ready + agent 1/1 post-Cilium, and that cluster.yaml still declares cni: none/proxy.disabled). It isn't wired into any Woodpecker pipeline (checked .woodpecker/*.yml — nothing invokes bootstrap.sh), which matches the PR's framing of it as a manual pre-flight check requiring Docker; that's a reasonable tradeoff for infra code that can't run in ordinary CI. The actual headline fix (--insecure ordering in patch_for_machine) has no automated coverage — understandably, since it requires a real/maintenance-mode-capable node — but the PR body documents a manual QEMU repro (unknown command vs. tls: certificate required) that's convincing evidence the fix is correct.

Recommendation

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

🤖 **Automated review** — model `CLI default` (complexity: complex), commit `524c26c8047cd52f86617dce4d106e5532a1c2ae`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary This PR fixes the maintenance-mode serial lookup in `kubernetes/bootstrap.sh` (a misplaced `--insecure` flag that made the cold-rebuild node-identification path always fail and silently fall back to a path that can't work on an unconfigured machine), plus two related `set -e`/pipeline swallowing bugs uncovered by an actual QEMU test run, and adds a `selftest` phase that proves the Cilium CNI install path against a throwaway Docker cluster. It also makes the talosconfig/kubeconfig write locations overridable so the `flux` phase can be rehearsed without touching `$HOME`. The fix for the headline bug is correct and well-reasoned; I found no blocking defects, only a few small follow-ups worth a look. ## 🟡 Suggestions - `kubernetes/RUNBOOK-cold-rebuild.md:133` — `talosctl -n <maintenance-ip> --insecure get disks` places `--insecure` *before* the `get` subcommand, the exact ordering this PR identifies as broken for `get systeminformation` (`--insecure` is a flag of `get`, not global). If that diagnosis is right, this runbook line has the same bug and silently fails the same way on a real cold rebuild. Worth a quick check/fix while the failure mode is fresh, even though this file isn't part of the diff. - `kubernetes/bootstrap.sh:131-167` (`patch_for_machine`) docstring says the lookup "works equally on a configured node, which is why the same path serves a rebuild and a single node being re-added." That's true for the identify step (it has a non-`--insecure` fallback), but the subsequent apply step at `kubernetes/bootstrap.sh:376` still hardcodes `talosctl apply-config --insecure`. `--insecure` talks to Talos's maintenance-mode listener, which an already-configured node doesn't expose (consistent with the runbook's own stage-2 example at line 202, which drops `--insecure` and uses `--mode=try` instead). If so, re-adding an already-configured single node via `apply_node_config` would identify correctly but then spin through all 60 apply attempts and fail with "Timeout applying to $addr". This predates the diff (the old DNS-based path had the same hardcoded `--insecure`), so it's not a regression, but the new docstring's claim is broader than what the code actually delivers — worth either scoping the comment down or handling the configured-node apply case for real. - `kubernetes/bootstrap.sh:112` — `fail()` writes its message with a plain `echo` (stdout), not `>&2`. In `patch_for_machine`, calls like `node_patch=$(patch_for_machine "$addr" 2>/dev/null)` only redirect stderr, so a `fail()` invoked inside is captured into `node_patch` as text instead of being discarded. It's harmless today only because `fail()` always `exit`s non-zero, which the caller's `if node_patch=$(...) && [[ -n "$node_patch" ]]` checks before ever using `$node_patch`'s content — but it's a fragile invariant to lean on silently. Sending `fail()`'s output to stderr would remove the dependency. ## 🟢 Nits - `kubernetes/bootstrap.sh:337-339` — `mkdir -p ~/.talos` is left over from before `TALOSCONFIG_PATH` was introduced; the very next line already does `mkdir -p "$(dirname "$TALOSCONFIG_PATH")"`, which is the directory that's actually used. The old line is now dead weight and, worse, hardcodes `$HOME` in a section whose whole point is to stop touching it unconditionally. ## Tests The PR's own test is the new `selftest` phase, and it's a solid regression guard for the CNI bootstrap contract (asserts `NotReady` pre-Cilium, `Ready` + agent `1/1` post-Cilium, and that `cluster.yaml` still declares `cni: none`/`proxy.disabled`). It isn't wired into any Woodpecker pipeline (checked `.woodpecker/*.yml` — nothing invokes `bootstrap.sh`), which matches the PR's framing of it as a manual pre-flight check requiring Docker; that's a reasonable tradeoff for infra code that can't run in ordinary CI. The actual headline fix (`--insecure` ordering in `patch_for_machine`) has no automated coverage — understandably, since it requires a real/maintenance-mode-capable node — but the PR body documents a manual QEMU repro (`unknown command` vs. `tls: certificate required`) that's convincing evidence the fix is correct. ## Recommendation Looks reasonable — no blocking issues; comments above are optional.

claude-bot added 1 commit 2026-08-28 19:52:48 +00:00

fix(bootstrap): drop the leftover mkdir into $HOME

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

28edd95de1

Redundant since the line below creates the directory of whatever
TALOSCONFIG_PATH resolves to, and it defeated the point of the change that
introduced it: a rehearsal pointed at throwaway files still created ~/.talos.

Harmless in itself -- an empty directory -- but the previous commit claims
nothing in $HOME is touched, and that claim has to be true without exceptions
to be worth anything.

review-bot reviewed

2026-08-28 19:58:07 +00:00

review-bot left a comment

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

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

Summary

This PR fixes the maintenance-mode serial lookup added by the (unmerged) #710 — the --insecure flag was placed before the get subcommand instead of after it, which made talosctl reject the command before ever contacting the node and silently disabled the only code path a cold rebuild can use. It also fixes two set -e/grep interactions that hid the resulting failures, adds a Docker-based selftest phase, and makes ~/.talos/~/.kube paths overridable via TALOSCONFIG/KUBECONFIG so the new flux rehearsal mode (FLUX_BRANCH) can't clobber the operator's real kubeconfig. I read the whole script and the three touched Talos patches; the core fix is correct and well-supported by the PR's own repro. I found no confirmed blocking issues, but one new code path (the selftest phase's talosctl cluster create invocation) is unverified and worth a second look before relying on it, and the cold-rebuild runbook is now out of sync with what this PR revives.

🟡 Suggestions

kubernetes/bootstrap.sh:263 — talosctl cluster create docker \ ... passes docker as a bare positional argument. If talosctl cluster create takes the provisioner via --provisioner/-p (as I recall from Talos' CLI, with docker as the default) rather than as a positional, this would fail the exact same way the --insecure bug did — talosctl rejecting the invocation before doing anything — except this path isn't wired into CI (selftest is explicitly "not part of all", and nothing in the repo's Woodpecker config or docs references it), so a syntax error here wouldn't be caught automatically. I couldn't verify this in the sandbox (no talosctl/docker/network access here). Worth running ./bootstrap.sh selftest once end-to-end (or checking talosctl cluster create --help) before trusting it as a pre-flight check — the PR body's QEMU test verified install_cilium and the serial lookup on real-ish hardware, not this Docker code path specifically.
kubernetes/RUNBOOK-cold-rebuild.md:391-396 — the runbook still says "bootstrap.sh talos is dead — it cannot generate a config (#581), and Step 2 no longer uses it," directing operators to a manual apply procedure instead. This PR is squarely about making the automated talos phase's maintenance-mode identification actually work during a cold rebuild, which is the exact scenario that note tells people to bypass. Worth a follow-up updating that note (or confirming it's still accurate and the automated phase remains unused by design) so a rebuild under pressure doesn't get routed around a phase this PR just fixed.
kubernetes/bootstrap.sh:86-88 — the KUBECONFIG colon-list check runs unconditionally at script start, before phase dispatch. It's relevant to talos/flux/wait-apps (which write KUBECONFIG_PATH), but not to selftest, which exports its own isolated KUBECONFIG and never touches KUBECONFIG_PATH. An operator who normally runs with a multi-path KUBECONFIG (a common kubectl convention) would hit this failure even for ./bootstrap.sh selftest, where it's a false alarm. Minor — fails loud and early rather than misbehaving, but easy to scope to the phases that actually use KUBECONFIG_PATH.
kubernetes/bootstrap.sh:360-367 — on the last retry (i -eq 60) patch_for_machine "$addr" is called a second time, unsuppressed, to surface the real error. If that second call happens to succeed (the machine answers on this call after failing the first one this iteration), its output isn't captured into node_patch, so the subsequent [[ -n "$node_patch" ]] || fail "never identified..." still fires and reports a misleading "never identified" failure despite the machine having just answered. Narrow race window (needs the very last retry to flip from failure to success within the same iteration), and the outcome is still a safe hard-stop rather than a wrong config being applied, so this is a nit rather than a real risk.

🟢 Nits

kubernetes/talos/patches/k8s-1.yaml:1-10 / k8s-2.yaml / k8s-3.yaml — chassis serial numbers are now committed in plaintext to what pr.json shows is a public repo. These aren't credentials, but they are physical-hardware identifiers (useful for e.g. vendor-support impersonation). This matches the pre-existing convention of committing MAC addresses and disk serials in these same files, so it's not a new class of exposure introduced by this PR — just flagging for awareness, not asking for a change.

Tests

The PR's own testing is unusually well-documented in its description: a real QEMU-provisioned node with the actual factory schematic was used to reproduce the original bug (unknown command "systeminformation") and confirm the fix (tls: certificate required, i.e. the node was actually contacted), and to verify install_cilium brings a node to Ready with the expected Cilium datapath. That's solid evidence for the core fix.

The new selftest phase (Docker-based CNI bootstrap check) is a good addition for future regression coverage of install_cilium, but as noted above I couldn't confirm it was actually exercised as written — it's deliberately excluded from all and isn't referenced by any CI config in this repo, so nothing currently exercises it automatically. Recommend running it once manually to confirm the talosctl cluster create docker invocation is valid before treating it as a working pre-flight check.

Recommendation

Looks reasonable — no confirmed blocking issues; comments above (particularly verifying the selftest phase actually runs, and reconciling the runbook's "bootstrap.sh talos is dead" note) are worth following up on.

🤖 **Automated review** — model `CLI default` (complexity: complex), commit `28edd95de16a634505b0c9558d74cc2e3330077d`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary This PR fixes the maintenance-mode serial lookup added by the (unmerged) #710 — the `--insecure` flag was placed before the `get` subcommand instead of after it, which made `talosctl` reject the command before ever contacting the node and silently disabled the only code path a cold rebuild can use. It also fixes two `set -e`/`grep` interactions that hid the resulting failures, adds a Docker-based `selftest` phase, and makes `~/.talos`/`~/.kube` paths overridable via `TALOSCONFIG`/`KUBECONFIG` so the new `flux` rehearsal mode (`FLUX_BRANCH`) can't clobber the operator's real kubeconfig. I read the whole script and the three touched Talos patches; the core fix is correct and well-supported by the PR's own repro. I found no confirmed blocking issues, but one new code path (the `selftest` phase's `talosctl cluster create` invocation) is unverified and worth a second look before relying on it, and the cold-rebuild runbook is now out of sync with what this PR revives. ## 🟡 Suggestions - `kubernetes/bootstrap.sh:263` — `talosctl cluster create docker \ ... ` passes `docker` as a bare positional argument. If `talosctl cluster create` takes the provisioner via `--provisioner`/`-p` (as I recall from Talos' CLI, with `docker` as the default) rather than as a positional, this would fail the exact same way the `--insecure` bug did — `talosctl` rejecting the invocation before doing anything — except this path isn't wired into CI (`selftest` is explicitly "not part of `all`", and nothing in the repo's Woodpecker config or docs references it), so a syntax error here wouldn't be caught automatically. I couldn't verify this in the sandbox (no `talosctl`/`docker`/network access here). Worth running `./bootstrap.sh selftest` once end-to-end (or checking `talosctl cluster create --help`) before trusting it as a pre-flight check — the PR body's QEMU test verified `install_cilium` and the serial lookup on real-ish hardware, not this Docker code path specifically. - `kubernetes/RUNBOOK-cold-rebuild.md:391-396` — the runbook still says "`bootstrap.sh talos` is dead — it cannot generate a config (#581), and Step 2 no longer uses it," directing operators to a manual apply procedure instead. This PR is squarely about making the automated `talos` phase's maintenance-mode identification actually work during a cold rebuild, which is the exact scenario that note tells people to bypass. Worth a follow-up updating that note (or confirming it's still accurate and the automated phase remains unused by design) so a rebuild under pressure doesn't get routed around a phase this PR just fixed. - `kubernetes/bootstrap.sh:86-88` — the `KUBECONFIG` colon-list check runs unconditionally at script start, before phase dispatch. It's relevant to `talos`/`flux`/`wait-apps` (which write `KUBECONFIG_PATH`), but not to `selftest`, which exports its own isolated `KUBECONFIG` and never touches `KUBECONFIG_PATH`. An operator who normally runs with a multi-path `KUBECONFIG` (a common kubectl convention) would hit this failure even for `./bootstrap.sh selftest`, where it's a false alarm. Minor — fails loud and early rather than misbehaving, but easy to scope to the phases that actually use `KUBECONFIG_PATH`. - `kubernetes/bootstrap.sh:360-367` — on the last retry (`i -eq 60`) `patch_for_machine "$addr"` is called a second time, unsuppressed, to surface the real error. If that second call happens to succeed (the machine answers on this call after failing the first one this iteration), its output isn't captured into `node_patch`, so the subsequent `[[ -n "$node_patch" ]] || fail "never identified..."` still fires and reports a misleading "never identified" failure despite the machine having just answered. Narrow race window (needs the very last retry to flip from failure to success within the same iteration), and the outcome is still a safe hard-stop rather than a wrong config being applied, so this is a nit rather than a real risk. ## 🟢 Nits - `kubernetes/talos/patches/k8s-1.yaml:1-10` / `k8s-2.yaml` / `k8s-3.yaml` — chassis serial numbers are now committed in plaintext to what `pr.json` shows is a **public** repo. These aren't credentials, but they are physical-hardware identifiers (useful for e.g. vendor-support impersonation). This matches the pre-existing convention of committing MAC addresses and disk serials in these same files, so it's not a new class of exposure introduced by this PR — just flagging for awareness, not asking for a change. ## Tests The PR's own testing is unusually well-documented in its description: a real QEMU-provisioned node with the actual factory schematic was used to reproduce the original bug (`unknown command "systeminformation"`) and confirm the fix (`tls: certificate required`, i.e. the node was actually contacted), and to verify `install_cilium` brings a node to `Ready` with the expected Cilium datapath. That's solid evidence for the core fix. The new `selftest` phase (Docker-based CNI bootstrap check) is a good addition for future regression coverage of `install_cilium`, but as noted above I couldn't confirm it was actually exercised as written — it's deliberately excluded from `all` and isn't referenced by any CI config in this repo, so nothing currently exercises it automatically. Recommend running it once manually to confirm the `talosctl cluster create docker` invocation is valid before treating it as a working pre-flight check. ## Recommendation Looks reasonable — no confirmed blocking issues; comments above (particularly verifying the `selftest` phase actually runs, and reconciling the runbook's "bootstrap.sh talos is dead" note) are worth following up on.

claude-bot referenced this pull request

2026-08-29 06:37:45 +00:00

feat(bootstrap): FLUX_BRANCH, damit die Flux-Phase probelaufen kann #717

claude-bot commented

2026-08-29 06:46:49 +00:00

Aufgelöst statt rebased — nach #716 und #717 ist von den sieben Commits nichts mehr offen:

Commit	Verbleib
`$HOME`-Isolation (2 Commits)	in master über #716
Docker-Selftest	von der `vmtest`-Phase überholt: echtes Factory-Image, drei Nodes, derselbe Codepfad statt eines nachgebauten
`FLUX_BRANCH`	in master über #717
`--insecure`-Flag-Fix	gegenstandslos: er behebt einen Bug in `patch_for_machine`, und diese Funktion existiert in master nicht
Chassis-Serial-Marker + serienbasierte Identifikation	nie gemergt, Design kollidiert mit `NODE_ADDRS` aus #716

Der --insecure-Fix war korrekt und gegen eine konfigurationslose Maschine verifiziert — nur behebt er einen Fehler in Code, der nie in master lag.

Offen bleibt eine Entscheidung, kein Patch: NODE_ADDRS beantwortet „an welche Adresse", die Serial-Identifikation beantwortete „welche Maschine ist das überhaupt". Beim Cold Rebuild ohne DNS und ohne Hostnamen sind das verschiedene Fragen — dort liegen drei Leases vor, ohne Zuordnung zu den Nodes. Falls das zurückkommen soll, gehört es auf einen frischen Branch gegen den heutigen apply_node_config, nicht auf diesen Stand.

Aufgelöst statt rebased — nach #716 und #717 ist von den sieben Commits nichts mehr offen: | Commit | Verbleib | |---|---| | `$HOME`-Isolation (2 Commits) | in master über #716 | | Docker-Selftest | von der `vmtest`-Phase überholt: echtes Factory-Image, drei Nodes, derselbe Codepfad statt eines nachgebauten | | `FLUX_BRANCH` | in master über #717 | | `--insecure`-Flag-Fix | gegenstandslos: er behebt einen Bug in `patch_for_machine`, und diese Funktion existiert in master nicht | | Chassis-Serial-Marker + serienbasierte Identifikation | nie gemergt, Design kollidiert mit `NODE_ADDRS` aus #716 | Der `--insecure`-Fix war korrekt und gegen eine konfigurationslose Maschine verifiziert — nur behebt er einen Fehler in Code, der nie in master lag. Offen bleibt eine Entscheidung, kein Patch: `NODE_ADDRS` beantwortet „an welche Adresse", die Serial-Identifikation beantwortete „welche Maschine ist das überhaupt". Beim Cold Rebuild ohne DNS und ohne Hostnamen sind das verschiedene Fragen — dort liegen drei Leases vor, ohne Zuordnung zu den Nodes. Falls das zurückkommen soll, gehört es auf einen frischen Branch gegen den heutigen `apply_node_config`, nicht auf diesen Stand.

claude-bot closed this pull request

2026-08-29 06:46:49 +00:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: konairius/homelab#711