fix(multus): drop the CPU limit and make resources configurable - #4830

[MAJOR] the "keep a memory limit" invariant is documented but not enforced at render

values.yaml:16 states "Keep a memory limit whatever its size" and the comment above resources explains why: with no memory.max on the pod cgroup the Talos userspace OOM handler (v1.12+) makes multus an eviction candidate on every node, which is the exact failure this change exists to prevent. Nothing enforces that invariant.

Helm deep-merge protects the accidental paths ({}, a partial --set resources.limits.cpu=2, --set resources.requests.cpu=200m all keep the 4Gi limit, verified by render). But a deliberate null, which an operator can write through the documented spec.components.multus.values.resources override, reaches the dangerous state silently:

$ helm template r packages/system/multus --set resources.limits.memory=null --set resources.limits.cpu=2
          resources:
            limits:
              cpu: 2
            requests:
              cpu: 100m
              memory: 100Mi

A CPU limit is back and the memory limit is gone; resources: null and resources.limits: null drop it the same way (rendered). All 14 unittests stay green, so the suite pins only the default render. Before this PR the limit was hardcoded and could not be dropped at all, so making the block configurable opens a safety surface that did not exist.

This chart already sets the bar for exactly this class of value: stageCniPlugins is validated at render with {{- fail }} (template:244) and has a failedTemplate test plus a null test (tests/multus_test.yaml:134). Mirror it for the memory limit, e.g.

{{- if not (kindIs "map" .Values.resources) }}{{- fail (printf "resources: expected a map, got %s" (kindOf .Values.resources)) }}{{- end }}
{{- if not (dig "limits" "memory" "" .Values.resources) }}{{- fail "resources.limits.memory must be set, see the comment above" }}{{- end }}

and add a failedTemplate test for resources.limits.memory: null modeled on the stageCniPlugins one. What would change my mind: a namespace LimitRange that reliably stamps a memory limit regardless (the PR body claims one; grep -rn LimitRange found none in-tree).

The chart now refuses to render without a memory limit, fixed in 7d3d60c. The LimitRange exists, but the operator creates it in Go, so a grep over YAML can't find it. It only sets a default memory limit and request, and it is best effort: reconcile errors are just logged, and --system-namespace-memory-limit=0 turns it off. The chart can also be installed without the operator. So I kept the guard.

[MINOR] resources.requests: null reserves the 4Gi limit on every node

With resources.requests: null the block renders with only limits: {memory: 4Gi} (verified by render). Kubernetes defaults an absent request to the limit, so every DaemonSet pod then requests 4Gi of memory on every node. On a node whose allocatable memory is below 4Gi the multus pod stays Pending and that node has no CNI. This is admin-reachable, not an accidental path, hence MINOR; a dig "requests" "memory" check would cover it if you add the guard above, but it widens the chart's responsibility so it is your call.

I went with a guard here too. A default that survives null would override a value the admin set on purpose, while a failed render says what is missing. requests.cpu stays optional: without it the pod still schedules, just with fewer CPU shares.

[NIT] the overridable test covers only the additive direction

The new resources are overridable from values test adds limits.cpu and raises limits.memory; there is no test for the subtractive direction (null) or a wrong type. Without the guard from the MAJOR there is nothing to assert against, so this folds into that fix; listed separately so it is not lost. A failedTemplate case on resources.limits.memory: null closes both.

Covered by the new failure tests for both guards.

[MINOR] a zero-quantity memory limit passes the guard

dig "limits" "memory" "" .Values.resources tests truthiness, so null, "", 0 (int) and false fail loudly as intended. But 0Gi, 0Mi and "0" are non-empty strings and pass the guard: helm template r packages/system/multus --set resources.limits.memory=0Gi renders the kube-multus container with limits.memory: 0Gi and exits 0.

The kubelet forwards a zero memory limit to the runtime as no limit, so the pod cgroup ends up with no memory.max, the exact state this guard's message says it prevents, while the render reports success. It takes a deliberate 0Gi/0Mi/"0", and the operator can set a real value to recover, so this is a follow-up and not a blocker. Rejecting a zero numeric part on limits.memory closes it.

The render part is confirmed, 0Gi and "0" both render. The apply is louder than that, though. With the default 100Mi request the API server rejects the DaemonSet, because the request is above the limit. The pod runs without memory.max only when requests.memory is zero too, and the requests guard lets that through as well. Tracked in #4885 as rejecting a zero quantity on both keys.

[MINOR] the platform helper validates only that resources is a map

A mid-level type error such as networking.multus.resources.limits: 4Gi passes this check and renders a Package carrying limits: 4Gi. The platform HelmRelease then goes Ready and the failure surfaces one object away, in the child multus HelmRelease (resources.limits: expected a map, got string). Loud, but one hop late. The sibling stageCniPlugins path validates fully here at the platform.

Agreed. Tracked in #4885: the platform can run the same limits/requests map checks the multus chart does.

[NIT] empty-map memory limit fails with a misleading message

resources.limits.memory: {} fails with the "must be set" error although it is set: dig returns the empty map, which is falsy, so the render stops. The outcome is correct, but the wording misleads for a value that is present but empty.

Reproduced, limits.memory: {} stops with "must be set". Tracked in #4885 together with the zero-quantity case.

[NIT] comment says resources is passed only when set

An empty map resources: {} is still forwarded as resources: {} into the Package, even though the comment says resources is passed only when set. Harmless, since the chart deep-merge restores every default, but the comment is imprecise about the empty-map case.

{} is forwarded, and Helm keeps the defaults under it. Comment fix tracked in #4885.