From 9cba95d5231fdcf240b774da1e9a7fe01bb5af21 Mon Sep 17 00:00:00 2001 From: Steve Kriss Date: Fri, 15 Dec 2017 11:15:13 -0800 Subject: [PATCH] update GC controller unit tests Signed-off-by: Steve Kriss --- pkg/controller/gc_controller_test.go | 208 ++++++++++----------------- 1 file changed, 79 insertions(+), 129 deletions(-) diff --git a/pkg/controller/gc_controller_test.go b/pkg/controller/gc_controller_test.go index 273057df8f..40241b4f82 100644 --- a/pkg/controller/gc_controller_test.go +++ b/pkg/controller/gc_controller_test.go @@ -27,20 +27,16 @@ import ( core "k8s.io/client-go/testing" api "github.com/heptio/ark/pkg/apis/ark/v1" - "github.com/heptio/ark/pkg/cloudprovider" "github.com/heptio/ark/pkg/generated/clientset/versioned/fake" informers "github.com/heptio/ark/pkg/generated/informers/externalversions" arktest "github.com/heptio/ark/pkg/util/test" ) type gcTest struct { - name string - backups []*api.Backup - snapshots sets.String - nilSnapshotService bool - - expectedDeletions sets.String - expectedSnapshotsRemaining sets.String + name string + backups []*api.Backup + snapshots sets.String + expectedDeletions sets.String } func TestGarbageCollect(t *testing.T) { @@ -48,7 +44,10 @@ func TestGarbageCollect(t *testing.T) { tests := []gcTest{ { - name: "basic-expired", + name: "no backups results in no deletions", + }, + { + name: "expired backup is deleted", backups: []*api.Backup{ arktest.NewTestBackup().WithName("backup-1"). WithExpiration(fakeClock.Now().Add(-1*time.Second)). @@ -56,12 +55,10 @@ func TestGarbageCollect(t *testing.T) { WithSnapshot("pv-2", "snapshot-2"). Backup, }, - snapshots: sets.NewString("snapshot-1", "snapshot-2"), - expectedDeletions: sets.NewString("backup-1"), - expectedSnapshotsRemaining: sets.NewString(), + expectedDeletions: sets.NewString("backup-1"), }, { - name: "basic-unexpired", + name: "unexpired backup is not deleted", backups: []*api.Backup{ arktest.NewTestBackup().WithName("backup-1"). WithExpiration(fakeClock.Now().Add(1*time.Minute)). @@ -69,12 +66,10 @@ func TestGarbageCollect(t *testing.T) { WithSnapshot("pv-2", "snapshot-2"). Backup, }, - snapshots: sets.NewString("snapshot-1", "snapshot-2"), - expectedDeletions: sets.NewString(), - expectedSnapshotsRemaining: sets.NewString("snapshot-1", "snapshot-2"), + expectedDeletions: sets.NewString(), }, { - name: "one expired, one unexpired", + name: "expired backup is deleted and unexpired backup is not deleted", backups: []*api.Backup{ arktest.NewTestBackup().WithName("backup-1"). WithExpiration(fakeClock.Now().Add(-1*time.Minute)). @@ -87,80 +82,22 @@ func TestGarbageCollect(t *testing.T) { WithSnapshot("pv-4", "snapshot-4"). Backup, }, - snapshots: sets.NewString("snapshot-1", "snapshot-2", "snapshot-3", "snapshot-4"), - expectedDeletions: sets.NewString("backup-1"), - expectedSnapshotsRemaining: sets.NewString("snapshot-3", "snapshot-4"), - }, - { - name: "none expired in target bucket", - backups: []*api.Backup{ - arktest.NewTestBackup().WithName("backup-2"). - WithExpiration(fakeClock.Now().Add(1*time.Minute)). - WithSnapshot("pv-3", "snapshot-3"). - WithSnapshot("pv-4", "snapshot-4"). - Backup, - }, - snapshots: sets.NewString("snapshot-1", "snapshot-2", "snapshot-3", "snapshot-4"), - expectedDeletions: sets.NewString(), - expectedSnapshotsRemaining: sets.NewString("snapshot-1", "snapshot-2", "snapshot-3", "snapshot-4"), - }, - { - name: "orphan snapshots", - backups: []*api.Backup{ - arktest.NewTestBackup().WithName("backup-1"). - WithExpiration(fakeClock.Now().Add(-1*time.Minute)). - WithSnapshot("pv-1", "snapshot-1"). - WithSnapshot("pv-2", "snapshot-2"). - Backup, - }, - snapshots: sets.NewString("snapshot-1", "snapshot-2", "snapshot-3", "snapshot-4"), - expectedDeletions: sets.NewString("backup-1"), - expectedSnapshotsRemaining: sets.NewString("snapshot-3", "snapshot-4"), - }, - { - name: "no snapshot service only GC's backups without snapshots", - backups: []*api.Backup{ - arktest.NewTestBackup().WithName("backup-1"). - WithExpiration(fakeClock.Now().Add(-1*time.Second)). - WithSnapshot("pv-1", "snapshot-1"). - WithSnapshot("pv-2", "snapshot-2"). - Backup, - arktest.NewTestBackup().WithName("backup-2"). - WithExpiration(fakeClock.Now().Add(-1 * time.Second)). - Backup, - }, - snapshots: sets.NewString("snapshot-1", "snapshot-2"), - nilSnapshotService: true, - expectedDeletions: sets.NewString("backup-2"), + expectedDeletions: sets.NewString("backup-1"), }, } for _, test := range tests { - var ( - backupService = &arktest.BackupService{} - snapshotService *arktest.FakeSnapshotService - ) - - if !test.nilSnapshotService { - snapshotService = &arktest.FakeSnapshotService{SnapshotsTaken: test.snapshots} - } - t.Run(test.name, func(t *testing.T) { var ( client = fake.NewSimpleClientset() sharedInformers = informers.NewSharedInformerFactory(client, 0) - snapSvc cloudprovider.SnapshotService bucket = "bucket" logger = arktest.NewLogger() ) - if snapshotService != nil { - snapSvc = snapshotService - } - controller := NewGCController( - backupService, - snapSvc, + nil, + nil, bucket, 1*time.Millisecond, sharedInformers.Ark().V1().Backups(), @@ -171,18 +108,22 @@ func TestGarbageCollect(t *testing.T) { ).(*gcController) controller.clock = fakeClock - backupService.On("GetAllBackups", bucket).Return(test.backups, nil) - for _, b := range test.expectedDeletions.List() { - backupService.On("DeleteBackupDir", bucket, b).Return(nil) + for _, backup := range test.backups { + sharedInformers.Ark().V1().Backups().Informer().GetStore().Add(backup) } - controller.processBackups() - - if !test.nilSnapshotService { - assert.Equal(t, test.expectedSnapshotsRemaining, snapshotService.SnapshotsTaken) + expectedDeletions := make([]core.Action, 0, len(test.expectedDeletions)) + for backup := range test.expectedDeletions { + expectedDeletions = append(expectedDeletions, core.NewDeleteAction( + api.SchemeGroupVersion.WithResource("backups"), + api.DefaultNamespace, + backup, + )) } - backupService.AssertExpectations(t) + controller.run() + + assert.Equal(t, expectedDeletions, client.Actions()) }) } } @@ -191,38 +132,50 @@ func TestGarbageCollectBackup(t *testing.T) { tests := []struct { name string backup *api.Backup - deleteBackupFile bool snapshots sets.String backupFiles sets.String backupMetadataFiles sets.String restores []*api.Restore + nilSnapshotService bool + expectedErr bool expectedRestoreDeletes []string expectedBackupDelete string expectedSnapshots sets.String expectedObjectStorageDeletions sets.String }{ { - name: "deleteBackupFile=false, snapshot deletion fails, don't delete kube backup", + name: "nil snapshot service when backup has snapshots returns error", + backup: arktest.NewTestBackup().WithName("backup-1").WithSnapshot("pv-1", "snap-1").Backup, + nilSnapshotService: true, + expectedErr: true, + }, + { + name: "nil snapshot service when backup doesn't have snapshots does not error", + backup: arktest.NewTestBackup().WithName("backup-1").Backup, + nilSnapshotService: true, + expectedErr: false, + expectedObjectStorageDeletions: sets.NewString("backup-1"), + }, + { + name: "if snapshot deletion fails, don't delete kube backup and return error", backup: arktest.NewTestBackup().WithName("backup-1"). WithSnapshot("pv-1", "snapshot-1"). WithSnapshot("pv-2", "snapshot-2"). Backup, - deleteBackupFile: false, snapshots: sets.NewString("snapshot-1"), expectedSnapshots: sets.NewString(), - expectedObjectStorageDeletions: sets.NewString(), + expectedObjectStorageDeletions: sets.NewString("backup-1"), + expectedErr: true, }, { - name: "related restores should be deleted", - backup: arktest.NewTestBackup().WithName("backup-1").Backup, - deleteBackupFile: true, - snapshots: sets.NewString(), + name: "related restores should be deleted", + backup: arktest.NewTestBackup().WithName("backup-1").Backup, + snapshots: sets.NewString(), restores: []*api.Restore{ arktest.NewTestRestore(api.DefaultNamespace, "restore-1", api.RestorePhaseCompleted).WithBackup("backup-1").Restore, arktest.NewTestRestore(api.DefaultNamespace, "restore-2", api.RestorePhaseCompleted).WithBackup("backup-2").Restore, }, expectedRestoreDeletes: []string{"restore-1"}, - expectedBackupDelete: "backup-1", expectedSnapshots: sets.NewString(), expectedObjectStorageDeletions: sets.NewString("backup-1"), }, @@ -250,6 +203,10 @@ func TestGarbageCollectBackup(t *testing.T) { ).(*gcController) ) + if test.nilSnapshotService { + controller.snapshotService = nil + } + sharedInformers.Ark().V1().Backups().Informer().GetStore().Add(test.backup) for _, restore := range test.restores { sharedInformers.Ark().V1().Restores().Informer().GetStore().Add(restore) @@ -260,12 +217,17 @@ func TestGarbageCollectBackup(t *testing.T) { } // METHOD UNDER TEST - controller.garbageCollectBackup(test.backup, test.deleteBackupFile) + err := controller.garbageCollect(test.backup, logger) // VERIFY: + // error + assert.Equal(t, test.expectedErr, err != nil) + // remaining snapshots - assert.Equal(t, test.expectedSnapshots, snapshotService.SnapshotsTaken) + if !test.nilSnapshotService { + assert.Equal(t, test.expectedSnapshots, snapshotService.SnapshotsTaken) + } expectedActions := make([]core.Action, 0) // Restore client deletes @@ -297,35 +259,20 @@ func TestGarbageCollectBackup(t *testing.T) { func TestGarbageCollectPicksUpBackupUponExpiration(t *testing.T) { var ( - backupService = &arktest.BackupService{} - snapshotService = &arktest.FakeSnapshotService{} fakeClock = clock.NewFakeClock(time.Now()) - assert = assert.New(t) - ) - - scenario := gcTest{ - name: "basic-expired", - backups: []*api.Backup{ - arktest.NewTestBackup().WithName("backup-1"). - WithExpiration(fakeClock.Now().Add(1*time.Second)). - WithSnapshot("pv-1", "snapshot-1"). - WithSnapshot("pv-2", "snapshot-2"). - Backup, - }, - snapshots: sets.NewString("snapshot-1", "snapshot-2"), - } - - snapshotService.SnapshotsTaken = scenario.snapshots - - var ( client = fake.NewSimpleClientset() sharedInformers = informers.NewSharedInformerFactory(client, 0) logger = arktest.NewLogger() + backup = arktest.NewTestBackup().WithName("backup-1"). + WithExpiration(fakeClock.Now().Add(1*time.Second)). + WithSnapshot("pv-1", "snapshot-1"). + WithSnapshot("pv-2", "snapshot-2"). + Backup ) controller := NewGCController( - backupService, - snapshotService, + nil, + nil, "bucket", 1*time.Millisecond, sharedInformers.Ark().V1().Backups(), @@ -336,20 +283,23 @@ func TestGarbageCollectPicksUpBackupUponExpiration(t *testing.T) { ).(*gcController) controller.clock = fakeClock - backupService.On("GetAllBackups", "bucket").Return(scenario.backups, nil) + sharedInformers.Ark().V1().Backups().Informer().GetStore().Add(backup) // PASS 1 - controller.processBackups() - - backupService.AssertExpectations(t) - assert.Equal(scenario.snapshots, snapshotService.SnapshotsTaken, "snapshots should not be garbage-collected yet.") + controller.run() + assert.Equal(t, 0, len(client.Actions())) // PASS 2 - fakeClock.Step(1 * time.Minute) - backupService.On("DeleteBackupDir", "bucket", "backup-1").Return(nil) - controller.processBackups() + expectedActions := []core.Action{ + core.NewDeleteAction( + api.SchemeGroupVersion.WithResource("backups"), + api.DefaultNamespace, + "backup-1", + ), + } - assert.Equal(0, len(snapshotService.SnapshotsTaken), "snapshots should have been garbage-collected.") + fakeClock.Step(1 * time.Minute) + controller.run() - backupService.AssertExpectations(t) + assert.Equal(t, expectedActions, client.Actions()) }