Skip to content

fix(workspace-minio): make minio-sync backup run and mirror real data - #1119

Merged
mdheller merged 1 commit into
mainfrom
fix/minio-sync-backup
Aug 2, 2026
Merged

fix(workspace-minio): make minio-sync backup run and mirror real data#1119
mdheller merged 1 commit into
mainfrom
fix/minio-sync-backup

Conversation

@mdheller

Copy link
Copy Markdown
Member

Summary

minio-sync (the nightly workspace-minio backup CronJob) has failed on every run since it was authored and never once succeeded — it is absent from kube_cronjob_status_last_successful_time, and its CronJob status carries only lastScheduleTime, no lastSuccessfulTime. Nothing alerted, same class as the mail-backup regression.

Root cause (all confirmed in-cluster with disposable probe Jobs, socioprophet ns)

Three stacked defects, plus a phantom destination:

  1. Entrypoint vs command. The minio/mc image ENTRYPOINT is mc. The manifest set args: but not command:, so Kubernetes ran mc /bin/sh -c "<script>" — mc treats /bin/sh as a subcommand and the shell never runs. As uid 1000 it failed even earlier: mc: <ERROR> Unable to save new mc config. mkdir /.mc: permission denied (HOME defaults to /).
  2. Unwritable HOME. mc writes config to $HOME/.mc; uid 1000 cannot write /. Fixed with HOME=/tmp (verified writable by uid 1000).
  3. Credentials silently empty. Creds came via envFrom on minio-credentials, whose keys are hyphenated (access-key / secret-key) — invalid env-var identifiers, silently dropped — so $MINIO_ROOT_USER/$MINIO_ROOT_PASSWORD were always empty. The workspace-minio Deployment already maps them correctly via secretKeyRef; this now does the same.
  4. Phantom source & destination. The mirror targeted local/workspace (no such bucket — real buckets are zot ~20G and lifecycle-warden) and s3/prophet-workspace-backup (an s3 alias never configured; there is no S3 backup secret anywhere in the namespace), all masked by || true. Even a credential-fixed job would have exited green having copied nothing.

Fix

  • Override command: ["/bin/sh","-c"] so the shell runs.
  • HOME=/tmp.
  • Map creds via explicit secretKeyRef (access-keyMINIO_ROOT_USER, secret-keyMINIO_ROOT_PASSWORD).
  • Mirror every real bucket to a dedicated workspace-minio-backup PVC; POSIX-shell bucket loop (the image ships no awk/find).
  • Remove || true so a mirror failure fails the job loudly, and print bucket count + du -sh of the backup so a green-over-nothing run is impossible.
  • fsGroup: 1000 scoped to the destination PVC only — the MinIO source is read over S3 and never mounted, so fsGroup cannot rewrite source ownership.
  • New workspace-minio-backup PVC sized 100Gi to match the source PVC (a backup smaller than its source is a latent ENOSPC/silent-fail as the source grows).

Verification (cluster evidence; CI is spend-capped)

Disposable probe Jobs (all deleted afterward) reproduced the failure and proved the fix:

  • Original config → mkdir /.mc: permission denied.
  • command: override + HOME=/tmp + secretKeyRef creds → mc alias set ... Added 'local' successfully, enumerated real buckets (zot, lifecycle-warden), and mirrored real objects S3→filesystem (lifecycle-warden/audit/chunk-*.json, Total: 70.50 KiB, Transferred: 70.50 KiB).
  • The full ~20G zot copy onto the 100Gi PVC was not run in-session, to keep the cluster clean and within the spend cap. The transfer mechanism is identical to the verified run; fsGroup:1000 writability on a fresh GCE PD is independently proven by the sibling mail-backup job on this same cluster.
  • kubectl kustomize builds for base + p0-lab, and kubectl apply --dry-run=server accepts the CronJob and the new PVC.

Notes for reviewers

  • Do not merge without deciding PVC size — 100Gi matches the source; dial down if the registry backup footprint should be capped instead.
  • Coupling with the mail-backup lane: that lane must keep its fsGroup off the source (mount source read-only, write only to the destination PVC). This minio-sync design already follows that rule (source via S3, never mounted).

minio-sync has failed on every run since it was authored and has never once
succeeded (it is absent from kube_cronjob_status_last_successful_time; the
CronJob status carries only lastScheduleTime). Root causes, all confirmed
in-cluster with disposable probe Jobs:

