Skip to content

Commit

Permalink
Use force-conflicts instead of overwrite
Browse files Browse the repository at this point in the history
  • Loading branch information
Anthony Landreth committed Oct 26, 2023
1 parent f7c4e7b commit 7cec13c
Show file tree
Hide file tree
Showing 7 changed files with 141 additions and 30 deletions.
12 changes: 6 additions & 6 deletions docs/content/reference/pgo_backup.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ Backup cluster

### Synopsis

Backup allows you to take a backup of a PostgreSQL cluster, either using
the current "spec.backups.pgbackrest.manual" settings on the PostgreSQL cluster
or by overwriting those settings using the flags
Backup allows you to take a backup of a PostgreSQL cluster, using
the current "spec.backups.pgbackrest.manual" settings on the PostgreSQL cluster.
Overwriting those settings requires the --force-conflicts flag.

#### RBAC Requirements
Resources Verbs
Expand All @@ -31,16 +31,16 @@ pgo backup CLUSTER_NAME [flags]
# on the 'hippo' postgrescluster and trigger a backup
pgo backup hippo --repoName="repo1" --options="--type=full"
# When receiving an ownership conflict message
pgo backup hippo --overwrite
# Resolve ownership conflict
pgo backup hippo --force-conflicts
```

### Options

```
--force-conflicts take ownership and overwrite the backup annotation
-h, --help help for backup
--options stringArray options for taking a backup; can be used multiple times
--overwrite overwrite the backup annotation
--repoName string repoName to backup to
```

Expand Down
4 changes: 4 additions & 0 deletions docs/content/reference/pgo_restore.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,15 @@ pgo restore CLUSTER_NAME [flags]
# Restore the 'hippo' cluster to a specific point in time
pgo restore hippo --repoName repo1 --options '--type=time --target="2021-06-09 14:15:11-04"'
# Resolve ownership conflict
pgo restore hippo --force-conflicts
```

### Options

```
--force-conflicts take ownership and overwrite the restore annotation
-h, --help help for restore
--options stringArray options to pass to the "pgbackrest restore" command; can be used multiple times
--repoName string repository to restore from
Expand Down
22 changes: 11 additions & 11 deletions internal/cmd/backup.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ func newBackupCommand(config *internal.Config) *cobra.Command {
cmdBackup := &cobra.Command{
Use: "backup CLUSTER_NAME",
Short: "Backup cluster",
Long: `Backup allows you to take a backup of a PostgreSQL cluster, either using
the current "spec.backups.pgbackrest.manual" settings on the PostgreSQL cluster
or by overwriting those settings using the flags
Long: `Backup allows you to take a backup of a PostgreSQL cluster, using
the current "spec.backups.pgbackrest.manual" settings on the PostgreSQL cluster.
Overwriting those settings requires the --force-conflicts flag.
#### RBAC Requirements
Resources Verbs
Expand All @@ -54,18 +54,18 @@ pgo backup hippo
# on the 'hippo' postgrescluster and trigger a backup
pgo backup hippo --repoName="repo1" --options="--type=full"
# When receiving an ownership conflict message
pgo backup hippo --overwrite
# Resolve ownership conflict
pgo backup hippo --force-conflicts
`)

// Limit the number of args, that is, only one cluster name
cmdBackup.Args = cobra.ExactArgs(1)

// `backup` command accepts `repoName`, `overwrite` and `options` flags;
// `backup` command accepts `repoName`, `force-conflicts` and `options` flags;
// multiple options flags can be used, with each becoming a new line
// in the options array on the spec
backup := pgBackRestBackup{}
cmdBackup.Flags().BoolVar(&backup.Overwrite, "overwrite", false, "overwrite the backup annotation")
cmdBackup.Flags().BoolVar(&backup.ForceConflicts, "force-conflicts", false, "take ownership and overwrite the backup annotation")
cmdBackup.Flags().StringVar(&backup.RepoName, "repoName", "", "repoName to backup to")
cmdBackup.Flags().StringArrayVar(&backup.Options, "options", []string{},
"options for taking a backup; can be used multiple times")
Expand Down Expand Up @@ -112,7 +112,7 @@ pgo backup hippo --overwrite
// Update the spec/annotate
// TODO(benjaminjb): Would we want to allow a dry-run option here?
patchOptions := metav1.PatchOptions{}
if backup.Overwrite {
if backup.ForceConflicts {
b := true
patchOptions.Force = &b
}
Expand All @@ -139,9 +139,9 @@ pgo backup hippo --overwrite
}

type pgBackRestBackup struct {
Options []string
RepoName string
Overwrite bool
Options []string
RepoName string
ForceConflicts bool
}

