From f855a23b45524afd3e16f895a5b646bffc99e6a3 Mon Sep 17 00:00:00 2001 From: PiotrProkop Date: Fri, 30 Jun 2023 10:20:42 +0200 Subject: [PATCH] topologymanager: promote TopologyManagerPolicyOptions feature to beta * Promote TopologyManagerPolicyOptions feature to beta * Promote PreferClosestNUMANodes TopologyManagerPolicyOption to beta Signed-off-by: PiotrProkop --- cmd/kubelet/app/server.go | 24 +++---- pkg/features/kube_features.go | 2 +- pkg/kubelet/cm/container_manager.go | 24 +++---- pkg/kubelet/cm/container_manager_linux.go | 2 +- .../cm/topologymanager/policy_options.go | 4 +- .../cm/topologymanager/policy_options_test.go | 68 +++++++++++++------ .../cm/topologymanager/topology_manager.go | 7 +- 7 files changed, 81 insertions(+), 50 deletions(-) diff --git a/cmd/kubelet/app/server.go b/cmd/kubelet/app/server.go index 742524e325f..6b922abb276 100644 --- a/cmd/kubelet/app/server.go +++ b/cmd/kubelet/app/server.go @@ -743,18 +743,18 @@ func run(ctx context.Context, s *options.KubeletServer, kubeDeps *kubelet.Depend ReservedSystemCPUs: reservedSystemCPUs, HardEvictionThresholds: hardEvictionThresholds, }, - QOSReserved: *experimentalQOSReserved, - CPUManagerPolicy: s.CPUManagerPolicy, - CPUManagerPolicyOptions: cpuManagerPolicyOptions, - CPUManagerReconcilePeriod: s.CPUManagerReconcilePeriod.Duration, - ExperimentalMemoryManagerPolicy: s.MemoryManagerPolicy, - ExperimentalMemoryManagerReservedMemory: s.ReservedMemory, - PodPidsLimit: s.PodPidsLimit, - EnforceCPULimits: s.CPUCFSQuota, - CPUCFSQuotaPeriod: s.CPUCFSQuotaPeriod.Duration, - TopologyManagerPolicy: s.TopologyManagerPolicy, - TopologyManagerScope: s.TopologyManagerScope, - ExperimentalTopologyManagerPolicyOptions: topologyManagerPolicyOptions, + QOSReserved: *experimentalQOSReserved, + CPUManagerPolicy: s.CPUManagerPolicy, + CPUManagerPolicyOptions: cpuManagerPolicyOptions, + CPUManagerReconcilePeriod: s.CPUManagerReconcilePeriod.Duration, + ExperimentalMemoryManagerPolicy: s.MemoryManagerPolicy, + ExperimentalMemoryManagerReservedMemory: s.ReservedMemory, + PodPidsLimit: s.PodPidsLimit, + EnforceCPULimits: s.CPUCFSQuota, + CPUCFSQuotaPeriod: s.CPUCFSQuotaPeriod.Duration, + TopologyManagerPolicy: s.TopologyManagerPolicy, + TopologyManagerScope: s.TopologyManagerScope, + TopologyManagerPolicyOptions: topologyManagerPolicyOptions, }, s.FailSwapOn, kubeDeps.Recorder, diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 648fda0037f..0abd43004f7 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -1064,7 +1064,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS TopologyManagerPolicyBetaOptions: {Default: true, PreRelease: featuregate.Beta}, - TopologyManagerPolicyOptions: {Default: false, PreRelease: featuregate.Alpha}, + TopologyManagerPolicyOptions: {Default: true, PreRelease: featuregate.Beta}, VolumeCapacityPriority: {Default: false, PreRelease: featuregate.Alpha}, diff --git a/pkg/kubelet/cm/container_manager.go b/pkg/kubelet/cm/container_manager.go index 2c03bb59ee9..527b66bf50e 100644 --- a/pkg/kubelet/cm/container_manager.go +++ b/pkg/kubelet/cm/container_manager.go @@ -146,18 +146,18 @@ type NodeConfig struct { KubeletRootDir string ProtectKernelDefaults bool NodeAllocatableConfig - QOSReserved map[v1.ResourceName]int64 - CPUManagerPolicy string - CPUManagerPolicyOptions map[string]string - TopologyManagerScope string - CPUManagerReconcilePeriod time.Duration - ExperimentalMemoryManagerPolicy string - ExperimentalMemoryManagerReservedMemory []kubeletconfig.MemoryReservation - PodPidsLimit int64 - EnforceCPULimits bool - CPUCFSQuotaPeriod time.Duration - TopologyManagerPolicy string - ExperimentalTopologyManagerPolicyOptions map[string]string + QOSReserved map[v1.ResourceName]int64 + CPUManagerPolicy string + CPUManagerPolicyOptions map[string]string + TopologyManagerScope string + CPUManagerReconcilePeriod time.Duration + ExperimentalMemoryManagerPolicy string + ExperimentalMemoryManagerReservedMemory []kubeletconfig.MemoryReservation + PodPidsLimit int64 + EnforceCPULimits bool + CPUCFSQuotaPeriod time.Duration + TopologyManagerPolicy string + TopologyManagerPolicyOptions map[string]string } type NodeAllocatableConfig struct { diff --git a/pkg/kubelet/cm/container_manager_linux.go b/pkg/kubelet/cm/container_manager_linux.go index b2868d9d368..d2188144202 100644 --- a/pkg/kubelet/cm/container_manager_linux.go +++ b/pkg/kubelet/cm/container_manager_linux.go @@ -292,7 +292,7 @@ func NewContainerManager(mountUtil mount.Interface, cadvisorInterface cadvisor.I machineInfo.Topology, nodeConfig.TopologyManagerPolicy, nodeConfig.TopologyManagerScope, - nodeConfig.ExperimentalTopologyManagerPolicyOptions, + nodeConfig.TopologyManagerPolicyOptions, ) if err != nil { diff --git a/pkg/kubelet/cm/topologymanager/policy_options.go b/pkg/kubelet/cm/topologymanager/policy_options.go index 39fff52b789..15f94c696d2 100644 --- a/pkg/kubelet/cm/topologymanager/policy_options.go +++ b/pkg/kubelet/cm/topologymanager/policy_options.go @@ -30,10 +30,10 @@ const ( ) var ( - alphaOptions = sets.NewString( + alphaOptions = sets.NewString() + betaOptions = sets.NewString( PreferClosestNUMANodes, ) - betaOptions = sets.NewString() stableOptions = sets.NewString() ) diff --git a/pkg/kubelet/cm/topologymanager/policy_options_test.go b/pkg/kubelet/cm/topologymanager/policy_options_test.go index cf4a01caf9d..d01b63db799 100644 --- a/pkg/kubelet/cm/topologymanager/policy_options_test.go +++ b/pkg/kubelet/cm/topologymanager/policy_options_test.go @@ -29,6 +29,7 @@ import ( ) var fancyBetaOption = "fancy-new-option" +var fancyAlphaOption = "fancy-alpha-option" type optionAvailTest struct { option string @@ -39,15 +40,17 @@ type optionAvailTest struct { func TestNewTopologyManagerOptions(t *testing.T) { testCases := []struct { - description string - policyOptions map[string]string - featureGate featuregate.Feature - expectedErr error - expectedOptions PolicyOptions + description string + policyOptions map[string]string + featureGate featuregate.Feature + featureGateEnable bool + expectedErr error + expectedOptions PolicyOptions }{ { - description: "return TopologyManagerOptions with PreferClosestNUMA set to true", - featureGate: pkgfeatures.TopologyManagerPolicyAlphaOptions, + description: "return TopologyManagerOptions with PreferClosestNUMA set to true", + featureGate: pkgfeatures.TopologyManagerPolicyBetaOptions, + featureGateEnable: true, expectedOptions: PolicyOptions{ PreferClosestNUMA: true, }, @@ -55,39 +58,66 @@ func TestNewTopologyManagerOptions(t *testing.T) { PreferClosestNUMANodes: "true", }, }, + { + description: "fail to set option when TopologyManagerPolicyBetaOptions feature gate is not set", + featureGate: pkgfeatures.TopologyManagerPolicyBetaOptions, + policyOptions: map[string]string{ + PreferClosestNUMANodes: "true", + }, + expectedErr: fmt.Errorf("Topology Manager Policy Beta-level Options not enabled,"), + }, { description: "return empty TopologyManagerOptions", }, { - description: "fail to parse options", - featureGate: pkgfeatures.TopologyManagerPolicyAlphaOptions, + description: "fail to parse options", + featureGate: pkgfeatures.TopologyManagerPolicyAlphaOptions, + featureGateEnable: true, policyOptions: map[string]string{ PreferClosestNUMANodes: "not a boolean", }, expectedErr: fmt.Errorf("bad value for option"), }, { - description: "test beta options success", - featureGate: pkgfeatures.TopologyManagerPolicyBetaOptions, + description: "test beta options success", + featureGate: pkgfeatures.TopologyManagerPolicyBetaOptions, + featureGateEnable: true, policyOptions: map[string]string{ fancyBetaOption: "true", }, }, { - description: "test beta options success", + description: "test beta options fail", + featureGate: pkgfeatures.TopologyManagerPolicyBetaOptions, policyOptions: map[string]string{ fancyBetaOption: "true", }, expectedErr: fmt.Errorf("Topology Manager Policy Beta-level Options not enabled,"), }, + { + description: "test alpha options success", + featureGate: pkgfeatures.TopologyManagerPolicyAlphaOptions, + featureGateEnable: true, + policyOptions: map[string]string{ + fancyAlphaOption: "true", + }, + }, + { + description: "test alpha options fail", + policyOptions: map[string]string{ + fancyAlphaOption: "true", + }, + expectedErr: fmt.Errorf("Topology Manager Policy Alpha-level Options not enabled,"), + }, } - betaOptions = sets.NewString(fancyBetaOption) + betaOptions.Insert(fancyBetaOption) + alphaOptions = sets.NewString(fancyAlphaOption) for _, tcase := range testCases { t.Run(tcase.description, func(t *testing.T) { if tcase.featureGate != "" { - defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, tcase.featureGate, true)() + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, tcase.featureGate, tcase.featureGateEnable)() } opts, err := NewPolicyOptions(tcase.policyOptions) if tcase.expectedErr != nil { @@ -112,7 +142,7 @@ func TestPolicyDefaultsAvailable(t *testing.T) { }, { option: PreferClosestNUMANodes, - expectedAvailable: false, + expectedAvailable: true, }, } for _, testCase := range testCases { @@ -142,15 +172,15 @@ func TestPolicyOptionsAvailable(t *testing.T) { }, { option: PreferClosestNUMANodes, - featureGate: pkgfeatures.TopologyManagerPolicyAlphaOptions, + featureGate: pkgfeatures.TopologyManagerPolicyBetaOptions, featureGateEnable: true, expectedAvailable: true, }, { option: PreferClosestNUMANodes, - featureGate: pkgfeatures.TopologyManagerPolicyBetaOptions, - featureGateEnable: true, - expectedAvailable: false, + featureGate: pkgfeatures.TopologyManagerPolicyAlphaOptions, + featureGateEnable: false, + expectedAvailable: true, }, } for _, testCase := range testCases { diff --git a/pkg/kubelet/cm/topologymanager/topology_manager.go b/pkg/kubelet/cm/topologymanager/topology_manager.go index 467961657dd..3e7d461b563 100644 --- a/pkg/kubelet/cm/topologymanager/topology_manager.go +++ b/pkg/kubelet/cm/topologymanager/topology_manager.go @@ -21,7 +21,7 @@ import ( "time" cadvisorapi "github.com/google/cadvisor/info/v1" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" "k8s.io/klog/v2" "k8s.io/kubernetes/pkg/kubelet/cm/topologymanager/bitmask" "k8s.io/kubernetes/pkg/kubelet/lifecycle" @@ -133,10 +133,9 @@ var _ Manager = &manager{} // NewManager creates a new TopologyManager based on provided policy and scope func NewManager(topology []cadvisorapi.Node, topologyPolicyName string, topologyScopeName string, topologyPolicyOptions map[string]string) (Manager, error) { - klog.InfoS("Creating topology manager with policy per scope", "topologyPolicyName", topologyPolicyName, "topologyScopeName", topologyScopeName) - // When policy is none, the scope is not relevant, so we can short circuit here. if topologyPolicyName == PolicyNone { + klog.InfoS("Creating topology manager with none policy") return &manager{scope: NewNoneScope()}, nil } @@ -145,6 +144,8 @@ func NewManager(topology []cadvisorapi.Node, topologyPolicyName string, topology return nil, err } + klog.InfoS("Creating topology manager with policy per scope", "topologyPolicyName", topologyPolicyName, "topologyScopeName", topologyScopeName, "topologyPolicyOptions", opts) + numaInfo, err := NewNUMAInfo(topology, opts) if err != nil { return nil, fmt.Errorf("cannot discover NUMA topology: %w", err)