feat: determine backup need based on LastBackupTime

A new sub-object was added to the operator config that reflect a current
status of the backup controller and stores a last time the backup was
executed. This value is used to determine whether a backup of the
workspace is needed or if it already has been executed.

Signed-off-by: Ales Raszka <[email protected]>

Author: Allda

apis/controller/v1alpha1/devworkspaceoperatorconfig_types.go

@@ -349,14 +349,26 @@ type ConfigmapReference struct {
 	Namespace string `json:"namespace"`
 }
 
+type OperatorConfigurationStatus struct {
+	// Conditions represent the latest available observations of the OperatorConfiguration's state
+	Conditions []metav1.Condition `json:"conditions,omitempty"`
+	// LastBackupTime is the timestamp of the last successful backup. Nil if
+	// no backup is configured or no backup has yet succeeded.
+	LastBackupTime *metav1.Time `json:"lastBackupTime,omitempty"`
+}
+
 // DevWorkspaceOperatorConfig is the Schema for the devworkspaceoperatorconfigs API
 // +kubebuilder:object:root=true
+// +kubebuilder:subresource:status
 // +kubebuilder:resource:path=devworkspaceoperatorconfigs,scope=Namespaced,shortName=dwoc
 type DevWorkspaceOperatorConfig struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`
 
 	Config *OperatorConfiguration `json:"config,omitempty"`
+	// Status represents the current status of the DevWorkspaceOperatorConfig
+	// automatically managed by the DevWorkspace Operator.
+	Status *OperatorConfigurationStatus `json:"status,omitempty"`
 }
 
 // DevWorkspaceOperatorConfigList contains a list of DevWorkspaceOperatorConfig

apis/controller/v1alpha1/zz_generated.deepcopy.go

@@ -18,6 +18,7 @@ package controllers
 import (
 	"context"
 
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
@@ -163,8 +164,7 @@ func (r *BackupCronJobReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		return ctrl.Result{}, nil
 	}
 
-	backUpConfig := dwOperatorConfig.Config.Workspace.BackupCronJob
-	r.startCron(ctx, backUpConfig, log)
+	r.startCron(ctx, dwOperatorConfig, log)
 
 	return ctrl.Result{}, nil
 }
@@ -180,7 +180,7 @@ func (r *BackupCronJobReconciler) isBackupEnabled(config *controllerv1alpha1.Dev
 }
 
 // startCron starts the cron scheduler with the backup job according to the provided configuration.
-func (r *BackupCronJobReconciler) startCron(ctx context.Context, req ctrl.Request, backUpConfig *controllerv1alpha1.BackupCronJobConfig, logger logr.Logger) {
+func (r *BackupCronJobReconciler) startCron(ctx context.Context, dwOperatorConfig *controllerv1alpha1.DevWorkspaceOperatorConfig, logger logr.Logger) {
 	log := logger.WithName("backup cron")
 	log.Info("Starting backup cron scheduler")
 
@@ -193,12 +193,13 @@ func (r *BackupCronJobReconciler) startCron(ctx context.Context, req ctrl.Reques
 	}
 
 	// add cronjob task
+	backUpConfig := dwOperatorConfig.Config.Workspace.BackupCronJob
 	log.Info("Adding cronjob task", "schedule", backUpConfig.Schedule)
 	_, err := r.cron.AddFunc(backUpConfig.Schedule, func() {
 		taskLog := logger.WithName("cronTask")
 
 		taskLog.Info("Starting DevWorkspace backup job")
-		if err := r.executeBackupSync(ctx, req, backUpConfig, logger); err != nil {
+		if err := r.executeBackupSync(ctx, dwOperatorConfig, logger); err != nil {
 			taskLog.Error(err, "Failed to execute backup job for DevWorkspaces")
 		}
 		taskLog.Info("DevWorkspace backup job finished")
@@ -224,47 +225,63 @@ func (r *BackupCronJobReconciler) stopCron(logger logr.Logger) {
 	}
 
 	ctx := r.cron.Stop()
-	ctx.Done()
+	<-ctx.Done()
 
 	log.Info("Cron scheduler stopped")
 }
 
 // executeBackupSync executes the backup job for all DevWorkspaces in the cluster that
 // have been stopped in the last N minutes.
-func (r *BackupCronJobReconciler) executeBackupSync(ctx context.Context, req ctrl.Request, backUpConfig *controllerv1alpha1.BackupCronJobConfig, logger logr.Logger) error {
+func (r *BackupCronJobReconciler) executeBackupSync(ctx context.Context, dwOperatorConfig *controllerv1alpha1.DevWorkspaceOperatorConfig, logger logr.Logger) error {
 	log := logger.WithName("executeBackupSync")
 	log.Info("Executing backup sync for all DevWorkspaces")
+	backUpConfig := dwOperatorConfig.Config.Workspace.BackupCronJob
 	devWorkspaces := &dw.DevWorkspaceList{}
 	err := r.List(ctx, devWorkspaces)
 	if err != nil {
 		log.Error(err, "Failed to list DevWorkspaces")
 		return err
 	}
+	var lastBackupTime *metav1.Time
+	if dwOperatorConfig.Status != nil && dwOperatorConfig.Status.LastBackupTime != nil {
+		lastBackupTime = dwOperatorConfig.Status.LastBackupTime
+	}
 	for _, dw := range devWorkspaces.Items {
-		if !r.wasStoppedInTimeRange(&dw, 30, ctx, logger) {
+		if !r.wasStoppedSinceLastBackup(&dw, lastBackupTime, logger) {
 			log.Info("Skipping backup for DevWorkspace that wasn't stopped recently", "namespace", dw.Namespace, "name", dw.Name)
 			continue
 		}
 		dwID := dw.Status.DevWorkspaceId
 		log.Info("Found DevWorkspace", "namespace", dw.Namespace, "devworkspace", dw.Name, "id", dwID)
 
-		if err := r.createBackupJob(&dw, ctx, req, backUpConfig, logger); err != nil {
+		if err := r.createBackupJob(&dw, ctx, backUpConfig, logger); err != nil {
 			log.Error(err, "Failed to create backup Job for DevWorkspace", "id", dwID)
 			continue
 		}
 		log.Info("Backup Job created for DevWorkspace", "id", dwID)
 
 	}
+	origConfig := client.MergeFrom(dwOperatorConfig.DeepCopy())
+	if dwOperatorConfig.Status == nil {
+		dwOperatorConfig.Status = &controllerv1alpha1.OperatorConfigurationStatus{}
+	}
+	dwOperatorConfig.Status.LastBackupTime = &metav1.Time{Time: metav1.Now().Time}
+
+	err = r.Status().Patch(ctx, dwOperatorConfig, origConfig)
+	if err != nil {
+		log.Error(err, "Failed to update DevWorkspaceOperatorConfig status with last backup time")
+		// Not returning error as the backup jobs were created successfully
+	}
 	return nil
 }
 
-// wasStoppedInTimeRange checks if the DevWorkspace was stopped in the last N minutes.
-func (r *BackupCronJobReconciler) wasStoppedInTimeRange(workspace *dw.DevWorkspace, timeRangeInMinute float64, ctx context.Context, logger logr.Logger) bool {
-	log := logger.WithName("wasStoppedInTimeRange")
+// wasStoppedSinceLastBackup checks if the DevWorkspace was stopped since the last backup time.
+func (r *BackupCronJobReconciler) wasStoppedSinceLastBackup(workspace *dw.DevWorkspace, lastBackupTime *metav1.Time, logger logr.Logger) bool {
+	log := logger.WithName("wasStoppedSinceLastBackup")
 	if workspace.Status.Phase != dw.DevWorkspaceStatusStopped {
 		return false
 	}
-	log.Info("DevWorkspace is currently stopped, checking if it was stopped recently", "namespace", workspace.Namespace, "name", workspace.Name)
+	log.Info("DevWorkspace is currently stopped, checking if it was stopped since last backup", "namespace", workspace.Namespace, "name", workspace.Name)
 	// Check if the workspace was stopped in the last N minutes
 	if workspace.Status.Conditions != nil {
 		lastTimeStopped := metav1.Time{}
@@ -273,11 +290,13 @@ func (r *BackupCronJobReconciler) wasStoppedInTimeRange(workspace *dw.DevWorkspa
 				lastTimeStopped = condition.LastTransitionTime
 			}
 		}
-		// Calculate the time difference
 		if !lastTimeStopped.IsZero() {
-			timeDiff := metav1.Now().Sub(lastTimeStopped.Time)
-			if timeDiff.Minutes() <= timeRangeInMinute {
-				log.Info("DevWorkspace was stopped recently", "namespace", workspace.Namespace, "name", workspace.Name)
+			if lastBackupTime == nil {
+				// No previous backup, so consider it stopped since last backup
+				return true
+			}
+			if lastTimeStopped.Time.After(lastBackupTime.Time) {
+				log.Info("DevWorkspace was stopped since last backup", "namespace", workspace.Namespace, "name", workspace.Name)
 				return true
 			}
 		}
@@ -290,7 +309,7 @@ func ptrInt32(i int32) *int32 { return &i }
 func ptrBool(b bool) *bool    { return &b }
 
 // createBackupJob creates a Kubernetes Job to back up the workspace's PVC data.
-func (r *BackupCronJobReconciler) createBackupJob(workspace *dw.DevWorkspace, ctx context.Context, req ctrl.Request, backUpConfig *controllerv1alpha1.BackupCronJobConfig, logger logr.Logger) error {
+func (r *BackupCronJobReconciler) createBackupJob(workspace *dw.DevWorkspace, ctx context.Context, backUpConfig *controllerv1alpha1.BackupCronJobConfig, logger logr.Logger) error {
 	log := logger.WithName("createBackupJob")
 	dwID := workspace.Status.DevWorkspaceId
 
@@ -304,8 +323,12 @@ func (r *BackupCronJobReconciler) createBackupJob(workspace *dw.DevWorkspace, ct
 
 	job := &batchv1.Job{
 		ObjectMeta: metav1.ObjectMeta{
-			GenerateName: "backup-job-",
+			GenerateName: constants.DevWorkspaceBackupJobNamePrefix,
 			Namespace:    workspace.Namespace,
+			Labels: map[string]string{
+				constants.DevWorkspaceIDLabel:        dwID,
+				constants.DevWorkspaceBackupJobLabel: "true",
+			},
 		},
 		Spec: batchv1.JobSpec{
 			Template: corev1.PodTemplateSpec{
@@ -375,6 +398,9 @@ func (r *BackupCronJobReconciler) createBackupJob(workspace *dw.DevWorkspace, ct
 			},
 		},
 	}
+	if err := controllerutil.SetControllerReference(workspace, job, r.Scheme); err != nil {
+		return err
+	}
 	err = r.Create(ctx, job)
 	if err != nil {
 		log.Error(err, "Failed to create backup Job for DevWorkspace", "devworkspace", workspace.Name)

controllers/backupcronjob/backupcronjob_controller.go

@@ -210,7 +210,20 @@ var _ = Describe("BackupCronJobReconciler", func() {
 	})
 
 	Context("executeBackupSync", func() {
-		It("creates a Job for a DevWorkspace stopped within last 30 minutes", func() {
+		It("creates a Job for a DevWorkspace stopped with no previsou backup", func() {
+			enabled := true
+			schedule := "* * * * *"
+			dwoc := &controllerv1alpha1.DevWorkspaceOperatorConfig{
+				ObjectMeta: metav1.ObjectMeta{Name: nameNamespace.Name, Namespace: nameNamespace.Namespace},
+				Config: &controllerv1alpha1.OperatorConfiguration{
+					Workspace: &controllerv1alpha1.WorkspaceConfig{
+						BackupCronJob: &controllerv1alpha1.BackupCronJobConfig{
+							Enable:   &enabled,
+							Schedule: schedule,
+						},
+					},
+				},
+			}
 			dw := createDevWorkspace("dw-recent", "ns-a", false, metav1.NewTime(time.Now().Add(-10*time.Minute)))
 			dw.Status.Phase = dwv2.DevWorkspaceStatusStopped
 			dw.Status.DevWorkspaceId = "id-recent"
@@ -219,14 +232,31 @@ var _ = Describe("BackupCronJobReconciler", func() {
 			pvc := &corev1.PersistentVolumeClaim{ObjectMeta: metav1.ObjectMeta{Name: "claim-devworkspace", Namespace: dw.Namespace}}
 			Expect(fakeClient.Create(ctx, pvc)).To(Succeed())
 
-			Expect(reconciler.executeBackupSync(ctx, log)).To(Succeed())
+			Expect(reconciler.executeBackupSync(ctx, dwoc, log)).To(Succeed())
 
 			jobList := &batchv1.JobList{}
 			Expect(fakeClient.List(ctx, jobList, &client.ListOptions{Namespace: dw.Namespace})).To(Succeed())
 			Expect(jobList.Items).To(HaveLen(1))
 		})
 
 		It("does not create a Job when the DevWorkspace was stopped beyond time range", func() {
+			enabled := true
+			schedule := "* * * * *"
+			lastBackupTime := metav1.NewTime(time.Now().Add(-15 * time.Minute))
+			dwoc := &controllerv1alpha1.DevWorkspaceOperatorConfig{
+				ObjectMeta: metav1.ObjectMeta{Name: nameNamespace.Name, Namespace: nameNamespace.Namespace},
+				Config: &controllerv1alpha1.OperatorConfiguration{
+					Workspace: &controllerv1alpha1.WorkspaceConfig{
+						BackupCronJob: &controllerv1alpha1.BackupCronJobConfig{
+							Enable:   &enabled,
+							Schedule: schedule,
+						},
+					},
+				},
+				Status: &controllerv1alpha1.OperatorConfigurationStatus{
+					LastBackupTime: &lastBackupTime,
+				},
+			}
 			dw := createDevWorkspace("dw-old", "ns-b", false, metav1.NewTime(time.Now().Add(-60*time.Minute)))
 			dw.Status.Phase = dwv2.DevWorkspaceStatusStopped
 			dw.Status.DevWorkspaceId = "id-old"
@@ -235,21 +265,34 @@ var _ = Describe("BackupCronJobReconciler", func() {
 			pvc := &corev1.PersistentVolumeClaim{ObjectMeta: metav1.ObjectMeta{Name: "claim-devworkspace", Namespace: dw.Namespace}}
 			Expect(fakeClient.Create(ctx, pvc)).To(Succeed())
 
-			Expect(reconciler.executeBackupSync(ctx, log)).To(Succeed())
+			Expect(reconciler.executeBackupSync(ctx, dwoc, log)).To(Succeed())
 
 			jobList := &batchv1.JobList{}
 			Expect(fakeClient.List(ctx, jobList, &client.ListOptions{Namespace: dw.Namespace})).To(Succeed())
 			Expect(jobList.Items).To(HaveLen(0))
 		})
 
 		It("does not create a Job for a running DevWorkspace", func() {
+			enabled := true
+			schedule := "* * * * *"
+			dwoc := &controllerv1alpha1.DevWorkspaceOperatorConfig{
+				ObjectMeta: metav1.ObjectMeta{Name: nameNamespace.Name, Namespace: nameNamespace.Namespace},
+				Config: &controllerv1alpha1.OperatorConfiguration{
+					Workspace: &controllerv1alpha1.WorkspaceConfig{
+						BackupCronJob: &controllerv1alpha1.BackupCronJobConfig{
+							Enable:   &enabled,
+							Schedule: schedule,
+						},
+					},
+				},
+			}
 			dw := createDevWorkspace("dw-running", "ns-c", true, metav1.NewTime(time.Now().Add(-5*time.Minute)))
 			Expect(fakeClient.Create(ctx, dw)).To(Succeed())
 
 			pvc := &corev1.PersistentVolumeClaim{ObjectMeta: metav1.ObjectMeta{Name: "claim-devworkspace", Namespace: dw.Namespace}}
 			Expect(fakeClient.Create(ctx, pvc)).To(Succeed())
 
-			Expect(reconciler.executeBackupSync(ctx, log)).To(Succeed())
+			Expect(reconciler.executeBackupSync(ctx, dwoc, log)).To(Succeed())
 
 			jobList := &batchv1.JobList{}
 			Expect(fakeClient.List(ctx, jobList, &client.ListOptions{Namespace: dw.Namespace})).To(Succeed())