create RS irrespective of PV mount status.

Fix for [[2319824] [RDR] Replication via Volsync requires PVC to be mounted before PVC is synchronized](https://issues.redhat.com/browse/DFBUGS-580) Create ReplicationSource irrespective of PV being mounted to running pod. Signed-off-by: pruthvitd <[email protected]>
RamenDR · Dec 17, 2024 · 57fbcce · 57fbcce
1 parent 2ed6b0b
commit 57fbcce
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 51 deletions.
diff --git a/internal/controller/volsync/vshandler.go b/internal/controller/volsync/vshandler.go
@@ -350,11 +350,7 @@ func (v *VSHandler) ReconcileRS(rsSpec ramendrv1alpha1.VolSyncReplicationSourceS
 	return false, replicationSource, err
 }
 
-// Need to validate that our PVC is no longer in use before proceeding
-// If in final sync and the source PVC no longer exists, this could be from
-// a 2nd call to runFinalSync and we may have already cleaned up the PVC - so if pvc does not
-// exist, treat the same as not in use - continue on with reconcile of the RS (and therefore
-// check status to confirm final sync is complete)
+// Validate that the PVC is no longer in use before proceeding with the final sync.
 func (v *VSHandler) validatePVCBeforeRS(rsSpec ramendrv1alpha1.VolSyncReplicationSourceSpec,
 	runFinalSync bool) (bool, error,
 ) {
@@ -373,42 +369,11 @@ func (v *VSHandler) validatePVCBeforeRS(rsSpec ramendrv1alpha1.VolSyncReplicatio
 
 			return false, nil
 		}
-
-		return true, nil // Good to proceed - PVC is not in use, not mounted to node (or does not exist-should not happen)
-	}
-
-	// Not running final sync - if we have not yet created an RS for this PVC, then make sure a pod has mounted
-	// the PVC and is in "Running" state before attempting to create an RS.
-	// This is a best effort to confirm the app that is using the PVC is started before trying to replicate the PVC.
-	_, err := v.getRS(getReplicationSourceName(rsSpec.ProtectedPVC.Name), rsSpec.ProtectedPVC.Namespace)
-	if err != nil && kerrors.IsNotFound(err) {
-		l.Info("ReplicationSource does not exist yet. " +
-			"validating that the PVC to be protected is in use by a ready pod ...")
-		// RS does not yet exist - consider PVC is ok if it's mounted and in use by running pod
-		inUseByReadyPod, err := v.pvcExistsAndInUse(util.ProtectedPVCNamespacedName(rsSpec.ProtectedPVC),
-			true /* Check mounting pod is Ready */)
-		if err != nil {
-			return false, err
-		}
-
-		if !inUseByReadyPod {
-			l.Info("PVC is not in use by ready pod, not creating RS yet ...")
-
-			return false, nil
-		}
-
-		l.Info("PVC is use by ready pod, proceeding to create RS ...")
-
-		return true, nil
 	}
 
-	if err != nil {
-		// Err looking up the RS, return it
-		return false, err
-	}
-
-	// Replication source already exists, no need for any pvc checking
-	return true, nil
+	l.Info("Good to proceed with final sync.")
+
+	return true, nil  // Good to proceed - PVC is not in use, not mounted to node (or does not exist-should not happen)
 }
 
 func isFinalSyncComplete(replicationSource *volsyncv1alpha1.ReplicationSource, log logr.Logger) bool {

diff --git a/internal/controller/volsync/vshandler_test.go b/internal/controller/volsync/vshandler_test.go
@@ -793,41 +793,50 @@ var _ = Describe("VolSync_Handler", func() {
 				})
 
 				Context("When no running pod is mounting the PVC to be protected", func() {
-					It("Should return a nil replication source and no RS should be created", func() {
+					It("Should return a replication source and a RS should be created", func() {
 						// Run another reconcile - we have the psk secret now but the pvc is not in use by
 						// a running pod
 						finalSyncCompl, rs, err := vsHandler.ReconcileRS(rsSpec, false)
 						Expect(err).ToNot(HaveOccurred())
 						Expect(finalSyncCompl).To(BeFalse())
-						Expect(rs).To(BeNil())
+						Expect(rs).ToNot(BeNil())
 
-						// ReconcileRS should not have created the replication source - since the secret isn't there
+						// ReconcileRS should have created the replication source - since the secret isn't there
 						Consistently(func() error {
 							return k8sClient.Get(ctx,
 								types.NamespacedName{Name: rsSpec.ProtectedPVC.Name, Namespace: testNamespace.GetName()}, createdRS)
-						}, 1*time.Second, interval).ShouldNot(BeNil())
+						}, 1*time.Second, interval).Should(BeNil())
 					})
 				})
-
 				Context("When the PVC to be protected is mounted by a pod that is NOT in running phase", func() {
 					JustBeforeEach(func() {
 						// Create PVC and pod that is mounting it - pod phase will be "Pending"
 						createDummyPVCAndMountingPod(testPVCName, testNamespace.GetName(),
 							capacity, map[string]string{"a": "b"}, corev1.PodPending, false)
 					})
 
-					It("Should return a nil replication source and no RS should be created", func() {
+					It("Should return a replication source and RS should be created", func() {
 						// Run another reconcile - a pod is mounting the PVC but it is not in running phase
 						finalSyncCompl, rs, err := vsHandler.ReconcileRS(rsSpec, false)
 						Expect(err).ToNot(HaveOccurred())
 						Expect(finalSyncCompl).To(BeFalse())
-						Expect(rs).To(BeNil())
+						Expect(rs).ToNot(BeNil())
 
-						// ReconcileRS should not have created the RS - since the pod is not in running phase
+						// RS should be created with name=PVCName
+						Eventually(func() error {
+							return k8sClient.Get(ctx, types.NamespacedName{
+								Name:      rsSpec.ProtectedPVC.Name,
+								Namespace: testNamespace.GetName(),
+							}, createdRS)
+						}, maxWait, interval).Should(Succeed())
+
+						Expect(createdRS.Spec.RsyncTLS.StorageClassName).To(Equal(rsSpec.ProtectedPVC.StorageClassName))
+
+						// ReconcileRS should have created the RS - though the pod is not in running phase
 						Consistently(func() error {
 							return k8sClient.Get(ctx,
 								types.NamespacedName{Name: rsSpec.ProtectedPVC.Name, Namespace: testNamespace.GetName()}, createdRS)
-						}, 1*time.Second, interval).ShouldNot(BeNil())
+						}, 1*time.Second, interval).Should(BeNil())
 					})
 				})
 
@@ -844,13 +853,20 @@ var _ = Describe("VolSync_Handler", func() {
 						finalSyncCompl, rs, err := vsHandler.ReconcileRS(rsSpec, false)
 						Expect(err).ToNot(HaveOccurred())
 						Expect(finalSyncCompl).To(BeFalse())
-						Expect(rs).To(BeNil())
+						Expect(rs).ToNot(BeNil())
 
-						// ReconcileRS should not have created the RS - since the pod is not Ready
+						// RS should be created with name=PVCName
+						Eventually(func() error {
+							return k8sClient.Get(ctx, types.NamespacedName{
+								Name:      rsSpec.ProtectedPVC.Name,
+								Namespace: testNamespace.GetName(),
+							}, createdRS)
+						}, maxWait, interval).Should(Succeed())
+						// ReconcileRS should have created the RS - though the pod is not Ready
 						Consistently(func() error {
 							return k8sClient.Get(ctx,
 								types.NamespacedName{Name: rsSpec.ProtectedPVC.Name, Namespace: testNamespace.GetName()}, createdRS)
-						}, 1*time.Second, interval).ShouldNot(BeNil())
+						}, 1*time.Second, interval).Should(BeNil())
 					})
 				})