Ensure PVC is deselected only when not in both VR and VS lists #2121
Conversation
Current deselection code checks if a PVC is part of VR PVC lists and if not invokes deselection. This was "assumed" safe as there was a finalizer check for the PVC to ensure it is protected by VR before progressing. This check was deep in the code, and was not applicable to PVCs that were marked for CG based protection. It was also not safe as it removed the PVC from the protectedPVC status unconditionally. Further, if 8000 the PVC was part of VolSync protection, the above was unconditionally executed, and caused PVC protection issues for VolSync PVCs. The fix provided, ensures that a PVC is NOT protected by either VolSync or VR based protection, before deselecting the same. Further it uses the VR finalizer on the PVC to determine of it was protected by VR or by VolSync to invoke the right deselection function. Additional changes: The envtests for deselect/delete of PVCs did not account for an eventual clause to wait for VRG to report that the PVC is no loner protected, before checking S3 for related contents to be absent. This is also fixed as part of this commit. Signed-off-by: Shyamsundar Ranganathan <srangana@redhat.com>
|
@ShyamsundarR I'll build a patch and run it on the QE system where we are seeing this issue. |
| _, pvcUnderVRProtection := pvcsVr[pvcNamespacedName] | ||
| _, pvcUnderVSProtection := pvcsVs[pvcNamespacedName] | ||
|
|
||
| if pvcUnderVRProtection || pvcUnderVSProtection { |
There was a problem hiding this comment.
If the PVC is deleted, but the label is still there, it will still be in both lists (pvcsOwned and either pvcsVr or pvcsVr). That means we will not unprotect the deleted PVC as the condition will evaluate to true.
There was a problem hiding this comment.
Deleted PVCs go through the un-protection flow from reconcileVolRepsAsPrimary and pvcUnprotectVolSyncIfDeleted, so you are correct that in this code path they will be in one of the lists and get processed later for deletion. This (pvcsDeselectedUnprotect) function is only for deselected PVCs.
@ShyamsundarR The patch worked. It looks good to me, other than the one comment. |
Current deselection code checks if a PVC is part of VR PVC lists and if not invokes deselection. This was "assumed" safe as there was a finalizer check for the PVC to ensure it is protected by VR before progressing. This check was deep in the code, and was not applicable to PVCs that were marked for CG based protection. It was also not safe as it removed the PVC from the protectedPVC status unconditionally.
Further, if the PVC was part of VolSync protection, the above was unconditionally executed, and caused PVC protection issues for VolSync PVCs.
The fix provided, ensures that a PVC is NOT protected by either VolSync or VR based protection, before deselecting the same. Further it uses the VR finalizer on the PVC to determine of it was protected by VR or by VolSync to invoke the right deselection function.
Additional changes:
The envtests for deselect/delete of PVCs did not account for an eventual clause to wait for VRG to report that the PVC is no loner protected, before checking S3 for related contents to be absent. This is also fixed as part of this commit.