Disown volsync managed pvcs upon vrg deletion

This commit introduces changes to disown PVC resources when a VRG is deleted,
particularly in scenarios where the DR is disabled. The motivation behind this
adjustment is to give back the management of the PVC lifecycle to OCM.

Upon disabling DR, the VRG now iterates through all volsync-managed PVCs.
It removes VRG ownership from these PVCs and reinstates OCM annotations.
This ensures that PVCs are not prematurely garbage collected upon VRG deletion.

In order for that to work, a new annotation needs to be added to the VRG to
indicate that the PVCs should be disowned upon DR disabling
drplacementcontrol.ramendr.openshift.io/do-not-delete-pvc: "true"

Signed-off-by: Benamar Mekhissi <[email protected]>

Author: BenamarMk

controllers/drplacementcontrol.go

@@ -1524,6 +1524,7 @@ func (d *DRPCInstance) generateVRG(dstCluster string, repState rmn.ReplicationSt
 			Namespace: d.vrgNamespace,
 			Annotations: map[string]string{
 				DestinationClusterAnnotationKey: dstCluster,
+				DoNotDeletePVCAnnotation:        d.instance.GetAnnotations()[DoNotDeletePVCAnnotation],
 			},
 		},
 		Spec: rmn.VolumeReplicationGroupSpec{

controllers/drplacementcontrol_controller.go

@@ -59,6 +59,9 @@ const (
 	MaxPlacementDecisionConflictCount = 5
 
 	DestinationClusterAnnotationKey = "drplacementcontrol.ramendr.openshift.io/destination-cluster"
+
+	DoNotDeletePVCAnnotation    = "drplacementcontrol.ramendr.openshift.io/do-not-delete-pvc"
+	DoNotDeletePVCAnnotationVal = "true"
 )
 
 var InitialWaitTimeForDRPCPlacementRule = errorswrapper.New("Waiting for DRPC Placement to produces placement decision")

controllers/volsync/vshandler.go

@@ -521,22 +521,6 @@ func (v *VSHandler) TakePVCOwnership(pvcName string) (bool, error) {
 		return false, err
 	}
 
-	// Remove acm annotations from the PVC just as a precaution - ACM uses the annotations to track that the
-	// pvc is owned by an application.  Removing them should not be necessary now that we are adding
-	// the do-not-delete annotation.  With the do-not-delete annotation (see ACMAppSubDoNotDeleteAnnotation), ACM
-	// should not delete the pvc when the application is removed.
-	updatedAnnotations := map[string]string{}
-
-	for currAnnotationKey, currAnnotationValue := range pvc.Annotations {
-		// We want to only preserve annotations not from ACM (i.e. remove all ACM annotations to break ownership)
-		if !strings.HasPrefix(currAnnotationKey, "apps.open-cluster-management.io") ||
-			currAnnotationKey == ACMAppSubDoNotDeleteAnnotation {
-			updatedAnnotations[currAnnotationKey] = currAnnotationValue
-		}
-	}
-
-	pvc.Annotations = updatedAnnotations
-
 	err = v.client.Update(v.ctx, pvc)
 	if err != nil {
 		l.Error(err, "Error updating annotations on PVC to break appsub ownership")
@@ -578,21 +562,21 @@ func (v *VSHandler) pvcExistsAndInUse(pvcName string, inUsePodMustBeReady bool)
 	return util.IsPVAttachedToNode(v.ctx, v.client, v.log, pvc)
 }
 
-func (v *VSHandler) pvcExists(pvcName string) (bool, error) {
-	_, err := v.getPVC(pvcName)
+func (v *VSHandler) pvcExists(pvcName string) (bool, *corev1.PersistentVolumeClaim, error) {
+	pvc, err := v.getPVC(pvcName)
 	if err != nil {
 		if !kerrors.IsNotFound(err) {
 			v.log.V(1).Info("failed to get PVC")
 
-			return false, err
+			return false, pvc, err
 		}
 
-		return false, nil
+		return false, pvc, nil
 	}
 
 	v.log.V(1).Info("PVC found")
 
-	return true, nil
+	return true, pvc, nil
 }
 
 func (v *VSHandler) getPVC(pvcName string) (*corev1.PersistentVolumeClaim, error) {
@@ -946,16 +930,16 @@ func (v *VSHandler) EnsurePVCforDirectCopy(ctx context.Context,
 		return fmt.Errorf("capacity must be provided %v", rdSpec.ProtectedPVC)
 	}
 
-	exists, err := v.pvcExists(rdSpec.ProtectedPVC.Name)
+	exists, pvc, err := v.pvcExists(rdSpec.ProtectedPVC.Name)
 	if err != nil {
 		return err
 	}
 
 	if exists {
-		return nil
+		return v.removeOCMAnnotationsAndUpdate(pvc)
 	}
 
-	pvc := &corev1.PersistentVolumeClaim{
+	pvc = &corev1.PersistentVolumeClaim{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      rdSpec.ProtectedPVC.Name,
 			Namespace: v.owner.GetNamespace(),
@@ -1043,6 +1027,17 @@ func (v *VSHandler) validateSnapshotAndEnsurePVC(rdSpec ramendrv1alpha1.VolSyncR
 		}
 	}
 
+	pvc, err := v.getPVC(rdSpec.ProtectedPVC.Name)
+	if err != nil {
+		return err
+	}
+
+	// Once the PVC is restored/rolled back, need to re-add the annotations from old Primary
+	err = v.addBackOCMAnnotationsAndUpdate(pvc, rdSpec.ProtectedPVC.Annotations)
+	if err != nil {
+		return err
+	}
+
 	// Add ownerRef on snapshot pointing to the vrg - if/when the VRG gets cleaned up, then GC can cleanup the snap
 	return v.addOwnerReferenceAndUpdate(snap, v.owner)
 }
@@ -2110,3 +2105,46 @@ func (v *VSHandler) checkLastSnapshotSyncStatus(lrs *volsyncv1alpha1.Replication
 
 	return !completed
 }
+
+func (v *VSHandler) DisownVolSyncManagedPVC(pvc *corev1.PersistentVolumeClaim) error {
+	// TODO: Remove just the VRG ownerReference instead of blindly removing all ownerreferences.
+	// For now, this is fine, given that the VRG is the sole owner of the PVC after DR is enabled.
+	pvc.ObjectMeta.OwnerReferences = nil
+	v.deleteAnnotation(pvc, ACMAppSubDoNotDeleteAnnotation)
+
+	return v.client.Update(v.ctx, pvc)
+}
+
+func (v *VSHandler) deleteAnnotation(obj client.Object, annotationName string) {
+	annotations := obj.GetAnnotations()
+	delete(annotations, annotationName)
+	obj.SetAnnotations(annotations)
+}
+
+func (v *VSHandler) addBackOCMAnnotationsAndUpdate(obj client.Object, annotations map[string]string) error {
+	updatedAnnotations := obj.GetAnnotations()
+
+	for key, val := range annotations {
+		if strings.HasPrefix(key, "apps.open-cluster-management.io") {
+			updatedAnnotations[key] = val
+		}
+	}
+
+	obj.SetAnnotations(updatedAnnotations)
+
+	return v.client.Update(v.ctx, obj)
+}
+
+func (v *VSHandler) removeOCMAnnotationsAndUpdate(obj client.Object) error {
+	updatedAnnotations := map[string]string{}
+
+	for key, val := range obj.GetAnnotations() {
+		if !strings.HasPrefix(key, "apps.open-cluster-management.io") {
+			updatedAnnotations[key] = val
+		}
+	}
+
+	obj.SetAnnotations(updatedAnnotations)
+
+	return v.client.Update(v.ctx, obj)
+}

controllers/volsync/vshandler_test.go

@@ -1807,25 +1807,6 @@ var _ = Describe("VolSync_Handler", func() {
 				Expect(pvcPreparationErr).ToNot(HaveOccurred())
 				Expect(pvcPreparationComplete).To(BeTrue())
 
-				Eventually(func() int {
-					err := k8sClient.Get(ctx, client.ObjectKeyFromObject(testPVC), testPVC)
-					if err != nil {
-						return 0
-					}
-
-					return len(testPVC.Annotations)
-				}, maxWait, interval).Should(Equal(len(initialAnnotations) - 2))
-				// We had 2 acm annotations in initialAnnotations
-
-				for key, val := range testPVC.Annotations {
-					if key != volsync.ACMAppSubDoNotDeleteAnnotation {
-						// Other ACM annotations should be deleted
-						Expect(strings.HasPrefix(key, "apps.open-cluster-management.io")).To(BeFalse())
-
-						Expect(initialAnnotations[key]).To(Equal(val)) // Other annotations should still be there
-					}
-				}
-
 				// ACM do-not-delete annotation should be added to the PVC
 				pvcAnnotations := testPVC.GetAnnotations()
 				val, ok := pvcAnnotations[volsync.ACMAppSubDoNotDeleteAnnotation]

controllers/volumereplicationgroup_controller.go

@@ -774,6 +774,12 @@ func (v *VRGInstance) processForDeletion() ctrl.Result {
 
 	defer v.log.Info("Exiting processing VolumeReplicationGroup")
 
+	if err := v.disownPVCs(); err != nil {
+		v.log.Info("Disowning PVCs failed", "error", err)
+
+		return ctrl.Result{Requeue: true}
+	}
+
 	if !containsString(v.instance.ObjectMeta.Finalizers, vrgFinalizerName) {
 		v.log.Info("Finalizer missing from resource", "finalizer", vrgFinalizerName)
 

controllers/vrg_volsync.go

@@ -402,3 +402,21 @@ func (v *VRGInstance) pvcUnprotectVolSync(pvc corev1.PersistentVolumeClaim, log
 	// TODO Delete ReplicationSource, ReplicationDestination, etc.
 	v.pvcStatusDeleteIfPresent(pvc.Namespace, pvc.Name, log)
 }
+
+// disownPVCs this function is disassociating all PVCs (targeted for VolSync replication) from its owner (VRG)
+func (v *VRGInstance) disownPVCs() error {
+	if v.instance.GetAnnotations()[DoNotDeletePVCAnnotation] != DoNotDeletePVCAnnotationVal {
+		return nil
+	}
+
+	for idx := range v.volSyncPVCs {
+		pvc := &v.volSyncPVCs[idx]
+
+		err := v.volSyncHandler.DisownVolSyncManagedPVC(pvc)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}