From 8ca6d2cfd5bc144d14fee0e4a1e51b180e2b9ba9 Mon Sep 17 00:00:00 2001
From: Mike Camp <mcamp@nvidia.com>
Date: Tue, 18 Aug 2026 14:20:55 -0400
Subject: [PATCH 1/2] fix(self-managed): render pdb-value-wiring against the
 whole stack

make test failed on the first cassandra case with "no releases found that
matches specified selector(name=cassandra)", even though cassandra is
declared in 01-dependencies.

render_chart_values tried 01-dependencies and fell back to 02-core, but it
only passed the gateway state values on the 02-core attempt. global.yaml
.gotmpl is shared by every state and marks those values required, so the
01-dependencies attempt failed on the required check, 2>/dev/null discarded
that error, and the fallback reported the one thing that was true of
02-core: it does not declare cassandra.

Run every invocation against the whole helmfile.d directory with a selector,
the way the stack is applied, and drop the fallback and the suppression.
Copy the whole stacks tree rather than self-managed alone, because
00-observability-infrastructure includes a sibling stack through a relative
path, and seed a per-test environment file for each stack the copy reaches.

Two further defects were hidden behind the first failure. The chart
validation cases rendered the published chart reference, which fails on a
chart pull offline and passed as if the chart had rejected the values; they
now render the in-repo chart and assert the both-fields and neither-field
diagnostics separately. The neither-field case could never reach that state
because base.yaml ships cassandra minAvailable: 2, so it now clears it with
the chart's own unset sentinel.

Add assert_single_release, which requires every targeted release to resolve
to exactly one release in the stack. Zero means it moved or was renamed, more
than one means two states declare it. The value assertions also move from
whole-file greps to keyed yq lookups, so a match in an unrelated release's
block no longer counts.

Closes #955

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../self-managed/tests/pdb-value-wiring.sh    | 223 +++++++++++-------
 1 file changed, 139 insertions(+), 84 deletions(-)

diff --git a/deploy/stacks/self-managed/tests/pdb-value-wiring.sh b/deploy/stacks/self-managed/tests/pdb-value-wiring.sh
index fbb7cbb0d..cb82c71f6 100755
--- a/deploy/stacks/self-managed/tests/pdb-value-wiring.sh
+++ b/deploy/stacks/self-managed/tests/pdb-value-wiring.sh
@@ -5,8 +5,11 @@
 set -euo pipefail
 
 stack_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
+stacks_dir="$(cd "$stack_dir/.." && pwd)"
+helm_dir="$(cd "$stack_dir/../../helm" && pwd)"
 work_dir="$(mktemp -d)"
-test_stack_dir="$work_dir/self-managed"
+test_stacks_dir="$work_dir/stacks"
+test_stack_dir="$test_stacks_dir/self-managed"
 environment_name="pdb-value-wiring-test"
 environment_file="$test_stack_dir/environments/$environment_name.yaml"
 secrets_file="$test_stack_dir/secrets/$environment_name-secrets.yaml"
@@ -17,63 +20,111 @@ fail() {
   exit 1
 }
 
-mkdir -p "$test_stack_dir"
-cp -R "$stack_dir"/. "$test_stack_dir"
+# Copy the whole stacks directory, not just self-managed. The self-managed
+# helmfile.d pulls in a sibling stack through a relative path, so a copy of
+# self-managed alone is not a working stack.
+mkdir -p "$test_stacks_dir"
+cp -R "$stacks_dir"/. "$test_stacks_dir"
 printf '{}\n' >"$secrets_file"
 
