diff --git a/openshift-kube-apiserver/admission/admissionenablement/register.go b/openshift-kube-apiserver/admission/admissionenablement/register.go index eb822bd996542..c0fbd8f7d4ec4 100644 --- a/openshift-kube-apiserver/admission/admissionenablement/register.go +++ b/openshift-kube-apiserver/admission/admissionenablement/register.go @@ -22,6 +22,7 @@ import ( ingressadmission "k8s.io/kubernetes/openshift-kube-apiserver/admission/route" "k8s.io/kubernetes/openshift-kube-apiserver/admission/route/hostassignment" projectnodeenv "k8s.io/kubernetes/openshift-kube-apiserver/admission/scheduler/nodeenv" + "k8s.io/kubernetes/openshift-kube-apiserver/admission/scheduler/nodeselectoradjuster" schedulerpodnodeconstraints "k8s.io/kubernetes/openshift-kube-apiserver/admission/scheduler/podnodeconstraints" "k8s.io/kubernetes/openshift-kube-apiserver/admission/storage/csiinlinevolumesecurity" "k8s.io/kubernetes/openshift-kube-apiserver/admission/storage/performantsecuritypolicy" @@ -46,6 +47,9 @@ func RegisterOpenshiftKubeAdmissionPlugins(plugins *admission.Plugins) { restrictedendpoints.RegisterRestrictedEndpoints(plugins) csiinlinevolumesecurity.Register(plugins) performantsecuritypolicy.Register(plugins) + if !nodeselectoradjuster.IsStandalone() { + nodeselectoradjuster.Register(plugins) + } } var ( @@ -62,25 +66,31 @@ var ( ) // openshiftAdmissionPluginsForKubeBeforeMutating are the admission plugins to add after kube admission, before mutating webhooks - openshiftAdmissionPluginsForKubeBeforeMutating = []string{ - "autoscaling.openshift.io/ClusterResourceOverride", - managementcpusoverride.PluginName, // "autoscaling.openshift.io/ManagementCPUsOverride" - "authorization.openshift.io/RestrictSubjectBindings", - "autoscaling.openshift.io/RunOnceDuration", - "scheduling.openshift.io/PodNodeConstraints", - "scheduling.openshift.io/OriginPodNodeEnvironment", - "network.openshift.io/ExternalIPRanger", - "network.openshift.io/RestrictedEndpointsAdmission", - imagepolicyapiv1.PluginName, // "image.openshift.io/ImagePolicy" - "security.openshift.io/SecurityContextConstraint", - "security.openshift.io/SCCExecRestrictions", - "route.openshift.io/IngressAdmission", - hostassignment.PluginName, // "route.openshift.io/RouteHostAssignment" - csiinlinevolumesecurity.PluginName, // "storage.openshift.io/CSIInlineVolumeSecurity" - managednode.PluginName, // "autoscaling.openshift.io/ManagedNode" - mixedcpus.PluginName, // "autoscaling.openshift.io/MixedCPUs" - performantsecuritypolicy.PluginName, // "storage.openshift.io/PerformantSecurityPolicy" - } + openshiftAdmissionPluginsForKubeBeforeMutating = func() []string { + plugins := []string{ + "autoscaling.openshift.io/ClusterResourceOverride", + managementcpusoverride.PluginName, // "autoscaling.openshift.io/ManagementCPUsOverride" + "authorization.openshift.io/RestrictSubjectBindings", + "autoscaling.openshift.io/RunOnceDuration", + "scheduling.openshift.io/PodNodeConstraints", + "scheduling.openshift.io/OriginPodNodeEnvironment", + "network.openshift.io/ExternalIPRanger", + "network.openshift.io/RestrictedEndpointsAdmission", + imagepolicyapiv1.PluginName, // "image.openshift.io/ImagePolicy" + "security.openshift.io/SecurityContextConstraint", + "security.openshift.io/SCCExecRestrictions", + "route.openshift.io/IngressAdmission", + hostassignment.PluginName, // "route.openshift.io/RouteHostAssignment" + csiinlinevolumesecurity.PluginName, // "storage.openshift.io/CSIInlineVolumeSecurity" + managednode.PluginName, // "autoscaling.openshift.io/ManagedNode" + mixedcpus.PluginName, // "autoscaling.openshift.io/MixedCPUs" + performantsecuritypolicy.PluginName, // "storage.openshift.io/PerformantSecurityPolicy" + } + if !nodeselectoradjuster.IsStandalone() { + plugins = append(plugins, nodeselectoradjuster.PluginName) // "scheduling.openshift.io/NodeSelectorAdjuster" + } + return plugins + }() // openshiftAdmissionPluginsForKubeAfterResourceQuota are the plugins to add after ResourceQuota plugin openshiftAdmissionPluginsForKubeAfterResourceQuota = []string{ diff --git a/openshift-kube-apiserver/admission/scheduler/nodeselectoradjuster/admission.go b/openshift-kube-apiserver/admission/scheduler/nodeselectoradjuster/admission.go new file mode 100644 index 0000000000000..009349b6fbae8 --- /dev/null +++ b/openshift-kube-apiserver/admission/scheduler/nodeselectoradjuster/admission.go @@ -0,0 +1,108 @@ +package nodeselectoradjuster + +// The NodeSelectorAdjuster admission plugin removes the +// node-role.kubernetes.io/master node selector from qualifying pods. It only +// activates on HCP (hosted control plane) clusters, detected by +// POD_NAMESPACE != openshift-kube-apiserver. On standalone OpenShift clusters +// the plugin does not register itself and takes no action, leaving the master +// node selector in place so that qualifying pods run on master nodes as +// intended. + +import ( + "context" + "fmt" + "io" + "os" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apiserver/pkg/admission" + coreapi "k8s.io/kubernetes/pkg/apis/core" +) + +const ( + // PluginName is the name used to identify this plugin in the admission chain. + PluginName = "scheduling.openshift.io/NodeSelectorAdjuster" + + // masterRoleKey is the node role label used as a node selector key in + // 4.x VPA operator manifests. + masterRoleKey = "node-role.kubernetes.io/master" + + // vpaOperatorLabelKey / vpaOperatorLabelValue identify the VPA operator pod. + vpaOperatorLabelKey = "k8s-app" + vpaOperatorLabelValue = "vertical-pod-autoscaler-operator" + // vpaOperatorNamespace is the namespace the VPA operator is expected to run in. + vpaOperatorNamespace = "openshift-vertical-pod-autoscaler" + + // standaloneEnvVar is the environment variable checked at start-up. + // It is injected by the downward API and reflects the namespace the + // kube-apiserver pod runs in. + standaloneEnvVar = "POD_NAMESPACE" + // standaloneEnvValue is the namespace used by the kube-apiserver on a + // standalone OpenShift cluster. + standaloneEnvValue = "openshift-kube-apiserver" +) + +// IsStandalone reports whether the current process is running inside a standalone +// OpenShift cluster. It is checked once at start-up to decide whether the plugin +// should register itself. +func IsStandalone() bool { + return os.Getenv(standaloneEnvVar) == standaloneEnvValue +} + +// Register adds the plugin to the admission plugin registry. It must only be +// called when IsStandalone() returns false (i.e. on HCP clusters). +func Register(plugins *admission.Plugins) { + plugins.Register(PluginName, func(_ io.Reader) (admission.Interface, error) { + return &nodeSelectorAdjuster{ + Handler: admission.NewHandler(admission.Create), + }, nil + }) +} + +// nodeSelectorAdjuster implements admission.MutationInterface. +type nodeSelectorAdjuster struct { + *admission.Handler +} + +var _ admission.MutationInterface = &nodeSelectorAdjuster{} + +// Admit examines newly-created Pod objects and, for qualifying pods, removes the +// master node selector so that they can run on worker nodes in HCP clusters. +func (p *nodeSelectorAdjuster) Admit(_ context.Context, attr admission.Attributes, _ admission.ObjectInterfaces) error { + if attr.GetResource().GroupResource() != corev1.Resource("pods") || attr.GetSubresource() != "" { + return nil + } + + pod, ok := attr.GetObject().(*coreapi.Pod) + if !ok { + return admission.NewForbidden(attr, fmt.Errorf("unexpected object type: %T", attr.GetObject())) + } + + if !requiresNodeSelectorAdjustment(pod) { + return nil + } + + delete(pod.Spec.NodeSelector, masterRoleKey) + return nil +} + +// ValidateInitialization satisfies admission.InitializationValidator. The plugin +// has no external dependencies to validate. +func (p *nodeSelectorAdjuster) ValidateInitialization() error { + return nil +} + +// requiresNodeSelectorAdjustment returns true when the pod carries a label that +// opts it in to node placement and lives in a namespace where that +// label is expected. Control-plane-adjacent Day 2 operators can be added here. +func requiresNodeSelectorAdjustment(pod *coreapi.Pod) bool { + // For VPA, we only want to update if the sole node selector is the + // master role key, which is the default from the 4.x VPA operator manifests. + if pod.Labels[vpaOperatorLabelKey] == vpaOperatorLabelValue && + pod.Namespace == vpaOperatorNamespace && len(pod.Spec.NodeSelector) == 1 { + if _, found := pod.Spec.NodeSelector[masterRoleKey]; found { + return true + } + } + return false +} diff --git a/openshift-kube-apiserver/admission/scheduler/nodeselectoradjuster/admission_test.go b/openshift-kube-apiserver/admission/scheduler/nodeselectoradjuster/admission_test.go new file mode 100644 index 0000000000000..9c78248b1701a --- /dev/null +++ b/openshift-kube-apiserver/admission/scheduler/nodeselectoradjuster/admission_test.go @@ -0,0 +1,280 @@ +package nodeselectoradjuster + +import ( + "context" + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apiserver/pkg/admission" + "k8s.io/apiserver/pkg/authentication/user" + coreapi "k8s.io/kubernetes/pkg/apis/core" +) + +func TestAdmit(t *testing.T) { + tests := []struct { + name string + pod *coreapi.Pod + resource schema.GroupVersionResource + subresource string + expectedNodeSelector map[string]string + }{ + { + name: "VPA operator pod with master node selector: selector is removed", + pod: makePod( + withNamespace(vpaOperatorNamespace), + withLabels(map[string]string{vpaOperatorLabelKey: vpaOperatorLabelValue}), + withNodeSelector(map[string]string{masterRoleKey: ""}), + ), + resource: coreapi.Resource("pods").WithVersion("v1"), + expectedNodeSelector: map[string]string{}, + }, + { + name: "VPA operator pod with no node selector: not modified", + pod: makePod( + withNamespace(vpaOperatorNamespace), + withLabels(map[string]string{vpaOperatorLabelKey: vpaOperatorLabelValue}), + ), + resource: coreapi.Resource("pods").WithVersion("v1"), + expectedNodeSelector: nil, + }, + { + name: "VPA operator pod with non-default node selector: not modified", + pod: makePod( + withNamespace(vpaOperatorNamespace), + withLabels(map[string]string{vpaOperatorLabelKey: vpaOperatorLabelValue}), + withNodeSelector(map[string]string{"topology.kubernetes.io/zone": "us-east-1a"}), + ), + resource: coreapi.Resource("pods").WithVersion("v1"), + expectedNodeSelector: map[string]string{"topology.kubernetes.io/zone": "us-east-1a"}, + }, + { + name: "VPA operator pod with extra node selectors including master: not modified", + pod: makePod( + withNamespace(vpaOperatorNamespace), + withLabels(map[string]string{vpaOperatorLabelKey: vpaOperatorLabelValue}), + withNodeSelector(map[string]string{masterRoleKey: "", "topology.kubernetes.io/zone": "us-east-1a"}), + ), + resource: coreapi.Resource("pods").WithVersion("v1"), + expectedNodeSelector: map[string]string{ + masterRoleKey: "", + "topology.kubernetes.io/zone": "us-east-1a", + }, + }, + { + name: "non-qualifying pod: not modified", + pod: makePod( + withLabels(map[string]string{"app": "some-other-app"}), + ), + resource: coreapi.Resource("pods").WithVersion("v1"), + expectedNodeSelector: nil, + }, + { + name: "pod with no labels: not modified", + pod: makePod(), + resource: coreapi.Resource("pods").WithVersion("v1"), + expectedNodeSelector: nil, + }, + { + name: "VPA operator label with wrong value: not modified", + pod: makePod( + withNamespace(vpaOperatorNamespace), + withLabels(map[string]string{vpaOperatorLabelKey: "some-other-operator"}), + ), + resource: coreapi.Resource("pods").WithVersion("v1"), + expectedNodeSelector: nil, + }, + { + name: "VPA operator label in wrong namespace: not modified", + pod: makePod( + withNamespace("other-namespace"), + withLabels(map[string]string{vpaOperatorLabelKey: vpaOperatorLabelValue}), + ), + resource: coreapi.Resource("pods").WithVersion("v1"), + expectedNodeSelector: nil, + }, + { + name: "non-pod resource: request is ignored", + pod: makePod( + withLabels(map[string]string{vpaOperatorLabelKey: vpaOperatorLabelValue}), + ), + resource: coreapi.Resource("nodes").WithVersion("v1"), + expectedNodeSelector: nil, + }, + { + name: "pod subresource: request is ignored", + pod: makePod( + withLabels(map[string]string{vpaOperatorLabelKey: vpaOperatorLabelValue}), + ), + resource: coreapi.Resource("pods").WithVersion("v1"), + subresource: "exec", + expectedNodeSelector: nil, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + plugin := &nodeSelectorAdjuster{ + Handler: admission.NewHandler(admission.Create), + } + + attrs := admission.NewAttributesRecord( + tc.pod, + nil, + schema.GroupVersionKind{}, + tc.pod.Namespace, + tc.pod.Name, + tc.resource, + tc.subresource, + admission.Create, + nil, + false, + fakeUser(), + ) + + if err := plugin.Admit(context.TODO(), attrs, nil); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Verify node selector. + gotSelector := tc.pod.Spec.NodeSelector + if len(gotSelector) != len(tc.expectedNodeSelector) { + t.Errorf("node selector: expected %v, got %v", tc.expectedNodeSelector, gotSelector) + } else { + for k, v := range tc.expectedNodeSelector { + if gotSelector[k] != v { + t.Errorf("node selector key %q: expected %q, got %q", k, v, gotSelector[k]) + } + } + } + }) + } +} + +func TestRequiresNodeSelectorAdjustment(t *testing.T) { + tests := []struct { + name string + pod *coreapi.Pod + expected bool + }{ + { + name: "VPA operator with master node selector: match", + pod: makePod(withNamespace(vpaOperatorNamespace), withLabels(map[string]string{vpaOperatorLabelKey: vpaOperatorLabelValue}), withNodeSelector(map[string]string{masterRoleKey: ""})), + expected: true, + }, + { + name: "VPA operator with no node selector: no match", + pod: makePod(withNamespace(vpaOperatorNamespace), withLabels(map[string]string{vpaOperatorLabelKey: vpaOperatorLabelValue})), + expected: false, + }, + { + name: "VPA operator with non-default node selector: no match", + pod: makePod(withNamespace(vpaOperatorNamespace), withLabels(map[string]string{vpaOperatorLabelKey: vpaOperatorLabelValue}), withNodeSelector(map[string]string{"topology.kubernetes.io/zone": "us-east-1a"})), + expected: false, + }, + { + name: "VPA operator with extra node selectors: no match", + pod: makePod(withNamespace(vpaOperatorNamespace), withLabels(map[string]string{vpaOperatorLabelKey: vpaOperatorLabelValue}), withNodeSelector(map[string]string{masterRoleKey: "", "extra": "value"})), + expected: false, + }, + { + name: "VPA operator in wrong namespace: no match", + pod: makePod(withNamespace("other-namespace"), withLabels(map[string]string{vpaOperatorLabelKey: vpaOperatorLabelValue}), withNodeSelector(map[string]string{masterRoleKey: ""})), + expected: false, + }, + { + name: "no labels: no match", + pod: makePod(), + expected: false, + }, + { + name: "unrelated labels: no match", + pod: makePod(withNamespace(vpaOperatorNamespace), withLabels(map[string]string{"app": "foo", "version": "v1"})), + expected: false, + }, + { + name: "VPA operator label with wrong value: no match", + pod: makePod(withNamespace(vpaOperatorNamespace), withLabels(map[string]string{vpaOperatorLabelKey: "some-other-operator"})), + expected: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := requiresNodeSelectorAdjustment(tc.pod) + if got != tc.expected { + t.Errorf("expected %v, got %v", tc.expected, got) + } + }) + } +} + +func TestIsStandalone(t *testing.T) { + tests := []struct { + name string + envValue string + expected bool + }{ + { + name: "env var set to 'openshift-kube-apiserver': IsStandalone returns true", + envValue: "openshift-kube-apiserver", + expected: true, + }, + { + name: "env var set to empty string: IsStandalone returns false", + envValue: "", + expected: false, + }, + { + name: "env var set to another namespace: IsStandalone returns false", + envValue: "clusters-my-cluster", + expected: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Setenv(standaloneEnvVar, tc.envValue) + got := IsStandalone() + if got != tc.expected { + t.Errorf("expected %v, got %v", tc.expected, got) + } + }) + } +} + +// makePod constructs a coreapi.Pod, applying each option in order. +func makePod(opts ...func(*coreapi.Pod)) *coreapi.Pod { + p := &coreapi.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-pod", + Namespace: "test-namespace", + }, + } + for _, opt := range opts { + opt(p) + } + return p +} + +func withNamespace(ns string) func(*coreapi.Pod) { + return func(p *coreapi.Pod) { + p.Namespace = ns + } +} + +func withLabels(labels map[string]string) func(*coreapi.Pod) { + return func(p *coreapi.Pod) { + p.Labels = labels + } +} + +func withNodeSelector(selector map[string]string) func(*coreapi.Pod) { + return func(p *coreapi.Pod) { + p.Spec.NodeSelector = selector + } +} + +func fakeUser() user.Info { + return &user.DefaultInfo{Name: "testuser"} +}