Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bug 2224325: Add checks to ensure existing PVC matches PVC to restore #117

Merged
merged 2 commits into from
Aug 2, 2023
Merged

Bug 2224325: Add checks to ensure existing PVC matches PVC to restore #117

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
68859eb
Add checks to ensure existing PVC matches PVC to restore
ShyamsundarR Jul 20, 2023
a4fe104
Add comment to function validateExistingPV[C] functions
ShyamsundarR Jul 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions controllers/vrg_volrep.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1959,6 +1959,8 @@ func (v *VRGInstance) updateExistingPVForSync(pv *corev1.PersistentVolume) error
return nil
}

// validateExistingPV validates if an existing PV matches the passed in PV for certain fields. Returns error
// if a match fails or a match is not possible given the state of the existing PV
func (v *VRGInstance) validateExistingPV(pv *corev1.PersistentVolume) error {
log := v.log.WithValues("PV", pv.Name)

Expand Down Expand Up @@ -2016,22 +2018,31 @@ func (v *VRGInstance) validateExistingPV(pv *corev1.PersistentVolume) error {
return fmt.Errorf("found existing PV (%s) not restored by Ramen and not matching with backed up PV", existingPV.Name)
}

// validateExistingPVC validates if an existing PVC matches the passed in PVC for certain fields. Returns error
// if a match fails or a match is not possible given the state of the existing PVC
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 existingPVC.Status.Phase != corev1.ClaimBound {
return fmt.Errorf("PVC %s exists and is not bound (phase: %s)", pvcNSName.String(), existingPVC.Status.Phase)
}

// 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)
if !existingPVC.DeletionTimestamp.IsZero() {
return fmt.Errorf("existing bound PVC %s is being deleted", pvcNSName.String())
}

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)
}

v.log.Info(fmt.Sprintf("PVC %s exists and bound to desired PV %s", pvcNSName.String(), existingPVC.Spec.VolumeName))

return nil
}
Expand Down