Merge pull request #423 from red-hat-storage/sync_us--main

Syncing latest changes from upstream main for ramen

.github/workflows/dependency-review.yaml

@@ -10,7 +10,7 @@ permissions:
 
 jobs:
   dependency-review:
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-24.04
     steps:
       - name: 'Checkout Repository'
         uses: actions/checkout@v4

.github/workflows/ramen.yaml

@@ -32,7 +32,7 @@ defaults:
 jobs:
   lint:
     name: Linters
-    runs-on: ubuntu-20.04
+    runs-on: ubuntu-24.04
     steps:
       - name: Checkout source
         uses: actions/checkout@v4
@@ -67,7 +67,7 @@ jobs:
 
   golangci:
     name: Golangci Lint
-    runs-on: ubuntu-20.04
+    runs-on: ubuntu-24.04
     strategy:
       matrix:
         directory: [., api, e2e]
@@ -98,7 +98,7 @@ jobs:
 
   unit-test:
     name: Unit tests
-    runs-on: ubuntu-20.04
+    runs-on: ubuntu-24.04
     steps:
       - name: Checkout source
         uses: actions/checkout@v4
@@ -113,7 +113,7 @@ jobs:
 
   build-image:
     name: Build image
-    runs-on: ubuntu-20.04
+    runs-on: ubuntu-24.04
     steps:
       - name: Checkout source
         uses: actions/checkout@v4
@@ -139,7 +139,7 @@ jobs:
   deploy-check:
     name: Check artifacts and operator deployment
     needs: [build-image]
-    runs-on: ubuntu-20.04
+    runs-on: ubuntu-24.04
     strategy:
       fail-fast: false
       matrix:
@@ -216,7 +216,7 @@ jobs:
       (github.ref == 'refs/heads/main' ||
        startsWith(github.ref, 'refs/heads/release-') ||
        startsWith(github.ref, 'refs/tags/v'))
-    runs-on: ubuntu-20.04
+    runs-on: ubuntu-24.04
     steps:
       - name: Download image artifact
         uses: actions/download-artifact@v4

.github/workflows/tools.yaml

@@ -25,7 +25,7 @@ jobs:
     strategy:
       matrix:
         os:
-          - ubuntu-22.04
+          - ubuntu-24.04
         python-version:
           - "3.10"
           - "3.11"
@@ -150,7 +150,7 @@ jobs:
     strategy:
       matrix:
         os:
-          - ubuntu-22.04
+          - ubuntu-24.04
         python-version:
           - "3.10"
           - "3.11"

e2e/dractions/actions.go

