prole/docs/completed/06-patch-garage-script-fixes.md

Junie brief — Queue #6: fix scripts/patch_garage_cross_cluster.sh

Self-contained brief. Three concrete defects in the script + a related manifest split. Land as one or two commits (your call — see §5).

---

1. Why

scripts/patch_garage_cross_cluster.sh was written one-shot during the 2026-04-29 cluster recovery to migrate CNPG backups off Garage and onto GCS+Workload-Identity. It worked at the time, but reading it back surfaced three defects that would bite the next person who runs it. We want it to be re-runnable cleanly (e.g. for a future second customer or DR rebuild) without surprises.

Live-cluster context (do not change): GCS migration is complete, ObjectStore is active, ScheduledBackup is plugin-method, three CNPG pods on knoe-dev-cnpg-0 are healthy. You don't need to run the script to fix it. This is a code-only commit.

2. The three defects

Defect A — stale DB_CLUSTER default

scripts/patch_garage_cross_cluster.sh:39

DB_CLUSTER="${DB_CLUSTER:-knoe-cnpg-0}"

Should be knoe-dev-cnpg-0 (matches conf/gke.cfg line init_cluster.db_cluster_kubecontext and CLAUDE.md cluster table). The current default resolves to the wrong context and kubectl operations would silently target the wrong cluster (or fail with "context not found", depending on the environment).

The script's APP_CLUSTER="${APP_CLUSTER:-knoe-dev-0}" on line 40 is already correct.

Header comment on lines 16-18 also references the wrong name in the ./etc/init_cnpg_gke.sh ... --cluster knoe-cnpg-0 example — fix that too.

Defect B — Phase 1 misses service/garage-s3-ilb

scripts/patch_garage_cross_cluster.sh:86-95 — Phase 1's deletion loop:

for resource in "statefulset/garage" "service/garage" "configmap/garage-config"; do
  ...
done

The loop deletes service/garage (the ClusterIP) but not service/garage-s3-ilb (the LoadBalancer). The ObjectStore's endpointURL historically pointed at the ILB, so leaving the LB orphaned costs money and confuses cluster state. The 2026-04-29 cleanup did this manually after the script ran.

Fix: add "service/garage-s3-ilb" to the loop's resource list. Order doesn't matter (all --ignore-not-found=true).

Defect C — Phase 2 applies a manifest containing both SA AND a legacy ScheduledBackup

scripts/patch_garage_cross_cluster.sh:114-122 — Phase 2 applies deploy/gcp/gke/knoe-db-backup-gcs.yaml. That manifest currently contains:

1. ServiceAccount cnpg-backup-sa — wanted; this is the WI-bound SA.
2. ScheduledBackup knoe-db-daily with method: barmanObjectStore — unwanted. Two problems:
   • Live cluster already has a plugin-method ScheduledBackup. Applying this would create a duplicate that runs alongside the active one.
   • method: barmanObjectStore (the legacy in-tree method) is being removed in CNPG v1.30 per upstream release notes. New work should not pin to it.

Fix (one of two paths — pick the cleanest):

