Skip to content

Commit

Permalink
Adds a --force-conflicts flag to force annotation updates (#70)
Browse files Browse the repository at this point in the history
Issue: PGO-39
  • Loading branch information
tony-landreth authored Oct 27, 2023
1 parent 9a8503c commit a7009c6
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 16 deletions.
9 changes: 7 additions & 2 deletions docs/content/reference/pgo_backup.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ Backup cluster

### Synopsis

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

### RBAC Requirements
Resources Verbs
Expand All @@ -33,6 +34,9 @@ pgo backup hippo
# on the 'hippo' postgrescluster and trigger a backup
pgo backup hippo --repoName="repo1" --options="--type=full"
# Resolve ownership conflict
pgo backup hippo --force-conflicts
```
### Example output
```
Expand All @@ -42,6 +46,7 @@ postgresclusters/hippo backup initiated
### Options

```
--force-conflicts take ownership and overwrite the backup settings
-h, --help help for backup
--options stringArray options for taking a backup; can be used multiple times
--repoName string repoName to backup to
Expand Down
5 changes: 5 additions & 0 deletions docs/content/reference/pgo_restore.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,16 @@ WARNING: This action is destructive and PostgreSQL will be unavailable while its
Do you want to continue? (yes/no): yes
postgresclusters/hippo patched
# Resolve ownership conflict
pgo restore hippo --force-conflicts
```

### Options

```
--force-conflicts take ownership and overwrite the restore settings
-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
24 changes: 17 additions & 7 deletions internal/cmd/backup.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,10 @@ 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
Long: `Backup allows you to backup a PostgreSQL cluster either by using
the current "spec.backups.pgbackrest.manual" settings on the PostgreSQL cluster
or by overwriting those settings using the flags
or by using flags to write your settings. Overwriting those settings may require
the --force-conflicts flag.
### RBAC Requirements
Resources Verbs
Expand All @@ -55,16 +56,20 @@ pgo backup hippo
# on the 'hippo' postgrescluster and trigger a backup
pgo backup hippo --repoName="repo1" --options="--type=full"
# Resolve ownership conflict
pgo backup hippo --force-conflicts
### Example output
postgresclusters/hippo backup initiated`)

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

// `backup` command accepts `repoName` 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.ForceConflicts, "force-conflicts", false, "take ownership and overwrite the backup settings")
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 @@ -110,12 +115,16 @@ postgresclusters/hippo backup initiated`)

// Update the spec/annotate
// TODO(benjaminjb): Would we want to allow a dry-run option here?
// TODO(benjaminjb): Would we want to allow a force option here?
patchOptions := metav1.PatchOptions{}
if backup.ForceConflicts {
b := true
patchOptions.Force = &b
}
_, err = client.Namespace(configNamespace).Patch(ctx,
args[0], // the name of the cluster object, limited to one name through `ExactArgs(1)`
types.ApplyPatchType,
patch,
config.Patch.PatchOptions(metav1.PatchOptions{}),
config.Patch.PatchOptions(patchOptions),
)

if err != nil {
Expand All @@ -134,8 +143,9 @@ postgresclusters/hippo backup initiated`)
}

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

func (config pgBackRestBackup) modifyIntent(
Expand Down
33 changes: 26 additions & 7 deletions internal/cmd/restore.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,11 @@ WARNING: You are about to restore from pgBackRest with {options:[] repoName:repo
WARNING: This action is destructive and PostgreSQL will be unavailable while its data is restored.
Do you want to continue? (yes/no): yes
postgresclusters/hippo patched`)
postgresclusters/hippo patched
# Resolve ownership conflict
pgo restore hippo --force-conflicts
`)

restore := pgBackRestRestore{Config: config}

Expand All @@ -64,6 +68,8 @@ postgresclusters/hippo patched`)
cmd.Flags().StringVar(&restore.RepoName, "repoName", "",
"repository to restore from")

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

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

Expand Down Expand Up @@ -119,8 +125,9 @@ postgresclusters/hippo patched`)
type pgBackRestRestore struct {
*internal.Config

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

PostgresCluster string
}
Expand Down Expand Up @@ -168,14 +175,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 @@ -191,10 +204,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
29 changes: 29 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,29 @@
---
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
27 changes: 27 additions & 0 deletions testing/kuttl/e2e/restore/31--force-conflicts.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
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 a7009c6

Please sign in to comment.