From 20be2624500dd0d27849b8f8711a3a7fd82c4735 Mon Sep 17 00:00:00 2001 From: Erez Freiberger Date: Mon, 4 Oct 2021 23:07:52 +0300 Subject: [PATCH] allow setting scheduler name in pod spec (#12036) * allow setting scheduler name in pod spec * update features configmap documentation * run ./hack/update-codegen.sh --- config/core/configmaps/features.yaml | 8 ++++- pkg/apis/config/features.go | 3 ++ pkg/apis/config/features_test.go | 29 +++++++++++++++++++ pkg/apis/serving/fieldmask.go | 4 ++- pkg/apis/serving/k8s_validation_test.go | 17 +++++++++++ .../route/resources/service_test.go | 1 + pkg/reconciler/route/table_test.go | 1 + 7 files changed, 61 insertions(+), 2 deletions(-) diff --git a/config/core/configmaps/features.yaml b/config/core/configmaps/features.yaml index b13066048f5d..c0eeeba2c93f 100644 --- a/config/core/configmaps/features.yaml +++ b/config/core/configmaps/features.yaml @@ -22,7 +22,7 @@ metadata: app.kubernetes.io/version: devel app.kubernetes.io/part-of: knative-serving annotations: - knative.dev/example-checksum: "5b8c26fe" + knative.dev/example-checksum: "0b69357a" data: _example: |- ################################ @@ -106,6 +106,12 @@ data: # See: https://knative.dev/docs/serving/feature-flags/#kubernetes-priority-class-name kubernetes.podspec-priorityclassname: "disabled" + # Indicates whether Kubernetes SchedulerName support is enabled + # + # WARNING: Cannot safely be disabled once enabled. + # See: https://knative.dev/docs/serving/feature-flags/#kubernetes-scheduler-name + kubernetes.podspec-schedulername: "disabled" + # This feature flag allows end-users to add a subset of capabilities on the Pod's SecurityContext. # # When set to "enabled" or "allowed" it allows capabilities to be added to the container. diff --git a/pkg/apis/config/features.go b/pkg/apis/config/features.go index cfdda32de260..17e75fc07844 100644 --- a/pkg/apis/config/features.go +++ b/pkg/apis/config/features.go @@ -50,6 +50,7 @@ func defaultFeaturesConfig() *Features { PodSpecRuntimeClassName: Disabled, PodSpecSecurityContext: Disabled, PodSpecPriorityClassName: Disabled, + PodSpecSchedulerName: Disabled, ContainerSpecAddCapabilities: Disabled, PodSpecTolerations: Disabled, PodSpecVolumesEmptyDir: Disabled, @@ -72,6 +73,7 @@ func NewFeaturesConfigFromMap(data map[string]string) (*Features, error) { asFlag("kubernetes.podspec-runtimeclassname", &nc.PodSpecRuntimeClassName), asFlag("kubernetes.podspec-securitycontext", &nc.PodSpecSecurityContext), asFlag("kubernetes.podspec-priorityclassname", &nc.PodSpecPriorityClassName), + asFlag("kubernetes.podspec-schedulername", &nc.PodSpecSchedulerName), asFlag("kubernetes.containerspec-addcapabilities", &nc.ContainerSpecAddCapabilities), asFlag("kubernetes.podspec-tolerations", &nc.PodSpecTolerations), asFlag("kubernetes.podspec-volumes-emptydir", &nc.PodSpecVolumesEmptyDir), @@ -98,6 +100,7 @@ type Features struct { PodSpecRuntimeClassName Flag PodSpecSecurityContext Flag PodSpecPriorityClassName Flag + PodSpecSchedulerName Flag ContainerSpecAddCapabilities Flag PodSpecTolerations Flag PodSpecVolumesEmptyDir Flag diff --git a/pkg/apis/config/features_test.go b/pkg/apis/config/features_test.go index 9db3d93b06cb..cc35f5b6848e 100644 --- a/pkg/apis/config/features_test.go +++ b/pkg/apis/config/features_test.go @@ -68,6 +68,7 @@ func TestFeaturesConfiguration(t *testing.T) { PodSpecSecurityContext: Enabled, PodSpecTolerations: Enabled, PodSpecPriorityClassName: Enabled, + PodSpecSchedulerName: Enabled, TagHeaderBasedRouting: Enabled, }), data: map[string]string{ @@ -80,6 +81,7 @@ func TestFeaturesConfiguration(t *testing.T) { "kubernetes.podspec-securitycontext": "Enabled", "kubernetes.podspec-tolerations": "Enabled", "kubernetes.podspec-priorityclassname": "Enabled", + "kubernetes.podspec-schedulername": "Enabled", "tag-header-based-routing": "Enabled", }, }, { @@ -379,6 +381,33 @@ func TestFeaturesConfiguration(t *testing.T) { data: map[string]string{ "kubernetes.podspec-priorityclassname": "Disabled", }, + }, { + name: "kubernetes.podspec-schedulername Allowed", + wantErr: false, + wantFeatures: defaultWith(&Features{ + PodSpecSchedulerName: Allowed, + }), + data: map[string]string{ + "kubernetes.podspec-schedulername": "Allowed", + }, + }, { + name: "kubernetes.podspec-schedulername Enabled", + wantErr: false, + wantFeatures: defaultWith(&Features{ + PodSpecSchedulerName: Enabled, + }), + data: map[string]string{ + "kubernetes.podspec-schedulername": "Enabled", + }, + }, { + name: "kubernetes.podspec-schedulername Disabled", + wantErr: false, + wantFeatures: defaultWith(&Features{ + PodSpecSchedulerName: Disabled, + }), + data: map[string]string{ + "kubernetes.podspec-schedulername": "Disabled", + }, }} for _, tt := range configTests { diff --git a/pkg/apis/serving/fieldmask.go b/pkg/apis/serving/fieldmask.go index 30f349f68cc7..aa6a86853117 100644 --- a/pkg/apis/serving/fieldmask.go +++ b/pkg/apis/serving/fieldmask.go @@ -210,6 +210,9 @@ func PodSpecMask(ctx context.Context, in *corev1.PodSpec) *corev1.PodSpec { if cfg.Features.PodSpecPriorityClassName != config.Disabled { out.PriorityClassName = in.PriorityClassName } + if cfg.Features.PodSpecSchedulerName != config.Disabled { + out.SchedulerName = in.SchedulerName + } // Disallowed fields // This list is unnecessary, but added here for clarity @@ -225,7 +228,6 @@ func PodSpecMask(ctx context.Context, in *corev1.PodSpec) *corev1.PodSpec { out.ShareProcessNamespace = nil out.Hostname = "" out.Subdomain = "" - out.SchedulerName = "" out.Priority = nil out.DNSConfig = nil out.ReadinessGates = nil diff --git a/pkg/apis/serving/k8s_validation_test.go b/pkg/apis/serving/k8s_validation_test.go index 7837469e16d3..836f2cbbea33 100644 --- a/pkg/apis/serving/k8s_validation_test.go +++ b/pkg/apis/serving/k8s_validation_test.go @@ -112,6 +112,13 @@ func withPodSpecPriorityClassNameEnabled() configOption { } } +func withPodSpecSchedulerNameEnabled() configOption { + return func(cfg *config.Config) *config.Config { + cfg.Features.PodSpecSchedulerName = config.Enabled + return cfg + } +} + func TestPodSpecValidation(t *testing.T) { tests := []struct { name string @@ -696,6 +703,16 @@ func TestPodSpecFeatureValidation(t *testing.T) { Paths: []string{"priorityClassName"}, }, cfgOpts: []configOption{withPodSpecPriorityClassNameEnabled()}, + }, { + name: "SchedulerName", + featureSpec: corev1.PodSpec{ + SchedulerName: "foo", + }, + err: &apis.FieldError{ + Message: "must not set the field(s)", + Paths: []string{"schedulerName"}, + }, + cfgOpts: []configOption{withPodSpecSchedulerNameEnabled()}, }} featureTests := []struct { diff --git a/pkg/reconciler/route/resources/service_test.go b/pkg/reconciler/route/resources/service_test.go index 633ccddbd99e..6564f615f833 100644 --- a/pkg/reconciler/route/resources/service_test.go +++ b/pkg/reconciler/route/resources/service_test.go @@ -430,6 +430,7 @@ func testConfig() *config.Config { PodSpecTolerations: apiConfig.Disabled, PodSpecVolumesEmptyDir: apiConfig.Disabled, PodSpecPriorityClassName: apiConfig.Disabled, + PodSpecSchedulerName: apiConfig.Disabled, TagHeaderBasedRouting: apiConfig.Disabled, }, } diff --git a/pkg/reconciler/route/table_test.go b/pkg/reconciler/route/table_test.go index 44b8756dad3a..e7a2d4f16752 100644 --- a/pkg/reconciler/route/table_test.go +++ b/pkg/reconciler/route/table_test.go @@ -3280,6 +3280,7 @@ func reconcilerTestConfig() *config.Config { PodSpecNodeSelector: cfgmap.Disabled, PodSpecTolerations: cfgmap.Disabled, PodSpecPriorityClassName: cfgmap.Disabled, + PodSpecSchedulerName: cfgmap.Disabled, TagHeaderBasedRouting: cfgmap.Disabled, }, }