Path 1 (recommended): split the manifest. Move the ScheduledBackup out of deploy/gcp/gke/knoe-db-backup-gcs.yaml into its own file (or delete it outright if there's no consumer — verify by grep -r "knoe-db-daily" across the repo first; if nothing references it, just remove it). Keep knoe-db-backup-gcs.yaml as the canonical SA-only manifest. The script's Phase 2 stays unchanged (still applies the SA file).

Path 2 (minimal): adjust the script only. Use a kubectl filter so Phase 2 only applies the ServiceAccount portion of the file:

GCP_PROJECT_ID="${GCP_PROJECT}" envsubst '${GCP_PROJECT_ID}' < "${GCS_MANIFEST}" \
  | yq 'select(.kind == "ServiceAccount")' \
  | kubectl --context="${DB_CTX}" apply -f -

(yq is already used elsewhere in etc/. If it's not in the script's preflight command -v check, add it.)

Path 1 is recommended because it makes the manifest file's contents match its filename's intent (-gcs.yaml should be GCS plumbing, not backup scheduling), and removes the deprecated-method footgun for any future runner.

If you take Path 1, also verify the live ScheduledBackup in knoe-db-0 namespace still exists and uses the plugin method — but do not run kubectl against the live cluster from the script change. Just inspect via kubectl --context=$DB get scheduledbackup -n knoe-db-0 -o yaml once if you want to confirm; don't modify it.

Defect D (bonus, mentioned in TODO) — Phase 3 unconditional re-apply

Phase 3 (lines 127-134) unconditionally re-applies k8s/knoe/knoe-db-barman-objectstore-gcs.yaml. Manifest currently matches live state, so this is harmless. The TODO suggests "worth a guard" — add a kubectl get objectstore knoe-db-barman-objectstore -n $DB_NS check that diffs against the manifest and skips re-apply if matching. Skip this one if it adds significant scope — it's a nice-to-have, not a defect.

3. What to verify

# 1) Defaults resolve correctly when no env vars set
bash -n scripts/patch_garage_cross_cluster.sh    # syntax check
( unset DB_CLUSTER APP_CLUSTER GCP_PROJECT GCP_REGION SERVICE_NS DB_NS
  source <(sed -n '36,50p' scripts/patch_garage_cross_cluster.sh)
  echo "DB_CLUSTER=$DB_CLUSTER, DB_CTX=$DB_CTX" )
# Expected: DB_CLUSTER=knoe-dev-cnpg-0, DB_CTX=gke_..._knoe-dev-cnpg-0

# 2) Dry-run the script in CONFIRM=false mode (existing safety: prints the
#    plan and exits 1) — verify the printed plan reads sensibly
./scripts/patch_garage_cross_cluster.sh
# Expected: prints "1. Remove Garage from knoe-dev-cnpg-0/knoe-system" etc.

# 3) Phase 1 deletion list now includes garage-s3-ilb
grep -A8 'for resource in' scripts/patch_garage_cross_cluster.sh
# Expected: list contains "service/garage-s3-ilb"

# 4) (If Path 1 chosen) the SA-only manifest has only ServiceAccount kind
grep '^kind:' deploy/gcp/gke/knoe-db-backup-gcs.yaml
# Expected: only "kind: ServiceAccount"

4. Out of scope

• Don't run the live script. This is a code-only fix. The script is for someone migrating a fresh cluster; we're not migrating.
• Don't change any RBAC or IAM. The Workload Identity binding is correct in the live cluster; this brief touches the script and (optionally) the manifest split, nothing more.
• The CNPG SA wiring inside etc/init_cnpg_gke.sh (cluster.spec.serviceAccountName
  • extended RoleBindings) is queue item #7 with its own brief. Don't conflate.

5. Commit shape

If you take Path 1 (manifest split): two commits, atomic each:

1. chore(deploy): split knoe-db-backup-gcs manifest — SA only, drop legacy ScheduledBackup
2. fix(scripts): patch_garage_cross_cluster defaults + missing garage-s3-ilb

If you take Path 2 (script-only): one commit.

fix(scripts): patch_garage_cross_cluster — three defects from 2026-04-29 review

Defects A, B, C from docs/plans/junie/06-patch-garage-script-fixes.md:

  A. DB_CLUSTER default was knoe-cnpg-0 (stale rebrand artifact);
     should be knoe-dev-cnpg-0 to match conf/gke.cfg.

  B. Phase 1 deletion loop missed service/garage-s3-ilb (the
     LoadBalancer the ObjectStore endpoint historically pointed at).
     Now included.

  C. Phase 2 was applying knoe-db-backup-gcs.yaml whole, including a
     legacy ScheduledBackup using method:barmanObjectStore (being
     removed in CNPG v1.30). Manifest split: SA stays in
     knoe-db-backup-gcs.yaml; ScheduledBackup removed.

Header comment + usage example also updated for the rename.
Closes queue item #6 in docs/TODO.md (drift R9).

6. Definition of done

• DB_CLUSTER default + header comment updated.
• Phase 1 loop includes service/garage-s3-ilb.
• Phase 2 doesn't apply a legacy ScheduledBackup (Path 1 or 2).
• bash -n clean; CONFIRM=false dry-run prints sensible plan.
• If Path 1: kind: ServiceAccount is the only kind in deploy/gcp/gke/knoe-db-backup-gcs.yaml.
• docs/TODO.md queue item #6 + drift R9 row archived to Done with date and commit ref.