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

Syncing latest changes from upstream main for ramen

Author: ShyamsundarR

controllers/drcluster_controller.go

@@ -149,22 +149,17 @@ func (r *DRClusterReconciler) drClusterConfigMapMapFunc(
 	return requests
 }
 
-func (r *DRClusterReconciler) drClusterSecretMapFunc(ctx context.Context, secret client.Object) []reconcile.Request {
-	if secret.GetNamespace() != NamespaceName() {
+func (r *DRClusterReconciler) drClusterSecretMapFunc(ctx context.Context, obj client.Object) []reconcile.Request {
+	if obj.GetNamespace() != NamespaceName() {
 		return []reconcile.Request{}
 	}
 
-	drcusters := &ramen.DRClusterList{}
-	if err := r.Client.List(context.TODO(), drcusters); err != nil {
+	secret, ok := obj.(*corev1.Secret)
+	if !ok {
 		return []reconcile.Request{}
 	}
 
-	requests := make([]reconcile.Request, len(drcusters.Items))
-	for i := range drcusters.Items {
-		requests[i].Name = drcusters.Items[i].GetName()
-	}
-
-	return requests
+	return filterDRClusterSecret(ctx, r.Client, secret)
 }
 
 // drpcPred watches for updates to the DRPC resource and checks if it requires an appropriate DRCluster reconcile
@@ -277,6 +272,46 @@ func filterDRClusterMCV(mcv *viewv1beta1.ManagedClusterView) []ctrl.Request {
 	}
 }
 
+func filterDRClusterSecret(ctx context.Context, reader client.Reader, secret *corev1.Secret) []ctrl.Request {
+	log := ctrl.Log.WithName("filterDRClusterSecret").WithName("Secret")
+
+	drclusters := &ramen.DRClusterList{}
+	if err := reader.List(ctx, drclusters); err != nil {
+		log.Error(err, "Failed to list DRClusters")
+
+		return []reconcile.Request{}
+	}
+
+	requests := []reconcile.Request{}
+
+	for i := range drclusters.Items {
+		drcluster := &drclusters.Items[i]
+
+		s3ProfileName := drcluster.Spec.S3ProfileName
+
+		if s3ProfileName == NoS3StoreAvailable {
+			continue
+		}
+
+		s3StoreProfile, err := GetRamenConfigS3StoreProfile(context.TODO(), reader, s3ProfileName)
+		if err != nil {
+			log.Info("Failed to filter secret", "secret", secret.GetName(), "drcluster", drcluster.Name, "reason", err.Error())
+
+			continue
+		}
+
+		if secret.GetName() == s3StoreProfile.S3SecretRef.Name {
+			requests = append(requests,
+				reconcile.Request{
+					NamespacedName: types.NamespacedName{Name: drcluster.GetName()},
+				},
+			)
+		}
+	}
+
+	return requests
+}
+
 //nolint:lll
 // +kubebuilder:rbac:groups=ramendr.openshift.io,resources=drclusters,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=ramendr.openshift.io,resources=drclusters/status,verbs=get;update;patch

controllers/drplacementcontrol_controller.go

@@ -1666,6 +1666,7 @@ func (r *DRPlacementControlReconciler) getStatusCheckDelay(
 	return time.Until(beforeProcessing.Add(StatusCheckDelay))
 }
 
+//nolint:cyclop
 func (r *DRPlacementControlReconciler) updateDRPCStatus(
 	ctx context.Context, drpc *rmn.DRPlacementControl, userPlacement client.Object, log logr.Logger,
 ) error {
@@ -1681,9 +1682,12 @@ func (r *DRPlacementControlReconciler) updateDRPCStatus(
 		r.updateResourceCondition(drpc, clusterDecision.ClusterName, vrgNamespace, log)
 	}
 
-	if err := r.setDRPCMetrics(ctx, drpc, log); err != nil {
-		// log the error but do not return the error
-		log.Info("failed to set drpc metrics", "errMSg", err)
+	// do not set metrics if DRPC is being deleted
+	if !isBeingDeleted(drpc, userPlacement) {
+		if err := r.setDRPCMetrics(ctx, drpc, log); err != nil {
+			// log the error but do not return the error
+			log.Info("failed to set drpc metrics", "errMSg", err)
+		}
 	}
 
 	for i, condition := range drpc.Status.Conditions {

controllers/volumereplicationgroup_controller.go

@@ -578,55 +578,32 @@ func (v *VRGInstance) validateVRGMode() error {
 	return nil
 }
 
-func (v *VRGInstance) clusterDataRestore(result *ctrl.Result) error {
-	if v.instance.Spec.PrepareForFinalSync || v.instance.Spec.RunFinalSync {
-		msg := "PV restore skipped, as VRG is orchestrating final sync"
-		setVRGClusterDataReadyCondition(&v.instance.Status.Conditions, v.instance.Generation, msg)
-
-		return nil
-	}
+func (v *VRGInstance) clusterDataRestore(result *ctrl.Result) (int, error) {
+	v.log.Info("Restoring PVs and PVCs")
 
-	clusterDataReady := findCondition(v.instance.Status.Conditions, VRGConditionTypeClusterDataReady)
-	if clusterDataReady != nil {
-		v.log.Info("ClusterDataReady condition",
-			"status", clusterDataReady.Status,
-			"reason", clusterDataReady.Reason,
-			"message", clusterDataReady.Message,
-			"observedGeneration", clusterDataReady.ObservedGeneration,
-			"generation", v.instance.Generation,
-		)
-
-		if clusterDataReady.Status == metav1.ConditionTrue &&
-			clusterDataReady.ObservedGeneration == v.instance.Generation {
-			v.log.Info("VRG's ClusterDataReady condition found. PV restore must have already been applied")
-
-			return nil
-		}
-	} else {
-		v.log.Info("ClusterDataReady condition absent")
-	}
-
-	err := v.restorePVsForVolSync()
+	numRestoredForVS, err := v.restorePVsAndPVCsForVolSync()
 	if err != nil {
-		v.log.Info("VolSync PV restore failed")
+		v.log.Info("VolSync PV/PVC restore failed")
 
-		result.Requeue = true
-
-		return fmt.Errorf("failed to restore PVs for VolSync (%w)", err)
+		return numRestoredForVS, fmt.Errorf("failed to restore PV/PVC for VolSync (%w)", err)
 	}
 
-	err = v.clusterDataRestoreForVolRep(result)
+	numRestoredForVR, err := v.restorePVsAndPVCsForVolRep(result)
 	if err != nil {
-		v.log.Info("VolRep ClusterData restore failed")
+		v.log.Info("VolRep PV/PVC restore failed")
 
-		return fmt.Errorf("failed to restore ClusterData for VolRep (%w)", err)
+		return numRestoredForVS + numRestoredForVR, fmt.Errorf("failed to restore PV/PVC for VolRep (%w)", err)
 	}
 
 	// Only after both succeed, we mark ClusterDataReady as true
-	msg := "Restored cluster data"
+	msg := "Restored PVs and PVCs"
+	if numRestoredForVS+numRestoredForVR == 0 {
+		msg = "Nothing to restore"
+	}
+
 	setVRGClusterDataReadyCondition(&v.instance.Status.Conditions, v.instance.Generation, msg)
 
-	return nil
+	return numRestoredForVS + numRestoredForVR, nil
 }
 
 func (v *VRGInstance) listPVCsByVrgPVCSelector() (*corev1.PersistentVolumeClaimList, error) {
@@ -877,8 +854,18 @@ func (v *VRGInstance) processAsPrimary() ctrl.Result {
 		return v.dataError(err, "PVCs deselected unprotect failed", v.result.Requeue)
 	}
 
-	if err := v.clusterDataRestore(&v.result); err != nil {
-		return v.clusterDataError(err, "Failed to restore PVs", v.result)
+	if v.shouldRestoreClusterData() {
+		v.result.Requeue = true
+
+		numOfRestoredRes, err := v.clusterDataRestore(&v.result)
+		if err != nil {
+			return v.clusterDataError(err, "Failed to restore PVs/PVCs", v.result)
+		}
+
+		// Save status and requeue if we restored any resources (PV/PVCs). Otherwise, continue
+		if numOfRestoredRes != 0 {
+			return v.updateVRGConditionsAndStatus(v.result)
+		}
 	}
 
 	v.reconcileAsPrimary()
@@ -895,6 +882,35 @@ func (v *VRGInstance) processAsPrimary() ctrl.Result {
 	return v.updateVRGConditionsAndStatus(v.result)
 }
 
+func (v *VRGInstance) shouldRestoreClusterData() bool {
+	if v.instance.Spec.PrepareForFinalSync || v.instance.Spec.RunFinalSync {
+		msg := "PV restore skipped, as VRG is orchestrating final sync"
+		setVRGClusterDataReadyCondition(&v.instance.Status.Conditions, v.instance.Generation, msg)
+
+		return false
+	}
+
+	clusterDataReady := findCondition(v.instance.Status.Conditions, VRGConditionTypeClusterDataReady)
+	if clusterDataReady != nil {
+		v.log.Info("ClusterDataReady condition",
+			"status", clusterDataReady.Status,
+			"reason", clusterDataReady.Reason,
+			"message", clusterDataReady.Message,
+			"observedGeneration", clusterDataReady.ObservedGeneration,
+			"generation", v.instance.Generation,
+		)
+
+		if clusterDataReady.Status == metav1.ConditionTrue &&
+			clusterDataReady.ObservedGeneration == v.instance.Generation {
+			v.log.Info("VRG's ClusterDataReady condition found. PV restore must have already been applied")
+
+			return false
+		}
+	}
+
+	return true
+}
+
 func (v *VRGInstance) reconcileAsPrimary() {
 	var finalSyncPrepared struct {
 		volSync bool

controllers/vrg_recipe_test.go

@@ -269,7 +269,7 @@ var _ = Describe("VolumeReplicationGroupRecipe", func() {
 		return matchers
 	}
 	vrgPvcsConsistOfEventually := func(pvcs ...*corev1.PersistentVolumeClaim) {
-		Eventually(vrgPvcsGet).Should(ConsistOf(vrgPvcNamesMatchPvcs(pvcs...)))
+		Eventually(vrgPvcsGet, timeout, interval).Should(ConsistOf(vrgPvcNamesMatchPvcs(pvcs...)))
 	}
 	vrgPvcSelectorGet := func() (controllers.PvcSelector, error) {
 		return controllers.GetPVCSelector(ctx, apiReader, *vrg, *ramenConfig, testLogger)
@@ -538,7 +538,7 @@ var _ = Describe("VolumeReplicationGroupRecipe", func() {
 							Expect(pvcSelector.NamespaceNames).To(ConsistOf(vrg.Namespace))
 						})
 						It("sets DataReady condition's message to something besides a recipe error", func() {
-							Eventually(vrgDataReadyConditionGetAndExpectNonNil).Should(MatchFields(IgnoreExtras, Fields{
+							Eventually(vrgDataReadyConditionGetAndExpectNonNil, timeout, interval).Should(MatchFields(IgnoreExtras, Fields{
 								"Message": Not(HavePrefix(recipeErrorMessagePrefix)),
 							}))
 						})

controllers/vrg_volrep.go

@@ -1817,30 +1817,31 @@ func (v *VRGInstance) s3KeyPrefix() string {
 	return S3KeyPrefix(v.namespacedName)
 }
 
-func (v *VRGInstance) clusterDataRestoreForVolRep(result *ctrl.Result) error {
+func (v *VRGInstance) restorePVsAndPVCsForVolRep(result *ctrl.Result) (int, error) {
 	v.log.Info("Restoring VolRep PVs and PVCs")
 
 	if len(v.instance.Spec.S3Profiles) == 0 {
 		v.log.Info("No S3 profiles configured")
 
 		result.Requeue = true
 
-		return fmt.Errorf("no S3Profiles configured")
+		return 0, fmt.Errorf("no S3Profiles configured")
 	}
 
 	v.log.Info(fmt.Sprintf("Restoring PVs and PVCs to this managed cluster. ProfileList: %v", v.instance.Spec.S3Profiles))
 
-	if err := v.restorePVsAndPVCsFromS3(result); err != nil {
+	count, err := v.restorePVsAndPVCsFromS3(result)
+	if err != nil {
 		errMsg := fmt.Sprintf("failed to restore PVs and PVCs using profile list (%v)", v.instance.Spec.S3Profiles)
 		v.log.Info(errMsg)
 
-		return fmt.Errorf("%s: %w", errMsg, err)
+		return 0, fmt.Errorf("%s: %w", errMsg, err)
 	}
 
-	return nil
+	return count, nil
 }
 
-func (v *VRGInstance) restorePVsAndPVCsFromS3(result *ctrl.Result) error {
+func (v *VRGInstance) restorePVsAndPVCsFromS3(result *ctrl.Result) (int, error) {
 	err := errors.New("s3Profiles empty")
 	NoS3 := false
 
@@ -1891,16 +1892,16 @@ func (v *VRGInstance) restorePVsAndPVCsFromS3(result *ctrl.Result) error {
 
 		v.log.Info(fmt.Sprintf("Restored %d PVs and %d PVCs using profile %s", pvCount, pvcCount, s3ProfileName))
 
-		return v.kubeObjectsRecover(result, s3StoreProfile, objectStore)
+		return pvCount + pvcCount, v.kubeObjectsRecover(result, s3StoreProfile, objectStore)
 	}
 
 	if NoS3 {
-		return nil
+		return 0, nil
 	}
 
 	result.Requeue = true
 
-	return err
+	return 0, err
 }
 
 func (v *VRGInstance) restorePVsFromObjectStore(objectStore ObjectStorer, s3ProfileName string) (int, error) {

controllers/vrg_volrep_test.go

@@ -335,7 +335,7 @@ var _ = Describe("VolumeReplicationGroupVolRepController", func() {
 						Expect(vrg.GetGeneration()).To(Equal(vrgGenerationNext))
 
 						return vrg.Status.ObservedGeneration
-					}).Should(Equal(vrgGenerationNext))
+					}, timeout, interval).Should(Equal(vrgGenerationNext))
 				})
 				It("sets PVC's namespace name in VRG status", func() {
 					Expect(vrg.Status.ProtectedPVCs).To(HaveLen(len(t.pvcNames)))

controllers/vrg_volsync.go

@@ -15,13 +15,13 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
-func (v *VRGInstance) restorePVsForVolSync() error {
+func (v *VRGInstance) restorePVsAndPVCsForVolSync() (int, error) {
 	v.log.Info("VolSync: Restoring VolSync PVs")
 
 	if len(v.instance.Spec.VolSync.RDSpec) == 0 {
 		v.log.Info("No RDSpec entries. There are no PVCs to restore")
 		// No ReplicationDestinations (i.e. no PVCs) to restore
-		return nil
+		return 0, nil
 	}
 
 	numPVsRestored := 0
@@ -59,12 +59,12 @@ func (v *VRGInstance) restorePVsForVolSync() error {
 	}
 
 	if numPVsRestored != len(v.instance.Spec.VolSync.RDSpec) {
-		return fmt.Errorf("failed to restore all PVCs using RDSpec (%v)", v.instance.Spec.VolSync.RDSpec)
+		return numPVsRestored, fmt.Errorf("failed to restore all PVCs using RDSpec (%v)", v.instance.Spec.VolSync.RDSpec)
 	}
 
 	v.log.Info("Success restoring VolSync PVs", "Total", numPVsRestored)
 
-	return nil
+	return numPVsRestored, nil
 }
 
 func (v *VRGInstance) reconcileVolSyncAsPrimary(finalSyncPrepared *bool) (requeue bool) {