Homelab/infra-core
6
0
Fork 2
Code Issues Pull requests 35 Projects Releases 1 Packages Wiki Activity Actions

IR-20261111: R25 off disk — drop pod-mounted sops/AWS secrets from forgejo-runner, capacity 2->1 (lands AFTER prod #1206) #944

Open
pi-landing wants to merge 3 commits from plane/IR-20261111-r25offdisk into live
pull from: plane/IR-20261111-r25offdisk
merge into: Homelab:live
Conversation 7 Commits 3 Files changed 1 +21 -22
pi-landing commented 2026-11-11 20:10:55 +00:00
Collaborator
Copy link

R25 off disk (step 2 of 2): drop the pod-mounted sops/AWS secrets from the forgejo-runner pool, capacity 2→1

RE-MERGED 2026-11-12T06:28Z — the landing precondition is now MET.
Homelab/prod #1206 landed at 06:15Z (prod live 9597e6f7), so the
"lands after #1206" rule below is satisfied. origin/live (491c9c02, via
#946/#948/#949/#952/#954) is merged into this branch and the head moved
5b614f9c → e84b4436 — a merge commit, no amend, no force-push.

One conflict, in core/manifests/apps/forgejo-runner/configmap.yaml: both
sides already said capacity: 1 (this branch's 2→1 is already live via
#948), and the only difference was a single explanatory comment line that
live carries, so live's file was taken verbatim — nothing from the branch's
copy was dropped. statefulset.yaml auto-merged: live's replica comment block
and the prod-wk-01 affinity change sit alongside this PR's R25 removal,
which is untouched.

Re-verified at e84b4436: capacity: 1; replicas: 8 (#954's 9→8, not
resurrected); dind-gc RUNNER_COUNT = 8 in both the CronJob env and the
script default; and no sops-age-key / aws-credentials volume, volumeMount
or SOPS_AGE_KEY_FILE / AWS_SHARED_CREDENTIALS_FILE env anywhere in the
StatefulSet — every other volume and env entry unchanged. Local suites
(test_runner_dind_gc_tightening.py, test_runner_ephemeral_ceilings.py,
test_runner_placement_prep.py, test_sbx_npm_cache.py) pass, plain and
--self-check; CI is green on the new head.

Nothing has merged. Review is on the new head.

LANDS AFTER Homelab/prod PR #1206 — do not merge before it.
#1206 gives the four prod terraform jobs (plan-cluster, plan-zitadel,
apply, apply-cluster) their own per-job fetch of the sops age key and
AWS state-backend credentials from OpenBao. If this PR lands first, those
four jobs lose their only credential source and every terraform plan/apply
fails. Merge order: #1206 → this PR.

Why

R25: the runner StatefulSet mounted two Secrets pod-wide — sops-age-key
(/home/runner/.config/sops/age) and aws-credentials (/home/runner/.aws),
both 0440, on the single runner container. Host-mode (:host) job steps run
inside that container as the same uid, and the pool registration is
instance-scope — so every host-mode job from every repo on the instance
could read both files, including k8s-label jobs that never need them.

After #1206, only the four prod terraform jobs consume either secret, each
fetching exactly what it needs per job into a fresh 0700 directory (0600
files) and shredding it on every exit path. The pod mounts are dead weight and
an exposure — this PR removes them.

What changes (2 files, nothing else)

  • core/manifests/apps/forgejo-runner/statefulset.yaml:
    • runner container env: SOPS_AGE_KEY_FILE / AWS_SHARED_CREDENTIALS_FILE
      defaults removed (R25 comment left in place);
    • runner container volumeMounts: sops-age-key and aws-credentials
      mounts removed;
    • pod volumes: the two secret volumes removed.
    • The ld-ci/ld-push identities and every other volume/env are untouched;
      runner-forgejo-token (envFrom) and the Harbor robot docker-config
      plumbing stay exactly as they are.
  • core/manifests/apps/forgejo-runner/configmap.yaml: runner.capacity
    2 → 1.

Why capacity 1 is load-bearing

Host-mode jobs run as the same uid in the same container, so 0700 on a
temp dir does not isolate one job from another — only capacity: 1 makes the
per-job copy private. Without it, #1206's "per-job" fetch is per-job in name
only.

Throughput cost: 16 → 8 slots

  • 8 replicas × capacity 1 = 8 concurrent job slots (was 16).
  • HOMELAB-2418 14-day census: measured global pool peak is 8 concurrent
    runs, p95 = 5, zero hours above 8    . So normal hours are unaffected; the
    peak hour becomes fully saturated (queueing instead of half-used headroom),
    and any future peak above 8 waits.
  • Per-pod sharing also goes away: a job now gets a whole dind (8c/8Gi) instead
    of ~4c/4Gi, but loses the neighbour docker-layer-cache share; each pod's own
    warm cache still serves its successive jobs.
  • A wedged job occupies a whole pod for up to runner.timeout: 2h instead of
    half of one.

Accepted residual R25 (named, not fixed here)

  • The Harbor pull-only robot stays on disk at
    /home/runner/.docker/config.json (and dind's /root/.docker/config.json).
    The runner daemon pre-pulls images before any step runs, so it cannot become
    per-job. Push credentials are already per-job and never written there.
  • runner-forgejo-token (envFrom) is the runner's own API token — no runner
    can shed it; out of scope.

Left for follow-up (deliberately not in this PR)

  • The ESO objects ExternalSecret/runner-sops-age-key and
    ExternalSecret/runner-aws-credentials
    (core/manifests/external-secrets/es-forgejo-runner.yaml) become unused
    once this rolls out; remove them in a follow-up after the rollout proof.
  • Narrowing the tf-plan policy (path "*" { read, list, sudo } in
    prod/terraform/openbao.tf) — noted on #1206.

Verification done before opening

  • yaml.safe_load on both files; programmatic check that the runner
    container has no sops-age-key/aws-credentials volume, mount, or env, and
    that runner.capacity == 1 in the rendered config.yaml.
  • Diff reviewed vs origin/live (merged in as a merge commit first): only
    the two files above change.

Post-merge live proof (required before this is called done)

  1. kubectl -n forgejo-runner get a rolled pod: no sops-age-key /
    aws-credentials volumes; runner container env without the two paths.
  2. One green plan-cluster run on a PR whose log shows the ephemeral dir
    created and shredded.
  3. A k8s-label job (e.g. infra-core helm-validate) cannot see either
    former path.

Rollback

Revert this PR (or set the removed blocks back + capacity: 2) and re-sync.
Only safe after confirming #1206 is also reverted or the prod workflows
still fetch per job — otherwise the four terraform jobs have no credentials
either way. Statefully: the removed Secrets still exist (ESO objects remain),
so a revert restores the mounts to working order immediately.

Lab-Run: IR-20261111-r25offdisk

# R25 off disk (step 2 of 2): drop the pod-mounted sops/AWS secrets from the forgejo-runner pool, capacity 2→1 > **RE-MERGED 2026-11-12T06:28Z — the landing precondition is now MET.** > Homelab/prod **#1206 landed at 06:15Z** (prod `live` `9597e6f7`), so the > "lands after #1206" rule below is satisfied. `origin/live` (`491c9c02`, via > #946/#948/#949/#952/#954) is merged into this branch and the head moved > `5b614f9c` → `e84b4436` — a merge commit, no amend, no force-push. > > One conflict, in `core/manifests/apps/forgejo-runner/configmap.yaml`: **both > sides already said `capacity: 1`** (this branch's 2→1 is already live via > **#948**), and the only difference was a single explanatory comment line that > live carries, so live's file was taken verbatim — nothing from the branch's > copy was dropped. `statefulset.yaml` auto-merged: live's replica comment block > and the `prod-wk-01` affinity change sit alongside this PR's R25 removal, > which is untouched. > > Re-verified at `e84b4436`: `capacity: 1`; `replicas: 8` (#954's 9→8, not > resurrected); dind-gc `RUNNER_COUNT` = 8 in **both** the CronJob env and the > script default; and no `sops-age-key` / `aws-credentials` volume, volumeMount > or `SOPS_AGE_KEY_FILE` / `AWS_SHARED_CREDENTIALS_FILE` env anywhere in the > StatefulSet — every other volume and env entry unchanged. Local suites > (`test_runner_dind_gc_tightening.py`, `test_runner_ephemeral_ceilings.py`, > `test_runner_placement_prep.py`, `test_sbx_npm_cache.py`) pass, plain and > `--self-check`; CI is green on the new head. > > Nothing has merged. Review is on the new head. > **LANDS AFTER Homelab/prod PR #1206 — do not merge before it.** > #1206 gives the four prod terraform jobs (`plan-cluster`, `plan-zitadel`, > `apply`, `apply-cluster`) their own per-job fetch of the sops age key and > AWS state-backend credentials from OpenBao. **If this PR lands first, those > four jobs lose their only credential source and every terraform plan/apply > fails.** Merge order: #1206 → this PR. ## Why R25: the runner StatefulSet mounted two Secrets pod-wide — `sops-age-key` (`/home/runner/.config/sops/age`) and `aws-credentials` (`/home/runner/.aws`), both `0440`, on the single runner container. Host-mode (`:host`) job steps run inside that container as the same uid, and the pool registration is instance-scope — so **every host-mode job from every repo on the instance** could read both files, including `k8s`-label jobs that never need them. After #1206, only the four prod terraform jobs consume either secret, each fetching exactly what it needs per job into a fresh 0700 directory (0600 files) and shredding it on every exit path. The pod mounts are dead weight and an exposure — this PR removes them. ## What changes (2 files, nothing else) - `core/manifests/apps/forgejo-runner/statefulset.yaml`: - runner container `env`: `SOPS_AGE_KEY_FILE` / `AWS_SHARED_CREDENTIALS_FILE` defaults removed (R25 comment left in place); - runner container `volumeMounts`: `sops-age-key` and `aws-credentials` mounts removed; - pod `volumes`: the two `secret` volumes removed. - The `ld-ci`/`ld-push` identities and every other volume/env are untouched; `runner-forgejo-token` (envFrom) and the Harbor robot docker-config plumbing stay exactly as they are. - `core/manifests/apps/forgejo-runner/configmap.yaml`: `runner.capacity` **2 → 1**. ## Why capacity 1 is load-bearing Host-mode jobs run as the **same uid in the same container**, so `0700` on a temp dir does not isolate one job from another — only `capacity: 1` makes the per-job copy private. Without it, #1206's "per-job" fetch is per-job in name only. ## Throughput cost: 16 → 8 slots - 8 replicas × capacity 1 = 8 concurrent job slots (was 16). - HOMELAB-2418 14-day census: measured global pool peak is **8 concurrent runs, p95 = 5, zero hours above 8**. So normal hours are unaffected; the peak hour becomes fully saturated (queueing instead of half-used headroom), and any future peak above 8 waits. - Per-pod sharing also goes away: a job now gets a whole dind (8c/8Gi) instead of ~4c/4Gi, but loses the neighbour docker-layer-cache share; each pod's own warm cache still serves its successive jobs. - A wedged job occupies a whole pod for up to `runner.timeout: 2h` instead of half of one. ## Accepted residual R25 (named, not fixed here) - The Harbor **pull-only** robot stays on disk at `/home/runner/.docker/config.json` (and dind's `/root/.docker/config.json`). The runner daemon pre-pulls images before any step runs, so it cannot become per-job. Push credentials are already per-job and never written there. - `runner-forgejo-token` (envFrom) is the runner's own API token — no runner can shed it; out of scope. ## Left for follow-up (deliberately not in this PR) - The ESO objects `ExternalSecret/runner-sops-age-key` and `ExternalSecret/runner-aws-credentials` (`core/manifests/external-secrets/es-forgejo-runner.yaml`) become unused once this rolls out; remove them in a follow-up after the rollout proof. - Narrowing the `tf-plan` policy (`path "*" { read, list, sudo }` in `prod/terraform/openbao.tf`) — noted on #1206. ## Verification done before opening - `yaml.safe_load` on both files; programmatic check that the runner container has no `sops-age-key`/`aws-credentials` volume, mount, or env, and that `runner.capacity == 1` in the rendered `config.yaml`. - Diff reviewed vs `origin/live` (merged in as a merge commit first): only the two files above change. ## Post-merge live proof (required before this is called done) 1. `kubectl -n forgejo-runner` get a rolled pod: no `sops-age-key` / `aws-credentials` volumes; runner container env without the two paths. 2. One green `plan-cluster` run on a PR whose log shows the ephemeral dir created and shredded. 3. A `k8s`-label job (e.g. `infra-core` `helm-validate`) cannot see either former path. ## Rollback Revert this PR (or set the removed blocks back + `capacity: 2`) and re-sync. **Only safe after confirming #1206 is also reverted or the prod workflows still fetch per job** — otherwise the four terraform jobs have no credentials either way. Statefully: the removed Secrets still exist (ESO objects remain), so a revert restores the mounts to working order immediately. Lab-Run: IR-20261111-r25offdisk
IR-20261111: fix(runner): drop pod-mounted sops/AWS secrets, capacity 2->1
Some checks failed
Plan failed.
CI Review / helm-validate (pull_request) Successful in 12s
Details
CI Review / runner-manifest-test (pull_request) Successful in 16s
Details
CI Review / receiver-test (pull_request) Successful in 15s
Details
CI Review / alert-receiver-test (pull_request) Successful in 32s
Details
Lint & Validate / shellcheck (pull_request) Successful in 12s
Details
Lint & Validate / yaml-lint (pull_request) Successful in 30s
Details
CI Review / record-sql-test (pull_request) Successful in 1m14s
Details
Lint & Validate / terraform-validate (pull_request) Successful in 2m45s
Details
CI Review / cockpit-test (pull_request) Successful in 3m46s
Details
CI Review / cockpit-e2e (pull_request) Successful in 3m43s
Details
CI Review / dispatcher-test (pull_request) Successful in 4m10s
Details
5b614f9c73 
R25 off disk, step 2 of 2 (lands AFTER Homelab/prod #1206):

- statefulset.yaml: remove the pod-wide sops-age-key and aws-credentials
  Secret volumes, volumeMounts, and SOPS_AGE_KEY_FILE /
  AWS_SHARED_CREDENTIALS_FILE env defaults. Both Secrets were readable by
  every host-mode job on the shared runner pool; the four prod terraform
  jobs that need them now fetch per job from OpenBao into a 0700 temp dir
  and shred at job end (prod #1206). ESO objects left in place, unused —
  follow-up removal.
- configmap.yaml: runner.capacity 2 -> 1. Host-mode jobs share uid +
  container, so only capacity 1 makes the per-job copy private. Slots
  16 -> 8; measured peak 8 / p95 5 (HOMELAB-2418), so normal hours are
  unaffected, peak hour saturates.

Lab-Run: IR-20261111-r25offdisk
atlantis commented 2026-11-11 20:11:20 +00:00
Collaborator
Copy link

Plan Error

running git clone --depth=1 --branch plane/IR-20261111-r25offdisk --single-branch https://atlantis:<redacted>@forgejo.aaron.reynoza.org/Homelab/infra-core.git /atlantis-data/repos/Homelab/infra-core/944/default: Cloning into '/atlantis-data/repos/Homelab/infra-core/944/default'...
remote: no healthy upstream
fatal: unable to access 'https://forgejo.aaron.reynoza.org/Homelab/infra-core.git/': The requested URL returned error: 503
: exit status 128
**Plan Error** ``` running git clone --depth=1 --branch plane/IR-20261111-r25offdisk --single-branch https://atlantis:<redacted>@forgejo.aaron.reynoza.org/Homelab/infra-core.git /atlantis-data/repos/Homelab/infra-core/944/default: Cloning into '/atlantis-data/repos/Homelab/infra-core/944/default'... remote: no healthy upstream fatal: unable to access 'https://forgejo.aaron.reynoza.org/Homelab/infra-core.git/': The requested URL returned error: 503 : exit status 128 ```
ld-reviewer commented 2026-11-12 02:48:36 +00:00
Collaborator
Copy link

APPROVE — ld-reviewer — head 5b614f9c73

CI on this exact head (5b614f9c73; commit date 20:09:08Z, all 12 statuses posted 20:11–20:15Z, i.e. on this sha): all 11 named workflow statuses are success — CI Review / helm-validate, runner-manifest-test, receiver-test, alert-receiver-test, record-sql-test, cockpit-test, cockpit-e2e, dispatcher-test, and Lint & Validate / shellcheck, yaml-lint, terraform-validate. The combined status for the same sha reads failure for one reason only: a single atlantis status with an EMPTY context (Plan failed., target_url empty) posted 20:11:19Z. That same empty-context atlantis status also sits on live-history commit a59be8e6bc while the workflow statuses on that commit are green, so it is a recurring atlantis status posting rather than something produced by this diff, and it carries no plan output to inspect. I state that row explicitly instead of folding it into a "green" claim.

Scope against base 3563ec7f: the cumulative diff is exactly two files, +30/-28 — core/manifests/apps/forgejo-runner/statefulset.yaml (+21/-22) and core/manifests/apps/forgejo-runner/configmap.yaml (+9/-6). The blob shas in the diff index (6469ebd9..88454095, 478e40c2..3cb4fc27) match the files fetched at both refs. No RBAC, no CNP, no ld-pool statefulsets or configmap, no workflow file; neither ld-ci nor ld-push appears anywhere in the diff.

statefulset.yaml — the removals are exactly the brief's list and nothing else. I loaded base and head with yaml.safe_load and deep-compared the documents: after removing the two Secret volumes (sops-age-key, aws-credentials) from the pod and the env/volumeMounts keys from the runner container, base and head are equal. So resources (requests 500m/1003Mi/10Gi, limits 2/2560Mi/14Gi), image digest a16f937f…, command/args, envFrom: runner-forgejo-token, every remaining env var, every remaining volume and mount, the dind container, both initContainers (register, docker-config) and the Harbor robot pull plumbing are byte-identical to base. Env removed: SOPS_AGE_KEY_FILE, AWS_SHARED_CREDENTIALS_FILE. Mounts removed: those two, no others. Volumes removed: those two Secrets, no others. Neither the runner nor dind container has liveness/readiness probes in either revision, so that item is unchanged rather than regressed. Pod securityContext (fsGroup 1000), affinity and topologySpreadConstraints are unchanged.

configmap.yaml — I rendered config.yaml from both sides and flattened it: runner.capacity 2 → 1 is the only value that changed; runner.timeout stays 2h, and the cache, host, log, runner sections are otherwise identical key-for-key. The only non-comment line change in the rendered document is capacity: 2 → capacity: 1; the rest of the ConfigMap diff is comment text. 1 × 8 replicas = 8 slots against the stated HOMELAB-2418 census (peak 8, p95 5) is arithmetically consistent, and the comment says plainly that the peak hour saturates and queues instead of claiming headroom it does not have.

Consumers of the removed material, checked at head rather than assumed: core/manifests/external-secrets/es-forgejo-runner.yaml still defines runner-sops-age-key and runner-aws-credentials, so the diff's "left in place, unused, follow-up" note is accurate; the Secrets that stay consumed — runner-forgejo-token (envFrom) and runner-harbor-robot (dind pre-pull) — are still defined there. All 19 files in .forgejo/workflows at head contain no reference to sops, SOPS_AGE, AWS_SHARED_CREDENTIALS, .aws/credentials or age/keys, so no pipeline in this repo reads the two removed paths; the in-repo blast radius of the removal is empty.

Landing preconditions, stated plainly so they are not lost: (1) Homelab/prod #1206 is still open, not merged, at the time of this review — the comment's "the four prod jobs now fetch each key from OpenBao per job" becomes true only when #1206 merges, and the land order named in this PR's title (prod #1206 first) is the binding one; merging this while #1206 is unmerged removes the only credentials path the prod terraform jobs have. (2) Post-merge live proof still has to be produced: new pod shape, green plan-cluster with the create+shred visible in its log, and a k8s job unable to see the former paths.

Two notes for follow-up, neither part of this diff: the fsGroup: 1000 comment at the pod securityContext (statefulset.yaml ~L105) still gives the removed mode-0440 sops/AWS mounts as its reason and is untouched here — its rewrite belongs with the ESO cleanup. And the head's shape differs from the brief: 5b614f9c has a single parent (f72e94c2, Merge branch 'live' into plane/IR-20261111-r25offdisk, which brings in base 3563ec7f) rather than being the live merge itself; the content claim the brief makes still holds, since the head contains base 3563ec7f and the net diff against it is the two files above.

Approving on the change's own merits: it closes a pod-wide credential exposure that any host-mode job from any repo on this pool could read, keeps the capacity-1 justification in the same commit where it is required, and confines itself to the two files it names.

model: omniroute/deepseek-flash
session: IR-20261111-r25offdisk-seatB-20261112T0244Z

**APPROVE — ld-reviewer — head 5b614f9c731b7f68e87bd7383e42da6f257a7b84** CI on this exact head (5b614f9c731b7f68e87bd7383e42da6f257a7b84; commit date 20:09:08Z, all 12 statuses posted 20:11–20:15Z, i.e. on this sha): all 11 named workflow statuses are `success` — `CI Review / helm-validate`, `runner-manifest-test`, `receiver-test`, `alert-receiver-test`, `record-sql-test`, `cockpit-test`, `cockpit-e2e`, `dispatcher-test`, and `Lint & Validate / shellcheck`, `yaml-lint`, `terraform-validate`. The combined status for the same sha reads `failure` for one reason only: a single atlantis status with an EMPTY context (`Plan failed.`, `target_url` empty) posted 20:11:19Z. That same empty-context atlantis status also sits on live-history commit a59be8e6bc4c while the workflow statuses on that commit are green, so it is a recurring atlantis status posting rather than something produced by this diff, and it carries no plan output to inspect. I state that row explicitly instead of folding it into a "green" claim. Scope against base 3563ec7f: the cumulative diff is exactly two files, +30/-28 — `core/manifests/apps/forgejo-runner/statefulset.yaml` (+21/-22) and `core/manifests/apps/forgejo-runner/configmap.yaml` (+9/-6). The blob shas in the diff index (6469ebd9..88454095, 478e40c2..3cb4fc27) match the files fetched at both refs. No RBAC, no CNP, no ld-pool statefulsets or configmap, no workflow file; neither `ld-ci` nor `ld-push` appears anywhere in the diff. statefulset.yaml — the removals are exactly the brief's list and nothing else. I loaded base and head with yaml.safe_load and deep-compared the documents: after removing the two Secret volumes (`sops-age-key`, `aws-credentials`) from the pod and the `env`/`volumeMounts` keys from the runner container, base and head are equal. So resources (requests 500m/1003Mi/10Gi, limits 2/2560Mi/14Gi), image digest `a16f937f…`, command/args, `envFrom: runner-forgejo-token`, every remaining env var, every remaining volume and mount, the `dind` container, both initContainers (`register`, `docker-config`) and the Harbor robot pull plumbing are byte-identical to base. Env removed: `SOPS_AGE_KEY_FILE`, `AWS_SHARED_CREDENTIALS_FILE`. Mounts removed: those two, no others. Volumes removed: those two Secrets, no others. Neither the runner nor dind container has liveness/readiness probes in either revision, so that item is unchanged rather than regressed. Pod securityContext (fsGroup 1000), affinity and topologySpreadConstraints are unchanged. configmap.yaml — I rendered `config.yaml` from both sides and flattened it: `runner.capacity` 2 → 1 is the only value that changed; `runner.timeout` stays 2h, and the `cache`, `host`, `log`, `runner` sections are otherwise identical key-for-key. The only non-comment line change in the rendered document is `capacity: 2` → `capacity: 1`; the rest of the ConfigMap diff is comment text. 1 × 8 replicas = 8 slots against the stated HOMELAB-2418 census (peak 8, p95 5) is arithmetically consistent, and the comment says plainly that the peak hour saturates and queues instead of claiming headroom it does not have. Consumers of the removed material, checked at head rather than assumed: `core/manifests/external-secrets/es-forgejo-runner.yaml` still defines `runner-sops-age-key` and `runner-aws-credentials`, so the diff's "left in place, unused, follow-up" note is accurate; the Secrets that stay consumed — `runner-forgejo-token` (envFrom) and `runner-harbor-robot` (dind pre-pull) — are still defined there. All 19 files in `.forgejo/workflows` at head contain no reference to `sops`, `SOPS_AGE`, `AWS_SHARED_CREDENTIALS`, `.aws/credentials` or `age/keys`, so no pipeline in this repo reads the two removed paths; the in-repo blast radius of the removal is empty. Landing preconditions, stated plainly so they are not lost: (1) Homelab/prod #1206 is still open, not merged, at the time of this review — the comment's "the four prod jobs now fetch each key from OpenBao per job" becomes true only when #1206 merges, and the land order named in this PR's title (prod #1206 first) is the binding one; merging this while #1206 is unmerged removes the only credentials path the prod terraform jobs have. (2) Post-merge live proof still has to be produced: new pod shape, green `plan-cluster` with the create+shred visible in its log, and a `k8s` job unable to see the former paths. Two notes for follow-up, neither part of this diff: the `fsGroup: 1000` comment at the pod securityContext (statefulset.yaml ~L105) still gives the removed mode-0440 sops/AWS mounts as its reason and is untouched here — its rewrite belongs with the ESO cleanup. And the head's shape differs from the brief: 5b614f9c has a single parent (f72e94c2, `Merge branch 'live' into plane/IR-20261111-r25offdisk`, which brings in base 3563ec7f) rather than being the live merge itself; the content claim the brief makes still holds, since the head contains base 3563ec7f and the net diff against it is the two files above. Approving on the change's own merits: it closes a pod-wide credential exposure that any host-mode job from any repo on this pool could read, keeps the capacity-1 justification in the same commit where it is required, and confines itself to the two files it names. model: omniroute/deepseek-flash session: IR-20261111-r25offdisk-seatB-20261112T0244Z
ld-reviewer commented 2026-11-12 02:51:44 +00:00
Collaborator
Copy link

REQUEST CHANGES — ld-reviewer — head 5b614f9c73

The two files are right. What stops me is this PR's own landing precondition, which is not met, plus a non-green exact head. Details, in the order I checked them.

SCOPE — verified from the trees, not from the description. I compared the complete recursive tree of this head (2690 entries) with the complete recursive tree of the current live tip 3563ec7fad (2690 entries). The only differing leaf entries in the entire repository are core/manifests/apps/forgejo-runner/statefulset.yaml and core/manifests/apps/forgejo-runner/configmap.yaml. ciliumnetworkpolicy.yaml, ld-pool-statefulsets.yaml and rbac.yaml are byte-identical between live tip and this head — the merge of live into the branch (f72e94c25f, parents baa20941c3 + 3563ec7f) left them untouched. No RBAC, no CNP, no ld-pool, no workflow changes.

STATEFULSET — exactly the three removals briefed:

  • runner env: base had 5 entries including SOPS_AGE_KEY_FILE and AWS_SHARED_CREDENTIALS_FILE; head has 3 (DOCKER_HOST, GIT_HTTP_LOW_SPEED_LIMIT, GIT_HTTP_LOW_SPEED_TIME).
  • runner volumeMounts: runner-data, runner-config, workspace, docker-config — the sops-age-key and aws-credentials mounts are gone.
  • pod volumes: runner-config, runner-secret, workspace, docker-config, harbor-robot — the two secret volumes are gone.
  • No active (non-comment) line in the head file mentions either variable, either volume name or either Secret name; every remaining occurrence sits inside an R25 comment block.
  • Everything else is unchanged: I built both revisions to objects client-side and compared them with runner.env, runner.volumeMounts and pod.volumes removed — identical. runner-forgejo-token envFrom, both images by digest, initContainers (register, docker-config), dind and its mounts, resources and limits (runner 500m/1003Mi -> 2/2560Mi/14Gi, dind unchanged), 8 replicas, RollingUpdate. (Neither revision declares a probe on the runner container, so there was nothing there to alter.)

CONFIGMAP — I extracted the rendered config.yaml from both revisions; the only differing leaf in the whole document is runner.capacity: 2 -> 1, one capacity: occurrence only, runner.timeout still 2h, same top-level sections. The old HOMELAB-1634 block is replaced by an R25 comment carrying the HOMELAB-2418 numbers (peak 8, p95 5). Both files parse and build.

ld-ci / ld-push — the names appear nowhere in the diff or in either head file, and they are separate StatefulSets with their own configmaps (ld-ci-runner-config, ld-push-runner-config, both already capacity 1), so this change does not reach them.

THE PROBLEM — the ordered landing precondition. This PR must land after Homelab/prod #1206, and #1206 is open, unmerged as I write this. It is not an administrative detail:

  • On prod live at 3185b789f5580efbf31693219ecba4003ab46f69 the four terraform jobs still read exactly the material this diff removes: SOPS_AGE_KEY_FILE=/home/runner/.config/sops/age/keys.txt and AWS_SHARED_CREDENTIALS_FILE=/home/runner/.aws/credentials in infra-plan.yaml (plan-cluster, plan-zitadel), infra-apply.yaml (apply) and infra-apply-cluster.yaml (apply-cluster) — all four runs-on: infra, and this StatefulSet registers infra:host,k8s:host, i.e. host-mode jobs in exactly this container.
  • I scanned every repo visible to me (9) for these variables and paths: prod's three workflow files are the only consumers anywhere. So #1206 is the whole dependency, and it is outstanding. Landing this head today takes the age key and the AWS credentials out from under four prod jobs before their replacement exists. Merging this first breaks every terraform plan/apply, as the ticket itself says.
  • I read #1206's diff to confirm the replacement is real work and not a placeholder: per-job OpenBao fetch into a mktemp -d chmod 700 directory, 0600 files, shred with EXIT/INT/TERM traps, and the two variables exported by the fetch step. It is real, and it is not merged.

SECOND PROBLEM — the exact head is not green as a whole. All 11 named pipeline contexts are success on 5b614f9c (CI Review: helm-validate, runner-manifest-test, receiver-test, alert-receiver-test, record-sql-test, cockpit-test, cockpit-e2e, dispatcher-test; Lint & Validate: shellcheck, yaml-lint, terraform-validate), and the two Actions pull_request runs on this commit report success. The combined commit status at 2026-11-12T02:51Z is nevertheless failure, because of a single status posted by the atlantis account with an EMPTY context, no target URL, and the description "Plan failed." (2026-11-11T20:11:19Z, 24s after the PR opened). It is not one of the pipeline contexts and I cannot attribute it from this seat — ld-reviewer has no hook or admin visibility on this repo, so the plan output is unreachable to me. The same status appears on an earlier branch commit of this workstream (a59be8e6bc), so the pattern is not new, but a "Plan failed." on the exact head is not something I will quietly certify away.

WHAT CLEARS THIS: prod #1206 merged, then a fresh verdict on whatever head the branch has at that point — the two files re-check in minutes, and nothing in them needs rework. Separately, either that atlantis status being re-run to success, or someone with real visibility on it stating what it planned and why it failed and that it does not touch this diff, so the exact head is unambiguously green.

The acknowledged residuals check out and I agree with them: the Harbor pull robot still reaches the runner through the docker-config initContainer's emptyDir at /home/runner/.docker; runner-forgejo-token envFrom is untouched; the ESO objects for the two removed Secrets stay in place unused. Capacity 1 for the stated reason — host-mode jobs share uid and container, so a 0700 per-job directory does not isolate co-located jobs — matches what the manifests actually show, and 8 pods x 1 slot against a measured peak of 8 leaves no headroom at the peak hour, which the comment says plainly.

model: omniroute/deepseek-flash
session: IR-20261111-r25offdisk-seatA-20261112T0244Z

**REQUEST CHANGES — ld-reviewer — head 5b614f9c731b7f68e87bd7383e42da6f257a7b84** The two files are right. What stops me is this PR's own landing precondition, which is not met, plus a non-green exact head. Details, in the order I checked them. SCOPE — verified from the trees, not from the description. I compared the complete recursive tree of this head (2690 entries) with the complete recursive tree of the current `live` tip 3563ec7fad6fea09940a1ffeb523d2ce40e01c01 (2690 entries). The only differing leaf entries in the entire repository are `core/manifests/apps/forgejo-runner/statefulset.yaml` and `core/manifests/apps/forgejo-runner/configmap.yaml`. `ciliumnetworkpolicy.yaml`, `ld-pool-statefulsets.yaml` and `rbac.yaml` are byte-identical between live tip and this head — the merge of live into the branch (f72e94c25f52e0504e9e5edfcbde35cfe2f48555, parents baa20941c3 + 3563ec7f) left them untouched. No RBAC, no CNP, no ld-pool, no workflow changes. STATEFULSET — exactly the three removals briefed: - runner `env`: base had 5 entries including SOPS_AGE_KEY_FILE and AWS_SHARED_CREDENTIALS_FILE; head has 3 (DOCKER_HOST, GIT_HTTP_LOW_SPEED_LIMIT, GIT_HTTP_LOW_SPEED_TIME). - runner `volumeMounts`: runner-data, runner-config, workspace, docker-config — the sops-age-key and aws-credentials mounts are gone. - pod `volumes`: runner-config, runner-secret, workspace, docker-config, harbor-robot — the two `secret` volumes are gone. - No active (non-comment) line in the head file mentions either variable, either volume name or either Secret name; every remaining occurrence sits inside an R25 comment block. - Everything else is unchanged: I built both revisions to objects client-side and compared them with runner.env, runner.volumeMounts and pod.volumes removed — identical. `runner-forgejo-token` envFrom, both images by digest, initContainers (register, docker-config), dind and its mounts, resources and limits (runner 500m/1003Mi -> 2/2560Mi/14Gi, dind unchanged), 8 replicas, RollingUpdate. (Neither revision declares a probe on the runner container, so there was nothing there to alter.) CONFIGMAP — I extracted the rendered `config.yaml` from both revisions; the only differing leaf in the whole document is `runner.capacity: 2 -> 1`, one `capacity:` occurrence only, `runner.timeout` still 2h, same top-level sections. The old HOMELAB-1634 block is replaced by an R25 comment carrying the HOMELAB-2418 numbers (peak 8, p95 5). Both files parse and build. ld-ci / ld-push — the names appear nowhere in the diff or in either head file, and they are separate StatefulSets with their own configmaps (`ld-ci-runner-config`, `ld-push-runner-config`, both already capacity 1), so this change does not reach them. THE PROBLEM — the ordered landing precondition. This PR must land after Homelab/prod #1206, and #1206 is open, unmerged as I write this. It is not an administrative detail: - On prod `live` at 3185b789f5580efbf31693219ecba4003ab46f69 the four terraform jobs still read exactly the material this diff removes: SOPS_AGE_KEY_FILE=/home/runner/.config/sops/age/keys.txt and AWS_SHARED_CREDENTIALS_FILE=/home/runner/.aws/credentials in `infra-plan.yaml` (plan-cluster, plan-zitadel), `infra-apply.yaml` (apply) and `infra-apply-cluster.yaml` (apply-cluster) — all four `runs-on: infra`, and this StatefulSet registers `infra:host,k8s:host`, i.e. host-mode jobs in exactly this container. - I scanned every repo visible to me (9) for these variables and paths: prod's three workflow files are the only consumers anywhere. So #1206 is the whole dependency, and it is outstanding. Landing this head today takes the age key and the AWS credentials out from under four prod jobs before their replacement exists. Merging this first breaks every terraform plan/apply, as the ticket itself says. - I read #1206's diff to confirm the replacement is real work and not a placeholder: per-job OpenBao fetch into a `mktemp -d` chmod 700 directory, 0600 files, `shred` with EXIT/INT/TERM traps, and the two variables exported by the fetch step. It is real, and it is not merged. SECOND PROBLEM — the exact head is not green as a whole. All 11 named pipeline contexts are success on 5b614f9c (CI Review: helm-validate, runner-manifest-test, receiver-test, alert-receiver-test, record-sql-test, cockpit-test, cockpit-e2e, dispatcher-test; Lint & Validate: shellcheck, yaml-lint, terraform-validate), and the two Actions pull_request runs on this commit report success. The combined commit status at 2026-11-12T02:51Z is nevertheless `failure`, because of a single status posted by the `atlantis` account with an EMPTY context, no target URL, and the description "Plan failed." (2026-11-11T20:11:19Z, 24s after the PR opened). It is not one of the pipeline contexts and I cannot attribute it from this seat — ld-reviewer has no hook or admin visibility on this repo, so the plan output is unreachable to me. The same status appears on an earlier branch commit of this workstream (a59be8e6bc), so the pattern is not new, but a "Plan failed." on the exact head is not something I will quietly certify away. WHAT CLEARS THIS: prod #1206 merged, then a fresh verdict on whatever head the branch has at that point — the two files re-check in minutes, and nothing in them needs rework. Separately, either that atlantis status being re-run to success, or someone with real visibility on it stating what it planned and why it failed and that it does not touch this diff, so the exact head is unambiguously green. The acknowledged residuals check out and I agree with them: the Harbor pull robot still reaches the runner through the docker-config initContainer's emptyDir at /home/runner/.docker; `runner-forgejo-token` envFrom is untouched; the ESO objects for the two removed Secrets stay in place unused. Capacity 1 for the stated reason — host-mode jobs share uid and container, so a 0700 per-job directory does not isolate co-located jobs — matches what the manifests actually show, and 8 pods x 1 slot against a measured peak of 8 leaves no headroom at the peak hour, which the comment says plainly. model: omniroute/deepseek-flash session: IR-20261111-r25offdisk-seatA-20261112T0244Z
IR-20261111: merge origin/live into plane/IR-20261111-r25offdisk (precondition #1206 landed)
All checks were successful
CI Review / receiver-test (pull_request) Successful in 5s
Details
CI Review / helm-validate (pull_request) Successful in 9s
Details
CI Review / runner-manifest-test (pull_request) Successful in 10s
Details
CI Review / alert-receiver-test (pull_request) Successful in 20s
Details
Lint & Validate / shellcheck (pull_request) Successful in 5s
Details
Lint & Validate / yaml-lint (pull_request) Successful in 23s
Details
CI Review / record-sql-test (pull_request) Successful in 56s
Details
0/0 projects policies checked successfully.
Lint & Validate / terraform-validate (pull_request) Successful in 2m13s
Details
CI Review / cockpit-e2e (pull_request) Successful in 2m44s
Details
CI Review / cockpit-test (pull_request) Successful in 2m54s
Details
CI Review / dispatcher-test (pull_request) Successful in 3m5s
Details
e84b4436b3 
#1206 landed in prod at 06:15Z (prod live 9597e6f7), so this PR's own stated
precondition — land AFTER #1206 — is now true. infra-core live has since moved
through #946/#948/#949/#952/#954, so the branch head (5b614f9c, held by seat A
for exactly that reason) is merged up to origin/live 491c9c02. Nothing lands
with this commit; fresh review seats follow.

CONFLICT — core/manifests/apps/forgejo-runner/configmap.yaml (content, one
hunk). Both sides had already made the same change (capacity 1 from
IR-20261111-r25offdisk, landed on live via #948); the only difference was one
explanatory comment line that live carries:

    # pulled forward from #944 after two e2e jobs timed out sharing runner-0 (11-11)

Resolved by taking LIVE's file verbatim (git checkout --theirs; blob
d6abda22, byte-identical to origin/live). Nothing from the branch's version of
this file was needed, since live is a strict superset — #944's capacity 2 -> 1
is already live via #948, so the PR's remaining substance is the R25 secret
removal.

statefulset.yaml auto-merged: live's two hunks (the 2026-11-11/11-12 replica
comment block, and prod-wk-01 removed from the runner affinity In list plus an
explicit NotIn) combine cleanly with this branch's R25 removal, which is
untouched. Verified by diff: merged statefulset vs origin/live differs ONLY by
the three R25 deletion hunks, and vs the branch head differs ONLY by live's
two hunks.

INVARIANTS AT THE MERGED HEAD
1. runner.capacity: 1 in configmap.yaml (line 40).
2. spec.replicas: 8 in statefulset.yaml (#954's 9 -> 8; 9 not resurrected).
3. dind-gc RUNNER_COUNT == 8 in BOTH dind-gc-cronjob.yaml env and the
   dind-gc-script.yaml default, matching replicas.
4. #944's purpose intact: no sops-age-key / aws-credentials volume,
   volumeMount, or SOPS_AGE_KEY_FILE / AWS_SHARED_CREDENTIALS_FILE env in the
   statefulset. The five remaining volumes are live's seven minus the two
   removed ones (runner-config, runner-secret, workspace, docker-config,
   harbor-robot) — every other volume and env entry unchanged.
5. Local suites: test_runner_dind_gc_tightening.py, test_runner_ephemeral_ceilings.py
   (both plain and --self-check) PASS; test_runner_placement_prep.py and
   test_sbx_npm_cache.py (which live's #952 added) also PASS. The brief's third
   named suite, test_vector_config_checksum.py, exists on no ref of this repo at
   this head (it belongs to an unrelated in-flight branch) and so is out of
   scope for this change, not failing.
6. The runner directory has no kustomization.yaml (pre-existing on live too), so
   `kubectl kustomize core/manifests/apps/forgejo-runner` cannot run; verified
   equivalently by parsing all 26 manifests (29 docs, no parse errors) and
   asserting on the forgejo-runner StatefulSet object: replicas 8, five volumes,
   no runner-sops-age-key / runner-aws-credentials, no forbidden env or mounts.

Lab-Run: IR-20261111-r25offdisk
pi-landing commented 2026-11-12 06:30:28 +00:00
Author
Collaborator
Copy link

atlantis plan

atlantis plan
atlantis commented 2026-11-12 06:30:35 +00:00
Collaborator
Copy link

Ran Plan for 0 projects:

Ran Plan for 0 projects:
ld-reviewer commented 2026-11-12 06:53:17 +00:00
Collaborator
Copy link

APPROVE — ld-reviewer — head e84b4436b3

1. PRECONDITION — MET. GET /repos/Homelab/prod/pulls/1206 → merged: true, merged_at 2026-11-12T06:04:20Z, merge_commit_sha 9597e6f7a2dd…, head 6a89659426e2…. In prod's graph that merge commit's second parent IS 6a896594, and prod live is now 4087d04beac1…, whose FIRST parent is 9597e6f7. So 6a896594 is an ancestor of prod live: #1206 is landed and its head is contained. The stated order rule is satisfied. (Nit, not a finding: the body says "landed at 06:15Z"; the API says 06:04:20Z. Substance — merged, head in live — is true either way.)

2. SCOPE at head vs live — exactly the R25 removal, one file. git diff 491c9c02 e84b4436 (491c9c02 is live at merge time, i.e. the true base) = 1 file, core/manifests/apps/forgejo-runner/statefulset.yaml, 21+/22−, three hunks and nothing else:

  • runner container env: SOPS_AGE_KEY_FILE and AWS_SHARED_CREDENTIALS_FILE gone (R25 comment left in their place, lines 276–284);
  • runner volumeMounts: sops-age-key (/home/runner/.config/sops/age) and aws-credentials (/home/runner/.aws) mounts gone;
  • pod volumes: the two secret volumes (runner-sops-age-key, runner-aws-credentials) gone.

Five volumes legitimately REMAIN: runner-config, runner-secret, workspace, docker-config, harbor-robot (the accepted residual R25 pull-only robot). envFrom: secretRef runner-forgejo-token is intact at head, and no other container (register init, dind) is touched — the diff has no hunk outside those three regions. configmap.yaml at head is blob d6abda22, byte-identical to origin/live. grep for the forbidden names in the runner dir at head returns comments only.

3. Nothing regressed. spec.replicas: 8 (statefulset.yaml:125; #954's 9→8 not resurrected), capacity: 1 (configmap.yaml:40), and dind-gc RUNNER_COUNT = 8 in BOTH the CronJob env (dind-gc-cronjob.yaml:145-146 value: "8") and the script default (dind-gc-script.yaml:93 RUNNER_COUNT="${RUNNER_COUNT:-8}"), matching replicas. All three equal origin/live at head.

4. Merge, not rewrite, and nothing smuggled by the resolution. Two parents: 5b614f9c731b… (the branch's old head = parent 1, so ancestry is intact and the pre-merge head is contained) and 491c9c02d50c… (an origin/live commit — ancestor of live, 3 commits behind its tip). I re-ran the merge independently: git merge-tree --write-tree 5b614f9c 491c9c02 produces a tree that differs from e84b4436 in configmap.yaml only — i.e. every other path in the merge commit is exactly what a plain 3-way merge of the two parents yields, so the human resolution touched nothing but the one conflict. That conflict is real (three distinct stages 478e40c2/3cb4fc27/d6abda22) and was resolved to live's blob d6abda22 verbatim; live's version is the branch's version plus one comment line, so nothing from the branch was dropped. R25 lines are identical between the branch head and the merged head (git diff 5b614f9c e84b4436 has no sops/aws line). Message's last trailer is Lab-Run: IR-20261111-r25offdisk (last non-empty line; no other trailers).

5. Claim honesty — accurate on substance, two stale lines. The post-merge note describes what happened: capacity: 1 is already live via #948 (pulls/948 merged: "capacity 2 -> 1, pulled forward from #944"), the only conflict was configmap.yaml, and this PR's remaining substance is the R25 secret removal — all of that matches the diff, and live really is a strict superset of the branch's configmap. Not overstated. Two leftovers to know about, neither misleading on its own: the original "What changes (2 files, nothing else)" and the rollback line "…+ capacity: 2" are stale at this head — configmap.yaml no longer differs from live, so the change is one file and the rollback is the statefulset hunks alone. Please read the post-merge note, not the original section, for scope.

6. Tests — the job that covers this ran on this exact head and passed. Commit status for e84b4436…: 12 entries, state: success, all created 06:28–06:31Z by the runs triggered by the merge push (runs 3673 "CI Review", 3674 "Lint & Validate"). Specifically CI Review / runner-manifest-test (pull_request) — success, which is the job that runs scripts/manifests/tests/test_runner_ephemeral_ceilings.py and test_runner_dind_gc_tightening.py (both plain and --self-check) plus test_sbx_npm_cache.py; Lint & Validate / yaml-lint and helm-validate also success. Those suites do not assert the presence of the two removed secret volumes (no test references sops-age/AWS), so the removal is not a test regression by construction, and the manifest-parse/ephemeral-storage assertions passed on the merged tree. I did not re-run the suites locally (seat rules).

Observation, non-blocking: git diff origin/live e84b4436 additionally shows live's own post-merge #947 sendrast doc commits (962db01f, 2f1be91f, 2d837e9b — 5 files under core/directors/sendrast/) as reversions. They landed on live AFTER this merge (491c9c02..origin/live touches those 5 files only, nothing runner-related); landing this head onto live keeps them. So the true PR content is the 3 R25 hunks, as stated.

Disclosed follow-ups (ESO object cleanup, tf-plan policy narrowing, Harbor pull robot) are post-landing, as scoped.

model: omniroute/deepseek-flash
session: IR-20261111-r25offdisk-remerge-seatA-20261112T0645Z

**APPROVE — ld-reviewer — head e84b4436b389ca2b071728109addafc0f2aa03e1** **1. PRECONDITION — MET.** `GET /repos/Homelab/prod/pulls/1206` → `merged: true`, `merged_at 2026-11-12T06:04:20Z`, `merge_commit_sha 9597e6f7a2dd…`, `head 6a89659426e2…`. In prod's graph that merge commit's second parent IS `6a896594`, and prod `live` is now `4087d04beac1…`, whose FIRST parent is `9597e6f7`. So `6a896594` is an ancestor of prod `live`: #1206 is landed and its head is contained. The stated order rule is satisfied. (Nit, not a finding: the body says "landed at 06:15Z"; the API says 06:04:20Z. Substance — merged, head in `live` — is true either way.) **2. SCOPE at head vs live — exactly the R25 removal, one file.** `git diff 491c9c02 e84b4436` (491c9c02 is live at merge time, i.e. the true base) = **1 file, `core/manifests/apps/forgejo-runner/statefulset.yaml`, 21+/22−, three hunks and nothing else**: - `runner` container `env`: `SOPS_AGE_KEY_FILE` and `AWS_SHARED_CREDENTIALS_FILE` gone (R25 comment left in their place, lines 276–284); - `runner` `volumeMounts`: `sops-age-key` (`/home/runner/.config/sops/age`) and `aws-credentials` (`/home/runner/.aws`) mounts gone; - pod `volumes`: the two `secret` volumes (`runner-sops-age-key`, `runner-aws-credentials`) gone. Five volumes legitimately REMAIN: `runner-config`, `runner-secret`, `workspace`, `docker-config`, `harbor-robot` (the accepted residual R25 pull-only robot). `envFrom: secretRef runner-forgejo-token` is intact at head, and no other container (`register` init, `dind`) is touched — the diff has no hunk outside those three regions. `configmap.yaml` at head is blob `d6abda22`, byte-identical to `origin/live`. `grep` for the forbidden names in the runner dir at head returns comments only. **3. Nothing regressed.** `spec.replicas: 8` (statefulset.yaml:125; #954's 9→8 not resurrected), `capacity: 1` (configmap.yaml:40), and dind-gc `RUNNER_COUNT` = `8` in BOTH the CronJob env (`dind-gc-cronjob.yaml:145-146 value: "8"`) and the script default (`dind-gc-script.yaml:93 RUNNER_COUNT="${RUNNER_COUNT:-8}"`), matching replicas. All three equal `origin/live` at head. **4. Merge, not rewrite, and nothing smuggled by the resolution.** Two parents: `5b614f9c731b…` (the branch's old head = parent 1, so ancestry is intact and the pre-merge head is contained) and `491c9c02d50c…` (an `origin/live` commit — ancestor of live, 3 commits behind its tip). I re-ran the merge independently: `git merge-tree --write-tree 5b614f9c 491c9c02` produces a tree that differs from `e84b4436` in **`configmap.yaml` only** — i.e. every other path in the merge commit is exactly what a plain 3-way merge of the two parents yields, so the human resolution touched nothing but the one conflict. That conflict is real (three distinct stages 478e40c2/3cb4fc27/d6abda22) and was resolved to live's blob `d6abda22` verbatim; live's version is the branch's version plus one comment line, so nothing from the branch was dropped. R25 lines are identical between the branch head and the merged head (`git diff 5b614f9c e84b4436` has no sops/aws line). Message's last trailer is `Lab-Run: IR-20261111-r25offdisk` (last non-empty line; no other trailers). **5. Claim honesty — accurate on substance, two stale lines.** The post-merge note describes what happened: `capacity: 1` is **already live via #948** (`pulls/948` merged: "capacity 2 -> 1, pulled forward from #944"), the only conflict was `configmap.yaml`, and this PR's remaining substance is the R25 secret removal — all of that matches the diff, and live really is a strict superset of the branch's configmap. Not overstated. Two leftovers to know about, neither misleading on its own: the original "**What changes (2 files, nothing else)**" and the rollback line "**…+ capacity: 2**" are stale at this head — `configmap.yaml` no longer differs from live, so the change is one file and the rollback is the statefulset hunks alone. Please read the post-merge note, not the original section, for scope. **6. Tests — the job that covers this ran on this exact head and passed.** Commit status for `e84b4436…`: 12 entries, `state: success`, all created 06:28–06:31Z by the runs triggered by the merge push (runs 3673 "CI Review", 3674 "Lint & Validate"). Specifically **`CI Review / runner-manifest-test (pull_request) — success`**, which is the job that runs `scripts/manifests/tests/test_runner_ephemeral_ceilings.py` and `test_runner_dind_gc_tightening.py` (both plain and `--self-check`) plus `test_sbx_npm_cache.py`; `Lint & Validate / yaml-lint` and `helm-validate` also success. Those suites do not assert the presence of the two removed secret volumes (no test references sops-age/AWS), so the removal is not a test regression by construction, and the manifest-parse/ephemeral-storage assertions passed on the merged tree. I did not re-run the suites locally (seat rules). **Observation, non-blocking:** `git diff origin/live e84b4436` additionally shows live's own post-merge #947 sendrast doc commits (`962db01f`, `2f1be91f`, `2d837e9b` — 5 files under `core/directors/sendrast/`) as reversions. They landed on live AFTER this merge (`491c9c02..origin/live` touches those 5 files only, nothing runner-related); landing this head onto live keeps them. So the true PR content is the 3 R25 hunks, as stated. Disclosed follow-ups (ESO object cleanup, `tf-plan` policy narrowing, Harbor pull robot) are post-landing, as scoped. model: omniroute/deepseek-flash session: IR-20261111-r25offdisk-remerge-seatA-20261112T0645Z
ld-reviewer commented 2026-11-12 06:55:35 +00:00
Collaborator
Copy link

APPROVE — ld-reviewer — head e84b4436b3

Read-only seat B. Judged the head against origin/live and inspected the merge commit itself (fresh bare clone of Homelab/infra-core + API reads; no cluster commands, no test harness).

1. Precondition — MET. Homelab/prod #1206 is merged=true, merged_at 2026-11-12T06:04:20Z; its merge commit 9597e6f7 has parents 3185b789 + the PR's own head 6a896594, and it is in prod live (live has since moved to 4087d04b via #1213; 9597e6f7 is still on the branch). I also checked the substance of the ordering: at prod live, infra-plan.yaml, infra-apply.yaml and infra-apply-cluster.yaml each fetch sops/AWS per job into mktemp -d "$TMPBASE/r25-secrets.XXXXXX" (chmod 700, 0600 files, shred on exit), so the pod mounts this PR deletes are no longer any job's only credential source.

2. Scope at head vs origin/live — exactly the R25 removal. diff 491c9c02(live at merge)..e84b4436 touches ONE file, core/manifests/apps/forgejo-runner/statefulset.yaml, in 3 hunks: the SOPS_AGE_KEY_FILE / AWS_SHARED_CREDENTIALS_FILE env pair, the two volumeMounts (/home/runner/.config/sops/age, /home/runner/.aws), and the two secret volumes (runner-sops-age-key, runner-aws-credentials). Those hunk lines are identical to the branch's own pre-merge change (diffed against 3563ec7f..5b614f9c), so the merge resolved the file rather than rewriting it. Volumes REMAINING (5): runner-config (configMap forgejo-runner-config), runner-secret (secret forgejo-runner-secret), workspace (emptyDir 10Gi), docker-config (emptyDir), harbor-robot (secret runner-harbor-robot, defaultMode 0440) — i.e. live's seven minus the two removed; the runner-data/docker-data volumeClaimTemplates, env POD_NAME, DOCKER_HOST, GIT_HTTP_LOW_SPEED_LIMIT/TIME and envFrom: runner-forgejo-token are untouched. configmap.yaml, dind-gc-cronjob.yaml, dind-gc-script.yaml and es-forgejo-runner.yaml are byte-identical to live (sha256 match), so nothing was smuggled in and nothing else moved. No infra-core workflow referenced the removed paths, so this repo's own CI loses no credential.

3. Nothing regressed by the merge. replicas: 8 (statefulset:125, not 9); capacity: 1 (configmap:40); dind-gc RUNNER_COUNT is "8" in the CronJob env and RUNNER_COUNT="${RUNNER_COUNT:-8}" as the script default. Live's affinity rework survived intact: In [prod-wk-02, prod-wk-05, prod-wk-06] plus an explicit NotIn [prod-wk-01] in the same nodeSelectorTerm.

4. It is a merge, not a rewrite. Parents are 5b614f9c (the branch's old head, first parent) and 491c9c02 (an origin/live commit — live's tip at merge time, #946's merge, 05:37Z, before #947 at 06:42Z). git log head --not 5b614f9c 491c9c02 returns ONLY e84b4436; the 17 live commits came in unchanged; the old head is an ancestor, so no amend/force-push signature. The message's last line is Lab-Run: IR-20261111-r25offdisk.

5. Claim honesty — accurate, one timestamp nit. The post-merge note's substance checks out: capacity: 1 was already live via #948 (merged 03:44Z) and live's configmap at the merge base is byte-identical to the head's, including live's extra # pulled forward from #944 … line, so the PR's remaining substance really is the secret removal; the one conflict really was configmap.yaml, resolved by taking live verbatim, and that is provably lossless (the branch's configmap hunk — 2→1 plus its R25 comment — is a strict subset of live's text). Nit: the note and the merge message say #1206 "landed at 06:15Z"; the API says merged_at 06:04:20Z (merge commit created 06:04:19Z). Ordering is unaffected — flagging only so the record is right. Also noted, not defects: head's tree predates #947 (06:42Z), which is why diff live..head additionally lists four core/directors/sendrast/... files — those are #947's, byte-identical to the merge's live base and untouched by this PR; and the fsGroup comment at statefulset:137-139 still names the removed 0440 sops/AWS mounts as its rationale although it is now load-bearing for harbor-robot (cosmetic, optional follow-up).

6. Tests. On the exact head sha all 12 commit-status contexts report success (combined state success). The job that covers this change — CI Review / runner-manifest-test (ci.yaml job at :90, running test_runner_ephemeral_ceilings.py, test_runner_dind_gc_tightening.py, each plain and --self-check, plus test_sbx_npm_cache.py) — ran on the 06:28Z push and passed in 10s; Lint & Validate / yaml-lint, helm-validate, shellcheck and terraform-validate also passed. Caveat: test_runner_placement_prep.py exists at head but is wired into no workflow, so its "passes" is a local-only claim — the suites that actually gate this manifest all ran green.

The disclosed follow-ups (ESO object cleanup, tf-plan policy narrowing, Harbor pull robot) I treat as post-landing work, not defects of this PR; the ESO objects are left in place, so a revert still restores working mounts.

model: omniroute/deepseek-flash
session: IR-20261111-r25offdisk-remerge-seatB-20261112T0645Z

**APPROVE — ld-reviewer — head e84b4436b389ca2b071728109addafc0f2aa03e1** Read-only seat B. Judged the head against `origin/live` and inspected the merge commit itself (fresh bare clone of Homelab/infra-core + API reads; no cluster commands, no test harness). **1. Precondition — MET.** Homelab/prod #1206 is `merged=true`, merged_at `2026-11-12T06:04:20Z`; its merge commit `9597e6f7` has parents `3185b789` + the PR's own head `6a896594`, and it is in prod `live` (live has since moved to `4087d04b` via #1213; `9597e6f7` is still on the branch). I also checked the substance of the ordering: at prod `live`, `infra-plan.yaml`, `infra-apply.yaml` and `infra-apply-cluster.yaml` each fetch sops/AWS per job into `mktemp -d "$TMPBASE/r25-secrets.XXXXXX"` (chmod 700, 0600 files, shred on exit), so the pod mounts this PR deletes are no longer any job's only credential source. **2. Scope at head vs origin/live — exactly the R25 removal.** `diff 491c9c02(live at merge)..e84b4436` touches ONE file, `core/manifests/apps/forgejo-runner/statefulset.yaml`, in 3 hunks: the `SOPS_AGE_KEY_FILE` / `AWS_SHARED_CREDENTIALS_FILE` env pair, the two `volumeMounts` (`/home/runner/.config/sops/age`, `/home/runner/.aws`), and the two `secret` volumes (`runner-sops-age-key`, `runner-aws-credentials`). Those hunk lines are identical to the branch's own pre-merge change (diffed against `3563ec7f..5b614f9c`), so the merge resolved the file rather than rewriting it. Volumes REMAINING (5): `runner-config` (configMap `forgejo-runner-config`), `runner-secret` (secret `forgejo-runner-secret`), `workspace` (emptyDir 10Gi), `docker-config` (emptyDir), `harbor-robot` (secret `runner-harbor-robot`, defaultMode 0440) — i.e. live's seven minus the two removed; the `runner-data`/`docker-data` volumeClaimTemplates, env `POD_NAME`, `DOCKER_HOST`, `GIT_HTTP_LOW_SPEED_LIMIT/TIME` and `envFrom: runner-forgejo-token` are untouched. `configmap.yaml`, `dind-gc-cronjob.yaml`, `dind-gc-script.yaml` and `es-forgejo-runner.yaml` are byte-identical to live (sha256 match), so nothing was smuggled in and nothing else moved. No infra-core workflow referenced the removed paths, so this repo's own CI loses no credential. **3. Nothing regressed by the merge.** `replicas: 8` (statefulset:125, not 9); `capacity: 1` (configmap:40); dind-gc `RUNNER_COUNT` is `"8"` in the CronJob env and `RUNNER_COUNT="${RUNNER_COUNT:-8}"` as the script default. Live's affinity rework survived intact: `In [prod-wk-02, prod-wk-05, prod-wk-06]` plus an explicit `NotIn [prod-wk-01]` in the same nodeSelectorTerm. **4. It is a merge, not a rewrite.** Parents are `5b614f9c` (the branch's old head, first parent) and `491c9c02` (an origin/live commit — live's tip at merge time, #946's merge, 05:37Z, before #947 at 06:42Z). `git log head --not 5b614f9c 491c9c02` returns ONLY `e84b4436`; the 17 live commits came in unchanged; the old head is an ancestor, so no amend/force-push signature. The message's last line is `Lab-Run: IR-20261111-r25offdisk`. **5. Claim honesty — accurate, one timestamp nit.** The post-merge note's substance checks out: `capacity: 1` was already live via #948 (merged 03:44Z) and live's configmap at the merge base is byte-identical to the head's, including live's extra `# pulled forward from #944 …` line, so the PR's remaining substance really is the secret removal; the one conflict really was `configmap.yaml`, resolved by taking live verbatim, and that is provably lossless (the branch's configmap hunk — 2→1 plus its R25 comment — is a strict subset of live's text). Nit: the note and the merge message say #1206 "landed at 06:15Z"; the API says `merged_at 06:04:20Z` (merge commit created 06:04:19Z). Ordering is unaffected — flagging only so the record is right. Also noted, not defects: head's tree predates #947 (06:42Z), which is why `diff live..head` additionally lists four `core/directors/sendrast/...` files — those are #947's, byte-identical to the merge's live base and untouched by this PR; and the fsGroup comment at statefulset:137-139 still names the removed 0440 sops/AWS mounts as its rationale although it is now load-bearing for `harbor-robot` (cosmetic, optional follow-up). **6. Tests.** On the exact head sha all 12 commit-status contexts report success (combined state `success`). The job that covers this change — `CI Review / runner-manifest-test` (ci.yaml job at :90, running `test_runner_ephemeral_ceilings.py`, `test_runner_dind_gc_tightening.py`, each plain and `--self-check`, plus `test_sbx_npm_cache.py`) — ran on the 06:28Z push and passed in 10s; `Lint & Validate / yaml-lint`, `helm-validate`, `shellcheck` and `terraform-validate` also passed. Caveat: `test_runner_placement_prep.py` exists at head but is wired into no workflow, so its "passes" is a local-only claim — the suites that actually gate this manifest all ran green. The disclosed follow-ups (ESO object cleanup, `tf-plan` policy narrowing, Harbor pull robot) I treat as post-landing work, not defects of this PR; the ESO objects are left in place, so a revert still restores working mounts. model: omniroute/deepseek-flash session: IR-20261111-r25offdisk-remerge-seatB-20261112T0645Z
All checks were successful
CI Review / receiver-test (pull_request) Successful in 5s
Details
CI Review / helm-validate (pull_request) Successful in 9s
Required
Details
CI Review / runner-manifest-test (pull_request) Successful in 10s
Details
CI Review / alert-receiver-test (pull_request) Successful in 20s
Details
Lint & Validate / shellcheck (pull_request) Successful in 5s
Required
Details
Lint & Validate / yaml-lint (pull_request) Successful in 23s
Required
Details
CI Review / record-sql-test (pull_request) Successful in 56s
Details
0/0 projects policies checked successfully.
Lint & Validate / terraform-validate (pull_request) Successful in 2m13s
Required
Details
CI Review / cockpit-e2e (pull_request) Successful in 2m44s
Details
CI Review / cockpit-test (pull_request) Successful in 2m54s
Details
CI Review / dispatcher-test (pull_request) Successful in 3m5s
Details
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin plane/IR-20261111-r25offdisk:plane/IR-20261111-r25offdisk
git switch plane/IR-20261111-r25offdisk
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
operator-review

Requires human operator review before merge — dangerous class (RBAC, secrets, network policy, Terraform) or explicitly gated. Never auto-merge.

No labels
operator-review
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Homelab/infra-core!944
No description provided.