Skip to content

Commit 9214430

Browse files
yankawailexfrei
yankawai
authored and
lexfrei
committed
fix(backups): give the barman-cloud sidecar its own resources
The barman-cloud plugin injects its sidecar without resource requests or limits. Tenant namespaces ship a LimitRange that defaults containers to 128Mi, so the sidecar inherits that default and is OOMKilled mid-backup: the ObjectStore stays healthy, the Cluster stays Ready, and only the backup fails. A cgroup high-water mark of 254 MiB on a 38 MB database shows the default cannot hold it. The shared cozy-lib helper and the Go path that builds the platform's own ObjectStore now request 100m/256Mi and cap memory at 1Gi. No CPU limit is set, matching the entityOperator precedent in the kafka chart: a throttled sidecar stalls WAL archiving instead of failing it. The helper is renamed from checksumSidecarConfiguration to sidecarConfiguration, since it no longer carries only the checksum pin. Signed-off-by: Yan Bondarenko <[email protected]>
1 parent 88a7ea0 commit 9214430

10 files changed

Lines changed: 203 additions & 34 deletions

File tree

‎internal/backupcontroller/cnpgstrategy_controller.go‎

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
corev1 "k8s.io/api/core/v1"
1616
apierrors "k8s.io/apimachinery/pkg/api/errors"
1717
apimeta "k8s.io/apimachinery/pkg/api/meta"
18+
"k8s.io/apimachinery/pkg/api/resource"
1819
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1920
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2021
"k8s.io/apimachinery/pkg/runtime"
@@ -1664,17 +1665,39 @@ func buildBarmanPlugin(objectStoreName, serverName string) cnpgtypes.PluginConfi
16641665
}
16651666
}
16661667

