-
Notifications
You must be signed in to change notification settings - Fork 0
feat(chart): one-time metadata backfill as a post-upgrade Helm hook #380
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+356
−0
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
f71813b
feat(chart): one-time metadata backfill as a post-upgrade Helm hook
divyasinghds 8a5239d
fix(chart): nil-guard metadataBackfill for --reuse-values upgrades
divyasinghds d17fc7a
fix(chart): stop the backfill hook from failing the auto-upgrade (Bug…
divyasinghds 22ef7dd
fix(chart): backfill timeout headroom + values.schema entry (Bugbot #…
divyasinghds 35038c9
fix(chart): hard-stop the backfill timeout with -k (Bugbot #380)
divyasinghds d1399aa
fix(chart): widen backfill Helm headroom to 120s (Bugbot #380)
divyasinghds 046a516
fix(chart): backstop ImagePullBackOff with activeDeadlineSeconds (Bug…
divyasinghds 4be4357
fix(chart): make backfill deadline consistent with the pull budget (B…
divyasinghds 17ec7cb
fix(chart): backfill hook is post-upgrade only, not post-install (Bug…
divyasinghds File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,164 @@ | ||
| {{- /* Nil-guard the whole block: an `--reuse-values` upgrade from a release | ||
| that predates the `metadataBackfill` key stores no such value, so | ||
| `.Values.metadataBackfill` is nil and a bare `.enabled` access would fail | ||
| template rendering (same hazard the images.ingestor keys guard against). | ||
| Absent key => feature off on that path; the production auto-upgrade uses | ||
| `--reset-then-reuse-values`, which layers in the new default (enabled: | ||
| true), so the backfill still runs on the real upgrade path. */ -}} | ||
| {{- $mb := default (dict) .Values.metadataBackfill -}} | ||
| {{- if $mb.enabled }} | ||
| {{- $ing := default dict .Values.images.ingestor -}} | ||
| {{- $repo := $ing.repository | default "ghcr.io/tracebloc/ingestor" -}} | ||
| {{- $digest := $ing.digest | default "" -}} | ||
| {{- /* Fallback tag matches values.yaml images.ingestor.tag and jobs-manager's | ||
| INGESTOR_IMAGE_TAG default — an older tag here would pull an ingestor that | ||
| predates the `tracebloc-backfill` entrypoint and the sweep would no-op. | ||
| (bugbot) The default only applies on a --reuse-values upgrade whose stored | ||
| values predate the images.ingestor key; keep it in lockstep with those. */ -}} | ||
| {{- $tag := $ing.tag | default "0.7" -}} | ||
| # One-time pre-cutover metadata backfill (data-ingestors #393, backend | ||
| # #1166/#1198). Runs the ingestor image's `tracebloc-backfill` entrypoint, which | ||
| # sweeps THIS client's already-registered datasets: GET each from the backend, | ||
| # recompute the enriched {schema, meta_data} from the persisted MySQL rows, and | ||
| # POST it back (which also re-folds any competition built from them). Datasets | ||
| # ingested before the federated-alignment cutover otherwise carry no enriched | ||
| # schema / feature_stats, so the backend's combine-time alignment checks and the | ||
| # edge metadata API can't reconcile them — old competitions/experiments that | ||
| # rely on that metadata would degrade under the new code. | ||
| # | ||
| # Trigger: a Helm post-upgrade hook. The auto-upgrade CronJob runs `helm upgrade` | ||
| # when a new chart is published, which fires this hook — so a client that | ||
| # auto-updates to this release runs the backfill once, automatically, with no | ||
| # operator step. (Not post-install: a fresh install has nothing to backfill.) | ||
| # | ||
| # One-time / idempotent: `before-hook-creation` replaces the prior hook Job on | ||
| # each upgrade; the sweep skips datasets already carrying new-shape metadata and | ||
| # the backend upsert is a safe overwrite, so re-runs are cheap no-ops. It is | ||
| # deliberately BEST-EFFORT — the entrypoint's exit code is swallowed so a | ||
| # backfill hiccup can never fail the `helm upgrade` (which would strand the | ||
| # cluster on the old chart). Operators read the outcome via `kubectl logs`. | ||
| apiVersion: batch/v1 | ||
| kind: Job | ||
| metadata: | ||
| name: {{ .Release.Name }}-metadata-backfill | ||
| namespace: {{ .Release.Namespace }} | ||
| labels: | ||
| {{- include "tracebloc.labels" . | nindent 4 }} | ||
| app.kubernetes.io/component: metadata-backfill | ||
| annotations: | ||
| # post-upgrade ONLY — deliberately NOT post-install. A fresh install has no | ||
| # pre-cutover datasets to backfill, so running there is pure risk (Helm waits | ||
| # on hooks even without --wait, so a cold ingestor-image pull / ImagePullBackOff | ||
| # could fail the client install for no benefit). A cluster that does carry old | ||
| # data reaches this on its FIRST auto-upgrade. (bugbot) | ||
| helm.sh/hook: post-upgrade | ||
| # Run late, after the release's core resources (MySQL, jobs-manager) are in | ||
| # place — the sweep talks to in-cluster MySQL and the backend API. | ||
| helm.sh/hook-weight: "5" | ||
| helm.sh/hook-delete-policy: before-hook-creation | ||
| spec: | ||
| # activeDeadlineSeconds is a Job-level BACKSTOP for a pod that never runs its | ||
| # container — chiefly ImagePullBackOff, which sits Pending forever (backoffLimit | ||
| # can't catch it, since Pending isn't a failed attempt). Without it such a pod | ||
| # hangs the hook until Helm's own --timeout before failing. | ||
| # | ||
| # It is Job wall-clock and INCLUDES the image pull, so it must exceed | ||
| # (cold pull + full in-container run) or it would kill a HEALTHY sweep. Budget: | ||
| # backfillTimeoutSeconds (in-container sweep) + 30s kill grace + 120s cold | ||
| # pull/scheduling = 270s at the default. | ||
| # A healthy run therefore fits (warm pull ≈ instant; a cold pull up to ~120s | ||
| # plus the ≤150s container still lands on the deadline), while a Pending/stuck | ||
| # pod is bounded at 270s. That is well under the auto-upgrade CronJob's 10m | ||
| # --timeout (the path this feature targets); on a MANUAL `helm install/upgrade` | ||
| # use `--timeout 10m` on a cold cluster, since the 5m default leaves little room | ||
| # once resource rollout + a first-ever pull are counted. A pull slower than the | ||
| # budget hits the deadline and fails that upgrade, then self-heals on the next | ||
| # one (the image is cached by then). (bugbot) | ||
| activeDeadlineSeconds: {{ add (int ($mb.backfillTimeoutSeconds | default 120)) 150 }} | ||
| # backoffLimit 0: never retry a failed pod (a retry loop would only burn more | ||
| # of Helm's wait). The deadline above is the real bound. | ||
| backoffLimit: 0 | ||
| ttlSecondsAfterFinished: 86400 | ||
| template: | ||
| metadata: | ||
| labels: | ||
| {{- include "tracebloc.selectorLabels" . | nindent 8 }} | ||
| app.kubernetes.io/component: metadata-backfill | ||
| spec: | ||
| restartPolicy: Never | ||
| # The sweep never calls the Kubernetes API — it only reaches MySQL and the | ||
| # backend — so no ServiceAccount token is mounted. | ||
| automountServiceAccountToken: false | ||
| {{- if include "tracebloc.useImagePullSecrets" . }} | ||
| imagePullSecrets: | ||
| - name: {{ include "tracebloc.registrySecretName" . }} | ||
| {{- end }} | ||
| securityContext: | ||
| runAsNonRoot: true | ||
| runAsUser: 65534 | ||
| seccompProfile: | ||
| type: RuntimeDefault | ||
| containers: | ||
| - name: metadata-backfill | ||
| image: {{ if $digest }}{{ printf "%s@%s" $repo $digest | quote }}{{ else }}{{ printf "%s:%s" $repo $tag | quote }}{{ end }} | ||
| imagePullPolicy: {{ if $digest }}IfNotPresent{{ else }}Always{{ end }} | ||
|
divyasinghds marked this conversation as resolved.
|
||
| # Bound the sweep with `timeout` and ALWAYS exit 0, so neither a slow | ||
| # run nor a backfill error can fail the hook (and thus the upgrade). | ||
| # backfillTimeoutSeconds must stay under autoUpgrade.timeout (10m) so | ||
| # the pod finishes before Helm's --timeout; a run that hits the bound | ||
| # is killed cleanly and resumes on the next upgrade (the sweep is | ||
| # idempotent and skips datasets already backfilled). (bugbot) | ||
| command: ["/bin/sh", "-c"] | ||
| args: | ||
| - | | ||
| set -u | ||
| TIMEOUT={{ $mb.backfillTimeoutSeconds | default 120 }} | ||
| echo "[metadata-backfill] running (timeout ${TIMEOUT}s)" | ||
| # -k 30: if the backfill ignores the SIGTERM at TIMEOUT (or is | ||
| # wedged), escalate to SIGKILL 30s later so `timeout` can't wait | ||
| # forever and hang the hook past Helm's --timeout. Worst-case | ||
| # wrapper runtime is TIMEOUT+30s, still well under the 5m default. (bugbot) | ||
| if timeout -k 30 "${TIMEOUT}" tracebloc-backfill; then | ||
|
divyasinghds marked this conversation as resolved.
|
||
| echo "[metadata-backfill] completed" | ||
| else | ||
| echo "[metadata-backfill] did not finish (exit $?); remaining tables resume on the next upgrade (non-fatal)" | ||
| fi | ||
| exit 0 | ||
|
divyasinghds marked this conversation as resolved.
divyasinghds marked this conversation as resolved.
|
||
| env: | ||
| {{- include "tracebloc.proxyEnv" . | nindent 12 }} | ||
| # Backend auth: the ingestor's APIClient mints a token from these | ||
| # (same credentials Secret jobs-manager uses). BACKEND_TOKEN would be | ||
| # preferred, but the chart holds no static backend token — jobs-manager | ||
| # mints its own at runtime — so the backfill uses the client creds. | ||
| - name: CLIENT_ID | ||
| valueFrom: | ||
| secretKeyRef: | ||
| name: {{ include "tracebloc.secretName" . }} | ||
| key: CLIENT_ID | ||
| - name: CLIENT_PASSWORD | ||
| valueFrom: | ||
| secretKeyRef: | ||
| name: {{ include "tracebloc.secretName" . }} | ||
| key: CLIENT_PASSWORD | ||
| # In-cluster MySQL + backend endpoint, mirroring the jobs-manager / | ||
| # ingestion-Job env so the ingestor's Config resolves the same store. | ||
| - name: MYSQL_HOST | ||
| value: "mysql-client" | ||
| - name: CLIENT_ENV | ||
| value: {{ .Values.env.CLIENT_ENV | default "prod" | quote }} | ||
| - name: HOME | ||
| value: "/tmp" | ||
| securityContext: | ||
| allowPrivilegeEscalation: false | ||
| readOnlyRootFilesystem: true | ||
| capabilities: | ||
| drop: ["ALL"] | ||
| resources: | ||
| {{- toYaml ($mb.resources | default dict) | nindent 12 }} | ||
| volumeMounts: | ||
| - name: tmp | ||
| mountPath: /tmp | ||
| volumes: | ||
| - name: tmp | ||
| emptyDir: {} | ||
| {{- end }} | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| suite: metadata-backfill post-upgrade hook Job | ||
| templates: | ||
| - templates/metadata-backfill-hook.yaml | ||
| release: | ||
| name: stg | ||
| namespace: tracebloc | ||
| set: | ||
| clientId: "test-id" | ||
| clientPassword: "test" | ||
| tests: | ||
| - it: renders a hook Job by default | ||
| asserts: | ||
| - hasDocuments: | ||
| count: 1 | ||
| - isKind: | ||
| of: Job | ||
| - equal: | ||
| path: metadata.name | ||
| value: stg-metadata-backfill | ||
|
|
||
| - it: is a post-upgrade (only) hook that replaces its prior run | ||
| asserts: | ||
| - equal: | ||
| path: metadata.annotations["helm.sh/hook"] | ||
| value: post-upgrade | ||
| - equal: | ||
| path: metadata.annotations["helm.sh/hook-delete-policy"] | ||
| value: before-hook-creation | ||
|
|
||
| - it: runs the backfill entrypoint on the ingestor image, best-effort (exit 0) | ||
| set: | ||
| images.ingestor.tag: "0.9" | ||
| asserts: | ||
| - equal: | ||
| path: spec.template.spec.containers[0].image | ||
| value: ghcr.io/tracebloc/ingestor:0.9 | ||
| - equal: | ||
| path: spec.template.spec.containers[0].imagePullPolicy | ||
| value: Always | ||
| - matchRegex: | ||
| path: spec.template.spec.containers[0].args[0] | ||
| pattern: tracebloc-backfill | ||
|
|
||
| - it: is best-effort — self-bounded by timeout, always exits 0, deadline backstops a stuck pull | ||
| asserts: | ||
| # Job-level backstop bounds an ImagePullBackOff/Pending pod (backfillTimeout | ||
| # 120 + 30 grace + 120 pull = 270), sized to exceed a healthy cold-pull run. | ||
| - equal: | ||
| path: spec.activeDeadlineSeconds | ||
| value: 270 | ||
| - equal: | ||
| path: spec.backoffLimit | ||
| value: 0 | ||
| - matchRegex: | ||
| path: spec.template.spec.containers[0].args[0] | ||
| pattern: 'timeout -k 30 "\$\{TIMEOUT\}" tracebloc-backfill' | ||
| - matchRegex: | ||
| path: spec.template.spec.containers[0].args[0] | ||
| pattern: exit 0 | ||
|
|
||
| - it: pins the ingestor image by digest when one is set | ||
| set: | ||
| images.ingestor.digest: "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" | ||
| asserts: | ||
| - equal: | ||
| path: spec.template.spec.containers[0].image | ||
| value: ghcr.io/tracebloc/ingestor@sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | ||
| - equal: | ||
| path: spec.template.spec.containers[0].imagePullPolicy | ||
| value: IfNotPresent | ||
|
|
||
| - it: sources backend creds from the release Secret and points at in-cluster MySQL | ||
| asserts: | ||
| - contains: | ||
| path: spec.template.spec.containers[0].env | ||
| content: | ||
| name: CLIENT_ID | ||
| valueFrom: | ||
| secretKeyRef: | ||
| name: stg-secrets | ||
| key: CLIENT_ID | ||
| - contains: | ||
| path: spec.template.spec.containers[0].env | ||
| content: | ||
| name: MYSQL_HOST | ||
| value: "mysql-client" | ||
|
|
||
| - it: hardened pod — non-root, no SA token, read-only rootfs, caps dropped | ||
| asserts: | ||
| - equal: | ||
| path: spec.template.spec.securityContext.runAsNonRoot | ||
| value: true | ||
| - equal: | ||
| path: spec.template.spec.automountServiceAccountToken | ||
| value: false | ||
| - equal: | ||
| path: spec.template.spec.containers[0].securityContext.readOnlyRootFilesystem | ||
| value: true | ||
| - equal: | ||
| path: spec.template.spec.containers[0].securityContext.allowPrivilegeEscalation | ||
| value: false | ||
|
|
||
| - it: renders nothing when metadataBackfill.enabled=false | ||
| set: | ||
| metadataBackfill.enabled: false | ||
| asserts: | ||
| - hasDocuments: | ||
| count: 0 |
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
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
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.