From 077c0aa1be079d59e97cac50f44b64244590135b Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Tue, 14 Sep 2021 17:40:16 +0200 Subject: [PATCH] node: graduate CPUManagerPolicyOptions to beta We graduate the `CPUManagerPolicyOptions` feature to beta in the 1.23 cycle, and we add new experimental feature gates to guard new options which are planned in the 1.23 and in the following cycles. We introduce additional feature gate called `CPUManagerPolicyAlphaOptions` and `CPUManagerPolicyBetaOptions`. The basic idea is to avoid the cumbersome process of adding a feature gate for each option, and to have feature gates which track the maturity level of _groups_ of options. Besides this change, the graduation process, and the process in general, for adding new policy options is still unchanged. The `full-pcpus-only` option added in the 1.22 cycle is intentionally moved into the beta policy options For more details: - KEP: https://github.com/kubernetes/enhancements/pull/2933 - sig-arch discussion: https://groups.google.com/u/1/g/kubernetes-sig-architecture/c/Nxsc7pfe5rw Signed-off-by: Francesco Romani --- pkg/features/kube_features.go | 34 ++++++- pkg/kubelet/cm/cpumanager/policy_options.go | 34 +++++++ .../cm/cpumanager/policy_options_test.go | 94 +++++++++++++++++++ test/e2e_node/cpu_manager_test.go | 2 + 4 files changed, 162 insertions(+), 2 deletions(-) create mode 100644 pkg/kubelet/cm/cpumanager/policy_options_test.go diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 9aac4506457..1b604fc15ba 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -730,8 +730,9 @@ const ( // owner: @fromanirh // alpha: v1.22 + // beta: v1.23 // - // Allow fine-tuning of cpumanager policies + // Allow the usage of options to fine-tune the cpumanager policies. CPUManagerPolicyOptions featuregate.Feature = "CPUManagerPolicyOptions" // owner: @jiahuif @@ -740,6 +741,33 @@ const ( // // Enables Leader Migration for kube-controller-manager and cloud-controller-manager ControllerManagerLeaderMigration featuregate.Feature = "ControllerManagerLeaderMigration" + + // owner: @fromanirh + // alpha: v1.23 + // beta: see below. + // + // Allow fine-tuning of cpumanager policies, experimental, alpha-quality options + // Per https://groups.google.com/g/kubernetes-sig-architecture/c/Nxsc7pfe5rw/m/vF2djJh0BAAJ + // We want to avoid a proliferation of feature gates. This feature gate: + // - will guard *a group* of cpumanager options whose quality level is alpha. + // - will never graduate to beta or stable. + // See https://groups.google.com/g/kubernetes-sig-architecture/c/Nxsc7pfe5rw/m/vF2djJh0BAAJ + // for details about the removal of this feature gate. + CPUManagerPolicyAlphaOptions featuregate.Feature = "CPUManagerPolicyAlphaOptions" + + // owner: @fromanirh + // beta: v1.23 + // beta: see below. + // + // Allow fine-tuning of cpumanager policies, experimental, beta-quality options + // Per https://groups.google.com/g/kubernetes-sig-architecture/c/Nxsc7pfe5rw/m/vF2djJh0BAAJ + // We want to avoid a proliferation of feature gates. This feature gate: + // - will guard *a group* of cpumanager options whose quality level is beta. + // - is thus *introduced* as beta + // - will never graduate to stable. + // See https://groups.google.com/g/kubernetes-sig-architecture/c/Nxsc7pfe5rw/m/vF2djJh0BAAJ + // for details about the removal of this feature gate. + CPUManagerPolicyBetaOptions featuregate.Feature = "CPUManagerPolicyBetaOptions" ) func init() { @@ -849,8 +877,10 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS DelegateFSGroupToCSIDriver: {Default: false, PreRelease: featuregate.Alpha}, KubeletInUserNamespace: {Default: false, PreRelease: featuregate.Alpha}, MemoryQoS: {Default: false, PreRelease: featuregate.Alpha}, - CPUManagerPolicyOptions: {Default: false, PreRelease: featuregate.Alpha}, + CPUManagerPolicyOptions: {Default: true, PreRelease: featuregate.Beta}, ControllerManagerLeaderMigration: {Default: true, PreRelease: featuregate.Beta}, + CPUManagerPolicyAlphaOptions: {Default: false, PreRelease: featuregate.Alpha}, + CPUManagerPolicyBetaOptions: {Default: true, PreRelease: featuregate.Beta}, // inherited features from generic apiserver, relisted here to get a conflict if it is changed // unintentionally on either side: diff --git a/pkg/kubelet/cm/cpumanager/policy_options.go b/pkg/kubelet/cm/cpumanager/policy_options.go index cf92a2e11c4..84461cf1b94 100644 --- a/pkg/kubelet/cm/cpumanager/policy_options.go +++ b/pkg/kubelet/cm/cpumanager/policy_options.go @@ -19,6 +19,10 @@ package cpumanager import ( "fmt" "strconv" + + "k8s.io/apimachinery/pkg/util/sets" + utilfeature "k8s.io/apiserver/pkg/util/feature" + kubefeatures "k8s.io/kubernetes/pkg/features" ) const ( @@ -26,6 +30,30 @@ const ( FullPCPUsOnlyOption string = "full-pcpus-only" ) +var ( + alphaOptions = sets.NewString() + betaOptions = sets.NewString( + FullPCPUsOnlyOption, + ) + stableOptions = sets.NewString() +) + +func CheckPolicyOptionAvailable(option string) error { + if !alphaOptions.Has(option) && !betaOptions.Has(option) && !stableOptions.Has(option) { + return fmt.Errorf("unknown CPU Manager Policy option: %q", option) + } + + if alphaOptions.Has(option) && !utilfeature.DefaultFeatureGate.Enabled(kubefeatures.CPUManagerPolicyAlphaOptions) { + return fmt.Errorf("CPU Manager Policy Alpha-level Options not enabled, but option %q provided", option) + } + + if betaOptions.Has(option) && !utilfeature.DefaultFeatureGate.Enabled(kubefeatures.CPUManagerPolicyBetaOptions) { + return fmt.Errorf("CPU Manager Policy Beta-level Options not enabled, but option %q provided", option) + } + + return nil +} + type StaticPolicyOptions struct { // flag to enable extra allocation restrictions to avoid // different containers to possibly end up on the same core. @@ -41,6 +69,10 @@ type StaticPolicyOptions struct { func NewStaticPolicyOptions(policyOptions map[string]string) (StaticPolicyOptions, error) { opts := StaticPolicyOptions{} for name, value := range policyOptions { + if err := CheckPolicyOptionAvailable(name); err != nil { + return opts, err + } + switch name { case FullPCPUsOnlyOption: optValue, err := strconv.ParseBool(value) @@ -49,6 +81,8 @@ func NewStaticPolicyOptions(policyOptions map[string]string) (StaticPolicyOption } opts.FullPhysicalCPUsOnly = optValue default: + // this should never be reached, we already detect unknown options, + // but we keep it as further safety. return opts, fmt.Errorf("unsupported cpumanager option: %q (%s)", name, value) } } diff --git a/pkg/kubelet/cm/cpumanager/policy_options_test.go b/pkg/kubelet/cm/cpumanager/policy_options_test.go new file mode 100644 index 00000000000..b50794fb4e6 --- /dev/null +++ b/pkg/kubelet/cm/cpumanager/policy_options_test.go @@ -0,0 +1,94 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cpumanager + +import ( + "testing" + + utilfeature "k8s.io/apiserver/pkg/util/feature" + "k8s.io/component-base/featuregate" + featuregatetesting "k8s.io/component-base/featuregate/testing" + pkgfeatures "k8s.io/kubernetes/pkg/features" +) + +type optionAvailTest struct { + option string + featureGate featuregate.Feature + featureGateEnable bool + expectedAvailable bool +} + +func TestPolicyDefaultsAvailable(t *testing.T) { + testCases := []optionAvailTest{ + { + option: "this-option-does-not-exist", + expectedAvailable: false, + }, + { + option: FullPCPUsOnlyOption, + expectedAvailable: true, + }, + } + for _, testCase := range testCases { + t.Run(testCase.option, func(t *testing.T) { + err := CheckPolicyOptionAvailable(testCase.option) + isEnabled := (err == nil) + if isEnabled != testCase.expectedAvailable { + t.Errorf("option %q available got=%v expected=%v", testCase.option, isEnabled, testCase.expectedAvailable) + } + }) + } +} + +func TestPolicyBetaOptionsAvailable(t *testing.T) { + testCases := []optionAvailTest{ + { + option: "this-option-does-not-exist", + featureGate: pkgfeatures.CPUManagerPolicyBetaOptions, + featureGateEnable: false, + expectedAvailable: false, + }, + { + option: "this-option-does-not-exist", + featureGate: pkgfeatures.CPUManagerPolicyBetaOptions, + featureGateEnable: true, + expectedAvailable: false, + }, + { + option: FullPCPUsOnlyOption, + featureGate: pkgfeatures.CPUManagerPolicyBetaOptions, + featureGateEnable: true, + expectedAvailable: true, + }, + { + option: FullPCPUsOnlyOption, + featureGate: pkgfeatures.CPUManagerPolicyBetaOptions, + featureGateEnable: false, + expectedAvailable: false, + }, + } + for _, testCase := range testCases { + t.Run(testCase.option, func(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, testCase.featureGate, testCase.featureGateEnable)() + err := CheckPolicyOptionAvailable(testCase.option) + isEnabled := (err == nil) + if isEnabled != testCase.expectedAvailable { + t.Errorf("option %q available got=%v expected=%v", testCase.option, isEnabled, testCase.expectedAvailable) + } + }) + } +} diff --git a/test/e2e_node/cpu_manager_test.go b/test/e2e_node/cpu_manager_test.go index 2ca84bcd90a..a23edd8631d 100644 --- a/test/e2e_node/cpu_manager_test.go +++ b/test/e2e_node/cpu_manager_test.go @@ -233,6 +233,8 @@ func configureCPUManagerInKubelet(f *framework.Framework, policyName string, cle } newCfg.FeatureGates["CPUManager"] = true newCfg.FeatureGates["CPUManagerPolicyOptions"] = enableOptions + newCfg.FeatureGates["CPUManagerPolicyBetaOptions"] = enableOptions + newCfg.FeatureGates["CPUManagerPolicyAlphaOptions"] = enableOptions // After graduation of the CPU Manager feature to Beta, the CPU Manager // "none" policy is ON by default. But when we set the CPU Manager policy to