Repository navigation
fix(chart): a failed pg_dump fails the kates backup job, and backup.enabled without the bundled database fails rendering - #372
Merged
Conversation
…nabled without the bundled database fails rendering The kates chart's backup CronJob dumps the bundled PostgreSQL only, and rendered only when backup.enabled and postgresql.enabled were both set. values-prod.yaml set backup.enabled (retention 30, a 50Gi PVC) with externalDatabase, since productionMode refuses the bundled database, so production rendered no CronJob and no PVC while NOTES printed a backup schedule. The README's production example did the same. Where the job did run, as on values-staging.yaml, its script piped pg_dump into gzip without pipefail. A dump that failed (wrong password, database down) still left a valid, empty archive as the newest file, the job printed "Backup complete" and passed, and the prune then deleted the real backups older than the retention. Each OnFailure retry added another empty archive. backup.enabled without postgresql.enabled now fails rendering and says to back the external database up with its own tooling. values-prod.yaml and the README example no longer set it. The script runs with set -eo pipefail and dumps to a .partial file, renamed only once the dump succeeds; each run clears partials a failed try left. A failed dump now fails the job before the prune and leaves no archive a restore would pick. A helm-lint step in ci.yml renders the prod overlay and asserts the combination is refused with this message. Chart 0.11.0: values that rendered before now fail on upgrade.
This was referenced Oct 17, 2026
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
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.
Why
The kates chart's backup CronJob has two defects. One means production never gets a backup; the other means a failing backup can delete good ones. Checked in the code:
templates/backup-cronjob.yamlrendered only under{{- if and .Values.backup.enabled .Values.postgresql.enabled }}, because it dumps the bundled PostgreSQL only.values-prod.yaml:76-82setbackup.enabled: true, retention 30 and a 50Gi PVC, but prod usesexternalDatabase, sinceproductionModerefuses the bundled database. So production rendered no CronJob and no PVC. AndNOTES.txt:70-75checks onlybackup.enabled, so it still printedBackups: Schedule 0 1 * * * | Retention: 30 days | Storage: PVC (50Gi). The production example incharts/kates/README.md(--set backup.enabled=truewith an external database) has the same bug.values-corporate.yamlhas no backup block.pg_dump ... | gzip > "${FILENAME}"without pipefail. Ifpg_dumpfails (wrong password, database down), gzip still writes a valid, empty archive. The script prints "Backup complete" and exits 0, andfind /backups -name "kates-*.sql.gz" -mtime +N -deletethen prunes the real backups older than the retention. pipefail alone isn't enough: gzip creates the file beforepg_dumpfails, so the empty archive stays as the newest file, the one a restore would pick. WithrestartPolicy: OnFailure, each retry in the same pod adds another. On staging (values-staging.yaml: bundled database, 10Gi PVC, retention 14), a broken dump leaves no usable backup after 14 days, and every job shows green.The emptyDir case (backup on,
backup.persistence.enabledoff) isn't silent: NOTES saysemptyDir (non-persistent), and no shipped overlay enables backup without persistence. It is left alone (see Decisions).Changes
charts/kates/templates/backup-cronjob.yamlbackup.enabledset andpostgresql.enabledoff, rendering fails: "backup.enabled: the backup CronJob dumps the bundled PostgreSQL (postgresql.enabled) only, so with externalDatabase no backup would run. Back the external database up with its own tooling and set backup.enabled=false." This follows thefailstyle insecret.yamlanddeployment.yaml. With the fail in place, NOTES printsBackups:only when the CronJob renders, soNOTES.txtneeds no edit.set -eo pipefail. It clears/backups/kates-*.sql.gz.partialleft by a failed try, dumps to${FILENAME}.partial, and runsmvto the final name only after the dump succeeds. A failed dump now fails the job before the prune and leaves no archive. The prune is unchanged, and itskates-*.sql.gzpattern doesn't match.partialfiles.cleanup-cronjob.yaml.kates.selectorLabels. Postgres' NetworkPolicy admits only pods that carry them, and staging setsnetworkPolicy.enabled.charts/kates/values-prod.yamlbackup:block is gone. In its place is a four-line comment: the chart's backup dumps only the bundled PostgreSQL, whichproductionModerefuses, so back the external database up with its own tooling (CloudNativePG backups, a managed service's point-in-time recovery). It points to theexternalDatabasecomment invalues.yamlby content, not by line number. Prod never rendered a backup, so no object goes away.charts/kates/values.yamlbackup.enabledcomment says it dumps the bundled PostgreSQL only, and that rendering fails withpostgresql.enabled=false. The edit is at line 677. The anchored caveat refs intovalues.yaml:395-405(mcp_caveats.go) and:605-673(mcp_tools_scenario.go) end above it, so they don't move.charts/kates/README.mdbackup.enabledrow says "bundled PostgreSQL only; rendering fails withpostgresql.enabled=false".--set backup.enabled=true, which now fails rendering.charts/kates/Chart.yamlversiongoes from 0.10.11 to 0.11.0, and the change comment above it describes this PR.appVersionstays 1.25.1.artifacthub.io/changesentry (kind: fixed) starts with ACTION REQUIRED, because values that rendered before now fail.Version tables
README.mdchart table anddocs/book/appendix-d-versions.md: the kates chart is 0.11.0. Neither generator writes in place, so I edited both cells by hand and confirmed them withscripts/gen-chart-table.sh --checkandscripts/gen-version-matrix.sh --check..github/workflows/ci.yml--set externalDatabase.existingSecret=ci. Then it checks thatbackup.enabled=truewithpostgresql.enabled=falseand an external database fails to render. It also checks that the failure output contains the new message, so a render that fails for some other reason (the credentials check, say) can't pass by accident.Tests
charts/kateshas no helm-unittest suite:templates/testsholds only the helm-test hooks. The new ci.yml step is the lasting test of the new branch. It runs in helm-lint on every PR that triggers that job.git diff --name-only origin/main...HEAD: one commit ahead of origin/main. Changed paths:.github/workflows/ci.yml,README.md,charts/kates/{Chart.yaml,README.md,templates/backup-cronjob.yaml,values-prod.yaml,values.yaml},docs/book/appendix-d-versions.md. Jobs triggered:ci.yml: the code filter fires becauseci.ymlitself changed, so build-and-test, dependency-scan, yaml-validation and go-tests run, plus helm-lint and kyverno-policy-test;lint.yml: actionlint, yamllint;docs.yml: chart table, mermaid, link-check;book.yml: render;ci-kafka-charts.yml: the dashboards, library and matrix jobs, becausecharts/kates/**is in its filter.The worktree was still clean after every check.
The new ci.yml step, run locally: the prod overlay renders, and the refused combination fails with the expected message ("OK: backup without the bundled database is refused").
helm lint charts/kates/: 1 chart linted, 0 failed. Local helm is v4.3.0; CI pins v3.17.0.helm template kates charts/kates/with the defaults and eachvalues-*.yamloverlay:productionMode) render with--set externalDatabase.existingSecret=x. Without it they fail on the existing credentials check, as on main;set -eo pipefail, then the.partialfile and itsmv. The image ispostgres:16-alpine, whose shell is busybox ash.scripts/check-chart-test-paths.sh: all 2/apipaths resolve.helm dependency build, thenhelm lintandhelm templatefor every chart, in a scratch copy with an isolated helm config: rc 0.kates-platformpackages the kates 0.11.0 subchart from disk and renders; it sets no backup values.kyverno applyof the kates policies againstkates/k8s/deployment.yaml(kyverno 1.19.1): fail 0, error 0.yamllint -c .yamllint.yaml .github config charts(1.38.0): 0 errors. The one warning, a blank line atvalues-corporate.yaml:142, is in a file this PR doesn't touch.actionlint1.7.12: rc 0. It also shellchecks the newrunblock.cd cli && go test -count=1 ./...: all packages ok. Run because CLI tests readcharts/kates/values.yamlthrough anchored caveat refs (see Changes). CI runs it as-p 1 -race -v; here it ran without-race.go vet ./...;golangci-lint run --new-from-rev=origin/main ./...(2.14.0): 0 issues;govulncheck ./...: no vulnerabilities.scripts/check-cli-style.sh,scripts/check-cli-compat.sh.scripts/check-versions.shand--workflows: every pin agrees, and no workflow hardcodes a CI toolchain pin.scripts/gen-chart-table.sh --check(kates 0.11.0) andscripts/gen-version-matrix.sh --check.scripts/check-book-style.sh(within the ratchet baseline),check-book-contrast.py(all 170 colour pairs meet WCAG AA),check-mermaid.sh --quiet(156 diagrams, 0 invalid).A scratch-copy HTML render of the book (
prepare-book-render.sh, then quarto 1.10.19): 39 chapters, rc 0, no WARN or ERROR lines, and appendix-d shows 0.11.0 (app 1.25.1).check-book-fragments.pyfinds all 7,458 fragment links in 38 pages landing on an id.The ci-kafka-charts dashboards job:
scripts/check-dashboards.pypasses all 12 dashboards, andpython3 -m unittest discover -s dashboardsis OK (5 skipped).cd kates && ./mvnw verify:kates/is unchanged, so it gives the same result as on main;gen-dashboards.py --checkand ci.yml's yaml-validation, because PyYAML isn't installed locally. This PR touches no dashboard and nothing underconfig/, and yamllint parsedconfig/cleanly;make check-helpand shellcheck on scripts, because no Makefile or shell script changed;./mvnw verify, surefire plus the*ITfailsafe tests). It runs becauseci.ymlchanged;kates/is unchanged;kates, govulncheck);-race -p 1 -v;gen-dashboards --check, the kafka-common helm-unittest library job, and the chart matrix;docs.yml's lychee link check;book.yml's pdf job);kates/,cli/,config/or Dockerfile change);github-advanced-securityAI scan, which fails on every PR for a GitHub-side reason.Decisions
backup.enabledwithexternalDatabase, including the README example) now fail on upgrade, and 0.9.0 also shipped its removed-key fails as a minor.kates-platform's range>=0.5.0 <1.0.0accepts it.externalDatabase. That needs the host fromexternalDatabase.host/portand a different secret key (password, notQUARKUS_DATASOURCE_PASSWORD). It would also put a production database's backups on an in-cluster PVC. The fail and the removal ship together, orvalues-prod.yamlwould stop rendering.emptyDir (non-persistent), and a fail would break a bare--set backup.enabled=trueon kind.set -eo pipefail, dumps to a.partialfile, renames it withmvafter success, and clears stale partials at the start.Deliberately not done
backup.enabledwithexternalDatabase, such as copied prod values or the README example. Their backups never ran. The ACTION REQUIRED changelog entry says so.postgres:16-alpine, because Docker is off locally. So busybox ash'spipefailsupport wasn't checked in the real image. If it were missing, every run would fail loudly rather than pass silently.postgresql.auth.existingSecretnot checked. The job readsQUARKUS_DATASOURCE_*keys from it, and I didn't check that such a secret carries them. That is outside this finding.Landing
This PR shares no file with the earlier plan PRs (#370, #371), so it can land at any time. It also merges cleanly with the open Actions bump #367, which edits
ci.ymltoo. A later PR that editscharts/kateswill conflict on the chart version inChart.yaml,README.mdandappendix-d-versions.md. Land chart PRs one at a time and redo the bump on rebase.