refactor: Replace ActionTestFailover action with DryRun attribute

Replace the separate ActionTestFailover action type with a simpler attribute-based
approach using a DryRun boolean field. This cleaner design separates concerns:
- Action Failover indicates the operation to perform
- DryRun boolean indicates if the operation should be non-destructive/test mode
- Progression status (TestingFailover) continues to indicate test mode

Changes:
- Remove ActionTestFailover from DRAction and VRGAction enums
- Remove TestFailover and TestFailedOver DRState constants
- Add DryRun field to DRPlacementControlSpec and VolumeReplicationGroupSpec
- Update all references to ActionTestFailover to check DryRun flag instead

The progression status ProgressionTestingFailover is retained as it provides
a unified indicator of test mode across both DRPC and VRG resources.

Author: BenamarMk

api/v1alpha1/drplacementcontrol_types.go

@@ -8,8 +8,8 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-// DRAction which will be either a Failover, Relocate or TestFailover action
-// +kubebuilder:validation:Enum=Failover;Relocate;TestFailover
+// DRAction which will be either a Failover or Relocate action
+// +kubebuilder:validation:Enum=Failover;Relocate
 type DRAction string
 
 // These are the valid values for DRAction
@@ -20,11 +20,6 @@ const (
 	// Relocate, restore PVs to the designated TargetCluster.  PreferredCluster will change
 	// to be the TargetCluster.
 	ActionRelocate = DRAction("Relocate")
-
-	// TestFailover is to test the failover to the TargetCluster without impacting the PrimaryCluster.
-	// This is a non-disruptive action and is used to validate the failover on the TargetCluster
-	// cluster before performing an actual failover.
-	ActionTestFailover = DRAction("TestFailover")
 )
 
 // DRState for keeping track of the DR placement
