Skip to content

Commit 52c5927

Browse files
myasnikovdaniillexfrei
myasnikovdaniil
authored and
lexfrei
committed
fix(velero): exempt the node-agent from a memory limit
The 2Gi ceiling was sized from the wrong number, as the comment beside it admitted: 121Mi is the 14-day peak at idle, and node-agent only consumes real memory while it is moving data, when kopia/restic walk, hash and index the volume's file tree. Demand there scales with file count and volume size on that node rather than with cluster size, so no measurement a 3-node dev cluster can produce bounds the case the limit has to cover. A guessed ceiling is worse than none on this workload. The kill it prevents happens only when the node is genuinely short of memory; the kill it introduces happens on a healthy node in the middle of a backup, the one moment the agent has to survive, and it lands on clusters whose large backups completed fine while node-agent was unlimited. Raising 2Gi to a larger guess moves the threshold without removing the failure mode or acquiring the evidence. So node-agent now carries requests and no limit, the same call linstor-controller and the satellite make. The ceiling is not dropped, it moves: the default LimitRange the operator maintains in every system namespace defaults container memory in cozy-velero, so the pod still gets memory.max and the number is tunable once, centrally, on a cluster whose backups need more. Before that LimitRange exists node-agent is limit-free and stays in the OOM handler's victim set, which is what it does today. Settling a fitted limit instead needs a peak measured under a PodVolumeBackup of a large, many-file volume on a real cluster. velero_test.yaml pins the exemption with notExists so a ceiling cannot come back quietly, and pins the cpu request that carried no assertion. Assisted-by: LLM Signed-off-by: Myasnikov Daniil <[email protected]>
1 parent 9b6ed66 commit 52c5927

2 files changed

Lines changed: 37 additions & 18 deletions

File tree

‎packages/system/velero/tests/velero_test.yaml‎

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -124,20 +124,41 @@ tests:
124124
# candidate on every node whatever caused the pressure, and killing it
125125
# mid-backup fails the backup.
126126
#
127-
# The limit is deliberately loose rather than fitted: measured usage is at
128-
# idle, and the agent streams volume data during a backup, which is exactly
129-
# when it must not be killed. Set from the umbrella values against the
130-
# vendored subchart, so an upstream bump that moves the key would render
131-
# nothing and leave no other trace; that is what this pins against.
132-
- it: node-agent declares a memory limit and request
127+
# It is nonetheless exempted from a limit, the same call linstor-controller and
128+
# the satellite make, because its memory is driven by the data it moves rather
129+
# than by cluster size: during a PodVolumeBackup kopia/restic walk, hash and
130+
# index the volume's file tree, so demand scales with file count and volume
131+
# size on that node. The 121Mi 14d peak is an idle figure and nothing measured
132+
# on a 3-node dev cluster bounds the backup case, so a ceiling written here
133+
# would be a guess — and a guess on this workload trades a kill that happens
134+
# only when the node is genuinely short of memory for one that happens on a
135+
# healthy node mid-backup, the single moment the agent has to survive.
136+
#
137+
# The ceiling is not dropped, it moves: the default LimitRange the operator
138+
# maintains in every system namespace defaults container memory in cozy-velero,
139+
# so the pod still gets memory.max and the value is tunable once, centrally,
140+
# on the cluster whose backups need more. Before that LimitRange exists
141+
# node-agent is limit-free and stays in the victim set — which is what it does
142+
# today, so no regression, just not yet the fix.
143+
#
144+
# What would settle a fitted limit instead: a peak measured under a
145+
# PodVolumeBackup of a large, many-file volume on a real cluster.
146+
#
147+
# Set from the umbrella values against the vendored subchart, so an upstream
148+
# bump that moves the key would render nothing and leave no other trace; that
149+
# is what the request assertions pin against, and notExists pins the exemption
150+
# so a ceiling cannot come back quietly.
151+
- it: node-agent declares a memory request and deliberately no limit
133152
template: charts/velero/templates/node-agent-daemonset.yaml
134153
documentSelector:
135154
path: kind
136155
value: DaemonSet
137156
asserts:
138-
- equal:
139-
path: spec.template.spec.containers[?(@.name=="node-agent")].resources.limits.memory
140-
value: 2Gi
141157
- equal:
142158
path: spec.template.spec.containers[?(@.name=="node-agent")].resources.requests.memory
143159
value: 128Mi
160+
- equal:
161+
path: spec.template.spec.containers[?(@.name=="node-agent")].resources.requests.cpu
162+
value: 50m
163+
- notExists:
164+
path: spec.template.spec.containers[?(@.name=="node-agent")].resources.limits

‎packages/system/velero/values.yaml‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,19 +20,17 @@ velero:
2020

2121
deployNodeAgent: true
2222
nodeAgent:
23-
# node-agent is a DaemonSet and ran BestEffort. A memory limit is what sets
24-
# memory.max on the pod cgroup, and the Talos userspace OOM handler (v1.12+)
25-
# only ever kills cgroups that lack one — so it was an eviction candidate on
26-
# every node, independent of what caused the pressure. Killing it mid-backup
27-
# fails the backup, so the limit is deliberately loose: 14d peak is 121Mi at
28-
# idle on a dev cluster, but the agent streams volume data during a backup
29-
# and that is exactly when it must not be killed.
23+
# Requests only, deliberately no limit — the same call linstor-controller and
24+
# the satellite make. node-agent's memory is driven by the data it moves, not
25+
# by cluster size, so there is no dev-cluster figure to size a ceiling from:
26+
# the 121Mi 14d peak is idle. The ceiling comes from the default LimitRange
27+
# the operator maintains in every system namespace instead, which is one
28+
# operator-facing value rather than a guess per package.
29+
# Full reasoning, and the assertion that pins it: tests/velero_test.yaml.
3030
resources:
3131
requests:
3232
cpu: 50m
3333
memory: 128Mi
34-
limits:
35-
memory: 2Gi
3634
configuration:
3735
# defaultVolumesToFsBackup: true
3836
# cozy-default BackupStorageLocation lives in the

0 commit comments

Comments
 (0)