Fix expire not running when target PVC is deleted before backup expires

Signed-off-by: Yuji Ito <llamerada.jp@gmail.com>

Author: llamerada-jp

internal/controller/mantlebackup_controller.go

@@ -288,32 +288,34 @@ func (r *MantleBackupReconciler) getSnapshotTarget(ctx context.Context, backup *
 	return &snapshotTarget{&pvc, &pv, imageName, poolName}, ctrl.Result{}, nil
 }
 
-// expire deletes the backup if it is already expired. Otherwise it schedules deletion.
-// Note that this function does not use requeue to scheduled deletion because the caller
-// will do other tasks after this function returns.
-func (r *MantleBackupReconciler) expire(ctx context.Context, backup *mantlev1.MantleBackup) error {
+// expire deletes the backup if expired, or schedules deletion via expireQueueCh if not yet.
+// Returns true when DeletionTimestamp was set, signaling the caller to stop further processing.
+func (r *MantleBackupReconciler) expire(ctx context.Context, backup *mantlev1.MantleBackup) (bool, error) {
 	logger := log.FromContext(ctx)
 	if backup.Status.CreatedAt.IsZero() {
 		// the RBD snapshot has not be taken yet, do nothing.
-		return nil
+		return false, nil
 	}
 
 	if v, ok := backup.Annotations[annotRetainIfExpired]; ok && v == "true" {
 		// retain this backup.
 		// If the annotation is deleted, reconciliation will run, so no need to schedule.
-		return nil
+		return false, nil
 	}
 
 	expire, err := strfmt.ParseDuration(backup.Spec.Expire)
 	if err != nil {
-		return err
+		return false, err
 	}
 	expireAt := backup.Status.CreatedAt.Add(expire)
 	if time.Now().UTC().After(expireAt) {
 		// already expired, delete it immediately.
 		logger.Info("delete expired backup", "createdAt", backup.Status.CreatedAt, "expire", expire)
 
-		return r.Delete(ctx, backup)
+		if err := r.Delete(ctx, backup); err != nil {
+			return false, err
+		}
+		return true, nil
 	}
 
 	// not expired yet. schedule deletion.
@@ -323,7 +325,7 @@ func (r *MantleBackupReconciler) expire(ctx context.Context, backup *mantlev1.Ma
 		Object: backup,
 	}
 
-	return nil
+	return false, nil
 }
 
 //+kubebuilder:rbac:groups=mantle.cybozu.io,resources=mantlebackups,verbs=get;list;watch;create;update;patch;delete
@@ -384,6 +386,10 @@ func (r *MantleBackupReconciler) reconcileLocalBackup(ctx context.Context, backu
 		return ctrl.Result{}, nil
 	}
 
+	if updated, err := r.expire(ctx, backup); updated || err != nil {
+		return ctrl.Result{}, err
+	}
+
 	target, result, err := r.getSnapshotTarget(ctx, backup)
 	notFound := aerrors.IsNotFound(err)
 	switch {
@@ -428,10 +434,6 @@ func (r *MantleBackupReconciler) reconcileLocalBackup(ctx context.Context, backu
 		return ctrl.Result{}, nil
 	}
 
-	if err := r.expire(ctx, backup); err != nil {
-		return ctrl.Result{}, err
-	}
-
 	if !backup.IsSnapshotCaptured() {
 		if err := r.provisionRBDSnapshot(ctx, backup, target); err != nil {
 			return ctrl.Result{}, err
@@ -499,6 +501,10 @@ func (r *MantleBackupReconciler) reconcileAsSecondary(ctx context.Context, backu
 		return ctrl.Result{}, nil
 	}
 
+	if updated, err := r.expire(ctx, backup); updated || err != nil {
+		return ctrl.Result{}, err
+	}
+
 	target, result, err := r.getSnapshotTarget(ctx, backup)
 	notFound := aerrors.IsNotFound(err)
 	switch {
@@ -518,10 +524,6 @@ func (r *MantleBackupReconciler) reconcileAsSecondary(ctx context.Context, backu
 		return ctrl.Result{}, err
 	}
 
-	if err := r.expire(ctx, backup); err != nil {
-		return ctrl.Result{}, err
-	}
-
 	if !backup.IsSnapshotCaptured() {
 		result, err := r.startImport(ctx, backup, target)
 		if err != nil || !result.IsZero() {