Split out S3BucketReconcileRequired function

Author: cblecker

pkg/apis/managed/v1alpha1/reconcile.go

@@ -0,0 +1,21 @@
+package v1alpha1
+
+import (
+	"time"
+)
+
+func (i *Velero) S3BucketReconcileRequired(reconcilePeriod time.Duration) bool {
+	// If any of the following are true, reconcile the S3 bucket:
+	// - Name is empty
+	// - Provisioned is false
+	// - The LastSyncTimestamp is unset
+	// - It's been longer than 1 hour since last sync
+	if i.Status.S3Bucket.Name == "" ||
+		!i.Status.S3Bucket.Provisioned ||
+		i.Status.S3Bucket.LastSyncTimestamp.IsZero() ||
+		time.Since(i.Status.S3Bucket.LastSyncTimestamp.Time) > reconcilePeriod {
+		return true
+	}
+
+	return false
+}

pkg/apis/managed/v1alpha1/reconcile_test.go

@@ -0,0 +1,87 @@
+package v1alpha1
+
+import (
+	"testing"
+	"time"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestS3BucketReconcileRequired(t *testing.T) {
+	var testcases = []struct {
+		testName          string
+		bucketName        string
+		bucketProvisioned bool
+		shouldReconcile   bool
+		timestamp         time.Time
+		reconcilePeriod   time.Duration
+	}{
+		{
+			testName:          "default bucket, provisioned 30 mins ago, 60 minute period",
+			bucketName:        "test-bucket",
+			bucketProvisioned: true,
+			timestamp:         time.Now().Add(-time.Minute * 30),
+			reconcilePeriod:   time.Minute * 60,
+			shouldReconcile:   false,
+		},
+		{
+			testName:          "bucket name empty",
+			bucketName:        "",
+			bucketProvisioned: true,
+			timestamp:         time.Now(),
+			reconcilePeriod:   time.Minute * 60,
+			shouldReconcile:   true,
+		},
+		{
+			testName:          "bucket not provioned",
+			bucketName:        "test-bucket",
+			bucketProvisioned: false,
+			timestamp:         time.Now(),
+			reconcilePeriod:   time.Minute * 60,
+			shouldReconcile:   true,
+		},
+		{
+			testName:          "timestamp is epoch",
+			bucketName:        "test-bucket",
+			bucketProvisioned: true,
+			timestamp:         time.Unix(0, 0),
+			reconcilePeriod:   time.Minute * 60,
+			shouldReconcile:   true,
+		},
+		{
+			testName:          "timestamp is unset",
+			bucketName:        "test-bucket",
+			bucketProvisioned: true,
+			reconcilePeriod:   time.Minute * 60,
+			shouldReconcile:   true,
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Logf("Running scenario %q", tc.testName)
+
+		instance := &Velero{
+			Spec: VeleroSpec{},
+			Status: VeleroStatus{
+				S3Bucket: S3Bucket{
+					Name:        tc.bucketName,
+					Provisioned: tc.bucketProvisioned,
+				},
+			},
+		}
+
+		if !tc.timestamp.IsZero() {
+			instance.Status.S3Bucket.LastSyncTimestamp = &metav1.Time{Time: tc.timestamp}
+		}
+
+		reconcile := instance.S3BucketReconcileRequired(tc.reconcilePeriod)
+
+		if reconcile != tc.shouldReconcile {
+			if tc.shouldReconcile {
+				t.Errorf("did not reconcile when expecting one")
+			} else {
+				t.Errorf("reconciled when not expecting one")
+			}
+		}
+	}
+}

pkg/controller/velero/controller.go

@@ -143,15 +143,8 @@ func (r *ReconcileVelero) Reconcile(request reconcile.Request) (reconcile.Result
 		return reconcile.Result{}, err
 	}
 
-	// If any of the following are true, reconcile the S3 bucket:
-	// - Name is empty
-	// - Provisioned is false
-	// - The LastSyncTimestamp is unset
-	// - It's been longer than 1 hour since last sync
-	if instance.Status.S3Bucket.Name == "" ||
-		!instance.Status.S3Bucket.Provisioned ||
-		instance.Status.S3Bucket.LastSyncTimestamp.IsZero() ||
-		time.Since(instance.Status.S3Bucket.LastSyncTimestamp.Time) > s3ReconcilePeriod {
+	// Check if bucket needs to be reconciled
+	if instance.S3BucketReconcileRequired(s3ReconcilePeriod) {
 		// Always directly return from this, as we will either update the
 		// timestamp when complete, or return an error.
 		return r.provisionS3(reqLogger, s3Client, instance)