Fixup

Signed-off-by: Anatolii Bazko <abazko@redhat.com>

Author: tolusha

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
 			}
 		}

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())
+		})
 	})
 
 })