From 51e41bc8f0519d23d91bbde9ab335997cf758f2d Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Tue, 1 Sep 2026 11:41:13 +0200 Subject: [PATCH 1/3] fix(chart): collect the upgrade path own logs (backend#2935) When a customer edge stops upgrading we could see THAT it failed and never WHY: helm and refresh were not in classAContainers. The agreement guard could not see them either -- it enumerated three kinds and missed CronJob. Co-Authored-By: Claude Opus 4.8 --- client/Chart.yaml | 4 +- client/values.yaml | 15 +++++++ scripts/tests/collector-class-a-agreement.sh | 41 +++++++++++++++++--- 3 files changed, 52 insertions(+), 8 deletions(-) diff --git a/client/Chart.yaml b/client/Chart.yaml index ce7e8ef4..f1ea1b7a 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.93 -appVersion: "1.9.93" +version: 1.9.94 +appVersion: "1.9.94" keywords: - tracebloc - kubernetes diff --git a/client/values.yaml b/client/values.yaml index 245ffe01..888a8fd7 100644 --- a/client/values.yaml +++ b/client/values.yaml @@ -670,6 +670,21 @@ telemetryCollector: - api - pods-monitor-container - squid + # THE UPGRADE PATH'S OWN OUTPUT (backend#2935). `helm` is the auto-upgrade + # CronJob's container and `refresh` is image-refresh's. Without them the + # collector tails neither, so when a customer's edge stops upgrading we can + # see THAT it failed -- the controller reports the pod's exit status -- and + # never WHY. + # + # Measured on a live workspace: 477 failures in 24 hours, 431 of them a bare + # `exit code 1` whose reason exists only in these containers' stdout and + # reached nothing. An edge that cannot upgrade stops receiving security + # fixes, which makes this the least diagnosable failure with the highest + # cost -- the worst possible pairing. + # + # Cheap: both are hourly CronJobs that log a few lines and exit. + - helm + - refresh # Class A containers that run in the node-agents namespace rather than the # release namespace. Separate list rather than a magic value inside the one diff --git a/scripts/tests/collector-class-a-agreement.sh b/scripts/tests/collector-class-a-agreement.sh index 8de9ab4d..306ca901 100755 --- a/scripts/tests/collector-class-a-agreement.sh +++ b/scripts/tests/collector-class-a-agreement.sh @@ -62,11 +62,29 @@ except ImportError: docs = [d for d in yaml.safe_load_all(sys.stdin) if d] # Side 1: the containers the chart really deploys, as (namespace, container). +# CRONJOBS AND JOBS COUNT (backend#2935). This enumerated three kinds and +# looked total; it was not. A CronJob's pod spec is nested one level deeper +# (`spec.jobTemplate.spec.template.spec`), so `helm` (auto-upgrade) and +# `refresh` (image-refresh) were invisible here -- and adding either to +# `classAContainers` was reported as a glob matching nothing, which is the +# opposite of the truth. The kinds a chart can deploy a container in is the +# input domain; enumerating a subset of it is the same defect this file +# exists to catch, one level up. +def _pod_specs(d): + kind = d.get("kind") + if kind in ("Deployment", "DaemonSet", "StatefulSet"): + return [d["spec"]["template"]["spec"]] + if kind == "Job": + return [d["spec"]["template"]["spec"]] + if kind == "CronJob": + return [d["spec"]["jobTemplate"]["spec"]["template"]["spec"]] + return [] + real = set() for d in docs: - if d.get("kind") in ("Deployment", "DaemonSet", "StatefulSet"): - ns = d["metadata"].get("namespace", "") - for c in d["spec"]["template"]["spec"].get("containers", []): + ns = d.get("metadata", {}).get("namespace", "") + for spec in _pod_specs(d): + for c in spec.get("containers", []): real.add((ns, c["name"])) # Side 2: the filelog include globs out of the Collector's own config. @@ -220,13 +238,24 @@ esac partial="$(helm template t "$CHART" \ --set clientId=x --set clientPassword=y --set storageClass.create=false \ --set telemetryCollector.enabled=true 2>/dev/null | grep -c ' - "/var/log/pods/')" -if [ "$partial" -ne 4 ]; then - echo "[ERROR] a partial telemetryCollector map rendered $partial include globs, want 4 —" >&2 +# DERIVED, NOT RESTATED (backend#2935). This was hardcoded `4`, so adding a +# Class A container made the coalescing control fail for a reason that had +# nothing to do with coalescing. The number of globs IS the number of declared +# containers; reading it from the declaration is what keeps this control about +# the mechanism it names. +want="$(python3 - <<'WANT' +import yaml +v = yaml.safe_load(open("client/values.yaml"))["telemetryCollector"] +print(len(v["classAContainers"]) + len(v["classANodeAgentContainers"])) +WANT +)" +if [ "$partial" -ne "$want" ]; then + echo "[ERROR] a partial telemetryCollector map rendered $partial include globs, want $want —" >&2 echo " chart defaults are no longer coalescing, which is the failure the" >&2 echo " original finding described." >&2 na_status=1 else - printf ' ok a partial map still coalesces all 4 Class A globs\n' + printf ' ok a partial map still coalesces all %s Class A globs\n' "$want" fi [ "$na_status" -eq 0 ] || exit 1 From 32de591c0510f8c00886b0554c095779b394e069 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Tue, 1 Sep 2026 12:36:30 +0200 Subject: [PATCH 2/3] fix(tests): preflight PyYAML in the class-A count heredoc (backend#2935) Co-Authored-By: Claude Opus 4.8 --- scripts/tests/collector-class-a-agreement.sh | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/tests/collector-class-a-agreement.sh b/scripts/tests/collector-class-a-agreement.sh index 306ca901..72ada985 100755 --- a/scripts/tests/collector-class-a-agreement.sh +++ b/scripts/tests/collector-class-a-agreement.sh @@ -244,7 +244,11 @@ partial="$(helm template t "$CHART" \ # containers; reading it from the declaration is what keeps this control about # the mechanism it names. want="$(python3 - <<'WANT' -import yaml +import sys +try: + import yaml +except ImportError: + sys.exit("[ERROR] PyYAML required (pip install pyyaml)") v = yaml.safe_load(open("client/values.yaml"))["telemetryCollector"] print(len(v["classAContainers"]) + len(v["classANodeAgentContainers"])) WANT From 07d4eaac3345eb0f63802807eca474f153f27380 Mon Sep 17 00:00:00 2001 From: Lukas Wuttke Date: Tue, 1 Sep 2026 13:09:01 +0200 Subject: [PATCH 3/3] test(telemetry): pin that the upgrade containers are collected, not just agreed with (backend#2961) @saadqbal removed `- helm` and `- refresh` from `classAContainers` and nothing went red: the agreement guard passed at 4 globs and all 61 telemetry unittests still passed. The fix was removable without one leg noticing. The cause is that the guard checks AGREEMENT -- every glob targets a container the chart really deploys -- which is a different property from COVERAGE. Deriving the expected count from the declaration was right in itself, and it also means the count moves WITH the list, so a shrink is invisible to it by construction. Coverage has to be pinned by name. Two matchRegex assertions in the existing Class A path test, and two more in the partial-map test so a fleet install that sets some Collector keys but not `classAContainers` is covered too. Mutation-proved against the exact removal that motivated this: dropping either name, or both, now fails 2 tests; restored, 657 pass. Chart 1.9.94 -> 1.9.95: #922 also wrote 1.9.94 against a 1.9.93 base, and because both are literally equal git will not conflict and the version guard -- which only asserts `version:` CHANGED -- passes on each. With `strict: false` fleet-wide the second merge never re-evaluates, so both would ship different chart content under one published 1.9.94. Co-Authored-By: Claude Opus 4.8 --- client/Chart.yaml | 4 ++-- client/tests/telemetry_collector_test.yaml | 28 ++++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/client/Chart.yaml b/client/Chart.yaml index f1ea1b7a..a0485a84 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.94 -appVersion: "1.9.94" +version: 1.9.95 +appVersion: "1.9.95" keywords: - tracebloc - kubernetes diff --git a/client/tests/telemetry_collector_test.yaml b/client/tests/telemetry_collector_test.yaml index b94bb601..3c2fc4cb 100644 --- a/client/tests/telemetry_collector_test.yaml +++ b/client/tests/telemetry_collector_test.yaml @@ -182,6 +182,25 @@ tests: - matchRegex: path: data["config.yaml"] pattern: '/var/log/pods/tracebloc_[*]/squid/[*][.]log' + # THE UPGRADE PATH'S OWN CONTAINERS (backend#2961). `helm` is the + # auto-upgrade CronJob and `refresh` the image-refresh Job -- the two + # things failing 431 times a day with a bare `exit 1` while being the only + # Class A workloads nobody collected. + # + # THESE ASSERTIONS ARE THE FIX, not the values list. Adding the two names + # to `classAContainers` and deriving the guard's expected count from that + # same list makes the count move WITH the list, so deleting both entries + # left `collector-class-a-agreement.sh` green at 4 globs and all 61 + # unittests passing -- the change was removable without one leg going red + # (@saadqbal on #951). That guard checks AGREEMENT (globs are a subset of + # the containers the chart deploys); this checks COVERAGE (these two are + # actually collected). Only the second can notice a shrink. + - matchRegex: + path: data["config.yaml"] + pattern: '/var/log/pods/tracebloc_[*]/helm/[*][.]log' + - matchRegex: + path: data["config.yaml"] + pattern: '/var/log/pods/tracebloc_[*]/refresh/[*][.]log' # A DIFFERENT NAMESPACE. resource-monitor is a DaemonSet in # `nodeAgents.namespace`, so scoping every path to the release namespace # missed a whole Class A component while the Collector reported healthy. @@ -1198,6 +1217,15 @@ tests: - matchRegex: path: data["config.yaml"] pattern: '/var/log/pods/tracebloc_[*]/squid/[*][.]log' + # The upgrade path's containers must survive a PARTIAL map too — this is + # what a fleet install gets when it sets some Collector keys and not + # `classAContainers` (backend#2961). + - matchRegex: + path: data["config.yaml"] + pattern: '/var/log/pods/tracebloc_[*]/helm/[*][.]log' + - matchRegex: + path: data["config.yaml"] + pattern: '/var/log/pods/tracebloc_[*]/refresh/[*][.]log' - matchRegex: path: data["config.yaml"] pattern: '/var/log/pods/tracebloc-node-agents_edge-[*]/tracebloc-resource-monitor/[*][.]log'