Skip to content

Commit

Permalink
Revert back partial of code that deals with LocalDirect
Browse files Browse the repository at this point in the history 
LocalDirect is not longer needed in favor of 'Direct' copyMethod

Signed-off-by: Benamar Mekhissi <[email protected]>
  • Loading branch information
Benamar Mekhissi authored and DF Build Team committed Oct 28, 2023
1 parent 41f7fe2 commit 1d0f7ec
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 197 deletions.
44 changes: 14 additions & 30 deletions controllers/volsync/vshandler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,6 @@ const (

OwnerNameAnnotation = "ramendr.openshift.io/owner-name"
OwnerNamespaceAnnotation = "ramendr.openshift.io/owner-namespace"

CopyMethodLocalDirect = "LocalDirect"
)

type VSHandler struct {
Expand Down Expand Up @@ -98,8 +96,6 @@ func NewVSHandler(ctx context.Context, client client.Client, log logr.Logger, ow

// returns replication destination only if create/update is successful and the RD is considered available.
// Callers should assume getting a nil replication destination back means they should retry/requeue.
//
//nolint:gocognit,cyclop
func (v *VSHandler) ReconcileRD(
rdSpec ramendrv1alpha1.VolSyncReplicationDestinationSpec) (*volsyncv1alpha1.ReplicationDestination, error,
) {
Expand Down Expand Up @@ -146,16 +142,8 @@ func (v *VSHandler) ReconcileRD(
return nil, nil
}

l.V(1).Info(fmt.Sprintf("Copy method: %s", v.destinationCopyMethod))

if v.IsCopyMethodLocalDirect() {
lrd, lrs, err := v.reconcileLocalReplication(rd, rdSpec, pskSecretName, l)
if lrd == nil || lrs == nil || err != nil {
return nil, err
}
}

l.V(1).Info(fmt.Sprintf("ReplicationDestination Reconcile Complete rd=%s", rd.Name))
l.V(1).Info(fmt.Sprintf("ReplicationDestination Reconcile Complete rd=%s, Copy method: %s",
rd.Name, v.destinationCopyMethod))

return rd, nil
}
Expand Down Expand Up @@ -505,12 +493,12 @@ func (v *VSHandler) createOrUpdateRS(rsSpec ramendrv1alpha1.VolSyncReplicationSo
return rs, nil
}

func (v *VSHandler) PreparePVC(pvcName string, prepFinalSync, copyMethodDirectOrLocalDirect bool) error {
if prepFinalSync || copyMethodDirectOrLocalDirect {
func (v *VSHandler) PreparePVC(pvcName string, prepFinalSync, copyMethodDirect bool) error {
if prepFinalSync || copyMethodDirect {
prepared, err := v.TakePVCOwnership(pvcName)
if err != nil || !prepared {
return fmt.Errorf("waiting to take pvc ownership (%w), prepFinalSync: %t, DirectOrLocalDirect: %t",
err, prepFinalSync, copyMethodDirectOrLocalDirect)
return fmt.Errorf("waiting to take pvc ownership (%w), prepFinalSync: %t, Direct: %t",
err, prepFinalSync, copyMethodDirect)
}
}

Expand Down Expand Up @@ -726,7 +714,7 @@ func (v *VSHandler) DeleteRD(pvcName string) error {
rd := currentRDListByOwner.Items[i]

if rd.GetName() == getReplicationDestinationName(pvcName) {
if v.IsCopyMethodDirectOrLocalDirect() {
if v.IsCopyMethodDirect() {
err := v.deleteLocalRDAndRS(&rd)
if err != nil {
return err
Expand Down Expand Up @@ -772,7 +760,7 @@ func (v *VSHandler) deleteLocalRDAndRS(rd *volsyncv1alpha1.ReplicationDestinatio
return err
}

// For LocalDirect, localRS trigger must point to the latest RD snapshot image. Otherwise,
// For Local Direct, localRS trigger must point to the latest RD snapshot image. Otherwise,
// we wait for local final sync to take place first befor cleaning up.
if lrs.Spec.Trigger != nil && lrs.Spec.Trigger.Manual == latestRDImage.Name {
// When local final sync is complete, we cleanup all locally created resources except the app PVC
Expand Down Expand Up @@ -1013,7 +1001,7 @@ func (v *VSHandler) validateSnapshotAndEnsurePVC(rdSpec ramendrv1alpha1.VolSyncR
return err
}

if v.IsCopyMethodDirectOrLocalDirect() {
if v.IsCopyMethodDirect() {
// Directly use the RD pvc
v.log.V(1).Info(fmt.Sprintf("Using copyMethod '%s'. latestImage %s. pvcName %s",
v.destinationCopyMethod, snapshotRef.Name, rdSpec.ProtectedPVC.Name))
Expand Down Expand Up @@ -1641,14 +1629,6 @@ func (v *VSHandler) IsCopyMethodDirect() bool {
return v.destinationCopyMethod == volsyncv1alpha1.CopyMethodDirect
}

func (v *VSHandler) IsCopyMethodLocalDirect() bool {
return v.destinationCopyMethod == CopyMethodLocalDirect
}

func (v *VSHandler) IsCopyMethodDirectOrLocalDirect() bool {
return v.IsCopyMethodDirect() || v.IsCopyMethodLocalDirect()
}

func isLatestImageReady(latestImage *corev1.TypedLocalObjectReference) bool {
if latestImage == nil || latestImage.Name == "" || latestImage.Kind != VolumeSnapshotKind {
return false
Expand Down Expand Up @@ -2131,6 +2111,10 @@ func (v *VSHandler) pauseRD(rdName string) (*volsyncv1alpha1.ReplicationDestinat
return nil, err
}

if rd.Spec.Paused {
return rd, nil
}

rd.Spec.Paused = true

return rd, v.updateResource(rd)
Expand Down Expand Up @@ -2158,7 +2142,7 @@ func (v *VSHandler) checkLastSnapshotSyncStatus(lrs *volsyncv1alpha1.Replication
const completed = true

v.log.V(1).Info("Local RS trigger", "trigger", lrs.Spec.Trigger, "snapName", snapshotRef.Name)
// For LocalDirect, localRS trigger must point to the latest RD snapshot image. Otherwise,
// For Local Direct, localRS trigger must point to the latest RD snapshot image. Otherwise,
// we wait for local final sync to take place first befor cleaning up.
if lrs.Spec.Trigger != nil && lrs.Spec.Trigger.Manual == snapshotRef.Name {
// When local final sync is complete, we cleanup all locally created resources except the app PVC
Expand Down
96 changes: 0 additions & 96 deletions controllers/volsync/vshandler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -647,102 +647,6 @@ var _ = Describe("VolSync_Handler", func() {
})
})
})
Context("When reconciling RD for local replication", func() {
var vsHandler *volsync.VSHandler
var dummyPSKSecret *corev1.Secret
myTestAddress := "https://fakeaddress.abc.org:8888"
BeforeEach(func() {
vsHandler = volsync.NewVSHandler(ctx, k8sClient, logger, owner, asyncSpec,
"openshift-storage.cephfs.csi.ceph.com", "LocalDirect")

dummyPSKSecret = &corev1.Secret{
ObjectMeta: metav1.ObjectMeta{
Name: volsync.GetVolSyncPSKSecretNameFromVRGName(owner.GetName()),
Namespace: testNamespace.GetName(),
},
}
Expect(k8sClient.Create(ctx, dummyPSKSecret)).To(Succeed())
Expect(dummyPSKSecret.GetName()).NotTo(BeEmpty())

// Make sure the secret is created to avoid any timing issues
Eventually(func() error {
return k8sClient.Get(ctx,
types.NamespacedName{
Name: dummyPSKSecret.GetName(),
Namespace: dummyPSKSecret.GetNamespace(),
}, dummyPSKSecret)
}, maxWait, interval).Should(Succeed())

// Run ReconcileRD
var err error
returnedRD, err = vsHandler.ReconcileRD(rdSpec)
Expect(err).ToNot(HaveOccurred())

// RD should be created with name=PVCName
Eventually(func() error {
return k8sClient.Get(ctx, types.NamespacedName{
Name: rdSpec.ProtectedPVC.Name,
Namespace: testNamespace.GetName(),
}, createdRD)
}, maxWait, interval).Should(Succeed())

// Expect the RD should be owned by owner
Expect(ownerMatches(createdRD, owner.GetName(), "ConfigMap", true /*should be controller*/)).To(BeTrue())

// Fake the address and latestImage in the status
createdRD.Status = &volsyncv1alpha1.ReplicationDestinationStatus{
RsyncTLS: &volsyncv1alpha1.ReplicationDestinationRsyncTLSStatus{
Address: &myTestAddress,
},
}
Expect(k8sClient.Status().Update(ctx, createdRD)).To(Succeed())

Eventually(func() *string {
err := k8sClient.Get(ctx, client.ObjectKeyFromObject(createdRD), createdRD)
if err != nil || createdRD.Status == nil || createdRD.Status.RsyncTLS == nil {
return nil
}

return createdRD.Status.RsyncTLS.Address
}, maxWait, interval).Should(Not(BeNil()))
})
It("Should create lRD and lRS", func() {
_, err := vsHandler.ReconcileRD(rdSpec)
Expect(err).ToNot(HaveOccurred())

// Local RD should be created with name=PVCName-local
localRD := &volsyncv1alpha1.ReplicationDestination{}
Eventually(func() error {
return k8sClient.Get(ctx, types.NamespacedName{
Name: rdSpec.ProtectedPVC.Name + "-local",
Namespace: testNamespace.GetName(),
}, localRD)
}, maxWait, interval).Should(Succeed())

Expect(localRD.Spec.RsyncTLS).NotTo(BeNil())
Expect(localRD.Spec.RsyncTLS.CopyMethod).To(Equal(volsyncv1alpha1.CopyMethodDirect))

// Fake the address and latestImage in the status
localRD.Status = &volsyncv1alpha1.ReplicationDestinationStatus{
RsyncTLS: &volsyncv1alpha1.ReplicationDestinationRsyncTLSStatus{
Address: &myTestAddress,
},
}
Expect(k8sClient.Status().Update(ctx, localRD)).To(Succeed())

Eventually(func() *string {
err := k8sClient.Get(ctx, client.ObjectKeyFromObject(localRD), localRD)
if err != nil || localRD.Status == nil || localRD.Status.RsyncTLS == nil {
return nil
}

return localRD.Status.RsyncTLS.Address
}, maxWait, interval).Should(Not(BeNil()))

Expect(localRD.Status.RsyncTLS).NotTo(BeNil())
Expect(localRD.Status.RsyncTLS.Address).NotTo(BeNil())
})
})

Context("With CopyMethod 'Direct'", func() {
var vsHandler *volsync.VSHandler
Expand Down
70 changes: 0 additions & 70 deletions controllers/volumereplicationgroup_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,6 @@ func (r *VolumeReplicationGroupReconciler) SetupWithManager(
handler.EnqueueRequestsFromMapFunc(r.pvcMapFunc),
builder.WithPredicates(pvcPredicateFunc()),
).
Watches(&source.Kind{Type: &corev1.PersistentVolumeClaim{}},
handler.EnqueueRequestsFromMapFunc(r.rdMapFunc),
builder.WithPredicates(replicationDestinationPredicateFunc()),
).
Watches(&source.Kind{Type: &corev1.ConfigMap{}}, handler.EnqueueRequestsFromMapFunc(r.configMapFun)).
Owns(&volrep.VolumeReplication{})

Expand Down Expand Up @@ -305,72 +301,6 @@ func filterPVC(reader client.Reader, pvc *corev1.PersistentVolumeClaim, log logr
return req
}

func replicationDestinationPredicateFunc() predicate.Funcs {
log := ctrl.Log.WithName("RDPredicate").WithName("RD")
rdPredicate := predicate.Funcs{
CreateFunc: func(e event.CreateEvent) bool {
return false
},
GenericFunc: func(e event.GenericEvent) bool {
return false
},
DeleteFunc: func(e event.DeleteEvent) bool {
log.Info("Delete event")

return true
},
UpdateFunc: func(e event.UpdateEvent) bool {
oldRD, ok := e.ObjectOld.DeepCopyObject().(*volsyncv1alpha1.ReplicationDestination)
if !ok {
log.Info("Failed to deep copy older MCV")

return false
}
newRD, ok := e.ObjectNew.DeepCopyObject().(*volsyncv1alpha1.ReplicationDestination)
if !ok {
log.Info("Failed to deep copy newer MCV")

return false
}

log.Info(fmt.Sprintf("Update event for MCV %s/%s", oldRD.Name, oldRD.Namespace))

return oldRD.Status.LatestImage.Name != newRD.Status.LatestImage.Name
},
}

return rdPredicate
}

func (r *VolumeReplicationGroupReconciler) rdMapFunc(obj client.Object) []reconcile.Request {
log := ctrl.Log.WithName("rdmap").WithName("VolumeReplicationGroup")

rd, ok := obj.(*volsyncv1alpha1.ReplicationDestination)
if !ok {
log.Info("ReplicationDestination (RD) map function received non-RD resource")

return []reconcile.Request{}
}

return filterRD(rd)
}

func filterRD(rd *volsyncv1alpha1.ReplicationDestination) []ctrl.Request {
if rd.Annotations[volsync.OwnerNameAnnotation] == "" ||
rd.Annotations[volsync.OwnerNamespaceAnnotation] == "" {
return []ctrl.Request{}
}

return []ctrl.Request{
reconcile.Request{
NamespacedName: types.NamespacedName{
Name: rd.Annotations[volsync.OwnerNameAnnotation],
Namespace: rd.Annotations[volsync.OwnerNamespaceAnnotation],
},
},
}
}

func GetPVCLabelSelector(
ctx context.Context, reader client.Reader, vrg ramendrv1alpha1.VolumeReplicationGroup, log logr.Logger,
) (metav1.LabelSelector, error) {
Expand Down
2 changes: 1 addition & 1 deletion controllers/vrg_volsync.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ func (v *VRGInstance) reconcilePVCAsVolSyncPrimary(pvc corev1.PersistentVolumeCl
}

err := v.volSyncHandler.PreparePVC(pvc.Name, v.instance.Spec.PrepareForFinalSync,
v.volSyncHandler.IsCopyMethodDirectOrLocalDirect())
v.volSyncHandler.IsCopyMethodDirect())
if err != nil {
return true
}
Expand Down

0 comments on commit 1d0f7ec

Please sign in to comment.