@@ -59,14 +54,6 @@ const (
 	// process has completed
 	FailedOver = DRState("FailedOver")
 
-	// TestFailover, state recorded in the DRPC status when the test failover
-	// is initiated
-	TestFailover = DRState("TestFailover")
-
-	// TestFailedOver, state recorded in the DRPC status when the test failover process
-	// is in progress but has not been completed yet
-	TestFailedOver = DRState("TestFailedOver")
-
 	// Relocating, state recorded in the DRPC status to indicate that the
 	// relocation is in progress
 	Relocating = DRState("Relocating")
@@ -172,6 +159,12 @@ type DRPlacementControlSpec struct {
 	// Action is either Failover or Relocate operation
 	Action DRAction `json:"action,omitempty"`
 
+	// DryRun when set to true, makes the action Failover non-destructive.
+	// The secondary is temporarily promoted to primary to verify readiness and data consistency
+	// without committing to the actual failover. Can be aborted to return to the original state.
+	// +kubebuilder:validation:Optional
+	DryRun bool `json:"dryRun,omitempty"`
+
 	// +optional
 	KubeObjectProtection *KubeObjectProtectionSpec `json:"kubeObjectProtection,omitempty"`
 

api/v1alpha1/volumereplicationgroup_types.go

@@ -160,7 +160,7 @@ type MoverConfig struct {
 }
 
 // VRGAction which will be either a Failover or Relocate
-// +kubebuilder:validation:Enum=Failover;Relocate;TestFailover
+// +kubebuilder:validation:Enum=Failover;Relocate
 type VRGAction string
 
 // These are the valid values for VRGAction
@@ -172,11 +172,6 @@ const (
 	// Relocate, VRG was relocated to/from this cluster,
 	// the to/from is determined by VRG spec.ReplicationState values of Primary/Secondary respectively
 	VRGActionRelocate = VRGAction("Relocate")
-
-	// TestFailover, VRG is in a test failover state where the secondary is temporarily promoted
-	// to primary to verify readiness and data consistency without committing to the actual failover.
-	// Test failover is non-destructive and can be aborted to return to the original state.
-	VRGActionTestFailover = VRGAction("TestFailover")
 )
 
 type KubeObjectProtectionSpec struct {
@@ -257,6 +252,10 @@ type VolumeReplicationGroupSpec struct {
 	// Action is either Failover or Relocate
 	//+optional
 	Action VRGAction `json:"action,omitempty"`
+	// DryRun indicates whether the action should be executed in test/non-destructive mode.
+	// When true, no permanent changes are made on the failover cluster.
+	//+optional
+	DryRun bool `json:"dryRun,omitempty"`
 	//+optional
 	KubeObjectProtection *KubeObjectProtectionSpec `json:"kubeObjectProtection,omitempty"`
 

config/crd/bases/ramendr.openshift.io_drplacementcontrols.yaml

@@ -79,7 +79,6 @@ spec:
                 enum:
                 - Failover
                 - Relocate
-                - TestFailover
                 type: string
               drPolicyRef:
                 description: DRPolicyRef is the reference to the DRPolicy participating
@@ -128,6 +127,12 @@ spec:
                 x-kubernetes-validations:
                 - message: drPolicyRef is immutable
                   rule: self == oldSelf
+              dryRun:
+                description: |-
+                  DryRun when set to true, makes the action (Failover or Relocate) non-destructive.
+                  The secondary is temporarily promoted to primary to verify readiness and data consistency
+                  without committing to the actual failover. Can be aborted to return to the original state.
+                type: boolean
               failoverCluster:
                 description: |-
                   FailoverCluster is the cluster name that the user wants to failover the application to.

config/crd/bases/ramendr.openshift.io_protectedvolumereplicationgrouplists.yaml

@@ -115,7 +115,6 @@ spec:
                           enum:
                           - Failover
                           - Relocate
-                          - TestFailover
                           type: string
                         async:
                           description: VRGAsyncSpec has the parameters associated
@@ -331,6 +330,11 @@ spec:
                           required:
                           - schedulingInterval
                           type: object
+                        dryRun:
+                          description: |-
+                            DryRun indicates whether the action should be executed in test/non-destructive mode.
+                            When true, no permanent changes are made to the replication group or data.
+                          type: boolean
                         kubeObjectProtection:
                           properties:
                             captureInterval:

config/crd/bases/ramendr.openshift.io_volumereplicationgroups.yaml

@@ -66,7 +66,6 @@ spec:
                 enum:
                 - Failover
                 - Relocate
-                - TestFailover
                 type: string
               async:
                 description: VRGAsyncSpec has the parameters associated with RegionalDR
@@ -281,6 +280,11 @@ spec:
                 required:
                 - schedulingInterval
                 type: object
+              dryRun:
+                description: |-
+                  DryRun indicates whether the action should be executed in test/non-destructive mode.
+                  When true, no permanent changes are made to the replication group or data.
+                type: boolean
               kubeObjectProtection:
                 properties:
                   captureInterval:

internal/controller/drplacementcontrol.go

@@ -110,15 +110,15 @@ func (d *DRPCInstance) processPlacement() (bool, error) {
 	d.log.Info("Process DRPC Placement", "DRAction", d.instance.Spec.Action)
 
 	// Handle test failover cleanup when switching away from test failover to another action
-	if d.instance.Spec.Action != rmn.ActionTestFailover {
+	if !d.instance.Spec.DryRun {
 		rmnutil.AddAnnotation(d.instance, "ramendr.openshift.io/last-action", string(d.instance.Spec.Action))
 
 		// If we're exiting a test failover (were in TestingFailover progression), clean up placement
 		// decisions that were retained during the test, then requeue for the new action.
 		if d.instance.Status.Progression == rmn.ProgressionTestingFailover {
 			for clusterName, vrg := range d.vrgs {
 				// Find the primary VRG that was used for testing and clean up after it
-				if isVRGPrimary(vrg) && vrg.Spec.Action == rmn.VRGActionTestFailover {
+				if isVRGPrimary(vrg) && vrg.Spec.DryRun {
 					// Remove cluster decision that was retained during test failover
 					if err := d.reconciler.removeClusterDecisionAfterTestFailover(d.ctx, d.userPlacement, clusterName); err != nil {
 						return false, err
@@ -143,8 +143,6 @@ func (d *DRPCInstance) executeAction() (bool, error) {
 		return d.RunFailover()
 	case rmn.ActionRelocate:
 		return d.RunRelocate()
-	case rmn.ActionTestFailover:
-		return d.RunTestFailover()
 	}
 
 	// Not a failover or a relocation.  Must be an initial deployment.
@@ -366,7 +364,7 @@ func (d *DRPCInstance) startDeploying(homeCluster, homeClusterNamespace string)
 // on the failover cluster
 // 3. Else, initiate failover to the desired failoverCluster (switchToFailoverCluster)
 //
-//nolint:cyclop,funlen,nestif
+//nolint:cyclop,funlen,nestif,gocognit
 func (d *DRPCInstance) RunFailover() (bool, error) {
 	d.log.Info("Entering RunFailover", "state", d.getLastDRState())
 
@@ -417,7 +415,7 @@ func (d *DRPCInstance) RunFailover() (bool, error) {
 			metav1.ConditionTrue, string(d.instance.Status.Phase), "Completed")
 
 		// Opltimize by adding a reconciler so that we reconcile at 1 minute at most.
-		if d.instance.Spec.Action == rmn.ActionTestFailover {
+		if d.instance.Spec.DryRun && d.instance.Spec.Action == rmn.ActionFailover {
 			d.setProgression(rmn.ProgressionTestingFailover)
 
 			if err := d.ensurePlacement(failoverCluster); err != nil {
@@ -1184,7 +1182,7 @@ func (d *DRPCInstance) areMultipleVRGsPrimary() bool {
 	numOfPrimaries := 0
 
 	for _, vrg := range d.vrgs {
-		if isVRGPrimary(vrg) && vrg.Spec.Action != rmn.VRGActionTestFailover {
+		if isVRGPrimary(vrg) && !vrg.Spec.DryRun {
 			numOfPrimaries++
 		}
 	}
@@ -1534,7 +1532,7 @@ func (d *DRPCInstance) updateUserPlacementRule(homeCluster, reason string) error
 		d.userPlacement.GetName(), homeCluster))
 
 	added := false
-	if d.instance.Spec.Action != rmn.ActionTestFailover {
+	if !d.instance.Spec.DryRun {
 		added = rmnutil.AddAnnotation(d.instance, LastAppDeploymentCluster, homeCluster)
 	}
 
@@ -1958,8 +1956,6 @@ func vrgAction(drpcAction rmn.DRAction) rmn.VRGAction {
 		return rmn.VRGActionFailover
 	case rmn.ActionRelocate:
 		return rmn.VRGActionRelocate
-	case rmn.ActionTestFailover:
-		return rmn.VRGActionTestFailover
 	default:
 		return ""
 	}
@@ -1972,6 +1968,9 @@ func (d *DRPCInstance) setVRGAction(vrg *rmn.VolumeReplicationGroup) {
 	}
 
 	vrg.Spec.Action = action
+	if action == rmn.VRGActionFailover {
+		vrg.Spec.DryRun = d.instance.Spec.DryRun
+	}
 }
 
 func (d *DRPCInstance) newVRG(
@@ -2546,25 +2545,11 @@ func (d *DRPCInstance) setDRState(nextState rmn.DRState) {
 }
 
 func (d *DRPCInstance) adjustPhaseIfTestFailover(nextState rmn.DRState) rmn.DRState {
-	if d.instance.Spec.Action == rmn.ActionTestFailover {
-		return mapPhaseForTestFailover(nextState)
-	}
-
+	// With the DryRun approach, we don't need to adjust phases for test failover
+	// The progression status (ProgressionTestingFailover) indicates test mode
 	return nextState
 }
 
-//nolint:exhaustive
-func mapPhaseForTestFailover(nextState rmn.DRState) rmn.DRState {
-	switch nextState {
-	case rmn.FailingOver:
-		return rmn.TestFailover
-	case rmn.FailedOver:
-		return rmn.TestFailedOver
-	default:
-		return nextState
-	}
-}
-
 func updateDRPCProgression(
 	drpc *rmn.DRPlacementControl, nextProgression rmn.ProgressionStatus, log logr.Logger,
 ) bool {

internal/controller/drplacementcontrol_controller.go

@@ -1577,10 +1577,11 @@ func (r *DRPlacementControlReconciler) clusterForVRGStatus(
 
 	switch drpc.Spec.Action {
 	case rmn.ActionFailover:
-	case rmn.ActionTestFailover:
 		// Failover can rely on inspecting VRG from clusterDecision as it is never made nil, hence till
 		// placementDecision is changed to failoverCluster, we can inspect VRG from the existing cluster
-		return clusterName
+		if drpc.Spec.DryRun {
+			return clusterName
+		}
 	case rmn.ActionRelocate:
 		if drpc.Status.ObservedGeneration != drpc.Generation {
 			log.Info("DPRC observedGeneration mismatches current generation, using ClusterDecision instead",

internal/controller/vrg_volrep.go

@@ -1263,8 +1263,7 @@ func (v *VRGInstance) autoResync(state volrep.ReplicationState) bool {
 		return false
 	}
 
-	if v.instance.Spec.Action != ramendrv1alpha1.VRGActionFailover &&
-		v.instance.Spec.Action != ramendrv1alpha1.VRGActionTestFailover {
+	if v.instance.Spec.Action != ramendrv1alpha1.VRGActionFailover {
 		return false
 	}
 

internal/controller/vrg_volsync.go

@@ -33,8 +33,7 @@ func (v *VRGInstance) restorePVsAndPVCsForVolSync() (int, error) {
 	numPVsRestored := 0
 
 	for _, rdSpec := range v.instance.Spec.VolSync.RDSpec {
-		failoverAction := (v.instance.Spec.Action == ramendrv1alpha1.VRGActionFailover) ||
-			(v.instance.Spec.Action == ramendrv1alpha1.VRGActionTestFailover)
+		failoverAction := v.instance.Spec.Action == ramendrv1alpha1.VRGActionFailover
 
 		var err error
 		// Source conditions are not needed and should not be added to vrg.status.ProtectedPVCs,