- The minio/mc image ENTRYPOINT is `mc`. The manifest set `args:` but not
  `command:`, so Kubernetes ran `mc /bin/sh -c "<script>"` and the shell never
  executed. Running as uid 1000 it failed even earlier, at
  `mkdir /.mc: permission denied` (HOME defaulted to `/`).
- Credentials came via `envFrom` on a Secret whose keys are hyphenated
  (access-key / secret-key) — not valid env-var identifiers, so they were
  silently dropped and $MINIO_ROOT_USER/$MINIO_ROOT_PASSWORD were always empty.
- The mirror targeted `local/workspace` (a bucket that does not exist) and
  `s3/prophet-workspace-backup` (an alias never configured; no S3 backup secret
  exists in the namespace), the whole line masked by `|| true` — so even a
  credential-fixed job would exit green having copied nothing.

Fix: override `command:` so the shell runs; set HOME=/tmp; map credentials via
explicit secretKeyRef (as the workspace-minio Deployment already does); mirror
every real bucket to a dedicated workspace-minio-backup PVC; drop `|| true` so a
mirror failure fails the job. fsGroup:1000 is scoped to the destination PVC
only — the source is read over S3 and is never mounted, so it cannot rewrite
source ownership.

Verified in-cluster (socioprophet): with these fixes a run authenticates,
enumerates the real buckets (zot ~20G, lifecycle-warden) and mirrors real
objects S3->filesystem (lifecycle-warden audit chunks, 70.50 KiB). The full 20G
copy onto the 100Gi PVC was not run in-session to keep the cluster clean and
within the spend cap.
Copilot AI review requested due to automatic review settings July 30, 2026 07:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Fixes the workspace-minio nightly minio-sync CronJob so it actually executes and mirrors real MinIO bucket data into an in-cluster backup PVC (instead of silently “succeeding” while copying nothing).

Changes:

  • Override container command to run the shell script, set writable HOME, and map MinIO credentials via secretKeyRef.
  • Mirror all discovered buckets from the MinIO endpoint to a mounted backup PVC and fail loudly on mirror errors.
  • Add a dedicated workspace-minio-backup PVC (100Gi) with ArgoCD prune disabled.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +67 to +76
# NOTE: the minio/mc image ships no awk/find, so bucket parsing is POSIX shell only.
mc ls local > /tmp/buckets.txt
buckets=0
while read -r line; do
b=${line##* }; b=${b%/}
[ -n "$b" ] || continue
echo "mirroring bucket: $b"
mc mirror --overwrite --remove "local/$b" "/backup/minio/$b"
buckets=$((buckets + 1))
done < /tmp/buckets.txt
# namespace), the whole line masked by `|| true`, so it could only ever exit
# green having moved nothing. There is deliberately no `|| true` below: a mirror
# failure now fails the job loudly.
# NOTE: the minio/mc image ships no awk/find, so bucket parsing is POSIX shell only.
mc mirror --overwrite --remove "local/$b" "/backup/minio/$b"
buckets=$((buckets + 1))
done < /tmp/buckets.txt
echo "MinIO mirror complete: ${buckets} bucket(s); backup now $(du -sh /backup/minio | cut -f1) at /backup/minio"
@mdheller

Copy link
Copy Markdown
Member Author

Merge-gate disposition (Copilot 3-channel + adversarial pass)

Core fix verified: the old manifest ran mc /bin/sh -c (mc's entrypoint treated /bin/sh as a subcommand, so the shell never executed) and mirrored a non-existent bucket to an unconfigured alias under || true. The new version overrides command, maps the hyphenated secret keys explicitly (envFrom silently dropped access-key/secret-key), mirrors every real bucket to a mounted 100Gi PVC, and drops || true under set -eu — real bytes, fails loud, no green-over-nothing.

Rejected — du/cut dependency finding (3680969995, 3680970024): the du -sh … | cut is in the final status echo, after the backup has already completed; under set -e a failed command substitution used as an echo argument does not abort the script, so at worst the reported size is blank while the mirror itself succeeded. Not a job failure.

Acknowledged, non-blocking: b=${line##* } bucket parsing is tightly coupled to mc ls output but correct (bucket names cannot contain spaces).

Verdict: MERGE-READY.

@mdheller
mdheller merged commit ae1d03e into main Aug 2, 2026
72 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants