From fdc4a7d9f4a9b15f03184d603d36bd485d1762af Mon Sep 17 00:00:00 2001 From: Keith Jackson Date: Tue, 1 Sep 2026 01:40:20 -0500 Subject: [PATCH] =?UTF-8?q?fix(cilium):=20maxUnavailable=202=20->=201=20?= =?UTF-8?q?=E2=80=94=20bound=20CNI=20rollout=20blast=20radius=20(P2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit During the substitution incident a bad render (k8sServiceHost: "") took down TWO of three Cilium agents simultaneously. The agents' `config` init container falls back to the API ClusterIP, which only the Cilium agent itself can translate — and that agent is the one being replaced — so it deadlocks on `dial tcp 10.43.0.1:443: i/o timeout`. Root cause of the 2-at-once: `updateStrategy.rollingUpdate.maxUnavailable: 2` is the UPSTREAM CHART DEFAULT, confirmed by downloading cilium-1.20.1.tgz and reading values.yaml:437-444 — not a local choice, and not overridden anywhere in this repo. maxUnavailable: 1 makes a rollout replace ONE agent at a time and stop at the first pod that fails to become Ready. The whole value of the setting is that the rollout halts on the first bad node instead of taking the majority of the dataplane with it, and that guarantee only exists at 1. It matters far more on a cold PROD bootstrap, where there is no healthy peer to recover from — dev only self-healed because k3s-dev-3 kept a working agent throughout. Set as a [parity] field in ALL THREE places, per this repo's documented contract that the Ansible bootstrap values and the Flux HelmRelease values must agree on settings that define cluster networking (Ansible installs Cilium at bootstrap; the HelmRelease adopts it; the two layers fight otherwise): platform/cilium/helmrelease.yaml (authoritative for Flux) infra/ansible/roles/cilium/templates/cilium-values.yaml.j2 (authoritative for bootstrap) platform/cilium/values.yaml (reference mirror, kept in sync) Verified by rendering the .j2 with roles/cilium/defaults and diffing full value sets: every [parity] field is byte-equal across all three; only k8sServiceHost differs, as designed (${K3S_API_HOST} vs {{ k3s_api_host }}). MERGING THIS IS NON-DISRUPTIVE. updateStrategy lives at DaemonSet.spec.updateStrategy — a SIBLING of spec.template (chart daemonset.yaml:49-53). The DaemonSet controller decides a pod is out of date by hashing spec.template alone, so the pod-template hash does not move, no new ControllerRevision is created, and zero pods are recreated. Helm patches the field in place. (If it were disruptive it would be self-defeating: rolling agents under the old maxUnavailable: 2 in order to install maxUnavailable: 1.) Verify after reconcile with `kubectl -n kube-system get ds cilium -o jsonpath='{.spec.updateStrategy.rollingUpdate.maxUnavailable}'` plus unchanged pod ages — that is the cleanest proof nothing rolled. Trade-off recorded in the comments: upgrades serialise, roughly two waves to three at 3 nodes (a minute or two). On a large cluster this becomes painful and a percentage such as "10%" would be right — noted so a future larger cluster revisits it deliberately. SCOPE: this bounds blast radius, it does NOT close the substitution race. A bad render still ships; it now breaks one node and stalls rather than taking the majority. platform.dependsOn: [flux-system] (P0) is the fix for the cause. Serves: R-PLAT-01, R-NET-01. Co-Authored-By: Claude Opus 5 (1M context) --- .../cilium/templates/cilium-values.yaml.j2 | 17 +++++++++++++++ platform/cilium/helmrelease.yaml | 21 +++++++++++++++++++ platform/cilium/values.yaml | 10 +++++++++ 3 files changed, 48 insertions(+) diff --git a/infra/ansible/roles/cilium/templates/cilium-values.yaml.j2 b/infra/ansible/roles/cilium/templates/cilium-values.yaml.j2 index 0704adb..8a22edf 100644 --- a/infra/ansible/roles/cilium/templates/cilium-values.yaml.j2 +++ b/infra/ansible/roles/cilium/templates/cilium-values.yaml.j2 @@ -42,5 +42,22 @@ cgroup: enabled: false hostRoot: /run/cilium/cgroupv2 +# Roll the cilium agent DaemonSet ONE node at a time. [parity] must match the +# platform/cilium HelmRelease. The cilium 1.20.1 chart defaults this to 2, which on a +# 3-node cluster lets one bad render take down two of three agents simultaneously — +# exactly the 2026-09-01 incident, where k8sServiceHost: "" made the agent's `config` +# init container fall back to the in-cluster ClusterIP 10.43.0.1:443. Only the Cilium +# agent can translate that ClusterIP, and the agent in question was the one being +# replaced: `dial tcp 10.43.0.1:443: i/o timeout`, deadlocked. +# A CNI rollout has no healthy-peer fallback — the component being upgraded is the one +# the upgrade needs to reach the API server. This bites hardest on a COLD BOOTSTRAP +# (this role's job), where nothing healthy exists to recover from. With maxUnavailable: 1 +# the DaemonSet will not proceed past an unready pod, bounding damage to a single node. +# Cost: Cilium upgrades serialise and take longer; at 3 nodes that is an acceptable trade. +updateStrategy: + type: RollingUpdate + rollingUpdate: + maxUnavailable: 1 + operator: replicas: {{ cilium_operator_replicas }} diff --git a/platform/cilium/helmrelease.yaml b/platform/cilium/helmrelease.yaml index 781964d..68086d6 100644 --- a/platform/cilium/helmrelease.yaml +++ b/platform/cilium/helmrelease.yaml @@ -89,6 +89,27 @@ spec: autoMount: enabled: false hostRoot: /run/cilium/cgroupv2 + # --- Agent rollout: ONE node at a time (blast-radius bound) — 2026-09-01 incident --- + # [parity] MUST equal Ansible: updateStrategy.rollingUpdate.maxUnavailable: 1. + # The chart's own default is 2 (cilium 1.20.1 values.yaml: updateStrategy.rollingUpdate. + # maxUnavailable: 2) — NOT a local choice, and on a 3-node cluster that lets a single bad + # Helm render take down two of three agents simultaneously. That is exactly what happened + # when a fails-open post-build substitution shipped k8sServiceHost: "": the agent's `config` + # init container fell back to the in-cluster ClusterIP 10.43.0.1:443, which only the Cilium + # agent itself can translate — and the agent translating it was the one being replaced. + # `dial tcp 10.43.0.1:443: i/o timeout`, deadlocked, on 2 of 3 nodes at once. + # A CNI rollout has no healthy-peer fallback: the thing being upgraded is the thing the + # upgrade needs in order to reach the API server. With maxUnavailable: 1 the DaemonSet + # will not proceed past an unready pod, so a bad render costs one node, not the cluster. + # This matters most on a COLD PROD BOOTSTRAP, where every knob is new at once and there is + # no already-healthy agent to recover from (see DECISIONS-LOG 2026-09-01 INCIDENT/RESEARCH). + # Cost, recorded: Cilium upgrades serialise and get slower. On 3 nodes that is minutes, and + # it buys a hard stop on the first bad node — the right trade here. Revisit if the cluster + # grows enough that serialised CNI upgrades become an operational problem. + updateStrategy: + type: RollingUpdate + rollingUpdate: + maxUnavailable: 1 # NOTE (NON-parity): operator.replicas is deliberately omitted here. The Ansible dev render sets # replicas: 1 to save RAM, but defaults/main.yml declares it explicitly NOT a parity field # (Flux/prod may differ). Only the [parity] values above must match the bootstrap. diff --git a/platform/cilium/values.yaml b/platform/cilium/values.yaml index 3223eaf..e9f791b 100644 --- a/platform/cilium/values.yaml +++ b/platform/cilium/values.yaml @@ -42,6 +42,16 @@ cgroup: autoMount: enabled: false hostRoot: /run/cilium/cgroupv2 +# [parity] roll the agent DaemonSet ONE node at a time. Overrides the cilium 1.20.1 chart +# default of 2, which on 3 nodes lets a single bad render break 2/3 agents at once — as the +# 2026-09-01 incident did (k8sServiceHost: "" → the `config` init container fell back to the +# ClusterIP 10.43.0.1:443, translatable only by the very agent being replaced). A CNI upgrade +# has no healthy-peer fallback, least of all on a cold prod bootstrap; maxUnavailable: 1 stops +# the rollout at the first unready pod. Cost: serialised, slower upgrades (fine at 3 nodes). +updateStrategy: + type: RollingUpdate + rollingUpdate: + maxUnavailable: 1 # NON-parity (Flux/prod may differ; dev sets 1 to save RAM) — omitted from the HelmRelease: # operator.replicas #