func (config pgBackRestBackup) modifyIntent(
Expand Down
30 changes: 24 additions & 6 deletions internal/cmd/restore.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ pgo restore hippo --repoName repo1
# Restore the 'hippo' cluster to a specific point in time
pgo restore hippo --repoName repo1 --options '--type=time --target="2021-06-09 14:15:11-04"'
# Resolve ownership conflict
pgo restore hippo --force-conflicts
`)

restore := pgBackRestRestore{Config: config}
Expand All @@ -57,6 +60,8 @@ pgo restore hippo --repoName repo1 --options '--type=time --target="2021-06-09 1
cmd.Flags().StringVar(&restore.RepoName, "repoName", "",
"repository to restore from")

cmd.Flags().BoolVar(&restore.ForceConflicts, "force-conflicts", false, "take ownership and overwrite the restore annotation")

// Only one positional argument: the PostgresCluster name.
cmd.Args = cobra.ExactArgs(1)

Expand Down Expand Up @@ -104,8 +109,9 @@ This is recommended after your restore is complete. Running "pgo restore" will e
type pgBackRestRestore struct {
*internal.Config

Options []string
RepoName string
Options []string
RepoName string
ForceConflicts bool

PostgresCluster string
}
Expand Down Expand Up @@ -153,14 +159,20 @@ func (config pgBackRestRestore) Run(ctx context.Context) error {
if err != nil {
return err
}
patchOptions := metav1.PatchOptions{
DryRun: []string{metav1.DryRunAll},
}

if config.ForceConflicts {
b := true
patchOptions.Force = &b
}

// Perform a dry-run patch to understand what settings will be used should
// the restore proceed.
cluster, err = client.Namespace(namespace).Patch(ctx,
config.PostgresCluster, types.ApplyPatchType, patch,
config.Patch.PatchOptions(metav1.PatchOptions{
DryRun: []string{metav1.DryRunAll},
}))
config.Patch.PatchOptions(patchOptions))
if err != nil {
return err
}
Expand All @@ -176,10 +188,16 @@ func (config pgBackRestRestore) Run(ctx context.Context) error {
return nil
}

patchOptions = metav1.PatchOptions{}
if config.ForceConflicts {
b := true
patchOptions.Force = &b
}

// They agreed to continue. Send the patch again without dry-run.
_, err = client.Namespace(namespace).Patch(ctx,
config.PostgresCluster, types.ApplyPatchType, patch,
config.Patch.PatchOptions(metav1.PatchOptions{}))
config.Patch.PatchOptions(patchOptions))

if err == nil {
fmt.Fprintf(config.Out, "%s/%s patched\n",
Expand Down
43 changes: 43 additions & 0 deletions testing/kuttl/e2e/backup/12--backup-with-force-conflicts.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
---
# The cluster should not have changed.
apiVersion: postgres-operator.crunchydata.com/v1beta1
kind: PostgresCluster
metadata:
name: backup-cluster
spec:
backups:
pgbackrest:
manual:
options:
- --type=full
repoName: repo4

---
apiVersion: kuttl.dev/v1beta1
kind: TestStep
commands:
- script: |
pgbackrest_backup_annotation() {
kubectl get --namespace "${NAMESPACE}" postgrescluster/backup-cluster \
--output 'go-template={{ index .metadata.annotations "postgres-operator.crunchydata.com/pgbackrest-backup" }}'
}
kubectl --namespace "${NAMESPACE}" annotate postgrescluster/backup-cluster \
postgres-operator.crunchydata.com/pgbackrest-backup="$(date)" --overwrite || exit
PRIOR=$(pgbackrest_backup_annotation)
RESULT=$(kubectl-pgo --namespace "${NAMESPACE}" backup backup-cluster --force-conflicts)
CURRENT=$(pgbackrest_backup_annotation)
if [ "${CURRENT}" = "${PRIOR}" ]; then
printf 'Expected annotation to change, got %q' "${CURRENT}"
exit 1
fi
echo "RESULT from taking backup: ${RESULT}"
if [[ -n $RESULT && "$RESULT" == "postgresclusters/backup-cluster backup initiated" ]]; then
exit 0
fi
exit 1
7 changes: 0 additions & 7 deletions testing/kuttl/e2e/backup/12--backup-with-overwrite.yaml

This file was deleted.

53 changes: 53 additions & 0 deletions testing/kuttl/e2e/restore/31--force-conflicts.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
---
apiVersion: postgres-operator.crunchydata.com/v1beta1
kind: PostgresCluster
metadata:
name: restore-cluster
spec:
postgresVersion: 14
instances:
- name: instance1
dataVolumeClaimSpec:
accessModes: [ReadWriteOnce]
resources: { requests: { storage: 1Gi } }
backups:
pgbackrest:
repos:
- name: repo2
volume:
volumeClaimSpec:
accessModes: [ReadWriteOnce]
resources: { requests: { storage: 1Gi } }
restore:
enabled: true
repoName: repo2
options:
- --db-include=restore-cluster

---
apiVersion: kuttl.dev/v1beta1
kind: TestStep
commands:
- script: |
pgbackrest_restore_annotation() {
kubectl --namespace "${NAMESPACE}" get postgrescluster/restore-cluster \
--output "jsonpath-as-json={.metadata.annotations['postgres-operator\.crunchydata\.com/pgbackrest-restore']}"
}
kubectl --namespace "${NAMESPACE}" annotate postgrescluster/restore-cluster \
postgres-operator.crunchydata.com/pgbackrest-restore="$(date)" --overwrite || exit
PRIOR=$(pgbackrest_restore_annotation)
# Running restore will update the annotation.
echo yes | kubectl-pgo --namespace="${NAMESPACE}" restore restore-cluster --options="--db-include=restore-cluster" --repoName="repo2" --force-conflicts
CURRENT=$(pgbackrest_restore_annotation)
if [ "${CURRENT}" != "${PRIOR}" ]; then
exit 0
fi
printf 'Expected annotation to change, got PRIOR %q CURRENT %q' "${PRIOR}" "${CURRENT}"
echo "RESULT from taking restore: ${RESULT}"
exit 1

0 comments on commit 7cec13c

Please sign in to comment.