diff --git a/packages/system/cilium/Makefile b/packages/system/cilium/Makefile index fa90cf568..b221bb3da 100644 --- a/packages/system/cilium/Makefile +++ b/packages/system/cilium/Makefile @@ -6,6 +6,12 @@ export NAMESPACE=cozy-$(NAME) include ../../../hack/common-envs.mk include ../../../hack/package.mk +# hack/helm-unit-tests.sh discovers work by probing for this target, so a +# package without one is skipped in silence however many suites sit under +# tests/. +test: + helm unittest . + update: rm -rf charts helm repo add cilium https://helm.cilium.io/ diff --git a/packages/system/cilium/tests/agent_resources_test.yaml b/packages/system/cilium/tests/agent_resources_test.yaml new file mode 100644 index 000000000..3ccd1421d --- /dev/null +++ b/packages/system/cilium/tests/agent_resources_test.yaml @@ -0,0 +1,81 @@ +suite: cilium-agent memory requests and limits + +# The cilium-agent DaemonSet ran with no resources at all, which put it in the +# Talos userspace OOM handler's victim set: that handler (v1.12+) only ever +# selects cgroups that lack memory.max, and picks by QoS class rather than by +# what caused the pressure. Losing cilium-agent on a node takes that node's +# datapath programming with it. +# +# The limit is what sets memory.max, and it is set on the POD cgroup only when +# EVERY container in the pod has one — so all seven are asserted here, not just +# the agent. Five of the init containers take their values from initResources; +# install-cni-binaries carries upstream's own cni.resources and is asserted to +# document that it is already covered rather than overlooked. +# +# Both keys are set from the umbrella values.yaml against the vendored subchart. +# An upstream bump that renames or relocates either one renders nothing and +# leaves no other trace, silently returning the pod to the victim set; catching +# that is the main thing these assertions are for. +release: + name: cilium + namespace: cozy-cilium +tests: + - it: cilium-agent declares a memory limit and request + template: charts/cilium/templates/cilium-agent/daemonset.yaml + documentSelector: + path: kind + value: DaemonSet + asserts: + - equal: + path: spec.template.spec.containers[?(@.name=="cilium-agent")].resources.limits.memory + value: 4Gi + - equal: + path: spec.template.spec.containers[?(@.name=="cilium-agent")].resources.requests.memory + value: 256Mi + + # Asserted one container at a time. A negated matcher over the init container + # list reads as if it covers them all but passes vacuously, so each limit is + # pinned explicitly instead. + - it: every cilium-agent init container declares a memory limit + template: charts/cilium/templates/cilium-agent/daemonset.yaml + documentSelector: + path: kind + value: DaemonSet + asserts: + - equal: + path: spec.template.spec.initContainers[?(@.name=="config")].resources.limits.memory + value: 256Mi + - equal: + path: spec.template.spec.initContainers[?(@.name=="mount-cgroup")].resources.limits.memory + value: 256Mi + - equal: + path: spec.template.spec.initContainers[?(@.name=="apply-sysctl-overwrites")].resources.limits.memory + value: 256Mi + - equal: + path: spec.template.spec.initContainers[?(@.name=="mount-bpf-fs")].resources.limits.memory + value: 256Mi + - equal: + path: spec.template.spec.initContainers[?(@.name=="clean-cilium-state")].resources.limits.memory + value: 256Mi + # Upstream's cni.resources, not initResources — pinned so a bump that + # drops it is caught here rather than by a pod that quietly loses + # memory.max. + - equal: + path: spec.template.spec.initContainers[?(@.name=="install-cni-binaries")].resources.limits.memory + value: 1Gi + + # cilium-envoy is a second DaemonSet on the same nodes. Its limit predates + # this suite; it is pinned here because the pod-level argument above applies + # to it identically. + - it: cilium-envoy declares a memory limit and request + template: charts/cilium/templates/cilium-envoy/daemonset.yaml + documentSelector: + path: kind + value: DaemonSet + asserts: + - equal: + path: spec.template.spec.containers[?(@.name=="cilium-envoy")].resources.limits.memory + value: 1Gi + - equal: + path: spec.template.spec.containers[?(@.name=="cilium-envoy")].resources.requests.memory + value: 512Mi diff --git a/packages/system/cilium/values.yaml b/packages/system/cilium/values.yaml index 74ce563e3..5e1ef9630 100644 --- a/packages/system/cilium/values.yaml +++ b/packages/system/cilium/values.yaml @@ -38,6 +38,30 @@ cilium: memory: 512Mi limits: memory: 1Gi + # The cilium-agent DaemonSet had the same gap the envoy DaemonSet above + # already closed. Beyond the node-headroom argument, a memory limit is what + # sets memory.max on the pod cgroup, and the Talos userspace OOM handler + # (v1.12+) only ever selects cgroups that lack it — so an unlimited agent is + # an eviction candidate on every node regardless of what caused the pressure. + # 14d peak on a 3-node dev cluster is 575Mi, and the agent grows with + # endpoint and policy count, so the limit matches the 4Gi in the upstream + # values' own commented example rather than being fitted to that measurement. + resources: + requests: + cpu: 100m + memory: 256Mi + limits: + memory: 4Gi + # Shared by the agent's init containers (config, mount-cgroup, mount-bpf-fs, + # apply-sysctl-overwrites, clean-cilium-state). All are short-lived setup + # steps, but each one still has to carry a limit for the pod cgroup to get + # memory.max at all. + initResources: + requests: + cpu: 10m + memory: 32Mi + limits: + memory: 256Mi gatewayAPI: enabled: true # Upstream default is false — Envoy then advertises no ALPN protocols on diff --git a/packages/system/kubevirt-csi-node/templates/deploy.yaml b/packages/system/kubevirt-csi-node/templates/deploy.yaml index 4b0afad79..cac1db440 100644 --- a/packages/system/kubevirt-csi-node/templates/deploy.yaml +++ b/packages/system/kubevirt-csi-node/templates/deploy.yaml @@ -196,10 +196,17 @@ spec: timeoutSeconds: 3 periodSeconds: 10 failureThreshold: 5 + # Limits, not just requests: a memory limit is what sets memory.max on + # the pod cgroup, and the Talos userspace OOM handler (v1.12+) only + # ever kills cgroups that lack one. Requests alone left this DaemonSet + # an eviction candidate on every node whatever caused the pressure. + # Every container in the pod needs one for the pod cgroup to get it. resources: requests: memory: 50Mi cpu: 10m + limits: + memory: 512Mi - name: csi-node-driver-registrar image: quay.io/openshift/origin-csi-node-driver-registrar:latest args: @@ -224,6 +231,8 @@ spec: requests: memory: 20Mi cpu: 5m + limits: + memory: 128Mi - name: csi-liveness-probe image: quay.io/openshift/origin-csi-livenessprobe:latest args: @@ -237,6 +246,8 @@ spec: requests: memory: 20Mi cpu: 5m + limits: + memory: 128Mi volumes: - name: kubelet-dir hostPath: diff --git a/packages/system/kubevirt-csi-node/tests/daemonset_resources_test.yaml b/packages/system/kubevirt-csi-node/tests/daemonset_resources_test.yaml new file mode 100644 index 000000000..6c3bf5368 --- /dev/null +++ b/packages/system/kubevirt-csi-node/tests/daemonset_resources_test.yaml @@ -0,0 +1,48 @@ +suite: csi-node DaemonSet memory limits + +# The node DaemonSet carried requests but no limits, which leaves it Burstable +# rather than Guaranteed-of-a-cgroup-limit: a memory limit is what sets +# memory.max on the pod cgroup, and the Talos userspace OOM handler (v1.12+) +# only ever selects cgroups that lack it. Requests alone did not take the pod +# out of the victim set. +# +# memory.max lands on the POD cgroup only when EVERY container has a limit, so +# all three are asserted rather than the driver alone — one limit-free sidecar +# would put the whole pod back in the victim set while the other two assertions +# still passed, which is precisely the failure this suite exists to catch. + +templates: + - templates/deploy.yaml + +tests: + - it: every csi-node container declares a memory limit + documentSelector: + path: kind + value: DaemonSet + asserts: + - equal: + path: spec.template.spec.containers[?(@.name=="csi-driver")].resources.limits.memory + value: 512Mi + - equal: + path: spec.template.spec.containers[?(@.name=="csi-node-driver-registrar")].resources.limits.memory + value: 128Mi + - equal: + path: spec.template.spec.containers[?(@.name=="csi-liveness-probe")].resources.limits.memory + value: 128Mi + + # Pinned alongside the limits because they are what the limits were sized + # against; a drift in either direction is worth catching. + - it: csi-node containers keep their memory requests + documentSelector: + path: kind + value: DaemonSet + asserts: + - equal: + path: spec.template.spec.containers[?(@.name=="csi-driver")].resources.requests.memory + value: 50Mi + - equal: + path: spec.template.spec.containers[?(@.name=="csi-node-driver-registrar")].resources.requests.memory + value: 20Mi + - equal: + path: spec.template.spec.containers[?(@.name=="csi-liveness-probe")].resources.requests.memory + value: 20Mi diff --git a/packages/system/kubevirt/templates/kubevirt-cr.yaml b/packages/system/kubevirt/templates/kubevirt-cr.yaml index 10754aad3..d0738b29e 100644 --- a/packages/system/kubevirt/templates/kubevirt-cr.yaml +++ b/packages/system/kubevirt/templates/kubevirt-cr.yaml @@ -42,6 +42,19 @@ spec: - Evict batchEvictionInterval: 1m batchEvictionSize: 10 - customizeComponents: {} + # virt-handler is a node DaemonSet and ran BestEffort. The Talos userspace OOM + # handler (v1.12+) only ever kills cgroups with no memory.max, and picks its + # victim by QoS class rather than by what caused the pressure — so virt-handler + # was an eviction candidate on every node, and losing it stops VM lifecycle + # operations there. virt-operator owns the DaemonSet, so resources cannot be + # set from chart values; customizeComponents.patches is the supported seam. + # 14d peak 207Mi on a 3-node dev cluster; grows with VMs per node. + customizeComponents: + patches: + - resourceName: virt-handler + resourceType: DaemonSet + type: strategic + patch: | + {"spec":{"template":{"spec":{"containers":[{"name":"virt-handler","resources":{"requests":{"cpu":"100m","memory":"128Mi"},"limits":{"memory":"2Gi"}}}]}}}} imagePullPolicy: IfNotPresent monitorNamespace: tenant-root diff --git a/packages/system/kubevirt/tests/kubevirt_cr_test.yaml b/packages/system/kubevirt/tests/kubevirt_cr_test.yaml index eda3218ca..9ea588810 100644 --- a/packages/system/kubevirt/tests/kubevirt_cr_test.yaml +++ b/packages/system/kubevirt/tests/kubevirt_cr_test.yaml @@ -92,3 +92,27 @@ tests: path: spec.configuration.mediatedDevicesConfiguration.mediatedDeviceTypes value: - nvidia-740 + + # virt-operator owns the virt-handler DaemonSet, so resources cannot come from + # chart values — customizeComponents.patches is the only seam. virt-handler ran + # BestEffort on every node, and the Talos userspace OOM handler (v1.12+) only + # kills cgroups with no memory.max, so it was a standing eviction candidate. + # Pin the patch: a malformed or dropped entry fails silently at runtime, since + # virt-operator simply reconciles the DaemonSet without the resources. + - it: patches virt-handler with memory requests and limits + asserts: + - equal: + path: spec.customizeComponents.patches[0].resourceName + value: virt-handler + - equal: + path: spec.customizeComponents.patches[0].resourceType + value: DaemonSet + - equal: + path: spec.customizeComponents.patches[0].type + value: strategic + - matchRegex: + path: spec.customizeComponents.patches[0].patch + pattern: '"limits":\{"memory":"2Gi"\}' + - matchRegex: + path: spec.customizeComponents.patches[0].patch + pattern: '"requests":\{"cpu":"100m","memory":"128Mi"\}' diff --git a/packages/system/linstor/templates/satellites-cozy.yaml b/packages/system/linstor/templates/satellites-cozy.yaml index e6f877a04..7897189f4 100644 --- a/packages/system/linstor/templates/satellites-cozy.yaml +++ b/packages/system/linstor/templates/satellites-cozy.yaml @@ -14,6 +14,24 @@ spec: containers: - name: linstor-satellite image: {{ .Values.piraeusServer.image.repository }}:{{ .Values.piraeusServer.image.tag }} + # The satellite DaemonSet ran BestEffort on every storage node, which + # made it a standing victim for the Talos userspace OOM handler + # (v1.12+): that handler only ever kills cgroups with no memory.max, and + # picks by QoS class rather than by what caused the pressure. A tenant + # workload thrashing against its own limit would get the satellite + # killed, taking DRBD connections down with it. + # + # Note this is the opposite call from linstor-controller, which is + # deliberately left limit-free in values.yaml (a JVM that must not be + # OOM-killed mid-operation). The satellite is a Go binary with a bounded + # working set, so a generous limit costs nothing and buys immunity. + # 14d peak 368Mi on a 3-node dev cluster; grows with resource count. + resources: + requests: + cpu: 100m + memory: 128Mi + limits: + memory: 2Gi startupProbe: tcpSocket: port: linstor diff --git a/packages/system/linstor/templates/satellites-plunger.yaml b/packages/system/linstor/templates/satellites-plunger.yaml index ab71298bf..00018faf6 100644 --- a/packages/system/linstor/templates/satellites-plunger.yaml +++ b/packages/system/linstor/templates/satellites-plunger.yaml @@ -12,6 +12,17 @@ spec: containers: - name: plunger image: {{ .Values.piraeusServer.image.repository }}:{{ .Values.piraeusServer.image.tag }} + # Sidecars in the satellite pod. Every container needs a memory limit + # for the pod cgroup to get memory.max, which is what keeps the pod out + # of the Talos userspace OOM handler's victim set — one limit-free + # sidecar is enough to put the whole satellite pod back in it. + # 14d peaks on a 3-node dev cluster: plunger 18Mi, drbd-logger 5Mi. + resources: + requests: + cpu: 10m + memory: 32Mi + limits: + memory: 128Mi command: - "/scripts/plunger-satellite.sh" securityContext: @@ -49,6 +60,12 @@ spec: readOnly: true - name: drbd-logger image: {{ .Values.piraeusServer.image.repository }}:{{ .Values.piraeusServer.image.tag }} + resources: + requests: + cpu: 10m + memory: 32Mi + limits: + memory: 128Mi command: - "/scripts/plunger-drbd-logger.sh" securityContext: diff --git a/packages/system/linstor/tests/satellites_test.yaml b/packages/system/linstor/tests/satellites_test.yaml new file mode 100644 index 000000000..6e4935637 --- /dev/null +++ b/packages/system/linstor/tests/satellites_test.yaml @@ -0,0 +1,39 @@ +suite: satellite pod resource limits + +# The satellite DaemonSet ran BestEffort, which made it a standing victim for the +# Talos userspace OOM handler (v1.12+): that handler only ever kills cgroups with +# no memory.max, and picks its victim by QoS class rather than by what caused the +# pressure. A tenant workload thrashing against its own limit would get the +# satellite killed, taking DRBD connections on that node down with it. +# +# Every container in the pod needs a limit for the pod cgroup to get memory.max, +# so the plunger sidecars are pinned alongside the satellite itself — one +# limit-free sidecar is enough to put the whole pod back in the victim set. +# +# Note this is the opposite call from linstor-controller, which cluster_test.yaml +# pins as deliberately limit-free (a JVM that must not be OOM-killed mid-operation). + +templates: + - templates/satellites-cozy.yaml + - templates/satellites-plunger.yaml + +tests: + - it: satellite declares a memory limit and request + template: templates/satellites-cozy.yaml + asserts: + - equal: + path: spec.podTemplate.spec.containers[?(@.name=="linstor-satellite")].resources.limits.memory + value: 2Gi + - equal: + path: spec.podTemplate.spec.containers[?(@.name=="linstor-satellite")].resources.requests.memory + value: 128Mi + + - it: plunger sidecars declare memory limits + template: templates/satellites-plunger.yaml + asserts: + - equal: + path: spec.podTemplate.spec.containers[?(@.name=="plunger")].resources.limits.memory + value: 128Mi + - equal: + path: spec.podTemplate.spec.containers[?(@.name=="drbd-logger")].resources.limits.memory + value: 128Mi diff --git a/packages/system/metallb/tests/metallb_test.yaml b/packages/system/metallb/tests/metallb_test.yaml index 29f9bdc28..3af5ad68e 100644 --- a/packages/system/metallb/tests/metallb_test.yaml +++ b/packages/system/metallb/tests/metallb_test.yaml @@ -185,3 +185,49 @@ tests: - equal: path: metadata.name value: metallb-frr-k8s + + # Both node DaemonSets must carry a memory limit. That limit is what sets + # memory.max on the pod cgroup, and the Talos userspace OOM handler (v1.12+) + # only ever kills cgroups that lack one — a regression here silently puts the + # speaker back in the victim set, which is the failure this pins against. + - it: speaker declares a memory limit and request + template: charts/metallb/templates/speaker.yaml + documentSelector: + path: kind + value: DaemonSet + asserts: + - equal: + path: spec.template.spec.containers[0].resources.limits.memory + value: 512Mi + - equal: + path: spec.template.spec.containers[0].resources.requests.memory + value: 64Mi + + - it: every frr-k8s long-running container declares a memory limit + template: charts/metallb/charts/frr-k8s/templates/controller.yaml + documentSelector: + path: kind + value: DaemonSet + # Asserted one container at a time. A negated matcher over the container + # list looks like it covers all five but passes vacuously, so each limit is + # pinned explicitly instead. + # + # The four cp-* initContainers are intentionally not asserted: upstream + # hardcodes them with no resources knob, so they rely on the + # operator-managed namespace LimitRange instead. + asserts: + - equal: + path: spec.template.spec.containers[?(@.name=="controller")].resources.limits.memory + value: 512Mi + - equal: + path: spec.template.spec.containers[?(@.name=="frr")].resources.limits.memory + value: 512Mi + - equal: + path: spec.template.spec.containers[?(@.name=="reloader")].resources.limits.memory + value: 128Mi + - equal: + path: spec.template.spec.containers[?(@.name=="frr-metrics")].resources.limits.memory + value: 256Mi + - equal: + path: spec.template.spec.containers[?(@.name=="frr-status")].resources.limits.memory + value: 256Mi diff --git a/packages/system/metallb/values.yaml b/packages/system/metallb/values.yaml index 60d31c6cd..209948673 100644 --- a/packages/system/metallb/values.yaml +++ b/packages/system/metallb/values.yaml @@ -42,6 +42,22 @@ metallb: image: repository: ghcr.io/cozystack/cozystack/metallb-speaker tag: v1.6.0@sha256:61ab7e2381f0897607b15df7cb5ba4d51a947c261c1c197fcda3b79c7a280016 + # A memory limit is what sets memory.max on the pod cgroup, and the Talos + # userspace OOM handler (v1.12+) only ever kills cgroups that lack it. The + # speaker ran BestEffort, so it was picked as the victim whenever a tenant + # workload drove node memory PSI — the pressure and the victim being + # unrelated by design. See the same block on every node DaemonSet below. + # + # Requests come from observed usage (14d peak 97Mi on a 3-node dev cluster); + # limits are deliberately loose, sized as a runaway backstop rather than a + # fitted ceiling, because undersizing here converts a rare pressure-driven + # kill into a deterministic one. + resources: + requests: + cpu: 50m + memory: 64Mi + limits: + memory: 512Mi # The vendored metallb chart's values.yaml leaves `frr-k8s.prometheus:` as # a YAML block containing only commented examples — it parses to null. # Helm-controller's deep-merge then writes that null over the frr-k8s @@ -62,3 +78,47 @@ metallb: enabled: false podMonitor: enabled: false + # Same reasoning as speaker.resources above. frr-k8s is the sibling node + # DaemonSet that actually speaks BGP, so it shares the speaker's exposure. + # 14d peaks on a 3-node dev cluster: controller ~124Mi, frr 43Mi, + # frr-metrics 49Mi, frr-status 31Mi, reloader 7Mi. + # + # The four cp-* init containers are NOT covered here: the upstream chart + # hardcodes them with no resources knob, so the pod-level memory.max still + # depends on the default LimitRange the operator maintains in every system + # namespace. Lifting that needs an upstream change in metallb/frr-k8s. + frrk8s: + resources: + requests: + cpu: 50m + memory: 64Mi + limits: + memory: 512Mi + frr: + resources: + requests: + cpu: 50m + memory: 64Mi + limits: + memory: 512Mi + reloader: + resources: + requests: + cpu: 10m + memory: 16Mi + limits: + memory: 128Mi + frrMetrics: + resources: + requests: + cpu: 10m + memory: 32Mi + limits: + memory: 256Mi + frrStatus: + resources: + requests: + cpu: 10m + memory: 32Mi + limits: + memory: 256Mi diff --git a/packages/system/monitoring-agents/tests/node_daemonset_resources_test.yaml b/packages/system/monitoring-agents/tests/node_daemonset_resources_test.yaml new file mode 100644 index 000000000..f057bee84 --- /dev/null +++ b/packages/system/monitoring-agents/tests/node_daemonset_resources_test.yaml @@ -0,0 +1,37 @@ +suite: node DaemonSet memory requests and limits + +# fluent-bit and node-exporter both run on every node and both ran BestEffort. +# The Talos userspace OOM handler (v1.12+) only ever selects cgroups that lack +# memory.max, and it picks its victim by QoS class rather than by what caused +# the pressure — so either one could be killed for a tenant workload thrashing +# against its own limit, on a node that was otherwise healthy. +# +# A memory limit is the only thing that sets memory.max, so the limits below are +# the load-bearing assertions; the requests are pinned alongside them because +# they are what the values were sized from and a silent drift in either +# direction is worth catching. Both are set from the umbrella values.yaml, which +# means an upstream subchart bump that renames or moves the key would render +# nothing at all and leave no other trace — exactly what these pin against. +release: + name: monitoring-agents + namespace: cozy-monitoring +tests: + - it: fluent-bit declares a memory limit and request + template: charts/fluent-bit/templates/daemonset.yaml + asserts: + - equal: + path: spec.template.spec.containers[?(@.name=="fluent-bit")].resources.limits.memory + value: 1Gi + - equal: + path: spec.template.spec.containers[?(@.name=="fluent-bit")].resources.requests.memory + value: 128Mi + + - it: node-exporter declares a memory limit and request + template: charts/prometheus-node-exporter/templates/daemonset.yaml + asserts: + - equal: + path: spec.template.spec.containers[?(@.name=="node-exporter")].resources.limits.memory + value: 256Mi + - equal: + path: spec.template.spec.containers[?(@.name=="node-exporter")].resources.requests.memory + value: 32Mi diff --git a/packages/system/monitoring-agents/values.yaml b/packages/system/monitoring-agents/values.yaml index 4357db276..0770e1deb 100644 --- a/packages/system/monitoring-agents/values.yaml +++ b/packages/system/monitoring-agents/values.yaml @@ -285,6 +285,14 @@ vmagent: extraArgs: {} fluent-bit: + # 14d peak 319Mi; scales with log volume, so the limit carries wide headroom. + # See the node-DaemonSet note at the bottom of this file. + resources: + requests: + cpu: 50m + memory: 128Mi + limits: + memory: 1Gi rbac: eventsAccess: true # Relax probe timing to survive a loaded install. Fluent-bit's HTTP server on @@ -445,3 +453,19 @@ fluent-bit: scrapeRules: etcd: enabled: false + +# Node DaemonSets: a memory limit is what sets memory.max on the pod cgroup, and +# the Talos userspace OOM handler (v1.12+) only ever kills cgroups that lack one. +# Both of these ran BestEffort on every node, so they were eviction candidates +# whenever a tenant workload drove node memory PSI. Requests come from observed +# usage (14d peak on a 3-node dev cluster); limits are a runaway backstop rather +# than a fitted ceiling — fluent-bit in particular scales with log volume, which +# a dev cluster understates. +prometheus-node-exporter: + # 14d peak 42Mi. + resources: + requests: + cpu: 10m + memory: 32Mi + limits: + memory: 256Mi diff --git a/packages/system/multus/patches/customize-deployment.patch b/packages/system/multus/patches/customize-deployment.patch index c89e3e9de..e4afcf707 100644 --- a/packages/system/multus/patches/customize-deployment.patch +++ b/packages/system/multus/patches/customize-deployment.patch @@ -41,7 +41,30 @@ labels: tier: node app: multus -@@ -160,10 +161,9 @@ +@@ -157,13 +158,33 @@ + - name: kube-multus + image: ghcr.io/k8snetworkplumbingwg/multus-cni:snapshot-thick + command: [ "/usr/src/multus-cni/bin/multus-daemon" ] ++ # Upstream sets 50Mi for both the request and the limit. This patch ++ # already raised the request to 100Mi and DELETED the memory limit ++ # outright, leaving only the CPU one -- which left the pod cgroup with ++ # no memory.max, and the Talos userspace OOM handler (v1.12+) only ++ # ever kills cgroups that lack it. So dropping the limit to survive ++ # 50Mi is what made multus an eviction candidate on every node. ++ # ++ # Restore a limit, sized as a backstop rather than a ceiling: 14d peak ++ # is 230Mi on a 3-node dev cluster and this grows with pods per node, ++ # so upstream's 50Mi was never survivable and a fitted value would ++ # trade a rare pressure-driven kill for a deterministic one. ++ # ++ # 1Gi rather than a tighter multiple of that peak, because the peak is ++ # the part least worth trusting here: multus-daemon holds per-pod ++ # state and a 3-node dev cluster understates pods per node more than ++ # it understates anything else measured for this change. The scar this ++ # avoids is the vertical-pod-autoscaler updater, which OOM-looped at a ++ # 110Mi limit for the same reason -- it scaled with pods in the ++ # cluster -- and was given a limit 2.5x its measured request to stop ++ # it. A limit costs nothing until it is hit; only the request reserves. resources: requests: cpu: "100m" @@ -50,10 +73,26 @@ limits: cpu: "100m" - memory: "50Mi" ++ memory: "1Gi" securityContext: privileged: true terminationMessagePolicy: FallbackToLogsOnError -@@ -220,6 +220,76 @@ +@@ -209,10 +230,14 @@ + - "/host/opt/cni/bin" + - "-t" + - "thick" ++ # Short-lived, but it still needs a limit for the pod cgroup to get ++ # memory.max — the init container counts. + resources: + requests: + cpu: "10m" + memory: "15Mi" ++ limits: ++ memory: "128Mi" + securityContext: + privileged: true + terminationMessagePolicy: FallbackToLogsOnError +@@ -220,6 +245,81 @@ - name: cnibin mountPath: /host/opt/cni/bin mountPropagation: Bidirectional @@ -101,10 +140,15 @@ + echo "failed to install:${failed}" > /dev/termination-log 2>/dev/null || true + fi + exit 0 ++ # Short-lived, but it still needs a limit for the pod cgroup to get ++ # memory.max — the init container counts. Same 128Mi as the sibling ++ # install-multus-binary above: both only copy binaries to the host. + resources: + requests: + cpu: "10m" + memory: "15Mi" ++ limits: ++ memory: "128Mi" + # cilium's install-cni-binaries copies plugin binaries into this same + # directory with ALL capabilities dropped, in the spc_t domain, with + # no privileged flag. On a node with SELinux enforcing, a container diff --git a/packages/system/multus/templates/multus-daemonset-thick.yml b/packages/system/multus/templates/multus-daemonset-thick.yml index 793afbf30..33b1a03e1 100644 --- a/packages/system/multus/templates/multus-daemonset-thick.yml +++ b/packages/system/multus/templates/multus-daemonset-thick.yml @@ -158,12 +158,33 @@ spec: - name: kube-multus image: ghcr.io/cozystack/cozystack/multus-cni:v1.6.0@sha256:0a92e861ead179dd6b46f475fd8878d44b3cfa40ce4d7a8ff92f790d54250d25 command: [ "/usr/src/multus-cni/bin/multus-daemon" ] + # Upstream sets 50Mi for both the request and the limit. This patch + # already raised the request to 100Mi and DELETED the memory limit + # outright, leaving only the CPU one -- which left the pod cgroup with + # no memory.max, and the Talos userspace OOM handler (v1.12+) only + # ever kills cgroups that lack it. So dropping the limit to survive + # 50Mi is what made multus an eviction candidate on every node. + # + # Restore a limit, sized as a backstop rather than a ceiling: 14d peak + # is 230Mi on a 3-node dev cluster and this grows with pods per node, + # so upstream's 50Mi was never survivable and a fitted value would + # trade a rare pressure-driven kill for a deterministic one. + # + # 1Gi rather than a tighter multiple of that peak, because the peak is + # the part least worth trusting here: multus-daemon holds per-pod + # state and a 3-node dev cluster understates pods per node more than + # it understates anything else measured for this change. The scar this + # avoids is the vertical-pod-autoscaler updater, which OOM-looped at a + # 110Mi limit for the same reason -- it scaled with pods in the + # cluster -- and was given a limit 2.5x its measured request to stop + # it. A limit costs nothing until it is hit; only the request reserves. resources: requests: cpu: "100m" memory: "100Mi" limits: cpu: "100m" + memory: "1Gi" securityContext: privileged: true terminationMessagePolicy: FallbackToLogsOnError @@ -209,10 +230,14 @@ spec: - "/host/opt/cni/bin" - "-t" - "thick" + # Short-lived, but it still needs a limit for the pod cgroup to get + # memory.max — the init container counts. resources: requests: cpu: "10m" memory: "15Mi" + limits: + memory: "128Mi" securityContext: privileged: true terminationMessagePolicy: FallbackToLogsOnError @@ -264,10 +289,15 @@ spec: echo "failed to install:${failed}" > /dev/termination-log 2>/dev/null || true fi exit 0 + # Short-lived, but it still needs a limit for the pod cgroup to get + # memory.max — the init container counts. Same 128Mi as the sibling + # install-multus-binary above: both only copy binaries to the host. resources: requests: cpu: "10m" memory: "15Mi" + limits: + memory: "128Mi" # cilium's install-cni-binaries copies plugin binaries into this same # directory with ALL capabilities dropped, in the spc_t domain, with # no privileged flag. On a node with SELinux enforcing, a container diff --git a/packages/system/multus/tests/multus_test.yaml b/packages/system/multus/tests/multus_test.yaml index 04fd5e76c..2855daadc 100644 --- a/packages/system/multus/tests/multus_test.yaml +++ b/packages/system/multus/tests/multus_test.yaml @@ -16,6 +16,33 @@ tests: path: spec.template.spec.containers[0].image pattern: '^ghcr\.io/cozystack/cozystack/multus-cni:' + # multus runs on every node and had a CPU limit but no memory one, which + # leaves the pod cgroup without memory.max. The Talos userspace OOM handler + # (v1.12+) only ever selects cgroups that lack it, and picks its victim by QoS + # class rather than by what caused the pressure, so multus was an eviction + # candidate on every node however healthy it was itself. + # + # memory.max lands on the POD cgroup only when EVERY container carries a + # limit. install-cni-plugins is asserted further down, where the rest of that + # container's contract lives; the two below complete the set for a default + # render. Keep all three: dropping any one silently returns the whole pod to + # the victim set while the other assertions still pass. + - it: kube-multus and its binary installer declare memory limits + template: templates/multus-daemonset-thick.yml + documentSelector: + path: kind + value: DaemonSet + asserts: + - equal: + path: spec.template.spec.containers[?(@.name=="kube-multus")].resources.limits.memory + value: 1Gi + - equal: + path: spec.template.spec.containers[?(@.name=="kube-multus")].resources.requests.memory + value: 100Mi + - equal: + path: spec.template.spec.initContainers[?(@.name=="install-multus-binary")].resources.limits.memory + value: 128Mi + # Guards the cozystack CNI-ordering fix (multusMasterCNI pinned to the cilium # conflist) so multus does not auto-detect kube-ovn ahead of cilium at node # startup and bypass the cilium CNI chain. This is a hand-edit that lives in @@ -263,11 +290,23 @@ tests: - equal: path: spec.template.spec.initContainers[1].resources.requests.memory value: 15Mi - # And no limits: a memory limit low enough to kill the copy turns staging - # into a crashloop, and there is nothing here whose runtime is worth - # capping -- it copies fourteen files and exits. - - notExists: - path: spec.template.spec.initContainers[1].resources.limits + # And a memory limit, which this assertion used to deny. The reasoning it + # carried -- nothing here is worth capping, it copies fourteen files and + # exits -- is about capping runtime, and that is not what the limit is + # for. A limit is what puts memory.max on the POD cgroup, and the Talos + # userspace OOM handler (v1.12+) only ever selects cgroups that lack it. + # The value is per-pod, not per-container: one limit-free container here + # leaves the whole multus pod in the victim set, and the system bundle + # defaults stageCniPlugins to true, so that is the default deployment. + # + # The original worry stands on its own terms and is answered by the size, + # not by omitting the limit: 128Mi is what would have to be low enough to + # kill a `cp` of fourteen small binaries. It matches install-multus-binary + # above, which does the same work, and page cache charged to the cgroup is + # reclaimable under pressure rather than a cause of OOM. + - equal: + path: spec.template.spec.initContainers[1].resources.limits.memory + value: 128Mi # Writing into the hostPath needs uid 0 IN THE CONTAINER. Nothing sets a # user today, so it runs as root; a runAsUser or runAsNonRoot at either # level turns every install into a permission error, and the script then diff --git a/packages/system/velero/tests/velero_test.yaml b/packages/system/velero/tests/velero_test.yaml index 44177de01..135c2f7e6 100644 --- a/packages/system/velero/tests/velero_test.yaml +++ b/packages/system/velero/tests/velero_test.yaml @@ -105,3 +105,27 @@ tests: path: spec.template.spec.containers[0].livenessProbe - notExists: path: spec.template.spec.containers[0].readinessProbe + + # node-agent is a DaemonSet and ran BestEffort. A memory limit is what sets + # memory.max on the pod cgroup, and the Talos userspace OOM handler (v1.12+) + # only ever selects cgroups that lack it — so node-agent was an eviction + # candidate on every node whatever caused the pressure, and killing it + # mid-backup fails the backup. + # + # The limit is deliberately loose rather than fitted: measured usage is at + # idle, and the agent streams volume data during a backup, which is exactly + # when it must not be killed. Set from the umbrella values against the + # vendored subchart, so an upstream bump that moves the key would render + # nothing and leave no other trace; that is what this pins against. + - it: node-agent declares a memory limit and request + template: charts/velero/templates/node-agent-daemonset.yaml + documentSelector: + path: kind + value: DaemonSet + asserts: + - equal: + path: spec.template.spec.containers[?(@.name=="node-agent")].resources.limits.memory + value: 2Gi + - equal: + path: spec.template.spec.containers[?(@.name=="node-agent")].resources.requests.memory + value: 128Mi diff --git a/packages/system/velero/values.yaml b/packages/system/velero/values.yaml index a2c54e615..c3fae2421 100644 --- a/packages/system/velero/values.yaml +++ b/packages/system/velero/values.yaml @@ -19,6 +19,20 @@ velero: name: plugins deployNodeAgent: true + nodeAgent: + # node-agent is a DaemonSet and ran BestEffort. A memory limit is what sets + # memory.max on the pod cgroup, and the Talos userspace OOM handler (v1.12+) + # only ever kills cgroups that lack one — so it was an eviction candidate on + # every node, independent of what caused the pressure. Killing it mid-backup + # fails the backup, so the limit is deliberately loose: 14d peak is 121Mi at + # idle on a dev cluster, but the agent streams volume data during a backup + # and that is exactly when it must not be killed. + resources: + requests: + cpu: 50m + memory: 128Mi + limits: + memory: 2Gi configuration: # defaultVolumesToFsBackup: true # cozy-default BackupStorageLocation lives in the