mirror of
https://github.com/cozystack/cozystack.git
synced 2026-10-23 03:27:50 +09:00
fix(backups): refuse concurrent restores and skip the CNPG purge only for this restore's Cluster (#4754)
## What this PR does Fixes #3322. An in-place Postgres (CNPG) restore decided whether to skip deleting the live `Cluster` by comparing its `creationTimestamp` with the RestoreJob's `StartedAt`. The timestamp says the Cluster is new, not which restore rendered it, so two cases let a restore skip its purge and report `Succeeded` on data from another restore: 1. Two RestoreJobs into the same application at once. The second sees the Cluster the first produced, created after its own `StartedAt`, and skips its purge. 2. The live Cluster is deleted while the restore waits for the WAL archive. The still-active HelmRelease re-renders it from the previous restore's values, again created after `StartedAt`. ### Refuse a restore into an application another restore holds The RestoreJob controller now refuses, for every driver, a RestoreJob that has not started yet when another unfinished RestoreJob in the namespace writes into the same application and goes first. The refused job fails with reason `ConcurrentRestore`, and its message names the job that goes first. It fails instead of waiting, so the application is never restored a second time later, after the first restore, by a job its author may no longer expect to run. A started restore goes before one that has not started. Between two that have not, the older goes first, by creation time and then by name; creation times have one-second precision, so two jobs created in the same second are ordered by name. A running restore is never failed by a newer one. The target application is resolved by `restoreJobTarget`, the same helper the BackupJob hold uses, so a Velero restore into another namespace does not collide with an in-place one. The gate lists RestoreJobs through the manager's API reader, not the informer cache. A job reconciled before the cache holds its peer's `StartedAt` would otherwise see that peer as not started and, on a same-second name tie, proceed alongside it. The controller runs a single worker, so a peer's `StartedAt` written by an earlier reconcile is already on the API server when the next job reads it; `SetupWithManager` now sets `MaxConcurrentReconciles: 1` explicitly, since the guarantee depends on it. ### Skip the CNPG purge only for this restore's Cluster The restore patch already sets `bootstrap.newServerName` to `restoredServerName(cluster, RestoreJob UID)`, a value unique to each RestoreJob. The postgres chart now annotates the `Cluster` with it as `postgres.cozystack.io/restored-server-name`. The annotation is rendered even when the barman plugin is not; the plugin's `serverName` also carries the value, but the plugin renders only with chart-level S3 coordinates. The driver skips the purge only when the annotation names this RestoreJob. A Cluster produced by a concurrent restore, or re-rendered from an earlier restore's values, carries another value and is purged. The mark lives on the rendered Cluster rather than in RestoreJob status. That keeps it valid in the case the skip exists for: the purge succeeded, but the `TargetPurged` status write failed. A UID recorded in status was rejected in the code for exactly that reason. A Cluster rendered by a chart that predates the annotation has none and keeps the creation-time check, so a restore in flight across an upgrade behaves as before. ### Tests - `TestRestoreAhead` covers order, a started job going first, the name tie-break, finished and terminating jobs, other applications, retargeted restores and a Velero restore into another namespace. - `TestReconcileRestore_ConcurrentRestoreIsRefused` checks that the newer job fails with `ConcurrentRestore` and the running one is left alone. - `TestReconcileRestore_ConcurrentRestoreReadsPastTheCache` gives the cache a stale, not-started peer and the API reader the started one, and requires the job to fail with `ConcurrentRestore`. - `TestCNPGClusterFreshlyRecovered` adds both cases from the issue: a Cluster marked by another restore and created after `StartedAt` must be purged. The timestamp-only cases are kept for the unmarked fallback. - `TestReconcileCNPGRestore_RepeatInPlacePurgesStaleRecoveryCluster` gains two reconcile-level cases. One purges a Cluster that another restore marked, even though it was created after `StartedAt`. The other runs a full purge, takes the mark from the `bootstrap.newServerName` the restore patch wrote, drops `TargetPurged` as a lost status write would, and requires a Cluster carrying that mark to survive, although it predates `StartedAt`. That pins the purge decision to the value the chart actually renders. - `packages/apps/postgres/tests/restore_mark_test.yaml` checks that the annotation is rendered with and without the barman plugin, and is absent on a Cluster that was never restored. Each new test was checked against the code it guards: disabling the gate call, listing through the cache, flipping the started-job precedence, ignoring the annotation, or deriving the expected mark from the app name instead of the cluster name each turns one red. `go test ./internal/backupcontroller/...` and `helm unittest packages/apps/postgres` (152 tests) pass. The restore flow was not run against a live cluster. `docs/operations/backup-classes.md` documents the one-restore-at-a-time rule and the annotation. ### Screenshots N/A — no UI changes. ### Downstream repositories Walked the trigger map in `docs/agents/contributing.md` against the diff: Go changes in `internal/backupcontroller` (no CRD or API type change), one annotation added to the postgres chart's Cluster (no `values.yaml` or `values.schema.json` change), and a docs paragraph. No package is added or renamed, no platform values, ApplicationDefinition, RestoreJob CRD, metric, label or namespace changes, so no trigger is reached. - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(backups): a RestoreJob now fails with reason ConcurrentRestore when another unfinished RestoreJob is already restoring the same application, instead of running alongside it; create it again once the first restore has finished. An in-place Postgres restore now skips deleting the live Cluster only when that Cluster was rendered for this RestoreJob, identified by the new postgres.cozystack.io/restored-server-name annotation, so it no longer reports Succeeded on data from a concurrent or earlier restore. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Restores targeting the same application are now handled one at a time. A competing restore is refused rather than queued, with started restores taking priority. * PostgreSQL now calculates a default `shared_buffers` value when the memory limit is below 512 MiB, while respecting explicitly configured values. * **Bug Fixes** * Recovery clusters are more reliably distinguished from manual recoveries and clusters created by other restores, reducing the risk of purging the wrong cluster. * **Documentation** * Clarified restore ordering and how recovery-cluster cleanup is determined. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
8 files changed
+697
-104
No files matched your search
@@ -375,6 +375,8 @@ spec:
|
||||
name: orders-db
|
||||
```
|
||||
|
||||
An application takes one restore at a time. A RestoreJob that targets an application another unfinished RestoreJob is already restoring fails with reason `ConcurrentRestore` before it changes anything, and its message names the job that goes first. It is not queued: once that job has finished, create a new RestoreJob if the restore is still wanted. A restore that has started always goes first. Between two that have not, the older one runs; creation times have one-second precision, so of two created in the same second, the one whose name sorts first runs.
|
||||
|
||||
## Point-in-time recovery (PostgreSQL)
|
||||
|
||||
A `RestoreJob` restores a `Postgres` application from a `Backup`. Omit `spec.options.recoveryTime` to recover to the latest point in the WAL archive; set it (RFC3339) to recover the database to an exact instant — a point-in-time recovery (PITR). The CNPG barman-cloud plugin restores a base backup and replays archived WAL from it: up to `recoveryTime` when one is set, so the restored cluster reflects the database as of that instant and later writes are absent, and to the end of the archive otherwise.
|
||||
@@ -441,7 +443,7 @@ Recovery follows `recovery_target_timeline: latest`, which CNPG leaves as Postgr
|
||||
|
||||
### Idempotency under GitOps
|
||||
|
||||
An in-progress restore is safe to reconcile. The driver purges the target `Cluster` + PVCs exactly once per RestoreJob (guarded by the `TargetPurged` condition and a freshly-recovered check), suspends the target's HelmRelease across the purge so Flux cannot race the bootstrap swap, and resumes it once the recovery cluster is rendered. A Flux reconcile (or a controller restart) mid-restore therefore re-attaches to the recovering cluster rather than deleting it and starting over.
|
||||
An in-progress restore is safe to reconcile. The driver purges the target `Cluster` + PVCs exactly once per RestoreJob (guarded by the `TargetPurged` condition and by the `postgres.cozystack.io/restored-server-name` annotation, which identifies the RestoreJob whose purge produced the live recovery `Cluster`), suspends the target's HelmRelease across the purge so Flux cannot race the bootstrap swap, and resumes it once the recovery cluster is rendered. A Flux reconcile (or a controller restart) mid-restore therefore re-attaches to the recovering cluster rather than deleting it and starting over.
|
||||
|
||||
## Kafka: topic metadata only
|
||||
|
||||
|
||||
@@ -69,6 +69,10 @@ const (
|
||||
cnpgS3SecretRefKey = "cnpg.io/s3-secret-ref"
|
||||
cnpgMajorVersionKey = "cnpg.io/major-version"
|
||||
|
||||
// Annotation the postgres chart renders on the Cluster from
|
||||
// bootstrap.newServerName.
|
||||
cnpgRestoredServerNameAnnotation = "postgres.cozystack.io/restored-server-name"
|
||||
|
||||
// Polling cadence for the CNPG backup/restore lifecycle. Mirrors the
|
||||
// Velero strategy's defaults so behaviour is uniform across drivers.
|
||||
cnpgPollInterval = 5 * time.Second
|
||||
@@ -521,40 +525,43 @@ func cnpgPurgeNeeded(purgedCondition, liveClusterFreshlyRecovered bool) bool {
|
||||
|
||||
// cnpgClusterFreshlyRecovered reports whether a live cnpg.io Cluster that
|
||||
// carries spec.bootstrap.recovery was produced by THIS RestoreJob's own purge
|
||||
// + chart re-render (its creationTimestamp is strictly after the job's
|
||||
// StartedAt) rather than being left over from an earlier, already-completed
|
||||
// restore.
|
||||
// + chart re-render rather than rendered for some other restore.
|
||||
//
|
||||
// Only the former is safe to skip re-purging. A leftover recovery Cluster from
|
||||
// a prior restore predates StartedAt and still holds the prior data, so it must
|
||||
// be purged for the new restore to re-bootstrap from the backup. The fresh
|
||||
// Cluster in the status-write-race case is always created after StartedAt (the
|
||||
// job sets StartedAt on its first reconcile, long before it purges and the
|
||||
// chart re-renders), so the timestamp comparison cleanly separates the two.
|
||||
// Only the former is safe to skip re-purging. Any other recovery Cluster holds
|
||||
// another restore's data and must be purged for this restore to re-bootstrap
|
||||
// from its backup. That includes a Cluster a concurrent RestoreJob produced,
|
||||
// and one the chart re-renders from an earlier restore's values after being
|
||||
// deleted while this restore waits for the WAL archive: both are created after
|
||||
// this job's StartedAt, so a timestamp alone cannot tell them apart.
|
||||
//
|
||||
// The comparison is strict (created > started), so an exact tie resolves to
|
||||
// "not fresh" and the caller purges. That matches the conservative default
|
||||
// below: the only classification that must never be wrong is calling a stale
|
||||
// leftover "fresh" (which reintroduces the silent no-op), and a fresh Cluster
|
||||
// is always created well after StartedAt (see above), never exactly at it.
|
||||
// The chart annotates the Cluster with bootstrap.newServerName, which the
|
||||
// restore patch sets to restoredServerName(cluster, RestoreJob UID). The value
|
||||
// lives on the object the chart renders, not in RestoreJob status, so it
|
||||
// survives the failed status write the skip exists for, and it names the
|
||||
// restore that rendered the Cluster. When the annotation is present, the
|
||||
// Cluster is fresh only if it carries wantMark.
|
||||
//
|
||||
// An identity-based alternative was considered - recording the purged Cluster's
|
||||
// UID on the TargetPurged condition and comparing UIDs on later reconciles -
|
||||
// which is immune to clock skew. It was rejected because it leans on the same
|
||||
// status write that the status-write-race path (the whole reason this skip
|
||||
// exists) assumes can fail, so it cannot cover that case; the timestamp
|
||||
// comparison needs no extra persisted state and the fresh-vs-stale gap
|
||||
// (a full purge + re-render cycle) always dwarfs any plausible control-plane
|
||||
// clock skew.
|
||||
// A Cluster without the annotation was rendered either by a chart that
|
||||
// predates it or from values with no bootstrap.newServerName (a manually
|
||||
// configured recovery). It is fresh only if appMark, the application's current
|
||||
// bootstrap.newServerName, is wantMark - this restore has patched the values -
|
||||
// and it was created strictly after StartedAt, which separates a Cluster from
|
||||
// an earlier completed restore from this restore's own re-render.
|
||||
//
|
||||
// Returns false when freshness cannot be determined (no recovery bootstrap, or
|
||||
// a missing timestamp): the caller then proceeds to purge, which is the safe
|
||||
// default for any pre-existing, non-fresh Cluster.
|
||||
func cnpgClusterFreshlyRecovered(hasRecovery bool, clusterCreatedAt, restoreStartedAt *metav1.Time) bool {
|
||||
if !hasRecovery || clusterCreatedAt == nil || restoreStartedAt == nil {
|
||||
// a missing timestamp on the fallback path): the caller then purges, which is
|
||||
// the safe default.
|
||||
func cnpgClusterFreshlyRecovered(state recoveryClusterState, wantMark, appMark string, restoreStartedAt *metav1.Time) bool {
|
||||
if !state.hasRecovery {
|
||||
return false
|
||||
}
|
||||
return clusterCreatedAt.After(restoreStartedAt.Time)
|
||||
if state.restoreMark != "" {
|
||||
return state.restoreMark == wantMark
|
||||
}
|
||||
if appMark != wantMark || state.createdAt == nil || restoreStartedAt == nil {
|
||||
return false
|
||||
}
|
||||
return state.createdAt.After(restoreStartedAt.Time)
|
||||
}
|
||||
|
||||
// cnpgClusterGVR addresses Clusters on the dynamic client, for reads that must
|
||||
@@ -1021,17 +1028,16 @@ func (r *RestoreJobReconciler) reconcileCNPGRestore(ctx context.Context, restore
|
||||
// check the live Cluster for bootstrap.recovery: if present, the chart
|
||||
// has already re-rendered after a previous purge, and we must NOT delete
|
||||
// it again.
|
||||
hasRecovery, clusterCreatedAt, err := r.recoveryBootstrapClusterState(ctx, target.Namespace, clusterName)
|
||||
clusterState, err := r.recoveryBootstrapClusterState(ctx, target.Namespace, clusterName)
|
||||
if err != nil {
|
||||
return ctrl.Result{}, err
|
||||
}
|
||||
purgedCondition := apimeta.IsStatusConditionTrue(restoreJob.Status.Conditions, restoreCondTargetPurged)
|
||||
// Only skip the destructive purge for a recovery Cluster THIS restore just
|
||||
// produced (created at/after StartedAt). A recovery Cluster left over from
|
||||
// an earlier completed restore predates StartedAt and still holds the old
|
||||
// data, so it must be purged - otherwise a repeat in-place restore silently
|
||||
// no-ops.
|
||||
freshlyRecovered := cnpgClusterFreshlyRecovered(hasRecovery, clusterCreatedAt, restoreJob.Status.StartedAt)
|
||||
// produced. A recovery Cluster rendered for any other restore still holds
|
||||
// that restore's data, so it must be purged - otherwise a repeat in-place
|
||||
// restore silently no-ops.
|
||||
freshlyRecovered := cnpgClusterFreshlyRecovered(clusterState, restoredServerName(clusterName, restoreJob.UID), targetApp.Spec.Bootstrap.NewServerName, restoreJob.Status.StartedAt)
|
||||
if cnpgPurgeNeeded(purgedCondition, freshlyRecovered) {
|
||||
// A physical restore only opens on the major version that wrote the
|
||||
// data files. Across majors the recovery pods loop on "database files
|
||||
@@ -1206,7 +1212,7 @@ func (r *RestoreJobReconciler) reconcileCNPGRestore(ctx context.Context, restore
|
||||
// Getting the order wrong is not just a cosmetic mislabel - a false Failed
|
||||
// can be resubmitted, and the next RestoreJob's purge guard would then
|
||||
// delete the already-recovered Cluster + PVCs to start over.
|
||||
if hasRecovery {
|
||||
if clusterState.hasRecovery {
|
||||
healthy, herr := r.cnpgClusterHealthy(ctx, target.Namespace, clusterName)
|
||||
if herr != nil {
|
||||
return ctrl.Result{}, herr
|
||||
@@ -2070,22 +2076,37 @@ func (r *RestoreJobReconciler) readPodContainerLog(ctx context.Context, namespac
|
||||
// drops the change and the cluster ends up with the original initdb spec.
|
||||
// Holding here forces the caller to requeue until the old CR is fully GC'd
|
||||
// and the chart re-creates a fresh one.
|
||||
func (r *RestoreJobReconciler) recoveryBootstrapClusterState(ctx context.Context, namespace, clusterName string) (hasRecovery bool, createdAt *metav1.Time, err error) {
|
||||
func (r *RestoreJobReconciler) recoveryBootstrapClusterState(ctx context.Context, namespace, clusterName string) (recoveryClusterState, error) {
|
||||
cluster := &cnpgtypes.Cluster{}
|
||||
if err := r.Get(ctx, types.NamespacedName{Namespace: namespace, Name: clusterName}, cluster); err != nil {
|
||||
if apierrors.IsNotFound(err) {
|
||||
return false, nil, nil
|
||||
return recoveryClusterState{}, nil
|
||||
}
|
||||
return false, nil, err
|
||||
return recoveryClusterState{}, err
|
||||
}
|
||||
if !cluster.DeletionTimestamp.IsZero() {
|
||||
return false, nil, nil
|
||||
return recoveryClusterState{}, nil
|
||||
}
|
||||
if cluster.Spec.Bootstrap == nil || cluster.Spec.Bootstrap.Recovery == nil {
|
||||
return false, nil, nil
|
||||
return recoveryClusterState{}, nil
|
||||
}
|
||||
created := cluster.CreationTimestamp
|
||||
return true, &created, nil
|
||||
return recoveryClusterState{
|
||||
hasRecovery: true,
|
||||
createdAt: &created,
|
||||
restoreMark: cluster.Annotations[cnpgRestoredServerNameAnnotation],
|
||||
}, nil
|
||||
}
|
||||
|
||||
// recoveryClusterState is what the purge decision reads off the live Cluster.
|
||||
// The zero value is a Cluster that is absent, terminating, or not bootstrapped
|
||||
// from a recovery.
|
||||
type recoveryClusterState struct {
|
||||
hasRecovery bool
|
||||
createdAt *metav1.Time
|
||||
// restoreMark is cnpgRestoredServerNameAnnotation, or "" on a Cluster the
|
||||
// chart rendered before it set the annotation.
|
||||
restoreMark string
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
@@ -639,68 +639,108 @@ func TestCNPGPurgeNeeded(t *testing.T) {
|
||||
}
|
||||
|
||||
// TestCNPGClusterFreshlyRecovered locks in the signal that separates a
|
||||
// recovery Cluster THIS restore just produced from one left over by an
|
||||
// earlier completed restore. The repeat-in-place-restore bug was exactly a
|
||||
// stale recovery Cluster (created before StartedAt) being mistaken for a
|
||||
// freshly-recovered one and skipping the purge.
|
||||
// recovery Cluster THIS restore just produced from one rendered for another
|
||||
// restore. The repeat-in-place-restore bug was a stale recovery Cluster being
|
||||
// mistaken for a freshly-recovered one and skipping the purge; a Cluster
|
||||
// produced by a concurrent restore, or re-rendered from an earlier restore's
|
||||
// values, is created after StartedAt and must still be purged.
|
||||
func TestCNPGClusterFreshlyRecovered(t *testing.T) {
|
||||
started := metav1.NewTime(time.Now())
|
||||
after := metav1.NewTime(started.Add(time.Minute))
|
||||
before := metav1.NewTime(started.Add(-time.Hour))
|
||||
const ours = "postgres-app-restore-ours"
|
||||
const other = "postgres-app-restore-other"
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
hasRecovery bool
|
||||
createdAt *metav1.Time
|
||||
startedAt *metav1.Time
|
||||
want bool
|
||||
name string
|
||||
state recoveryClusterState
|
||||
appMark string
|
||||
startedAt *metav1.Time
|
||||
want bool
|
||||
}{
|
||||
{
|
||||
name: "no recovery bootstrap: not fresh",
|
||||
hasRecovery: false,
|
||||
createdAt: &after,
|
||||
startedAt: &started,
|
||||
want: false,
|
||||
name: "no recovery bootstrap: not fresh",
|
||||
state: recoveryClusterState{createdAt: &after, restoreMark: ours},
|
||||
startedAt: &started,
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "recovery cluster created after start (our own purge re-render): fresh",
|
||||
hasRecovery: true,
|
||||
createdAt: &after,
|
||||
startedAt: &started,
|
||||
want: true,
|
||||
name: "marked by this restore: fresh",
|
||||
state: recoveryClusterState{hasRecovery: true, createdAt: &after, restoreMark: ours},
|
||||
startedAt: &started,
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "recovery cluster created before start (leftover from previous restore): not fresh",
|
||||
hasRecovery: true,
|
||||
createdAt: &before,
|
||||
startedAt: &started,
|
||||
want: false,
|
||||
name: "marked by another restore, created after start (concurrent restore or re-render of an earlier restore): not fresh",
|
||||
state: recoveryClusterState{hasRecovery: true, createdAt: &after, restoreMark: other},
|
||||
startedAt: &started,
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "recovery cluster created exactly at start: not fresh (conservative tie -> purge)",
|
||||
hasRecovery: true,
|
||||
createdAt: &started,
|
||||
startedAt: &started,
|
||||
want: false,
|
||||
name: "marked by another restore, created before start (leftover from previous restore): not fresh",
|
||||
state: recoveryClusterState{hasRecovery: true, createdAt: &before, restoreMark: other},
|
||||
startedAt: &started,
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "missing cluster creation timestamp: not fresh",
|
||||
hasRecovery: true,
|
||||
createdAt: nil,
|
||||
startedAt: &started,
|
||||
want: false,
|
||||
name: "marked by this restore with no job start timestamp: fresh, the mark does not need one",
|
||||
state: recoveryClusterState{hasRecovery: true, createdAt: &after, restoreMark: ours},
|
||||
startedAt: nil,
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "missing job start timestamp: not fresh",
|
||||
hasRecovery: true,
|
||||
createdAt: &after,
|
||||
startedAt: nil,
|
||||
want: false,
|
||||
name: "unmarked, created after start (our own re-render by an older chart): fresh",
|
||||
state: recoveryClusterState{hasRecovery: true, createdAt: &after},
|
||||
appMark: ours,
|
||||
startedAt: &started,
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "unmarked, created after start, values not patched by this restore (manual recovery re-rendered): not fresh",
|
||||
state: recoveryClusterState{hasRecovery: true, createdAt: &after},
|
||||
appMark: "",
|
||||
startedAt: &started,
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "unmarked, created after start, values patched by another restore: not fresh",
|
||||
state: recoveryClusterState{hasRecovery: true, createdAt: &after},
|
||||
appMark: other,
|
||||
startedAt: &started,
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "unmarked, created before start (leftover from previous restore): not fresh",
|
||||
state: recoveryClusterState{hasRecovery: true, createdAt: &before},
|
||||
appMark: ours,
|
||||
startedAt: &started,
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "unmarked, created exactly at start: not fresh (conservative tie -> purge)",
|
||||
state: recoveryClusterState{hasRecovery: true, createdAt: &started},
|
||||
appMark: ours,
|
||||
startedAt: &started,
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "unmarked, missing cluster creation timestamp: not fresh",
|
||||
state: recoveryClusterState{hasRecovery: true},
|
||||
appMark: ours,
|
||||
startedAt: &started,
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "unmarked, missing job start timestamp: not fresh",
|
||||
state: recoveryClusterState{hasRecovery: true, createdAt: &after},
|
||||
appMark: ours,
|
||||
startedAt: nil,
|
||||
want: false,
|
||||
},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
got := cnpgClusterFreshlyRecovered(tc.hasRecovery, tc.createdAt, tc.startedAt)
|
||||
got := cnpgClusterFreshlyRecovered(tc.state, ours, tc.appMark, tc.startedAt)
|
||||
if got != tc.want {
|
||||
t.Errorf("got %v want %v", got, tc.want)
|
||||
}
|
||||
@@ -1095,22 +1135,24 @@ func TestRecoveryBootstrapClusterState_TerminatingCluster(t *testing.T) {
|
||||
}
|
||||
c := newCNPGStrategyTestClient(t, cluster)
|
||||
r := &RestoreJobReconciler{Client: c}
|
||||
got, createdAt, err := r.recoveryBootstrapClusterState(context.Background(), "tenant", "postgres-app")
|
||||
got, err := r.recoveryBootstrapClusterState(context.Background(), "tenant", "postgres-app")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if got {
|
||||
t.Fatalf("expected false for terminating cluster, got true")
|
||||
}
|
||||
if createdAt != nil {
|
||||
t.Fatalf("expected nil createdAt for terminating cluster, got %v", createdAt)
|
||||
if got != (recoveryClusterState{}) {
|
||||
t.Fatalf("expected the zero state for a terminating cluster, got %+v", got)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("live recovery cluster reports recovered with creation timestamp", func(t *testing.T) {
|
||||
created := metav1.NewTime(time.Now().Add(-time.Hour))
|
||||
cluster := &cnpgtypes.Cluster{
|
||||
ObjectMeta: metav1.ObjectMeta{Namespace: "tenant", Name: "postgres-app", CreationTimestamp: created},
|
||||
ObjectMeta: metav1.ObjectMeta{
|
||||
Namespace: "tenant",
|
||||
Name: "postgres-app",
|
||||
CreationTimestamp: created,
|
||||
Annotations: map[string]string{cnpgRestoredServerNameAnnotation: "postgres-app-restore-0123456789abcdef"},
|
||||
},
|
||||
Spec: cnpgtypes.ClusterSpec{
|
||||
Bootstrap: &cnpgtypes.BootstrapConfiguration{
|
||||
Recovery: &cnpgtypes.RecoverySource{Source: "pg-src"},
|
||||
@@ -1119,30 +1161,30 @@ func TestRecoveryBootstrapClusterState_TerminatingCluster(t *testing.T) {
|
||||
}
|
||||
c := newCNPGStrategyTestClient(t, cluster)
|
||||
r := &RestoreJobReconciler{Client: c}
|
||||
got, createdAt, err := r.recoveryBootstrapClusterState(context.Background(), "tenant", "postgres-app")
|
||||
got, err := r.recoveryBootstrapClusterState(context.Background(), "tenant", "postgres-app")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if !got {
|
||||
t.Fatalf("expected true for live recovery cluster, got false")
|
||||
if !got.hasRecovery {
|
||||
t.Fatalf("expected hasRecovery for live recovery cluster, got false")
|
||||
}
|
||||
if createdAt == nil {
|
||||
if got.createdAt == nil {
|
||||
t.Fatalf("expected non-nil createdAt for live recovery cluster")
|
||||
}
|
||||
if got.restoreMark != "postgres-app-restore-0123456789abcdef" {
|
||||
t.Fatalf("restoreMark = %q, want the %s annotation", got.restoreMark, cnpgRestoredServerNameAnnotation)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("missing cluster reports not-yet", func(t *testing.T) {
|
||||
c := newCNPGStrategyTestClient(t)
|
||||
r := &RestoreJobReconciler{Client: c}
|
||||
got, createdAt, err := r.recoveryBootstrapClusterState(context.Background(), "tenant", "postgres-app")
|
||||
got, err := r.recoveryBootstrapClusterState(context.Background(), "tenant", "postgres-app")
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if got {
|
||||
t.Fatalf("expected false for missing cluster, got true")
|
||||
}
|
||||
if createdAt != nil {
|
||||
t.Fatalf("expected nil createdAt for missing cluster, got %v", createdAt)
|
||||
if got != (recoveryClusterState{}) {
|
||||
t.Fatalf("expected the zero state for a missing cluster, got %+v", got)
|
||||
}
|
||||
})
|
||||
}
|
||||
@@ -2729,12 +2771,16 @@ func TestReconcileCNPGRestore_RepeatInPlacePurgesStaleRecoveryCluster(t *testing
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("freshly-recovered cluster from this restore is not re-purged", func(t *testing.T) {
|
||||
t.Run("unmarked cluster this restore's values rendered is not re-purged", func(t *testing.T) {
|
||||
backup := mkBackupArtifact(t)
|
||||
// creationTimestamp a minute after StartedAt: this restore's own re-render.
|
||||
// creationTimestamp a minute after StartedAt: this restore's own
|
||||
// re-render by a chart that does not annotate the Cluster.
|
||||
fresh := metav1.NewTime(startedAt.Add(time.Minute))
|
||||
c := newCNPGStrategyTestClient(t, backup, mkRestoreJob(), strategy, cnpgBackup,
|
||||
newPostgresApp(appName, ns), mkRecoveryCluster(fresh))
|
||||
rjSeed := mkRestoreJob()
|
||||
app := newPostgresApp(appName, ns)
|
||||
app.Spec.Bootstrap.NewServerName = restoredServerName(clusterName, rjSeed.UID)
|
||||
c := newCNPGStrategyTestClient(t, backup, rjSeed, strategy, cnpgBackup,
|
||||
app, mkRecoveryCluster(fresh))
|
||||
r := &RestoreJobReconciler{Client: c, Interface: dynamicfake.NewSimpleDynamicClient(testCNPGScheme(t)), Recorder: record.NewFakeRecorder(10)}
|
||||
|
||||
rj := &backupsv1alpha1.RestoreJob{}
|
||||
@@ -2756,6 +2802,112 @@ func TestReconcileCNPGRestore_RepeatInPlacePurgesStaleRecoveryCluster(t *testing
|
||||
t.Fatalf("freshly-recovered Cluster must not be marked for deletion")
|
||||
}
|
||||
})
|
||||
|
||||
// A manually configured recovery (bootstrap.enabled without
|
||||
// newServerName) renders no annotation. Re-rendered after StartedAt but
|
||||
// before this restore patches the values, it holds the manual recovery
|
||||
// source, not this restore's backup.
|
||||
t.Run("unmarked cluster from values this restore has not patched is purged even when created after start", func(t *testing.T) {
|
||||
backup := mkBackupArtifact(t)
|
||||
fresh := metav1.NewTime(startedAt.Add(time.Minute))
|
||||
app := newPostgresApp(appName, ns)
|
||||
app.Spec.Bootstrap.Enabled = true
|
||||
app.Spec.Bootstrap.OldName = "manual-source"
|
||||
c := newCNPGStrategyTestClient(t, backup, mkRestoreJob(), strategy, cnpgBackup,
|
||||
app, mkRecoveryCluster(fresh), mkClusterPVC())
|
||||
r := &RestoreJobReconciler{Client: c, Interface: dynamicfake.NewSimpleDynamicClient(testCNPGScheme(t)), Recorder: record.NewFakeRecorder(10)}
|
||||
|
||||
rj := &backupsv1alpha1.RestoreJob{}
|
||||
if err := c.Get(ctx, client.ObjectKey{Namespace: ns, Name: "rj"}, rj); err != nil {
|
||||
t.Fatalf("get seeded RestoreJob: %v", err)
|
||||
}
|
||||
if _, err := r.reconcileCNPGRestore(ctx, rj, backup); err != nil {
|
||||
t.Fatalf("reconcileCNPGRestore: %v", err)
|
||||
}
|
||||
err := c.Get(ctx, client.ObjectKey{Namespace: ns, Name: clusterName}, &cnpgtypes.Cluster{})
|
||||
if !apierrors.IsNotFound(err) {
|
||||
t.Fatalf("expected the manual recovery Cluster to be purged (NotFound), got err=%v", err)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("recovery cluster marked by another restore is purged even when created after start", func(t *testing.T) {
|
||||
backup := mkBackupArtifact(t)
|
||||
fresh := metav1.NewTime(startedAt.Add(time.Minute))
|
||||
other := mkRecoveryCluster(fresh)
|
||||
other.Annotations = map[string]string{cnpgRestoredServerNameAnnotation: clusterName + "-restore-0123456789abcdef"}
|
||||
c := newCNPGStrategyTestClient(t, backup, mkRestoreJob(), strategy, cnpgBackup,
|
||||
newPostgresApp(appName, ns), other, mkClusterPVC())
|
||||
r := &RestoreJobReconciler{Client: c, Interface: dynamicfake.NewSimpleDynamicClient(testCNPGScheme(t)), Recorder: record.NewFakeRecorder(10)}
|
||||
|
||||
rj := &backupsv1alpha1.RestoreJob{}
|
||||
if err := c.Get(ctx, client.ObjectKey{Namespace: ns, Name: "rj"}, rj); err != nil {
|
||||
t.Fatalf("get seeded RestoreJob: %v", err)
|
||||
}
|
||||
if _, err := r.reconcileCNPGRestore(ctx, rj, backup); err != nil {
|
||||
t.Fatalf("reconcileCNPGRestore: %v", err)
|
||||
}
|
||||
err := c.Get(ctx, client.ObjectKey{Namespace: ns, Name: clusterName}, &cnpgtypes.Cluster{})
|
||||
if !apierrors.IsNotFound(err) {
|
||||
t.Fatalf("expected another restore's Cluster to be purged (NotFound), got err=%v", err)
|
||||
}
|
||||
})
|
||||
|
||||
// The mark the purge decision expects has to be the one the restore patch
|
||||
// writes into bootstrap.newServerName, which the chart renders onto the
|
||||
// Cluster. The Cluster here predates StartedAt, so only a matching mark
|
||||
// keeps it: the creation-time fallback would purge it.
|
||||
t.Run("recovery cluster carrying the mark this restore's patch wrote is not re-purged", func(t *testing.T) {
|
||||
backup := mkBackupArtifact(t)
|
||||
stale := metav1.NewTime(startedAt.Add(-time.Hour))
|
||||
c := newCNPGStrategyTestClient(t, backup, mkRestoreJob(), strategy, cnpgBackup,
|
||||
newPostgresApp(appName, ns), mkRecoveryCluster(stale), mkClusterPVC())
|
||||
r := &RestoreJobReconciler{Client: c, Interface: dynamicfake.NewSimpleDynamicClient(testCNPGScheme(t)), Recorder: record.NewFakeRecorder(10)}
|
||||
|
||||
rj := &backupsv1alpha1.RestoreJob{}
|
||||
if err := c.Get(ctx, client.ObjectKey{Namespace: ns, Name: "rj"}, rj); err != nil {
|
||||
t.Fatalf("get seeded RestoreJob: %v", err)
|
||||
}
|
||||
if _, err := r.reconcileCNPGRestore(ctx, rj, backup); err != nil {
|
||||
t.Fatalf("first reconcileCNPGRestore: %v", err)
|
||||
}
|
||||
if err := c.Get(ctx, client.ObjectKey{Namespace: ns, Name: clusterName}, &cnpgtypes.Cluster{}); !apierrors.IsNotFound(err) {
|
||||
t.Fatalf("first pass should purge the stale Cluster, got err=%v", err)
|
||||
}
|
||||
app := &postgresapp.Postgres{}
|
||||
if err := c.Get(ctx, client.ObjectKey{Namespace: ns, Name: appName}, app); err != nil {
|
||||
t.Fatalf("get patched Postgres app: %v", err)
|
||||
}
|
||||
mark := app.Spec.Bootstrap.NewServerName
|
||||
if mark == "" {
|
||||
t.Fatalf("the restore patch left bootstrap.newServerName empty")
|
||||
}
|
||||
|
||||
// The chart re-renders the Cluster from the patched values, and the
|
||||
// TargetPurged status write is lost.
|
||||
rendered := mkRecoveryCluster(stale)
|
||||
rendered.Annotations = map[string]string{cnpgRestoredServerNameAnnotation: mark}
|
||||
if err := c.Create(ctx, rendered); err != nil {
|
||||
t.Fatalf("create re-rendered Cluster: %v", err)
|
||||
}
|
||||
if err := c.Get(ctx, client.ObjectKey{Namespace: ns, Name: "rj"}, rj); err != nil {
|
||||
t.Fatalf("reload RestoreJob: %v", err)
|
||||
}
|
||||
apimeta.RemoveStatusCondition(&rj.Status.Conditions, restoreCondTargetPurged)
|
||||
if err := c.Status().Update(ctx, rj); err != nil {
|
||||
t.Fatalf("drop TargetPurged: %v", err)
|
||||
}
|
||||
|
||||
if _, err := r.reconcileCNPGRestore(ctx, rj, backup); err != nil {
|
||||
t.Fatalf("second reconcileCNPGRestore: %v", err)
|
||||
}
|
||||
got := &cnpgtypes.Cluster{}
|
||||
if err := c.Get(ctx, client.ObjectKey{Namespace: ns, Name: clusterName}, got); err != nil {
|
||||
t.Fatalf("expected the Cluster this restore rendered to survive, got err=%v", err)
|
||||
}
|
||||
if !got.DeletionTimestamp.IsZero() {
|
||||
t.Fatalf("the Cluster this restore rendered must not be marked for deletion")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// TestReconcileCNPGRestore_RefusesCrossFlavorRestore: a physical backup only
|
||||
|
||||
@@ -19,6 +19,7 @@ import (
|
||||
ctrl "sigs.k8s.io/controller-runtime"
|
||||
"sigs.k8s.io/controller-runtime/pkg/client"
|
||||
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
|
||||
"sigs.k8s.io/controller-runtime/pkg/controller"
|
||||
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
|
||||
"sigs.k8s.io/controller-runtime/pkg/log"
|
||||
|
||||
@@ -50,9 +51,11 @@ type RestoreJobReconciler struct {
|
||||
readPodLog func(ctx context.Context, namespace, podName, container string) (string, error)
|
||||
CredentialsConfig BackupCredentialsConfig
|
||||
// APIReader is the manager's uncached reader. Reconcile re-reads the
|
||||
// RestoreJob through it before dispatching to a driver, and the
|
||||
// RestoreJob through it before dispatching to a driver, the
|
||||
// keepOriginalPVC rename reads back objects it has just written, which
|
||||
// the cache may not have seen yet. Wired in SetupWithManager.
|
||||
// the cache may not have seen yet, and the concurrent-restore gate must
|
||||
// see a peer's StartedAt before the cache does. Wired in
|
||||
// SetupWithManager.
|
||||
APIReader client.Reader
|
||||
}
|
||||
|
||||
@@ -154,6 +157,19 @@ func (r *RestoreJobReconciler) Reconcile(ctx context.Context, req ctrl.Request)
|
||||
}
|
||||
}
|
||||
|
||||
// Only a job that has not started yet is refused, so a restore already
|
||||
// rewriting the application is never failed by one created after it.
|
||||
if !restoreJobStarted(restoreJob) {
|
||||
ahead, err := restoreAhead(ctx, r.apiReader(), r.Client, restoreJob)
|
||||
if err != nil {
|
||||
return ctrl.Result{}, err
|
||||
}
|
||||
if ahead != "" {
|
||||
return r.markRestoreJobFailedReason(ctx, restoreJob, ConditionReasonConcurrentRestore, fmt.Sprintf(
|
||||
"RestoreJob %s is already restoring the same application; wait for it to finish, then create a new RestoreJob", ahead))
|
||||
}
|
||||
}
|
||||
|
||||
// Step 3: Project the platform-managed S3 credentials into the tenant
|
||||
// namespace so default Strategy CRs (re-rendered at restore time) can
|
||||
// reference a deterministic Secret name. Idempotent / no-op when
|
||||
@@ -213,6 +229,10 @@ func (r *RestoreJobReconciler) SetupWithManager(mgr ctrl.Manager) error {
|
||||
}
|
||||
return ctrl.NewControllerManagedBy(mgr).
|
||||
For(&backupsv1alpha1.RestoreJob{}).
|
||||
// One worker: restoreAhead lets exactly one of two concurrent restores
|
||||
// through only because a peer's StartedAt, written by an earlier
|
||||
// reconcile, is already on the API server when the next one reads.
|
||||
WithOptions(controller.Options{MaxConcurrentReconciles: 1}).
|
||||
Complete(r)
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,83 @@
|
||||
// SPDX-License-Identifier: Apache-2.0
|
||||
package backupcontroller
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
|
||||
"sigs.k8s.io/controller-runtime/pkg/client"
|
||||
|
||||
backupsv1alpha1 "github.com/cozystack/cozystack/api/backups/v1alpha1"
|
||||
)
|
||||
|
||||
// ConditionReasonConcurrentRestore is the Failed reason of a RestoreJob that
|
||||
// was refused because another restore into the same application goes first.
|
||||
const ConditionReasonConcurrentRestore = "ConcurrentRestore"
|
||||
|
||||
// restoreAhead returns the name of the first non-terminal RestoreJob in rj's
|
||||
// namespace that writes into the same application as rj and goes before it,
|
||||
// or "" when there is none.
|
||||
//
|
||||
// Two restores into one application must not interleave: each rewrites the
|
||||
// target from its own Backup, and the CNPG driver cannot tell which of them
|
||||
// produced the live Cluster when both run. The one that goes second fails
|
||||
// instead of waiting, so the application is never restored a second time,
|
||||
// later, by a job its author may no longer expect to run.
|
||||
//
|
||||
// A restore that has started goes before one that has not; between two that
|
||||
// have not, the older goes first, by creation time and then by name. Creation
|
||||
// times have one-second precision, so two jobs created in the same second are
|
||||
// ordered by name. The order is the same from either side.
|
||||
//
|
||||
// The list comes from reader, which must not be the informer cache: a job can
|
||||
// be reconciled before the cache holds a peer's StartedAt, see that peer as
|
||||
// not started, and proceed alongside it. Backups are read through c.
|
||||
func restoreAhead(ctx context.Context, reader client.Reader, c client.Client, rj *backupsv1alpha1.RestoreJob) (string, error) {
|
||||
target, ok, err := restoreJobTarget(ctx, c, rj)
|
||||
if err != nil || !ok {
|
||||
return "", err
|
||||
}
|
||||
list := &backupsv1alpha1.RestoreJobList{}
|
||||
if err := reader.List(ctx, list, client.InNamespace(rj.Namespace)); err != nil {
|
||||
return "", fmt.Errorf("list RestoreJobs: %w", err)
|
||||
}
|
||||
var first *backupsv1alpha1.RestoreJob
|
||||
for i := range list.Items {
|
||||
other := &list.Items[i]
|
||||
if other.Name == rj.Name {
|
||||
continue
|
||||
}
|
||||
if other.Status.Phase == backupsv1alpha1.RestoreJobPhaseSucceeded || other.Status.Phase == backupsv1alpha1.RestoreJobPhaseFailed {
|
||||
continue
|
||||
}
|
||||
if !other.DeletionTimestamp.IsZero() || !restoreGoesFirst(other, rj) {
|
||||
continue
|
||||
}
|
||||
otherTarget, ok, err := restoreJobTarget(ctx, c, other)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
if ok && sameApplication(otherTarget, target) && (first == nil || restoreGoesFirst(other, first)) {
|
||||
first = other
|
||||
}
|
||||
}
|
||||
if first == nil {
|
||||
return "", nil
|
||||
}
|
||||
return first.Name, nil
|
||||
}
|
||||
|
||||
func restoreJobStarted(rj *backupsv1alpha1.RestoreJob) bool {
|
||||
return rj.Status.StartedAt != nil || rj.Status.Phase == backupsv1alpha1.RestoreJobPhaseRunning
|
||||
}
|
||||
|
||||
// restoreGoesFirst reports whether a runs before b; see restoreAhead.
|
||||
func restoreGoesFirst(a, b *backupsv1alpha1.RestoreJob) bool {
|
||||
if sa, sb := restoreJobStarted(a), restoreJobStarted(b); sa != sb {
|
||||
return sa
|
||||
}
|
||||
if !a.CreationTimestamp.Equal(&b.CreationTimestamp) {
|
||||
return a.CreationTimestamp.Before(&b.CreationTimestamp)
|
||||
}
|
||||
return a.Name < b.Name
|
||||
}
|
||||
@@ -0,0 +1,242 @@
|
||||
// SPDX-License-Identifier: Apache-2.0
|
||||
package backupcontroller
|
||||
|
||||
import (
|
||||
"context"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
corev1 "k8s.io/api/core/v1"
|
||||
"k8s.io/apimachinery/pkg/api/meta"
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
"k8s.io/apimachinery/pkg/types"
|
||||
"k8s.io/client-go/tools/record"
|
||||
"sigs.k8s.io/controller-runtime/pkg/client"
|
||||
"sigs.k8s.io/controller-runtime/pkg/reconcile"
|
||||
|
||||
strategyv1alpha1 "github.com/cozystack/cozystack/api/backups/strategy/v1alpha1"
|
||||
backupsv1alpha1 "github.com/cozystack/cozystack/api/backups/v1alpha1"
|
||||
)
|
||||
|
||||
func TestRestoreAhead(t *testing.T) {
|
||||
t0 := metav1.NewTime(time.Date(2026, 10, 1, 12, 0, 0, 0, time.UTC))
|
||||
t1 := metav1.NewTime(t0.Add(time.Minute))
|
||||
started := metav1.NewTime(t0.Add(30 * time.Second))
|
||||
other := pgRef("other")
|
||||
|
||||
restore := func(name string, created metav1.Time, phase backupsv1alpha1.RestoreJobPhase, target *corev1.TypedLocalObjectReference) *backupsv1alpha1.RestoreJob {
|
||||
rj := lockRestore(name, "bk", phase, target, "")
|
||||
rj.CreationTimestamp = created
|
||||
return rj
|
||||
}
|
||||
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
self *backupsv1alpha1.RestoreJob
|
||||
others []client.Object
|
||||
want string
|
||||
}{
|
||||
{
|
||||
name: "no other restore",
|
||||
self: restore("second", t1, "", nil),
|
||||
want: "",
|
||||
},
|
||||
{
|
||||
name: "an older restore that has not started goes first",
|
||||
self: restore("second", t1, "", nil),
|
||||
others: []client.Object{restore("first", t0, backupsv1alpha1.RestoreJobPhasePending, nil)},
|
||||
want: "first",
|
||||
},
|
||||
{
|
||||
name: "a newer restore that has not started does not go first",
|
||||
self: restore("first", t0, "", nil),
|
||||
others: []client.Object{restore("second", t1, "", nil)},
|
||||
want: "",
|
||||
},
|
||||
{
|
||||
name: "a running restore goes first even when created later",
|
||||
self: restore("first", t0, "", nil),
|
||||
others: []client.Object{restore("second", t1, backupsv1alpha1.RestoreJobPhaseRunning, nil)},
|
||||
want: "second",
|
||||
},
|
||||
{
|
||||
name: "a restore with StartedAt goes first even when created later",
|
||||
self: restore("first", t0, "", nil),
|
||||
others: []client.Object{func() client.Object {
|
||||
rj := restore("second", t1, "", nil)
|
||||
rj.Status.StartedAt = &started
|
||||
return rj
|
||||
}()},
|
||||
want: "second",
|
||||
},
|
||||
{
|
||||
name: "an older restore goes first even when its name sorts later",
|
||||
self: restore("alpha", t1, "", nil),
|
||||
others: []client.Object{restore("zeta", t0, "", nil)},
|
||||
want: "zeta",
|
||||
},
|
||||
{
|
||||
name: "a newer restore does not go first even when its name sorts earlier",
|
||||
self: restore("zeta", t0, "", nil),
|
||||
others: []client.Object{restore("alpha", t1, "", nil)},
|
||||
want: "",
|
||||
},
|
||||
{
|
||||
name: "several ahead: names the one that goes first, not the lowest name",
|
||||
self: restore("self", metav1.NewTime(t1.Add(time.Minute)), "", nil),
|
||||
others: []client.Object{
|
||||
restore("alpha", t1, "", nil),
|
||||
restore("zeta", t0, backupsv1alpha1.RestoreJobPhaseRunning, nil),
|
||||
},
|
||||
want: "zeta",
|
||||
},
|
||||
{
|
||||
name: "created at the same time: the lower name goes first",
|
||||
self: restore("b", t0, "", nil),
|
||||
others: []client.Object{restore("a", t0, "", nil)},
|
||||
want: "a",
|
||||
},
|
||||
{
|
||||
name: "created at the same time: a higher name does not go first",
|
||||
self: restore("a", t0, "", nil),
|
||||
others: []client.Object{restore("b", t0, "", nil)},
|
||||
want: "",
|
||||
},
|
||||
{
|
||||
name: "finished restores do not hold",
|
||||
self: restore("third", t1, "", nil),
|
||||
others: []client.Object{
|
||||
restore("done", t0, backupsv1alpha1.RestoreJobPhaseSucceeded, nil),
|
||||
restore("failed", t0, backupsv1alpha1.RestoreJobPhaseFailed, nil),
|
||||
},
|
||||
want: "",
|
||||
},
|
||||
{
|
||||
name: "a restore being deleted does not hold",
|
||||
self: restore("second", t1, "", nil),
|
||||
others: []client.Object{func() client.Object {
|
||||
rj := restore("first", t0, backupsv1alpha1.RestoreJobPhaseRunning, nil)
|
||||
rj.DeletionTimestamp = &t1
|
||||
rj.Finalizers = []string{restoreJobFinalizer}
|
||||
return rj
|
||||
}()},
|
||||
want: "",
|
||||
},
|
||||
{
|
||||
name: "a restore into another application does not hold",
|
||||
self: restore("second", t1, "", nil),
|
||||
others: []client.Object{restore("first", t0, backupsv1alpha1.RestoreJobPhaseRunning, &other)},
|
||||
want: "",
|
||||
},
|
||||
{
|
||||
name: "a restore retargeted onto this job's application holds",
|
||||
self: restore("second", t1, "", &other),
|
||||
others: []client.Object{restore("first", t0, backupsv1alpha1.RestoreJobPhaseRunning, &other)},
|
||||
want: "first",
|
||||
},
|
||||
{
|
||||
name: "the lowest name is reported when several hold",
|
||||
self: restore("third", t1, "", nil),
|
||||
others: []client.Object{restore("zeta", t0, backupsv1alpha1.RestoreJobPhaseRunning, nil), restore("alpha", t0, backupsv1alpha1.RestoreJobPhaseRunning, nil)},
|
||||
want: "alpha",
|
||||
},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
objs := append([]client.Object{lockBackup("bk", pgRef("pg")), tc.self}, tc.others...)
|
||||
c := lockTestClient(t, objs...)
|
||||
got, err := restoreAhead(context.Background(), c, c, tc.self)
|
||||
if err != nil {
|
||||
t.Fatalf("restoreAhead: %v", err)
|
||||
}
|
||||
if got != tc.want {
|
||||
t.Fatalf("restoreAhead = %q, want %q", got, tc.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestRestoreAhead_VeleroIntoAnotherNamespaceIsNotHeld(t *testing.T) {
|
||||
t0 := metav1.NewTime(time.Date(2026, 10, 1, 12, 0, 0, 0, time.UTC))
|
||||
vm := corev1.TypedLocalObjectReference{APIGroup: new("apps.cozystack.io"), Kind: "VMInstance", Name: "vm"}
|
||||
first := lockRestore("first", "bk", backupsv1alpha1.RestoreJobPhaseRunning, nil, "")
|
||||
first.CreationTimestamp = t0
|
||||
second := lockRestore("second", "bk", "", nil, `{"targetNamespace":"tenant-copy"}`)
|
||||
second.CreationTimestamp = metav1.NewTime(t0.Add(time.Minute))
|
||||
c := lockTestClient(t, lockBackupBy("bk", vm, strategyv1alpha1.VeleroStrategyKind), first, second)
|
||||
got, err := restoreAhead(context.Background(), c, c, second)
|
||||
if err != nil {
|
||||
t.Fatalf("restoreAhead: %v", err)
|
||||
}
|
||||
if got != "" {
|
||||
t.Fatalf("restoreAhead = %q, want none: the job writes into another namespace", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestReconcileRestore_ConcurrentRestoreIsRefused(t *testing.T) {
|
||||
t0 := metav1.NewTime(time.Date(2026, 10, 1, 12, 0, 0, 0, time.UTC))
|
||||
first := lockRestore("first", "bk", backupsv1alpha1.RestoreJobPhaseRunning, nil, "")
|
||||
first.CreationTimestamp = t0
|
||||
first.Status.StartedAt = &t0
|
||||
second := lockRestore("second", "bk", "", nil, "")
|
||||
second.CreationTimestamp = metav1.NewTime(t0.Add(time.Minute))
|
||||
c := lockTestClient(t, lockBackup("bk", pgRef("pg")), first, second)
|
||||
r := &RestoreJobReconciler{Client: c, Recorder: record.NewFakeRecorder(10)}
|
||||
|
||||
if _, err := r.Reconcile(context.Background(), reconcile.Request{NamespacedName: types.NamespacedName{Namespace: lockNS, Name: "second"}}); err != nil {
|
||||
t.Fatalf("Reconcile: %v", err)
|
||||
}
|
||||
got := &backupsv1alpha1.RestoreJob{}
|
||||
if err := c.Get(context.Background(), types.NamespacedName{Namespace: lockNS, Name: "second"}, got); err != nil {
|
||||
t.Fatalf("get RestoreJob: %v", err)
|
||||
}
|
||||
if got.Status.Phase != backupsv1alpha1.RestoreJobPhaseFailed {
|
||||
t.Fatalf("Phase = %q, want Failed", got.Status.Phase)
|
||||
}
|
||||
ready := meta.FindStatusCondition(got.Status.Conditions, "Ready")
|
||||
if ready == nil || ready.Reason != ConditionReasonConcurrentRestore {
|
||||
t.Fatalf("Ready condition = %+v, want reason %s", ready, ConditionReasonConcurrentRestore)
|
||||
}
|
||||
if !strings.Contains(got.Status.Message, "first") {
|
||||
t.Fatalf("message %q does not name the restore that goes first", got.Status.Message)
|
||||
}
|
||||
|
||||
running := &backupsv1alpha1.RestoreJob{}
|
||||
if err := c.Get(context.Background(), types.NamespacedName{Namespace: lockNS, Name: "first"}, running); err != nil {
|
||||
t.Fatalf("get RestoreJob: %v", err)
|
||||
}
|
||||
if running.Status.Phase != backupsv1alpha1.RestoreJobPhaseRunning {
|
||||
t.Fatalf("the running restore changed phase to %q", running.Status.Phase)
|
||||
}
|
||||
}
|
||||
|
||||
// The informer cache can lag a peer's status: here it still holds zeta as not
|
||||
// started, while the API server already has its StartedAt. Read from the cache,
|
||||
// alpha would see zeta behind it by name and proceed alongside it.
|
||||
func TestReconcileRestore_ConcurrentRestoreReadsPastTheCache(t *testing.T) {
|
||||
t0 := metav1.NewTime(time.Date(2026, 10, 1, 12, 0, 0, 0, time.UTC))
|
||||
staleZeta := lockRestore("zeta", "bk", "", nil, "")
|
||||
staleZeta.CreationTimestamp = t0
|
||||
alpha := lockRestore("alpha", "bk", "", nil, "")
|
||||
alpha.CreationTimestamp = t0
|
||||
alpha.Finalizers = []string{restoreJobFinalizer}
|
||||
startedZeta := staleZeta.DeepCopy()
|
||||
startedZeta.Status.Phase = backupsv1alpha1.RestoreJobPhaseRunning
|
||||
startedZeta.Status.StartedAt = &t0
|
||||
|
||||
cached := lockTestClient(t, lockBackup("bk", pgRef("pg")), staleZeta, alpha)
|
||||
live := lockTestClient(t, lockBackup("bk", pgRef("pg")), startedZeta, alpha.DeepCopy())
|
||||
r := &RestoreJobReconciler{Client: cached, APIReader: live, Recorder: record.NewFakeRecorder(10)}
|
||||
|
||||
if _, err := r.Reconcile(context.Background(), reconcile.Request{NamespacedName: types.NamespacedName{Namespace: lockNS, Name: "alpha"}}); err != nil {
|
||||
t.Fatalf("Reconcile: %v", err)
|
||||
}
|
||||
got := &backupsv1alpha1.RestoreJob{}
|
||||
if err := cached.Get(context.Background(), types.NamespacedName{Namespace: lockNS, Name: "alpha"}, got); err != nil {
|
||||
t.Fatalf("get RestoreJob: %v", err)
|
||||
}
|
||||
ready := meta.FindStatusCondition(got.Status.Conditions, "Ready")
|
||||
if got.Status.Phase != backupsv1alpha1.RestoreJobPhaseFailed || ready == nil || ready.Reason != ConditionReasonConcurrentRestore {
|
||||
t.Fatalf("alpha: phase %q, Ready %+v; want Failed with reason %s, since zeta has started", got.Status.Phase, ready, ConditionReasonConcurrentRestore)
|
||||
}
|
||||
}
|
||||
@@ -6,6 +6,13 @@ apiVersion: postgresql.cnpg.io/v1
|
||||
kind: Cluster
|
||||
metadata:
|
||||
name: {{ .Release.Name }}
|
||||
{{- with .Values.bootstrap.newServerName }}
|
||||
{{- /* Rendered whether or not the barman plugin is: the CNPG restore driver
|
||||
reads it to tell the Cluster its own restore produced from one rendered
|
||||
for an earlier restore. */}}
|
||||
annotations:
|
||||
postgres.cozystack.io/restored-server-name: {{ . | quote }}
|
||||
{{- end }}
|
||||
spec:
|
||||
{{- if $autoscaling }}
|
||||
{{- /* Under active autoscaling KEDA owns the runtime count via the /scale
|
||||
|
||||
@@ -0,0 +1,66 @@
|
||||
suite: Cluster carries the restore that rendered it
|
||||
|
||||
# The CNPG restore driver sets bootstrap.newServerName to a value unique to each
|
||||
# RestoreJob and reads it back off the Cluster to decide whether the live
|
||||
# recovery Cluster is the one its own purge produced. The barman plugin's
|
||||
# serverName is not enough, since the plugin renders only with chart-level S3
|
||||
# coordinates, so the chart annotates the Cluster whenever the value is set.
|
||||
|
||||
templates:
|
||||
- templates/db.yaml
|
||||
|
||||
release:
|
||||
name: pg-test
|
||||
namespace: tenant-test
|
||||
|
||||
set:
|
||||
_cluster:
|
||||
scheduling: {}
|
||||
|
||||
tests:
|
||||
- it: annotates a restored Cluster with bootstrap.newServerName
|
||||
set:
|
||||
backup:
|
||||
enabled: true
|
||||
useSystemBucket: false
|
||||
s3AccessKey: AKIAEXAMPLE
|
||||
s3SecretKey: s3cr3t
|
||||
destinationPath: s3://bucket/pg-test/
|
||||
endpointURL: https://s3.example:9000
|
||||
bootstrap:
|
||||
enabled: true
|
||||
oldName: pg-test
|
||||
serverName: pg-test
|
||||
newServerName: pg-test-restore-deadbeef
|
||||
documentSelector:
|
||||
path: kind
|
||||
value: Cluster
|
||||
asserts:
|
||||
- equal:
|
||||
path: metadata.annotations["postgres.cozystack.io/restored-server-name"]
|
||||
value: pg-test-restore-deadbeef
|
||||
|
||||
- it: keeps the annotation when the barman plugin is not rendered
|
||||
set:
|
||||
backup:
|
||||
enabled: false
|
||||
bootstrap:
|
||||
enabled: false
|
||||
newServerName: pg-test-restore-deadbeef
|
||||
documentSelector:
|
||||
path: kind
|
||||
value: Cluster
|
||||
asserts:
|
||||
- notExists:
|
||||
path: spec.plugins
|
||||
- equal:
|
||||
path: metadata.annotations["postgres.cozystack.io/restored-server-name"]
|
||||
value: pg-test-restore-deadbeef
|
||||
|
||||
- it: renders no annotation on a Cluster that was never restored
|
||||
documentSelector:
|
||||
path: kind
|
||||
value: Cluster
|
||||
asserts:
|
||||
- notExists:
|
||||
path: metadata.annotations
|
||||
Reference in new issue
Block a user