From d7a30e1b089d0b7705a454c13d7d767762611e4f Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Thu, 4 Feb 2021 22:25:13 +0100 Subject: [PATCH] podresources: getallocatable: add feature gate Add feature gate to disable the GetAllocatableResources API. The feature gate isd alpha stage, disabled by default. Add e2e test to demonstrate the behaviour with feature gate disabled. Signed-off-by: Francesco Romani --- pkg/features/kube_features.go | 7 + pkg/kubelet/apis/podresources/server_v1.go | 7 + .../apis/podresources/server_v1_test.go | 5 + test/e2e_node/podresources_test.go | 136 +++++++++++++++++- 4 files changed, 150 insertions(+), 5 deletions(-) diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 5b2ce6fa433..42243edf035 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -724,6 +724,12 @@ const ( // Allows jobs to be created in the suspended state. SuspendJob featuregate.Feature = "SuspendJob" + // owner: @fromanirh + // alpha: v1.21 + // + // Enable POD resources API to return allocatable resources + KubeletPodResourcesGetAllocatable featuregate.Feature = "KubeletPodResourcesGetAllocatable" + // owner: @jayunit100 @abhiraut @rikatz // beta: v1.21 // @@ -838,6 +844,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS IngressClassNamespacedParams: {Default: false, PreRelease: featuregate.Alpha}, ServiceInternalTrafficPolicy: {Default: false, PreRelease: featuregate.Alpha}, SuspendJob: {Default: false, PreRelease: featuregate.Alpha}, + KubeletPodResourcesGetAllocatable: {Default: false, PreRelease: featuregate.Alpha}, NamespaceDefaultLabelName: {Default: true, PreRelease: featuregate.Beta}, // graduate to GA and lock to default in 1.22, remove in 1.24 // inherited features from generic apiserver, relisted here to get a conflict if it is changed diff --git a/pkg/kubelet/apis/podresources/server_v1.go b/pkg/kubelet/apis/podresources/server_v1.go index e756a8a29f9..b5d64f28146 100644 --- a/pkg/kubelet/apis/podresources/server_v1.go +++ b/pkg/kubelet/apis/podresources/server_v1.go @@ -18,7 +18,10 @@ package podresources import ( "context" + "fmt" + utilfeature "k8s.io/apiserver/pkg/util/feature" + kubefeatures "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/kubelet/metrics" "k8s.io/kubelet/pkg/apis/podresources/v1" @@ -73,6 +76,10 @@ func (p *v1PodResourcesServer) List(ctx context.Context, req *v1.ListPodResource // GetAllocatableResources returns information about all the resources known by the server - this more like the capacity, not like the current amount of free resources. func (p *v1PodResourcesServer) GetAllocatableResources(ctx context.Context, req *v1.AllocatableResourcesRequest) (*v1.AllocatableResourcesResponse, error) { + if !utilfeature.DefaultFeatureGate.Enabled(kubefeatures.KubeletPodResourcesGetAllocatable) { + return nil, fmt.Errorf("Pod Resources API GetAllocatableResources disabled") + } + metrics.PodResourcesEndpointRequestsTotalCount.WithLabelValues("v1").Inc() return &v1.AllocatableResourcesResponse{ diff --git a/pkg/kubelet/apis/podresources/server_v1_test.go b/pkg/kubelet/apis/podresources/server_v1_test.go index 4cb2f909f68..65b9bd8d60f 100644 --- a/pkg/kubelet/apis/podresources/server_v1_test.go +++ b/pkg/kubelet/apis/podresources/server_v1_test.go @@ -25,7 +25,10 @@ import ( "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" podresourcesapi "k8s.io/kubelet/pkg/apis/podresources/v1" + pkgfeatures "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/kubelet/cm/cpuset" "k8s.io/kubernetes/pkg/kubelet/cm/devicemanager" ) @@ -154,6 +157,8 @@ func TestListPodResourcesV1(t *testing.T) { } func TestAllocatableResources(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, pkgfeatures.KubeletPodResourcesGetAllocatable, true)() + allDevs := []*podresourcesapi.ContainerDevices{ { ResourceName: "resource", diff --git a/test/e2e_node/podresources_test.go b/test/e2e_node/podresources_test.go index 0cc8cc195fb..b4190d75e84 100644 --- a/test/e2e_node/podresources_test.go +++ b/test/e2e_node/podresources_test.go @@ -27,12 +27,17 @@ import ( v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + utilfeature "k8s.io/apiserver/pkg/util/feature" kubeletpodresourcesv1 "k8s.io/kubelet/pkg/apis/podresources/v1" + kubefeatures "k8s.io/kubernetes/pkg/features" + kubeletconfig "k8s.io/kubernetes/pkg/kubelet/apis/config" "k8s.io/kubernetes/pkg/kubelet/apis/podresources" + "k8s.io/kubernetes/pkg/kubelet/cm/cpumanager" "k8s.io/kubernetes/pkg/kubelet/cm/cpuset" "k8s.io/kubernetes/pkg/kubelet/util" "k8s.io/kubernetes/test/e2e/framework" + e2enode "k8s.io/kubernetes/test/e2e/framework/node" e2eskipper "k8s.io/kubernetes/test/e2e/framework/skipper" "github.com/onsi/ginkgo" @@ -485,7 +490,7 @@ func podresourcesGetAllocatableResourcesTests(f *framework.Framework, cli kubele } // Serial because the test updates kubelet configuration. -var _ = SIGDescribe("POD Resources [Serial] [Feature:PODResources][NodeFeature:PODResources]", func() { +var _ = SIGDescribe("POD Resources [Serial] [Feature:PodResources][NodeFeature:PodResources]", func() { f := framework.NewDefaultFramework("podresources-test") reservedSystemCPUs := cpuset.MustParse("1") @@ -505,8 +510,8 @@ var _ = SIGDescribe("POD Resources [Serial] [Feature:PODResources][NodeFeature:P onlineCPUs, err := getOnlineCPUs() framework.ExpectNoError(err) - // Enable CPU Manager in the kubelet. - oldCfg := configureCPUManagerInKubelet(f, true, reservedSystemCPUs) + // Make sure all the feature gates and the right settings are in place. + oldCfg := configurePodResourcesInKubelet(f, true, reservedSystemCPUs) defer func() { // restore kubelet config setOldKubeletConfig(f, oldCfg) @@ -528,6 +533,8 @@ var _ = SIGDescribe("POD Resources [Serial] [Feature:PODResources][NodeFeature:P framework.ExpectNoError(err) defer conn.Close() + waitForSRIOVResources(f, sd) + ginkgo.By("checking List()") podresourcesListTests(f, cli, sd) ginkgo.By("checking GetAllocatableResources()") @@ -542,6 +549,15 @@ var _ = SIGDescribe("POD Resources [Serial] [Feature:PODResources][NodeFeature:P e2eskipper.Skipf("this test is meant to run on a system with at least one configured VF from SRIOV device") } + oldCfg := enablePodResourcesFeatureGateInKubelet(f) + defer func() { + // restore kubelet config + setOldKubeletConfig(f, oldCfg) + + // Delete state file to allow repeated runs + deleteStateFile() + }() + configMap := getSRIOVDevicePluginConfigMap(framework.TestContext.SriovdpConfigMapFile) sd := setupSRIOVConfigOrFail(f, configMap) defer teardownSRIOVConfigOrFail(f, sd) @@ -555,6 +571,8 @@ var _ = SIGDescribe("POD Resources [Serial] [Feature:PODResources][NodeFeature:P framework.ExpectNoError(err) defer conn.Close() + waitForSRIOVResources(f, sd) + // intentionally passing empty cpuset instead of onlineCPUs because with none policy // we should get no allocatable cpus - no exclusively allocatable CPUs, depends on policy static podresourcesGetAllocatableResourcesTests(f, cli, sd, cpuset.CPUSet{}, cpuset.CPUSet{}) @@ -577,8 +595,8 @@ var _ = SIGDescribe("POD Resources [Serial] [Feature:PODResources][NodeFeature:P onlineCPUs, err := getOnlineCPUs() framework.ExpectNoError(err) - // Enable CPU Manager in the kubelet. - oldCfg := configureCPUManagerInKubelet(f, true, reservedSystemCPUs) + // Make sure all the feature gates and the right settings are in place. + oldCfg := configurePodResourcesInKubelet(f, true, reservedSystemCPUs) defer func() { // restore kubelet config setOldKubeletConfig(f, oldCfg) @@ -605,6 +623,15 @@ var _ = SIGDescribe("POD Resources [Serial] [Feature:PODResources][NodeFeature:P e2eskipper.Skipf("this test is meant to run on a system with no configured VF from SRIOV device") } + oldCfg := enablePodResourcesFeatureGateInKubelet(f) + defer func() { + // restore kubelet config + setOldKubeletConfig(f, oldCfg) + + // Delete state file to allow repeated runs + deleteStateFile() + }() + endpoint, err := util.LocalEndpoint(defaultPodResourcesPath, podresources.Socket) framework.ExpectNoError(err) @@ -616,6 +643,24 @@ var _ = SIGDescribe("POD Resources [Serial] [Feature:PODResources][NodeFeature:P // we should get no allocatable cpus - no exclusively allocatable CPUs, depends on policy static podresourcesGetAllocatableResourcesTests(f, cli, nil, cpuset.CPUSet{}, cpuset.CPUSet{}) }) + + ginkgo.It("should return the expected error with the feature gate disabled", func() { + if utilfeature.DefaultFeatureGate.Enabled(kubefeatures.KubeletPodResourcesGetAllocatable) { + e2eskipper.Skipf("this test is meant to run with the POD Resources Extensions feature gate disabled") + } + + endpoint, err := util.LocalEndpoint(defaultPodResourcesPath, podresources.Socket) + framework.ExpectNoError(err) + + cli, conn, err := podresources.GetV1Client(endpoint, defaultPodResourcesTimeout, defaultPodResourcesMaxSize) + framework.ExpectNoError(err) + defer conn.Close() + + ginkgo.By("checking GetAllocatableResources fail if the feature gate is not enabled") + _, err = cli.GetAllocatableResources(context.TODO(), &kubeletpodresourcesv1.AllocatableResourcesRequest{}) + framework.ExpectError(err, "With feature gate disabled, the call must fail") + }) + }) }) @@ -626,3 +671,84 @@ func getOnlineCPUs() (cpuset.CPUSet, error) { } return cpuset.Parse(strings.TrimSpace(string(onlineCPUList))) } + +func configurePodResourcesInKubelet(f *framework.Framework, cleanStateFile bool, reservedSystemCPUs cpuset.CPUSet) (oldCfg *kubeletconfig.KubeletConfiguration) { + // we also need CPUManager with static policy to be able to do meaningful testing + oldCfg, err := getCurrentKubeletConfig() + framework.ExpectNoError(err) + newCfg := oldCfg.DeepCopy() + if newCfg.FeatureGates == nil { + newCfg.FeatureGates = make(map[string]bool) + } + newCfg.FeatureGates["CPUManager"] = true + newCfg.FeatureGates["KubeletPodResourcesGetAllocatable"] = true + + // 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 + // "static" in this test and the Kubelet is restarted so that "static" + // policy can take effect, there will always be a conflict with the state + // checkpointed in the disk (i.e., the policy checkpointed in the disk will + // be "none" whereas we are trying to restart Kubelet with "static" + // policy). Therefore, we delete the state file so that we can proceed + // with the tests. + // Only delete the state file at the begin of the tests. + if cleanStateFile { + deleteStateFile() + } + + // Set the CPU Manager policy to static. + newCfg.CPUManagerPolicy = string(cpumanager.PolicyStatic) + + // Set the CPU Manager reconcile period to 1 second. + newCfg.CPUManagerReconcilePeriod = metav1.Duration{Duration: 1 * time.Second} + + if reservedSystemCPUs.Size() > 0 { + cpus := reservedSystemCPUs.String() + framework.Logf("configurePodResourcesInKubelet: using reservedSystemCPUs=%q", cpus) + newCfg.ReservedSystemCPUs = cpus + } else { + // The Kubelet panics if either kube-reserved or system-reserved is not set + // when CPU Manager is enabled. Set cpu in kube-reserved > 0 so that + // kubelet doesn't panic. + if newCfg.KubeReserved == nil { + newCfg.KubeReserved = map[string]string{} + } + + if _, ok := newCfg.KubeReserved["cpu"]; !ok { + newCfg.KubeReserved["cpu"] = "200m" + } + } + // Update the Kubelet configuration. + framework.ExpectNoError(setKubeletConfiguration(f, newCfg)) + + // Wait for the Kubelet to be ready. + gomega.Eventually(func() bool { + nodes, err := e2enode.TotalReady(f.ClientSet) + framework.ExpectNoError(err) + return nodes == 1 + }, time.Minute, time.Second).Should(gomega.BeTrue()) + + return oldCfg +} + +func enablePodResourcesFeatureGateInKubelet(f *framework.Framework) (oldCfg *kubeletconfig.KubeletConfiguration) { + oldCfg, err := getCurrentKubeletConfig() + framework.ExpectNoError(err) + newCfg := oldCfg.DeepCopy() + if newCfg.FeatureGates == nil { + newCfg.FeatureGates = make(map[string]bool) + } + newCfg.FeatureGates["KubeletPodResourcesGetAllocatable"] = true + + // Update the Kubelet configuration. + framework.ExpectNoError(setKubeletConfiguration(f, newCfg)) + + // Wait for the Kubelet to be ready. + gomega.Eventually(func() bool { + nodes, err := e2enode.TotalReady(f.ClientSet) + framework.ExpectNoError(err) + return nodes == 1 + }, time.Minute, time.Second).Should(gomega.BeTrue()) + + return oldCfg +}