Add checks to ensure existing PVC matches PVC to restore

Currently, the check for existing PVC, if found during PVC
restore, checks to see if PVC exists and is restored by Ramen.

This is insufficient in the case of a StatefulSet where the
PVC maybe left behind (similar to the PV) post a failover or
a relocate and on restore of the PVC if the older PVC is
found, it may not be restored by Ramen.

The checks for existing PVCs is hence expanded to ensure:
- PVC is not deleted
- PVC is bound
- PVC is bound to the same PV volume name we would have restored

The above ensures that we would have restored the same PVC and
it would have been bound to the PV as desired.

Also, the check to ensure that if the PVC exists it is restored
by Ramen is dropped, as that may fail in cases as mentioned
above.

Signed-off-by: Shyamsundar Ranganathan <[email protected]>
(cherry picked from commit 1805b30)

controllers/vrg_volrep.go

@@ -2018,20 +2018,32 @@ func (v *VRGInstance) validateExistingPV(pv *corev1.PersistentVolume) error {
 
 func (v *VRGInstance) validateExistingPVC(pvc *corev1.PersistentVolumeClaim) error {
 	existingPVC := &corev1.PersistentVolumeClaim{}
+	pvcNSName := types.NamespacedName{Name: pvc.Name, Namespace: pvc.Namespace}
 
-	err := v.reconciler.Get(v.ctx, types.NamespacedName{Name: pvc.Name, Namespace: pvc.Namespace}, existingPVC)
+	err := v.reconciler.Get(v.ctx, pvcNSName, existingPVC)
 	if err != nil {
-		return fmt.Errorf("failed to get existing PVC (%w)", err)
+		return fmt.Errorf("failed to get existing PVC %s (%w)", pvcNSName.String(), err)
 	}
 
-	if existingPVC.ObjectMeta.Annotations == nil ||
-		existingPVC.ObjectMeta.Annotations[RestoreAnnotation] != RestoredByRamen {
-		return fmt.Errorf("found PVC object not restored by Ramen for PVC %s", existingPVC.Name)
+	// If PVC not "Bound" return error
+	if existingPVC.Status.Phase != corev1.ClaimBound {
+		return fmt.Errorf("PVC %s exists and is not bound (phase: %s)", pvcNSName.String(), existingPVC.Status.Phase)
+	}
+
+	// If PVC is terminating return error
+	if !existingPVC.DeletionTimestamp.IsZero() {
+		return fmt.Errorf("existing bound PVC %s is being deleted", pvcNSName.String())
+	}
+
+	// Check match to the PV we expect PVC to be bound to
+	if existingPVC.Spec.VolumeName != pvc.Spec.VolumeName {
+		return fmt.Errorf("PVC %s exists and bound to a different PV %s than PV %s desired",
+			pvcNSName.String(), existingPVC.Spec.VolumeName, pvc.Spec.VolumeName)
 	}
 
 	// Should we check and see if PV in being deleted? Should we just treat it as exists
 	// and then we don't care if deletion takes place later, which is what we do now?
-	v.log.Info("PVC exists and managed by Ramen", "PVC", existingPVC)
+	v.log.Info(fmt.Sprintf("PVC %s exists and bound to desired PV %s", pvcNSName.String(), existingPVC.Spec.VolumeName))
 
 	return nil
 }