api: make the pvcSelector optional in the DRPlacementControl CRD #904
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When admission webhooks are enabled, then not having a omitempty tag on the pvcSelector causes the DRPC controller to update the field which is denied by the admission webhook. Signed-off-by: Raghavendra Talur <raghavendra.talur@gmail.com>
rakeshgm
approved these changes
May 31, 2023
|
Quick comment:
The issue seems to be an initial label selector that is actually empty, but carries an empty sub-struct, and a subsequent update is an empty selector in all. Potentially we could look at the webhook to be smarter in this regard and allow a variation of empty sub-structs in the label selector, than adding an omitempty to the field for the above reasons. |
BenamarMk
reviewed
Jun 23, 2023
| @@ -112,7 +112,7 @@ type DRPlacementControlSpec struct { | |||
| // Label selector to identify all the PVCs that need DR protection. | |||
| // This selector is assumed to be the same for all subscriptions that | |||
| // need DR protection. It will be passed in to the VRG when it is created | |||
| PVCSelector metav1.LabelSelector `json:"pvcSelector"` | |||
| PVCSelector metav1.LabelSelector `json:"pvcSelector,omitempty"` | |||
There was a problem hiding this comment.
Should not be allowed empty. If it is allowed, then by default everything in the namespace is protected. This will not work with VolSync (as its PVCs are on the same namespace), and it will not work with applications where all its PVCs should be protected. Kafka is an example of where zookeeper PVCs should NOT be protected.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When admission webhooks are enabled, then not having a omitempty tag on the pvcSelector causes the DRPC controller to update the field which is denied by the admission webhook.