@@ -5,7 +5,6 @@ package dractions
 
 import (
 	"strings"
-	"time"
 
 	ramen "github.com/ramendr/ramen/api/v1alpha1"
 	"github.com/ramendr/ramen/e2e/deployers"
@@ -17,8 +16,6 @@ import (
 
 const (
 	OcmSchedulingDisable = "cluster.open-cluster-management.io/experimental-scheduling-disable"
-
-	FiveSecondsDuration = 5 * time.Second
 )
 
 // If AppSet/Subscription, find Placement
@@ -126,7 +123,7 @@ func Failover(ctx types.Context) error {
 
 	log.Info("Failing over workload")
 
-	return failoverRelocate(ctx, ramen.ActionFailover)
+	return failoverRelocate(ctx, ramen.ActionFailover, ramen.FailedOver)
 }
 
 // Determine DRPC
@@ -143,24 +140,23 @@ func Relocate(ctx types.Context) error {
 
 	log.Info("Relocating workload")
 
-	return failoverRelocate(ctx, ramen.ActionRelocate)
+	return failoverRelocate(ctx, ramen.ActionRelocate, ramen.Relocated)
 }
 
-func failoverRelocate(ctx types.Context, action ramen.DRAction) error {
-	name := ctx.Name()
+func failoverRelocate(ctx types.Context, action ramen.DRAction, state ramen.DRState) error {
+	drpcName := ctx.Name()
 	namespace := ctx.Namespace()
-	drpcName := name
 	client := util.Ctx.Hub.Client
 
 	if err := waitAndUpdateDRPC(ctx, client, namespace, drpcName, action); err != nil {
 		return err
 	}
 
-	if action == ramen.ActionFailover {
-		return waitDRPC(ctx, client, namespace, name, ramen.FailedOver)
+	if err := waitDRPCPhase(ctx, client, namespace, drpcName, state); err != nil {
+		return err
 	}
 
-	return waitDRPC(ctx, client, namespace, name, ramen.Relocated)
+	return waitDRPCReady(ctx, client, namespace, drpcName)
 }
 
 func waitAndUpdateDRPC(

e2e/dractions/actionsdiscoveredapps.go

@@ -90,18 +90,18 @@ func FailoverDiscoveredApps(ctx types.Context) error {
 	log := ctx.Logger()
 	log.Info("Failing over workload")
 
-	return failoverRelocateDiscoveredApps(ctx, ramen.ActionFailover)
+	return failoverRelocateDiscoveredApps(ctx, ramen.ActionFailover, ramen.FailedOver)
 }
 
 func RelocateDiscoveredApps(ctx types.Context) error {
 	log := ctx.Logger()
 	log.Info("Relocating workload")
 
-	return failoverRelocateDiscoveredApps(ctx, ramen.ActionRelocate)
+	return failoverRelocateDiscoveredApps(ctx, ramen.ActionRelocate, ramen.Relocated)
 }
 
 // nolint:funlen,cyclop
-func failoverRelocateDiscoveredApps(ctx types.Context, action ramen.DRAction) error {
+func failoverRelocateDiscoveredApps(ctx types.Context, action ramen.DRAction, state ramen.DRState) error {
 	name := ctx.Name()
 	log := ctx.Logger()
 	namespace := ctx.Namespace() // this namespace is in hub
@@ -142,11 +142,11 @@ func failoverRelocateDiscoveredApps(ctx types.Context, action ramen.DRAction) er
 		return err
 	}
 
-	if err = waitDRPCProgression(ctx, client, namespace, name, ramen.ProgressionCompleted); err != nil {
+	if err := waitDRPCPhase(ctx, client, namespace, name, state); err != nil {
 		return err
 	}
 
-	if err = waitDRPCReady(ctx, client, namespace, name); err != nil {
+	if err := waitDRPCReady(ctx, client, namespace, name); err != nil {
 		return err
 	}
 

e2e/dractions/retry.go

@@ -12,6 +12,8 @@ import (
 	"github.com/ramendr/ramen/e2e/types"
 	"github.com/ramendr/ramen/e2e/util"
 	"k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"open-cluster-management.io/api/cluster/v1beta1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -48,58 +50,36 @@ func waitDRPCReady(ctx types.Context, client client.Client, namespace string, dr
 	log := ctx.Logger()
 	startTime := time.Now()
 
+	log.Info("Waiting until drpc is ready")
+
 	for {
 		drpc, err := getDRPC(client, namespace, drpcName)
 		if err != nil {
 			return err
 		}
 
-		conditionReady := checkDRPCConditions(drpc)
-		if conditionReady && drpc.Status.LastGroupSyncTime != nil {
+		available := conditionMet(drpc.Status.Conditions, ramen.ConditionAvailable)
+		peerReady := conditionMet(drpc.Status.Conditions, ramen.ConditionPeerReady)
+
+		if available && peerReady && drpc.Status.LastGroupSyncTime != nil {
 			log.Info("drpc is ready")
 
 			return nil
 		}
 
 		if time.Since(startTime) > util.Timeout {
-			if !conditionReady {
-				log.Info("drpc condition 'Available' or 'PeerReady' is not True")
-			}
-
-			if conditionReady && drpc.Status.LastGroupSyncTime == nil {
-				log.Info("drpc LastGroupSyncTime is nil")
-			}
-
-			return fmt.Errorf("drpc %q is not ready yet before timeout, fail", drpcName)
+			return fmt.Errorf("timeout waiting for drpc to become ready (Available: %v, PeerReady: %v, lastGroupSyncTime: %v)",
+				available, peerReady, drpc.Status.LastGroupSyncTime)
 		}
 
 		time.Sleep(util.RetryInterval)
 	}
 }
 
-func checkDRPCConditions(drpc *ramen.DRPlacementControl) bool {
-	available := false
-	peerReady := false
-
-	for _, cond := range drpc.Status.Conditions {
-		if cond.Type == "Available" {
-			if cond.Status != "True" {
-				return false
-			}
-
-			available = true
-		}
-
-		if cond.Type == "PeerReady" {
-			if cond.Status != "True" {
-				return false
-			}
+func conditionMet(conditions []metav1.Condition, conditionType string) bool {
+	condition := meta.FindStatusCondition(conditions, conditionType)
 
-			peerReady = true
-		}
-	}
-
-	return available && peerReady
+	return condition != nil && condition.Status == "True"
 }
 
 func waitDRPCPhase(ctx types.Context, client client.Client, namespace, name string, phase ramen.DRState) error {
@@ -161,16 +141,6 @@ func getTargetCluster(client client.Client, namespace, placementName string, drp
 	return targetCluster, nil
 }
 
-// first wait DRPC to have the expected phase, then check DRPC conditions
-func waitDRPC(ctx types.Context, client client.Client, namespace, name string, expectedPhase ramen.DRState) error {
-	// check Phase
-	if err := waitDRPCPhase(ctx, client, namespace, name, expectedPhase); err != nil {
-		return err
-	}
-	// then check Conditions
-	return waitDRPCReady(ctx, client, namespace, name)
-}
-
 func waitDRPCDeleted(ctx types.Context, client client.Client, namespace string, name string) error {
 	log := ctx.Logger()
 	startTime := time.Now()

internal/controller/drplacementcontrol.go

@@ -398,6 +398,9 @@ func (d *DRPCInstance) isValidFailoverTarget(cluster string) bool {
 
 	vrg, err := d.reconciler.MCVGetter.GetVRGFromManagedCluster(d.instance.Name, d.vrgNamespace, cluster, annotations)
 	if err != nil {
+		d.log.Info("Failed to get VRG from managed cluster", "name", d.instance.Name, "namespace", d.vrgNamespace,
+			"cluster", cluster, "annotations", annotations, "error", err)
+
 		return false
 	}
 

internal/controller/drplacementcontrol_controller.go

@@ -724,15 +724,15 @@ func (r *DRPlacementControlReconciler) cleanupVRGs(
 		return fmt.Errorf("failed to retrieve VRGs. We'll retry later. Error (%w)", err)
 	}
 
-	if !ensureVRGsManagedByDRPC(r.Log, mwu, vrgs, drpc, vrgNamespace) {
-		return fmt.Errorf("VRG adoption in progress")
+	// We have to ensure the secondary VRG is deleted before deleting the primary VRG. This will fail until there
+	// is no secondary VRG in the vrgs list.
+	if err := r.ensureVRGsDeleted(mwu, vrgs, drpc, vrgNamespace, rmn.Secondary); err != nil {
+		return err
 	}
 
-	// delete VRG manifestwork
-	for _, drClusterName := range rmnutil.DRPolicyClusterNames(drPolicy) {
-		if err := mwu.DeleteManifestWork(mwu.BuildManifestWorkName(rmnutil.MWTypeVRG), drClusterName); err != nil {
-			return fmt.Errorf("%w", err)
-		}
+	// This will fail until there is no primary VRG in the vrgs list.
+	if err := r.ensureVRGsDeleted(mwu, vrgs, drpc, vrgNamespace, rmn.Primary); err != nil {
+		return err
 	}
 
 	if len(vrgs) != 0 {
@@ -747,6 +747,38 @@ func (r *DRPlacementControlReconciler) cleanupVRGs(
 	return nil
 }
 
+// ensureVRGsDeleted ensure that secondary or primary VRGs are deleted. Return an error if a vrg could not be deleted,
+// or deletion is in progress. Return nil if vrg of specified type was not found.
+func (r *DRPlacementControlReconciler) ensureVRGsDeleted(
+	mwu rmnutil.MWUtil,
+	vrgs map[string]*rmn.VolumeReplicationGroup,
+	drpc *rmn.DRPlacementControl,
+	vrgNamespace string,
+	replicationState rmn.ReplicationState,
+) error {
+	var inProgress bool
+
+	for cluster, vrg := range vrgs {
+		if vrg.Spec.ReplicationState == replicationState {
+			if !ensureVRGsManagedByDRPC(r.Log, mwu, vrgs, drpc, vrgNamespace) {
+				return fmt.Errorf("%s VRG adoption in progress", replicationState)
+			}
+
+			if err := mwu.DeleteManifestWork(mwu.BuildManifestWorkName(rmnutil.MWTypeVRG), cluster); err != nil {
+				return fmt.Errorf("failed to delete %s VRG manifestwork for cluster %q: %w", replicationState, cluster, err)
+			}
+
+			inProgress = true
+		}
+	}
+
+	if inProgress {
+		return fmt.Errorf("%s VRG manifestwork deletion in progress", replicationState)
+	}
+
+	return nil
+}
+
 func (r *DRPlacementControlReconciler) deleteAllManagedClusterViews(
 	drpc *rmn.DRPlacementControl, clusterNames []string,
 ) error {