diff --git a/internal/controller/volsync/vshandler.go b/internal/controller/volsync/vshandler.go index 3382dddff..03b1f9b1e 100644 --- a/internal/controller/volsync/vshandler.go +++ b/internal/controller/volsync/vshandler.go @@ -319,7 +319,7 @@ func (v *VSHandler) ReconcileRS(rsSpec ramendrv1alpha1.VolSyncReplicationSourceS return false, nil, err } - pvcOk, err := v.validatePVCBeforeRS(rsSpec, runFinalSync) + pvcOk, err := v.validatePVCForFinalSync(rsSpec, runFinalSync) if !pvcOk || err != nil { // Return the replicationSource if it already exists existingRS, getRSErr := v.getRS(getReplicationSourceName(rsSpec.ProtectedPVC.Name), rsSpec.ProtectedPVC.Namespace) @@ -350,16 +350,10 @@ 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) -func (v *VSHandler) validatePVCBeforeRS(rsSpec ramendrv1alpha1.VolSyncReplicationSourceSpec, +// Validate that the PVC is no longer in use before proceeding with the final sync. +func (v *VSHandler) validatePVCForFinalSync(rsSpec ramendrv1alpha1.VolSyncReplicationSourceSpec, runFinalSync bool) (bool, error, ) { - l := v.log.WithValues("rsSpec", rsSpec, "runFinalSync", runFinalSync) - if runFinalSync { // If runFinalSync, check the PVC and make sure it's not mounted to a pod // as we want the app to be quiesced/removed before running final sync @@ -373,41 +367,8 @@ 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 } diff --git a/internal/controller/volsync/vshandler_test.go b/internal/controller/volsync/vshandler_test.go index 1e5e5ec01..67f0dc8be 100644 --- a/internal/controller/volsync/vshandler_test.go +++ b/internal/controller/volsync/vshandler_test.go @@ -793,22 +793,21 @@ 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" @@ -816,18 +815,28 @@ var _ = Describe("VolSync_Handler", func() { 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()) }) })