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' diff --git a/client/values.yaml b/client/values.yaml index 4b10c33d..9498f355 100644 --- a/client/values.yaml +++ b/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 diff --git a/scripts/tests/collector-class-a-agreement.sh b/scripts/tests/collector-class-a-agreement.sh index 8de9ab4d..72ada985 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,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