From 608e6848828e17eb8a3bbf158d4e91e2d05e16ed Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Mon, 22 Dec 2025 15:46:37 +0100 Subject: [PATCH] chore: Handle DevWorkspace backup annotations Signed-off-by: Anatolii Bazko --- .../backupcronjob/backupcronjob_controller.go | 71 ++- .../backupcronjob_controller_test.go | 529 ++++++++++++++++++ .../backupcronjob/backupcronjob_handler.go | 158 ++++++ ...kspace-operator.clusterserviceversion.yaml | 2 + deploy/deployment/kubernetes/combined.yaml | 2 + ...workspace-controller-role.ClusterRole.yaml | 2 + deploy/deployment/openshift/combined.yaml | 2 + ...workspace-controller-role.ClusterRole.yaml | 2 + deploy/templates/components/rbac/role.yaml | 2 + pkg/constants/metadata.go | 13 + 10 files changed, 769 insertions(+), 14 deletions(-) create mode 100644 controllers/backupcronjob/backupcronjob_handler.go diff --git a/controllers/backupcronjob/backupcronjob_controller.go b/controllers/backupcronjob/backupcronjob_controller.go index f07134b33..e4175cef4 100644 --- a/controllers/backupcronjob/backupcronjob_controller.go +++ b/controllers/backupcronjob/backupcronjob_controller.go @@ -18,6 +18,7 @@ package controllers import ( "context" "reflect" + "time" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/reconcile" @@ -40,6 +41,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" @@ -93,7 +95,8 @@ func (r *BackupCronJobReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). Named("BackupCronJob"). - Watches(&controllerv1alpha1.DevWorkspaceOperatorConfig{}, + Watches( + &controllerv1alpha1.DevWorkspaceOperatorConfig{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request { operatorNamespace, err := infrastructure.GetNamespace() // Ignore events from other namespaces @@ -111,17 +114,22 @@ func (r *BackupCronJobReconciler) SetupWithManager(mgr ctrl.Manager) error { }, } }), + builder.WithPredicates(configPredicate), + ). + Watches( + &batchv1.Job{}, + r.getBackupJobEventHandler(), + builder.WithPredicates(r.getBackupJobPredicate()), ). - WithEventFilter(configPredicate). Complete(r) } // +kubebuilder:rbac:groups="",resources=persistentvolumeclaims,verbs=get;list // +kubebuilder:rbac:groups="",resources=secrets,verbs=get;list;create;update;patch;delete // +kubebuilder:rbac:groups="",resources=serviceaccounts;,verbs=get;list;create;update;patch;delete -// +kubebuilder:rbac:groups=batch,resources=jobs,verbs=get;list;create;update;patch;delete +// +kubebuilder:rbac:groups=batch,resources=jobs,verbs=get;list;create;update;patch;delete;watch // +kubebuilder:rbac:groups=controller.devfile.io,resources=devworkspaceoperatorconfigs,verbs=get;list;update;patch;watch -// +kubebuilder:rbac:groups=workspace.devfile.io,resources=devworkspaces,verbs=get;list +// +kubebuilder:rbac:groups=workspace.devfile.io,resources=devworkspaces,verbs=get;list;update;patch // +kubebuilder:rbac:groups=rbac.authorization.k8s.io,resources=clusterrolebindings,verbs=get;list;create;update;patch;delete // +kubebuilder:rbac:groups="",resources=builds,verbs=get // +kubebuilder:rbac:groups="",resources=builds/details,verbs=update @@ -215,7 +223,7 @@ func (r *BackupCronJobReconciler) stopCron(log logr.Logger) { } // executeBackupSync executes the backup job for all DevWorkspaces in the cluster that -// have been stopped in the last N minutes. +// have been stopped since their last backup. func (r *BackupCronJobReconciler) executeBackupSync(ctx context.Context, dwOperatorConfig *controllerv1alpha1.DevWorkspaceOperatorConfig, log logr.Logger) error { log.Info("Executing backup sync for all DevWorkspaces") @@ -264,13 +272,49 @@ func (r *BackupCronJobReconciler) executeBackupSync(ctx context.Context, dwOpera return nil } -// wasStoppedSinceLastBackup checks if the DevWorkspace was stopped since the last backup time. -func (r *BackupCronJobReconciler) wasStoppedSinceLastBackup(workspace *dw.DevWorkspace, lastBackupTime *metav1.Time, log logr.Logger) bool { +// wasStoppedSinceLastBackup checks if the DevWorkspace was stopped since its last backup. +// It reads the last backup time from the DevWorkspace annotation, or falls back to the +// provided globalLastBackupTime if the annotation doesn't exist. +func (r *BackupCronJobReconciler) wasStoppedSinceLastBackup( + workspace *dw.DevWorkspace, + globalLastBackupTime *metav1.Time, + log logr.Logger, +) bool { if workspace.Status.Phase != dw.DevWorkspaceStatusStopped { return false } 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 + + // Get the last backup time and success status from the workspace annotations + var lastBackupTime *metav1.Time + var lastBackupSuccessful bool + if workspace.Annotations != nil { + if lastBackupTimeStr, ok := workspace.Annotations[constants.DevWorkspaceLastBackupTimeAnnotation]; ok { + parsedTime, err := time.Parse(time.RFC3339Nano, lastBackupTimeStr) + if err != nil { + log.Error(err, "Failed to parse last backup time annotation, treating as no previous backup", "value", lastBackupTimeStr) + } else { + lastBackupTime = &metav1.Time{Time: parsedTime} + } + } + + lastBackupSuccessful = workspace.Annotations[constants.DevWorkspaceLastBackupSuccessfulAnnotation] == "true" + } + + // Fall back to globalLastBackupTime if annotation doesn't exist + if lastBackupTime == nil && globalLastBackupTime != nil { + lastBackupTime = globalLastBackupTime + } + + if lastBackupTime == nil { + return true + } + + if !lastBackupSuccessful { + return true + } + + // Check if the workspace was stopped since the last successful backup if workspace.Status.Conditions != nil { lastTimeStopped := metav1.Time{} for _, condition := range workspace.Status.Conditions { @@ -278,17 +322,15 @@ func (r *BackupCronJobReconciler) wasStoppedSinceLastBackup(workspace *dw.DevWor lastTimeStopped = condition.LastTransitionTime } } + if !lastTimeStopped.IsZero() { - 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) + log.Info("DevWorkspace was stopped since last successful backup", "namespace", workspace.Namespace, "name", workspace.Name) return true } } } + return false } @@ -336,6 +378,7 @@ func (r *BackupCronJobReconciler) createBackupJob( Namespace: workspace.Namespace, Labels: map[string]string{ constants.DevWorkspaceIDLabel: dwID, + constants.DevWorkspaceNameLabel: workspace.Name, constants.DevWorkspaceBackupJobLabel: "true", }, }, @@ -532,7 +575,7 @@ func (r *BackupCronJobReconciler) copySecret(ctx context.Context, workspace *dw. } err = r.Create(ctx, namespaceSecret) if err == nil { - log.Info("Sucesfully created secret", "name", namespaceSecret.Name, "namespace", workspace.Namespace) + log.Info("Successfully created secret", "name", namespaceSecret.Name, "namespace", workspace.Namespace) } return namespaceSecret, err } diff --git a/controllers/backupcronjob/backupcronjob_controller_test.go b/controllers/backupcronjob/backupcronjob_controller_test.go index 1f94e54b2..8027cea1c 100644 --- a/controllers/backupcronjob/backupcronjob_controller_test.go +++ b/controllers/backupcronjob/backupcronjob_controller_test.go @@ -400,6 +400,10 @@ var _ = Describe("BackupCronJobReconciler", func() { 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" + // Set successful annotation so the time-based logic is checked + dw.Annotations = map[string]string{ + constants.DevWorkspaceLastBackupSuccessfulAnnotation: "true", + } Expect(fakeClient.Create(ctx, dw)).To(Succeed()) pvc := &corev1.PersistentVolumeClaim{ObjectMeta: metav1.ObjectMeta{Name: "claim-devworkspace", Namespace: dw.Namespace}} @@ -538,6 +542,286 @@ var _ = Describe("BackupCronJobReconciler", func() { Expect(err).ToNot(HaveOccurred()) }) }) + + Context("handleBackupJobStatus", func() { + It("updates DevWorkspace annotations on successful backup job", func() { + dw := createDevWorkspace("dw-success", "ns-success", false, metav1.NewTime(time.Now().Add(-10*time.Minute))) + dw.Status.DevWorkspaceId = "id-success" + Expect(fakeClient.Create(ctx, dw)).To(Succeed()) + + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "backup-job-success", + Namespace: dw.Namespace, + Labels: map[string]string{ + constants.DevWorkspaceIDLabel: dw.Status.DevWorkspaceId, + constants.DevWorkspaceNameLabel: dw.Name, + constants.DevWorkspaceBackupJobLabel: "true", + }, + }, + Status: batchv1.JobStatus{ + Conditions: []batchv1.JobCondition{ + { + Type: batchv1.JobComplete, + Status: corev1.ConditionTrue, + }, + }, + }, + } + Expect(fakeClient.Create(ctx, job)).To(Succeed()) + + err := reconciler.handleBackupJobStatus(ctx, job) + Expect(err).ToNot(HaveOccurred()) + + updatedDw := &dwv2.DevWorkspace{} + Expect(fakeClient.Get(ctx, types.NamespacedName{Name: dw.Name, Namespace: dw.Namespace}, updatedDw)).To(Succeed()) + Expect(updatedDw.Annotations).ToNot(BeNil()) + Expect(updatedDw.Annotations[constants.DevWorkspaceLastBackupSuccessfulAnnotation]).To(Equal("true")) + Expect(updatedDw.Annotations[constants.DevWorkspaceLastBackupTimeAnnotation]).ToNot(BeEmpty()) + Expect(updatedDw.Annotations).ToNot(HaveKey(constants.DevWorkspaceLastBackupErrorAnnotation)) + }) + + It("updates DevWorkspace annotations on failed backup job", func() { + dw := createDevWorkspace("dw-fail", "ns-fail", false, metav1.NewTime(time.Now().Add(-10*time.Minute))) + dw.Status.DevWorkspaceId = "id-fail" + Expect(fakeClient.Create(ctx, dw)).To(Succeed()) + + errorMessage := "Backup failed due to network issue" + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "backup-job-fail", + Namespace: dw.Namespace, + Labels: map[string]string{ + constants.DevWorkspaceIDLabel: dw.Status.DevWorkspaceId, + constants.DevWorkspaceNameLabel: dw.Name, + constants.DevWorkspaceBackupJobLabel: "true", + }, + }, + Status: batchv1.JobStatus{ + Conditions: []batchv1.JobCondition{ + { + Type: batchv1.JobFailed, + Status: corev1.ConditionTrue, + Message: errorMessage, + }, + }, + }, + } + Expect(fakeClient.Create(ctx, job)).To(Succeed()) + + err := reconciler.handleBackupJobStatus(ctx, job) + Expect(err).ToNot(HaveOccurred()) + + updatedDw := &dwv2.DevWorkspace{} + Expect(fakeClient.Get(ctx, types.NamespacedName{Name: dw.Name, Namespace: dw.Namespace}, updatedDw)).To(Succeed()) + Expect(updatedDw.Annotations).ToNot(BeNil()) + Expect(updatedDw.Annotations[constants.DevWorkspaceLastBackupSuccessfulAnnotation]).To(Equal("false")) + Expect(updatedDw.Annotations[constants.DevWorkspaceLastBackupTimeAnnotation]).ToNot(BeEmpty()) + Expect(updatedDw.Annotations[constants.DevWorkspaceLastBackupErrorAnnotation]).To(Equal(errorMessage)) + }) + + It("truncates error message if it exceeds maximum length", func() { + dw := createDevWorkspace("dw-long-error", "ns-long-error", false, metav1.NewTime(time.Now().Add(-10*time.Minute))) + dw.Status.DevWorkspaceId = "id-long-error" + Expect(fakeClient.Create(ctx, dw)).To(Succeed()) + + // Create an error message longer than 1024 characters + longErrorMessage := "" + for i := 0; i < 1100; i++ { + longErrorMessage += "x" + } + + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "backup-job-long-error", + Namespace: dw.Namespace, + Labels: map[string]string{ + constants.DevWorkspaceIDLabel: dw.Status.DevWorkspaceId, + constants.DevWorkspaceNameLabel: dw.Name, + constants.DevWorkspaceBackupJobLabel: "true", + }, + }, + Status: batchv1.JobStatus{ + Conditions: []batchv1.JobCondition{ + { + Type: batchv1.JobFailed, + Status: corev1.ConditionTrue, + Message: longErrorMessage, + }, + }, + }, + } + Expect(fakeClient.Create(ctx, job)).To(Succeed()) + + err := reconciler.handleBackupJobStatus(ctx, job) + Expect(err).ToNot(HaveOccurred()) + + updatedDw := &dwv2.DevWorkspace{} + Expect(fakeClient.Get(ctx, types.NamespacedName{Name: dw.Name, Namespace: dw.Namespace}, updatedDw)).To(Succeed()) + Expect(updatedDw.Annotations).ToNot(BeNil()) + Expect(updatedDw.Annotations[constants.DevWorkspaceLastBackupErrorAnnotation]).To(HaveLen(1024)) + Expect(updatedDw.Annotations[constants.DevWorkspaceLastBackupErrorAnnotation]).To(HaveSuffix("...")) + }) + + It("does nothing when job has no completed or failed conditions", func() { + dw := createDevWorkspace("dw-pending", "ns-pending", false, metav1.NewTime(time.Now().Add(-10*time.Minute))) + dw.Status.DevWorkspaceId = "id-pending" + Expect(fakeClient.Create(ctx, dw)).To(Succeed()) + + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "backup-job-pending", + Namespace: dw.Namespace, + Labels: map[string]string{ + constants.DevWorkspaceIDLabel: dw.Status.DevWorkspaceId, + constants.DevWorkspaceNameLabel: dw.Name, + constants.DevWorkspaceBackupJobLabel: "true", + }, + }, + Status: batchv1.JobStatus{ + Conditions: []batchv1.JobCondition{}, + }, + } + Expect(fakeClient.Create(ctx, job)).To(Succeed()) + + err := reconciler.handleBackupJobStatus(ctx, job) + Expect(err).ToNot(HaveOccurred()) + + updatedDw := &dwv2.DevWorkspace{} + Expect(fakeClient.Get(ctx, types.NamespacedName{Name: dw.Name, Namespace: dw.Namespace}, updatedDw)).To(Succeed()) + Expect(updatedDw.Annotations).To(BeNil()) + }) + + It("returns error when DevWorkspace is not found", func() { + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "backup-job-no-workspace", + Namespace: "ns-no-workspace", + Labels: map[string]string{ + constants.DevWorkspaceIDLabel: "id-no-workspace", + constants.DevWorkspaceNameLabel: "dw-no-workspace", + constants.DevWorkspaceBackupJobLabel: "true", + }, + }, + Status: batchv1.JobStatus{ + Conditions: []batchv1.JobCondition{ + { + Type: batchv1.JobComplete, + Status: corev1.ConditionTrue, + }, + }, + }, + } + + err := reconciler.handleBackupJobStatus(ctx, job) + Expect(err).To(HaveOccurred()) + }) + + It("returns error when DevWorkspace name label is missing from job", func() { + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "backup-job-no-label", + Namespace: "ns-no-label", + Labels: map[string]string{ + constants.DevWorkspaceIDLabel: "id-no-label", + constants.DevWorkspaceBackupJobLabel: "true", + }, + }, + Status: batchv1.JobStatus{ + Conditions: []batchv1.JobCondition{ + { + Type: batchv1.JobComplete, + Status: corev1.ConditionTrue, + }, + }, + }, + } + + err := reconciler.handleBackupJobStatus(ctx, job) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("DevWorkspace name label not found")) + }) + }) + + Context("copySecret", func() { + It("creates a new secret in workspace namespace", func() { + dw := createDevWorkspace("dw-copy-secret", "ns-copy-secret", false, metav1.NewTime(time.Now().Add(-10*time.Minute))) + dw.Status.DevWorkspaceId = "id-copy-secret" + Expect(fakeClient.Create(ctx, dw)).To(Succeed()) + + sourceSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "source-secret", + Namespace: nameNamespace.Namespace, + }, + Data: map[string][]byte{ + ".dockerconfigjson": []byte(`{"auths":{"registry.example.com":{"auth":"dGVzdDp0ZXN0"}}}`), + }, + Type: corev1.SecretTypeDockerConfigJson, + } + + copiedSecret, err := reconciler.copySecret(ctx, dw, sourceSecret, log) + Expect(err).ToNot(HaveOccurred()) + Expect(copiedSecret).ToNot(BeNil()) + Expect(copiedSecret.Name).To(Equal(constants.DevWorkspaceBackupAuthSecretName)) + Expect(copiedSecret.Namespace).To(Equal(dw.Namespace)) + Expect(copiedSecret.Data).To(Equal(sourceSecret.Data)) + Expect(copiedSecret.Type).To(Equal(sourceSecret.Type)) + Expect(copiedSecret.Labels).To(HaveKeyWithValue(constants.DevWorkspaceIDLabel, dw.Status.DevWorkspaceId)) + Expect(copiedSecret.Labels).To(HaveKeyWithValue(constants.DevWorkspaceWatchSecretLabel, "true")) + + // Verify the secret was actually created + createdSecret := &corev1.Secret{} + err = fakeClient.Get(ctx, types.NamespacedName{ + Name: constants.DevWorkspaceBackupAuthSecretName, + Namespace: dw.Namespace, + }, createdSecret) + Expect(err).ToNot(HaveOccurred()) + }) + + It("deletes existing secret before creating new one", func() { + dw := createDevWorkspace("dw-replace-secret", "ns-replace-secret", false, metav1.NewTime(time.Now().Add(-10*time.Minute))) + dw.Status.DevWorkspaceId = "id-replace-secret" + Expect(fakeClient.Create(ctx, dw)).To(Succeed()) + + // Create an existing secret + existingSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: constants.DevWorkspaceBackupAuthSecretName, + Namespace: dw.Namespace, + }, + Data: map[string][]byte{ + ".dockerconfigjson": []byte(`{"old":"data"}`), + }, + Type: corev1.SecretTypeDockerConfigJson, + } + Expect(fakeClient.Create(ctx, existingSecret)).To(Succeed()) + + sourceSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "new-source-secret", + Namespace: nameNamespace.Namespace, + }, + Data: map[string][]byte{ + ".dockerconfigjson": []byte(`{"new":"data"}`), + }, + Type: corev1.SecretTypeDockerConfigJson, + } + + copiedSecret, err := reconciler.copySecret(ctx, dw, sourceSecret, log) + Expect(err).ToNot(HaveOccurred()) + Expect(copiedSecret.Data).To(Equal(sourceSecret.Data)) + + // Verify the secret has the new data + updatedSecret := &corev1.Secret{} + err = fakeClient.Get(ctx, types.NamespacedName{ + Name: constants.DevWorkspaceBackupAuthSecretName, + Namespace: dw.Namespace, + }, updatedSecret) + Expect(err).ToNot(HaveOccurred()) + Expect(updatedSecret.Data).To(Equal(sourceSecret.Data)) + }) + }) Context("wasStoppedSinceLastBackup", func() { It("returns true if DevWorkspace was stopped since last backup", func() { lastBackupTime := metav1.NewTime(time.Now().Add(-30 * time.Minute)) @@ -551,6 +835,10 @@ var _ = Describe("BackupCronJobReconciler", func() { lastBackupTime := metav1.NewTime(time.Now().Add(-5 * time.Minute)) workspaceStoppedTime := metav1.NewTime(time.Now().Add(-10 * time.Minute)) dw := createDevWorkspace("dw-test", "ns-test", false, workspaceStoppedTime) + // Set successful annotation so the time-based logic is checked + dw.Annotations = map[string]string{ + constants.DevWorkspaceLastBackupSuccessfulAnnotation: "true", + } result := reconciler.wasStoppedSinceLastBackup(dw, &lastBackupTime, log) Expect(result).To(BeFalse()) }) @@ -566,6 +854,62 @@ var _ = Describe("BackupCronJobReconciler", func() { result := reconciler.wasStoppedSinceLastBackup(dw, &lastBackupTime, log) Expect(result).To(BeFalse()) }) + + It("uses workspace annotation for last backup time if present", func() { + workspaceStoppedTime := metav1.NewTime(time.Now().Add(-10 * time.Minute)) + annotationBackupTime := metav1.NewTime(time.Now().Add(-20 * time.Minute)) + dw := createDevWorkspace("dw-test-annotation", "ns-test-annotation", false, workspaceStoppedTime) + dw.Annotations = map[string]string{ + constants.DevWorkspaceLastBackupTimeAnnotation: annotationBackupTime.Format(time.RFC3339Nano), + constants.DevWorkspaceLastBackupSuccessfulAnnotation: "true", + } + + result := reconciler.wasStoppedSinceLastBackup(dw, nil, log) + Expect(result).To(BeTrue()) + }) + + It("returns true if last backup was unsuccessful", func() { + workspaceStoppedTime := metav1.NewTime(time.Now().Add(-30 * time.Minute)) + annotationBackupTime := metav1.NewTime(time.Now().Add(-10 * time.Minute)) + dw := createDevWorkspace("dw-test-unsuccessful", "ns-test-unsuccessful", false, workspaceStoppedTime) + dw.Annotations = map[string]string{ + constants.DevWorkspaceLastBackupTimeAnnotation: annotationBackupTime.Format(time.RFC3339Nano), + constants.DevWorkspaceLastBackupSuccessfulAnnotation: "false", + } + + result := reconciler.wasStoppedSinceLastBackup(dw, nil, log) + Expect(result).To(BeTrue()) + }) + + It("handles invalid time format in annotation gracefully", func() { + workspaceStoppedTime := metav1.NewTime(time.Now().Add(-10 * time.Minute)) + dw := createDevWorkspace("dw-test-invalid-time", "ns-test-invalid-time", false, workspaceStoppedTime) + dw.Annotations = map[string]string{ + constants.DevWorkspaceLastBackupTimeAnnotation: "invalid-time-format", + constants.DevWorkspaceLastBackupSuccessfulAnnotation: "true", + } + + // Should fall back to treating as no previous backup + result := reconciler.wasStoppedSinceLastBackup(dw, nil, log) + Expect(result).To(BeTrue()) + }) + + It("prefers workspace annotation over global last backup time", func() { + globalLastBackupTime := metav1.NewTime(time.Now().Add(-5 * time.Minute)) + workspaceStoppedTime := metav1.NewTime(time.Now().Add(-10 * time.Minute)) + annotationBackupTime := metav1.NewTime(time.Now().Add(-20 * time.Minute)) + + dw := createDevWorkspace("dw-test-prefer-annotation", "ns-test-prefer-annotation", false, workspaceStoppedTime) + dw.Annotations = map[string]string{ + constants.DevWorkspaceLastBackupTimeAnnotation: annotationBackupTime.Format(time.RFC3339Nano), + constants.DevWorkspaceLastBackupSuccessfulAnnotation: "true", + } + + // With annotation time (-20min), workspace stopped at -10min, so should return true + // With global time (-5min), workspace stopped at -10min, so would return false + result := reconciler.wasStoppedSinceLastBackup(dw, &globalLastBackupTime, log) + Expect(result).To(BeTrue()) + }) }) }) @@ -613,6 +957,191 @@ func createAuthSecret(name, namespace string, data map[string][]byte) *corev1.Se } } +var _ = Describe("getBackupJobPredicate Tests", func() { + var ( + reconciler BackupCronJobReconciler + backupJobPredicate predicate.Funcs + ) + + BeforeEach(func() { + scheme := runtime.NewScheme() + Expect(batchv1.AddToScheme(scheme)).To(Succeed()) + + reconciler = BackupCronJobReconciler{ + Log: zap.New(zap.UseDevMode(true)).WithName("BackupJobPredicateTest"), + Scheme: scheme, + } + backupJobPredicate = reconciler.getBackupJobPredicate() + }) + + Context("UpdateFunc", func() { + It("returns true for backup job with required labels", func() { + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "backup-job", + Namespace: "test-namespace", + Labels: map[string]string{ + constants.DevWorkspaceBackupJobLabel: "true", + constants.DevWorkspaceNameLabel: "test-workspace", + }, + }, + } + + updateEvent := event.UpdateEvent{ + ObjectOld: job, + ObjectNew: job, + } + + result := backupJobPredicate.Update(updateEvent) + Expect(result).To(BeTrue()) + }) + + It("returns false for job without backup label", func() { + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "regular-job", + Namespace: "test-namespace", + Labels: map[string]string{ + constants.DevWorkspaceNameLabel: "test-workspace", + }, + }, + } + + updateEvent := event.UpdateEvent{ + ObjectOld: job, + ObjectNew: job, + } + + result := backupJobPredicate.Update(updateEvent) + Expect(result).To(BeFalse()) + }) + + It("returns false for job without workspace name label", func() { + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "backup-job", + Namespace: "test-namespace", + Labels: map[string]string{ + constants.DevWorkspaceBackupJobLabel: "true", + }, + }, + } + + updateEvent := event.UpdateEvent{ + ObjectOld: job, + ObjectNew: job, + } + + result := backupJobPredicate.Update(updateEvent) + Expect(result).To(BeFalse()) + }) + + It("returns false for job with empty workspace name label", func() { + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "backup-job", + Namespace: "test-namespace", + Labels: map[string]string{ + constants.DevWorkspaceBackupJobLabel: "true", + constants.DevWorkspaceNameLabel: "", + }, + }, + } + + updateEvent := event.UpdateEvent{ + ObjectOld: job, + ObjectNew: job, + } + + result := backupJobPredicate.Update(updateEvent) + Expect(result).To(BeFalse()) + }) + + It("returns false for non-job objects", func() { + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "test-namespace", + }, + } + + updateEvent := event.UpdateEvent{ + ObjectOld: pod, + ObjectNew: pod, + } + + result := backupJobPredicate.Update(updateEvent) + Expect(result).To(BeFalse()) + }) + }) + + Context("CreateFunc", func() { + It("returns false for all create events", func() { + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "backup-job", + Namespace: "test-namespace", + Labels: map[string]string{ + constants.DevWorkspaceBackupJobLabel: "true", + constants.DevWorkspaceNameLabel: "test-workspace", + }, + }, + } + + createEvent := event.CreateEvent{ + Object: job, + } + + result := backupJobPredicate.Create(createEvent) + Expect(result).To(BeFalse()) + }) + }) + + Context("DeleteFunc", func() { + It("returns false for all delete events", func() { + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "backup-job", + Namespace: "test-namespace", + Labels: map[string]string{ + constants.DevWorkspaceBackupJobLabel: "true", + constants.DevWorkspaceNameLabel: "test-workspace", + }, + }, + } + + deleteEvent := event.DeleteEvent{ + Object: job, + } + + result := backupJobPredicate.Delete(deleteEvent) + Expect(result).To(BeFalse()) + }) + }) + + Context("GenericFunc", func() { + It("returns false for all generic events", func() { + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: "backup-job", + Namespace: "test-namespace", + Labels: map[string]string{ + constants.DevWorkspaceBackupJobLabel: "true", + constants.DevWorkspaceNameLabel: "test-workspace", + }, + }, + } + + genericEvent := event.GenericEvent{ + Object: job, + } + + result := backupJobPredicate.Generic(genericEvent) + Expect(result).To(BeFalse()) + }) + }) +}) + var _ = Describe("DevWorkspaceOperatorConfig UpdateFunc Tests", func() { var configPredicate predicate.Funcs diff --git a/controllers/backupcronjob/backupcronjob_handler.go b/controllers/backupcronjob/backupcronjob_handler.go new file mode 100644 index 000000000..ead6abbea --- /dev/null +++ b/controllers/backupcronjob/backupcronjob_handler.go @@ -0,0 +1,158 @@ +// +// +// Copyright (c) 2019-2025 Red Hat, Inc. +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the Licens +//e is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +package controllers + +import ( + "context" + "fmt" + + dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" + "github.com/devfile/devworkspace-operator/pkg/constants" + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +func (r *BackupCronJobReconciler) getBackupJobPredicate() predicate.Funcs { + return predicate.Funcs{ + UpdateFunc: func(e event.UpdateEvent) bool { + job, ok := e.ObjectNew.(*batchv1.Job) + if !ok { + return false + } + + // Only reconcile if job related to DevWorkspace backup + return job.Labels[constants.DevWorkspaceBackupJobLabel] == "true" && + job.Labels[constants.DevWorkspaceNameLabel] != "" + }, + CreateFunc: func(e event.CreateEvent) bool { return false }, + DeleteFunc: func(e event.DeleteEvent) bool { return false }, + GenericFunc: func(e event.GenericEvent) bool { return false }, + } +} + +func (r *BackupCronJobReconciler) getBackupJobEventHandler() handler.EventHandler { + return handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, object client.Object) []reconcile.Request { + job, ok := object.(*batchv1.Job) + if !ok { + return []ctrl.Request{} + } + + if err := r.handleBackupJobStatus(ctx, job); err != nil { + r.Log.Error(err, "Failed to handle backup job status", "namespace", job.Namespace, "job", job.Name) + } + + // Don't enqueue any reconcile requests for the main reconcile loop + return []ctrl.Request{} + }) +} + +// handleBackupJobStatus checks the status of a backup job and updates the corresponding DevWorkspace annotations. +func (r *BackupCronJobReconciler) handleBackupJobStatus(ctx context.Context, job *batchv1.Job) error { + for _, condition := range job.Status.Conditions { + if condition.Status != corev1.ConditionTrue { + continue + } + + switch condition.Type { + case batchv1.JobComplete: + return r.recordBackupSuccess(ctx, job) + case batchv1.JobFailed: + return r.recordBackupFailure(ctx, job, condition.Message) + } + } + + return nil +} + +func (r *BackupCronJobReconciler) recordBackupSuccess( + ctx context.Context, + job *batchv1.Job, +) error { + devWorkspace, err := r.getWorkspaceFromJob(ctx, job) + if err != nil { + return err + } + + origDevWorkspace := devWorkspace.DeepCopy() + + if devWorkspace.Annotations == nil { + devWorkspace.Annotations = make(map[string]string) + } + + devWorkspace.Annotations[constants.DevWorkspaceLastBackupSuccessfulAnnotation] = "true" + devWorkspace.Annotations[constants.DevWorkspaceLastBackupTimeAnnotation] = metav1.Now().Format(metav1.RFC3339Micro) + delete(devWorkspace.Annotations, constants.DevWorkspaceLastBackupErrorAnnotation) + + return r.Patch(ctx, devWorkspace, client.MergeFrom(origDevWorkspace)) +} + +func (r *BackupCronJobReconciler) recordBackupFailure( + ctx context.Context, + job *batchv1.Job, + errorMsg string, +) error { + devWorkspace, err := r.getWorkspaceFromJob(ctx, job) + if err != nil { + return err + } + + origDevWorkspace := devWorkspace.DeepCopy() + + if devWorkspace.Annotations == nil { + devWorkspace.Annotations = make(map[string]string) + } + + // Truncate error message if it's too long (max 1024 chars for annotation values) + const maxLength = 1024 + if len(errorMsg) > maxLength { + errorMsg = errorMsg[:maxLength-3] + "..." + } + + devWorkspace.Annotations[constants.DevWorkspaceLastBackupSuccessfulAnnotation] = "false" + devWorkspace.Annotations[constants.DevWorkspaceLastBackupTimeAnnotation] = metav1.Now().Format(metav1.RFC3339Micro) + devWorkspace.Annotations[constants.DevWorkspaceLastBackupErrorAnnotation] = errorMsg + + return r.Patch(ctx, devWorkspace, client.MergeFrom(origDevWorkspace)) +} + +func (r *BackupCronJobReconciler) getWorkspaceFromJob( + ctx context.Context, + job *batchv1.Job, +) (*dw.DevWorkspace, error) { + devWorkspaceName, ok := job.Labels[constants.DevWorkspaceNameLabel] + if !ok || devWorkspaceName == "" { + return nil, fmt.Errorf("DevWorkspace name label not found for job %s in namespace %s", job.Name, job.Namespace) + } + + devWorkspace := &dw.DevWorkspace{} + devWorkspaceKey := types.NamespacedName{Name: devWorkspaceName, Namespace: job.Namespace} + + if err := r.Get(ctx, devWorkspaceKey, devWorkspace); err != nil { + return nil, err + } + + return devWorkspace, nil +} diff --git a/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml b/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml index 16abce73e..85ac24443 100644 --- a/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml +++ b/deploy/bundle/manifests/devworkspace-operator.clusterserviceversion.yaml @@ -361,6 +361,8 @@ spec: - delete - get - list + - patch + - update serviceAccountName: devworkspace-controller-serviceaccount deployments: - name: devworkspace-controller-manager diff --git a/deploy/deployment/kubernetes/combined.yaml b/deploy/deployment/kubernetes/combined.yaml index 47de54ecc..1b43b7713 100644 --- a/deploy/deployment/kubernetes/combined.yaml +++ b/deploy/deployment/kubernetes/combined.yaml @@ -26256,6 +26256,8 @@ rules: - delete - get - list + - patch + - update --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole diff --git a/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml b/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml index d4fa6dfeb..b6fefb5e9 100644 --- a/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml +++ b/deploy/deployment/kubernetes/objects/devworkspace-controller-role.ClusterRole.yaml @@ -281,3 +281,5 @@ rules: - delete - get - list + - patch + - update diff --git a/deploy/deployment/openshift/combined.yaml b/deploy/deployment/openshift/combined.yaml index 3f309fa56..9a06741c6 100644 --- a/deploy/deployment/openshift/combined.yaml +++ b/deploy/deployment/openshift/combined.yaml @@ -26256,6 +26256,8 @@ rules: - delete - get - list + - patch + - update --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole diff --git a/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml b/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml index d4fa6dfeb..b6fefb5e9 100644 --- a/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml +++ b/deploy/deployment/openshift/objects/devworkspace-controller-role.ClusterRole.yaml @@ -281,3 +281,5 @@ rules: - delete - get - list + - patch + - update diff --git a/deploy/templates/components/rbac/role.yaml b/deploy/templates/components/rbac/role.yaml index a87be7060..057db945c 100644 --- a/deploy/templates/components/rbac/role.yaml +++ b/deploy/templates/components/rbac/role.yaml @@ -279,3 +279,5 @@ rules: - delete - get - list + - patch + - update diff --git a/pkg/constants/metadata.go b/pkg/constants/metadata.go index b27cef445..068ba7124 100644 --- a/pkg/constants/metadata.go +++ b/pkg/constants/metadata.go @@ -180,4 +180,17 @@ const ( DevWorkspaceBackupJobLabel = "controller.devfile.io/backup-job" DevWorkspaceBackupAuthSecretName = "devworkspace-backup-registry-auth" + + // DevWorkspaceLastBackupSuccessfulAnnotation is an annotation that indicates whether the last backup + // attempt for this DevWorkspace was successful. Value is either "true" or "false". + DevWorkspaceLastBackupSuccessfulAnnotation = "controller.devfile.io/last-backup-successful" + + // DevWorkspaceLastBackupTimeAnnotation is an annotation that stores the timestamp (in RFC3339Micro format) + // of when the last backup was attempted for this DevWorkspace, regardless of success or failure. + DevWorkspaceLastBackupTimeAnnotation = "controller.devfile.io/last-backup-time" + + // DevWorkspaceLastBackupErrorAnnotation is an annotation that stores the error message from the last + // failed backup attempt. This annotation is only present when the last backup failed, and is cleared + // when a backup succeeds. + DevWorkspaceLastBackupErrorAnnotation = "controller.devfile.io/last-backup-error" )