diff --git a/client/Chart.yaml b/client/Chart.yaml index f8b0cd9b..d8b3ed31 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.115 -appVersion: "1.9.115" +version: 1.9.116 +appVersion: "1.9.116" keywords: - tracebloc - kubernetes diff --git a/client/templates/NOTES.txt b/client/templates/NOTES.txt index 338b8da3..87543597 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 7928f9af..0f7360b3 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 @@ -1556,7 +1593,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 cd8f1cfe..4a016941 100644 --- a/client/templates/jobs-manager-deployment.yaml +++ b/client/templates/jobs-manager-deployment.yaml @@ -750,7 +750,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 81bf11c4..08cec0eb 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 2e9c7d24..4f163a85 100644 --- a/client/tests/resource_monitor_test.yaml +++ b/client/tests/resource_monitor_test.yaml @@ -251,3 +251,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 4fc48e03..63dd3783 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 f7964a8d..f52fb307 100644 --- a/client/values.yaml +++ b/client/values.yaml @@ -376,8 +376,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,