diff --git a/pkg/apis/ark/v1/delete_backup_request.go b/pkg/apis/ark/v1/delete_backup_request.go index cfd1571d9..f6c8d6eac 100644 --- a/pkg/apis/ark/v1/delete_backup_request.go +++ b/pkg/apis/ark/v1/delete_backup_request.go @@ -34,7 +34,10 @@ const ( // DeleteBackupRequestPhaseProcessed means the DeleteBackupRequest has been processed. DeleteBackupRequestPhaseProcessed DeleteBackupRequestPhase = "Processed" + // BackupNameLabel is the label key used by a DeleteBackupRequest to identify its backup by name. BackupNameLabel = "ark.heptio.com/backup-name" + // BackupUIDLabel is the label key used by a DeleteBackupRequest to identify its backup by uid. + BackupUIDLabel = "ark.heptio.com/backup-uid" ) // DeleteBackupRequestStatus is the current status of a DeleteBackupRequest. diff --git a/pkg/backup/delete_helpers.go b/pkg/backup/delete_helpers.go index cf2535189..96215bbde 100644 --- a/pkg/backup/delete_helpers.go +++ b/pkg/backup/delete_helpers.go @@ -23,13 +23,14 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -// NewDeleteBackupRequest creates a DeleteBackupRequest for the backup identified by name. -func NewDeleteBackupRequest(name string) *v1.DeleteBackupRequest { +// NewDeleteBackupRequest creates a DeleteBackupRequest for the backup identified by name and uid. +func NewDeleteBackupRequest(name string, uid string) *v1.DeleteBackupRequest { return &v1.DeleteBackupRequest{ ObjectMeta: metav1.ObjectMeta{ GenerateName: name + "-", Labels: map[string]string{ v1.BackupNameLabel: name, + v1.BackupUIDLabel: uid, }, }, Spec: v1.DeleteBackupRequestSpec{ @@ -39,9 +40,9 @@ func NewDeleteBackupRequest(name string) *v1.DeleteBackupRequest { } // NewDeleteBackupRequestListOptions creates a ListOptions with a label selector configured to -// find DeleteBackupRequests for the backup identified by name. -func NewDeleteBackupRequestListOptions(name string) metav1.ListOptions { +// find DeleteBackupRequests for the backup identified by name and uid. +func NewDeleteBackupRequestListOptions(name, uid string) metav1.ListOptions { return metav1.ListOptions{ - LabelSelector: fmt.Sprintf("%s=%s", v1.BackupNameLabel, name), + LabelSelector: fmt.Sprintf("%s=%s,%s=%s", v1.BackupNameLabel, name, v1.BackupUIDLabel, uid), } } diff --git a/pkg/cmd/cli/backup/delete.go b/pkg/cmd/cli/backup/delete.go index 31aa8702d..cc9bee64c 100644 --- a/pkg/cmd/cli/backup/delete.go +++ b/pkg/cmd/cli/backup/delete.go @@ -23,6 +23,7 @@ import ( "os" "strings" + "github.com/heptio/ark/pkg/apis/ark/v1" "github.com/heptio/ark/pkg/backup" clientset "github.com/heptio/ark/pkg/generated/clientset/versioned" "github.com/spf13/cobra" @@ -33,6 +34,7 @@ import ( "github.com/heptio/ark/pkg/cmd" ) +// NewDeleteCommand creates a new command that deletes a backup. func NewDeleteCommand(f client.Factory, use string) *cobra.Command { o := &DeleteOptions{} @@ -52,31 +54,22 @@ func NewDeleteCommand(f client.Factory, use string) *cobra.Command { return c } +// DeleteOptions contains parameters for deleting a backup. type DeleteOptions struct { Name string Confirm bool client clientset.Interface namespace string + backup *v1.Backup } +// BindFlags binds options for this command to flags. func (o *DeleteOptions) BindFlags(flags *pflag.FlagSet) { flags.BoolVar(&o.Confirm, "confirm", o.Confirm, "Confirm deletion") } -func (o *DeleteOptions) Validate(c *cobra.Command, args []string, f client.Factory) error { - if o.client == nil { - return errors.New("Ark client is not set; unable to proceed") - } - - _, err := o.client.ArkV1().Backups(f.Namespace()).Get(args[0], metav1.GetOptions{}) - if err != nil { - return err - } - - return nil -} - +// Complete fills out the remainder of the parameters based on user input. func (o *DeleteOptions) Complete(f client.Factory, args []string) error { o.Name = args[0] @@ -88,22 +81,42 @@ func (o *DeleteOptions) Complete(f client.Factory, args []string) error { } o.client = client + backup, err := o.client.ArkV1().Backups(f.Namespace()).Get(o.Name, metav1.GetOptions{}) + if err != nil { + return err + } + o.backup = backup + return nil } +// Validate ensures all of the parameters have been filled in correctly. +func (o *DeleteOptions) Validate(c *cobra.Command, args []string, f client.Factory) error { + if o.client == nil { + return errors.New("Ark client is not set; unable to proceed") + } + + if o.backup == nil { + return errors.New("backup is not set; unable to proceed") + } + + return nil +} + +// Run performs the delete backup operation. func (o *DeleteOptions) Run() error { if !o.Confirm && !getConfirmation() { // Don't do anything unless we get confirmation return nil } - deleteRequest := backup.NewDeleteBackupRequest(o.Name) + deleteRequest := backup.NewDeleteBackupRequest(o.backup.Name, string(o.backup.UID)) if _, err := o.client.ArkV1().DeleteBackupRequests(o.namespace).Create(deleteRequest); err != nil { return err } - fmt.Printf("Request to delete backup %q submitted successfully.\nThe backup will be fully deleted after all associated data (disk snapshots, backup files, restores) are removed.\n", o.Name) + fmt.Printf("Request to delete backup %q submitted successfully.\nThe backup will be fully deleted after all associated data (disk snapshots, backup files, restores) are removed.\n", o.backup.Name) return nil } diff --git a/pkg/cmd/cli/backup/describe.go b/pkg/cmd/cli/backup/describe.go index 96c73f9bd..a06f33a59 100644 --- a/pkg/cmd/cli/backup/describe.go +++ b/pkg/cmd/cli/backup/describe.go @@ -55,17 +55,13 @@ func NewDescribeCommand(f client.Factory, use string) *cobra.Command { first := true for _, backup := range backups.Items { - var deleteRequests []v1.DeleteBackupRequest - if backup.Status.Phase == v1.BackupPhaseDeleting { - deleteRequestListOptions := pkgbackup.NewDeleteBackupRequestListOptions(backup.Name) - deleteRequestList, err := arkClient.ArkV1().DeleteBackupRequests(f.Namespace()).List(deleteRequestListOptions) - if err != nil { - fmt.Fprintf(os.Stderr, "error getting DeleteBackupRequests for backup %s: %v\n", backup.Name, err) - } - deleteRequests = deleteRequestList.Items + deleteRequestListOptions := pkgbackup.NewDeleteBackupRequestListOptions(backup.Name, string(backup.UID)) + deleteRequestList, err := arkClient.ArkV1().DeleteBackupRequests(f.Namespace()).List(deleteRequestListOptions) + if err != nil { + fmt.Fprintf(os.Stderr, "error getting DeleteBackupRequests for backup %s: %v\n", backup.Name, err) } - s := output.DescribeBackup(&backup, deleteRequests) + s := output.DescribeBackup(&backup, deleteRequestList.Items) if first { first = false fmt.Print(s) diff --git a/pkg/cmd/util/output/backup_describer.go b/pkg/cmd/util/output/backup_describer.go index 20895f574..850a6659e 100644 --- a/pkg/cmd/util/output/backup_describer.go +++ b/pkg/cmd/util/output/backup_describer.go @@ -34,11 +34,7 @@ func DescribeBackup(backup *v1.Backup, deleteRequests []v1.DeleteBackupRequest) if phase == "" { phase = v1.BackupPhaseNew } - d.Printf("Phase:\t%s", phase) - if count := failedDeletionCount(deleteRequests); count > 0 { - d.Printf(" (%d failed attempt(s))", count) - } - d.Println() + d.Printf("Phase:\t%s\n", phase) d.Println() DescribeBackupSpec(d, backup.Spec) @@ -199,7 +195,11 @@ func DescribeBackupStatus(d *Describer, status v1.BackupStatus) { // DescribeDeleteBackupRequests describes delete backup requests in human-readable format. func DescribeDeleteBackupRequests(d *Describer, requests []v1.DeleteBackupRequest) { - d.Printf("Deletion Attempts:\n") + d.Printf("Deletion Attempts") + if count := failedDeletionCount(requests); count > 0 { + d.Printf(" (%d failed)", count) + } + d.Println(":") started := false for _, req := range requests { diff --git a/pkg/controller/backup_deletion_controller.go b/pkg/controller/backup_deletion_controller.go index b69a28b17..87e4eaaf8 100644 --- a/pkg/controller/backup_deletion_controller.go +++ b/pkg/controller/backup_deletion_controller.go @@ -232,7 +232,7 @@ func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) e // Everything deleted correctly, so we can delete all DeleteBackupRequests for this backup if len(errs) == 0 { - listOptions := pkgbackup.NewDeleteBackupRequestListOptions(backup.Name) + listOptions := pkgbackup.NewDeleteBackupRequestListOptions(backup.Name, string(backup.UID)) err = c.deleteBackupRequestClient.DeleteBackupRequests(req.Namespace).DeleteCollection(nil, listOptions) if err != nil { // If this errors, all we can do is log it. diff --git a/pkg/controller/backup_deletion_controller_test.go b/pkg/controller/backup_deletion_controller_test.go index 09b182ade..46f4d6364 100644 --- a/pkg/controller/backup_deletion_controller_test.go +++ b/pkg/controller/backup_deletion_controller_test.go @@ -42,7 +42,7 @@ import ( ) func TestBackupDeletionControllerControllerHasUpdateFunc(t *testing.T) { - req := pkgbackup.NewDeleteBackupRequest("foo") + req := pkgbackup.NewDeleteBackupRequest("foo", "uid") req.Namespace = "heptio-ark" expected := kube.NamespaceAndName(req) @@ -127,7 +127,7 @@ func TestBackupDeletionControllerProcessQueueItem(t *testing.T) { assert.NoError(t, err) // Already processed - req := pkgbackup.NewDeleteBackupRequest("foo") + req := pkgbackup.NewDeleteBackupRequest("foo", "uid") req.Namespace = "foo" req.Name = "foo-abcde" req.Status.Phase = v1.DeleteBackupRequestPhaseProcessed @@ -179,7 +179,7 @@ func setupBackupDeletionControllerTest(objects ...runtime.Object) *backupDeletio sharedInformers := informers.NewSharedInformerFactory(client, 0) backupService := &arktest.BackupService{} snapshotService := &arktest.FakeSnapshotService{SnapshotsTaken: sets.NewString()} - req := pkgbackup.NewDeleteBackupRequest("foo") + req := pkgbackup.NewDeleteBackupRequest("foo", "uid") data := &backupDeletionControllerTestData{ client: client, @@ -299,6 +299,7 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { t.Run("full delete, no errors", func(t *testing.T) { backup := arktest.NewTestBackup().WithName("foo").WithSnapshot("pv-1", "snap-1").Backup + backup.UID = "uid" restore1 := arktest.NewTestRestore("heptio-ark", "restore-1", v1.RestorePhaseCompleted).WithBackup("foo").Restore restore2 := arktest.NewTestRestore("heptio-ark", "restore-2", v1.RestorePhaseCompleted).WithBackup("foo").Restore @@ -372,7 +373,7 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { core.NewDeleteCollectionAction( v1.SchemeGroupVersion.WithResource("deletebackuprequests"), td.req.Namespace, - pkgbackup.NewDeleteBackupRequestListOptions(td.req.Spec.BackupName), + pkgbackup.NewDeleteBackupRequestListOptions(td.req.Spec.BackupName, "uid"), ), } diff --git a/pkg/controller/gc_controller.go b/pkg/controller/gc_controller.go index 586e4799e..284663d4c 100644 --- a/pkg/controller/gc_controller.go +++ b/pkg/controller/gc_controller.go @@ -132,7 +132,7 @@ func (c *gcController) processQueueItem(key string) error { log.Info("Backup has expired. Creating a DeleteBackupRequest.") - req := pkgbackup.NewDeleteBackupRequest(name) + req := pkgbackup.NewDeleteBackupRequest(backup.Name, string(backup.UID)) _, err = c.deleteBackupRequestClient.DeleteBackupRequests(ns).Create(req) if err != nil {