Skip to content

Commit

Permalink
create RS irrespective of PV mount status.
Browse files Browse the repository at this point in the history 
	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.
        * Renamed "validatePVCBeforeRS" as "validatePVCForFinalSync"

Signed-off-by: pruthvitd <prd@redhat.com>

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 <prd@redhat.com>
  • Loading branch information
@pruthvitd @BenamarMk
pruthvitd authored and BenamarMk committed Dec 18, 2024
1 parent 8f2da93 commit a896bb1
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 54 deletions.
45 changes: 3 additions & 42 deletions internal/controller/volsync/vshandler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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
}

Expand Down
40 changes: 28 additions & 12 deletions internal/controller/volsync/vshandler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())
})
})

Expand All @@ -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())
})
})

Expand Down

0 comments on commit a896bb1

Please sign in to comment.