From a7009c67c7a3e9270e367061509459311da0d5d6 Mon Sep 17 00:00:00 2001 From: Tony Landreth <56887169+tony-landreth@users.noreply.github.com> Date: Fri, 27 Oct 2023 11:52:25 -0400 Subject: [PATCH] Adds a --force-conflicts flag to force annotation updates (#70) Issue: PGO-39 --- docs/content/reference/pgo_backup.md | 9 +++-- docs/content/reference/pgo_restore.md | 5 +++ internal/cmd/backup.go | 24 ++++++++++---- internal/cmd/restore.go | 33 +++++++++++++++---- .../12--backup-with-force-conflicts.yaml | 29 ++++++++++++++++ .../e2e/restore/31--force-conflicts.yaml | 27 +++++++++++++++ 6 files changed, 111 insertions(+), 16 deletions(-) create mode 100644 testing/kuttl/e2e/backup/12--backup-with-force-conflicts.yaml create mode 100644 testing/kuttl/e2e/restore/31--force-conflicts.yaml diff --git a/docs/content/reference/pgo_backup.md b/docs/content/reference/pgo_backup.md index 275282e0..e1cdbf91 100644 --- a/docs/content/reference/pgo_backup.md +++ b/docs/content/reference/pgo_backup.md @@ -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 @@ -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 ``` @@ -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 diff --git a/docs/content/reference/pgo_restore.md b/docs/content/reference/pgo_restore.md index 6f5addb7..863ef732 100644 --- a/docs/content/reference/pgo_restore.md +++ b/docs/content/reference/pgo_restore.md @@ -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 diff --git a/internal/cmd/backup.go b/internal/cmd/backup.go index c3ba7338..8665444c 100644 --- a/internal/cmd/backup.go +++ b/internal/cmd/backup.go @@ -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 @@ -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") @@ -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 { @@ -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( diff --git a/internal/cmd/restore.go b/internal/cmd/restore.go index 70969f17..d608047b 100644 --- a/internal/cmd/restore.go +++ b/internal/cmd/restore.go @@ -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} @@ -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) @@ -119,8 +125,9 @@ postgresclusters/hippo patched`) type pgBackRestRestore struct { *internal.Config - Options []string - RepoName string + Options []string + RepoName string + ForceConflicts bool PostgresCluster string } @@ -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 } @@ -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", diff --git a/testing/kuttl/e2e/backup/12--backup-with-force-conflicts.yaml b/testing/kuttl/e2e/backup/12--backup-with-force-conflicts.yaml new file mode 100644 index 00000000..05c37459 --- /dev/null +++ b/testing/kuttl/e2e/backup/12--backup-with-force-conflicts.yaml @@ -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 diff --git a/testing/kuttl/e2e/restore/31--force-conflicts.yaml b/testing/kuttl/e2e/restore/31--force-conflicts.yaml new file mode 100644 index 00000000..e37bf97b --- /dev/null +++ b/testing/kuttl/e2e/restore/31--force-conflicts.yaml @@ -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