From 376181aea6b37af5c972f09b53ca02d600c08b9b Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Fri, 11 Sep 2026 10:47:39 +0200 Subject: [PATCH 1/5] fix(chart): reconcile the telemetry Collector in place once its token Secret exists The -telemetry-status record for skipped-no-token promised the skip "resolves itself once jobs-manager writes the Secret". Nothing performed that: the lookup deciding the Collector's three-state rule runs only inside a helm upgrade, and the hourly auto-upgrade tick upgraded only onto a NEWER published chart. Measured on a fresh edge: Secret present within minutes, three ticks, no DaemonSet, until a new chart happened to ship a day later. Tracked internally. Mechanism: with no newer chart to install, the tick now runs one tri-state decider (telemetry_reconcile_verdict): stored state skipped-no-token AND no Collector DaemonSet in the stored manifest AND the repo serves the deployed version AND a --dry-run=server of that same version resolves `enabled` -> a same-version `helm upgrade --version "$CURRENT"` with the normal flags. Any other state behaves as before; "cannot tell" (unreadable manifest, failed dry-run, contradictory record, version not served by the repo) never acts and is logged. Derived from the chart's own decider and helpers; nothing restated. Record: names the real trigger (the next auto-upgrade tick, schedule and repo read from autoUpgrade.*; or "run helm upgrade" when autoUpgrade is off) and says plainly that a version ahead of the repo's index cannot be re-rendered. Tests: scripts/tests/auto-upgrade-telemetry-reconcile.sh drives the rendered script through a stub helm (23 cases) and its -mutations.sh sibling proves all four gates red on their own regression; both registered in `make drift`. helm-unittest +7 (kubernetesProvider reaches skipped-no-token offline). Chart 1.9.116 -> 1.9.117. Co-Authored-By: Claude Fable 5.1 --- Makefile | 2 + client/Chart.yaml | 4 +- client/templates/_helpers.tpl | 23 ++ client/templates/auto-upgrade-cronjob.yaml | 133 ++++++- .../templates/telemetry-collector-status.yaml | 31 +- client/tests/auto_upgrade_test.yaml | 39 ++ client/tests/telemetry_collector_test.yaml | 118 ++++++ .../tests/telemetry_status_wiring_test.yaml | 87 +++++ client/values.yaml | 12 + ...o-upgrade-telemetry-reconcile-mutations.sh | 127 +++++++ .../tests/auto-upgrade-telemetry-reconcile.sh | 357 ++++++++++++++++++ 11 files changed, 924 insertions(+), 9 deletions(-) create mode 100644 client/tests/telemetry_status_wiring_test.yaml create mode 100755 scripts/tests/auto-upgrade-telemetry-reconcile-mutations.sh create mode 100755 scripts/tests/auto-upgrade-telemetry-reconcile.sh diff --git a/Makefile b/Makefile index e292e16a..639c3958 100644 --- a/Makefile +++ b/Makefile @@ -390,6 +390,8 @@ DRIFT_GUARDS := scripts/gen-manifest.sh --check|\ bash scripts/tests/gate-default-prose-mutations.sh|\ bash scripts/tests/guards-survive-spaced-paths.sh|\ bash scripts/tests/auto-upgrade-inflight-vs-wedge.sh|\ + bash scripts/tests/auto-upgrade-telemetry-reconcile.sh|\ + bash scripts/tests/auto-upgrade-telemetry-reconcile-mutations.sh|\ bash scripts/tests/hostpath-reads-guarded.sh|\ bash scripts/tests/jobs-manager-waits-for-mysql.sh|\ bash scripts/tests/control-plane-footprint.sh|\ diff --git a/client/Chart.yaml b/client/Chart.yaml index d8b3ed31..ecd2e38c 100644 --- a/client/Chart.yaml +++ b/client/Chart.yaml @@ -2,8 +2,8 @@ apiVersion: v2 name: client description: A unified Helm chart for tracebloc on AKS, EKS, bare-metal, and OpenShift type: application -version: 1.9.116 -appVersion: "1.9.116" +version: 1.9.117 +appVersion: "1.9.117" keywords: - tracebloc - kubernetes diff --git a/client/templates/_helpers.tpl b/client/templates/_helpers.tpl index 0f7360b3..c564f4af 100644 --- a/client/templates/_helpers.tpl +++ b/client/templates/_helpers.tpl @@ -1331,6 +1331,29 @@ can be kept above the configured helm timeout. {{- printf "%s-telemetry-collector" (include "tracebloc.fullname" .) | trunc 63 | trimSuffix "-" -}} {{- end -}} +{{/* + tracebloc.telemetryStatusName — the ConfigMap that records what the Collector + decided, and why (templates/telemetry-collector-status.yaml). ONE resolver for + its two readers: the status template that writes it and the auto-upgrade + CronJob that reads it back out of the stored release manifest to decide whether + a same-version re-render is due (backend#3550). Deliberately NOT under the + `telemetry-collector` prefix — see the status template for why five shell gates + depend on that. +*/}} +{{- define "tracebloc.telemetryStatusName" -}} +{{- printf "%s-telemetry-status" (include "tracebloc.fullname" .) -}} +{{- end -}} + +{{/* + tracebloc.telemetryStateAnnotation — the annotation key the status ConfigMap + carries its resolved state under. Shared by the writer (status template) and the + reader (auto-upgrade script, which greps it out of `helm get manifest` and out of + a server-side dry-run render) so the two cannot disagree about the spelling. +*/}} +{{- define "tracebloc.telemetryStateAnnotation" -}} +tracebloc.io/telemetry-collector-state +{{- end -}} + {{/* tracebloc.telemetryTokenLegacyName — the pre-backend#2625 fixed Secret name. diff --git a/client/templates/auto-upgrade-cronjob.yaml b/client/templates/auto-upgrade-cronjob.yaml index 1ffdbdfd..2ef460e0 100644 --- a/client/templates/auto-upgrade-cronjob.yaml +++ b/client/templates/auto-upgrade-cronjob.yaml @@ -116,6 +116,123 @@ data: }' } + # --- telemetry Collector same-version reconcile: begin (backend#3550) ------- + # The Collector's three-state rule (`telemetryCollector.enabled` unset -> + # collect iff the token Secret exists, else record `skipped-no-token`) is + # decided by a template `lookup`, and a `lookup` runs ONLY inside a + # `helm upgrade`. This tick used to upgrade only onto a NEWER published chart, + # so the Secret appearing (jobs-manager writes it minutes after install) + # triggered nothing: measured on a fresh edge, three hourly ticks passed with + # the Secret present and no Collector, which then rendered a day later only + # because a new chart happened to ship. So a tick that has no newer chart to + # install now asks the one question that matters and re-renders the DEPLOYED + # version in place when the answer is yes. + # + # DERIVED, NOT RESTATED. The decision is read from the chart's own decider, + # twice: the stored state out of `helm get manifest` (what the last render + # decided) and the would-be state out of a server-side dry-run of the SAME + # chart version (`--dry-run=server`, helm 3.13+, runs the templates' real + # `lookup`s against the cluster). No Secret name, Class A rule or DaemonSet + # gate is copied here; the three names this needs arrive in env from the same + # helpers the templates render with (TELEMETRY_STATUS_NAME, + # TELEMETRY_STATUS_ANNOTATION, TELEMETRY_COLLECTOR_NAME). + # + # TRI-STATE, and "cannot tell" never acts: 0 reconcile / 1 nothing to do / + # 2 cannot tell. An unreadable manifest, a failed dry-run, a contradictory + # record, or a deployed version the repo cannot serve is not evidence either + # way, so the tick leaves the release alone and says so on stdout. + # + # AHEAD OF THE INDEX IS THE ONE STATE THIS CANNOT FIX, said plainly: a release + # deployed from a dev chart (current > latest) runs a version the repo does + # not offer, and nothing here can re-render a chart it cannot fetch. That edge + # converges on the next published chart, and the verdict names it every tick. + + # Resolved Collector state carried by the manifest on stdin: the state + # annotation of the ConfigMap named TELEMETRY_STATUS_NAME. Prints nothing when + # the record is absent (a release rendered by a chart that predates it). + telemetry_state_of() { + awk -v name="$TELEMETRY_STATUS_NAME" -v key="$TELEMETRY_STATUS_ANNOTATION" ' + /^---/ { doc = ""; next } + /^ name: / && $2 == name { doc = "status"; next } + doc == "status" && $1 == key ":" { v = $2; gsub(/"/, "", v); print v; exit }' + } + + # Exit 0 iff the manifest on stdin carries a DaemonSet named + # TELEMETRY_COLLECTOR_NAME, i.e. the last render DID enable the Collector. + telemetry_manifest_has_collector() { + awk -v name="$TELEMETRY_COLLECTOR_NAME" ' + /^---/ { kind = ""; next } + /^kind: / { kind = $2; next } + kind == "DaemonSet" && /^ name: / && $2 == name { found = 1; exit } + END { exit found ? 0 : 1 }' + } + + # THE condition, in one place, so the test and its mutation drive this code + # and not a copy. Reads helm; prints one reason line; returns the tri-state. + # Reconcile iff: the stored record says skipped-no-token (which by the + # decider's construction means `enabled` was unset) AND the stored manifest + # carries no Collector DaemonSet AND a server-side dry-run of the deployed + # chart version now resolves to `enabled` (the token Secret has arrived). + telemetry_reconcile_verdict() { + if [ -z "${TELEMETRY_STATUS_NAME:-}" ] || [ -z "${TELEMETRY_STATUS_ANNOTATION:-}" ] \ + || [ -z "${TELEMETRY_COLLECTOR_NAME:-}" ]; then + echo "reconcile inputs missing from env (TELEMETRY_STATUS_NAME / TELEMETRY_STATUS_ANNOTATION / TELEMETRY_COLLECTOR_NAME)" + return 2 + fi + _manifest="$(helm get manifest "$RELEASE_NAME" -n "$RELEASE_NAMESPACE" 2>/dev/null)" \ + || { echo "cannot read the deployed manifest (helm get manifest failed)"; return 2; } + [ -n "$_manifest" ] || { echo "cannot read the deployed manifest (empty)"; return 2; } + _stored="$(printf '%s\n' "$_manifest" | telemetry_state_of)" + case "$_stored" in + "") echo "release carries no telemetry status record (chart predates it); nothing to reconcile"; return 1 ;; + skipped-no-token) ;; + *) echo "telemetry Collector state is '$_stored'; nothing to reconcile"; return 1 ;; + esac + if printf '%s\n' "$_manifest" | telemetry_manifest_has_collector; then + echo "status record says skipped-no-token but the release already carries DaemonSet $TELEMETRY_COLLECTOR_NAME: contradictory, not touching it" + return 2 + fi + _search="$(helm search repo "${REPO_NAME}/${CHART_NAME}" --version "$CURRENT" -o yaml 2>&1)" \ + || { echo "cannot ask $REPO_URL whether it serves $CURRENT: $(printf '%s\n' "$_search" | tail -n 1)"; return 2; } + _served="$(printf '%s\n' "$_search" | awk '/^[[:space:]]*version:/ {print $2; exit}')" + [ "$_served" = "$CURRENT" ] \ + || { echo "deployed chart $CURRENT is not served by $REPO_URL (newest there: $LATEST): a same-version re-render is impossible, the Collector renders on the next published chart"; return 2; } + _render="$(helm upgrade "$RELEASE_NAME" "${REPO_NAME}/${CHART_NAME}" \ + --namespace "$RELEASE_NAMESPACE" --version "$CURRENT" \ + --reset-then-reuse-values --dry-run=server --hide-secret 2>&1)" \ + || { echo "server-side dry-run of $CURRENT failed: $(printf '%s\n' "$_render" | tail -n 1)"; return 2; } + _would="$(printf '%s\n' "$_render" | telemetry_state_of)" + case "$_would" in + enabled) echo "token Secret is now present, a re-render of $CURRENT enables the Collector"; return 0 ;; + "") echo "dry-run render of $CURRENT carries no telemetry status record; cannot tell"; return 2 ;; + *) echo "a re-render would still decide '$_would'; nothing to reconcile"; return 1 ;; + esac + } + + # Runs where this tick has no newer chart to install. The reconcile IS the + # upgrade at the bottom of this script, pinned to the DEPLOYED version, so it + # keeps every flag and the same values discipline. Only verdict 0 acts; 2 is + # logged because "cannot tell" must never be silent; 1 stays quiet, as today. + telemetry_reconcile_if_needed() { + if _why="$(telemetry_reconcile_verdict)"; then + log "reconcile: $_why; re-rendering $CURRENT in place (same-version upgrade, values reused)" + helm upgrade "$RELEASE_NAME" "${REPO_NAME}/${CHART_NAME}" \ + --namespace "$RELEASE_NAMESPACE" \ + --version "$CURRENT" \ + --reset-then-reuse-values \ + --atomic \ + --cleanup-on-fail \ + --timeout "$UPGRADE_TIMEOUT" + log "reconcile complete: $CURRENT re-rendered in place" + else + _rc=$? + if [ "$_rc" -eq 2 ]; then + log "reconcile: NOT re-rendering, cannot tell whether the Collector is due: $_why" + fi + fi + } + # --- telemetry Collector same-version reconcile: end ------------------------ + log "release=$RELEASE_NAME namespace=$RELEASE_NAMESPACE repo=$REPO_URL" helm repo add "$REPO_NAME" "$REPO_URL" >/dev/null @@ -229,14 +346,16 @@ data: log "current=$CURRENT latest=$LATEST" if [ "$LATEST" = "$CURRENT" ]; then - log "already at latest; nothing to do" + log "already at latest; nothing to upgrade" + telemetry_reconcile_if_needed exit 0 fi # Semver-aware compare via `sort -V` so 1.10.0 > 1.9.0. NEWER="$(printf '%s\n%s\n' "$CURRENT" "$LATEST" | sort -V | tail -n 1)" if [ "$NEWER" != "$LATEST" ]; then - log "deployed version is ahead of repo (current=$CURRENT > latest=$LATEST); skipping" + log "deployed version is ahead of repo (current=$CURRENT > latest=$LATEST); skipping the upgrade" + telemetry_reconcile_if_needed exit 0 fi @@ -374,6 +493,16 @@ spec: # predates the key (same pattern as activeDeadlineSeconds). - name: WEDGE_MIN_AGE_SECONDS value: {{ include "tracebloc.durationSeconds" (.Values.autoUpgrade.pendingWedgeMinAge | default "45m") | quote }} + # backend#3550: the names the same-version reconcile reads the + # stored manifest and the dry-run render by. Rendered from the SAME + # helpers the status template and the Collector DaemonSet use, so + # the reader and the writers cannot disagree about a spelling. + - name: TELEMETRY_STATUS_NAME + value: {{ include "tracebloc.telemetryStatusName" . | quote }} + - name: TELEMETRY_STATUS_ANNOTATION + value: {{ include "tracebloc.telemetryStateAnnotation" . | quote }} + - name: TELEMETRY_COLLECTOR_NAME + value: {{ include "tracebloc.telemetryCollectorName" . | quote }} securityContext: allowPrivilegeEscalation: false readOnlyRootFilesystem: true diff --git a/client/templates/telemetry-collector-status.yaml b/client/templates/telemetry-collector-status.yaml index 5f0d9d77..76a7190c 100644 --- a/client/templates/telemetry-collector-status.yaml +++ b/client/templates/telemetry-collector-status.yaml @@ -30,10 +30,31 @@ label selectors. */}} {{- $state := include "tracebloc.telemetryCollectorState" . -}} +{{- /* + WHAT ACTUALLY RE-RENDERS A SKIP (backend#3550). The record used to say the + no-token skip "resolves itself once jobs-manager writes the Secret". It does + not: the `lookup` that decides the state runs only inside a `helm upgrade`, and + the Secret appearing triggers none. Measured on a fresh edge — the Secret was + present within minutes, three hourly ticks passed, no Collector; it rendered a + day later, when a NEWER chart happened to be published. So the trigger is a + helm upgrade, and the record must name the one that will actually happen. The + auto-upgrade tick now performs a same-version re-render when the stored state is + skipped-no-token and a server-side dry-run says the Secret has arrived + (auto-upgrade-cronjob.yaml), but only when the deployed chart version is one + its repo can serve — an edge running a version AHEAD of the published index + (a dev chart) has nothing the tick can re-render, and waits for the next + published chart. Both the schedule and the repo are read from the values that + drive that CronJob, never restated here. +*/ -}} +{{- $au := default (dict) .Values.autoUpgrade -}} +{{- $trigger := "autoUpgrade is off in this release, so nothing re-renders it for you: run `helm upgrade` on this release once the Secret exists" -}} +{{- if $au.enabled -}} +{{- $trigger = printf "the next auto-upgrade tick (CronJob schedule %q) re-renders this release in place once the Secret exists, provided the deployed chart version %s is one the repo %s serves — a version ahead of that repo's index cannot be re-rendered and waits for the next published chart" ($au.schedule | default "unset" | toString) .Chart.Version ($au.repoUrl | default "unset" | toString) -}} +{{- end -}} apiVersion: v1 kind: ConfigMap metadata: - name: {{ printf "%s-telemetry-status" (include "tracebloc.fullname" .) }} + name: {{ include "tracebloc.telemetryStatusName" . }} namespace: {{ .Values.nodeAgents.namespace.name }} labels: {{- include "tracebloc.labels" . | nindent 4 }} @@ -41,8 +62,8 @@ metadata: annotations: # Sweepable across a cluster without parsing the data block: # kubectl get cm -A -l app.kubernetes.io/component=telemetry-collector-status \ - # -o custom-columns=NS:.metadata.namespace,NAME:.metadata.name,STATE:.metadata.annotations.tracebloc\.io/telemetry-collector-state - tracebloc.io/telemetry-collector-state: {{ $state | quote }} + # -o custom-columns=NS:.metadata.namespace,NAME:.metadata.name,STATE:.metadata.annotations.{{ include "tracebloc.telemetryStateAnnotation" . | replace "." "\\." }} + {{ include "tracebloc.telemetryStateAnnotation" . }}: {{ $state | quote }} data: state: {{ $state | quote }} # WHY, in the operator's terms — the state alone does not say what to do next. @@ -51,7 +72,7 @@ data: {{- else if eq $state "disabled-by-operator" -}} "telemetryCollector.enabled is explicitly false in this release's values. Nothing is collected, by choice." {{- else if eq $state "skipped-incomplete-values" -}} - {{ printf "No Collector: nobody set telemetryCollector.enabled, and this release carries no Class A container lists — helm DELETES a key set to null and does not coalesce chart defaults back under --reuse-values, so a release predating the Collector arrives without them. Upgrading with --reset-then-reuse-values (which auto-upgrade uses) restores them and this resolves itself." | quote }} + {{ printf "No Collector: nobody set telemetryCollector.enabled, and this release carries no Class A container lists — helm DELETES a key set to null and does not coalesce chart defaults back under --reuse-values, so a release predating the Collector arrives without them. Upgrading with --reset-then-reuse-values (which auto-upgrade uses) restores them — on the next chart upgrade, which is the next auto-upgrade tick that finds a newer published chart or a `helm upgrade` you run; nothing re-renders this release by itself." | quote }} {{- else -}} - {{ printf "No Collector: nobody set telemetryCollector.enabled, and its token Secret is not present in namespace %q (looked for %q, and the legacy %q). jobs-manager writes that Secret when it next re-authenticates (backend#2274); this resolves itself once it does. Set telemetryCollector.enabled: false to opt out permanently." .Values.nodeAgents.namespace.name (include "tracebloc.telemetryTokenSecretName" .) (include "tracebloc.telemetryTokenLegacyName" .) | quote }} + {{ printf "No Collector: nobody set telemetryCollector.enabled, and its token Secret is not present in namespace %q (looked for %q, and the legacy %q). jobs-manager writes that Secret when it next re-authenticates (backend#2274). The Secret appearing triggers nothing by itself — the Collector renders on the next helm upgrade of this release: %s. Set telemetryCollector.enabled: false to opt out permanently." .Values.nodeAgents.namespace.name (include "tracebloc.telemetryTokenSecretName" .) (include "tracebloc.telemetryTokenLegacyName" .) $trigger | quote }} {{- end }} diff --git a/client/tests/auto_upgrade_test.yaml b/client/tests/auto_upgrade_test.yaml index 09e0fbca..e8312da0 100644 --- a/client/tests/auto_upgrade_test.yaml +++ b/client/tests/auto_upgrade_test.yaml @@ -213,6 +213,45 @@ tests: path: data["auto-upgrade.sh"] pattern: 'oh > 23 \|\| om > 59' + # backend#3550: with no newer chart to install, the tick re-renders the DEPLOYED + # version in place once the Collector's token Secret exists. The condition is + # DRIVEN, not read, by scripts/tests/auto-upgrade-telemetry-reconcile.sh (a stub + # helm, 23 cases, tri-state); what this pins is the shape a reader can verify + # here: the decision reads the chart's own decider (stored manifest + a + # server-side dry-run), the reconcile is pinned to $CURRENT, and both no-upgrade + # branches reach it. + - it: script reconciles the telemetry Collector in place when its token arrives (backend#3550) + template: templates/auto-upgrade-cronjob.yaml + documentIndex: 0 + asserts: + - matchRegex: + path: data["auto-upgrade.sh"] + pattern: "telemetry_reconcile_verdict\\(\\) \\{" + # Reads the stored decision, never re-deriving the Secret rule here. + - matchRegex: + path: data["auto-upgrade.sh"] + pattern: 'helm get manifest "\$RELEASE_NAME"' + # Asks the chart's real lookups what a re-render would decide. + - matchRegex: + path: data["auto-upgrade.sh"] + pattern: '--version "\$CURRENT" \\\n\s*--reset-then-reuse-values --dry-run=server --hide-secret' + # The reconcile IS the upgrade, pinned to the deployed version, same flags. + - matchRegex: + path: data["auto-upgrade.sh"] + pattern: '(?s)telemetry_reconcile_if_needed\(\) \{.*--version "\$CURRENT".*--reset-then-reuse-values.*--atomic.*--cleanup-on-fail.*--timeout "\$UPGRADE_TIMEOUT"' + # Both branches that install nothing consult it; the newer-chart path does + # not need to, because that upgrade re-renders anyway. + - matchRegex: + path: data["auto-upgrade.sh"] + pattern: '(?s)already at latest; nothing to upgrade"\s*\n\s*telemetry_reconcile_if_needed' + - matchRegex: + path: data["auto-upgrade.sh"] + pattern: '(?s)skipping the upgrade"\s*\n\s*telemetry_reconcile_if_needed' + # Verdict 2 is logged, never acted on. + - matchRegex: + path: data["auto-upgrade.sh"] + pattern: 'reconcile: NOT re-rendering, cannot tell' + - it: CronJob passes the wedge-age threshold to the upgrade script (#2877) template: templates/auto-upgrade-cronjob.yaml documentIndex: 1 diff --git a/client/tests/telemetry_collector_test.yaml b/client/tests/telemetry_collector_test.yaml index 3c2fc4cb..3e7a5d0c 100644 --- a/client/tests/telemetry_collector_test.yaml +++ b/client/tests/telemetry_collector_test.yaml @@ -119,6 +119,124 @@ tests: # --set telemetryCollector.classAContainers=null # -> at '/telemetryCollector': missing property 'classAContainers'.) + # ── the no-token skip names its REAL trigger (backend#3550) ───────────────── + # The record used to end "this resolves itself once it does". Nothing performed + # that: the `lookup` deciding the state runs only inside a `helm upgrade`, and + # the Secret appearing triggers none. Measured on a fresh edge — Secret present + # within minutes, three hourly ticks, no Collector, until a NEWER chart happened + # to ship a day later. The auto-upgrade tick now re-renders in place + # (auto-upgrade-cronjob.yaml), and the record says so, in the words of the + # values that actually schedule it — the schedule and repo below come from + # autoUpgrade.*, not from a copy in the template. + # + # `kubernetesProvider` gives `lookup` a cluster with a kube-system Namespace and + # NO token Secret, so the decider resolves skipped-no-token offline — the one + # state the plain `helm template` path (lookup empty -> "enabled") cannot reach. + - it: the no-token skip names the trigger that actually re-renders it, not a self-resolution + template: templates/telemetry-collector-status.yaml + kubernetesProvider: + scheme: + "v1/Namespace": + gvr: + version: v1 + resource: namespaces + namespaced: false + objects: + - kind: Namespace + apiVersion: v1 + metadata: + name: kube-system + asserts: + - equal: + path: data.state + value: skipped-no-token + - notMatchRegex: + path: data.reason + pattern: "resolves itself" + - matchRegex: + path: data.reason + pattern: "triggers nothing by itself" + - matchRegex: + path: data.reason + pattern: "next auto-upgrade tick" + # Derived from autoUpgrade.schedule, the CronJob's own schedule. + - matchRegex: + path: data.reason + pattern: 'CronJob schedule "23 \* \* \* \*"' + # The one state the tick cannot fix is named, not hidden. + - matchRegex: + path: data.reason + pattern: "ahead of that repo's index cannot be re-rendered" + + - it: the no-token skip follows an operator's schedule override, never a baked-in one + set: + autoUpgrade.schedule: "7 */2 * * *" + template: templates/telemetry-collector-status.yaml + kubernetesProvider: + scheme: + "v1/Namespace": + gvr: + version: v1 + resource: namespaces + namespaced: false + objects: + - kind: Namespace + apiVersion: v1 + metadata: + name: kube-system + asserts: + - matchRegex: + path: data.reason + pattern: 'CronJob schedule "7 \*/2 \* \* \*"' + - notMatchRegex: + path: data.reason + pattern: '"23 \* \* \* \*"' + + - it: with autoUpgrade off, the no-token skip says nobody will re-render it for you + set: + autoUpgrade.enabled: false + template: templates/telemetry-collector-status.yaml + kubernetesProvider: + scheme: + "v1/Namespace": + gvr: + version: v1 + resource: namespaces + namespaced: false + objects: + - kind: Namespace + apiVersion: v1 + metadata: + name: kube-system + asserts: + - equal: + path: data.state + value: skipped-no-token + - notMatchRegex: + path: data.reason + pattern: "resolves itself" + - notMatchRegex: + path: data.reason + pattern: "auto-upgrade tick" + - matchRegex: + path: data.reason + pattern: "autoUpgrade is off in this release" + + - it: the incomplete-values skip no longer promises to resolve itself either + set: + telemetryCollector: null + template: templates/telemetry-collector-status.yaml + asserts: + - equal: + path: data.state + value: skipped-incomplete-values + - notMatchRegex: + path: data.reason + pattern: "resolves itself" + - matchRegex: + path: data.reason + pattern: "nothing re-renders this release by itself" + - it: the status ConfigMap is ALWAYS rendered, so absence means only "chart too old" template: templates/telemetry-collector-status.yaml asserts: diff --git a/client/tests/telemetry_status_wiring_test.yaml b/client/tests/telemetry_status_wiring_test.yaml new file mode 100644 index 00000000..280af173 --- /dev/null +++ b/client/tests/telemetry_status_wiring_test.yaml @@ -0,0 +1,87 @@ +# The auto-upgrade tick reads the telemetry status record back out of the stored +# release to decide whether a same-version re-render is due (backend#3550). The +# reader and the writer must name the same ConfigMap, the same annotation key and +# the same DaemonSet — each through ONE helper — so this suite renders both +# templates side by side and pins that the CronJob's env is what the record and +# the Collector actually render as. A restated spelling in either would drift +# silently: the awk would match nothing and the reconcile would quietly never fire. +suite: telemetry status record <-> auto-upgrade reconcile wiring (backend#3550) +templates: + - templates/telemetry-collector-status.yaml + - templates/auto-upgrade-cronjob.yaml +set: + clientId: "test-id" + clientPassword: "test" + storageClass: + create: false +release: + name: edge + namespace: tracebloc +tests: + - it: auto-upgrade reads the status record by the name and key this template writes + templates: + - templates/telemetry-collector-status.yaml + - templates/auto-upgrade-cronjob.yaml + asserts: + - equal: + path: metadata.name + value: edge-telemetry-status + template: templates/telemetry-collector-status.yaml + - isNotNull: + path: metadata.annotations["tracebloc.io/telemetry-collector-state"] + template: templates/telemetry-collector-status.yaml + - contains: + path: spec.jobTemplate.spec.template.spec.containers[0].env + content: + name: TELEMETRY_STATUS_NAME + value: edge-telemetry-status + template: templates/auto-upgrade-cronjob.yaml + documentSelector: + path: kind + value: CronJob + - contains: + path: spec.jobTemplate.spec.template.spec.containers[0].env + content: + name: TELEMETRY_STATUS_ANNOTATION + value: tracebloc.io/telemetry-collector-state + template: templates/auto-upgrade-cronjob.yaml + documentSelector: + path: kind + value: CronJob + - contains: + path: spec.jobTemplate.spec.template.spec.containers[0].env + content: + name: TELEMETRY_COLLECTOR_NAME + value: edge-telemetry-collector + template: templates/auto-upgrade-cronjob.yaml + documentSelector: + path: kind + value: CronJob + + + - it: every name follows fullnameOverride together, reader and writers alike + set: + fullnameOverride: renamed + asserts: + - equal: + path: metadata.name + value: renamed-telemetry-status + template: templates/telemetry-collector-status.yaml + - contains: + path: spec.jobTemplate.spec.template.spec.containers[0].env + content: + name: TELEMETRY_STATUS_NAME + value: renamed-telemetry-status + template: templates/auto-upgrade-cronjob.yaml + documentSelector: + path: kind + value: CronJob + - contains: + path: spec.jobTemplate.spec.template.spec.containers[0].env + content: + name: TELEMETRY_COLLECTOR_NAME + value: renamed-telemetry-collector + template: templates/auto-upgrade-cronjob.yaml + documentSelector: + path: kind + value: CronJob diff --git a/client/values.yaml b/client/values.yaml index f52fb307..241550a8 100644 --- a/client/values.yaml +++ b/client/values.yaml @@ -1676,6 +1676,18 @@ autoUpgrade: # Hourly at :23. The off-hour minute spreads load across the # tracebloc.github.io/client GitHub Pages origin. schedule: "23 * * * *" + # A tick with NO newer chart to install is not idle (backend#3550): when the + # deployed release recorded the telemetry Collector as `skipped-no-token` + # (nobody set telemetryCollector.enabled and jobs-manager had not yet written + # the token Secret), the tick asks a server-side dry-run of the DEPLOYED chart + # version what it would decide now, and if the answer is `enabled` it re-runs + # `helm upgrade` at that same version so the Collector renders. The `lookup` + # that decides the three-state rule runs only inside an upgrade, so without + # this the Secret appearing changed nothing until the next chart release — the + # -telemetry-status ConfigMap names this tick as the trigger. It needs + # the deployed version to be one `repoUrl` serves; an edge running a version + # AHEAD of that index (a dev chart) cannot be re-rendered and waits for the next + # published chart, which the tick says every hour rather than guessing. # Helm chart repo to poll. Override only when mirroring internally. repoUrl: "https://tracebloc.github.io/client" repoName: "tracebloc" diff --git a/scripts/tests/auto-upgrade-telemetry-reconcile-mutations.sh b/scripts/tests/auto-upgrade-telemetry-reconcile-mutations.sh new file mode 100755 index 00000000..0b31a551 --- /dev/null +++ b/scripts/tests/auto-upgrade-telemetry-reconcile-mutations.sh @@ -0,0 +1,127 @@ +#!/usr/bin/env bash +# +# auto-upgrade-telemetry-reconcile-mutations.sh — prove the backend#3550 gates +# can actually FAIL on the regressions they were written for. +# +# A guard that has never been seen red is a claim, not a check (repo CLAUDE.md +# rule 5). Each case below COPIES the chart and the gate into a throwaway tree, +# breaks ONE thing in the copy, and asserts the gate reddens WITH THE SPECIFIC +# FINDING for that break (rule 10) — a gate failing for an unrelated reason and +# a gate that works produce the same exit status, so only the message can tell +# them apart. The real tree is never written to. Baseline first: a copy that +# was already red would make every mutation meaningless. +# +# Mutations, and what each must redden: +# (a) drop the DaemonSet-absent check -> the contradictory case reconciles +# (b) let the dry-run's skipped-no-token pass -> the Secret-still-absent case reconciles +# (c) unhook the reconcile from the tick -> the Secret-arrived case does not reconcile +# (d) restore "this resolves itself" wording -> the status-record unit test reddens +# +set -euo pipefail + +ROOT=$(cd "$(dirname "$0")/../.." && pwd) +GATE="$ROOT/scripts/tests/auto-upgrade-telemetry-reconcile.sh" +CRONJOB="client/templates/auto-upgrade-cronjob.yaml" +STATUS="client/templates/telemetry-collector-status.yaml" +SUITE="tests/telemetry_collector_test.yaml" +[ -r "$GATE" ] || { echo "FAIL: $GATE missing" >&2; exit 2; } +[ -r "$ROOT/$CRONJOB" ] || { echo "FAIL: $CRONJOB missing" >&2; exit 2; } +command -v helm >/dev/null 2>&1 || { echo "[SKIP] helm not installed"; exit 0; } + +pass=0; fail=0 +TMP=$(mktemp -d); trap 'rm -rf "$TMP"' EXIT + +mkfixture() { # $1 = destination root + mkdir -p "$1/scripts/tests" + cp -R "$ROOT/client" "$1/client" + cp "$GATE" "$1/scripts/tests/" +} + +# mutate FILE OLD NEW — exact, unique anchor, or the mutation is inert and the +# case would pass by accident (an inert mutation and good coverage look alike). +mutate() { + python3 - "$1" "$2" "$3" <<'PY' +import sys +p, old, new = sys.argv[1], sys.argv[2], sys.argv[3] +s = open(p).read() +n = s.count(old) +if n != 1: + sys.exit("FAIL: mutation anchor matched %d times in %s, not exactly 1 -- inert or ambiguous:\n%s" % (n, p, old)) +open(p, "w").write(s.replace(old, new)) +PY +} + +run_gate_case() { # $1 label, $2 want rc, $3 want substring, $4 fixture root + local label="$1" want_rc="$2" want="$3" d="$4" out rc + set +e + out=$(cd "$d" && bash scripts/tests/auto-upgrade-telemetry-reconcile.sh 2>&1); rc=$? + set -e + if [ "$rc" -ne "$want_rc" ]; then + printf ' [FAIL] %s -- exit %s, wanted %s\n' "$label" "$rc" "$want_rc" + printf '%s\n' "$out" | sed 's/^/ | /'; fail=$((fail+1)); return + fi + if ! grep -qF -- "$want" <<<"$out"; then + printf ' [FAIL] %s -- exit %s as expected but the finding did not name it\n' "$label" "$rc" + printf ' wanted substring: %s\n' "$want" + printf '%s\n' "$out" | sed 's/^/ | /'; fail=$((fail+1)); return + fi + printf ' [ok] %s\n' "$label"; pass=$((pass+1)) +} + +echo "== auto-upgrade-telemetry-reconcile: can it fail? ==" + +# ---- 0. baseline: the tree as shipped is green ------------------------------ +D="$TMP/base"; mkfixture "$D" +run_gate_case "the tree as shipped passes the gate" 0 "all 23 cases" "$D" + +# ---- (a) the DaemonSet-absent check is load-bearing -------------------------- +D="$TMP/no-ds-check"; mkfixture "$D" +mutate "$D/$CRONJOB" \ +' if printf '"'"'%s\n'"'"' "$_manifest" | telemetry_manifest_has_collector; then' \ +' if false; then' +run_gate_case "(a) dropping the DaemonSet-absent check reddens on the contradictory case" 1 \ + "contradictory: record says skipped but the DaemonSet is deployed -> no upgrade, said loudly: an upgrade RAN" "$D" + +# ---- (b) the dry-run verdict is load-bearing --------------------------------- +D="$TMP/no-would-check"; mkfixture "$D" +mutate "$D/$CRONJOB" \ +' enabled) echo "token Secret is now present, a re-render of $CURRENT enables the Collector"; return 0 ;;' \ +' enabled|skipped-no-token) echo "token Secret is now present, a re-render of $CURRENT enables the Collector"; return 0 ;;' +run_gate_case "(b) accepting a skipped-no-token dry-run reddens on the Secret-still-absent case" 1 \ + "Secret still absent: dry-run still skipped-no-token -> no upgrade: an upgrade RAN" "$D" + +# ---- (c) the tick actually calls the reconcile ------------------------------- +D="$TMP/unhooked"; mkfixture "$D" +mutate "$D/$CRONJOB" \ +' log "already at latest; nothing to upgrade" + telemetry_reconcile_if_needed' \ +' log "already at latest; nothing to upgrade"' +run_gate_case "(c) unhooking the reconcile from the at-latest branch reddens on the Secret-arrived case" 1 \ + "Secret arrived: stored skipped-no-token, no DaemonSet, dry-run enabled -> reconcile: expected a same-version reconcile, no upgrade ran" "$D" + +# ---- (d) the status record's honesty is pinned by the chart unit tests ------- +# The old sentence, byte for byte, back in the no-token branch. The unit suite +# for the Collector must name the record as the failing test. +D="$TMP/old-wording"; mkfixture "$D" +mutate "$D/$STATUS" \ +'The Secret appearing triggers nothing by itself — the Collector renders on the next helm upgrade of this release: %s. Set telemetryCollector.enabled: false to opt out permanently." .Values.nodeAgents.namespace.name (include "tracebloc.telemetryTokenSecretName" .) (include "tracebloc.telemetryTokenLegacyName" .) $trigger | quote }}' \ +'this resolves itself once it does. Set telemetryCollector.enabled: false to opt out permanently." .Values.nodeAgents.namespace.name (include "tracebloc.telemetryTokenSecretName" .) (include "tracebloc.telemetryTokenLegacyName" .) | quote }}' +set +e +out=$(cd "$D" && helm unittest ./client -f "$SUITE" 2>&1); rc=$? +set -e +if [ "$rc" -eq 0 ]; then + printf ' [FAIL] (d) restoring "this resolves itself" left the Collector unit suite GREEN\n'; fail=$((fail+1)) +elif ! grep -qF -- "names the trigger that actually re-renders it, not a self-resolution" <<<"$out"; then + printf ' [FAIL] (d) suite reddened, but not on the honest-record test\n' + printf '%s\n' "$out" | sed 's/^/ | /'; fail=$((fail+1)) +else + printf ' [ok] (d) restoring "this resolves itself" reddens the honest-record unit test\n'; pass=$((pass+1)) +fi + +echo +if [ "$fail" -ne 0 ]; then + echo "[ERROR] $fail mutation(s) did not redden their gate ($pass did): a guard here is decorative (backend#3550)" >&2 + exit 1 +fi +[ "$pass" -gt 0 ] || { echo "[ERROR] zero mutations ran — refusing to report green" >&2; exit 2; } +echo " [OK] all $pass cases: every backend#3550 gate has been seen red on its own regression" diff --git a/scripts/tests/auto-upgrade-telemetry-reconcile.sh b/scripts/tests/auto-upgrade-telemetry-reconcile.sh new file mode 100755 index 00000000..e61acd80 --- /dev/null +++ b/scripts/tests/auto-upgrade-telemetry-reconcile.sh @@ -0,0 +1,357 @@ +#!/usr/bin/env bash +# +# auto-upgrade-telemetry-reconcile.sh — the hourly auto-upgrade tick must +# re-render the deployed chart IN PLACE once the telemetry Collector's token +# Secret exists, and must do so under exactly one condition (backend#3550). +# +# WHY THIS EXISTS. The Collector's three-state rule (`telemetryCollector.enabled` +# unset -> collect iff the token Secret exists, else record `skipped-no-token`) +# is decided by a template `lookup`, which runs only inside a `helm upgrade`. The +# tick used to upgrade only onto a NEWER published chart, so the Secret +# appearing triggered nothing; the status record nevertheless promised the skip +# "resolves itself". Measured: Secret present within minutes, three ticks, no +# Collector, until a new chart happened to ship a day later. +# +# WHAT IT ASSERTS, by DRIVING the rendered script (the exact bytes the fleet +# runs) with a stub `helm` on PATH — never by reading it: +# +# reconcile iff stored state == skipped-no-token +# AND the stored manifest carries no Collector DaemonSet +# AND a server-side dry-run of the DEPLOYED version resolves `enabled` +# AND the repo actually serves that version +# any other combination -> the tick behaves exactly as before (no upgrade) +# unreadable state (manifest, dry-run, repo) -> NO upgrade, and the log says +# so — "cannot tell" is a finding, never a silent pass +# +# The condition lives in ONE function (`telemetry_reconcile_verdict`), so the +# table below is ALSO driven against that function directly, extracted from the +# same render — the test and its mutation (auto-upgrade-telemetry-reconcile- +# mutations.sh) both call the real code, never a copy of it. +# +# FIXTURES ARE RENDERED, NOT TYPED. Every stored manifest / dry-run body is a +# real `helm template` of this chart, so the awk that locates the status record +# and the DaemonSet is exercised against the shape helm actually emits. The +# names the script matches on come from the rendered CronJob's own env, exactly +# as in the pod. +# +# Exit 0 all cases hold, 1 a case failed, 2 could not set the scenario up. +set -euo pipefail + +cd "$(dirname "${BASH_SOURCE[0]}")/../.." + +command -v helm >/dev/null 2>&1 || { echo "[SKIP] helm not installed"; exit 0; } +command -v python3 >/dev/null 2>&1 || { echo "[ERROR] python3 required" >&2; exit 2; } + +echo "== auto-upgrade reconciles the telemetry Collector in place (backend#3550) ==" + +WORK="$(mktemp -d -t auto-upgrade-3550.XXXXXX)" +trap 'rm -rf "$WORK"' EXIT + +KUBE_VERSION="${HELM_KUBE_VERSION:-1.28.0}" +render() { # $1 = --show-only template, rest = extra --set flags + local tpl="$1"; shift + helm template t client \ + --kube-version "$KUBE_VERSION" \ + --set clientId=x --set clientPassword=y --set storageClass.create=false \ + "$@" --show-only "$tpl" +} + +# --- the script and its env, from the rendered CronJob -------------------------- +render templates/auto-upgrade-cronjob.yaml >"$WORK/cronjob.yaml" +SCRIPT="$WORK/auto-upgrade.sh" +ENVFILE="$WORK/telemetry.env" +python3 - "$WORK/cronjob.yaml" "$SCRIPT" "$ENVFILE" <<'PY' +import sys +try: + import yaml +except ImportError: + sys.exit("[ERROR] PyYAML required (pip install pyyaml)") +src, out, envout = sys.argv[1], sys.argv[2], sys.argv[3] +script = env = None +for d in yaml.safe_load_all(open(src)): + if not d: + continue + if d.get("kind") == "ConfigMap" and "auto-upgrade.sh" in (d.get("data") or {}): + script = d["data"]["auto-upgrade.sh"] + if d.get("kind") == "CronJob": + c = d["spec"]["jobTemplate"]["spec"]["template"]["spec"]["containers"][0] + env = {e["name"]: e.get("value", "") for e in c.get("env", [])} +if script is None: + sys.exit("[ERROR] no ConfigMap carrying auto-upgrade.sh was rendered") +if env is None: + sys.exit("[ERROR] no CronJob was rendered") +open(out, "w").write(script) +names = ["TELEMETRY_STATUS_NAME", "TELEMETRY_STATUS_ANNOTATION", "TELEMETRY_COLLECTOR_NAME"] +missing = [n for n in names if not env.get(n)] +if missing: + sys.exit("[ERROR] the CronJob env does not carry %s — the script would have nothing to match on" % ", ".join(missing)) +with open(envout, "w") as f: + for n in names: + f.write("%s=%s\n" % (n, env[n])) +PY +[ -s "$SCRIPT" ] || { echo "[ERROR] extracted script is empty" >&2; exit 2; } +# shellcheck disable=SC1090 +. "$ENVFILE" +export TELEMETRY_STATUS_NAME TELEMETRY_STATUS_ANNOTATION TELEMETRY_COLLECTOR_NAME + +# --- rendered fixtures ---------------------------------------------------------- +# Offline, `lookup` is empty, so fleet mode resolves `enabled`; the states a live +# cluster produces are reached by rewriting ONLY the state token of a real render. +FULL_ON="$WORK/manifest-enabled.yaml" # status enabled + Collector DaemonSet +FULL_OFF="$WORK/manifest-disabled.yaml" # status disabled-by-operator, no DaemonSet +helm template t client --kube-version "$KUBE_VERSION" \ + --set clientId=x --set clientPassword=y --set storageClass.create=false \ + --set telemetryCollector.enabled=true >"$FULL_ON" +helm template t client --kube-version "$KUBE_VERSION" \ + --set clientId=x --set clientPassword=y --set storageClass.create=false \ + --set telemetryCollector.enabled=false >"$FULL_OFF" +# The fixtures are only meaningful if the real render carries what we rewrite. +grep -q "^ name: ${TELEMETRY_STATUS_NAME}\$" "$FULL_ON" \ + || { echo "[ERROR] rendered chart carries no ConfigMap named $TELEMETRY_STATUS_NAME" >&2; exit 2; } +grep -q "^ name: ${TELEMETRY_COLLECTOR_NAME}\$" "$FULL_ON" \ + || { echo "[ERROR] enabled render carries no object named $TELEMETRY_COLLECTOR_NAME" >&2; exit 2; } +if grep -q "^ name: ${TELEMETRY_COLLECTOR_NAME}\$" "$FULL_OFF"; then + echo "[ERROR] disabled render still carries $TELEMETRY_COLLECTOR_NAME — fixture premise broken" >&2; exit 2 +fi +grep -q '"disabled-by-operator"' "$FULL_OFF" || { echo "[ERROR] disabled render lacks its state token" >&2; exit 2; } +grep -q '"enabled"' "$FULL_ON" || { echo "[ERROR] enabled render lacks its state token" >&2; exit 2; } + +NO_TOKEN="$WORK/manifest-no-token.yaml" # what a live cluster without the Secret stores +sed 's/"disabled-by-operator"/"skipped-no-token"/g' "$FULL_OFF" >"$NO_TOKEN" +INCOMPLETE="$WORK/manifest-incomplete.yaml" +sed 's/"disabled-by-operator"/"skipped-incomplete-values"/g' "$FULL_OFF" >"$INCOMPLETE" +CONTRADICTORY="$WORK/manifest-contradictory.yaml" # record says skipped, DaemonSet present +sed 's/"enabled"/"skipped-no-token"/g' "$FULL_ON" >"$CONTRADICTORY" +NO_RECORD="$WORK/manifest-no-record.yaml" # a release rendered before the record existed +python3 - "$FULL_OFF" "$NO_RECORD" "$TELEMETRY_STATUS_NAME" <<'PY' +import sys +src, out, name = sys.argv[1], sys.argv[2], sys.argv[3] +docs = open(src).read().split("\n---\n") +kept = [d for d in docs if ("\n name: %s\n" % name) not in d and not d.startswith(" name: %s\n" % name)] +if len(kept) != len(docs) - 1: + sys.exit("[ERROR] expected to drop exactly one document (the status record), dropped %d" % (len(docs) - len(kept))) +open(out, "w").write("\n---\n".join(kept)) +PY + +# --- a stub `helm` that answers from harness variables -------------------------- +BIN="$WORK/bin"; mkdir -p "$BIN" +cat >"$BIN/helm" <<'STUB' +#!/bin/sh +# Stub helm for the backend#3550 reconcile gate. Harness variables: +# STUB_LATEST newest version the repo lists (plain `helm search repo`) +# STUB_SERVED space-separated versions `helm search repo --version X` finds +# STUB_CURRENT deployed chart version (`helm list` -> chart: client-) +# STUB_MANIFEST_FILE body for `helm get manifest` (STUB_MANIFEST_FAIL=1 -> exit 1) +# STUB_DRYRUN_FILE body for `helm upgrade --dry-run=server` (STUB_DRYRUN_FAIL=1 -> exit 1) +# UPGRADE_MARKER touched by a REAL `helm upgrade`; its args land in UPGRADE_ARGS +# DRYRUN_MARKER touched by a dry-run upgrade +sub="$1"; shift 2>/dev/null || true +case "$sub" in + repo) exit 0 ;; + status) printf 'NAME: %s\nSTATUS: deployed\nREVISION: 3\n' "$1" ;; + list) echo "chart: client-${STUB_CURRENT}" ;; + search) + want="" + while [ $# -gt 0 ]; do + if [ "$1" = "--version" ]; then want="$2"; shift; fi + shift + done + # Real `helm search repo -o yaml` shape: a list item whose `version:` sits on + # its own indented line — the script's awk keys on exactly that. + if [ -z "$want" ]; then + printf -- '- name: tracebloc/client\n version: %s\n' "${STUB_LATEST}" + else + hit=no + for v in ${STUB_SERVED}; do [ "$v" = "$want" ] && hit=yes; done + if [ "$hit" = yes ]; then printf -- '- name: tracebloc/client\n version: %s\n' "${want}"; else echo "[]"; fi + fi + ;; + get) + [ "$1" = manifest ] || { echo "stub: unexpected helm get $1" >&2; exit 1; } + [ "${STUB_MANIFEST_FAIL:-0}" = 1 ] && { echo "Error: release: not found" >&2; exit 1; } + cat "$STUB_MANIFEST_FILE" + ;; + upgrade) + dry=no + for a in "$@"; do [ "$a" = "--dry-run=server" ] && dry=yes; done + if [ "$dry" = yes ]; then + : >"$DRYRUN_MARKER" + [ "${STUB_DRYRUN_FAIL:-0}" = 1 ] && { echo "Error: lookup forbidden" >&2; exit 1; } + cat "$STUB_DRYRUN_FILE" + else + : >"$UPGRADE_MARKER" + printf '%s\n' "$@" >"$UPGRADE_ARGS" + echo "upgraded" + fi + ;; + rollback) echo "stub: rollback should not be reached" >&2; exit 1 ;; + *) exit 0 ;; +esac +STUB +chmod +x "$BIN/helm" + +fails=0 +pass=0 + +# Common harness env for one run. Markers are reset per case. +setup_case() { + UPGRADE_MARKER="$WORK/upgrade.marker"; DRYRUN_MARKER="$WORK/dryrun.marker" + UPGRADE_ARGS="$WORK/upgrade.args"; OUT="$WORK/out.txt" + rm -f "$UPGRADE_MARKER" "$DRYRUN_MARKER" "$UPGRADE_ARGS" "$OUT" + export UPGRADE_MARKER DRYRUN_MARKER UPGRADE_ARGS + export RELEASE_NAME=t RELEASE_NAMESPACE=tracebloc REPO_URL=https://example.invalid \ + REPO_NAME=tracebloc CHART_NAME=client UPGRADE_TIMEOUT=10m WEDGE_MIN_AGE_SECONDS=2700 +} + +# run_tick LABEL EXPECT(reconcile|upgrade|none) EXPECT_LOG_SUBSTRING [ENV=VAL ...] +# `reconcile` = a real upgrade pinned to STUB_CURRENT; `upgrade` = a real upgrade +# to STUB_LATEST with NO dry-run consulted; `none` = no real upgrade at all. +run_tick() { + local label="$1" expect="$2" want="$3"; shift 3 + setup_case + local rc=0 + env PATH="$BIN:$PATH" "$@" sh "$SCRIPT" >"$OUT" 2>&1 || rc=$? + if [ "$rc" -ne 0 ]; then + echo " [FAIL] $label: tick exited $rc (must exit 0 — a no-op is not a failure)"; sed 's/^/ | /' "$OUT"; fails=$((fails+1)); return + fi + local upgraded=no; [ -e "$UPGRADE_MARKER" ] && upgraded=yes + local dry=no; [ -e "$DRYRUN_MARKER" ] && dry=yes + case "$expect" in + reconcile) + if [ "$upgraded" != yes ]; then + echo " [FAIL] $label: expected a same-version reconcile, no upgrade ran"; sed 's/^/ | /' "$OUT"; fails=$((fails+1)); return + fi + if ! grep -qx -- "--version" "$UPGRADE_ARGS" || ! grep -qx -- "$STUB_CURRENT_FOR_ASSERT" "$UPGRADE_ARGS"; then + echo " [FAIL] $label: reconcile did not pin --version $STUB_CURRENT_FOR_ASSERT"; sed 's/^/ | /' "$UPGRADE_ARGS"; fails=$((fails+1)); return + fi + for flag in --reset-then-reuse-values --atomic --cleanup-on-fail; do + grep -qx -- "$flag" "$UPGRADE_ARGS" || { echo " [FAIL] $label: reconcile dropped $flag"; fails=$((fails+1)); return; } + done + ;; + upgrade) + if [ "$upgraded" != yes ]; then + echo " [FAIL] $label: expected the normal upgrade, none ran"; sed 's/^/ | /' "$OUT"; fails=$((fails+1)); return + fi + if [ "$dry" = yes ]; then + echo " [FAIL] $label: a newer chart was available yet the reconcile dry-run ran — the upgrade itself re-renders"; fails=$((fails+1)); return + fi + ;; + none) + if [ "$upgraded" = yes ]; then + echo " [FAIL] $label: an upgrade RAN where the condition does not hold"; sed 's/^/ | /' "$OUT"; fails=$((fails+1)); return + fi + ;; + *) echo "[ERROR] bad expectation '$expect'" >&2; exit 2 ;; + esac + if ! grep -qF -- "$want" "$OUT"; then + echo " [FAIL] $label: log did not say '$want'"; sed 's/^/ | /' "$OUT"; fails=$((fails+1)); return + fi + echo " [OK] $label" + pass=$((pass+1)) +} + +# Every script-level case pins the deployed version it expects a reconcile to name. +STUB_CURRENT_FOR_ASSERT=9.9.9 +COMMON="STUB_LATEST=9.9.9 STUB_SERVED=9.9.9 STUB_CURRENT=9.9.9" + +echo "-- the tick, end to end --" +# shellcheck disable=SC2086 +run_tick "Secret arrived: stored skipped-no-token, no DaemonSet, dry-run enabled -> reconcile" reconcile \ + "reconcile: token Secret is now present" $COMMON STUB_MANIFEST_FILE="$NO_TOKEN" STUB_DRYRUN_FILE="$FULL_ON" +# shellcheck disable=SC2086 +run_tick "Secret still absent: dry-run still skipped-no-token -> no upgrade" none \ + "already at latest" $COMMON STUB_MANIFEST_FILE="$NO_TOKEN" STUB_DRYRUN_FILE="$NO_TOKEN" +# shellcheck disable=SC2086 +run_tick "Collector already enabled -> no upgrade" none \ + "already at latest" $COMMON STUB_MANIFEST_FILE="$FULL_ON" STUB_DRYRUN_FILE="$FULL_ON" +# shellcheck disable=SC2086 +run_tick "operator opted out (disabled-by-operator) -> no upgrade" none \ + "already at latest" $COMMON STUB_MANIFEST_FILE="$FULL_OFF" STUB_DRYRUN_FILE="$FULL_ON" +# shellcheck disable=SC2086 +run_tick "skipped-incomplete-values is out of scope -> no upgrade" none \ + "already at latest" $COMMON STUB_MANIFEST_FILE="$INCOMPLETE" STUB_DRYRUN_FILE="$FULL_ON" +# shellcheck disable=SC2086 +run_tick "release predates the status record -> no upgrade" none \ + "already at latest" $COMMON STUB_MANIFEST_FILE="$NO_RECORD" STUB_DRYRUN_FILE="$FULL_ON" +# shellcheck disable=SC2086 +run_tick "contradictory: record says skipped but the DaemonSet is deployed -> no upgrade, said loudly" none \ + "contradictory" $COMMON STUB_MANIFEST_FILE="$CONTRADICTORY" STUB_DRYRUN_FILE="$FULL_ON" +# shellcheck disable=SC2086 +run_tick "unreadable manifest -> no upgrade, cannot tell is logged" none \ + "cannot tell" $COMMON STUB_MANIFEST_FILE="$NO_TOKEN" STUB_MANIFEST_FAIL=1 STUB_DRYRUN_FILE="$FULL_ON" +# shellcheck disable=SC2086 +run_tick "dry-run fails -> no upgrade, cannot tell is logged" none \ + "cannot tell" $COMMON STUB_MANIFEST_FILE="$NO_TOKEN" STUB_DRYRUN_FILE="$FULL_ON" STUB_DRYRUN_FAIL=1 +# shellcheck disable=SC2086 +run_tick "dry-run body carries no record -> no upgrade, cannot tell is logged" none \ + "cannot tell" $COMMON STUB_MANIFEST_FILE="$NO_TOKEN" STUB_DRYRUN_FILE="$NO_RECORD" +run_tick "deployed AHEAD of the index (dev chart): repo cannot serve it -> no upgrade, named" none \ + "is not served by" STUB_LATEST=9.9.9 STUB_SERVED=9.9.9 STUB_CURRENT=9.9.10 \ + STUB_MANIFEST_FILE="$NO_TOKEN" STUB_DRYRUN_FILE="$FULL_ON" +run_tick "a newer chart exists -> the normal upgrade runs and the reconcile is not consulted" upgrade \ + "upgrading 9.9.8 -> 9.9.9" STUB_LATEST=9.9.9 STUB_SERVED="9.9.8 9.9.9" STUB_CURRENT=9.9.8 \ + STUB_MANIFEST_FILE="$NO_TOKEN" STUB_DRYRUN_FILE="$FULL_ON" +# shellcheck disable=SC2086 +run_tick "names missing from env -> no upgrade, cannot tell is logged (fails closed)" none \ + "inputs missing" $COMMON STUB_MANIFEST_FILE="$NO_TOKEN" STUB_DRYRUN_FILE="$FULL_ON" \ + TELEMETRY_STATUS_NAME= TELEMETRY_COLLECTOR_NAME= + +# --- the condition function itself, extracted from the same render -------------- +# Everything between the begin/end markers, so the helpers it calls come with it. +FN="$WORK/reconcile-block.sh" +awk '/telemetry Collector same-version reconcile: begin/{c=1} c{print} c && /telemetry Collector same-version reconcile: end/{exit}' "$SCRIPT" >"$FN" +grep -q 'telemetry_reconcile_verdict()' "$FN" \ + || { echo "[ERROR] could not extract telemetry_reconcile_verdict from the rendered script" >&2; exit 2; } + +# run_fn LABEL EXPECT_RC EXPECT_SUBSTRING [ENV=VAL ...] +run_fn() { + local label="$1" want_rc="$2" want="$3"; shift 3 + setup_case + local rc=0 out + out="$(env PATH="$BIN:$PATH" CURRENT=9.9.9 LATEST=9.9.9 "$@" \ + sh -c '. "$0"; telemetry_reconcile_verdict' "$FN" 2>&1)" || rc=$? + if [ "$rc" -ne "$want_rc" ]; then + echo " [FAIL] $label: verdict rc $rc, wanted $want_rc ($out)"; fails=$((fails+1)); return + fi + if ! grep -qF -- "$want" <<<"$out"; then + echo " [FAIL] $label: rc $rc as wanted but the reason did not say '$want': $out"; fails=$((fails+1)); return + fi + echo " [OK] $label (rc $rc)" + pass=$((pass+1)) +} + +echo "-- telemetry_reconcile_verdict, the one condition --" +# shellcheck disable=SC2086 +run_fn "0: skipped-no-token + no DaemonSet + dry-run enabled" 0 "token Secret is now present" \ + $COMMON STUB_MANIFEST_FILE="$NO_TOKEN" STUB_DRYRUN_FILE="$FULL_ON" +# shellcheck disable=SC2086 +run_fn "1: dry-run still skipped-no-token" 1 "would still decide 'skipped-no-token'" \ + $COMMON STUB_MANIFEST_FILE="$NO_TOKEN" STUB_DRYRUN_FILE="$NO_TOKEN" +# shellcheck disable=SC2086 +run_fn "1: stored enabled" 1 "state is 'enabled'" $COMMON STUB_MANIFEST_FILE="$FULL_ON" STUB_DRYRUN_FILE="$FULL_ON" +# shellcheck disable=SC2086 +run_fn "1: stored disabled-by-operator" 1 "state is 'disabled-by-operator'" \ + $COMMON STUB_MANIFEST_FILE="$FULL_OFF" STUB_DRYRUN_FILE="$FULL_ON" +# shellcheck disable=SC2086 +run_fn "1: no record" 1 "no telemetry status record" $COMMON STUB_MANIFEST_FILE="$NO_RECORD" STUB_DRYRUN_FILE="$FULL_ON" +# shellcheck disable=SC2086 +run_fn "2: contradictory record vs DaemonSet" 2 "contradictory" \ + $COMMON STUB_MANIFEST_FILE="$CONTRADICTORY" STUB_DRYRUN_FILE="$FULL_ON" +# shellcheck disable=SC2086 +run_fn "2: manifest unreadable" 2 "cannot read the deployed manifest" \ + $COMMON STUB_MANIFEST_FILE="$NO_TOKEN" STUB_MANIFEST_FAIL=1 STUB_DRYRUN_FILE="$FULL_ON" +# shellcheck disable=SC2086 +run_fn "2: dry-run failed" 2 "dry-run of 9.9.9 failed" \ + $COMMON STUB_MANIFEST_FILE="$NO_TOKEN" STUB_DRYRUN_FILE="$FULL_ON" STUB_DRYRUN_FAIL=1 +run_fn "2: repo does not serve the deployed version" 2 "is not served by" \ + STUB_LATEST=9.9.8 STUB_SERVED=9.9.8 STUB_CURRENT=9.9.9 STUB_MANIFEST_FILE="$NO_TOKEN" STUB_DRYRUN_FILE="$FULL_ON" +# shellcheck disable=SC2086 +run_fn "2: env names missing" 2 "inputs missing" \ + $COMMON STUB_MANIFEST_FILE="$NO_TOKEN" STUB_DRYRUN_FILE="$FULL_ON" TELEMETRY_STATUS_ANNOTATION= + +if [ "$fails" -ne 0 ]; then + echo "[ERROR] $fails case(s) failed, $pass passed: the same-version reconcile condition is broken (backend#3550)" >&2 + exit 1 +fi +[ "$pass" -gt 0 ] || { echo "[ERROR] zero cases ran — refusing to report green" >&2; exit 2; } +echo " [OK] all $pass cases: the tick re-renders exactly when the Secret has arrived, and never on 'cannot tell'" From ea1f4724008c65fd4447ba13134619cfcb742ead Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Fri, 11 Sep 2026 10:59:27 +0200 Subject: [PATCH 2/5] =?UTF-8?q?chore(chart):=20bump=20to=201.9.118=20?= =?UTF-8?q?=E2=80=94=20develop=20took=201.9.117=20under=20#1039=20while=20?= =?UTF-8?q?this=20PR=20was=20open?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5.1 --- client/Chart.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/Chart.yaml b/client/Chart.yaml index ecd2e38c..a5982420 100644 --- a/client/Chart.yaml +++ b/client/Chart.yaml @@ -2,8 +2,8 @@ apiVersion: v2 name: client description: A unified Helm chart for tracebloc on AKS, EKS, bare-metal, and OpenShift type: application -version: 1.9.117 -appVersion: "1.9.117" +version: 1.9.118 +appVersion: "1.9.118" keywords: - tracebloc - kubernetes From 13b7fb08b0cf56ded9717d16dc133ba8a072eadd Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Fri, 11 Sep 2026 11:16:27 +0200 Subject: [PATCH 3/5] =?UTF-8?q?fix(tests):=20run=20mutation=20(d)=20where?= =?UTF-8?q?=20helm-unittest=20exists=20=E2=80=94=20MODE=3Drecord=20in=20th?= =?UTF-8?q?e=20Helm=20unit=20tests=20job,=20refuse=20without=20the=20plugi?= =?UTF-8?q?n?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The drift job has no helm-unittest plugin, so (d) read `unknown command "unittest"` as "suite reddened, but not on the honest-record test": a gate red for the wrong reason (measured 2026-09-11, run 34581907348). The script now has two modes: reconcile (default, `make drift`: baseline + a-c) and record (`make helm-unittest` + helm-unit.yaml: baseline + d), and record mode refuses rather than skips when the plugin is missing, so it can never go decorative. Co-Authored-By: Claude Fable 5.1 --- .github/workflows/helm-unit.yaml | 8 +++++ Makefile | 1 + ...o-upgrade-telemetry-reconcile-mutations.sh | 31 ++++++++++++++++++- 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/.github/workflows/helm-unit.yaml b/.github/workflows/helm-unit.yaml index 16728cb3..ba18604e 100644 --- a/.github/workflows/helm-unit.yaml +++ b/.github/workflows/helm-unit.yaml @@ -89,3 +89,11 @@ jobs: - name: Run unit tests run: helm unittest ./client + + # Mutation (d) of the telemetry-reconcile gate: restoring the old + # "resolves itself" wording must redden the status-record unit test. It + # needs the plugin installed above, which is why it runs here and not in + # `make drift` (that job has no helm-unittest and read the missing plugin + # as a wrong-reason red on 2026-09-11). + - name: Prove the status-record honesty test can fail + run: MODE=record bash scripts/tests/auto-upgrade-telemetry-reconcile-mutations.sh diff --git a/Makefile b/Makefile index 639c3958..b569b0c8 100644 --- a/Makefile +++ b/Makefile @@ -587,3 +587,4 @@ helm-unittest: echo " helm plugin install https://github.com/helm-unittest/helm-unittest --version 0.5.2"; \ exit 1; } helm unittest ./client + MODE=record bash scripts/tests/auto-upgrade-telemetry-reconcile-mutations.sh diff --git a/scripts/tests/auto-upgrade-telemetry-reconcile-mutations.sh b/scripts/tests/auto-upgrade-telemetry-reconcile-mutations.sh index 0b31a551..fde84ec5 100755 --- a/scripts/tests/auto-upgrade-telemetry-reconcile-mutations.sh +++ b/scripts/tests/auto-upgrade-telemetry-reconcile-mutations.sh @@ -17,7 +17,17 @@ # (c) unhook the reconcile from the tick -> the Secret-arrived case does not reconcile # (d) restore "this resolves itself" wording -> the status-record unit test reddens # +# Two modes, because (d) needs the helm-unittest plugin and the drift job has +# none (measured 2026-09-11: `Error: unknown command "unittest" for "helm"` +# read as "suite reddened, but not on the honest-record test" -- a gate red for +# the wrong reason). MODE=reconcile (default; `make drift`) runs the baseline +# and (a)-(c). MODE=record (`make helm-unittest`, the Helm unit tests job) runs +# (d) and REFUSES without the plugin -- it never skips, so a runner that lost +# the plugin turns the job red instead of leaving (d) decorative. +# set -euo pipefail +MODE="${MODE:-reconcile}" +case "$MODE" in reconcile|record) ;; *) echo "FAIL: MODE must be reconcile or record, got '$MODE'" >&2; exit 2 ;; esac ROOT=$(cd "$(dirname "$0")/../.." && pwd) GATE="$ROOT/scripts/tests/auto-upgrade-telemetry-reconcile.sh" @@ -68,8 +78,9 @@ run_gate_case() { # $1 label, $2 want rc, $3 want substring, $ printf ' [ok] %s\n' "$label"; pass=$((pass+1)) } -echo "== auto-upgrade-telemetry-reconcile: can it fail? ==" +echo "== auto-upgrade-telemetry-reconcile: can it fail? (MODE=$MODE) ==" +if [ "$MODE" = reconcile ]; then # ---- 0. baseline: the tree as shipped is green ------------------------------ D="$TMP/base"; mkfixture "$D" run_gate_case "the tree as shipped passes the gate" 0 "all 23 cases" "$D" @@ -99,7 +110,24 @@ mutate "$D/$CRONJOB" \ run_gate_case "(c) unhooking the reconcile from the at-latest branch reddens on the Secret-arrived case" 1 \ "Secret arrived: stored skipped-no-token, no DaemonSet, dry-run enabled -> reconcile: expected a same-version reconcile, no upgrade ran" "$D" +fi # MODE=reconcile + +if [ "$MODE" = record ]; then # ---- (d) the status record's honesty is pinned by the chart unit tests ------- +# Needs the helm-unittest plugin; refuse rather than skip (rule 3: "cannot tell" +# is a finding, and a skip here would leave (d) decorative on the job that owns it). +helm plugin list 2>/dev/null | grep -q unittest \ + || { echo "FAIL: helm-unittest plugin missing -- MODE=record cannot prove (d); install it or run this in the Helm unit tests job" >&2; exit 2; } +# Baseline: the unmutated Collector suite is green, so (d)'s red below is the mutation's. +set +e +out=$(cd "$ROOT" && helm unittest ./client -f "$SUITE" 2>&1); rc=$? +set -e +if [ "$rc" -ne 0 ]; then + printf ' [FAIL] baseline: the Collector unit suite is already red on the tree as shipped\n' + printf '%s\n' "$out" | sed 's/^/ | /'; fail=$((fail+1)) +else + printf ' [ok] baseline: the Collector unit suite is green on the tree as shipped\n'; pass=$((pass+1)) +fi # The old sentence, byte for byte, back in the no-token branch. The unit suite # for the Collector must name the record as the failing test. D="$TMP/old-wording"; mkfixture "$D" @@ -117,6 +145,7 @@ elif ! grep -qF -- "names the trigger that actually re-renders it, not a self-re else printf ' [ok] (d) restoring "this resolves itself" reddens the honest-record unit test\n'; pass=$((pass+1)) fi +fi # MODE=record echo if [ "$fail" -ne 0 ]; then From 47b68aceb86071484a23ff1fdde860a1b3ce7821 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Fri, 11 Sep 2026 11:24:55 +0200 Subject: [PATCH 4/5] =?UTF-8?q?fix(tests):=20plugin=20check=20captures=20t?= =?UTF-8?q?hen=20matches=20=E2=80=94=20no=20early-closing=20pipe=20under?= =?UTF-8?q?=20errexit+pipefail?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pipefail early-close gate flagged `helm plugin list | grep -q unittest`. Co-Authored-By: Claude Fable 5.1 --- .../tests/auto-upgrade-telemetry-reconcile-mutations.sh | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/scripts/tests/auto-upgrade-telemetry-reconcile-mutations.sh b/scripts/tests/auto-upgrade-telemetry-reconcile-mutations.sh index fde84ec5..1de76c71 100755 --- a/scripts/tests/auto-upgrade-telemetry-reconcile-mutations.sh +++ b/scripts/tests/auto-upgrade-telemetry-reconcile-mutations.sh @@ -116,8 +116,13 @@ if [ "$MODE" = record ]; then # ---- (d) the status record's honesty is pinned by the chart unit tests ------- # Needs the helm-unittest plugin; refuse rather than skip (rule 3: "cannot tell" # is a finding, and a skip here would leave (d) decorative on the job that owns it). -helm plugin list 2>/dev/null | grep -q unittest \ - || { echo "FAIL: helm-unittest plugin missing -- MODE=record cannot prove (d); install it or run this in the Helm unit tests job" >&2; exit 2; } +# capture-then-match, not `| grep -q`: under errexit+pipefail a reader that +# closes early can SIGPIPE the writer and turn a present plugin into a refusal. +plugins=$(helm plugin list 2>/dev/null || true) +case "$plugins" in + *unittest*) ;; + *) echo "FAIL: helm-unittest plugin missing -- MODE=record cannot prove (d); install it or run this in the Helm unit tests job" >&2; exit 2 ;; +esac # Baseline: the unmutated Collector suite is green, so (d)'s red below is the mutation's. set +e out=$(cd "$ROOT" && helm unittest ./client -f "$SUITE" 2>&1); rc=$? From 53ac19e8b4cf66409000b4d3a5a6a970df5de39e Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Sat, 12 Sep 2026 09:31:31 +0200 Subject: [PATCH 5/5] fix(chart): close three review threads on the telemetry reconcile PR - tracebloc.telemetryStatusName now truncates/trims like its sibling tracebloc.telemetryCollectorName, so a long fullnameOverride/release name cannot leave the status ConfigMap name unbounded while the Collector's own name is capped at 63 chars. - Bound the same-version reconcile's server-side dry-run at --timeout 60s. It only templates/diffs against the API server and never waits on a rollout, so 60s is generous; left unbounded it shared helm's 5m default and could eat into the 300s repo-fetch buffer activeDeadlineSeconds reserves for the real --atomic upgrade that follows it on the same tick. - Extracted parse_repo_version() so the LATEST computation and the reconcile verdict's --version-pinned lookup share one awk pattern for helm search repo -o yaml output instead of two copies that could drift apart. Verified: scripts/tests/auto-upgrade-telemetry-reconcile.sh (23/23), scripts/tests/auto-upgrade-telemetry-reconcile-mutations.sh (4/4, including the pre-existing case (c) which now also covers the new dry-run timeout since it drives the real rendered bytes), helm unittest ./client (751/751, including telemetry_status_wiring_test.yaml), helm lint (0 failed), shellcheck -S warning -x on the extracted rendered script (clean). Co-Authored-By: Claude Fable 5.1 --- client/templates/_helpers.tpl | 2 +- client/templates/auto-upgrade-cronjob.yaml | 25 ++++++++++++++++------ 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/client/templates/_helpers.tpl b/client/templates/_helpers.tpl index c564f4af..ca2a74e3 100644 --- a/client/templates/_helpers.tpl +++ b/client/templates/_helpers.tpl @@ -1341,7 +1341,7 @@ can be kept above the configured helm timeout. depend on that. */}} {{- define "tracebloc.telemetryStatusName" -}} -{{- printf "%s-telemetry-status" (include "tracebloc.fullname" .) -}} +{{- printf "%s-telemetry-status" (include "tracebloc.fullname" .) | trunc 63 | trimSuffix "-" -}} {{- end -}} {{/* diff --git a/client/templates/auto-upgrade-cronjob.yaml b/client/templates/auto-upgrade-cronjob.yaml index 2ef460e0..6f429198 100644 --- a/client/templates/auto-upgrade-cronjob.yaml +++ b/client/templates/auto-upgrade-cronjob.yaml @@ -147,6 +147,16 @@ data: # not offer, and nothing here can re-render a chart it cannot fetch. That edge # converges on the next published chart, and the verdict names it every tick. + # Parses the `version:` field out of `helm search repo ... -o yaml` output on + # stdin. Search results are newest-first, so with no --version filter on the + # search itself the first hit is the latest; with one, it's the served + # version for that pin (or nothing, if the repo does not carry it). One + # parser for both call sites below so they cannot drift in how they read the + # same `helm search repo` output shape. + parse_repo_version() { + awk '/^[[:space:]]*version:/ {print $2; exit}' + } + # Resolved Collector state carried by the manifest on stdin: the state # annotation of the ConfigMap named TELEMETRY_STATUS_NAME. Prints nothing when # the record is absent (a release rendered by a chart that predates it). @@ -194,12 +204,17 @@ data: fi _search="$(helm search repo "${REPO_NAME}/${CHART_NAME}" --version "$CURRENT" -o yaml 2>&1)" \ || { echo "cannot ask $REPO_URL whether it serves $CURRENT: $(printf '%s\n' "$_search" | tail -n 1)"; return 2; } - _served="$(printf '%s\n' "$_search" | awk '/^[[:space:]]*version:/ {print $2; exit}')" + _served="$(printf '%s\n' "$_search" | parse_repo_version)" [ "$_served" = "$CURRENT" ] \ || { echo "deployed chart $CURRENT is not served by $REPO_URL (newest there: $LATEST): a same-version re-render is impossible, the Collector renders on the next published chart"; return 2; } + # Bounded at 60s: a server-side dry-run only templates and diffs against + # the API server, it never waits on a rollout, so it should complete in + # seconds. Left unbounded it would share helm's 5m default, which could eat + # deep into the 300s repo-fetch buffer activeDeadlineSeconds reserves below + # for the REAL --atomic upgrade that follows it on this same tick. _render="$(helm upgrade "$RELEASE_NAME" "${REPO_NAME}/${CHART_NAME}" \ --namespace "$RELEASE_NAMESPACE" --version "$CURRENT" \ - --reset-then-reuse-values --dry-run=server --hide-secret 2>&1)" \ + --reset-then-reuse-values --dry-run=server --hide-secret --timeout 60s 2>&1)" \ || { echo "server-side dry-run of $CURRENT failed: $(printf '%s\n' "$_render" | tail -n 1)"; return 2; } _would="$(printf '%s\n' "$_render" | telemetry_state_of)" case "$_would" in @@ -322,10 +337,8 @@ data: ;; esac - # Latest published version. Helm orders search results newest-first, - # so the first `version:` line in the YAML output is the latest. - LATEST="$(helm search repo "${REPO_NAME}/${CHART_NAME}" -o yaml \ - | awk '/^[[:space:]]*version:/ {print $2; exit}')" + # Latest published version. + LATEST="$(helm search repo "${REPO_NAME}/${CHART_NAME}" -o yaml | parse_repo_version)" # Currently deployed chart version. `helm list -o yaml` emits # `chart: client-` for the matching release.