Fix Negative Requeue Duration in getStatusCheckDelay of DRPlacementCo…

…ntrol Reconciler

Signed-off-by: Oded Viner <[email protected]>

internal/controller/drplacementcontrol_controller.go

@@ -500,7 +500,7 @@ func (r *DRPlacementControlReconciler) reconcileDRPCInstance(d *DRPCInstance, lo
 		afterProcessing = *d.instance.Status.LastUpdateTime
 	}
 
-	requeueTimeDuration := r.getStatusCheckDelay(beforeProcessing, afterProcessing)
+	requeueTimeDuration := r.GetStatusCheckDelay(beforeProcessing, afterProcessing)
 	log.Info("Requeue time", "duration", requeueTimeDuration)
 
 	return ctrl.Result{RequeueAfter: requeueTimeDuration}, nil
@@ -1229,7 +1229,7 @@ func (d *DRPCInstance) statusUpdateTimeElapsed() bool {
 	return d.instance.Status.LastUpdateTime.Add(StatusCheckDelay).Before(time.Now())
 }
 
-// getStatusCheckDelay returns the reconciliation requeue time duration when no requeue
+// GetStatusCheckDelay returns the reconciliation requeue time duration when no requeue
 // has been requested. We want the reconciliation to run at least once every StatusCheckDelay
 // in order to refresh DRPC status with VRG status. The reconciliation will be called at any time.
 // If it is called before the StatusCheckDelay has elapsed, and the DRPC status was not updated,
@@ -1242,22 +1242,21 @@ func (d *DRPCInstance) statusUpdateTimeElapsed() bool {
 // the reconciler is called, let's say, at 10:08am, and no update to the DRPC status was needed,
 // then the requeue time duration should be 2 minutes and NOT the full StatusCheckDelay. That is:
 // 10:00am + StatusCheckDelay - 10:08am = 2mins
-func (r *DRPlacementControlReconciler) getStatusCheckDelay(
+func (r *DRPlacementControlReconciler) GetStatusCheckDelay(
 	beforeProcessing metav1.Time, afterProcessing metav1.Time,
 ) time.Duration {
 	if beforeProcessing != afterProcessing {
-		// DRPC's VRG status update processing time has changed during this
-		// iteration of the reconcile loop.  Hence, the next attempt to update
-		// the status should be after a delay of a standard polling interval
-		// duration.
+		// Processing time changed: return the full delay.
 		return StatusCheckDelay
 	}
 
-	// DRPC's VRG status update processing time has NOT changed during this
-	// iteration of the reconcile loop.  Hence, the next attempt to update the
-	// status should be after the remaining duration of this polling interval has
-	// elapsed: (beforeProcessing + StatusCheckDelay - time.Now())
-	return time.Until(beforeProcessing.Add(StatusCheckDelay))
+	// Calculate the remaining time until the next scheduled update.
+	remaining := time.Until(beforeProcessing.Add(StatusCheckDelay))
+	if remaining < 0 {
+		// If the scheduled time is already in the past, requeue immediately.
+		return 0
+	}
+	return remaining
 }
 
 // updateDRPCStatus updates the DRPC sub-resource status with,

internal/controller/drplacementcontrol_controller_test.go

@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"runtime"
 	"strings"
+	"testing"
 	"time"
 
 	volrep "github.com/csi-addons/kubernetes-csi-addons/api/replication.storage/v1alpha1"
@@ -3192,3 +3193,75 @@ func forceCleanupClusterAfterAErrorTest() error {
 
 	return nil
 }
+
+// TestGetStatusCheckDelay verifies the behavior of getStatusCheckDelay across different scenarios.
+func TestGetStatusCheckDelay(t *testing.T) {
+	tests := []struct {
+		name          string
+		getTimes      func() (before, after metav1.Time)
+		expectedDelay func(before metav1.Time) time.Duration
+		tolerance     time.Duration
+	}{
+		{
+			name: "processing time changed returns full delay",
+			getTimes: func() (metav1.Time, metav1.Time) {
+				now := time.Now()
+				// Simulate processing time change by having after != before.
+				return metav1.NewTime(now), metav1.NewTime(now.Add(1 * time.Millisecond))
+			},
+			// When processing times differ, we expect the full StatusCheckDelay.
+			expectedDelay: func(before metav1.Time) time.Duration {
+				return controllers.StatusCheckDelay
+			},
+			tolerance: 0,
+		},
+		{
+			name: "remaining time returns nearly full delay",
+			getTimes: func() (metav1.Time, metav1.Time) {
+				now := time.Now()
+				// Processing times are equal; no status update occurred.
+				tBefore := metav1.NewTime(now)
+				return tBefore, tBefore
+			},
+			// Expected delay is the remaining time until beforeProcessing+StatusCheckDelay.
+			expectedDelay: func(before metav1.Time) time.Duration {
+				return time.Until(before.Add(controllers.StatusCheckDelay))
+			},
+			// Allow a small tolerance since time passes between calls.
+			tolerance: 10 * time.Millisecond,
+		},
+		{
+			name: "elapsed scheduled time returns 0",
+			getTimes: func() (metav1.Time, metav1.Time) {
+				// Set beforeProcessing in the past such that before+StatusCheckDelay is already elapsed.
+				past := time.Now().Add(-controllers.StatusCheckDelay - 1*time.Second)
+				tPast := metav1.NewTime(past)
+				return tPast, tPast
+			},
+			// If the scheduled time has passed, we expect a delay of 0.
+			expectedDelay: func(before metav1.Time) time.Duration {
+				return 0
+			},
+			tolerance: 0,
+		},
+	}
+
+	// Create an instance of the reconciler.
+	reconciler := &controllers.DRPlacementControlReconciler{}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			before, after := tc.getTimes()
+			// Calculate the expected delay using the provided function.
+			expected := tc.expectedDelay(before)
+			// Get the actual delay from the reconciler.
+			actual := reconciler.GetStatusCheckDelay(before, after)
+
+			// Check that the difference between expected and actual is within the tolerance.
+			diff := actual - expected
+			if diff < -tc.tolerance || diff > tc.tolerance {
+				t.Errorf("Test %q: expected delay ~%v (±%v), got %v", tc.name, expected, tc.tolerance, actual)
+			}
+		})
+	}
+}