-
Notifications
You must be signed in to change notification settings - Fork 39
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.
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
Ensure only one VR per PVC #213
Conversation
VolumeReplication should ensure there is only one VR per PVC, as otherwise it can lead to orchestrating the PVC based on multiple VRs to an inconsistent state. With this Patch only one VR will operator on a single PVC. Signed-off-by: Madhu Rajanna <[email protected]>
1e05d97
to
1fe592d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM !
As part of csi-addons#213 support was added to have one VR per PVC, once the VR is deleted we need to remove the annotation so new VR can be created for the same PVC. This PR adds the missing piece. Signed-off-by: Madhu Rajanna <[email protected]>
As part of csi-addons#213 support was added to have one VR per PVC, once the VR is deleted we need to remove the annotation so new VR can be created for the same PVC. This PR adds the missing piece. Signed-off-by: Madhu Rajanna <[email protected]>
As part of csi-addons#213 support was added to have one VR per PVC, once the VR is deleted we need to remove the annotation so new VR can be created for the same PVC. This PR adds the missing piece. Signed-off-by: Madhu Rajanna <[email protected]>
As part of #213 support was added to have one VR per PVC, once the VR is deleted we need to remove the annotation so new VR can be created for the same PVC. This PR adds the missing piece. Signed-off-by: Madhu Rajanna <[email protected]>
ownerName := pvc.ObjectMeta.Annotations[replicationv1alpha1.VolumeReplicationNameAnnotation] | ||
if ownerName == "" { | ||
pvc.ObjectMeta.Annotations[replicationv1alpha1.VolumeReplicationNameAnnotation] = req.Name | ||
err := r.Update(ctx, pvc) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 separate VR reconciles could update the annotation at the same time. (2 go routines etc.). This may need some rudimentary locking such that there is only one updater (fetch and update sequence) for a given PVC.
This should be quite rare, but something to keep track of.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree but am thinking if 2 VR acts on the same PVC to add annotation, either one will fail and one will pass and should be fine. what do you think? as its a quite rare condition we can see how to fix it.
As part of red-hat-storage#213 support was added to have one VR per PVC, once the VR is deleted we need to remove the annotation so new VR can be created for the same PVC. This PR adds the missing piece. Signed-off-by: Madhu Rajanna <[email protected]>
Syncing latest changes from main for kubernetes-csi-addons
VolumeReplication should ensure there is only one VR per PVC, as otherwise, it can lead to orchestrating the
PVC is based on multiple VRs to an inconsistent state. With this Patch only one VR will operate on a single PVC.
Signed-off-by: Madhu Rajanna [email protected]