mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-13 08:05:19 +09:00
Aleksei Sviridkin
3219f7ee9a
## What this PR does The barman-cloud plugin injects its sidecar with no resource requests or limits. What that means depends on the namespace. A tenant with `resourceQuotas` set ships a `LimitRange` that defaults containers to 128Mi (`packages/apps/tenant/templates/quota.yaml`), so there the sidecar inherits that default and is OOMKilled mid-backup. A namespace without a `LimitRange`, which is `cozy-keycloak` and any tenant that leaves `resourceQuotas` empty, gave the sidecar no requests and no limit at all, so nothing reserved memory for it and nothing bounded it. The failure on the tenant path is quiet from the control plane: the `ObjectStore` stays healthy, the `Cluster` stays `Ready`, and only the backup fails. Observed on a 1.6 cluster while backing up a 38 MB database: `lastState.terminated.reason: OOMKilled`, `exit 137`, four restarts on the sidecar, and a cgroup high-water mark of 254 MiB. `container_memory_working_set_bytes` peaked at only 64 MiB over the same window — it is sampled, and it misses the spike that actually triggers the kill, which is why the working-set number does not explain the failure on its own. Both paths that build an ObjectStore now carry resources: - `cozy-lib.barman.sidecarConfiguration`, used by `packages/apps/postgres` for the backup and recovery ObjectStores and by `packages/system/keycloak`; - `barmanSidecarConfiguration()` in `internal/backupcontroller/cnpgstrategy_controller.go`, which builds the platform's own ObjectStore on the `useSystemBucket=true` path. Requests are 100m/256Mi with a 1Gi memory limit on every caller: 256Mi holds the measured working set and 1Gi is four times the measurement, which is also the ceiling a caller without a LimitRange gets where it had none. No CPU limit is set, following the `entityOperator` precedent in `packages/apps/kafka/templates/kafka.yaml`: a throttled sidecar stalls WAL archiving instead of failing it, which is harder to notice than an outright failure. Measured CPU peak was 50 mCPU. Inside a tenant the limit is charged against the `ResourceQuota` `limits.memory` budget, 1Gi per instance pod; a 4Gi tenant spends a quarter of it per Postgres instance. The helper is renamed from `checksumSidecarConfiguration` to `sidecarConfiguration`, because it no longer carries only the checksum pin and its name and doc comment would otherwise be wrong. Tests cover both paths and both render sites: a new helm suite in `packages/apps/postgres`, extra assertions in the existing keycloak suite, an assertion in the controller test, and a deepcopy test for the new field. Each of them fails against the unfixed sources. One thing this PR deliberately does not do. This is the fifth time the 128Mi tenant default has been worked around per component — kafka and zookeeper presets (#2537), the kafka entity-operator (#2934), the cert-manager cainjector (#3199), and the note carried in the etcd-operator values. Whether the default itself should move looks like your call rather than something to fold into a fix for one sidecar, so it is left alone here. ### Screenshots Not a UI change. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff, entry by entry. The change touches one cozy-lib helper, the two chart templates that include it, one Go function, and tests. It adds, renames or removes no package; changes no `values.yaml`, `values.schema.json`, `Chart.yaml` or `README.md`, so no version enum, default or generated artifact moves; changes no `ApplicationDefinition` semantics, no `release.prefix`, no output Secret or Service name; touches no CRD, no namespace, no `hack/` file, no telemetry metric or label, no `cozy-proxy` annotation, no node or network requirement, and no commit or PR convention. No entry matches. - [x] No downstream repository is affected by this change ### Release note ```release-note fix(backups): give the barman-cloud sidecar its own resources The barman-cloud sidecar was injected without resource requests or limits, so in tenant namespaces it inherited the 128Mi `LimitRange` default and was OOMKilled during backups while the `ObjectStore` and the `Cluster` both stayed healthy. It now requests 100m/256Mi and caps memory at 1Gi on the chart path and on the platform's Go path alike; in namespaces without a LimitRange the sidecar had no limit before and gets the same 1Gi ceiling. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added explicit CPU and memory resource settings for barman-cloud backup and recovery sidecars. - Backup sidecars request 100m CPU and 256Mi memory, with a 1Gi memory limit. - Recovery sidecars receive a 1Gi memory limit. - Preserved the S3 checksum configuration. - **Bug Fixes** - Prevented sidecars from inheriting unsuitable tenant resource limits. - Ensured resource settings are preserved when backup configuration is copied. <!-- end of auto-generated comment: release notes by coderabbit.ai -->