Merge pull request #118816 from PiotrProkop/topo-opts-to-beta
topologymanager: Promote support for improved multi-numa alignment in Topology Manager to beta
This commit is contained in:
		@@ -742,18 +742,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,
 | 
			
		||||
 
 | 
			
		||||
@@ -1083,9 +1083,9 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 | 
			
		||||
 | 
			
		||||
	TopologyManagerPolicyAlphaOptions: {Default: false, PreRelease: featuregate.Alpha},
 | 
			
		||||
 | 
			
		||||
	TopologyManagerPolicyBetaOptions: {Default: false, PreRelease: featuregate.Beta},
 | 
			
		||||
	TopologyManagerPolicyBetaOptions: {Default: true, PreRelease: featuregate.Beta},
 | 
			
		||||
 | 
			
		||||
	TopologyManagerPolicyOptions: {Default: false, PreRelease: featuregate.Alpha},
 | 
			
		||||
	TopologyManagerPolicyOptions: {Default: true, PreRelease: featuregate.Beta},
 | 
			
		||||
 | 
			
		||||
	VolumeCapacityPriority: {Default: false, PreRelease: featuregate.Alpha},
 | 
			
		||||
 | 
			
		||||
 
 | 
			
		||||
@@ -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 {
 | 
			
		||||
 
 | 
			
		||||
@@ -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 {
 | 
			
		||||
 
 | 
			
		||||
@@ -30,10 +30,10 @@ const (
 | 
			
		||||
)
 | 
			
		||||
 | 
			
		||||
var (
 | 
			
		||||
	alphaOptions = sets.NewString(
 | 
			
		||||
	alphaOptions = sets.NewString()
 | 
			
		||||
	betaOptions  = sets.NewString(
 | 
			
		||||
		PreferClosestNUMANodes,
 | 
			
		||||
	)
 | 
			
		||||
	betaOptions   = sets.NewString()
 | 
			
		||||
	stableOptions = sets.NewString()
 | 
			
		||||
)
 | 
			
		||||
 | 
			
		||||
 
 | 
			
		||||
@@ -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 {
 | 
			
		||||
 
 | 
			
		||||
@@ -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)
 | 
			
		||||
 
 | 
			
		||||
@@ -42,6 +42,7 @@ func TestNewManager(t *testing.T) {
 | 
			
		||||
		expectedError  error
 | 
			
		||||
		topologyError  error
 | 
			
		||||
		policyOptions  map[string]string
 | 
			
		||||
		topology       []cadvisorapi.Node
 | 
			
		||||
	}{
 | 
			
		||||
		{
 | 
			
		||||
			description:    "Policy is set to none",
 | 
			
		||||
@@ -86,10 +87,59 @@ func TestNewManager(t *testing.T) {
 | 
			
		||||
				"unknown-option": "true",
 | 
			
		||||
			},
 | 
			
		||||
		},
 | 
			
		||||
		{
 | 
			
		||||
			description:    "can't get NUMA distances",
 | 
			
		||||
			policyName:     "best-effort",
 | 
			
		||||
			expectedPolicy: "best-effort",
 | 
			
		||||
			policyOptions: map[string]string{
 | 
			
		||||
				PreferClosestNUMANodes: "true",
 | 
			
		||||
			},
 | 
			
		||||
			expectedError: fmt.Errorf("error getting NUMA distances from cadvisor"),
 | 
			
		||||
			topology: []cadvisorapi.Node{
 | 
			
		||||
				{
 | 
			
		||||
					Id: 0,
 | 
			
		||||
				},
 | 
			
		||||
			},
 | 
			
		||||
		},
 | 
			
		||||
		{
 | 
			
		||||
			description:    "more than 8 NUMA nodes",
 | 
			
		||||
			policyName:     "best-effort",
 | 
			
		||||
			expectedPolicy: "best-effort",
 | 
			
		||||
			expectedError:  fmt.Errorf("unsupported on machines with more than %v NUMA Nodes", maxAllowableNUMANodes),
 | 
			
		||||
			topology: []cadvisorapi.Node{
 | 
			
		||||
				{
 | 
			
		||||
					Id: 0,
 | 
			
		||||
				},
 | 
			
		||||
				{
 | 
			
		||||
					Id: 1,
 | 
			
		||||
				},
 | 
			
		||||
				{
 | 
			
		||||
					Id: 2,
 | 
			
		||||
				},
 | 
			
		||||
				{
 | 
			
		||||
					Id: 3,
 | 
			
		||||
				},
 | 
			
		||||
				{
 | 
			
		||||
					Id: 4,
 | 
			
		||||
				},
 | 
			
		||||
				{
 | 
			
		||||
					Id: 5,
 | 
			
		||||
				},
 | 
			
		||||
				{
 | 
			
		||||
					Id: 6,
 | 
			
		||||
				},
 | 
			
		||||
				{
 | 
			
		||||
					Id: 7,
 | 
			
		||||
				},
 | 
			
		||||
				{
 | 
			
		||||
					Id: 8,
 | 
			
		||||
				},
 | 
			
		||||
			},
 | 
			
		||||
		},
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	for _, tc := range tcases {
 | 
			
		||||
		topology := []cadvisorapi.Node{}
 | 
			
		||||
		topology := tc.topology
 | 
			
		||||
 | 
			
		||||
		mngr, err := NewManager(topology, tc.policyName, "container", tc.policyOptions)
 | 
			
		||||
		if tc.expectedError != nil {
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user