From fbdb7ec444e5f4b33185fec7df7a0692df2aeb86 Mon Sep 17 00:00:00 2001 From: Arturo Peroni Date: Wed, 9 Sep 2026 13:02:13 +0200 Subject: [PATCH 1/2] [settings-naming] rename resourceMonitor -> resourceMonitor.enabled (D2) RFC-0076 S14 (client#1009). `resourceMonitor: ` becomes `resourceMonitor.enabled: ` (D2: component booleans are `.enabled`). This is a bool->object change, so every stored values.yaml and every `--set resourceMonitor=true` still arrives as a SCALAR. A template that read `.Values.resourceMonitor.enabled` blindly would `fail` ("can't evaluate field enabled in interface {}") on the scalar, and the old `ne .Values.resourceMonitor false` gate crashes ("incompatible types for comparison: map and bool") on the new object. So all reads now route through a single new helper, tracebloc.resourceMonitorEnabled, which resolves the effective flag from whichever shape is present (kindIs), preferring the new `.enabled` form and defaulting absent/`{}` to enabled to match the historical default. Effective behaviour is unchanged: resourceMonitor.enabled=true does exactly what resourceMonitor=true did. - values.yaml default is now the object form; legacy scalar still honoured through the alias window (remove_by: 2026-12-31). - values.schema.json accepts both a boolean and an object; the object CLOSES its keys (additionalProperties: false) so a mistyped `enabled` is refused at chart load instead of silently staying enabled. - migrated the eight readers (daemonset, rbac, scc, secrets, rbac.yaml, jobs-manager NODE_AGENTS_NAMESPACE, NOTES.txt, and the two _helpers predicates resourceMonitorRefreshPinned / nodeAgentsInUse). - Chart.yaml version+appVersion bumped 1.9.107 -> 1.9.108 (chart-version-guard). - helm-unittest: added legacy-scalar, new-object, unset, null, `{}`, both-disabled, and mistyped-key cases. Closes tracebloc/client#1009 Part of tracebloc/backend#3391 Co-Authored-By: Claude Opus 4.8 --- client/Chart.yaml | 4 +- client/templates/NOTES.txt | 2 +- client/templates/_helpers.tpl | 45 ++++++++- client/templates/jobs-manager-deployment.yaml | 2 +- client/templates/rbac.yaml | 2 +- .../templates/resource-monitor-daemonset.yaml | 4 +- client/templates/resource-monitor-rbac.yaml | 4 +- client/templates/resource-monitor-scc.yaml | 2 +- client/templates/secrets.yaml | 2 +- client/tests/resource_monitor_test.yaml | 96 +++++++++++++++++++ client/values.schema.json | 16 +++- client/values.yaml | 10 +- 12 files changed, 169 insertions(+), 20 deletions(-) diff --git a/client/Chart.yaml b/client/Chart.yaml index 36f9beaf..99f71ce9 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.107 -appVersion: "1.9.107" +version: 1.9.108 +appVersion: "1.9.108" keywords: - tracebloc - kubernetes diff --git a/client/templates/NOTES.txt b/client/templates/NOTES.txt index 4b2472f8..63ffa78a 100644 --- a/client/templates/NOTES.txt +++ b/client/templates/NOTES.txt @@ -5,7 +5,7 @@ {{ "\033[1;35m" }}Components:{{ "\033[0m" }} - {{ "\033[1;34m" }}Jobs Manager:{{ "\033[0m" }} {{ "\033[0;33m" }}{{ include "tracebloc.fullname" . }}-jobs-manager{{ "\033[0m" }} - {{ "\033[1;34m" }}MySQL Host:{{ "\033[0m" }} {{ "\033[0;33m" }}mysql-client{{ "\033[0m" }} -{{- if ne .Values.resourceMonitor false }} +{{- if (include "tracebloc.resourceMonitorEnabled" .) }} - {{ "\033[1;34m" }}Resource Monitor (DS):{{ "\033[0m" }} {{ "\033[0;33m" }}{{ include "tracebloc.resourceMonitorName" . }}{{ "\033[0m" }} {{- end }} diff --git a/client/templates/_helpers.tpl b/client/templates/_helpers.tpl index 46a36ba1..f9541d2e 100644 --- a/client/templates/_helpers.tpl +++ b/client/templates/_helpers.tpl @@ -126,6 +126,42 @@ tracebloc.io/seal-check-name: {{ .name | quote }} {{ include "tracebloc.fullname" . }}-resource-monitor {{- end }} +{{/* + tracebloc.resourceMonitorEnabled — the SINGLE reader of "is the resource-monitor + on", coalescing the two value shapes during the RFC-0076 alias window + (remove_by: 2026-12-31, client#1009): + + legacy scalar resourceMonitor: + new object resourceMonitor.enabled: (D2: .enabled) + + This is a bool→object rename, so a stored values.yaml or a bare + `--set resourceMonitor=true` still arrives as a SCALAR. Reading + `.Values.resourceMonitor.enabled` blindly would `fail` with "can't evaluate + field enabled in interface {}" on the scalar and, on a `--reuse-values` + upgrade that carries the scalar forward, silently drop the setting. So decide + the shape with kindIs and prefer the new `.enabled` form: + + map -> .enabled, defaulting to true when the key is absent + bool -> the scalar itself + absent -> enabled (the historical default: `ne false` was true) + + Effective behaviour is unchanged: resourceMonitor.enabled=true does exactly + what resourceMonitor=true did. Emits "true" or nothing, so callers use + `(include "tracebloc.resourceMonitorEnabled" .)` in an `and`/`or` and + `not (include ...)` for the disabled case — the same idiom as + tracebloc.nodeAgentsInUse. +*/}} +{{- define "tracebloc.resourceMonitorEnabled" -}} +{{- $rm := .Values.resourceMonitor -}} +{{- if kindIs "map" $rm -}} +{{- if ne (dig "enabled" true $rm) false -}}true{{- end -}} +{{- else if kindIs "invalid" $rm -}} +{{- "true" -}} +{{- else -}} +{{- if ne $rm false -}}true{{- end -}} +{{- end -}} +{{- end }} + {{- define "tracebloc.rbacName" -}} {{ include "tracebloc.fullname" . }}-jobs-manager-rbac {{- end }} @@ -384,11 +420,12 @@ nvidia-device-plugin-daemonset * `resourceMonitor: false` — there is no DaemonSet at all, so there is nothing to reconcile and a cross-namespace `set image` would just fail. - Nil-safe: `.Values.resourceMonitor` absent reads as enabled, matching the - `ne .Values.resourceMonitor false` gate on the DaemonSet itself. + Nil-safe via tracebloc.resourceMonitorEnabled, which absent reads as enabled, + matching the gate on the DaemonSet itself and honouring both the legacy scalar + and the new resourceMonitor.enabled object form. */}} {{- define "tracebloc.resourceMonitorRefreshPinned" -}} -{{- if eq .Values.resourceMonitor false -}} +{{- if not (include "tracebloc.resourceMonitorEnabled" .) -}} true {{- else if (default dict (default dict .Values.images).resourceMonitor).digest -}} true @@ -1473,7 +1510,7 @@ https://api.tracebloc.io/ became a second tenant, two of them were widened and the rest were not." The tri-state made `enabled` a second copy of the answer for a third time. */ -}} -{{- if or (ne .Values.resourceMonitor false) (eq (include "tracebloc.telemetryCollectorState" .) "enabled") }}true{{ end -}} +{{- if or (include "tracebloc.resourceMonitorEnabled" .) (eq (include "tracebloc.telemetryCollectorState" .) "enabled") }}true{{ end -}} {{- end -}} {{/* diff --git a/client/templates/jobs-manager-deployment.yaml b/client/templates/jobs-manager-deployment.yaml index 81007109..75460fd7 100644 --- a/client/templates/jobs-manager-deployment.yaml +++ b/client/templates/jobs-manager-deployment.yaml @@ -742,7 +742,7 @@ spec: # populated would list a namespace that isn't rendered — a 403 every # heartbeat for a workload the operator turned off. - name: NODE_AGENTS_NAMESPACE - value: {{ if ne .Values.resourceMonitor false }}{{ dig "namespace" "name" "" (.Values.nodeAgents | default dict) | quote }}{{ else }}""{{ end }} + value: {{ if (include "tracebloc.resourceMonitorEnabled" .) }}{{ dig "namespace" "name" "" (.Values.nodeAgents | default dict) | quote }}{{ else }}""{{ end }} # backend#664 (Utilization Ladder L0): with NEITHER env.RESOURCE_REQUESTS # nor env.RESOURCE_LIMITS set, BOTH vars are omitted and jobs-manager # sizes the envelope from node allocatable — BUT ONLY IF diff --git a/client/templates/rbac.yaml b/client/templates/rbac.yaml index 07765305..32341007 100644 --- a/client/templates/rbac.yaml +++ b/client/templates/rbac.yaml @@ -232,7 +232,7 @@ roleRef: {{- end }} {{- $nodeAgentsNs := dig "namespace" "name" "" (.Values.nodeAgents | default dict) }} -{{- if and (ne .Values.resourceMonitor false) $nodeAgentsNs (ne $nodeAgentsNs .Release.Namespace) }} +{{- if and (include "tracebloc.resourceMonitorEnabled" .) $nodeAgentsNs (ne $nodeAgentsNs .Release.Namespace) }} --- {{/* jobs-manager reads the resource-monitor DaemonSet for the heartbeat version diff --git a/client/templates/resource-monitor-daemonset.yaml b/client/templates/resource-monitor-daemonset.yaml index fca159ff..07340f5a 100644 --- a/client/templates/resource-monitor-daemonset.yaml +++ b/client/templates/resource-monitor-daemonset.yaml @@ -1,4 +1,4 @@ -{{- if ne .Values.resourceMonitor false }} +{{- if (include "tracebloc.resourceMonitorEnabled" .) }} {{/* Pre-flight: resource-monitor polls the metrics.k8s.io API, so metrics-server must be registered. We probe kube-system via `lookup` first — that returns @@ -66,7 +66,7 @@ {{- $preflight = "skipped-by-values" -}} {{- else -}} {{- if not (lookup "apiregistration.k8s.io/v1" "APIService" "" "v1beta1.metrics.k8s.io") -}} - {{- fail "resourceMonitor is enabled but the metrics.k8s.io/v1beta1 API is not registered. Install metrics-server (https://github.com/kubernetes-sigs/metrics-server) or set resourceMonitor: false. See SECURITY.md.\n\nIf THIS line was instead an `apiservices ... is forbidden` error, the problem is the caller's RBAC, not metrics-server: APIService is cluster-scoped and the built-in `admin` ClusterRole excludes it. Set nodeAgents.metricsServerPreflight: false to skip this check, or run the upgrade with cluster-scope read on apiservices (backend#2469)." -}} + {{- fail "resourceMonitor is enabled but the metrics.k8s.io/v1beta1 API is not registered. Install metrics-server (https://github.com/kubernetes-sigs/metrics-server) or set resourceMonitor.enabled: false. See SECURITY.md.\n\nIf THIS line was instead an `apiservices ... is forbidden` error, the problem is the caller's RBAC, not metrics-server: APIService is cluster-scoped and the built-in `admin` ClusterRole excludes it. Set nodeAgents.metricsServerPreflight: false to skip this check, or run the upgrade with cluster-scope read on apiservices (backend#2469)." -}} {{- end -}} {{- $preflight = "satisfied-by-apiservice" -}} {{- end -}} diff --git a/client/templates/resource-monitor-rbac.yaml b/client/templates/resource-monitor-rbac.yaml index da0339dc..5a68bf2c 100644 --- a/client/templates/resource-monitor-rbac.yaml +++ b/client/templates/resource-monitor-rbac.yaml @@ -1,4 +1,4 @@ -{{- if ne .Values.resourceMonitor false }} +{{- if (include "tracebloc.resourceMonitorEnabled" .) }} --- apiVersion: v1 kind: ServiceAccount @@ -27,7 +27,7 @@ metadata: the training/jobs isolation footprint elsewhere -- it must not cripple node telemetry by leaving the DaemonSet without the permissions it cannot run without. If a deployment genuinely cannot allow any cluster-scoped read, disable the - monitor entirely via .Values.resourceMonitor=false rather than deploying it broken. + monitor entirely via .Values.resourceMonitor.enabled=false rather than deploying it broken. */}} --- apiVersion: rbac.authorization.k8s.io/v1 diff --git a/client/templates/resource-monitor-scc.yaml b/client/templates/resource-monitor-scc.yaml index f9fba66c..b6b0f1f5 100644 --- a/client/templates/resource-monitor-scc.yaml +++ b/client/templates/resource-monitor-scc.yaml @@ -1,4 +1,4 @@ -{{- if and (ne .Values.resourceMonitor false) .Values.openshift.scc.enabled }} +{{- if and (include "tracebloc.resourceMonitorEnabled" .) .Values.openshift.scc.enabled }} --- apiVersion: security.openshift.io/v1 kind: SecurityContextConstraints diff --git a/client/templates/secrets.yaml b/client/templates/secrets.yaml index 474fc71c..cecfbeb3 100644 --- a/client/templates/secrets.yaml +++ b/client/templates/secrets.yaml @@ -510,7 +510,7 @@ data: {{- if (include "tracebloc.bootstrapDbReparent" .) }} DB_BOOTSTRAP_PASSWORD: {{ $bootstrapDbPassword | b64enc | quote }} {{- end }} -{{- if and (ne .Values.resourceMonitor false) (ne .Values.nodeAgents.namespace.name .Release.Namespace) }} +{{- if and (include "tracebloc.resourceMonitorEnabled" .) (ne .Values.nodeAgents.namespace.name .Release.Namespace) }} --- # Mirrored into the node-agents namespace so the resource-monitor DaemonSet # can read CLIENT_ID / CLIENT_PASSWORD via secretKeyRef. Secrets are diff --git a/client/tests/resource_monitor_test.yaml b/client/tests/resource_monitor_test.yaml index 6aa1f1bd..4941273e 100644 --- a/client/tests/resource_monitor_test.yaml +++ b/client/tests/resource_monitor_test.yaml @@ -249,3 +249,99 @@ tests: - equal: path: metadata.namespace value: tracebloc-node-agents + + # ── RFC-0076 bool→object alias window (client#1009) ─────────────────────────── + # `resourceMonitor: ` is being renamed to `resourceMonitor.enabled: ` + # (D2). This is a bool→object change, so a stored values.yaml or a bare + # `--set resourceMonitor=true` still arrives as a SCALAR; the templates route the + # gate through tracebloc.resourceMonitorEnabled, which must honour BOTH shapes + # for the whole window (remove_by: 2026-12-31). There are two crash directions + # the helper prevents: a blind `ne .Values.resourceMonitor false` gate fails + # "incompatible types for comparison: map and bool" on the new object, and a + # blind `.Values.resourceMonitor.enabled` read fails "can't evaluate field + # enabled in interface {}" on the legacy scalar. The three canonical inputs are + # legacy scalar, new object, and unset; null (a --reuse-values upgrade from a + # chart that predates the key) and `{}` (object with .enabled absent) are the + # two edges of "unset". + - it: renders the DaemonSet for the legacy scalar resourceMonitor=true + template: templates/resource-monitor-daemonset.yaml + set: + resourceMonitor: true + asserts: + - hasDocuments: + count: 1 + - isKind: + of: DaemonSet + + - it: renders the DaemonSet for the new object resourceMonitor.enabled=true + template: templates/resource-monitor-daemonset.yaml + set: + resourceMonitor: + enabled: true + asserts: + - hasDocuments: + count: 1 + - isKind: + of: DaemonSet + + - it: renders the DaemonSet on the chart default (unset — values.yaml object form) + template: templates/resource-monitor-daemonset.yaml + asserts: + - hasDocuments: + count: 1 + - isKind: + of: DaemonSet + + - it: treats an absent key (null, --reuse-values from a pre-key chart) as enabled + template: templates/resource-monitor-daemonset.yaml + set: + resourceMonitor: null + asserts: + - hasDocuments: + count: 1 + - isKind: + of: DaemonSet + + - it: treats an object with .enabled absent as enabled (default true) + template: templates/resource-monitor-daemonset.yaml + set: + resourceMonitor: {} + asserts: + - hasDocuments: + count: 1 + - isKind: + of: DaemonSet + + - it: renders nothing for the legacy scalar resourceMonitor=false + template: templates/resource-monitor-daemonset.yaml + set: + resourceMonitor: false + asserts: + - hasDocuments: + count: 0 + + - it: renders nothing for the new object resourceMonitor.enabled=false + template: templates/resource-monitor-daemonset.yaml + set: + resourceMonitor: + enabled: false + asserts: + - hasDocuments: + count: 0 + + # The object form CLOSES its keys (values.schema.json additionalProperties: + # false), so a mistyped `enabled` (enable / Enabled / disabled) is REFUSED at + # chart load rather than silently `dig`-defaulting back to true and turning the + # monitor back on — a silent-misconfiguration surface the scalar had no sub-key + # to expose. BARE failedTemplate, per the auto-upgrade schema tests: 0.5.2 + # reports a schema rejection as an errored test with no matchable render error, + # and errorPattern is silently ignored (backend#2606). To pin the specific + # "additional properties 'enable' not allowed" text, assert it from outside the + # plugin with `helm template` + grep. + - it: refuses a mistyped enabled sub-key instead of silently staying enabled + template: templates/resource-monitor-daemonset.yaml + set: + resourceMonitor: + enable: false + asserts: + - failedTemplate: {} diff --git a/client/values.schema.json b/client/values.schema.json index e98edc53..f035d7be 100644 --- a/client/values.schema.json +++ b/client/values.schema.json @@ -333,9 +333,19 @@ "description": "Use ClusterRole (true) or namespace-scoped Role (false). Defaults to true." }, "resourceMonitor": { - "type": "boolean", - "default": true, - "description": "Deploy resource-monitor DaemonSet. Defaults to true." + "type": [ + "boolean", + "object" + ], + "description": "Deploy the resource-monitor DaemonSet. RFC-0076 (D2): prefer the object form `resourceMonitor.enabled: `; the legacy scalar `resourceMonitor: ` (and `--set resourceMonitor=true`) is still accepted through the alias window (remove_by: 2026-12-31) — see tracebloc.resourceMonitorEnabled. Defaults to enabled.", + "properties": { + "enabled": { + "type": "boolean", + "default": true, + "description": "Deploy the resource-monitor DaemonSet. Defaults to true." + } + }, + "additionalProperties": false }, "nodeAgents": { "type": "object", diff --git a/client/values.yaml b/client/values.yaml index 4a4ba254..457b13d3 100644 --- a/client/values.yaml +++ b/client/values.yaml @@ -367,8 +367,14 @@ clusterScope: true # OC: uses the built-in OpenShift metrics stack. # kubeadm / bare-metal: install metrics-server manually; add # --kubelet-insecure-tls on clusters with self-signed kubelet certs. -# Set to false on clusters where metrics-server cannot be installed. -resourceMonitor: true +# Set enabled to false on clusters where metrics-server cannot be installed. +# +# RFC-0076 (D2): this was the scalar `resourceMonitor: ` and is now +# `resourceMonitor.enabled: `. The chart still honours the legacy scalar +# (and `--set resourceMonitor=true|false`) through the alias window +# (remove_by: 2026-12-31) — see tracebloc.resourceMonitorEnabled in _helpers.tpl. +resourceMonitor: + enabled: true # -- Node-level agents (currently: tracebloc-resource-monitor DaemonSet). # The resource-monitor needs hostPath /proc and /sys to read node metrics, From dc8ce418f3b4ef6ecec2ac05a2860c161e61e022 Mon Sep 17 00:00:00 2001 From: Arturo Peroni Date: Wed, 9 Sep 2026 13:48:10 +0200 Subject: [PATCH 2/2] =?UTF-8?q?chore(chart):=20bump=20to=201.9.109=20(one?= =?UTF-8?q?=20patch=20above=20develop,=20chart-version-guard)=20=E2=80=94?= =?UTF-8?q?=20client#1009?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Develop advanced to 1.9.108 after this branch bumped there too; re-bump so the version stays above develop. Co-Authored-By: Claude Opus 4.8 --- client/Chart.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/Chart.yaml b/client/Chart.yaml index 99f71ce9..bdaf83cb 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.108 -appVersion: "1.9.108" +version: 1.9.109 +appVersion: "1.9.109" keywords: - tracebloc - kubernetes