The Keycloak comment and the helper doc claimed the tenant LimitRange as the
reason for the resources, but cozy-keycloak has no LimitRange and neither
does a tenant that leaves resourceQuotas empty: there the sidecar had no
requests and no limit at all, and the change gives it a 1Gi ceiling where
there was none. The comments and both doc blocks now say what applies to
each caller. No rendered object changes.
Assisted-by: LLM
Signed-off-by: Yan Bondarenko <202671653+yankawai@users.noreply.github.com>
(cherry picked from commit 4dc6065ec9)
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 <202671653+yankawai@users.noreply.github.com>
(cherry picked from commit 9214430bd8)
An in-place RestoreJob (targetApplicationRef omitted) re-bootstraps the
Postgres app under its own name. On stock v1.6.2 it never comes back up,
for two independent reasons in the same restore patch - both surface only
in place, because a to-copy restore targets a different app.
1. Archive-vs-recovery serverName collision. The driver set the restored
cluster's WAL-archive serverName equal to the recovery source
(bootstrap.serverName = bootstrap.oldName = sourceServerName) while the
chart pinned the archiver serverName to the release name. In place those
coincide, so the cluster archived onto the source's own WAL prefix and
barman-cloud-check-wal-archive refused to start ("Expected empty
archive"). Fixed by giving the restored cluster a fresh
bootstrap.newServerName, keyed by the RestoreJob UID (unique per
restore, stable across reconciles); the chart archives there while
externalClusters still recovers from bootstrap.serverName. This is what
the manual CNPG strategies hit (serverName defaults to the cluster
name); the cozy-default flow dodged it (serverName is <ns>-<app>).
2. useSystemBucket guard. The restore populates explicit S3 coordinates
(destinationPath / s3CredentialsSecret / endpointCA) but left
backup.useSystemBucket=true, so the chart's "bootstrap.enabled is
incompatible with useSystemBucket" guard failed the HelmRelease upgrade
and the cluster never re-rendered. This is what the cozy-default
(system-bucket) flow hits on an in-place restore. Fixed by clearing
useSystemBucket in the restore patch, since the app is now on the
explicit-creds path. The driver's app view (postgresapp.Backup) gains
the field, without omitempty so a false survives the merge patch.
Both are exercised only by an in-place restore; the postgres backup e2e
drove only to-copy, so neither surfaced in CI. A step 50 in-place restore
now guards reason 1 (manual-strategy flow); reason 2 is covered by a Go
unit assertion (a system-bucket in-place e2e needs the platform bucket in
CI and is left as a follow-up).
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
(cherry picked from commit 5bcdb8cfb8)
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Assisted-By: LLM
Address review on the S3 request-checksum fix:
- Extend the fix to the platform-managed useSystemBucket=true flow, whose
ObjectStore is built in Go by applyClusterPluginBackup (SSA-applied), not
chart-rendered. Add InstanceSidecarConfiguration to cnpgtypes.ObjectStoreSpec
and set AWS_REQUEST_CHECKSUM_CALCULATION=when_required on it via
barmanSidecarConfiguration(). The platform's own default system bucket is
SeaweedFS, a non-AWS S3 gateway of the same class as Ceph RGW. Unit test in
cnpgstrategy_controller_test.go asserts the applied ObjectStore carries it.
- Factor the duplicated instanceSidecarConfiguration block (3x across two
charts) into a shared cozy-lib helper (cozy-lib.barman.checksumSidecarConfiguration).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io>
(cherry picked from commit b574e2176f)
Two defects of the same class — the WAL-archive S3 prefix (serverName)
changing or defaulting differently between components:
Recovery default regression (review blocker): native CNPG defaulted an
absent externalClusters serverName to the entry name (= bootstrap.oldName),
but the barman-cloud plugin defaults it to the NEW cluster's name, so a
legacy-flow restore with bootstrap.serverName unset would read the wrong
S3 prefix (hard failure, or silently restoring another cluster's data).
The chart now always emits serverName, defaulting to bootstrap.oldName.
Live-cluster serverName flip (found by the gated postgres backup e2e on a
real cluster): the chart renders spec.plugins without serverName
(effective prefix = cluster name) while a strategy template may name a
different one; the driver's ForceOwnership patch then flipped the prefix
mid-stream, splitting the WAL archive across two prefixes — the WAL
holding the backup's start was lost between them and the restore failed
with "WAL not found". The driver now preserves the live Cluster's
effective serverName (explicit parameter, or the cluster name when the
plugin is attached without one) and only applies the strategy's value to
a Cluster with no plugin attached; both charts pin serverName explicitly,
and the example strategy no longer sets one.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Review follow-ups (cozystack#3300):
- Lifecycle/leak: applyClusterPluginBackup now sets an ownerReference from the
driver-created barmancloud.cnpg.io/ObjectStore to its cnpg.io/Cluster, so
Kubernetes garbage-collects it when the Cluster is deleted (tenant app
teardown or the RestoreJob purge). In the platform useSystemBucket flow the
chart does not render this ObjectStore, so previously it was orphaned in the
tenant namespace. Drop the now-accurate-only 'delete' verb on objectstores
from the driver RBAC (GC handles removal; the driver never deletes it) and
correct the RBAC comment that falsely claimed the purge deletes it.
- Diagnostics/comments: the RestoreJob WAL-archive-deadline failure message and
several doc comments still told operators to check spec.backup.barmanObjectStore
/ externalClusters[].barmanObjectStore, fields no longer emitted after the
migration. Point them at the barman-cloud plugin (spec.plugins + ObjectStore,
externalClusters[].plugin) instead so they don't mislead during an incident.
- Test: TestApplyClusterPluginBackup_PatchesExistingCluster now asserts the
ObjectStore carries an ownerReference to the Cluster (kind/name/uid).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Address review feedback on the repeat in-place restore fix:
- Make the freshness discriminator strict (creationTimestamp > StartedAt) so
an exact tie resolves to "not fresh" and the caller purges, matching the
function's stated conservative default; a fresh Cluster is always created
well after StartedAt so this never misclassifies the status-write-race path.
- Document why a wall-clock timestamp comparison was chosen over recording the
purged Cluster's UID on the TargetPurged condition (the UID approach leans on
the same status write the race path assumes can fail).
- Add a reconcile-level regression test that drives reconcileCNPGRestore end to
end with a fake client: a stale leftover recovery Cluster from a prior restore
is purged, and a Cluster this restore just re-created is not re-purged. This
covers the actual call site where #3311 lived, not just the pure helpers.
Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com>
A repeat in-place PostgreSQL RestoreJob into an already-restored target
silently no-opped: the destructive purge was skipped because the live
Cluster still carried spec.bootstrap.recovery left over from the previous
restore, and the re-purge guard mistook that for a freshly-recovered
Cluster produced by this restore's own purge. The job then reported
Succeeded while the PVC, disk, and data were never touched.
Tell the two apart by the Cluster's creationTimestamp relative to the
RestoreJob's StartedAt: only a recovery Cluster created at/after StartedAt
is treated as this restore's own and skipped; one that predates StartedAt
is leftover from an earlier completed restore and is purged so the restore
re-bootstraps from the backup.
Signed-off-by: Ivan Okhotnikov <xorokhotnikov@gmail.com>
Yan Bondarenko