From 1157c1b0713b45df4937026efd19e9475f785531 Mon Sep 17 00:00:00 2001 From: Keith Jackson Date: Mon, 31 Aug 2026 23:49:43 -0500 Subject: [PATCH] fix(gitea-actions): wait for dind to ACCEPT connections, not just write certs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-authors the fix stranded on origin/fix/act-runner-dind-readiness (242951a), which applied cleanly but had never been merged. The act_runner startup waited only for the dind TLS CERT FILE (/certs/client/ca.pem), but dockerd writes the certs before it begins accepting TLS connections on :2376. A runner proceeding on the file alone still hard-exits with "cannot ping the docker daemon at tcp://localhost:2376" — observed crashlooping 11 times in ~8 minutes on a fresh pod after a node bounce. Adds a second bounded wait (nc -z -w1 127.0.0.1 2376, 60s cap) mirroring the existing cert-file loop, so the runner starts only once the daemon actually answers. busybox nc in the Alpine-based gitea/act_runner:0.2.11 supports -z/-w. This Deployment is the ENFORCED MERGE GATE for the whole repo, so it ships alone. A REPORTED "latent envsubst bug" in this file was INVESTIGATED AND DISPROVEN. The claim was that Flux envsubst expands bare `$i`, making the existing `[ "$i" -ge 60 ]` bound a silent no-op. Running the real `flux envsubst` (v2.4.0) shows bare `$i` is NOT substituted — even with i set in the environment, `$i` passes through literally while `${i}` expands. This matches the repo's own contract in clusters/base/apps-vms.yaml, which was itself written after empirical probing: "`$VAR` -> left alone by envsubst => bare shell vars need NO escaping." The existing 60s bound was always real. NO escaping was applied: `$${i}` would be an ACTIVE REGRESSION, correct only on the envsubst path — on a plain `kustomize build`/`kubectl apply -k`, sh reads `$$` as its own PID and the bound compares garbage against 60. Bare is correct on both paths. A comment now records the real contract so nobody "fixes" the bare form into escaping. NEAR MISS worth recording: the first draft of that comment spelled the dangerous forms out literally, and `flux envsubst` HARD-FAILED on it — "unable to parse variable name". Flux's envsubst runs over the entire manifest INCLUDING YAML COMMENTS, so a braced name containing punctuation is a fatal parse error that would have failed the whole apps-wave-1 Kustomization — breaking every merge in the repo. From a comment. The shipped comment names the forms in words; the file now contains no braced-dollar sequence at all. Verified without a cluster: flux envsubst over the file is byte-identical (exit 0, and --strict 0); post-envsubst YAML parses with containers unchanged; `sh -n` clean; a dry-run with a stubbed failing nc fires the stage-2 bound and exits 1; render==golden green (gitea-actions.yaml is an EXTRA_UNITS entry — render.py places the filename into a wave and never emits its bytes, so it is not among the 10 compared goldens). Follow-up documented in-file, deliberately NOT done here: convert dind to an initContainers sidecar (restartPolicy: Always) with a startupProbe that execs `docker info` over TLS — NOT a bare tcpSocket probe, which would reopen the same "port open but certs not ready" gap stage 1 closes — periodSeconds 1 / failureThreshold 60 to preserve the cap, after which runner drops its command: override. k3s v1.36 has GA sidecars, so this is the correct long-term shape; it ships separately because a failure in the merge gate must be unambiguous. Serves: R-GITOPS-01, R-TEST-03. Co-Authored-By: Claude Opus 5 (1M context) --- apps/wave-1/gitea-actions.yaml | 79 +++++++++++++++++++++++++++++++--- 1 file changed, 74 insertions(+), 5 deletions(-) diff --git a/apps/wave-1/gitea-actions.yaml b/apps/wave-1/gitea-actions.yaml index 4447d82..198edb5 100644 --- a/apps/wave-1/gitea-actions.yaml +++ b/apps/wave-1/gitea-actions.yaml @@ -103,21 +103,65 @@ spec: # ---- act_runner: registers with Gitea, dispatches jobs to the DinD daemon ---- - name: runner image: gitea/act_runner:0.2.11 - # Wait for the DinD sidecar to publish its TLS client certs into the shared - # emptyDir before starting the daemon — act_runner reads /certs/client/ca.pem at - # startup and hard-exits if it is missing (a startup race against the sidecar). - # Then exec the image's normal entrypoint (/sbin/tini -- /opt/act/run.sh), which - # registers-from-env on first boot and runs `act_runner daemon`. + # TWO-STAGE BOUNDED WAIT on the DinD sidecar, then exec the image's normal + # entrypoint (/sbin/tini -- /opt/act/run.sh), which registers-from-env on first + # boot and runs `act_runner daemon`. + # + # STAGE 1 — the certs must EXIST: act_runner reads /certs/client/ca.pem at startup + # and hard-exits if it is missing (a startup race against the sidecar). + # STAGE 2 — the daemon must actually ACCEPT a connection: the cert FILE appearing is + # necessary but NOT sufficient. dockerd writes the certs and only THEN begins + # listening on :2376, so a runner that proceeds on the file alone still hard-exits + # with "cannot ping the docker daemon at tcp://localhost:2376" — observed + # crashlooping 11 times in ~8 minutes on a fresh pod after a node bounce. + # Both loops are bounded (60 x 1s) and exit 1 on timeout, so a genuinely dead sidecar + # surfaces as a CrashLoopBackOff with a clear message instead of hanging forever. + # + # ENVSUBST SAFETY — the BARE $i / $j below are CORRECT. Do not "tidy" them into the + # braced form or the double-dollar form; both are wrong here, for different reasons. + # clusters/base/apps-wave-1.yaml carries postBuild.substituteFrom cluster-vars, so + # Flux envsubst runs over this ENTIRE manifest — YAML comments very much included. + # Its contract (drone/envsubst; the same one spelled out in clusters/base/apps-vms.yaml, + # and re-verified against this exact file with `flux envsubst`, flux v2.4.0) is: + # - A bare $name is NOT a reference to envsubst: it passes through untouched, and + # that holds even when the name IS set in cluster-vars. So $i and $j reach the + # cluster verbatim and both 60s bounds below are real, enforced bounds. + # - $((i + 1)) is likewise untouched: "(" cannot begin a variable name. + # - A BRACED dollar-name IS a reference and gets replaced — with the empty string + # when the name is unset. One of those here would make a bound read [ "" -ge 60 ], + # which errors, short-circuits the &&, and silently turns the wait UNBOUNDED. + # - A DOUBLE-dollar braced name is envsubst's escape and survives as the braced + # form — still wrong for us, because on any path where envsubst does NOT run + # (a plain `kustomize build` or `kubectl apply -k` of this directory) the shell + # reads that leading double-dollar as its own PID, so a bound would compare a + # string like 1234{i} against 60. The bare form is the only spelling that is + # correct on BOTH paths. + # - A braced name containing punctuation is a HARD ERROR ("unable to parse variable + # name") that fails this whole Kustomization — and this Kustomization gates every + # merge in the repo. So never write one anywhere in this file, INCLUDING in a + # comment. That is why this note names the three forms in words rather than + # showing them: an earlier draft of it spelled them out literally and broke + # `flux envsubst` on the file. (Same trap the indexed-expansion warning in + # clusters/base/apps-vms.yaml describes.) command: - /bin/sh - -c - | + # Stage 1: the sidecar's TLS client certs must appear in the shared emptyDir. i=0 until [ -f /certs/client/ca.pem ]; do i=$((i + 1)) [ "$i" -ge 60 ] && { echo "dind certs never appeared" >&2; exit 1; } echo "waiting for dind TLS certs ($i)"; sleep 1 done + # Stage 2: dockerd must actually accept a connection on :2376. busybox nc in this + # Alpine-based image (gitea/act_runner:0.2.11) supports -z (scan) and -w (timeout). + j=0 + until nc -z -w1 127.0.0.1 2376; do + j=$((j + 1)) + [ "$j" -ge 60 ] && { echo "dind daemon never accepted a connection on 2376" >&2; exit 1; } + echo "waiting for dind daemon on :2376 ($j)"; sleep 1 + done exec /sbin/tini -- /opt/act/run.sh env: - name: GITEA_INSTANCE_URL @@ -155,6 +199,31 @@ spec: limits: memory: 512Mi # ---- Docker-in-Docker: the job executor daemon (privileged) ---- + # + # FOLLOW-UP (deliberately NOT bundled into the readiness fix above — own change): + # Convert this container into a NATIVE SIDECAR and delete both shell wait-loops. + # k3s is now v1.36, where sidecar containers are GA, so kubelet can enforce the + # ordering that the loops above hand-roll — which also removes the envsubst hazard + # from this file entirely (no shell, no $i/$j, nothing to escape or mis-escape). + # Shape to build: + # 1. Move this whole container into `initContainers:` as the FIRST entry. + # 2. Add `restartPolicy: Always` to it. That single field is what makes an + # initContainer a sidecar: it starts before the regular containers, keeps + # running alongside them, and is terminated after them. + # 3. Give it a `startupProbe` that succeeds only when dockerd accepts a TLS + # connection — e.g. `exec` a `docker -H tcp://127.0.0.1:2376 --tlsverify + # --tlscacert /certs/client/ca.pem --tlscert /certs/client/cert.pem + # --tlskey /certs/client/key.pem info`. Do NOT use a bare `tcpSocket` probe: + # it would re-open the same "port accepting but TLS/certs not ready" gap that + # Stage 1 above exists to close. `periodSeconds: 1` + `failureThreshold: 60` + # preserves today's 60s cap. + # 4. kubelet then holds the `runner` container until that startupProbe passes, so + # `runner` can drop its `command:` override completely and run the image's + # default entrypoint. + # Why it is a separate change: this Deployment is the ENFORCED MERGE GATE for this + # entire repository. If the conversion is wrong, nothing can merge — so it must land + # alone, where the failure is unambiguous and not entangled with anything else, and + # only after a real workflow run has been observed green on the converted pod. - name: dind image: docker:27-dind securityContext: