From 8ce513acbda79c2b88fa03589377f42ceffb6b40 Mon Sep 17 00:00:00 2001 From: Carlisia Date: Thu, 26 Jul 2018 15:31:05 -0700 Subject: [PATCH] Only delete unused backup if they are complete Fixes #705 Signed-off-by: Carlisia --- pkg/controller/backup_sync_controller.go | 9 +++-- pkg/controller/backup_sync_controller_test.go | 33 +++++++++++++++++++ 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/pkg/controller/backup_sync_controller.go b/pkg/controller/backup_sync_controller.go index 1f30e528ee..5b3a482fb1 100644 --- a/pkg/controller/backup_sync_controller.go +++ b/pkg/controller/backup_sync_controller.go @@ -30,6 +30,7 @@ import ( "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/tools/cache" + api "github.com/heptio/ark/pkg/apis/ark/v1" "github.com/heptio/ark/pkg/cloudprovider" arkv1client "github.com/heptio/ark/pkg/generated/clientset/versioned/typed/ark/v1" informers "github.com/heptio/ark/pkg/generated/informers/externalversions/ark/v1" @@ -130,7 +131,8 @@ func (c *backupSyncController) run() { return } -// deleteUnused deletes backup objects from Kubernetes if there is no corresponding backup in the object storage. +// deleteUnused deletes backup objects from Kubernetes if they are complete +// and there is no corresponding backup in the object storage. func (c *backupSyncController) deleteUnused(cloudBackupNames sets.String) { // Backups objects in Kubernetes backups, err := c.backupLister.Backups(c.namespace).List(labels.Everything()) @@ -141,9 +143,10 @@ func (c *backupSyncController) deleteUnused(cloudBackupNames sets.String) { return } - // For each backup object in Kubernetes, verify if has a corresponding backup in the object storage. If not, delete it. + // For each completed backup object in Kubernetes, delete it if it + // does not have a corresponding backup in object storage for _, backup := range backups { - if !cloudBackupNames.Has(backup.Name) { + if backup.Status.Phase == api.BackupPhaseCompleted && !cloudBackupNames.Has(backup.Name) { if err := c.client.Backups(backup.Namespace).Delete(backup.Name, nil); err != nil { c.logger.WithError(errors.WithStack(err)).Error("Error deleting unused backup from Kubernetes") } else { diff --git a/pkg/controller/backup_sync_controller_test.go b/pkg/controller/backup_sync_controller_test.go index e990eb6295..71df0cf8fb 100644 --- a/pkg/controller/backup_sync_controller_test.go +++ b/pkg/controller/backup_sync_controller_test.go @@ -212,6 +212,39 @@ func TestDeleteUnused(t *testing.T) { }, expectedDeletes: sets.NewString(), }, + { + name: "no overlapping backups but including backups that are not complete", + namespace: "ns-1", + cloudBackups: []*v1.Backup{ + arktest.NewTestBackup().WithNamespace("ns-1").WithName("backup-1").Backup, + arktest.NewTestBackup().WithNamespace("ns-1").WithName("backup-2").Backup, + arktest.NewTestBackup().WithNamespace("ns-1").WithName("backup-3").Backup, + }, + k8sBackups: []*arktest.TestBackup{ + arktest.NewTestBackup().WithNamespace("ns-1").WithName("backupA").WithPhase(v1.BackupPhaseCompleted), + arktest.NewTestBackup().WithNamespace("ns-1").WithName("Deleting").WithPhase(v1.BackupPhaseDeleting), + arktest.NewTestBackup().WithNamespace("ns-1").WithName("Failed").WithPhase(v1.BackupPhaseFailed), + arktest.NewTestBackup().WithNamespace("ns-1").WithName("FailedValidation").WithPhase(v1.BackupPhaseFailedValidation), + arktest.NewTestBackup().WithNamespace("ns-1").WithName("InProgress").WithPhase(v1.BackupPhaseInProgress), + arktest.NewTestBackup().WithNamespace("ns-1").WithName("New").WithPhase(v1.BackupPhaseNew), + }, + expectedDeletes: sets.NewString("backupA"), + }, + { + name: "all overlapping backups and all backups that are not complete", + namespace: "ns-1", + cloudBackups: []*v1.Backup{ + arktest.NewTestBackup().WithNamespace("ns-1").WithName("backup-1").Backup, + arktest.NewTestBackup().WithNamespace("ns-1").WithName("backup-2").Backup, + arktest.NewTestBackup().WithNamespace("ns-1").WithName("backup-3").Backup, + }, + k8sBackups: []*arktest.TestBackup{ + arktest.NewTestBackup().WithNamespace("ns-1").WithName("backup-1").WithPhase(v1.BackupPhaseFailed), + arktest.NewTestBackup().WithNamespace("ns-1").WithName("backup-2").WithPhase(v1.BackupPhaseFailedValidation), + arktest.NewTestBackup().WithNamespace("ns-1").WithName("backup-3").WithPhase(v1.BackupPhaseInProgress), + }, + expectedDeletes: sets.NewString(), + }, } for _, test := range tests {