+# Every stack the copy reaches resolves ../environments/$HELMFILE_ENV.yaml
+# against its own directory, so each one needs a file under this test's
+# environment name.
+for stack_environments in "$test_stacks_dir"/*/environments; do
+  test -d "$stack_environments" || continue
+  test -e "$stack_environments/$environment_name.yaml" ||
+    printf '{}\n' >"$stack_environments/$environment_name.yaml"
+done
+
+# Run against the whole helmfile.d directory, the way the stack is actually
+# applied. Selecting a release out of one hand-picked state file only works
+# until that release moves, and the failure then looks like the release does
+# not exist rather than like the state file is wrong.
+#
+# global.yaml.gotmpl marks the shared gateway values as required, so every
+# invocation has to supply them, including the ones that never render a route.
+helmfile_common=(
+  --file "$test_stack_dir/helmfile.d"
+  --environment default
+  --state-values-set ingress.gatewayApi.controllerNamespace=envoy-gateway-system
+  --state-values-set ingress.gatewayApi.gateways.shared.name=shared-gw
+  --state-values-set ingress.gatewayApi.gateways.shared.namespace=envoy-gateway-system
+  --state-values-set ingress.gatewayApi.gateways.grpc.name=grpc-gw
+  --state-values-set ingress.gatewayApi.gateways.grpc.namespace=envoy-gateway-system
+)
+
+run_helmfile() {
+  HELMFILE_ENV="$environment_name" \
+    HELMFILE_CACHE_HOME="$work_dir/helmfile-cache" \
+    helmfile "${helmfile_common[@]}" "$@"
+}
+
 # ---------------------------------------------------------------------------
 # Helpers
 # ---------------------------------------------------------------------------
 
-render_chart_values() {
+# Guards the defect this harness previously hid: a selector that names a
+# release the selected state does not declare. One match is the only correct
+# answer. Zero means the release was renamed, removed, or looked for in the
+# wrong place. More than one means two states declare it, and a later apply
+# silently reinstalls whatever the earlier one decided.
+assert_single_release() {
   local release="$1"
-  local output_file="$2"
-  shift 2
 
-  HELMFILE_ENV="$environment_name" \
-    HELMFILE_CACHE_HOME="$work_dir/helmfile-cache" \
-    helmfile \
-      --file "$test_stack_dir/helmfile.d/01-dependencies.yaml.gotmpl" \
-      --environment default \
-      --selector "name=$release" \
-      "$@" \
-      write-values \
-      --output-file-template "$output_file" 2>/dev/null ||
-  HELMFILE_ENV="$environment_name" \
-    HELMFILE_CACHE_HOME="$work_dir/helmfile-cache" \
-    helmfile \
-      --file "$test_stack_dir/helmfile.d/02-core.yaml.gotmpl" \
-      --environment default \
-      --state-values-set ingress.gatewayApi.controllerNamespace=envoy-gateway-system \
-      --state-values-set ingress.gatewayApi.gateways.shared.name=shared-gw \
-      --state-values-set ingress.gatewayApi.gateways.shared.namespace=envoy-gateway-system \
-      --state-values-set ingress.gatewayApi.gateways.grpc.name=grpc-gw \
-      --state-values-set ingress.gatewayApi.gateways.grpc.namespace=envoy-gateway-system \
-      --selector "name=$release" \
-      "$@" \
-      write-values \
-      --output-file-template "$output_file"
+  # helmfile exits non-zero when a selector matches nothing, so the count has
+  # to survive that rather than take the whole script down with it.
+  local listed matches
+  listed="$(run_helmfile --selector "name=$release" list --skip-charts --output json 2>/dev/null || true)"
+  matches="$(printf '%s' "${listed:-[]}" |
+    jq -r --arg name "$release" '[.[] | select(.name == $name)] | length' 2>/dev/null || true)"
+  test "${matches:-0}" = "1" ||
+    fail "expected exactly one release named $release in the stack, found ${matches:-0}"
 }
 
-pdb_enabled_in_values() {
-  local values_file="$1"
-  local key_prefix="$2"   # e.g. "cassandra" or "" for root-level
-  grep -q "enabled: true" "$values_file"
-}
+render_chart_values() {
+  local release="$1"
+  local output_file="$2"
+  shift 2
 
-pdb_key_present() {
-  local values_file="$1"
-  grep -q "podDisruptionBudget:" "$values_file"
-}
+  run_helmfile \
+    --selector "name=$release" \
+    "$@" \
+    write-values \
+    --output-file-template "$output_file"
 
-pdb_min_available() {
-  local values_file="$1"
-  grep -q "minAvailable:" "$values_file"
+  test -s "$output_file" ||
+    fail "$release: helmfile wrote no values to $output_file"
 }
 
-pdb_max_unavailable() {
-  local values_file="$1"
-  grep -q "maxUnavailable:" "$values_file"
+# Renders a release against its in-repo chart so the chart's own validation is
+# what fails. Without --chart the run fails on a chart pull instead, which
+# looks the same to a test that only checks for a non-zero exit.
+expect_chart_error() {
+  local release="$1"
+  local chart_dir="$2"
+  local log_file="$3"
+  local expected_error="$4"
+  local forbidden_error="${5:-}"
+
+  if run_helmfile \
+    --selector "name=$release" \
+    --chart "$chart_dir" \
+    --skip-deps \
+    template >"$log_file" 2>&1; then
+    fail "$release: expected the chart to reject this configuration"
+  fi
+
+  grep -Fq "$expected_error" "$log_file" ||
+    fail "$release: error did not contain the expected message: $expected_error"
+
+  # The both-fields and neither-field diagnostics share a prefix, so each case
+  # has to rule out the other one. Without this, a chart that collapsed the two
+  # branches into one message would still pass both cases.
+  if test -n "$forbidden_error"; then
+    if grep -Fq "$forbidden_error" "$log_file"; then
+      fail "$release: matched the wrong diagnostic: $forbidden_error"
+    fi
+  fi
 }
 
 write_env() {
@@ -81,7 +132,7 @@ write_env() {
 }
 
 # ---------------------------------------------------------------------------
-# 1. Cassandra PDB — omitted (default off)
+# 0. Stack topology
 # ---------------------------------------------------------------------------
 write_env <<'EOF'
 global:
@@ -90,14 +141,22 @@ global:
     repository: test/nvcf
 EOF
 
+for release in cassandra nats openbao-server ratelimiter ess-api; do
+  assert_single_release "$release"
+done
+
+# ---------------------------------------------------------------------------
+# 1. Cassandra PDB - omitted (default off)
+# ---------------------------------------------------------------------------
 render_chart_values cassandra "$work_dir/cassandra-off-values.yaml" >/dev/null
 if grep -q "podDisruptionBudget:" "$work_dir/cassandra-off-values.yaml" &&
-   grep -A2 "podDisruptionBudget:" "$work_dir/cassandra-off-values.yaml" | grep -q "enabled: true"; then
+  yq -e '.cassandra.podDisruptionBudget.enabled == true' \
+    "$work_dir/cassandra-off-values.yaml" >/dev/null 2>&1; then
   fail "cassandra: PDB should be disabled by default but rendered enabled: true"
 fi
 
 # ---------------------------------------------------------------------------
-# 2. Cassandra PDB — minAvailable passthrough
+# 2. Cassandra PDB - minAvailable passthrough
 # ---------------------------------------------------------------------------
 write_env <<'EOF'
 global:
@@ -111,13 +170,15 @@ cassandra:
 EOF
 
 render_chart_values cassandra "$work_dir/cassandra-min-values.yaml" >/dev/null
-grep -A2 "podDisruptionBudget:" "$work_dir/cassandra-min-values.yaml" | grep -q "enabled: true" ||
+yq -e '.cassandra.podDisruptionBudget.enabled == true' \
+  "$work_dir/cassandra-min-values.yaml" >/dev/null ||
   fail "cassandra: minAvailable PDB did not set enabled: true"
-grep -q "minAvailable: 2" "$work_dir/cassandra-min-values.yaml" ||
+yq -e '.cassandra.podDisruptionBudget.minAvailable == 2' \
+  "$work_dir/cassandra-min-values.yaml" >/dev/null ||
   fail "cassandra: minAvailable: 2 did not reach the chart values"
 
 # ---------------------------------------------------------------------------
-# 3. Cassandra PDB — maxUnavailable passthrough
+# 3. Cassandra PDB - maxUnavailable passthrough
 # ---------------------------------------------------------------------------
 write_env <<'EOF'
 global:
@@ -131,11 +192,12 @@ cassandra:
 EOF
 
 render_chart_values cassandra "$work_dir/cassandra-max-values.yaml" >/dev/null
-grep -q "maxUnavailable: 1" "$work_dir/cassandra-max-values.yaml" ||
+yq -e '.cassandra.podDisruptionBudget.maxUnavailable == 1' \
+  "$work_dir/cassandra-max-values.yaml" >/dev/null ||
   fail "cassandra: maxUnavailable: 1 did not reach the chart values"
 
 # ---------------------------------------------------------------------------
-# 4. Cassandra PDB — chart fail: both fields set
+# 4. Cassandra PDB - chart fail: both fields set
 # ---------------------------------------------------------------------------
 write_env <<'EOF'
 global:
@@ -149,21 +211,16 @@ cassandra:
     maxUnavailable: 1
 EOF
 
-# helm template should fail when both fields are set
-if HELMFILE_ENV="$environment_name" \
-   HELMFILE_CACHE_HOME="$work_dir/helmfile-cache" \
-   helmfile \
-     --file "$test_stack_dir/helmfile.d/01-dependencies.yaml.gotmpl" \
-     --environment default \
-     --selector name=cassandra \
-     template >"$work_dir/cassandra-both.log" 2>&1; then
-  fail "cassandra: setting both minAvailable and maxUnavailable should have failed"
-fi
-grep -qi "exactly one" "$work_dir/cassandra-both.log" ||
-  fail "cassandra: both-fields error did not contain expected message"
+expect_chart_error cassandra "$helm_dir/cassandra/helm" \
+  "$work_dir/cassandra-both.log" \
+  "podDisruptionBudget: set exactly one of minAvailable or maxUnavailable, not both"
 
 # ---------------------------------------------------------------------------
-# 4b. Cassandra PDB — chart fail: neither field set
+# 4b. Cassandra PDB - chart fail: neither field set
+#
+# base.yaml ships minAvailable: 2, so an environment that only sets enabled
+# still inherits an availability field. Clear it with the chart's own unset
+# sentinel to reach the neither-field state the chart is supposed to reject.
 # ---------------------------------------------------------------------------
 write_env <<'EOF'
 global:
@@ -173,23 +230,17 @@ global:
 cassandra:
   podDisruptionBudget:
     enabled: true
+    minAvailable: ""
 EOF
 
-# helm template should fail when enabled but neither field is set
-if HELMFILE_ENV="$environment_name" \
-   HELMFILE_CACHE_HOME="$work_dir/helmfile-cache" \
-   helmfile \
-     --file "$test_stack_dir/helmfile.d/01-dependencies.yaml.gotmpl" \
-     --environment default \
-     --selector name=cassandra \
-     template >"$work_dir/cassandra-neither.log" 2>&1; then
-  fail "cassandra: enabled with neither minAvailable nor maxUnavailable should have failed"
-fi
-grep -qi "exactly one" "$work_dir/cassandra-neither.log" ||
-  fail "cassandra: neither-field error did not contain expected message"
+expect_chart_error cassandra "$helm_dir/cassandra/helm" \
+  "$work_dir/cassandra-neither.log" \
+  "podDisruptionBudget: set exactly one of minAvailable or maxUnavailable" \
+  "not both"
+
 
 # ---------------------------------------------------------------------------
-# 5. NATS PDB — passthrough (upstream chart; enabled by default)
+# 5. NATS PDB - passthrough (upstream chart; enabled by default)
 # ---------------------------------------------------------------------------
 write_env <<'EOF'
 global:
@@ -202,8 +253,8 @@ nats:
 EOF
 
 render_chart_values nats "$work_dir/nats-disabled-values.yaml" >/dev/null
-# The nats wrapper values should reflect the override
-grep -A2 "podDisruptionBudget:" "$work_dir/nats-disabled-values.yaml" | grep -q "enabled: false" ||
+yq -e '.nats.podDisruptionBudget.enabled == false' \
+  "$work_dir/nats-disabled-values.yaml" >/dev/null ||
   fail "nats: podDisruptionBudget.enabled: false did not reach chart values"
 
 # ---------------------------------------------------------------------------
@@ -223,7 +274,8 @@ openbao:
 EOF
 
 render_chart_values openbao-server "$work_dir/openbao-ha-values.yaml" >/dev/null
-grep -q "maxUnavailable: 1" "$work_dir/openbao-ha-values.yaml" ||
+yq -e '.openbao.server.ha.disruptionBudget.maxUnavailable == 1' \
+  "$work_dir/openbao-ha-values.yaml" >/dev/null ||
   fail "openbao: server.ha.disruptionBudget.maxUnavailable: 1 did not reach chart values"
 
 # ---------------------------------------------------------------------------
@@ -241,7 +293,8 @@ openbao:
 EOF
 
 render_chart_values openbao-server "$work_dir/openbao-injector-values.yaml" >/dev/null
-grep -q "minAvailable: 2" "$work_dir/openbao-injector-values.yaml" ||
+yq -e '.openbao.injector.podDisruptionBudget.minAvailable == 2' \
+  "$work_dir/openbao-injector-values.yaml" >/dev/null ||
   fail "openbao: injector.podDisruptionBudget.minAvailable: 2 did not reach chart values"
 
 # ---------------------------------------------------------------------------
@@ -259,7 +312,8 @@ rateLimiter:
 EOF
 
 render_chart_values ratelimiter "$work_dir/ratelimiter-values.yaml" >/dev/null
-grep -A2 "podDisruptionBudget:" "$work_dir/ratelimiter-values.yaml" | grep -q "enabled: true" ||
+yq -e '.rateLimiter.podDisruptionBudget.enabled == true' \
+  "$work_dir/ratelimiter-values.yaml" >/dev/null ||
   fail "rateLimiter: podDisruptionBudget.enabled: true did not reach chart values"
 
 # ---------------------------------------------------------------------------
@@ -277,7 +331,8 @@ ess:
 EOF
 
 render_chart_values ess-api "$work_dir/ess-values.yaml" >/dev/null
-grep -A2 "podDisruptionBudget:" "$work_dir/ess-values.yaml" | grep -q "enabled: true" ||
+yq -e '.ess.podDisruptionBudget.enabled == true' \
+  "$work_dir/ess-values.yaml" >/dev/null ||
   fail "ess: podDisruptionBudget.enabled: true did not reach chart values"
 
 echo "pdb-value-wiring: all checks passed"

From a72eaca6f145b744a13862296fb01e8065bc8c41 Mon Sep 17 00:00:00 2001
From: Mike Camp <mcamp@nvidia.com>
Date: Tue, 18 Aug 2026 14:33:23 -0400
Subject: [PATCH 2/2] test(self-managed): report pdb-value-wiring render
 failures as themselves

Review follow-up on #956.

assert_single_release swallowed every non-zero helmfile exit, so a stack
that failed to render was reported as a missing release. That is the same
misdirection this test was fixed for. It now separates the two: a selector
that matches nothing still reports the count, anything else prints the
helmfile error and fails as a render failure. render_chart_values gained the
same handling, so a write-values failure is attributed rather than dumped
raw.

Drop the redundant grep from the default-off case. The yq check already
covers it, and the conjunction hid the case where the key renders under
unexpected nesting.

Create the secrets directory before writing the test secrets file. The
directory is tracked today, so this is defensive only.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../self-managed/tests/pdb-value-wiring.sh    | 32 +++++++++++++------
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/deploy/stacks/self-managed/tests/pdb-value-wiring.sh b/deploy/stacks/self-managed/tests/pdb-value-wiring.sh
index cb82c71f6..7673757c3 100755
--- a/deploy/stacks/self-managed/tests/pdb-value-wiring.sh
+++ b/deploy/stacks/self-managed/tests/pdb-value-wiring.sh
@@ -25,6 +25,7 @@ fail() {
 # self-managed alone is not a working stack.
 mkdir -p "$test_stacks_dir"
 cp -R "$stacks_dir"/. "$test_stacks_dir"
+mkdir -p "$(dirname "$secrets_file")"
 printf '{}\n' >"$secrets_file"
 
 # Every stack the copy reaches resolves ../environments/$HELMFILE_ENV.yaml
@@ -71,10 +72,20 @@ run_helmfile() {
 assert_single_release() {
   local release="$1"
 
-  # helmfile exits non-zero when a selector matches nothing, so the count has
-  # to survive that rather than take the whole script down with it.
-  local listed matches
-  listed="$(run_helmfile --selector "name=$release" list --skip-charts --output json 2>/dev/null || true)"
+  # helmfile exits non-zero both when a selector matches nothing and when the
+  # stack fails to render. Only the first is this check's business. Reporting a
+  # render failure as a missing release is the same misdirection this test was
+  # fixed for, so let it surface as itself.
+  local list_log="$work_dir/$release-list.log"
+  local listed status=0
+  listed="$(run_helmfile --selector "name=$release" list --skip-charts --output json 2>"$list_log")" ||
+    status=$?
+  if test "$status" -ne 0 && ! grep -Fq "no releases found" "$list_log"; then
+    cat "$list_log" >&2
+    fail "$release: helmfile could not render the stack"
+  fi
+
+  local matches
   matches="$(printf '%s' "${listed:-[]}" |
     jq -r --arg name "$release" '[.[] | select(.name == $name)] | length' 2>/dev/null || true)"
   test "${matches:-0}" = "1" ||
@@ -86,11 +97,15 @@ render_chart_values() {
   local output_file="$2"
   shift 2
 
-  run_helmfile \
+  local render_log="$work_dir/$release-write-values.log"
+  if ! run_helmfile \
     --selector "name=$release" \
     "$@" \
     write-values \
-    --output-file-template "$output_file"
+    --output-file-template "$output_file" 2>"$render_log"; then
+    cat "$render_log" >&2
+    fail "$release: helmfile could not render the stack"
+  fi
 
   test -s "$output_file" ||
     fail "$release: helmfile wrote no values to $output_file"
@@ -149,9 +164,8 @@ done
 # 1. Cassandra PDB - omitted (default off)
 # ---------------------------------------------------------------------------
 render_chart_values cassandra "$work_dir/cassandra-off-values.yaml" >/dev/null
-if grep -q "podDisruptionBudget:" "$work_dir/cassandra-off-values.yaml" &&
-  yq -e '.cassandra.podDisruptionBudget.enabled == true' \
-    "$work_dir/cassandra-off-values.yaml" >/dev/null 2>&1; then
+if yq -e '.cassandra.podDisruptionBudget.enabled == true' \
+  "$work_dir/cassandra-off-values.yaml" >/dev/null 2>&1; then
   fail "cassandra: PDB should be disabled by default but rendered enabled: true"
 fi