-
Notifications
You must be signed in to change notification settings - Fork 446
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
Support labelSelectors to filter out services before syncing to the host cluster #2378
base: main
Are you sure you want to change the base?
Support labelSelectors to filter out services before syncing to the host cluster #2378
Conversation
✅ Deploy Preview for vcluster-docs canceled.Built without sensitive environment variables
|
a35c144
to
9907de7
Compare
Signed-off-by: abhay.tomar <[email protected]> Signed-off-by: abhay.tomar <[email protected]> Signed-off-by: abhay.tomar <[email protected]>
Signed-off-by: abhay.tomar <[email protected]>
9907de7
to
bb8ba3f
Compare
Signed-off-by: abhay.tomar <[email protected]>
@@ -500,10 +501,21 @@ type EnableSwitchWithPatches struct { | |||
// Enabled defines if this option should be enabled. | |||
Enabled bool `json:"enabled,omitempty"` | |||
|
|||
// Selector defines the labelSelector based filtering for syncing the resources to the host cluster. | |||
Selector *SyncSelector `json:"selector,omitempty"` |
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.
Adding this here would also apply to other resources that use EnableSwitchWithPatches, is that wanted?
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.
We can have a separate object specifically for this purpose. But I've added this in EnableSwitchWithPatches, keeping in mind that it would provide us an option to enable such selector based filtering for other resources as well. And the user can user let it be empty if they don't require any filtering.
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.
I'm not sure if we actually want to do this, whats the use case for allowing something like this? Services wouldn't work at all if they are not synced, so it seems to me this is not something we want to support
@FabianKramm If I understood this correctly then this feature enhancement which is requested is not a desirable one and hence the issue can be closed? |
What issue type does this pull request address? (keep at least one, remove the others)
/kind enhancement
What does this pull request do? Which issues does it resolve? (use
resolves #<issue_number>
if possible)resolves #1622
Please provide a short message that should be published in the vcluster release notes
Added a feature enhancement to support filtering out services using label selectors, in particular MatchExpressions (for eg. label/export-service NotIn ["false"]
What else do we need to know?