Expire processed deleted backup requests > 24hr

Signed-off-by: Andy Goldstein <andy.goldstein@gmail.com>

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)

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())
+		})
+
+	}
+}