From 1ae492da6cbb1da24d63158b76f203848f502007 Mon Sep 17 00:00:00 2001 From: Andy Goldstein Date: Wed, 4 Apr 2018 11:27:33 -0400 Subject: [PATCH] Expire processed deleted backup requests > 24hr Signed-off-by: Andy Goldstein --- pkg/controller/backup_deletion_controller.go | 40 +++++ .../backup_deletion_controller_test.go | 151 ++++++++++++++++++ 2 files changed, 191 insertions(+) diff --git a/pkg/controller/backup_deletion_controller.go b/pkg/controller/backup_deletion_controller.go index 1fbc8d945..b69a28b17 100644 --- a/pkg/controller/backup_deletion_controller.go +++ b/pkg/controller/backup_deletion_controller.go @@ -18,6 +18,7 @@ package controller import ( "encoding/json" + "time" jsonpatch "github.com/evanphx/json-patch" "github.com/heptio/ark/pkg/apis/ark/v1" @@ -33,6 +34,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/clock" "k8s.io/client-go/tools/cache" ) @@ -49,6 +51,7 @@ type backupDeletionController struct { restoreClient arkv1client.RestoresGetter processRequestFunc func(*v1.DeleteBackupRequest) error + clock clock.Clock } // NewBackupDeletionController creates a new backup deletion controller. @@ -73,6 +76,7 @@ func NewBackupDeletionController( bucket: bucket, restoreLister: restoreInformer.Lister(), restoreClient: restoreClient, + clock: &clock.RealClock{}, } c.syncHandler = c.processQueueItem @@ -86,6 +90,9 @@ func NewBackupDeletionController( }, ) + c.resyncPeriod = time.Hour + c.resyncFunc = c.deleteExpiredRequests + return c } @@ -236,6 +243,39 @@ func (c *backupDeletionController) processRequest(req *v1.DeleteBackupRequest) e return nil } +const deleteBackupRequestMaxAge = 24 * time.Hour + +func (c *backupDeletionController) deleteExpiredRequests() { + c.logger.Info("Checking for expired DeleteBackupRequests") + defer c.logger.Info("Done checking for expired DeleteBackupRequests") + + // Our shared informer factory filters on a single namespace, so asking for all is ok here. + requests, err := c.deleteBackupRequestLister.List(labels.Everything()) + if err != nil { + c.logger.WithError(err).Error("unable to check for expired DeleteBackupRequests") + return + } + + now := c.clock.Now() + + for _, req := range requests { + if req.Status.Phase != v1.DeleteBackupRequestPhaseProcessed { + continue + } + + age := now.Sub(req.CreationTimestamp.Time) + if age >= deleteBackupRequestMaxAge { + reqLog := c.logger.WithFields(logrus.Fields{"namespace": req.Namespace, "name": req.Name}) + reqLog.Info("Deleting expired DeleteBackupRequest") + + err = c.deleteBackupRequestClient.DeleteBackupRequests(req.Namespace).Delete(req.Name, nil) + if err != nil { + reqLog.WithError(err).Error("Error deleting DeleteBackupRequest") + } + } + } +} + func (c *backupDeletionController) patchDeleteBackupRequest(req *v1.DeleteBackupRequest, mutate func(*v1.DeleteBackupRequest)) (*v1.DeleteBackupRequest, error) { // Record original json oldData, err := json.Marshal(req) diff --git a/pkg/controller/backup_deletion_controller_test.go b/pkg/controller/backup_deletion_controller_test.go index be243da4e..09b182ade 100644 --- a/pkg/controller/backup_deletion_controller_test.go +++ b/pkg/controller/backup_deletion_controller_test.go @@ -33,7 +33,9 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/clock" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/watch" core "k8s.io/client-go/testing" @@ -64,6 +66,9 @@ func TestBackupDeletionControllerControllerHasUpdateFunc(t *testing.T) { client.ArkV1(), // restoreClient ).(*backupDeletionController) + // disable resync handler since we don't want to test it here + controller.resyncFunc = nil + keys := make(chan string) controller.syncHandler = func(key string) error { @@ -389,3 +394,149 @@ func TestBackupDeletionControllerProcessRequest(t *testing.T) { assert.Equal(t, 0, td.snapshotService.SnapshotsTaken.Len()) }) } + +func TestBackupDeletionControllerDeleteExpiredRequests(t *testing.T) { + now := time.Date(2018, 4, 4, 12, 0, 0, 0, time.UTC) + unexpired1 := time.Date(2018, 4, 4, 11, 0, 0, 0, time.UTC) + unexpired2 := time.Date(2018, 4, 3, 12, 0, 1, 0, time.UTC) + expired1 := time.Date(2018, 4, 3, 12, 0, 0, 0, time.UTC) + expired2 := time.Date(2018, 4, 3, 2, 0, 0, 0, time.UTC) + + tests := []struct { + name string + requests []*v1.DeleteBackupRequest + expectedDeletions []string + }{ + { + name: "no requests", + }, + { + name: "older than max age, phase = '', don't delete", + requests: []*v1.DeleteBackupRequest{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: "name", + CreationTimestamp: metav1.Time{Time: expired1}, + }, + Status: v1.DeleteBackupRequestStatus{ + Phase: "", + }, + }, + }, + }, + { + name: "older than max age, phase = New, don't delete", + requests: []*v1.DeleteBackupRequest{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: "name", + CreationTimestamp: metav1.Time{Time: expired1}, + }, + Status: v1.DeleteBackupRequestStatus{ + Phase: v1.DeleteBackupRequestPhaseNew, + }, + }, + }, + }, + { + name: "older than max age, phase = InProcess, don't delete", + requests: []*v1.DeleteBackupRequest{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: "name", + CreationTimestamp: metav1.Time{Time: expired1}, + }, + Status: v1.DeleteBackupRequestStatus{ + Phase: v1.DeleteBackupRequestPhaseInProgress, + }, + }, + }, + }, + { + name: "some expired, some not", + requests: []*v1.DeleteBackupRequest{ + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: "unexpired-1", + CreationTimestamp: metav1.Time{Time: unexpired1}, + }, + Status: v1.DeleteBackupRequestStatus{ + Phase: v1.DeleteBackupRequestPhaseProcessed, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: "expired-1", + CreationTimestamp: metav1.Time{Time: expired1}, + }, + Status: v1.DeleteBackupRequestStatus{ + Phase: v1.DeleteBackupRequestPhaseProcessed, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: "unexpired-2", + CreationTimestamp: metav1.Time{Time: unexpired2}, + }, + Status: v1.DeleteBackupRequestStatus{ + Phase: v1.DeleteBackupRequestPhaseProcessed, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: "expired-2", + CreationTimestamp: metav1.Time{Time: expired2}, + }, + Status: v1.DeleteBackupRequestStatus{ + Phase: v1.DeleteBackupRequestPhaseProcessed, + }, + }, + }, + expectedDeletions: []string{"expired-1", "expired-2"}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + client := fake.NewSimpleClientset() + sharedInformers := informers.NewSharedInformerFactory(client, 0) + + controller := NewBackupDeletionController( + arktest.NewLogger(), + sharedInformers.Ark().V1().DeleteBackupRequests(), + client.ArkV1(), // deleteBackupRequestClient + client.ArkV1(), // backupClient + nil, // snapshotService + nil, // backupService + "bucket", + sharedInformers.Ark().V1().Restores(), + client.ArkV1(), // restoreClient + ).(*backupDeletionController) + + fakeClock := &clock.FakeClock{} + fakeClock.SetTime(now) + controller.clock = fakeClock + + for i := range test.requests { + sharedInformers.Ark().V1().DeleteBackupRequests().Informer().GetStore().Add(test.requests[i]) + } + + controller.deleteExpiredRequests() + + expectedActions := []core.Action{} + for _, name := range test.expectedDeletions { + expectedActions = append(expectedActions, core.NewDeleteAction(v1.SchemeGroupVersion.WithResource("deletebackuprequests"), "ns", name)) + } + + assert.Equal(t, expectedActions, client.Actions()) + }) + + } +}