From 608e6848828e17eb8a3bbf158d4e91e2d05e16ed Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Mon, 22 Dec 2025 15:46:37 +0100 Subject: [PATCH 1/8] 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" ) From d5ee87808b10c63c5ae0cf27dae3740b0d9feb8d Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Mon, 5 Jan 2026 12:10:41 +0100 Subject: [PATCH 2/8] Fixup Signed-off-by: Anatolii Bazko --- .../backupcronjob/backupcronjob_controller.go | 2 +- .../backupcronjob_controller_test.go | 43 +++++++++++-------- .../backupcronjob/backupcronjob_handler.go | 41 +++++++++--------- pkg/constants/metadata.go | 6 +-- 4 files changed, 49 insertions(+), 43 deletions(-) diff --git a/controllers/backupcronjob/backupcronjob_controller.go b/controllers/backupcronjob/backupcronjob_controller.go index e4175cef4..077b29bec 100644 --- a/controllers/backupcronjob/backupcronjob_controller.go +++ b/controllers/backupcronjob/backupcronjob_controller.go @@ -289,7 +289,7 @@ func (r *BackupCronJobReconciler) wasStoppedSinceLastBackup( var lastBackupTime *metav1.Time var lastBackupSuccessful bool if workspace.Annotations != nil { - if lastBackupTimeStr, ok := workspace.Annotations[constants.DevWorkspaceLastBackupTimeAnnotation]; ok { + if lastBackupTimeStr, ok := workspace.Annotations[constants.DevWorkspaceLastBackupFinishedAtAnnotation]; 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) diff --git a/controllers/backupcronjob/backupcronjob_controller_test.go b/controllers/backupcronjob/backupcronjob_controller_test.go index 8027cea1c..a8690557d 100644 --- a/controllers/backupcronjob/backupcronjob_controller_test.go +++ b/controllers/backupcronjob/backupcronjob_controller_test.go @@ -1,3 +1,4 @@ +// // 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. @@ -10,6 +11,7 @@ // 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 @@ -562,8 +564,9 @@ var _ = Describe("BackupCronJobReconciler", func() { Status: batchv1.JobStatus{ Conditions: []batchv1.JobCondition{ { - Type: batchv1.JobComplete, - Status: corev1.ConditionTrue, + Type: batchv1.JobComplete, + Status: corev1.ConditionTrue, + LastTransitionTime: metav1.Now(), }, }, }, @@ -577,7 +580,7 @@ var _ = Describe("BackupCronJobReconciler", func() { 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[constants.DevWorkspaceLastBackupFinishedAtAnnotation]).ToNot(BeEmpty()) Expect(updatedDw.Annotations).ToNot(HaveKey(constants.DevWorkspaceLastBackupErrorAnnotation)) }) @@ -600,9 +603,10 @@ var _ = Describe("BackupCronJobReconciler", func() { Status: batchv1.JobStatus{ Conditions: []batchv1.JobCondition{ { - Type: batchv1.JobFailed, - Status: corev1.ConditionTrue, - Message: errorMessage, + Type: batchv1.JobFailed, + Status: corev1.ConditionTrue, + LastTransitionTime: metav1.Now(), + Message: errorMessage, }, }, }, @@ -616,7 +620,7 @@ var _ = Describe("BackupCronJobReconciler", func() { 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.DevWorkspaceLastBackupFinishedAtAnnotation]).ToNot(BeEmpty()) Expect(updatedDw.Annotations[constants.DevWorkspaceLastBackupErrorAnnotation]).To(Equal(errorMessage)) }) @@ -644,9 +648,10 @@ var _ = Describe("BackupCronJobReconciler", func() { Status: batchv1.JobStatus{ Conditions: []batchv1.JobCondition{ { - Type: batchv1.JobFailed, - Status: corev1.ConditionTrue, - Message: longErrorMessage, + Type: batchv1.JobFailed, + Status: corev1.ConditionTrue, + LastTransitionTime: metav1.Now(), + Message: longErrorMessage, }, }, }, @@ -706,8 +711,9 @@ var _ = Describe("BackupCronJobReconciler", func() { Status: batchv1.JobStatus{ Conditions: []batchv1.JobCondition{ { - Type: batchv1.JobComplete, - Status: corev1.ConditionTrue, + Type: batchv1.JobComplete, + Status: corev1.ConditionTrue, + LastTransitionTime: metav1.Now(), }, }, }, @@ -730,8 +736,9 @@ var _ = Describe("BackupCronJobReconciler", func() { Status: batchv1.JobStatus{ Conditions: []batchv1.JobCondition{ { - Type: batchv1.JobComplete, - Status: corev1.ConditionTrue, + Type: batchv1.JobComplete, + Status: corev1.ConditionTrue, + LastTransitionTime: metav1.Now(), }, }, }, @@ -860,7 +867,7 @@ var _ = Describe("BackupCronJobReconciler", func() { 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.DevWorkspaceLastBackupFinishedAtAnnotation: annotationBackupTime.Format(time.RFC3339Nano), constants.DevWorkspaceLastBackupSuccessfulAnnotation: "true", } @@ -873,7 +880,7 @@ var _ = Describe("BackupCronJobReconciler", func() { 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.DevWorkspaceLastBackupFinishedAtAnnotation: annotationBackupTime.Format(time.RFC3339Nano), constants.DevWorkspaceLastBackupSuccessfulAnnotation: "false", } @@ -885,7 +892,7 @@ var _ = Describe("BackupCronJobReconciler", 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.DevWorkspaceLastBackupFinishedAtAnnotation: "invalid-time-format", constants.DevWorkspaceLastBackupSuccessfulAnnotation: "true", } @@ -901,7 +908,7 @@ var _ = Describe("BackupCronJobReconciler", func() { dw := createDevWorkspace("dw-test-prefer-annotation", "ns-test-prefer-annotation", false, workspaceStoppedTime) dw.Annotations = map[string]string{ - constants.DevWorkspaceLastBackupTimeAnnotation: annotationBackupTime.Format(time.RFC3339Nano), + constants.DevWorkspaceLastBackupFinishedAtAnnotation: annotationBackupTime.Format(time.RFC3339Nano), constants.DevWorkspaceLastBackupSuccessfulAnnotation: "true", } diff --git a/controllers/backupcronjob/backupcronjob_handler.go b/controllers/backupcronjob/backupcronjob_handler.go index ead6abbea..a0126e680 100644 --- a/controllers/backupcronjob/backupcronjob_handler.go +++ b/controllers/backupcronjob/backupcronjob_handler.go @@ -1,5 +1,4 @@ // -// // 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. @@ -8,8 +7,7 @@ // 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, +// distributed under the License 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. @@ -76,11 +74,20 @@ func (r *BackupCronJobReconciler) handleBackupJobStatus(ctx context.Context, job continue } + if condition.Type != batchv1.JobComplete && condition.Type != batchv1.JobFailed { + continue + } + + devWorkspace, err := r.getWorkspaceFromJob(ctx, job) + if err != nil { + return err + } + switch condition.Type { case batchv1.JobComplete: - return r.recordBackupSuccess(ctx, job) + return r.recordBackupSuccess(ctx, devWorkspace, condition) case batchv1.JobFailed: - return r.recordBackupFailure(ctx, job, condition.Message) + return r.recordBackupFailure(ctx, devWorkspace, condition) } } @@ -89,13 +96,9 @@ func (r *BackupCronJobReconciler) handleBackupJobStatus(ctx context.Context, job func (r *BackupCronJobReconciler) recordBackupSuccess( ctx context.Context, - job *batchv1.Job, + devWorkspace *dw.DevWorkspace, + condition batchv1.JobCondition, ) error { - devWorkspace, err := r.getWorkspaceFromJob(ctx, job) - if err != nil { - return err - } - origDevWorkspace := devWorkspace.DeepCopy() if devWorkspace.Annotations == nil { @@ -103,7 +106,7 @@ func (r *BackupCronJobReconciler) recordBackupSuccess( } devWorkspace.Annotations[constants.DevWorkspaceLastBackupSuccessfulAnnotation] = "true" - devWorkspace.Annotations[constants.DevWorkspaceLastBackupTimeAnnotation] = metav1.Now().Format(metav1.RFC3339Micro) + devWorkspace.Annotations[constants.DevWorkspaceLastBackupFinishedAtAnnotation] = condition.LastTransitionTime.Format(metav1.RFC3339Micro) delete(devWorkspace.Annotations, constants.DevWorkspaceLastBackupErrorAnnotation) return r.Patch(ctx, devWorkspace, client.MergeFrom(origDevWorkspace)) @@ -111,14 +114,9 @@ func (r *BackupCronJobReconciler) recordBackupSuccess( func (r *BackupCronJobReconciler) recordBackupFailure( ctx context.Context, - job *batchv1.Job, - errorMsg string, + devWorkspace *dw.DevWorkspace, + condition batchv1.JobCondition, ) error { - devWorkspace, err := r.getWorkspaceFromJob(ctx, job) - if err != nil { - return err - } - origDevWorkspace := devWorkspace.DeepCopy() if devWorkspace.Annotations == nil { @@ -126,13 +124,14 @@ func (r *BackupCronJobReconciler) recordBackupFailure( } // Truncate error message if it's too long (max 1024 chars for annotation values) + errorMsg := condition.Message 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.DevWorkspaceLastBackupFinishedAtAnnotation] = condition.LastTransitionTime.Format(metav1.RFC3339Micro) devWorkspace.Annotations[constants.DevWorkspaceLastBackupErrorAnnotation] = errorMsg return r.Patch(ctx, devWorkspace, client.MergeFrom(origDevWorkspace)) @@ -144,12 +143,12 @@ func (r *BackupCronJobReconciler) getWorkspaceFromJob( ) (*dw.DevWorkspace, error) { devWorkspaceName, ok := job.Labels[constants.DevWorkspaceNameLabel] if !ok || devWorkspaceName == "" { + // Should not happen since we already checked this in the predicate 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 } diff --git a/pkg/constants/metadata.go b/pkg/constants/metadata.go index 068ba7124..a645de04b 100644 --- a/pkg/constants/metadata.go +++ b/pkg/constants/metadata.go @@ -185,9 +185,9 @@ const ( // 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" + // DevWorkspaceLastBackupFinishedAtAnnotation is an annotation that stores the timestamp (in RFC3339Micro format) + // of when the last backup finished for this DevWorkspace (whether it completed successfully or failed). + DevWorkspaceLastBackupFinishedAtAnnotation = "controller.devfile.io/last-backup-finished-at" // 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 From e6f9ede4c3dfa861ed1f25ebeadd156d4fac7ac9 Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Mon, 5 Jan 2026 14:04:44 +0100 Subject: [PATCH 3/8] Fixup Signed-off-by: Anatolii Bazko --- .../backupcronjob/backupcronjob_controller.go | 7 ++-- .../backupcronjob_controller_test.go | 35 +++++++++++++++++-- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/controllers/backupcronjob/backupcronjob_controller.go b/controllers/backupcronjob/backupcronjob_controller.go index 077b29bec..3e3b80400 100644 --- a/controllers/backupcronjob/backupcronjob_controller.go +++ b/controllers/backupcronjob/backupcronjob_controller.go @@ -299,11 +299,10 @@ func (r *BackupCronJobReconciler) wasStoppedSinceLastBackup( } lastBackupSuccessful = workspace.Annotations[constants.DevWorkspaceLastBackupSuccessfulAnnotation] == "true" - } - - // Fall back to globalLastBackupTime if annotation doesn't exist - if lastBackupTime == nil && globalLastBackupTime != nil { + } else { + // Fall back to globalLastBackupTime if annotation doesn't exist lastBackupTime = globalLastBackupTime + lastBackupSuccessful = true } if lastBackupTime == nil { diff --git a/controllers/backupcronjob/backupcronjob_controller_test.go b/controllers/backupcronjob/backupcronjob_controller_test.go index a8690557d..61418caa5 100644 --- a/controllers/backupcronjob/backupcronjob_controller_test.go +++ b/controllers/backupcronjob/backupcronjob_controller_test.go @@ -402,8 +402,9 @@ 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 + // Set successful annotation and backup time so the time-based logic is checked dw.Annotations = map[string]string{ + constants.DevWorkspaceLastBackupFinishedAtAnnotation: lastBackupTime.Format(time.RFC3339Nano), constants.DevWorkspaceLastBackupSuccessfulAnnotation: "true", } Expect(fakeClient.Create(ctx, dw)).To(Succeed()) @@ -842,11 +843,12 @@ 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 + // Set successful annotation and backup time so the time-based logic is checked dw.Annotations = map[string]string{ + constants.DevWorkspaceLastBackupFinishedAtAnnotation: lastBackupTime.Format(time.RFC3339Nano), constants.DevWorkspaceLastBackupSuccessfulAnnotation: "true", } - result := reconciler.wasStoppedSinceLastBackup(dw, &lastBackupTime, log) + result := reconciler.wasStoppedSinceLastBackup(dw, nil, log) Expect(result).To(BeFalse()) }) It("returns true if there is no last backup time", func() { @@ -917,6 +919,33 @@ var _ = Describe("BackupCronJobReconciler", func() { result := reconciler.wasStoppedSinceLastBackup(dw, &globalLastBackupTime, log) Expect(result).To(BeTrue()) }) + + It("falls back to global last backup time when annotations are nil", func() { + globalLastBackupTime := metav1.NewTime(time.Now().Add(-20 * time.Minute)) + workspaceStoppedTime := metav1.NewTime(time.Now().Add(-10 * time.Minute)) + + dw := createDevWorkspace("dw-test-nil-annotations", "ns-test-nil-annotations", false, workspaceStoppedTime) + // Explicitly ensure annotations is nil + dw.Annotations = nil + + // With global time (-20min), workspace stopped at -10min, should return true + // lastBackupSuccessful should be treated as true when falling back + result := reconciler.wasStoppedSinceLastBackup(dw, &globalLastBackupTime, log) + Expect(result).To(BeTrue()) + }) + + It("returns false when annotations are nil and workspace stopped before global backup time", func() { + globalLastBackupTime := metav1.NewTime(time.Now().Add(-5 * time.Minute)) + workspaceStoppedTime := metav1.NewTime(time.Now().Add(-10 * time.Minute)) + + dw := createDevWorkspace("dw-test-nil-old-stop", "ns-test-nil-old-stop", false, workspaceStoppedTime) + // Explicitly ensure annotations is nil + dw.Annotations = nil + + // With global time (-5min), workspace stopped at -10min, should return false + result := reconciler.wasStoppedSinceLastBackup(dw, &globalLastBackupTime, log) + Expect(result).To(BeFalse()) + }) }) }) From e260eff8152c16060de489c5ac58e5edd1682785 Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Mon, 5 Jan 2026 14:27:19 +0100 Subject: [PATCH 4/8] Fixup Signed-off-by: Anatolii Bazko --- .../backupcronjob/backupcronjob_controller.go | 39 ++++--- .../backupcronjob_controller_test.go | 106 +++++------------- 2 files changed, 48 insertions(+), 97 deletions(-) diff --git a/controllers/backupcronjob/backupcronjob_controller.go b/controllers/backupcronjob/backupcronjob_controller.go index 3e3b80400..7012e13f0 100644 --- a/controllers/backupcronjob/backupcronjob_controller.go +++ b/controllers/backupcronjob/backupcronjob_controller.go @@ -276,36 +276,39 @@ func (r *BackupCronJobReconciler) executeBackupSync(ctx context.Context, dwOpera // 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, + devWorkspace *dw.DevWorkspace, globalLastBackupTime *metav1.Time, log logr.Logger, ) bool { - if workspace.Status.Phase != dw.DevWorkspaceStatusStopped { + if devWorkspace.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) + log.Info("DevWorkspace is currently stopped, checking if it was stopped since last backup", "namespace", devWorkspace.Namespace, "name", devWorkspace.Name) - // Get the last backup time and success status from the workspace annotations - var lastBackupTime *metav1.Time + var lastBackupFinishedAt *metav1.Time var lastBackupSuccessful bool - if workspace.Annotations != nil { - if lastBackupTimeStr, ok := workspace.Annotations[constants.DevWorkspaceLastBackupFinishedAtAnnotation]; ok { - parsedTime, err := time.Parse(time.RFC3339Nano, lastBackupTimeStr) + + // Get the last backup time and success status from the workspace annotations + if devWorkspace.Annotations != nil { + if lastBackupFinishedAtStr, ok := devWorkspace.Annotations[constants.DevWorkspaceLastBackupFinishedAtAnnotation]; ok { + parsedTime, err := time.Parse(time.RFC3339Nano, lastBackupFinishedAtStr) if err != nil { - log.Error(err, "Failed to parse last backup time annotation, treating as no previous backup", "value", lastBackupTimeStr) + log.Error(err, "Failed to parse last backup time annotation, treating as no previous backup", "value", lastBackupFinishedAtStr) } else { - lastBackupTime = &metav1.Time{Time: parsedTime} + lastBackupFinishedAt = &metav1.Time{Time: parsedTime} } } - lastBackupSuccessful = workspace.Annotations[constants.DevWorkspaceLastBackupSuccessfulAnnotation] == "true" - } else { + lastBackupSuccessful = devWorkspace.Annotations[constants.DevWorkspaceLastBackupSuccessfulAnnotation] == "true" + } + + if lastBackupFinishedAt == nil { // Fall back to globalLastBackupTime if annotation doesn't exist - lastBackupTime = globalLastBackupTime + lastBackupFinishedAt = globalLastBackupTime lastBackupSuccessful = true } - if lastBackupTime == nil { + if lastBackupFinishedAt == nil { return true } @@ -314,17 +317,17 @@ func (r *BackupCronJobReconciler) wasStoppedSinceLastBackup( } // Check if the workspace was stopped since the last successful backup - if workspace.Status.Conditions != nil { + if devWorkspace.Status.Conditions != nil { lastTimeStopped := metav1.Time{} - for _, condition := range workspace.Status.Conditions { + for _, condition := range devWorkspace.Status.Conditions { if condition.Type == conditions.Started && condition.Status == corev1.ConditionFalse { lastTimeStopped = condition.LastTransitionTime } } if !lastTimeStopped.IsZero() { - if lastTimeStopped.Time.After(lastBackupTime.Time) { - log.Info("DevWorkspace was stopped since last successful backup", "namespace", workspace.Namespace, "name", workspace.Name) + if lastTimeStopped.Time.After(lastBackupFinishedAt.Time) { + log.Info("DevWorkspace was stopped since last successful backup", "namespace", devWorkspace.Namespace, "name", devWorkspace.Name) return true } } diff --git a/controllers/backupcronjob/backupcronjob_controller_test.go b/controllers/backupcronjob/backupcronjob_controller_test.go index 61418caa5..98ddc76d7 100644 --- a/controllers/backupcronjob/backupcronjob_controller_test.go +++ b/controllers/backupcronjob/backupcronjob_controller_test.go @@ -751,85 +751,6 @@ var _ = Describe("BackupCronJobReconciler", func() { }) }) - 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)) @@ -934,6 +855,20 @@ var _ = Describe("BackupCronJobReconciler", func() { Expect(result).To(BeTrue()) }) + It("falls back to global last backup time when annotations is empty", func() { + globalLastBackupTime := metav1.NewTime(time.Now().Add(-20 * time.Minute)) + workspaceStoppedTime := metav1.NewTime(time.Now().Add(-10 * time.Minute)) + + dw := createDevWorkspace("dw-test-empty-annotations", "ns-test-empty-annotations", false, workspaceStoppedTime) + // Explicitly ensure annotations is empty + dw.Annotations = map[string]string{} + + // With global time (-20min), workspace stopped at -10min, should return true + // lastBackupSuccessful should be treated as true when falling back + result := reconciler.wasStoppedSinceLastBackup(dw, &globalLastBackupTime, log) + Expect(result).To(BeTrue()) + }) + It("returns false when annotations are nil and workspace stopped before global backup time", func() { globalLastBackupTime := metav1.NewTime(time.Now().Add(-5 * time.Minute)) workspaceStoppedTime := metav1.NewTime(time.Now().Add(-10 * time.Minute)) @@ -946,6 +881,19 @@ var _ = Describe("BackupCronJobReconciler", func() { result := reconciler.wasStoppedSinceLastBackup(dw, &globalLastBackupTime, log) Expect(result).To(BeFalse()) }) + + It("returns false when annotations is empty and workspace stopped before global backup time", func() { + globalLastBackupTime := metav1.NewTime(time.Now().Add(-5 * time.Minute)) + workspaceStoppedTime := metav1.NewTime(time.Now().Add(-10 * time.Minute)) + + dw := createDevWorkspace("dw-test-empty-old-stop", "ns-test-empty-old-stop", false, workspaceStoppedTime) + // Explicitly ensure annotations is empty + dw.Annotations = map[string]string{} + + // With global time (-5min), workspace stopped at -10min, should return false + result := reconciler.wasStoppedSinceLastBackup(dw, &globalLastBackupTime, log) + Expect(result).To(BeFalse()) + }) }) }) From 9f648dfc8afc1883caffb28b61a6230ca07f9cf0 Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Mon, 5 Jan 2026 14:40:32 +0100 Subject: [PATCH 5/8] Fixup Signed-off-by: Anatolii Bazko --- controllers/backupcronjob/backupcronjob_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/backupcronjob/backupcronjob_controller.go b/controllers/backupcronjob/backupcronjob_controller.go index 7012e13f0..01f3308c9 100644 --- a/controllers/backupcronjob/backupcronjob_controller.go +++ b/controllers/backupcronjob/backupcronjob_controller.go @@ -283,7 +283,6 @@ func (r *BackupCronJobReconciler) wasStoppedSinceLastBackup( if devWorkspace.Status.Phase != dw.DevWorkspaceStatusStopped { return false } - log.Info("DevWorkspace is currently stopped, checking if it was stopped since last backup", "namespace", devWorkspace.Namespace, "name", devWorkspace.Name) var lastBackupFinishedAt *metav1.Time var lastBackupSuccessful bool @@ -318,6 +317,7 @@ func (r *BackupCronJobReconciler) wasStoppedSinceLastBackup( // Check if the workspace was stopped since the last successful backup if devWorkspace.Status.Conditions != nil { + log.Info("DevWorkspace is currently stopped, checking if it was stopped since last backup", "namespace", devWorkspace.Namespace, "name", devWorkspace.Name) lastTimeStopped := metav1.Time{} for _, condition := range devWorkspace.Status.Conditions { if condition.Type == conditions.Started && condition.Status == corev1.ConditionFalse { From 7e75177d7ff277074d09e3fc1dab6868e4cfd3c2 Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Mon, 5 Jan 2026 14:41:09 +0100 Subject: [PATCH 6/8] Fixup Signed-off-by: Anatolii Bazko --- controllers/backupcronjob/backupcronjob_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/backupcronjob/backupcronjob_controller.go b/controllers/backupcronjob/backupcronjob_controller.go index 01f3308c9..7012e13f0 100644 --- a/controllers/backupcronjob/backupcronjob_controller.go +++ b/controllers/backupcronjob/backupcronjob_controller.go @@ -283,6 +283,7 @@ func (r *BackupCronJobReconciler) wasStoppedSinceLastBackup( if devWorkspace.Status.Phase != dw.DevWorkspaceStatusStopped { return false } + log.Info("DevWorkspace is currently stopped, checking if it was stopped since last backup", "namespace", devWorkspace.Namespace, "name", devWorkspace.Name) var lastBackupFinishedAt *metav1.Time var lastBackupSuccessful bool @@ -317,7 +318,6 @@ func (r *BackupCronJobReconciler) wasStoppedSinceLastBackup( // Check if the workspace was stopped since the last successful backup if devWorkspace.Status.Conditions != nil { - log.Info("DevWorkspace is currently stopped, checking if it was stopped since last backup", "namespace", devWorkspace.Namespace, "name", devWorkspace.Name) lastTimeStopped := metav1.Time{} for _, condition := range devWorkspace.Status.Conditions { if condition.Type == conditions.Started && condition.Status == corev1.ConditionFalse { From b01db75fdd0ab8b93bc31b6592c211ee433a21f2 Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Mon, 5 Jan 2026 14:48:32 +0100 Subject: [PATCH 7/8] Fixup Signed-off-by: Anatolii Bazko --- .../backupcronjob_controller_test.go | 66 ------------------- 1 file changed, 66 deletions(-) diff --git a/controllers/backupcronjob/backupcronjob_controller_test.go b/controllers/backupcronjob/backupcronjob_controller_test.go index 98ddc76d7..cb2522def 100644 --- a/controllers/backupcronjob/backupcronjob_controller_test.go +++ b/controllers/backupcronjob/backupcronjob_controller_test.go @@ -1058,72 +1058,6 @@ var _ = Describe("getBackupJobPredicate Tests", func() { 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() { From 80a9d5c0d7365731eaed4093388178a336fd4939 Mon Sep 17 00:00:00 2001 From: Anatolii Bazko Date: Fri, 9 Jan 2026 10:43:15 +0100 Subject: [PATCH 8/8] fixup Signed-off-by: Anatolii Bazko --- controllers/backupcronjob/backupcronjob_handler.go | 6 +++--- pkg/constants/metadata.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/controllers/backupcronjob/backupcronjob_handler.go b/controllers/backupcronjob/backupcronjob_handler.go index a0126e680..7cd071b65 100644 --- a/controllers/backupcronjob/backupcronjob_handler.go +++ b/controllers/backupcronjob/backupcronjob_handler.go @@ -18,12 +18,12 @@ package controllers import ( "context" "fmt" + "time" 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" @@ -106,7 +106,7 @@ func (r *BackupCronJobReconciler) recordBackupSuccess( } devWorkspace.Annotations[constants.DevWorkspaceLastBackupSuccessfulAnnotation] = "true" - devWorkspace.Annotations[constants.DevWorkspaceLastBackupFinishedAtAnnotation] = condition.LastTransitionTime.Format(metav1.RFC3339Micro) + devWorkspace.Annotations[constants.DevWorkspaceLastBackupFinishedAtAnnotation] = condition.LastTransitionTime.Format(time.RFC3339Nano) delete(devWorkspace.Annotations, constants.DevWorkspaceLastBackupErrorAnnotation) return r.Patch(ctx, devWorkspace, client.MergeFrom(origDevWorkspace)) @@ -131,7 +131,7 @@ func (r *BackupCronJobReconciler) recordBackupFailure( } devWorkspace.Annotations[constants.DevWorkspaceLastBackupSuccessfulAnnotation] = "false" - devWorkspace.Annotations[constants.DevWorkspaceLastBackupFinishedAtAnnotation] = condition.LastTransitionTime.Format(metav1.RFC3339Micro) + devWorkspace.Annotations[constants.DevWorkspaceLastBackupFinishedAtAnnotation] = condition.LastTransitionTime.Format(time.RFC3339Nano) devWorkspace.Annotations[constants.DevWorkspaceLastBackupErrorAnnotation] = errorMsg return r.Patch(ctx, devWorkspace, client.MergeFrom(origDevWorkspace)) diff --git a/pkg/constants/metadata.go b/pkg/constants/metadata.go index a645de04b..b9664e3e1 100644 --- a/pkg/constants/metadata.go +++ b/pkg/constants/metadata.go @@ -185,7 +185,7 @@ const ( // attempt for this DevWorkspace was successful. Value is either "true" or "false". DevWorkspaceLastBackupSuccessfulAnnotation = "controller.devfile.io/last-backup-successful" - // DevWorkspaceLastBackupFinishedAtAnnotation is an annotation that stores the timestamp (in RFC3339Micro format) + // DevWorkspaceLastBackupFinishedAtAnnotation is an annotation that stores the timestamp (in RFC3339Nano format) // of when the last backup finished for this DevWorkspace (whether it completed successfully or failed). DevWorkspaceLastBackupFinishedAtAnnotation = "controller.devfile.io/last-backup-finished-at"