From 1ec8b4aecec6202133fdd759eb8cdff7f6f8b4ba Mon Sep 17 00:00:00 2001 From: Jeff McCollum <16550786+jeffmccollum@users.noreply.github.com> Date: Thu, 8 Oct 2026 23:08:51 +0000 Subject: [PATCH 1/2] Allow commit SHA image tags with the automation writer pool The v2.16.0 floor for brainstore.automationwriter called semverCompare on brainstore.image.tag unconditionally. A commit SHA tag failed the render with "invalid semantic version", and an all-digit SHA prefix passed only because it parsed as a huge major version. Only enforce the floor when the tag is a stable release tag (vX.Y.Z), using the same regex as the Brainstore startup gate. SHAs and other custom tags carry no version and are not checked. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 2 +- braintrust/README.md | 7 ++-- braintrust/templates/_helpers.tpl | 4 ++ .../brainstore-automationwriter_test.yaml | 17 +++++++++ .../brainstore-writer-configmap_test.yaml | 38 +++++++++++++++++++ braintrust/values.yaml | 6 +-- 6 files changed, 67 insertions(+), 7 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index de710bb..9a27a46 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -84,7 +84,7 @@ The four brainstore configmaps (`brainstore-reader-configmap.yaml`, `brainstore- ### Brainstore Automation Writer loop config -The optional Automation Writer pool (`brainstore.automationwriter`, default `replicas: 0`) isolates the automations writer loop via `BRAINSTORE_WRITER_LOOP_CONFIG`. That variable exists starting in brainstore image `v2.16.0`. Enabling the pool (`replicas > 0`) fails the render unless `brainstore.image.tag` is `v2.16.0` or newer; do not bump the chart-wide default image for this. `brainstore-automationwriter-configmap.yaml` sets `include:automations`, and `brainstore-writer-configmap.yaml` sets `exclude:automations` only when `brainstore.automationwriter.replicas > 0`. A null or missing replica count is 0: an empty `replicas` field would make Kubernetes run 1 pod while the writer still handled automations. A missing `brainstore.automationwriter` map — what `helm upgrade --reuse-values` retains from a chart that predates the pool — is disabled: the writer does not exclude automations, and the automation writer manifests are omitted. A partial map, such as `--set brainstore.automationwriter.replicas=1` on those retained values, is merged with the template defaults in `braintrust.automationWriter.defaults` before any field is read. On Azure with the Container Storage driver, `brainstore.automationwriter.volume.size` is required only when `replicas > 0`; a zero-replica pool renders with `emptyDir` so an upgrade does not demand disk for a pool that is off. These two must stay in sync: the automation writer runs automations and the regular writer must exclude them whenever the pool is enabled. There is no automation-writer Service — nothing routes to the pool by URL; it self-drives its writer loop through Postgres / Redis. +The optional Automation Writer pool (`brainstore.automationwriter`, default `replicas: 0`) isolates the automations writer loop via `BRAINSTORE_WRITER_LOOP_CONFIG`. That variable exists starting in brainstore image `v2.16.0`. Enabling the pool (`replicas > 0`) fails the render when `brainstore.image.tag` is a release tag (`vX.Y.Z`) older than `v2.16.0`. Commit SHAs and other custom tags must keep rendering, so `braintrust.automationWriter.validate` only calls `semverCompare` after the tag matches the release-tag regex (`semverCompare` errors on a SHA). Do not bump the chart-wide default image for this. `brainstore-automationwriter-configmap.yaml` sets `include:automations`, and `brainstore-writer-configmap.yaml` sets `exclude:automations` only when `brainstore.automationwriter.replicas > 0`. A null or missing replica count is 0: an empty `replicas` field would make Kubernetes run 1 pod while the writer still handled automations. A missing `brainstore.automationwriter` map — what `helm upgrade --reuse-values` retains from a chart that predates the pool — is disabled: the writer does not exclude automations, and the automation writer manifests are omitted. A partial map, such as `--set brainstore.automationwriter.replicas=1` on those retained values, is merged with the template defaults in `braintrust.automationWriter.defaults` before any field is read. On Azure with the Container Storage driver, `brainstore.automationwriter.volume.size` is required only when `replicas > 0`; a zero-replica pool renders with `emptyDir` so an upgrade does not demand disk for a pool that is off. These two must stay in sync: the automation writer runs automations and the regular writer must exclude them whenever the pool is enabled. There is no automation-writer Service — nothing routes to the pool by URL; it self-drives its writer loop through Postgres / Redis. ### Version Numbers diff --git a/braintrust/README.md b/braintrust/README.md index 96d5433..7b34051 100644 --- a/braintrust/README.md +++ b/braintrust/README.md @@ -398,9 +398,10 @@ those new writer pods are Ready. Turning the pool on briefly runs automations on both pools until the regular writers roll. The pool requires Brainstore `v2.16.0` or newer. That is the first image that -honors `BRAINSTORE_WRITER_LOOP_CONFIG`. The chart's default image is older. -Setting `replicas` above 0 fails the render until `brainstore.image.tag` is -`v2.16.0` or newer. +honors `BRAINSTORE_WRITER_LOOP_CONFIG`. When `brainstore.image.tag` is a release +tag (`vX.Y.Z`), setting `replicas` above 0 fails the render unless the tag is +`v2.16.0` or newer. Commit SHAs and other custom tags carry no version, so the +chart does not check them; make sure such an image includes `v2.16.0`. `helm upgrade --reuse-values` does not add this pool's default values. If the installed release has no `brainstore.automationwriter` map, the pool stays off. diff --git a/braintrust/templates/_helpers.tpl b/braintrust/templates/_helpers.tpl index a0ed651..a519666 100644 --- a/braintrust/templates/_helpers.tpl +++ b/braintrust/templates/_helpers.tpl @@ -265,13 +265,17 @@ A null replica count is 0. An empty replicas field would make Kubernetes run 1 p {{/* BRAINSTORE_WRITER_LOOP_CONFIG is ignored before brainstore v2.16.0. Enabling the pool on an older image adds writers that still run every writer loop. +Only stable release tags (vX.Y.Z) are checked. Commit SHAs and other custom +tags carry no version, so the operator is responsible for compatibility. */}} {{- define "braintrust.automationWriter.validate" -}} {{- $replicas := int (include "braintrust.automationWriter.replicas" .) -}} {{- if gt $replicas 0 -}} {{- $tag := .Values.brainstore.image.tag | toString -}} +{{- if regexMatch "^v?(0|[1-9][0-9]*)\\.(0|[1-9][0-9]*)\\.(0|[1-9][0-9]*)$" $tag -}} {{- if not (semverCompare ">=2.16.0" $tag) -}} {{- fail (printf "brainstore.automationwriter requires brainstore image v2.16.0 or newer. brainstore.image.tag is %q, which does not implement BRAINSTORE_WRITER_LOOP_CONFIG." $tag) -}} {{- end -}} {{- end -}} {{- end -}} +{{- end -}} diff --git a/braintrust/tests/brainstore-automationwriter_test.yaml b/braintrust/tests/brainstore-automationwriter_test.yaml index 03b6668..111a46f 100644 --- a/braintrust/tests/brainstore-automationwriter_test.yaml +++ b/braintrust/tests/brainstore-automationwriter_test.yaml @@ -57,6 +57,23 @@ tests: path: spec.replicas value: 2 + - it: should render the automation writer pool when the brainstore image tag is a commit SHA + template: brainstore-automationwriter-deployment.yaml + values: + - __fixtures__/base-values.yaml + set: + brainstore.automationwriter.replicas: 1 + brainstore.image.tag: 0f3a9c2b7e1d4a5f6b8c9d0e1f2a3b4c5d6e7f80 + release: + namespace: "braintrust" + asserts: + - equal: + path: spec.replicas + value: 1 + - equal: + path: spec.template.spec.containers[0].image + value: test/brainstore:0f3a9c2b7e1d4a5f6b8c9d0e1f2a3b4c5d6e7f80 + - it: should render custom Brainstore Automation Writer rollout controls template: brainstore-automationwriter-deployment.yaml values: diff --git a/braintrust/tests/brainstore-writer-configmap_test.yaml b/braintrust/tests/brainstore-writer-configmap_test.yaml index a9795a8..4f9cdb4 100644 --- a/braintrust/tests/brainstore-writer-configmap_test.yaml +++ b/braintrust/tests/brainstore-writer-configmap_test.yaml @@ -227,6 +227,44 @@ tests: - failedTemplate: errorMessage: "brainstore.automationwriter requires brainstore image v2.16.0 or newer. brainstore.image.tag is \"v2.14.0\", which does not implement BRAINSTORE_WRITER_LOOP_CONFIG." + - it: should reject an automation writer pool on a brainstore 2.15 release without the v prefix + values: + - __fixtures__/base-values.yaml + set: + brainstore.automationwriter.replicas: 1 + brainstore.image.tag: 2.15.3 + release: + namespace: "braintrust" + asserts: + - failedTemplate: + errorMessage: "brainstore.automationwriter requires brainstore image v2.16.0 or newer. brainstore.image.tag is \"2.15.3\", which does not implement BRAINSTORE_WRITER_LOOP_CONFIG." + + - it: should exclude automations when the brainstore image tag is a commit SHA + values: + - __fixtures__/base-values.yaml + set: + brainstore.automationwriter.replicas: 1 + brainstore.image.tag: 0f3a9c2b7e1d4a5f6b8c9d0e1f2a3b4c5d6e7f80 + release: + namespace: "braintrust" + asserts: + - equal: + path: data.BRAINSTORE_WRITER_LOOP_CONFIG + value: "exclude:automations" + + - it: should exclude automations when the brainstore image tag is a short commit SHA + values: + - __fixtures__/base-values.yaml + set: + brainstore.automationwriter.replicas: 1 + brainstore.image.tag: abc1234 + release: + namespace: "braintrust" + asserts: + - equal: + path: data.BRAINSTORE_WRITER_LOOP_CONFIG + value: "exclude:automations" + - it: should exclude automations when reused values enable the pool with only replicas values: - __fixtures__/base-values.yaml diff --git a/braintrust/values.yaml b/braintrust/values.yaml index afdc025..92cad53 100644 --- a/braintrust/values.yaml +++ b/braintrust/values.yaml @@ -600,9 +600,9 @@ brainstore: # An optional, dedicated Brainstore writer pool that handles only the # automations writer loop (BRAINSTORE_WRITER_LOOP_CONFIG=include:automations). # Requires brainstore image v2.16.0 or newer. Older images ignore that - # variable, so the chart rejects replicas > 0 unless brainstore.image.tag - # is at least v2.16.0. The chart default image is older; set the tag before - # enabling the pool. + # variable, so the chart rejects replicas > 0 when brainstore.image.tag is a + # release tag (vX.Y.Z) older than v2.16.0. Commit SHAs and other custom tags + # are not checked; make sure such an image includes v2.16.0. # When enabled (replicas > 0), the regular writer pool is automatically # configured to exclude automations. Disabled by default (replicas: 0) so # existing deployments are unaffected. This option is not recommended for most From 0a7a2868d5a554e13b916bc304cd030bc1f7c58c Mon Sep 17 00:00:00 2001 From: Jeff McCollum <16550786+jeffmccollum@users.noreply.github.com> Date: Thu, 8 Oct 2026 23:26:27 +0000 Subject: [PATCH 2/2] Require planning the full input matrix before shipping chart features Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 9a27a46..e044741 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,6 +2,20 @@ This document provides guidelines for AI agents reviewing or modifying this Helm chart repository. +## Plan the Full Input Matrix First + +Before implementing a new feature, validation, or gate, list every input shape the chart already supports for each value it reads. Plan how each one behaves, and add a test for each, before the change ships. A guard written only for the common case can block a configuration that already works. For example, a `semverCompare` image floor broke every deployment that pins Brainstore by commit SHA. + +Cover at least: + +- **Image tags**: release tags (`vX.Y.Z` and `X.Y.Z`), commit SHAs (full and short, including all-digit short SHAs), pre-release tags (`-rc.N`), and other custom tags. Sprig `semverCompare` errors on anything that is not semver. Only call it after the tag matches the release-tag regex in `_brainstore-startup-gate.tpl`. +- **Cloud providers**: `aws`, `google`, `azure`, plus the examples under `braintrust/examples/` and `braintrust/ci/`. +- **Enabled and disabled paths**: the feature on, the feature off, and the transitions between them in both directions. +- **Upgrades**: `helm upgrade --reuse-values` from the previous chart version, which has missing maps, and partial `--set` overrides on top of those values. +- **Empty values**: null, missing, empty string, and `0`, especially for replica counts, where an empty field means 1 pod. + +Write the matrix and the intended behavior for each row in the PR description. Add a helm-unittest case for every row that renders differently. Treat an input shape that was never checked as a release blocker. Do not discover it after release. + ## Testing Requirements ### Running Tests @@ -102,6 +116,7 @@ When reviewing PRs, verify: - [ ] New templates follow existing patterns - [ ] Tests are added for new functionality - [ ] Cloud-specific code is properly conditioned +- [ ] The PR lists the full input matrix (image tag formats, clouds, enabled/disabled, `--reuse-values` upgrades, null/missing values), and tests cover each row ## File Structure