feat(chart): harden upgrade ergonomics (#64)#65
Open
perbu wants to merge 1 commit into
Open
Conversation
Make chart upgrades less brittle and more self-documenting. - Wrap new top-level value blocks (serviceMonitor, dashboards) with nil-safe (.Values.X).enabled guards so helm upgrade --reuse-values no longer crashes with a nil-pointer when stored values predate them. - Add values.schema.json so type/enum errors surface as schema messages on helm install/upgrade instead of opaque template render failures. - Add make helm-test target plus helm-test CI job that lints the chart and renders against fixtures simulating older values shapes, catching nil-pointer regressions automatically. - Pin Varnish to 9.0.1 in chaperone.Dockerfile via required VARNISH_VERSION build-arg; stamp it into the image as org.opencontainers.image.varnish-version. The same value is read from chaperone.varnishVersion in values.yaml (single source of truth) by the Makefile, the docker workflow, and the test-rust job, so test and ship cannot drift. - Branch NOTES.txt on .Release.IsUpgrade. On upgrade, print the recommended --reset-then-reuse-values --force-conflicts incantation, the bundled Varnish version, and a link to docs/operations/upgrades.md. Pre-upgrade preflight Job (4.4 in #64) deferred. Pulling images and running varnishd from a Helm hook has poor failure semantics (stuck release on transient errors, extra RBAC) and is largely redundant once the OCI label is in place. Will revisit as a non-blocking reporter or a CLI subcommand. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #65 +/- ##
=======================================
Coverage 67.62% 67.62%
=======================================
Files 38 38
Lines 6039 6039
=======================================
Hits 4084 4084
- Misses 1622 1623 +1
+ Partials 333 332 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
gquintard
reviewed
May 11, 2026
|
|
||
| - name: Read pinned Varnish version | ||
| id: varnish | ||
| run: echo "version=$(awk -F'\"' '/^ varnishVersion:/{print $2; exit}' charts/varnish-gateway/values.yaml)" >> $GITHUB_OUTPUT |
| @@ -1,4 +1,4 @@ | |||
| {{- if .Values.serviceMonitor.enabled }} | |||
| {{- if (.Values.serviceMonitor).enabled }} | |||
Contributor
Author
There was a problem hiding this comment.
you're missing the --reuse-values upgrade path from a pre-monitoring release, which is half the point of the PR
| > /etc/apt/sources.list.d/varnish.list \ | ||
| && apt-get update \ | ||
| && apt-get install -y --no-install-recommends varnish libcap2-bin \ | ||
| && apt-get install -y --no-install-recommends "varnish=${VARNISH_VERSION}*" libcap2-bin \ |
There was a problem hiding this comment.
why not just grab the official docker image with the required version?
Contributor
Author
There was a problem hiding this comment.
I started building my own images after CI broke when 9.0.1 came out. It was faster to build the image from scratch rather than adding dev-dependencies on top of varnish:9,
Now varnish-dev:9 might change this. I'm not sure, why should I use the varnish:9 images? I need to have control over the go/rust build pipe anyways.
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
Closes #64 (4.1, 4.2, 4.3 plus follow-on hardening; 4.4 explicitly deferred).
serviceMonitor/dashboardsreferences with(.Values.X).enabledsohelm upgrade --reuse-valuesfrom a release that predates these keys no longer crashes withnil pointer evaluating interface {}.enabled.values.schema.json. Catches type/enum errors at validate-time with clear messages instead of opaque template render failures. Tolerant of missing top-level keys (additionalProperties: true, none required) so it doesn't itself break--reuse-values.make helm-testtarget lints the chart and renders it againstcharts/varnish-gateway/test/fixtures/values-*.yaml— including avalues-pre-monitoring.yamlfixture that explicit-nulls the new top-level keys to exercise the guards. Wired intoci.ymlas a newhelm-testjob, so any future top-level key added without a guard fails CI.chaperone.Dockerfilerequires aVARNISH_VERSIONbuild-arg and installsvarnish=${VARNISH_VERSION}*/varnish-dev=${VARNISH_VERSION}*. The runtime stage stampsorg.opencontainers.image.varnish-versionso the version is readable without pulling layers. The Makefile,docker.ymlchart-publish job, andci.ymltest-rust job all extract the version fromchaperone.varnishVersioninvalues.yaml— test and ship cannot drift..Release.IsUpgrade, prints the recommended--reset-then-reuse-values --force-conflictsincantation, the bundled Varnish version, and a link todocs/operations/upgrades.md. First-install path is unchanged.Deferred: 4.4 pre-upgrade preflight Job
Helm hook with image-pull + RBAC-bloat + stuck-release-on-transient-error semantics is the wrong shape, and largely redundant once the OCI label exists. Will revisit as either a non-blocking post-upgrade reporter or a
varnish-gateway preflightCLI subcommand.Test plan
helm lint charts/varnish-gatewaypasses.make helm-testpasses locally (renders default + 2 fixtures).operator.replicas: "two"andchaperone.image.pullPolicy: Sometimeswith clear messages.helm template -f /tmp/values-old.yaml(noserviceMonitor/dashboardskeys) renders cleanly.docker buildx build --checkpasses against the new Dockerfile.chaperone-buildandhelm-publishjobs succeed in CI on a tag (requires merge + tag).helm upgradeagainst an installed release.🤖 Generated with Claude Code