fix(chart): collect the upgrade path's own logs (backend#2935)

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

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'

client/values.yaml

@@ -689,6 +689,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

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,28 @@ 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 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
+)"
+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