1667-
// barmanSidecarConfiguration pins the barman-cloud sidecar's boto3 request
1668-
// checksum policy to "when_required". Since botocore ~1.36 the default
1669-
// (when_supported) attaches a flexible checksum to every PutObject, which
1670-
// non-AWS S3-compatible backends (Ceph RGW, and the platform's own default
1671-
// SeaweedFS system bucket) reject with "x-amz-content-sha256 must be
1672-
// UNSIGNED-PAYLOAD, ...". Compute a checksum only when required; AWS S3
1673-
// accepts that too, so it is a safe default everywhere. This mirrors the same
1674-
// env set on the chart-rendered ObjectStores (packages/{apps/postgres,
1675-
// system/keycloak}/templates/db.yaml) and the etcd-operator fix (#342).
1668+
// barmanSidecarConfiguration carries the two settings the barman-cloud sidecar
1669+
// cannot get right on its own.
1670+
//
1671+
// Resources: tenant namespaces ship a LimitRange defaulting containers to 128Mi
1672+
// (packages/apps/tenant/templates/quota.yaml). The plugin injects the sidecar
1673+
// without resources, so it inherits that default and is OOMKilled mid-backup,
1674+
// leaving the ObjectStore healthy and the backup failed. Measured cgroup
1675+
// high-water mark on a 38 MB database is 254 MiB, so 128Mi cannot hold it. No
1676+
// CPU limit is set, matching the entityOperator precedent in
1677+
// packages/apps/kafka: a throttled sidecar stalls WAL archiving instead of
1678+
// failing it.
1679+
//
1680+
// Checksum: since botocore ~1.36 the default (when_supported) attaches a
1681+
// flexible checksum to every PutObject, which non-AWS S3-compatible backends
1682+
// (Ceph RGW, and the platform's own default SeaweedFS system bucket) reject
1683+
// with "x-amz-content-sha256 must be UNSIGNED-PAYLOAD, ...". Compute a checksum
1684+
// only when required; AWS S3 accepts that too, so it is a safe default
1685+
// everywhere.
1686+
//
1687+
// This mirrors the chart-rendered ObjectStores, which take both fields from
1688+
// cozy-lib.barman.sidecarConfiguration (packages/{apps/postgres,
1689+
// system/keycloak}/templates/db.yaml), and the etcd-operator fix (#342).
16761690
func barmanSidecarConfiguration() *cnpgtypes.InstanceSidecarConfiguration {
16771691
return &cnpgtypes.InstanceSidecarConfiguration{
1692+
Resources: corev1.ResourceRequirements{
1693+
Requests: corev1.ResourceList{
1694+
corev1.ResourceCPU: resource.MustParse("100m"),
1695+
corev1.ResourceMemory: resource.MustParse("256Mi"),
1696+
},
1697+
Limits: corev1.ResourceList{
1698+
corev1.ResourceMemory: resource.MustParse("1Gi"),
1699+
},
1700+
},
16781701
Env: []cnpgtypes.EnvVar{{
16791702
Name: "AWS_REQUEST_CHECKSUM_CALCULATION",
16801703
Value: "when_required",

‎internal/backupcontroller/cnpgstrategy_controller_test.go‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1759,10 +1759,21 @@ func TestApplyClusterPluginBackup_PatchesExistingCluster(t *testing.T) {
17591759
// platform-managed useSystemBucket=true flow, so this Go path is the only
17601760
// place that sets it there.
17611761
sc := store.Spec.InstanceSidecarConfiguration
1762-
if sc == nil || len(sc.Env) != 1 ||
1762+
if sc == nil {
1763+
t.Fatal("ObjectStore has no instanceSidecarConfiguration")
1764+
}
1765+
if len(sc.Env) != 1 ||
17631766
sc.Env[0].Name != "AWS_REQUEST_CHECKSUM_CALCULATION" || sc.Env[0].Value != "when_required" {
17641767
t.Errorf("ObjectStore instanceSidecarConfiguration.env: got %+v, want [AWS_REQUEST_CHECKSUM_CALCULATION=when_required]", sc)
17651768
}
1769+
if sc.Resources.Requests.Memory().Value() != 256*1024*1024 ||
1770+
sc.Resources.Limits.Memory().Value() != 1024*1024*1024 ||
1771+
sc.Resources.Requests.Cpu().MilliValue() != 100 {
1772+
t.Errorf("ObjectStore leaves Barman subject to undersized tenant resource defaults: %+v", sc.Resources)
1773+
}
1774+
if _, ok := sc.Resources.Limits[corev1.ResourceCPU]; ok {
1775+
t.Errorf("ObjectStore caps sidecar CPU, which throttles WAL archiving instead of failing it: %+v", sc.Resources.Limits)
1776+
}
17661777
// The ObjectStore must be owner-referenced to the Cluster so Kubernetes GC
17671778
// removes it when the Cluster is deleted (no orphan in the platform flow,
17681779
// where the chart does not render this ObjectStore).

‎internal/backupcontroller/cnpgtypes/deepcopy_test.go‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
3+
package cnpgtypes
4+
5+
import (
6+
"testing"
7+
8+
corev1 "k8s.io/api/core/v1"
9+
"k8s.io/apimachinery/pkg/api/resource"
10+
)
11+
12+
func TestObjectStoreDeepCopySidecarResources(t *testing.T) {
13+
original := &ObjectStore{Spec: ObjectStoreSpec{
14+
InstanceSidecarConfiguration: &InstanceSidecarConfiguration{
15+
Resources: corev1.ResourceRequirements{
16+
Requests: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("256Mi")},
17+
Limits: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1Gi")},
18+
},
19+
Env: []EnvVar{{Name: "AWS_REQUEST_CHECKSUM_CALCULATION", Value: "when_required"}},
20+
},
21+
}}
22+
copy := original.DeepCopy()
23+
copy.Spec.InstanceSidecarConfiguration.Resources.Requests[corev1.ResourceMemory] = resource.MustParse("512Mi")
24+
copy.Spec.InstanceSidecarConfiguration.Resources.Limits[corev1.ResourceMemory] = resource.MustParse("2Gi")
25+
copy.Spec.InstanceSidecarConfiguration.Env[0].Value = "changed"
26+
got := original.Spec.InstanceSidecarConfiguration
27+
if got.Resources.Requests.Memory().String() != "256Mi" || got.Resources.Limits.Memory().String() != "1Gi" || got.Env[0].Value != "when_required" {
28+
t.Fatalf("mutating the copy changed the original sidecar configuration: %+v", got)
29+
}
30+
}

‎internal/backupcontroller/cnpgtypes/types.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
package cnpgtypes
1212

1313
import (
14+
corev1 "k8s.io/api/core/v1"
1415
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1516
"k8s.io/apimachinery/pkg/runtime"
1617
"k8s.io/apimachinery/pkg/runtime/schema"
@@ -215,9 +216,10 @@ type ObjectStoreSpec struct {
215216
}
216217

217218
// InstanceSidecarConfiguration is a minimal mirror of the barman-cloud
218-
// ObjectStore's spec.instanceSidecarConfiguration (only spec.env).
219+
// ObjectStore's spec.instanceSidecarConfiguration.
219220
type InstanceSidecarConfiguration struct {
220-
Env []EnvVar `json:"env,omitempty"`
221+
Env []EnvVar `json:"env,omitempty"`
222+
Resources corev1.ResourceRequirements `json:"resources,omitempty"`
221223
}
222224

223225
// EnvVar is a minimal corev1.EnvVar (name/value only) — enough to pass a

‎internal/backupcontroller/cnpgtypes/zz_generated.deepcopy.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,7 @@ func (in *ObjectStoreSpec) DeepCopyInto(out *ObjectStoreSpec) {
247247

248248
func (in *InstanceSidecarConfiguration) DeepCopyInto(out *InstanceSidecarConfiguration) {
249249
*out = *in
250+
in.Resources.DeepCopyInto(&out.Resources)
250251
if in.Env != nil {
251252
out.Env = make([]EnvVar, len(in.Env))
252253
copy(out.Env, in.Env)

‎packages/apps/postgres/templates/db.yaml‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -329,9 +329,9 @@ spec:
329329
{{- if .Values.backup.retentionPolicy }}
330330
retentionPolicy: {{ .Values.backup.retentionPolicy | quote }}
331331
{{- end }}
332-
# Pin the barman-cloud sidecar's S3 request checksum to when_required
333-
# (rationale in cozy-lib.barman.checksumSidecarConfiguration).
334-
{{- include "cozy-lib.barman.checksumSidecarConfiguration" . | nindent 2 }}
332+
# Size the barman-cloud sidecar off the tenant LimitRange default and pin its
333+
# S3 request checksum (rationale in cozy-lib.barman.sidecarConfiguration).
334+
{{- include "cozy-lib.barman.sidecarConfiguration" . | nindent 2 }}
335335
configuration:
336336
destinationPath: {{ $destinationPath }}
337337
endpointURL: {{ $endpointURL }}
@@ -358,8 +358,8 @@ kind: ObjectStore
358358
metadata:
359359
name: {{ .Release.Name }}-recovery
360360
spec:
361-
# Same S3 request-checksum pin as the backup ObjectStore above.
362-
{{- include "cozy-lib.barman.checksumSidecarConfiguration" . | nindent 2 }}
361+
# Same sidecar configuration as the backup ObjectStore above.
362+
{{- include "cozy-lib.barman.sidecarConfiguration" . | nindent 2 }}
363363
configuration:
364364
destinationPath: {{ $destinationPath }}
365365
endpointURL: {{ $endpointURL }}

‎packages/apps/postgres/tests/barman_resources_test.yaml‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
suite: Barman resources under tenant container limits
2+
templates:
3+
- templates/db.yaml
4+
release:
5+
name: postgres-source
6+
namespace: tenant-test
7+
set:
8+
_cluster:
9+
scheduling: {}
10+
backup:
11+
destinationPath: s3://backups/test/
12+
endpointURL: https://s3.example.org
13+
s3CredentialsSecret:
14+
name: postgres-backup
15+
tests:
16+
- it: sets resources for native backup without inheriting the tenant 128Mi limit
17+
set:
18+
backup.enabled: true
19+
asserts:
20+
- equal:
21+
path: spec.instanceSidecarConfiguration.resources
22+
value:
23+
requests:
24+
cpu: 100m
25+
memory: 256Mi
26+
limits:
27+
memory: 1Gi
28+
documentSelector:
29+
path: kind
30+
value: ObjectStore
31+
- notExists:
32+
path: spec.instanceSidecarConfiguration.resources.limits.cpu
33+
documentSelector:
34+
path: kind
35+
value: ObjectStore
36+
- equal:
37+
path: spec.configuration.s3Credentials.accessKeyId.name
38+
value: postgres-backup
39+
documentSelector:
40+
path: kind
41+
value: ObjectStore
42+
- it: gives recovery its own resources while backup remains disabled
43+
set:
44+
backup.enabled: false
45+
bootstrap:
46+
enabled: true
47+
oldName: postgres-original
48+
asserts:
49+
- equal:
50+
path: spec.instanceSidecarConfiguration.resources.limits.memory
51+
value: 1Gi
52+
documentSelector:
53+
path: kind
54+
value: ObjectStore
55+
- equal:
56+
path: metadata.name
57+
value: postgres-source-recovery
58+
documentSelector:
59+
path: kind
60+
value: ObjectStore
61+
- equal:
62+
path: spec.externalClusters[0].plugin.parameters.serverName
63+
value: postgres-original
64+
documentSelector:
65+
path: kind
66+
value: Cluster
67+
- it: does not enable backup for an application that has not requested it
68+
asserts:
69+
- hasDocuments:
70+
count: 2
71+
- notExists:
72+
path: spec.plugins
73+
documentSelector:
74+
path: kind
75+
value: Cluster

‎packages/library/cozy-lib/templates/_barman.tpl‎

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,38 @@
11
{{- /*
2-
cozy-lib.barman.checksumSidecarConfiguration renders a barman-cloud ObjectStore
3-
`spec.instanceSidecarConfiguration` that pins the sidecar's boto3 request-checksum
4-
policy to when_required.
2+
cozy-lib.barman.sidecarConfiguration renders a barman-cloud ObjectStore
3+
`spec.instanceSidecarConfiguration` shared by every chart that enables the
4+
plugin. It carries the two settings the sidecar cannot get right on its own.
55
6-
Since botocore ~1.36 (early 2025) the default RequestChecksumCalculation is
7-
when_supported, which attaches a flexible checksum (the x-amz-content-sha256
8-
header) to every PutObject. Non-AWS S3-compatible backends (Ceph RADOS Gateway,
9-
the platform's own SeaweedFS system bucket, some MinIO/Cloudflare R2 builds)
10-
reject it with "InvalidArgument: x-amz-content-sha256 must be UNSIGNED-PAYLOAD,
11-
...", so every backup/WAL-archive upload fails against them. when_required
12-
computes a checksum only when the operation mandates one; AWS S3 accepts that on
13-
a plain PutObject too, so it is a safe default everywhere.
6+
Resources: tenant namespaces ship a LimitRange defaulting containers to 128Mi
7+
(packages/apps/tenant/templates/quota.yaml). The plugin injects the sidecar
8+
without resources, so it inherits that default and is OOMKilled mid-backup,
9+
leaving the ObjectStore healthy and the backup failed. Measured cgroup
10+
high-water mark on a 38 MB database is 254 MiB, so 128Mi cannot hold it. No CPU
11+
limit is set, matching the entityOperator precedent in packages/apps/kafka: a
12+
throttled sidecar stalls WAL archiving instead of failing it.
1413
15-
Emit under an ObjectStore `spec:` with `{{- include "cozy-lib.barman.checksumSidecarConfiguration" . | nindent 2 }}`.
16-
The Go-driven platform (useSystemBucket=true) ObjectStore sets the same env in
14+
Checksum: since botocore ~1.36 (early 2025) the default
15+
RequestChecksumCalculation is when_supported, which attaches a flexible checksum
16+
(the x-amz-content-sha256 header) to every PutObject. Non-AWS S3-compatible
17+
backends (Ceph RADOS Gateway, the platform's own SeaweedFS system bucket, some
18+
MinIO/Cloudflare R2 builds) reject it with "InvalidArgument:
19+
x-amz-content-sha256 must be UNSIGNED-PAYLOAD, ...", so every backup/WAL-archive
20+
upload fails against them. when_required computes a checksum only when the
21+
operation mandates one; AWS S3 accepts that on a plain PutObject too, so it is a
22+
safe default everywhere.
23+
24+
Emit under an ObjectStore `spec:` with `{{- include "cozy-lib.barman.sidecarConfiguration" . | nindent 2 }}`.
25+
The Go-driven platform (useSystemBucket=true) ObjectStore sets the same fields in
1726
internal/backupcontroller/cnpgstrategy_controller.go (barmanSidecarConfiguration).
1827
*/ -}}
19-
{{- define "cozy-lib.barman.checksumSidecarConfiguration" -}}
28+
{{- define "cozy-lib.barman.sidecarConfiguration" -}}
2029
instanceSidecarConfiguration:
30+
resources:
31+
requests:
32+
cpu: 100m
33+
memory: 256Mi
34+
limits:
35+
memory: 1Gi
2136
env:
2237
- name: AWS_REQUEST_CHECKSUM_CALCULATION
2338
value: when_required

‎packages/system/keycloak/templates/db.yaml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,9 @@ spec:
5151
{{- if .Values.backup.retentionPolicy }}
5252
retentionPolicy: {{ .Values.backup.retentionPolicy | quote }}
5353
{{- end }}
54-
# Pin the barman-cloud sidecar's S3 request checksum to when_required
55-
# (rationale in cozy-lib.barman.checksumSidecarConfiguration).
56-
{{- include "cozy-lib.barman.checksumSidecarConfiguration" . | nindent 2 }}
54+
# Size the barman-cloud sidecar off the tenant LimitRange default and pin its
55+
# S3 request checksum (rationale in cozy-lib.barman.sidecarConfiguration).
56+
{{- include "cozy-lib.barman.sidecarConfiguration" . | nindent 2 }}
5757
configuration:
5858
destinationPath: {{ .Values.backup.destinationPath | quote }}
5959
{{- with .Values.backup.endpointURL }}

‎packages/system/keycloak/tests/db_backup_test.yaml‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,3 +38,15 @@ tests:
3838
documentSelector:
3939
path: kind
4040
value: ObjectStore
41+
- equal:
42+
path: spec.instanceSidecarConfiguration.resources.limits.memory
43+
value: 1Gi
44+
documentSelector:
45+
path: kind
46+
value: ObjectStore
47+
- equal:
48+
path: spec.instanceSidecarConfiguration.resources.requests.memory
49+
value: 256Mi
50+
documentSelector:
51+
path: kind
52+
value: ObjectStore

0 commit comments

Comments
 (0)