📖 Propose Adding Global Node Selector in Pod Syncer

[m-messiah]

What this PR does / why we need it: NodeSelector is a widely used mechanism to assign pods to specific node groups providing isolation and predictability of schedule. Current Multi-tenant Virtual Cluster implementation does not have any control over it. In this proposal, we suggest having a VirtualClusterNodeSelector object to tell PodSyncer to apply nodeSelector to every pod created in the virtual cluster.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: m-messiah Once this PR has been reviewed and has the lgtm label, please assign davidewatson for approval by writing /assign @davidewatson in a comment. For more information see:The Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/kubernetes-sigs/cluster-api-provider-nested/blob/main/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment

[christopherhein]

I think we can make this something slightly more generic and allow for more flexibility for continuing to iterate on the vc-syncer without needing to make upstream changes.

Have you considered making this a feature that a user can compile into their own version of the vc-syncer without needing to change the pkg/syncer/resources/pod/* packages. One of the commented out lines in the code base was eluding to this as an eventual goal. https://github.com/kubernetes-sigs/cluster-api-provider-nested/blob/main/virtualcluster/pkg/syncer/resources/pod/dws.go#L200-L206

What if instead of making this feature specifically about nodeSelectors we could implement a plugin mechanism that would allow anyone to "add" additional mutators first to pods but maybe later being able to extend this to other synced objects.

Looking at how the syncer registration is configured - https://github.com/kubernetes-sigs/cluster-api-provider-nested/blob/main/virtualcluster/pkg/syncer/resources/pod/controller.go#L51-L58 or how the quota plugin is done https://github.com/kubernetes-sigs/cluster-api-provider-nested/tree/main/virtualcluster/pkg/syncer/resources/pod/validationplugin might be a useful inspiration.

I can see this being really helpful in many ways, for example if someone wanted to set a runtimeClassName on all pods to use a sandboxed runtime but not allow tenants to "see/change" that field or add additional metadata (labels/annotations) with prefixes defined within opaqueMetaPrefixes so they weren't causing "diffs" or even adding a sidecar to handle initializing the network, configuring a service mesh, etc.

I'm not against this as a mechanism for doing NodeSelector support, but curious if we can dot it out-of-tree first then and see if others would be interested in in-tree support later.

What do you think?

[m-messiah]

Have you considered making this a feature that a user can compile into their own version of the vc-syncer without needing to change the pkg/syncer/resources/pod/* packages. One of the commented out lines in the code base was eluding to this as an eventual goal. https://github.com/kubernetes-sigs/cluster-api-provider-nested/blob/main/virtualcluster/pkg/syncer/resources/pod/dws.go#L200-L206

What if instead of making this feature specifically about nodeSelectors we could implement a plugin mechanism that would allow anyone to "add" additional mutators first to pods but maybe later being able to extend this to other synced objects.

It makes sense, I will think about how we could implement the plugin mechanism without rewriting the core methods. I am thinking about something like pluginMutatorInterface, but it needs to be done as an ordered list somehow, rather than just Registered, since it would be used in only one pod syncer, rather than being different resource syncers.

[m-messiah]

Do you think I should rewrite the proposal or could just try to implement the plugin mechanism and show how it would be?

[m-messiah]

The PodMutators were converted to plugins, so nodeSelector could be implemented as a plugin locally. I am rolling back the proposal as it is not needed