From be907f9375f5d0bdf814f94d67aab85f392d6c52 Mon Sep 17 00:00:00 2001 From: Benamar Mekhissi Date: Wed, 12 Jun 2024 07:47:42 -0400 Subject: [PATCH] Fix deletion of the wrong RD see comment: https://github.com/RamenDR/ramen/pull/1429#discussion_r1634820022 Signed-off-by: Benamar Mekhissi --- controllers/volsync/vshandler.go | 27 ++++++++++++++++++--------- controllers/volsync/vshandler_test.go | 12 ++++++++---- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/controllers/volsync/vshandler.go b/controllers/volsync/vshandler.go index 24975f752..08b066eed 100644 --- a/controllers/volsync/vshandler.go +++ b/controllers/volsync/vshandler.go @@ -42,7 +42,9 @@ const ( PodVolumePVCClaimIndexName string = "spec.volumes.persistentVolumeClaim.claimName" VolumeAttachmentToPVIndexName string = "spec.source.persistentVolumeName" - VRGOwnerLabel string = "volumereplicationgroups-owner" + VRGOwnerNameLabel string = "volumereplicationgroups-owner" + VRGOwnerNamespaceLabel string = "volumereplicationgroups-owner-namespace" + FinalSyncTriggerString string = "vrg-final-sync" SchedulingIntervalMinLength int = 2 @@ -209,7 +211,8 @@ func (v *VSHandler) createOrUpdateRD( } } - util.AddLabel(rd, VRGOwnerLabel, v.owner.GetName()) + util.AddLabel(rd, VRGOwnerNameLabel, v.owner.GetName()) + util.AddLabel(rd, VRGOwnerNamespaceLabel, v.owner.GetNamespace()) util.AddAnnotation(rd, OwnerNameAnnotation, v.owner.GetName()) util.AddAnnotation(rd, OwnerNamespaceAnnotation, v.owner.GetNamespace()) @@ -465,7 +468,8 @@ func (v *VSHandler) createOrUpdateRS(rsSpec ramendrv1alpha1.VolSyncReplicationSo } } - util.AddLabel(rs, VRGOwnerLabel, v.owner.GetName()) + util.AddLabel(rs, VRGOwnerNameLabel, v.owner.GetName()) + util.AddLabel(rs, VRGOwnerNamespaceLabel, v.owner.GetNamespace()) rs.Spec.SourcePVC = rsSpec.ProtectedPVC.Name @@ -854,7 +858,8 @@ func (v *VSHandler) CleanupRDNotInSpecList(rdSpecList []ramendrv1alpha1.VolSyncR foundInSpecList := false for _, rdSpec := range rdSpecList { - if rd.GetName() == getReplicationDestinationName(rdSpec.ProtectedPVC.Name) { + if rd.GetName() == getReplicationDestinationName(rdSpec.ProtectedPVC.Name) && + rd.GetNamespace() == rdSpec.ProtectedPVC.Namespace { foundInSpecList = true break @@ -946,10 +951,11 @@ func (v *VSHandler) listRDByOwner(rdNamespace string) (volsyncv1alpha1.Replicati return rdList, nil } -// Lists only RS/RD with VRGOwnerLabel that matches the owner +// Lists only RS/RD with VRGOwnerNameLabel that matches the owner func (v *VSHandler) listByOwner(list client.ObjectList, objNamespace string) error { matchLabels := map[string]string{ - VRGOwnerLabel: v.owner.GetName(), + VRGOwnerNameLabel: v.owner.GetName(), + VRGOwnerNamespaceLabel: v.owner.GetNamespace(), } listOptions := []client.ListOption{ client.InNamespace(objNamespace), @@ -1286,7 +1292,8 @@ func (v *VSHandler) validateAndProtectSnapshot( updater.AddOwner(v.owner, v.client.Scheme()) } - err = updater.AddLabel(VRGOwnerLabel, v.owner.GetName()). + err = updater.AddLabel(VRGOwnerNameLabel, v.owner.GetName()). + AddLabel(VRGOwnerNamespaceLabel, v.owner.GetNamespace()). AddLabel(VolSyncDoNotDeleteLabel, VolSyncDoNotDeleteLabelVal). Update(v.ctx, v.client) if err != nil { @@ -1786,7 +1793,8 @@ func (v *VSHandler) reconcileLocalRD(rdSpec ramendrv1alpha1.VolSyncReplicationDe } } - util.AddLabel(lrd, VRGOwnerLabel, v.owner.GetName()) + util.AddLabel(lrd, VRGOwnerNameLabel, v.owner.GetName()) + util.AddLabel(lrd, VRGOwnerNamespaceLabel, v.owner.GetNamespace()) util.AddLabel(lrd, VolSyncDoNotDeleteLabel, VolSyncDoNotDeleteLabelVal) pvcAccessModes := []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce} // Default value @@ -1864,7 +1872,8 @@ func (v *VSHandler) reconcileLocalRS(rd *volsyncv1alpha1.ReplicationDestination, } } - util.AddLabel(lrs, VRGOwnerLabel, v.owner.GetName()) + util.AddLabel(lrs, VRGOwnerNameLabel, v.owner.GetName()) + util.AddLabel(lrs, VRGOwnerNamespaceLabel, v.owner.GetNamespace()) // The name of the PVC is the same as the rd's latest snapshot name lrs.Spec.Trigger = &volsyncv1alpha1.ReplicationSourceTriggerSpec{ diff --git a/controllers/volsync/vshandler_test.go b/controllers/volsync/vshandler_test.go index 740e33906..865b68177 100644 --- a/controllers/volsync/vshandler_test.go +++ b/controllers/volsync/vshandler_test.go @@ -516,7 +516,8 @@ var _ = Describe("VolSync_Handler", func() { Namespace: testNamespace.GetName(), Labels: map[string]string{ // Need to simulate that it's owned by our VRG by using our label - volsync.VRGOwnerLabel: owner.GetName(), + volsync.VRGOwnerNameLabel: owner.GetName(), + volsync.VRGOwnerNamespaceLabel: owner.GetNamespace(), }, }, Spec: volsyncv1alpha1.ReplicationSourceSpec{}, @@ -571,7 +572,8 @@ var _ = Describe("VolSync_Handler", func() { Expect(*createdRD.Spec.RsyncTLS.StorageClassName).To(Equal(testStorageClassName)) Expect(*createdRD.Spec.RsyncTLS.VolumeSnapshotClassName).To(Equal(testVolumeSnapshotClassName)) Expect(createdRD.Spec.Trigger).To(BeNil()) // No schedule should be set - Expect(createdRD.GetLabels()).To(HaveKeyWithValue(volsync.VRGOwnerLabel, owner.GetName())) + Expect(createdRD.GetLabels()).To(HaveKeyWithValue(volsync.VRGOwnerNameLabel, owner.GetName())) + Expect(createdRD.GetLabels()).To(HaveKeyWithValue(volsync.VRGOwnerNamespaceLabel, owner.GetNamespace())) Expect(*createdRD.Spec.RsyncTLS.ServiceType).To(Equal(volsync.DefaultRsyncServiceType)) // Check that the secret has been updated to have our vrg as owner @@ -829,7 +831,8 @@ var _ = Describe("VolSync_Handler", func() { Namespace: testNamespace.GetName(), Labels: map[string]string{ // Need to simulate that it's owned by our VRG by using our label - volsync.VRGOwnerLabel: owner.GetName(), + volsync.VRGOwnerNameLabel: owner.GetName(), + volsync.VRGOwnerNamespaceLabel: owner.GetNamespace(), }, }, Spec: volsyncv1alpha1.ReplicationDestinationSpec{}, @@ -922,7 +925,8 @@ var _ = Describe("VolSync_Handler", func() { Expect(createdRS.Spec.Trigger).To(Equal(&volsyncv1alpha1.ReplicationSourceTriggerSpec{ Schedule: &expectedCronSpecSchedule, })) - Expect(createdRS.GetLabels()).To(HaveKeyWithValue(volsync.VRGOwnerLabel, owner.GetName())) + Expect(createdRS.GetLabels()).To(HaveKeyWithValue(volsync.VRGOwnerNameLabel, owner.GetName())) + Expect(createdRS.GetLabels()).To(HaveKeyWithValue(volsync.VRGOwnerNamespaceLabel, owner.GetNamespace())) }) It("Should create an ReplicationSource if one does not exist", func() {