Update Arango Backup Policy schedule (#458)

Author: ajanikow

pkg/apis/backup/v1alpha/backup_policy_validate.go

@@ -38,7 +38,7 @@ func (a *ArangoBackupPolicy) Validate() error {
 }
 
 func (a *ArangoBackupPolicySpec) Validate() error {
-	if expr, err := cron.Parse(a.Schedule); err != nil {
+	if expr, err := cron.ParseStandard(a.Schedule); err != nil {
 		return fmt.Errorf("error while parsing expr: %s", err.Error())
 	} else if expr.Next(time.Now()).IsZero() {
 		return fmt.Errorf("invalid schedule format")

pkg/backup/handlers/arango/policy/handler.go

@@ -101,17 +101,19 @@ func (h *handler) processBackupPolicy(policy *backupApi.ArangoBackupPolicy) (bac
 		}, nil
 	}
 
-	if policy.Status.Scheduled.IsZero() {
-		expr, err := cron.ParseStandard(policy.Spec.Schedule)
-		if err != nil {
-			h.eventRecorder.Warning(policy, policyError, "Policy Error: %s", err.Error())
+	now := time.Now()
 
-			return backupApi.ArangoBackupPolicyStatus{
-				Message: fmt.Sprintf("error while parsing expr: %s", err.Error()),
-			}, nil
-		}
+	expr, err := cron.ParseStandard(policy.Spec.Schedule)
+	if err != nil {
+		h.eventRecorder.Warning(policy, policyError, "Policy Error: %s", err.Error())
+
+		return backupApi.ArangoBackupPolicyStatus{
+			Message: fmt.Sprintf("error while parsing expr: %s", err.Error()),
+		}, nil
+	}
 
-		next := expr.Next(time.Now())
+	if policy.Status.Scheduled.IsZero() {
+		next := expr.Next(now)
 
 		return backupApi.ArangoBackupPolicyStatus{
 			Scheduled: meta.Time{
@@ -121,7 +123,19 @@ func (h *handler) processBackupPolicy(policy *backupApi.ArangoBackupPolicy) (bac
 	}
 
 	// Check if schedule is required
-	if policy.Status.Scheduled.Unix() > time.Now().Unix() {
+	if policy.Status.Scheduled.Unix() > now.Unix() {
+		// check if we need to update schedule in case that string changed
+		// in other case schedule string will be updated after scheduling objects
+		next := expr.Next(now)
+
+		if next != policy.Status.Scheduled.Time {
+			return backupApi.ArangoBackupPolicyStatus{
+				Scheduled: meta.Time{
+					Time: next,
+				},
+			}, nil
+		}
+
 		return policy.Status, nil
 	}
 
@@ -161,13 +175,6 @@ func (h *handler) processBackupPolicy(policy *backupApi.ArangoBackupPolicy) (bac
 		h.eventRecorder.Normal(policy, backupCreated, "Created ArangoBackup: %s/%s", b.Namespace, b.Name)
 	}
 
-	expr, err := cron.ParseStandard(policy.Spec.Schedule)
-	if err != nil {
-		return backupApi.ArangoBackupPolicyStatus{
-			Message: fmt.Sprintf("error while parsing expr: %s", err.Error()),
-		}, nil
-	}
-
 	next := expr.Next(time.Now())
 
 	h.eventRecorder.Normal(policy, rescheduled, "Rescheduled for: %s", next.String())

pkg/backup/handlers/arango/policy/handler_scheduler_test.go

@@ -201,3 +201,90 @@ func Test_Scheduler_Valid_MultipleObject_Selector(t *testing.T) {
 	require.NotNil(t, backups[1].Spec.PolicyName)
 	require.Equal(t, policy.Name, *backups[1].Spec.PolicyName)
 }
+
+func Test_Reschedule(t *testing.T) {
+	// Arrange
+	handler := newFakeHandler()
+
+	name := string(uuid.NewUUID())
+	namespace := string(uuid.NewUUID())
+
+	selectors := map[string]string{
+		"SELECTOR": string(uuid.NewUUID()),
+	}
+
+	policy := newArangoBackupPolicy("* 13 * * *", namespace, name, selectors, backupApi.ArangoBackupTemplate{})
+
+	// Act
+	createArangoBackupPolicy(t, handler, policy)
+
+	t.Run("First schedule", func(t *testing.T) {
+		require.NoError(t, handler.Handle(newItemFromBackupPolicy(operation.Update, policy)))
+
+		// Assert
+		newPolicy := refreshArangoBackupPolicy(t, handler, policy)
+		require.Empty(t, newPolicy.Status.Message)
+
+		require.Equal(t, 13, newPolicy.Status.Scheduled.Hour())
+	})
+
+	t.Run("First schedule - second iteration", func(t *testing.T) {
+		require.NoError(t, handler.Handle(newItemFromBackupPolicy(operation.Update, policy)))
+
+		// Assert
+		newPolicy := refreshArangoBackupPolicy(t, handler, policy)
+		require.Empty(t, newPolicy.Status.Message)
+
+		require.Equal(t, 13, newPolicy.Status.Scheduled.Hour())
+	})
+
+	t.Run("Change schedule", func(t *testing.T) {
+		policy = refreshArangoBackupPolicy(t, handler, policy)
+		policy.Spec.Schedule = "3 3 * * *"
+		updateArangoBackupPolicy(t, handler, policy)
+
+		require.NoError(t, handler.Handle(newItemFromBackupPolicy(operation.Update, policy)))
+
+		// Assert
+		newPolicy := refreshArangoBackupPolicy(t, handler, policy)
+		require.Empty(t, newPolicy.Status.Message)
+
+		require.Equal(t, 3, newPolicy.Status.Scheduled.Hour())
+		require.Equal(t, 3, newPolicy.Status.Scheduled.Minute())
+	})
+}
+
+func Test_Validate(t *testing.T) {
+	acceptedSchedules := []string{
+		"0 0 * * MON,TUE,WED,THU,FRI",
+		"* * * * *",
+	}
+
+	for _, c := range acceptedSchedules {
+		t.Run(c, func(t *testing.T) {
+			// Arrange
+			handler := newFakeHandler()
+
+			name := string(uuid.NewUUID())
+			namespace := string(uuid.NewUUID())
+
+			selectors := map[string]string{
+				"SELECTOR": string(uuid.NewUUID()),
+			}
+
+			policy := newArangoBackupPolicy(c, namespace, name, selectors, backupApi.ArangoBackupTemplate{})
+
+			require.NoError(t, policy.Validate())
+
+			// Act
+			createArangoBackupPolicy(t, handler, policy)
+
+			require.NoError(t, handler.Handle(newItemFromBackupPolicy(operation.Update, policy)))
+
+			// Assert
+			newPolicy := refreshArangoBackupPolicy(t, handler, policy)
+			require.Empty(t, newPolicy.Status.Message)
+			require.NotEmpty(t, newPolicy.Status.Scheduled)
+		})
+	}
+}

pkg/backup/handlers/arango/policy/handler_suite_test.go

@@ -109,6 +109,13 @@ func createArangoBackupPolicy(t *testing.T, h *handler, policies ...*backupApi.A
 	}
 }
 
+func updateArangoBackupPolicy(t *testing.T, h *handler, policies ...*backupApi.ArangoBackupPolicy) {
+	for _, policy := range policies {
+		_, err := h.client.BackupV1alpha().ArangoBackupPolicies(policy.Namespace).Update(policy)
+		require.NoError(t, err)
+	}
+}
+
 func newArangoDeployment(namespace string, labels map[string]string) *database.ArangoDeployment {
 	name := string(uuid.NewUUID())
 	return &database.ArangoDeployment{