The guard's regression cases only exercised renames, so dropping the
IsRenamed conjunct (failing every non-VMInstance restore) or keying the
check on targetApplicationRef.kind instead of the backup's kind left the
package green. Add the inputs the guard must let through, a VMDisk copy
that keeps its name and a VMDisk in-place restore without
targetApplicationRef, and a VMDisk backup restored with a VMInstance
target kind, which must still be rejected by the source kind.
Assisted-By: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
A RestoreJob that set targetApplicationRef.name to a new name and
options.targetNamespace to another namespace reported Succeeded for any
Velero-backed application, but only a VMInstance was actually renamed.
postRestoreRename looks up the HelmRelease under the vm-instance- prefix,
gets NotFound for every other kind and returns nil, so a Postgres,
MariaDB, ClickHouse, MongoDB or standalone VMDisk copy came up under the
source name with no error surfaced.
Renaming those kinds through the Velero strategy cannot work: DataUpload
restores volumes under their original names, and only a VMInstance keeps
using them after its HelmRelease is renamed, because its disks keep their
names and CDI adopts the restored PVCs. A renamed database Cluster or
VMDisk renders fresh volumes and comes up empty.
Reject the rename up front for every kind other than VMInstance, before
the RestoreJob enters Running, with a message that points at omitting
targetApplicationRef.name or using the engine-native strategy. The
existing same-namespace guard keeps precedence, so its message is
unchanged.
Closes#3537
Assisted-By: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
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>
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>
kafkaRunDeadline silently substituted the 30m default for a backupTimeout
that time.ParseDuration rejects or that falls below the one-minute floor,
with no Event, condition or log line. An operator who set an unparseable
or sub-minute value, then raised it, saw every run keep dying at
DeadlineExceeded with nothing naming the discarded parameter.
Return the applied deadline together with a message when the configured
value is rejected, and record it as a BackupTimeoutInvalid Warning Event
on the BackupJob and RestoreJob (the reconcilers already hold a Recorder).
An honoured or unset value stays silent. TestKafkaRunDeadline now asserts
the warning fires for the rejected cases and not otherwise.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
The restore half of the driver - the half
that mutates a live Kafka cluster - had no test reaching its happy path,
so three reversions of it left the suite green.
- Add restore reconcile tests mirroring the Rabbitmq driver this was
copied from: ToCopy (pins the targetApplicationRef.Name merge - a
to-copy restore must resolve the TARGET, not write the source's topics
back into the source), NoRecordedArtifactURI, TargetClusterNotReady and
TargetNotFound. Verified non-vacuous: neutralising the name merge reddens
ToCopy, and reverting the fail-fast ordering below reddens
NoRecordedArtifactURI.
- Fail fast on a Backup with no status.artifact.uri: move the check ahead
of the readiness gate so it terminates immediately instead of waiting out
the whole deadline and then reporting a misleading "timed out waiting for
Kafka cluster to become Ready" (reconcileRabbitmqRestore checks it first;
same order here).
- Add bats cases for the fail-closed guards that no test reddened: MODE
validation, the s3:// URI prefix, cleanup treating 404 as success, an
unparsable backup/live describe, a kafka-configs describe error, and an
unrecognised record kind (plus a CFG_FAIL knob in the configs stub). The
describe-parse guards abort via `set -euo pipefail` at the
`X=$(describe_field ...)` assignment, so these pin the fail-closed
behaviour; the explicit guards remain as belt-and-braces.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
A controller-side running-Job deadline was enforced in reconcileKafka /
reconcileKafkaRestore: at kafkaDefaultBackupDeadline from StartedAt the
BackupJob/RestoreJob was marked Failed. But the batch Job carried no
activeDeadlineSeconds and markBackupJobFailed only writes status, so the
Job kept running, finished, and its final `s3 PUT` uploaded the object —
while the terminal phase meant no Backup was ever created and cleanup
(keyed on Backup deletion) never ran. Every timed-out run thus orphaned a
uniquely-keyed object, and a cluster whose export legitimately exceeds the
deadline could never produce a usable backup. The shipped docs also
claimed the run "keeps the BackupJob Running ... rather than failing on a
wall clock", which the deadline made false.
Move the bound onto the Job as activeDeadlineSeconds (setKafkaJobDeadline):
Kubernetes fails an unschedulable or wedged Job as DeadlineExceeded and
kills its pod before the upload, so a timed-out run lands in the JobFailed
branch cleanly and leaves no object behind. Make it operator-tunable with
the `backupTimeout` BackupClass parameter (a Go duration), which
round-trips through the Backup so a restore is bounded by the same value;
the readiness-precondition wait honours it too. Correct the docs.
Also requeue on a transient broker-image List error instead of failing the
whole run: resolveKafkaBrokerImage now wraps a sentinel (errNoKafkaBroker)
so only a Ready cluster with no listable broker is terminal.
Tests: pin the deadline on the built Job, the backupTimeout override, a
JobFailed run marking the owner Failed, and the transient-List requeue.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
- delete_topic waited with `grep -qx` (BRE), so deleting `audit.events`
matched the live `audit-events` and spun to its timeout — the e2e
failed at step 25 on every run that reached it. Use `grep -qxF`.
- The two -k/--connect-to guards were written as `! grep`, which cannot
fail under hack/cozytest.sh (it wraps each @test in `set -e` + a
trailing `return 0`, and bash exempts a `!`-negated command from
errexit). Rewrite them as positive `if grep ...; then return 1; fi`;
verified the -k reinstatement now reddens the suite under cozytest.
- Cover the restore-side literal-topic guard that was unpinned at every
level: a bats case runs the existing-topic branch with a colliding
sibling and asserts the `--alter` argv carries `\Q…\E`, and run-all.sh
now leaves `audit.events` live so the e2e exercises that branch instead
of only `--create`. Add bats cases for the partition-shrink refusal, the
`-f` upload guard, and `--exclude-internal`.
- Bound a running Job: reconcileKafka / reconcileKafkaRestore now fail the
BackupJob/RestoreJob once kafkaDefaultBackupDeadline elapses in the
Job-running branch (buildJobStrategyBatchJob sets no
activeDeadlineSeconds), so an unschedulable pod no longer pins it in
Running forever. Pinned by TestReconcileKafka_FailsWhenJobNeverCompletes.
- Fail closed on an unknown record kind and an unknown MODE, and validate
the live partition/RF parsed on restore (an empty value silently skipped
reconciliation). Correct the stale "image pinned in the strategy" comment
in the example helpers and the chainsaw test (the controller now resolves
the target broker's image as .ClientImage).
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
The Kafka metadata strategy pinned kafkaBackupClientImage to an upstream
Strimzi tag with no producer: nothing stamped it and Renovate does not
manage helm-values, so it drifted from the broker version the operator
provisions. A dedicated small client image (the sibling curl-only drivers'
follow-up) is not viable here because the driver runs the Kafka Admin-API
CLI, which only the broker image ships.
Resolve the client image from a live broker pod of the target Strimzi Kafka
at reconcile time instead, so the CLI always matches the broker (any version
the operator picks, not only the chart default), and record it on the Backup
so cleanup can reuse it once the source cluster is gone. The chart no longer
pins the image; the strategy template renders it as the .ClientImage
placeholder the controller fills.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
- Regression fix: an earlier change dropped the curl --connect-to port workaround along
with -k, but curl 7.76.1 (the image's) 403s on ANY ported endpoint,
which is the shape backupStorage.endpoint ships (:8333) and what
provisionBucket:false and s3.ingress.enabled:false produce. Restore
--connect-to (sign a portless URL, redirect to the real port), keeping
-k off so TLS is still verified. IPv6-safe host/port split, path prefix
preserved. The port-less ingress passed; the ported case is the one that regressed.
- Correct the S3 envelope claim in values.yaml and backup-classes.md: the
403 is decided by the port, not a body-hash the backend recomputes, and
--connect-to handles it on SeaweedFS and MinIO alike.
- Hold the script fixes at the layer the contract lives at. Externalise
the driver to files/kafka-backup.sh (embedded verbatim via .Files.Get,
render byte-identical) and add hack/kafka-backup-script.bats, which
EXECUTES it against stubbed kafka CLIs + curl and asserts behaviour: \Q
literal match on the colliding pair, --connect-to with no -k on a ported
endpoint, fail-closed --list, comma-value bracket grouping, RF-divergence
abort, and the cleanup delete's HTTP handling. Not a template grep.
- Pin the Backup-delete dispatch and the readiness deadline in Go:
TestCleanupOnDelete_Kafka_RoutesToKafkaCleanup and
TestReconcileKafka_FailsAfterReadinessDeadline.
- Docs: config restore is additive (no --delete-config); export time
scales with topic count and the Job has no wall clock (as its siblings).
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
- createKafkaBackupArtifact now rejects adopting a same-named Backup that
describes a different application, so a retained Backup reused for a
different app of the same name is not reported as this run's success.
Mirrors the RabbitMQ driver's guard; pinned by a Go test.
- ArtifactURITemplate godoc said empty leaves status.artifact unset, but
the driver hard-fails an empty template. Reword to state it is required,
and regenerate the CRD description to match.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
- Drop `-k`: mirror the RabbitMQ S3 layer (ARTIFACT_URI as the single
source of truth, forcePathStyle honoured, whole base URL kept) so
curl --aws-sigv4 verifies TLS against the ACME endpoint the helper
returns. The derived endpoint is default-port, so the curl 7.76.1
--connect-to port workaround is gone too; this also fixes the IPv6
and path-prefix mangling in the old host-splitting.
- Own the artifact: a cleanup mode in the script plus cleanupKafkaBackup
wired into BackupReconciler.cleanupOnDelete delete a deleted Backup's
object (best-effort, waited on) instead of orphaning it, matching the
RabbitMQ precedent. Kafka has the identical own-your-object property.
- Restore probes existence with --list (fail closed), so an unreachable
broker aborts instead of being misread as "topic absent" and created.
- Hold the fixes with tests at the right level: a colliding topic pair
(audit.events / audit-events, distinct partition counts) in the e2e so
a revert of the \Q...\E literal-match breaks the round-trip, and Go
tests pinning the scope stamp and the cleanup Job.
- Correct the docs/comments: endpoint-shape row, the "cached on nodes"
claim, and the narrower curl 7.76.1 S3 envelope (empty-body payload
hash) that makes a small modern-curl client image the right follow-up.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
Reproduced end-to-end against a real Kafka 3.9.1 broker:
- [CRITICAL] The Strimzi image ships curl 7.76.1, whose --aws-sigv4
omits a non-default port from the SigV4 canonical Host while sending it
in the Host header, so every S3 PUT/GET 403s and the driver never
uploads or downloads. Sign/send the bare host and redirect the
connection to the real port with --connect-to (the same fix the
topic-data example already carries), and accept the in-cluster
self-signed cert with -k.
- [CRITICAL] kafka-topics --topic is a Java regex, not a literal: a
backup entry for "audit.events" also matched a live "audit-events", so
restore repartitioned an unrelated topic irreversibly and backup
recorded the wrong partition/replication values. Wrap the name in
\Q...\E on every filter call (--describe on both paths, --alter on
restore) so it matches exactly one topic; --create and
kafka-configs --entity-name are already literal. Applied in the example
helpers too.
- [MAJOR] cozy-default's Kafka route looked like a data backup at
runtime. Stamp scope=topic-metadata on the produced Backup's
DriverMetadata and say "topic definitions and configs only; no message
data" in the Ready condition, so kubectl describe backup is honest.
- [MAJOR] Add reconcile tests for the two Go behaviours a mutation could
silently drop: the Strimzi-Ready precondition on backup (BackupJob
stays out of Succeeded and creates no Job) and the applicationRef Kind
gate on the restore target.
- [MINOR] Fail closed on the config-capture grep (exit != 1 aborts) and
clarify the image pin has no automated producer.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
- Fail closed on topic listing: set -o pipefail, capture kafka-topics
--list --exclude-internal exit separately, and abort on failure so an
unreachable broker can no longer produce an empty backup reported as
Succeeded. An empty list after a successful call stays legitimate.
- Check each kafka-topics --describe exit and validate the parsed
partition count / replication factor before writing a record, so a
timed-out describe cannot write a corrupt orders\t\t\t line.
- Store topic configs one per line with key and value in separate
tab-delimited fields, sourced from kafka-configs --describe (one config
per line), so a value containing a comma (cleanup.policy=compact,delete)
round-trips; on restore each config is applied with the value bracketed
only when it contains a comma.
- Detect topic divergence on restore instead of a silent --if-not-exists
no-op: grow partitions with --alter when the backup asks for more, and
fail loudly (naming the topic and both values) when it asks for fewer
partitions or a different replication factor. A restore that cannot
reach the recorded state no longer reports Succeeded.
- Add KafkaStrategyKind to the RestoreJob cleanup-dispatch no-op group so
deleting a Kafka RestoreJob does not run a pointless Velero cleanup; add
a regression test alongside the siblings, plus a reconcile Kind-gate
test and a rendering test that actually renders the strategy template.
- Wire the kafka-metadata Chainsaw suite into both selectors
(select-install suite_to_source, select-e2e src_to_suites) so editing
the kafka app or the suite selects it and the bats unit tests pass.
- Pin the client image to the operator's default broker version
(kafka 3.9.1) with a digest, and align the example default; fix the
values comment. Point the RabbitMQ comment references (which shipped in
the CRD description) at the Job strategy actually mirrored, and trim the
in-pod comments that rendered into the manifest.
- Document the Strimzi Topic Operator caveat: for a topic owned by a
KafkaTopic CR the operator reconciles its config from the CR, so an
in-place config restore is durable only for out-of-band topics.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
Add a strategy.backups.cozystack.io/Kafka driver that backs up the topic
metadata of a Cozystack-managed Kafka application — partition counts,
replication factors and non-default topic configs — through the Kafka
Admin API, and restores it with kafka-topics --create + kafka-configs
--alter. Modelled on the Job/Rabbitmq strategies: the driver renders a
one-shot batch Job, gates on the Strimzi Kafka cluster reporting Ready,
and keys each backup by .BackupName so runs do not overwrite each other.
Scope is deliberately narrow: message payloads, consumer-group offsets,
ACLs, quotas and KafkaUsers are out of scope, and there is no PITR.
Restored topics are created directly (unmanaged on the target) — the
accepted tradeoff of an Admin-API-only driver. Both restore flows are
implemented: in-place recreates dropped topics into the source, to-copy
applies the source's topics onto a freshly-bootstrapped empty cluster.
Wire it into cozy-default (strategy-kafka-default.yaml gated on the
bucket name, a BackupClass binding, read RBAC on kafka.strimzi.io/kafkas
for the readiness gate, and the Strimzi Kafka client image), document it
in backup-classes.md, and add an honest round-trip example plus Chainsaw
e2e (examples/backups/kafka-metadata, kafka-3-metadata-roundtrip) that
proves a per-run retention.ms sentinel survives both restores.
Unit tests cover the driver's pure helpers (application-ref validation,
readiness verdict, parameter round-trip, per-run artifact-URI keying).
Not yet verified on a live cluster.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
Delete the backup object when its Backup is removed. The Redis driver
owns its S3 object (one per BackupJob, no engine retention prunes it), so
it now spawns a cleanup-mode Job that deletes the object and holds the
Backup's finalizer until it completes, mirroring the Rabbitmq driver.
Without this a retention-pruned Plan grew the bucket unboundedly and the
key was lost with the CR.
Size the strategy Pod from backupStrategyController.redisBackupResources
so an admin can raise the restore ceiling (the throwaway loader holds the
whole dataset in memory) via chart values, which a direct edit of the
cluster-scoped, Helm-managed Strategy CR could not survive. The
loader-not-ready diagnostic now names the memory limit, the likeliest
cause, instead of only the image version.
Make the endpoint CA volume optional: nothing projects that Secret into
the app namespace, so a required mount wedged every backup Pod on
FailedMount; a missing ca.crt now falls through to the system CA store
and fails the handshake fast, which is not the same as skipping verify.
Promote the Pod-level activeDeadlineSeconds to the Job so a wedged run is
bounded once, not (backoffLimit+1) times. Use find, not ls, for the base
AOF so its no-match diagnostic is not dead code under pipefail.
Pin the object key reaching the rendered Job: both fixtures now render
--key={{ .ObjectKey }} and assert it, so zeroing the key (which left the
package green) is caught. Document .ObjectKey and the cleanup mode in the
CRD render-context, list cozy-default-redis among the Helm-managed CRs,
and mirror the CA/TLS and find fixes into the example strategy.
Assisted-By: LLM
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
The readiness re-gating guard read `r.Get(...) == nil` to mean "the Job does
not exist yet", folding any non-NotFound error (an apiserver hiccup, an RBAC
denial) into that branch. On such a transient error the driver would re-enter
the readiness precondition even though the Job may already exist and have
completed, reintroducing the spurious terminal failure this guard removes.
Switch on the Get error: nil means the Job exists (observe it), NotFound
means run the pre-launch readiness gate, and any other error requeues without
deciding. Same on the backup and restore paths.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
The Redis backup/restore reconcilers checked the application's Ready
condition on every pass, ahead of the Job-state switch. Once the Job has
been created that re-check is wrong: if the app's Ready flips False after
launch (an unrelated HelmRelease blip) and stays so past the deadline, the
driver marks a run Failed whose Job may already have completed, a spurious
terminal failure of a successful backup or restore.
Gate the readiness precondition on the Job not yet existing; once it does,
observe the Job directly. The deadline still bounds the pre-launch wait.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-by: LLM
The restore path already asserts a terminal failure for an applicationRef
that passes the Redis gate but has no REST mapping; add the symmetric
backup-path case so both dispatch sides are covered.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-By: LLM
The spotahome RedisFailover operator ships no backup CRD, so add a
dedicated strategy.backups.cozystack.io/Redis driver that runs a one-shot
Job: it dumps an RDB snapshot from the current master (discovered via the
operator's sentinel Service) to the system bucket over S3, and on restore
replays it (RDB -> RESP via redis-cli --pipe) into the target's master.
Structurally a specialisation of the Job/Altinity script-Job pattern with
a Redis applicationRef gate and an app-readiness precondition. Backup
parameters round-trip through the Backup's DriverMetadata (no
underlyingResources snapshot); restore drives from .Backup.ApplicationRef
so a to-copy restore reads the source's object-storage prefix while
writing into a differently-named target. Both in-place and to-copy are
supported; PITR is not (RDB is a discrete snapshot).
- api: Redis/RedisList strategy type + generated CRD and deepcopy
- driver: reconcileRedis / reconcileRedisRestore + pure helpers, wired
into both dispatch switches, supportedBackupStrategyKinds and the no-op
restore cleanup case
- default: gated cozy-default-redis strategy (system-bucket coords via
cozy-backups-creds env secretKeyRef, https:// scheme + optional CA) and
the Redis route in the cozy-default BackupClass
- image: images/redis-backup bundling redis-cli + curl + an RDB->RESP
converter, wired via backupStrategyController.redisBackupClientImage
- tests: driver unit tests (gate, precondition, render, param round-trip,
both reconcile paths) and the helm rendering-gate coverage
- example: examples/backups/redis honest marker round-trip for both
restore flows
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Assisted-By: LLM
## What this PR does
Adds backup/restore support for the managed `RabbitMQ` application.
RabbitMQ's cluster operator ships no backup CRD and RabbitMQ has no
consistent online export of message payloads, so this backs up broker
**definitions** (vhosts, users, permissions, queues, exchanges,
bindings, policies, parameters) over the management HTTP API
(`GET`/`POST /api/definitions`) and stores them in the system bucket.
Message payloads and point-in-time recovery are intentionally out of
scope; a full-data backup would be a Velero volume snapshot, which is a
separate strategy.
It is delivered as a **first-class
`strategy.backups.cozystack.io/Rabbitmq` driver**, following the
Altinity pattern (a dedicated driver that renders a one-shot Job for an
HTTP-API engine):
- `reconcileRabbitmq` / `reconcileRabbitmqRestore` render the strategy's
`PodTemplateSpec` — `curl` `GET`/`POST /api/definitions`,
pushing/pulling the JSON to S3 with `curl --aws-sigv4` — as a
`batch/v1.Job`, covering both restore flows: in-place (into the source
broker) and to-copy (into a separate, differently-named RabbitMQ in the
same namespace, authenticating with the target's own default-user
credentials and reading the source's object key).
- A **Go-read precondition** gates each run on the `RabbitmqCluster`
reporting `AllReplicasReady` (read through a small `rabbitmqtypes`
partial type), so a not-yet-ready broker surfaces a precise
`Ready=False` wait on the BackupJob/RestoreJob rather than a Job that
starts and fails against an unreachable management API.
- The produced `Backup` records the stored object's location on
`status.artifact.uri`, rendered from the strategy's
`spec.artifactURITemplate`. The `Backup` CRD has no status subresource,
so this persists on Create, mirroring the Etcd driver.
Dispatch is wired in both controllers (plus
`supportedBackupStrategyKinds`), `rabbitmqtypes` is registered in the
scheme, and RBAC grants read on `rabbitmqclusters.rabbitmq.com` (the Job
itself is already covered by the existing `batch/jobs` grant). It ships
`cozy-default-rabbitmq` (a `$bucketName`-gated `Rabbitmq` strategy) and
routes `apps.cozystack.io/RabbitMQ` to it in the `cozy-default`
BackupClass, which also enrols it in the `DefaultObjectsGate`. The
RabbitMQ application chart is left untouched — it had no prior backup
wiring, and the strategy carries the S3 coordinates itself.
Unit tests cover the driver's pure helpers (applicationRef validation,
cluster-name derivation, the readiness-message builder, artifact-URI
rendering, parameter round-trip) and the reconcile precondition gate
(defers with `Ready=False` and no Job until the cluster is ready, then
proceeds). A runnable example under `examples/backups/rabbitmq/` and an
honest-roundtrip Chainsaw e2e under `hack/e2e-chainsaw/rabbitmq/` seed a
sentinel vhost, back it up, drop it, and assert it round-trips through
object storage on both restore flows.
The Job pod reuses the existing `platform-migrations` image (curl +
ca-certificates) under a new
`backupStrategyController.rabbitmqBackupClientImage` key, stamped in
lockstep with `chBackupClientImage` in `packages/core/platform/Makefile`
so the repository stays pinned at a single digest
(`hack/image-pin-consistency.bats`). A dedicated slim image built by
this package is a follow-up.
**Validation:** `go build`/`go vet`/`gofmt`/`go test
./internal/backupcontroller/... ./internal/template/...`, deepcopy +
Rabbitmq CRD generation, `helm template` proving the CRD-ordering gate
both on and off, `helm lint`, and `helm unittest` (18/18). The backup +
in-place + to-copy roundtrip was verified end-to-end on a dev cluster (a
real management-API export and a real S3 `--aws-sigv4` upload/download
against SeaweedFS), so the driver's core contract is exercised on a real
cluster; the in-CI Chainsaw run is the remaining automated evidence. The
object-cleanup, terminating-namespace teardown and readiness-deadline
paths added during review are unit-tested (including `Reconcile`-level
finalizer retention and the terminating-namespace release) but not yet
cluster-exercised. (Note found while testing: RabbitMQ 4.2 is OOMKilled
on the `nano` preset, so the example/e2e use `c1.small`.)
### Screenshots
Not applicable — no UI changes.
### Downstream repositories
Walked the trigger map against the diff. This is an additive backup
strategy plus a new strategy CRD; no downstream repository is
mechanically forced by the change. A documentation page for RabbitMQ
backup on [cozystack/website](https://github.com/cozystack/website)
would be a reasonable follow-up, but it is not required by this diff —
leaving the boxes unticked for a human to decide rather than ticking "no
downstream" to clear the checklist.
### Release note
```release-note
feat(backups): add RabbitMQ definitions backup/restore via a dedicated strategy.backups.cozystack.io/Rabbitmq driver; it gates each run on the RabbitmqCluster being ready and records the stored object on Backup.status.artifact.uri.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Added RabbitMQ backup and restore support for definitions, including
vhosts, users, queues, exchanges, bindings, policies, and related
settings.
* Added in-place and restore-to-copy workflows with readiness checks.
* Added scheduled and on-demand backup examples.
* Integrated RabbitMQ backups with the default backup configuration.
* Backups store definitions only; message payloads and point-in-time
recovery are not included.
* **Documentation**
* Added a complete RabbitMQ backup and restore demonstration guide.
* **Tests**
* Added end-to-end coverage for readiness and backup/restore round
trips.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
RabbitMQ's cluster operator ships no backup CRD and RabbitMQ has no
consistent online export of message payloads, so back up broker
DEFINITIONS (vhosts, users, permissions, queues, exchanges, bindings,
policies, parameters) over the management HTTP API and store them in the
system bucket. Message payloads and PITR are out of scope - a full-data
backup is a Velero volume snapshot, a separate strategy.
Add a first-class strategy.backups.cozystack.io/Rabbitmq driver, following
the Altinity pattern (a dedicated driver that renders a one-shot Job for an
HTTP-API engine):
- reconcileRabbitmq / reconcileRabbitmqRestore render the strategy's
PodTemplateSpec (curl GET/POST /api/definitions, push/pull to S3 with
--aws-sigv4) as a batch/v1.Job, for both in-place and to-copy restore.
- A Go-read precondition gates each run on the RabbitmqCluster reporting
AllReplicasReady (via the rabbitmqtypes partial type), surfacing a
precise Ready=False wait instead of a Job that starts and fails.
- The produced Backup records the object location on status.artifact.uri
(rendered from spec.artifactURITemplate; the Backup CRD has no status
subresource, so it persists on Create, as in the Etcd driver).
Wire dispatch in both controllers + supportedBackupStrategyKinds, register
rabbitmqtypes in the scheme, and grant read on rabbitmqclusters. Ship
cozy-default-rabbitmq (a $bucketName-gated Rabbitmq strategy) and route
apps.cozystack.io/RabbitMQ to it in the cozy-default BackupClass. The
application chart is untouched.
Add unit tests for the driver's pure helpers and the precondition gate, a
runnable example, and an honest-roundtrip Chainsaw e2e that seeds a
sentinel vhost, backs it up, drops it, and asserts it round-trips through
object storage on both restore flows.
The Job pod reuses the platform-migrations image (curl + ca-certificates)
under a new rabbitmqBackupClientImage key, stamped in lockstep with
chBackupClientImage so the repository stays pinned at one digest; a
dedicated slim image is a follow-up.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
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>
resolveMongoDBBackupSource folded two failure classes into one error return:
a transient API read error on the live PerconaServerMongoDBBackup CR (cache
not yet synced, apiserver hiccup) and the terminal no-restorable-destination
case. The RestoreJob caller turned every error into markRestoreJobFailed, so a
retryable read error failed a restore that would have succeeded on the next
reconcile. The backup path already treats the symmetric live-CR read error as
transient (return ctrl.Result{}, err).
Wrap the transient read error in a new errTransientRestoreSource sentinel and
requeue on it at the caller; only a genuinely unresolvable source stays
terminal. Adds a non-vacuous interceptor-driven test.
Addresses the MINOR robustness note from PR review.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Address the two blocking findings in PR #3562:
- Restore into a differently-named target now authenticates with the
TARGET cluster's own S3 credentials instead of the source's. The
backupSource carried the source release's -s3-creds Secret, which is
deleted together with the source app — the disaster-recovery case
backups exist for — leaving a to-copy restore unable to reach S3. The
restore replays into the target (gated on backup.enabled just above),
whose own -s3-creds Secret is present and grants access to the same
bucket; only the credential reference is swapped, bucket/prefix/endpoint
still come from the source archive. Falls back to the source reference
when the target declares no discoverable s3 credentials (in-place
restore resolves to the same Secret, so it is a no-op there).
- A non-string recoveryTime (spec.options is free-form runtime.RawExtension,
so {"recoveryTime": 20260805} passes admission) no longer silently
degrades a PITR request to a plain snapshot restore reported Succeeded.
When the typed decode fails and the blob carries a recoveryTime key, the
RestoreJob now fails terminally — the same guarantee pitrSpec already
makes for a malformed RFC3339 string.
Minor findings also addressed:
- BackupJob tolerates a not-yet-present MongoDB app CR with the same
bounded grace the psmdb-CR existence gate uses, instead of failing
terminally on a GitOps single-commit ordering race.
- examples: the to-copy target writes into its OWN S3 prefix
(mongodb-target/) rather than the source's, so the two clusters' oplog
and backup metadata no longer interleave under shared keys (both run
replset rs0).
- examples: wait_psmdb_ready no longer treats a transient state=error as
terminal — the cluster state is recomputed each reconcile and can blip
through error during replset init before converging to ready.
Unit tests cover the target-credential selector (preferred/default/sole
storage) and the load-bearing recoveryTime-key detection.
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
PITR: replace the bespoke spec.options.pitr{type,date} blob with a single
spec.options.recoveryTime RFC3339 string, matching the CNPG strategy's option
exactly (CNPGRestoreOptions.RecoveryTime). The driver converts recoveryTime to
the psmdb Restore's pitr{type:date,date} in the operator's "YYYY-MM-DD HH:MM:SS"
UTC format. An empty recoveryTime restores the snapshot as-taken (documented
difference from CNPG, whose empty value means latest WAL — a psmdb logical
backup is already consistent). A non-RFC3339 recoveryTime fails terminally.
Unknown options: parseMongoDBRestoreOptions now also returns the set of
spec.options keys the driver does not recognise; reconcileMongoDBRestore emits a
UnknownRestoreOption Warning event per unknown key (and logs it) instead of
silently ignoring it — so a "recoverytime" typo no longer silently degrades a
PITR restore to a snapshot restore. (BackupJob carries no spec.options blob, so
there is no backup-path equivalent; the only backup-time input is the free-form
BackupClass parameters used as template vars.)
Docs + examples updated to recoveryTime; tests cover the RFC3339→UTC mapping,
empty/invalid recoveryTime, and unknown-key reporting.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Address branch-review blocker: add TestCreateMongoDBBackupArtifact_ArtifactShape,
mirroring the MariaDB sibling. createMongoDBBackupArtifact writes the
driver-metadata keys (backup-name/namespace/destination), the Status.Artifact
URI, and the persisted snapshot that the restore path reads back — the
producer half of a seam previously exercised only end-to-end. A key-name drift
between this producer and resolveMongoDBBackupSource would otherwise pass every
unit test and surface only in e2e. The test asserts the metadata keys, phase,
artifact URI, and that the snapshot decodes to a usable by-reference
backupSource; a second case pins the empty-destination path (nil Artifact, no
destination key).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Address branch-review recommendation: give the MongoDB strategy an explicit
case in BackupReconciler.cleanupOnDelete, matching CNPG / Altinity / MariaDB /
FoundationDB. A deleted MongoDB Backup artifact previously fell through to the
Velero default (a safe no-op — MongoDB driverMetadata never carries
velero.io/backup-name), but the explicit branch documents the "we do not own
the psmdb S3 archive" contract and guards the seam against a future shared
helper incidentally stamping velero.io/backup-name onto MongoDB metadata.
Ships with TestBackupCleanup_MongoDB_DoesNotFallThroughToVelero, mirroring the
one-test-per-branch convention its three siblings follow: it seeds the velero
metadata keys + a matching velero.io/Backup and asserts no DeleteBackupRequest
is synthesised and the Velero Backup survives — so deleting the branch makes
the test fail, which is what makes the guard self-verifying.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Adds a MongoDB (Percona Server for MongoDB / psmdb) backup-strategy driver so
`apps.cozystack.io/MongoDB` reaches feature parity with the CNPG, MariaDB and
ClickHouse native drivers — logical backups, restore-to-differently-named
instances, and point-in-time recovery through the unified
BackupJob/RestoreJob/BackupClass/Plan interface (closes#3545).
The driver orchestrates the operator's PerconaServerMongoDBBackup /
PerconaServerMongoDBRestore CRDs:
- BackupJob: creates a one-shot PerconaServerMongoDBBackup, polls
status.state, and persists the S3 destination + storage descriptor
(by-reference, no credentials) onto the Cozystack Backup artifact.
- RestoreJob: drives PerconaServerMongoDBRestore via backupSource (read back
from the source backup status, snapshot fallback) so the same artifact
restores in-place or into a differently-named instance; PITR via
spec.options.pitr (date/latest).
Because psmdb resolves the backup storage by name against the cluster's
spec.backup.storages, the driver names the chart's storage (s3-storage) via the
cozy-default-mongodb Strategy rather than injecting it, and surfaces a clear
Ready=False precondition when a target has backups disabled.
Wiring:
- api/backups/strategy/v1alpha1: MongoDB strategy type + generated CRD/deepcopy.
- internal/backupcontroller: psmdbstrategy_controller + psmdbtypes/mongodbapp
partial-schema mirrors; dispatch, supported-kinds and scheme registration.
- backupstrategy-controller: cozy-default MongoDB route, cozy-default-mongodb
Strategy CR, and psmdb.percona.com RBAC.
- apps/mongodb: backup.insecureSkipTLSVerify knob (psmdb s3 storage has no
CA-bundle field) so backups can target Cozystack's self-signed seaweedfs.
Tests + e2e:
- unit tests for the driver (validation, ensure-idempotency, precondition,
backupSource resolution, snapshot round-trip, PITR option parsing).
- examples/backups/mongodb + a mongodb-2-backup-roundtrip Chainsaw suite that
drives run-all.sh end-to-end (sentinel document round-trips through S3 into a
restored copy while the source stays untouched), per docs/agents/e2e-testing.md.
- docs/operations/backup-classes.md updated.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Addresses the four findings on #3524.
**The `fail` in the bucket chart (MAJOR).** The <bucket>-<user>-credentials
Secret was made to `fail` the render while its COSI lookup was empty, so the
release would retry and converge instead of skipping the Secret permanently.
Helm cannot render a partial set, so that fail aborts the WHOLE
<bucket>-system release: the other users' Secrets, and the bucket UI
Deployment/Service/Ingress/HTTPRoute with them. One declared user whose
BucketAccess never provisions takes down every other user of that bucket,
and on an already-installed release parks it in Failed, blocking every later
upgrade.
**No per-bucket escape (MAJOR).** Rather than add one, remove the need for
it. The gate already forces the release that renders the lookup-gated
Strategy CRs; the credentials Secret is the same trap one release earlier,
and the gate reads that Secret to resolve the bucket name in the first
place. So while it is absent, or carries no bucket name, force the bucket's
<bucket>-system release instead of doing nothing. That cannot deadlock its
own precondition: the BucketClaim and the BucketAccess whose COSI Secret the
lookup reads are rendered unconditionally by the parent release, not by the
one being forced.
The two releases are throttled independently — they are forced in sequence
on a bootstrap, so a shared timestamp would delay the second by a full
MinForceInterval for no reason. Coordinates come from the chart under
provisionBucket, and are omitted on external S3 where the Secret is
admin-managed and no release renders it. No RBAC delta: patch on
helmreleases is already cluster-scoped.
The chart goes back to a partial render, with the comment now explaining why
the skip is not self-healing and why failing is the wrong lever, and a test
that renders the UI with every user unresolvable so a reintroduced `fail`
fails the suite. Tenant buckets keep their pre-existing behaviour;
generalising the repair to every bucket needs a controller that owns them
and is tracked separately.
**Stale gauge (MINOR).** cozystack_backup_default_objects_missing was only
written on the happy path, so the state the gate exists to catch reported 0
and the documented alert never fired. It is now written on the unresolved
path too — the credentials Secret counts as one of the objects the default
backups depend on, which is what it always was. On an API error the gauge is
deliberately left alone rather than flapping, so a new
cozystack_backup_default_objects_check_errors_total marks it stale, and the
doc pairs the two instead of overstating the gauge.
**Suspended releases (MINOR).** forceHelmRelease now does a point Get and
skips a release with spec.suspend: true. helm-controller ignores forceAt and
requestedAt while suspended (`cozyhr suspend` sets exactly that), so the gate
was re-stamping every MinForceInterval for the whole suspension and climbing
the force counter — which the runbook attributes to a render that is not
producing the objects, the wrong diagnosis. The skip is logged and not
counted, so a climbing counter keeps its documented meaning.
Reported-by: IvanHunters <xorokhotnikov@gmail.com>
Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
Assisted-By: Claude <noreply@anthropic.com>
The gate reads the BackupClass as the manifest of what must exist, but the Go
fixture listed only CNPG, Etcd and the two Velero routes while the shipped
backupclass-default.yaml also routes MariaDB and Altinity. Add both to the
fixture, and to the RESTMapper and list-kinds derived from it, so the
expected-object set the tests exercise equals the real route set.
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
The BSL existence check used a hardcoded GVR and the dynamic client, whose
Get returns a plain 404 (IsNotFound) for an unserved API group. The
IsNoMatchError branch meant to skip an absent Velero API was therefore dead
code, and if Velero were uninstalled after bootstrap while bslEnabled=true
the BSL would be counted missing and force a Helm upgrade forever. Route the
lookup through the RESTMapper like the strategy loop, so an absent velero.io
API is a NoMatch we skip. A regression test fails without the change.
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
The cozy-default BackupClass routes VMInstance/VMDisk to the Velero
Strategy CRs unconditionally, but the chart gates those CRs on
velero.bslEnabled. With the BSL disabled they never render, so the gate
counted them as permanently missing and forced a Helm upgrade every
MinForceInterval forever, climbing force_reconciles_total and pinning the
missing gauge. Skip Velero-kind strategyRefs when the BSL is disabled
(empty VeleroNamespace), the same flag that gates their render. The
regression test now omits the manually-created Velero objects so it fails
without the skip. Correct the values.yaml comment that wrongly claimed the
Strategy CRs still ship in this mode.
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
Two defects found in review of the periodic recovery loop.
checkAndLog passed the manager's root context straight into Check, which
makes several sequential API calls. That context is only cancelled at
shutdown, so an API server stalling on any one of them blocked the call
indefinitely. The ticker loop is sequential, so one stuck call stopped
every later check for the life of the pod: the recovery this gate exists
to provide went silent, leaving a stale gauge and no further log line,
with only a restart to bring it back. Each check now runs under a
timeout of half the Period, capped at 30s, so it can never overlap the
next tick.
Check also propagated a NotFound on the projector's source Secret,
although the very next branch treats an empty bucket name as an expected
no-op. Those are the same bootstrap state seen a moment apart, so the
absent Secret logged a check failure on every tick of the whole
bootstrap window for a condition the doc comment frames as graceful.
Both are pinned by tests that fail against the previous behaviour: one
reverts to an error on the absent Secret, the other to a timeout equal
to Period.
Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
The default Strategy CRs and the Velero BackupStorageLocation are
Helm-templated behind a `lookup` of the BucketClaim the same chart
creates, and the <bucket>-<user>-credentials Secret behind a `lookup` of
the COSI Secret. When those lookups are empty at install time the objects
were silently skipped, and helm-controller does not re-render a release
whose chart and values did not change (drift detection is off on
operator-generated HelmReleases), so the skip was permanent: clusters ran
for months with only BackupClass cozy-default and no Strategy CRs at all.
That also fail-closes the pre-adoption snapshot in the v1.6.0 etcd
migration, which reads the projected credentials Secret.
Add a DefaultObjectsGate runnable to backupstrategy-controller. Once the
bucket name is resolvable from the projector's source Secret, it checks
that every object cozy-default routes to exists and forces one real Helm
upgrade (reconcile.fluxcd.io/forceAt + requestedAt) when any is missing.
Helm remains the objects' only author; the gate only makes sure the render
that produces them actually happens. It cannot be a render-time `fail` in
this chart: the chart is the producer of the Bucket its own lookup reads,
so a failed render would deadlock the condition.
The bucket chart's user-credentials template CAN fail safely — the
BucketAccess that produces the COSI Secret belongs to the parent release
and that release retries forever — so make it fail loudly instead of
skipping, with requireUserCredentials=false as the offline-render escape.
Also correct the chart and doc comments that claimed Flux re-renders on
its interval, and document the two-release manual recovery for clusters
already affected.
Signed-off-by: Mattia Eleuteri <mattia@hidora.io>
## What this PR does
Makes PostgreSQL point-in-time recovery (PITR) a genuinely supported,
tested capability for the new `backups.cozystack.io` API. Fixes#2774.
Was **stacked on #3313** (`fix/3300-cnpg-barman-plugin`) for the
`recoveryTime` → CNPG `bootstrap.recovery.recoveryTarget.targetTime`
plumbing; #3313 has since merged and this branch is rebased on `main`.
This PR adds the reliability + verification layer #2774 asked for.
### Changes
- **Precise failure reason for an unreachable target.** The restore
deadline (`spec.options.restoreTimeoutSeconds`, default 30m) stays the
sole authority for "this restore is stuck" — the client waits the full
window either way. What changes is the *diagnosis*: once the deadline
has elapsed, if `recoveryTime` was set and the recovery Cluster never
went healthy, the CNPG driver reads the `full-recovery` pod logs and —
when several distinct attempts exited with PostgreSQL's `FATAL: recovery
ended before configured recovery target was reached` — fails the
RestoreJob with `RecoveryConverged=False` / `Ready=False` (reason
`RecoveryTargetUnreachable`) and an actionable message, instead of a
bare generic timeout. Timing is unchanged; only the reason/message
differ. (For a faster rejection, set a shorter `restoreTimeoutSeconds`.)
Design points:
- **We deliberately do NOT trip early off the FATAL.** A valid near-now
target emits that same FATAL transiently — repeatedly, for many
attempts, on a slow archiver — before it converges, so any earlier trip
would wrongly reject a recoverable restore (observed twice in CI). The
FATAL is used only to *explain* a failure the deadline has already
declared.
- **Health wins unconditionally** — the deadline/classification check
precedes the healthy check, but a recovery that converges within the
window reaches success and is never classified as failed (a live dev7
restore did exactly this).
- **Counting distinct FATAL-ing attempts** (not pod failures) tolerates
a one-off archive-lag miss on a near-now target and ignores transient
recovery-pod crashes (a node blip / brief API-server unreachability
never emits that FATAL).
- Adds the `pods/log` RBAC grant the log read needs (distinct
subresource from `pods`); a `Forbidden` is logged loudly and surfaced in
the failure message rather than silently disabling the classification.
- **e2e** (`examples/backups/postgres/run-all.sh`, wired into the
Chainsaw `postgres-2-backup-roundtrip` suite):
- Step 45 — write `before`, capture server timestamp (µs precision),
write `after`, restore to that timestamp, assert `before` present and
`after` absent.
- Step 46 — a `recoveryTime` an hour ahead must fail with reason
`RecoveryTargetUnreachable` once the deadline elapses (asserts the
*reason*, so it proves the classification fired, not just that a timeout
occurred).
- Chainsaw budget raised 25m → 40m to cover three sequential restores +
the post-deadline classification.
- **Docs + example**: uncommented `45-restorejob-pitr.yaml`; a
`docs/operations/backup-classes.md` PITR section (recoverable window,
discovering earliest/latest restorable time from the backup catalog
since `firstRecoverabilityPoint` is unreliable under the plugin,
archive-lag tolerance, idempotency under GitOps).
### Verification
- `go build` / `go vet` clean; unit tests pass
(`logIndicatesRecoveryTargetUnreachable`, `recoveryUnreachableFromLogs`,
`recoveryPodsToInspect`, nil-Clientset guard, a reconcile-level
success-path test asserting `Succeeded` + `RecoveryConverged=True`, and
— added per review —
`TestReconcileCNPGRestore_DeadlineClassifiesRecoveryTargetUnreachable`,
a reconcile-level test driving the deadline branch through all three
sub-paths: FATAL found → reason `RecoveryTargetUnreachable`, FATAL
absent → generic timeout, `Forbidden` log read → generic timeout +
missing-`pods/log`-RBAC note. It fails if the call-site reason is
reverted to the generic one).
- Reviewed via adversarial branch-review; fixes include a critical
missing-RBAC dead-on-arrival, a classification false-positive on
transient recovery-pod crashes, and a sub-second e2e flake.
- **dev7**: backup + WAL archiving + recovery convergence verified live
on the #3313 build. The load-bearing assumption of the classification —
CNPG **accumulates** failed `full-recovery` pods rather than reusing one
— was confirmed (observed 7 simultaneous failed recovery pods). A
fully-green live step-46 run was blocked by intermittent COSI
bucket-provisioning flakiness on dev7; the negative path is covered by
the CI Chainsaw e2e.
### Downstream repositories
Walked the trigger map in `docs/agents/contributing.md` against the
diff. The only user-facing surface is the new
`docs/operations/backup-classes.md` PITR section; the website keeps a
hand-mirrored copy per docs version.
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up: PITR docs mirror warranted (not yet opened; same pending
mirror as #3313's backup-classes changes)
- [x] No other downstream repository is affected by this change
```release-note
feat(backups): PostgreSQL RestoreJob now supports point-in-time recovery (PITR). Set spec.options.recoveryTime (RFC3339) to restore to an exact instant. A recoveryTime past the latest archived WAL still runs to the restore deadline (spec.options.restoreTimeoutSeconds, default 30m), but the failure is now reported with the precise reason RecoveryTargetUnreachable and an actionable message instead of a generic timeout; set a shorter restoreTimeoutSeconds for a faster rejection.
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **New Features**
* Added PostgreSQL point-in-time recovery (PITR) support via RFC3339
`recoveryTime`.
* Introduced a PITR RestoreJob example, plus helper variables and an
unreachable-target scenario.
* **Bug Fixes**
* Improved restore deadline handling with deterministic, log-based
fail-fast classification for unreachable recovery targets, including
clearer conditions.
* **Documentation**
* Added detailed PITR operational documentation (recovery windows,
failure behavior, and GitOps/idempotent behavior).
* Expanded README and example comments for PITR verification and failure
expectations.
* **Tests**
* Extended unit tests and end-to-end chainsaw workflow to cover PITR
success and unreachable failures.
* **Chores**
* Updated RBAC to allow pod log reads; refined cleanup and run scripts
for additional RestoreJob artifacts.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
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>
Addresses review CHANGES_REQUESTED on #3383.
CRITICAL: reconcileCNPGRestore checked the restore deadline BEFORE the
cluster-health check, so a reconcile that first observes "deadline elapsed"
AFTER the recovery Cluster already reached a healthy state (a controller
restart or stalled workqueue spanning restoreTimeoutSeconds) would mark an
already-converged restore Failed / RecoveryTargetUnreachable. A false Failed
can then escalate to data loss: a resubmitted RestoreJob's purge guard treats
the recovered Cluster as a stale leftover and deletes it + its PVCs.
Fix: check hasRecovery + cnpgClusterHealthy first and return Succeeded
unconditionally when healthy, regardless of how late the reconcile runs; only
a not-yet-healthy restore reaches the deadline/classification branch. Adds
TestReconcileCNPGRestore_HealthyPastDeadlineSucceeds (StartedAt 72h past the
deadline + healthy Cluster + recoveryTime set; the injected readPodLog fails
the test if the classification path is entered, proving health short-circuits).
MINOR: handle the deferred Close error on the pod-log reader (errcheck).
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
Review of #3383 found the RecoveryTargetUnreachable classification branch in
reconcileCNPGRestore was only covered through the pure helpers and
recoveryTargetUnreachable directly, never through reconcileCNPGRestore itself —
a mutation swapping the call-site reason back to the generic RestoreFailed kept
the suite green.
Add TestReconcileCNPGRestore_DeadlineClassifiesRecoveryTargetUnreachable: it
backdates StartedAt past the restore deadline with a recoveryTime set and drives
the real reconcile through the readPodLog seam, asserting all three deadline
sub-paths — FATAL present (reason RecoveryTargetUnreachable on both
RecoveryConverged and Ready), FATAL absent (generic timeout, no
RecoveryTargetUnreachable), and a Forbidden log read (generic timeout + the
missing-pods/log-RBAC note). Verified it fails when the call-site reason is
reverted to the generic RestoreFailed.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <andrey.kolkov@aenix.io>
Makes RestoreJob.spec.options.recoveryTime a genuinely supported, tested
capability for the new backups.cozystack.io API (issue #2774). The
recoveryTime -> CNPG bootstrap.recovery.recoveryTarget.targetTime plumbing
already existed on the barman-cloud-plugin branch; this adds the reliability
and verification layer the issue asked for:
- Fail-fast for an unreachable target: when a RestoreJob has recoveryTime set
and the recovery Cluster is not healthy, the driver reads the CNPG
full-recovery pod logs and, once several distinct attempts have exited with
PostgreSQL's "recovery ended before configured recovery target was reached"
FATAL, fails the RestoreJob with conditions RecoveryConverged=False /
Ready=False (reason RecoveryTargetUnreachable) instead of hanging to the
restore deadline. Health is checked first, so a converged recovery always
wins; counting distinct FATAL-ing attempts (not pod failures) tolerates a
transient archive-lag miss on a near-now target and ignores transient
recovery-pod crashes. Adds the pods/log RBAC grant the log read requires.
- e2e (examples/backups/postgres/run-all.sh, wired into the Chainsaw
postgres-2-backup-roundtrip suite): step 45 writes a before marker, captures
the server timestamp, writes an after marker, restores to that timestamp and
asserts the before row survived and the after row did not; step 46 submits a
recoveryTime an hour ahead and asserts it fails fast with reason
RecoveryTargetUnreachable.
- Working, uncommented example (45-restorejob-pitr.yaml) and a docs section
(docs/operations/backup-classes.md) covering the recoverable window, how to
discover the earliest/latest restorable time from the backup catalog, the
archive-lag tolerance, and idempotency under GitOps.
Refs #2774
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Andrey Kolkov <androndo@gmail.com>
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-up (cozystack#3300): ensureCNPGBackup stamps spec.method=plugin +
spec.pluginConfiguration on the postgresql.cnpg.io/Backup it creates — the field
that routes a driver-initiated run through the barman-cloud plugin. If it
regressed to the default (barmanObjectStore) method, CNPG would run the legacy
method against a Cluster that no longer has one and every platform backup would
fail. Add TestEnsureCNPGBackup_UsesPluginMethod asserting method, plugin name,
and cluster ref. (The chart ScheduledBackup path was already covered; the driver
path was not.)
Co-Authored-By: Claude Opus 4.8 (1M context) <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>
Reconcile the Chainsaw e2e migration with ~500 commits of main drift:
- keep main's run-kubernetes.sh helpers (kubectl_wait_retry, tenant
drain, LINSTOR pool-free wait, SC-fallback probe) under the branch's
_lib/ rename with the Chainsaw finally/snapshot deltas re-applied
- resolve modify/delete on 10 drifted hack/e2e-apps/*.bats as deleted;
their post-branch assertion drift is re-ported into the Chainsaw
suites in follow-up commits
- drop the 4 new main-side bats (kubernetes-oidc-*, securitygroup,
serviceexposure) from _lib/ relocation; ported as Chainsaw suites in
follow-up commits
- docs/agents/e2e-testing.md: merge main's plan-job TIA wording with
the Chainsaw suite phrasing
Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Rewrite docs/agents/e2e-testing.md for the Chainsaw suite: declarative
asserts replace the until-get/wait backstops, Chainsaw cleanup replaces
test-level EXIT traps, catch blocks replace the `|| { ...; false; }` idiom,
and the TIA/diagnostics sections describe suites + `make test-chainsaw
CHAINSAW_SUITES`. Move the BATS->Chainsaw bullet out of 'in-flight' since it
is now the standard.
Fix references the migration left stale: hack/e2e-apps/ -> hack/e2e-chainsaw/
in the gemini styleguide and the nats/etcd backup READMEs. The
backup-postgres.bats / backup-foundationdb.bats files cited by the postgres
examples, the postgres README, and the foundationdb controller comment never
existed -- drop the false 'CI e2e drives this' claims and describe those
backup flows as the manual reference flows they are.
Assisted-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Andrey Kolkov