Feature/kubernetes job runner - #3
Merged
Merged
Conversation
Add --s3auth=aws and --s3profile so s3gc can resolve temporary session-token credentials (e.g. from AWS SSO) instead of static keys; --s3-session-token for explicit temporary credentials
…o GHCR Found while using s3gc to reclaim 164.50 TiB across three customer clusters, including the Kubernetes Job runner's first production run. Boolean options were unusable from the environment. jsonargparse populates action="store_true" flags from env as the RAW STRING, and every non-empty string is truthy, so S3GC_S3USEIAM=false meant *true*: a Job selected the IAM credential provider instead of its static keys and hung indefinitely in the IMDS loop with no error, no exception and no log line, until activeDeadlineSeconds killed it. Thirteen flags shared the defect and four are set by job.yaml.tmpl; S3GC_S3SECURE_FLAG=false would likewise have stayed TLS. Coerce all boolean options once after parsing, reusing the existing strtobool helper, so true/false, yes/no, on/off, 1/0, empty and unset all behave. Bare CLI flags keep working, which the container entrypoint depends on. Retires the --order-by-objpath-flag twin that worked around this for one flag. --age was silently wrong for anything older than a day: timedelta.seconds is the sub-day remainder, so computed age never exceeded 23 h and --age 24 collected nothing, leaving an empty aux table and a dry-run reporting a clean bucket. Also: - --usecollected against a missing/empty aux table raised no error and exited 0, which reads as success. It is what a load-balanced CHHOST produces, since the aux table is node-local. It now fails loudly and says why. - Warn when --samples disagrees with the aux table's PARTITION BY; the mismatch loses partition pruning (measured ~26 min vs ~2 min per sample). - The final tally counted only the current attempt, understating one resumed run by 16.61 TiB. Say "in this attempt" and log the cumulative tombstones. - Detect GCS endpoints and fall back to per-object deletes, since GCS has no batch DeleteObjects. Images now publish publicly to ghcr.io/altinity/s3gc via the automatic GITHUB_TOKEN, matching altinity-mcp and altinity-sql-browser. A private image forced operators to copy a registry credential into the customer's namespace as an imagePullSecret and delete it afterwards; a public one removes that step. IMAGE_PULL_SECRET is now optional and the renderer omits the block when empty. Both architectures stay mandatory: one customer node pool is 5x arm64 + 1x amd64, where an amd64-only image cannot schedule. Documents the per-replica CHHOST requirement, the minimum grant set, the collect sharding recipe, and the per-cluster values that bite when wrong. 30 regression tests cover the boolean matrix, the age filter, the fail-loud path, the samples warning, GCS fallback and the renderer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Development history was only reconstructable from commit messages and a customer support ticket, which mixes two audiences: the ticket tracks a customer engagement, while this repository needs the engineering history. CHANGELOG.md separates them. It records the reasoning and the evidence behind defects found by running the tool against real clusters — the measurements are the expensive part to reconstruct, and without them a later reader cannot tell a deliberate design decision from an accident. Per CLAUDE.md rule 3 it carries no customer names, cluster identifiers or credentials; findings are described in terms of the behaviour they expose. Documents the unreleased work on this branch (boolean environment parsing, --age, fail-loud on a missing auxiliary table, GCS fallback, samples warning, cumulative tally, public multi-arch GHCR images, optional pull secret) plus the known gaps: collect still has no resume, and the GHCR package must be marked public once after the first publish. README's history section now points here; CLAUDE.md lists the file in the repository map and asks for it to be updated alongside behaviour, safety and deployment changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Verifying the fix against a development cluster showed the original scope claim was wrong. The codebase already paired each action="store_true" flag with a type=bool twin sharing the same dest, and eleven of the thirteen boolean flags had one and behaved correctly. Only --s3useiam and --listoptions lacked a twin, so the practical defect was --s3useiam alone. A dev cluster log from the earlier image proves the point: S3GC_S3SECURE_FLAG=false produced secure=False, a real bool, not the truthy string. Restores the --order-by-objpath-flag twin removed in 61b0ad2, since dropping it was an unnecessary command-line break, and routes all eleven surviving twins through coerce_bool instead of bool. type=bool rejects 0, 1 and empty values with an ArgumentError; coerce_bool accepts them, so the environment and the command line now agree on what a boolean looks like. The post-parse coercion stays as the uniform safety net: the twin pattern is easy to forget when adding a flag, which is exactly how --s3useiam broke. Adds migration notes for pull secrets being registry-scoped (a Docker Hub secret does not apply to ghcr.io, which surfaces as ImagePullBackOff with a 401 on the anonymous token), the one-time package visibility flip, and the changed meaning of S3GC_S3USEIAM=false. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Integrates #2 by @realyota, which added --s3auth=aws, --s3profile and --s3-session-token so credentials can come from the boto3 chain (AWS SSO) or as explicit temporary credentials. Merged here rather than into master first. Both branches share the same merge-base, so the conflicts are identical either way and ordering saves no work — but the CI workflow on this branch publishes a public image on every push to master, and PR #2 inherits master's unpinned dependencies. Merging it to master first would have published a public image with jsonargparse unpinned, which resolves to 4.50.x and fails at import. Resolving here also lets the 62-test suite and the Job runner validate the result first. Three resolutions were substantive rather than textual: PR #2 removed --s3useiam and IamAwsProvider, replacing workload identity with the boto3 chain. Both are kept, because they are not interchangeable: iam mode needs no boto3 and hands MinIO the credential *provider* rather than frozen keys, so credentials refresh across a long collect or delete. It is also what the validated Kubernetes deployments use. --s3auth now takes static|aws|iam, --s3profile implies aws, and --s3useiam is a deprecated alias for iam that still works with a warning, so existing manifests and Secrets are unaffected. Contradictory combinations are rejected instead of silently resolved. PR #2's Minio() call reverted the http_client to a bare PoolManager. That is the construct that hung a production run for 2h19m with no error, so the timeout/retry PoolManager and the reconnect helper are preserved, with a test asserting iam mode still gets them. Both branches defined strtobool. Ours is kept, since coerce_bool builds on it; PR #2's duplicate is dropped. Also kept from PR #2: UserVisibleError, the listing error that names the required s3:ListBucket permission and the aws CLI commands to verify it, and the widened log-secret filter. Adds what PR #2 did not include: Kubernetes wiring for the new flags (they were unreachable from a Job), pinned boto3==1.43.65, and 19 regression tests covering every mode, the implied-mode rules, the contradiction errors, the missing-boto3 path and the preserved transport. Co-Authored-By: realyota <realyota@users.noreply.github.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two omissions from the merge entry. The rendering env file is a breaking change that was not written down: the Job template no longer emits S3GC_S3USEIAM and render.py now requires S3AUTH and S3PROFILE, so an env file saved before this change fails with "missing required values: S3AUTH, S3PROFILE". Encountered while re-testing with an env file from the previous session, which is exactly how an operator will meet it. The note distinguishes the rendering key from the S3GC_S3USEIAM environment variable, which the script still honours as a deprecated alias, so an already-deployed Job manifest keeps working. Adds a Verified section so the entries above are provenance rather than claims: all three phases ran end to end against a development cluster with the image built from this branch, pulled anonymously with no imagePullSecret, and auth=iam confirms workload identity still resolves after the credential rewrite. States plainly that the GCS per-object fallback remains unit-tested only, pending a real endpoint. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR creates a Kubernetes workflow so the gc script can be deployed safely in a kubernetes env and leaves open more possible automations. Not only the kube deployments are added but also script improved with some gates and corner cases. The main core which is the ANTIJOIN query and the collect phase (which writes a ReplacingMergeTree table with all the objects in the bucket to do the ANTIJOIN and get a list of orphans to delete) is not touched, so results are deterministic.
Adds PHASE=dev-automation for non-production testing.
Keeps customer/production workflow unchanged:
Extends renderer validation:
Adds pytest coverage for:
Reorganizes the main README into a conventional Python-project layout:
Documents IAM-role behavior:
Validation
13 passed
Also validated:
python deploy/kubernetes/render.py deploy/kubernetes/example.env > /tmp/s3gc-job.yaml
kubectl apply --dry-run=client -f /tmp/s3gc-job.yaml
docker build -f docker/Dockerfile -t s3gc:dev-automation-check .
Notes for reviewers