nrt: enable consumption of MaxAllowedNUMANodes

kubernetes 1.31 gained support for topology manager options,
and most notably to bump the limit of max NUMA nodes,
which is no longer hardcoded to 8.

Thus, we need to adapt the scheduler code.
Likewise other key topology manager parameters,
we trust NRT objects to carry the values we need.
In this case, to mitigate DoS risks, we set an artificial
"high enough" (and hopefully good enough for a while)
limit.

Once obtained the vlaue, we pass it through to all
the code places on which the previous hardcoded value was used.

Signed-off-by: Francesco Romani <[email protected]>

Author: ffromani

pkg/noderesourcetopology/filter.go

@@ -35,10 +35,6 @@ import (
 	"sigs.k8s.io/scheduler-plugins/pkg/util"
 )
 
-// The maximum number of NUMA nodes that Topology Manager allows is 8
-// https://kubernetes.io/docs/tasks/administer-cluster/topology-manager/#known-limitations
-const highestNUMAID = 8
-
 type PolicyHandler func(pod *v1.Pod, zoneMap topologyv1alpha2.ZoneList) *framework.Status
 
 func singleNUMAContainerLevelHandler(lh logr.Logger, pod *v1.Pod, info *filterInfo) *framework.Status {
@@ -85,7 +81,7 @@ func singleNUMAContainerLevelHandler(lh logr.Logger, pod *v1.Pod, info *filterIn
 // resourcesAvailableInAnyNUMANodes checks for sufficient resource and return the NUMAID that would be selected by Kubelet.
 // this function requires NUMANodeList with properly populated NUMANode, NUMAID should be in range 0-63
 func resourcesAvailableInAnyNUMANodes(lh logr.Logger, info *filterInfo, resources v1.ResourceList) (int, bool, string) {
-	numaID := highestNUMAID
+	numaID := info.topologyManager.MaxNUMANodes // highest NUMA ID
 	bitmask := bm.NewEmptyBitMask()
 	// set all bits, each bit is a NUMA node, if resources couldn't be aligned
 	// on the NUMA node, bit should be unset

pkg/noderesourcetopology/least_numa.go

@@ -67,7 +67,7 @@ func leastNUMAContainerScopeScore(lh logr.Logger, pod *v1.Pod, info *scoreInfo)
 		return framework.MaxNodeScore, nil
 	}
 
-	return normalizeScore(maxNUMANodesCount, allContainersMinAvgDistance), nil
+	return normalizeScore(maxNUMANodesCount, allContainersMinAvgDistance, info.topologyManager.MaxNUMANodes), nil
 }
 
 func leastNUMAPodScopeScore(lh logr.Logger, pod *v1.Pod, info *scoreInfo) (int64, *framework.Status) {
@@ -85,11 +85,11 @@ func leastNUMAPodScopeScore(lh logr.Logger, pod *v1.Pod, info *scoreInfo) (int64
 		return framework.MinNodeScore, nil
 	}
 
-	return normalizeScore(numaNodes.Count(), isMinAvgDistance), nil
+	return normalizeScore(numaNodes.Count(), isMinAvgDistance, info.topologyManager.MaxNUMANodes), nil
 }
 
-func normalizeScore(numaNodesCount int, isMinAvgDistance bool) int64 {
-	numaNodeScore := framework.MaxNodeScore / highestNUMAID
+func normalizeScore(numaNodesCount int, isMinAvgDistance bool, highestNUMAID int) int64 {
+	numaNodeScore := framework.MaxNodeScore / int64(highestNUMAID)
 	score := framework.MaxNodeScore - int64(numaNodesCount)*numaNodeScore
 	if isMinAvgDistance {
 		// if distance between NUMA domains is optimal add half of numaNodeScore to make this node more favorable

pkg/noderesourcetopology/least_numa_test.go

@@ -25,6 +25,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/klog/v2"
 	"k8s.io/kubernetes/pkg/kubelet/cm/topologymanager/bitmask"
+	"sigs.k8s.io/scheduler-plugins/pkg/noderesourcetopology/nodeconfig"
 )
 
 const (
@@ -746,7 +747,7 @@ func TestNormalizeScore(t *testing.T) {
 
 	for _, tc := range tcases {
 		t.Run(tc.description, func(t *testing.T) {
-			normalizedScore := normalizeScore(tc.score, tc.optimalDistance)
+			normalizedScore := normalizeScore(tc.score, tc.optimalDistance, nodeconfig.DefaultMaxNUMANodes)
 			if normalizedScore != tc.expectedScore {
 				t.Errorf("Expected normalizedScore to be %d not %d", tc.expectedScore, normalizedScore)
 			}

pkg/noderesourcetopology/nodeconfig/topologymanager.go

@@ -18,6 +18,7 @@ package nodeconfig
 
 import (
 	"fmt"
+	"strconv"
 
 	kubeletconfig "k8s.io/kubernetes/pkg/kubelet/apis/config"
 
@@ -26,11 +27,16 @@ import (
 )
 
 const (
-	AttributeScope  = "topologyManagerScope"
-	AttributePolicy = "topologyManagerPolicy"
+	DefaultMaxNUMANodes = 8 // legacy setting and default value for TopologyManager. NOTE: kube doesn't expose this constant
+
+	LimitNUMANodes = 1024 // basic sanitization, but we will likely need to bump soon enough
 )
 
-// TODO: handle topologyManagerPolicyOptions added in k8s 1.26
+const (
+	AttributeScope        = "topologyManagerScope"
+	AttributePolicy       = "topologyManagerPolicy"
+	AttributeMaxNUMANodes = "topologyManagerMaxNUMANodes"
+)
 
 func IsValidScope(scope string) bool {
 	if scope == kubeletconfig.ContainerTopologyManagerScope || scope == kubeletconfig.PodTopologyManagerScope {
@@ -47,15 +53,25 @@ func IsValidPolicy(policy string) bool {
 	return false
 }
 
+func IsValidMaxNUMANodes(value int) bool {
+	// machines always report at least 1 NUMA node anyway, and furthermore the value is used in a division,
+	// so we need to enforce 1 (not 0) as minimum
+	// NOTE: there's no legit upper bound, so care must be taken to cap this value. But still, theoretically
+	// 4096 NUMA nodes is a valid legal value.
+	return value > 1
+}
+
 type TopologyManager struct {
-	Scope  string
-	Policy string
+	Scope        string
+	Policy       string
+	MaxNUMANodes int
 }
 
 func TopologyManagerDefaults() TopologyManager {
 	return TopologyManager{
-		Scope:  kubeletconfig.ContainerTopologyManagerScope,
-		Policy: kubeletconfig.NoneTopologyManagerPolicy,
+		Scope:        kubeletconfig.ContainerTopologyManagerScope,
+		Policy:       kubeletconfig.NoneTopologyManagerPolicy,
+		MaxNUMANodes: DefaultMaxNUMANodes,
 	}
 }
 
@@ -65,12 +81,12 @@ func TopologyManagerFromNodeResourceTopology(lh logr.Logger, nodeTopology *topol
 	// Backward compatibility (v1alpha2 and previous). Deprecated, will be removed when the NRT API moves to v1beta1.
 	cfg.updateFromPolicies(lh, nodeTopology.Name, nodeTopology.TopologyPolicies)
 	// preferred new configuration source (v1alpha2 and onwards)
-	cfg.updateFromAttributes(nodeTopology.Attributes)
+	cfg.updateFromAttributes(lh, nodeTopology.Attributes)
 	return conf
 }
 
 func (conf TopologyManager) String() string {
-	return fmt.Sprintf("policy=%q scope=%q", conf.Policy, conf.Scope)
+	return fmt.Sprintf("policy=%q scope=%q maxNUMANodes=%d", conf.Policy, conf.Scope, conf.MaxNUMANodes)
 }
 
 func (conf TopologyManager) Equal(other TopologyManager) bool {
@@ -80,10 +96,10 @@ func (conf TopologyManager) Equal(other TopologyManager) bool {
 	if conf.Policy != other.Policy {
 		return false
 	}
-	return true
+	return conf.MaxNUMANodes == other.MaxNUMANodes
 }
 
-func (conf *TopologyManager) updateFromAttributes(attrs topologyv1alpha2.AttributeList) {
+func (conf *TopologyManager) updateFromAttributes(lh logr.Logger, attrs topologyv1alpha2.AttributeList) {
 	for _, attr := range attrs {
 		if attr.Name == AttributeScope && IsValidScope(attr.Value) {
 			conf.Scope = attr.Value
@@ -93,8 +109,22 @@ func (conf *TopologyManager) updateFromAttributes(attrs topologyv1alpha2.Attribu
 			conf.Policy = attr.Value
 			continue
 		}
-		// TODO: handle topologyManagerPolicyOptions added in k8s 1.26
+		if attr.Name == AttributeMaxNUMANodes {
+			if val, err := strconv.Atoi(attr.Value); err == nil && IsValidMaxNUMANodes(val) {
+				conf.MaxNUMANodes = clampMaxNUMANodes(lh, val)
+				continue
+			}
+		}
+	}
+}
+
+func clampMaxNUMANodes(lh logr.Logger, val int) int {
+	if val > LimitNUMANodes {
+		// should never happen, so we are verbose
+		lh.Info("capped MaxNUMANodes value to limit", "value", val, "limit", LimitNUMANodes)
+		val = LimitNUMANodes
 	}
+	return val
 }
 
 func (conf *TopologyManager) updateFromPolicies(lh logr.Logger, nodeName string, topologyPolicies []string) {

pkg/noderesourcetopology/nodeconfig/topologymanager_test.go

@@ -136,12 +136,14 @@ func TestTopologyManagerEqual(t *testing.T) {
 		{
 			name: "matching",
 			tmA: TopologyManager{
-				Scope:  "container",
-				Policy: "single-numa-node",
+				Scope:        "container",
+				Policy:       "single-numa-node",
+				MaxNUMANodes: 9, // gracehopper
 			},
 			tmB: TopologyManager{
-				Scope:  "container",
-				Policy: "single-numa-node",
+				Scope:        "container",
+				Policy:       "single-numa-node",
+				MaxNUMANodes: 9, // gracehopper
 			},
 			expected: true,
 		},
@@ -181,6 +183,25 @@ func TestTopologyManagerEqual(t *testing.T) {
 			},
 			expected: false,
 		},
+		{
+			name: "nodes diff vs nil",
+			tmA: TopologyManager{
+				MaxNUMANodes: 16,
+			},
+			tmB:      TopologyManager{},
+			expected: false,
+		},
+		{
+			name: "nodes diff",
+			tmA: TopologyManager{
+				MaxNUMANodes: 16,
+			},
+			tmB: TopologyManager{
+				MaxNUMANodes: 9, // gracehopper
+			},
+			expected: false,
+		},
+
 		{
 			name: "scope diff, policy matching",
 			tmA: TopologyManager{
@@ -205,6 +226,19 @@ func TestTopologyManagerEqual(t *testing.T) {
 			},
 			expected: false,
 		},
+		{
+			name: "scope, policy matching, nodes diff",
+			tmA: TopologyManager{
+				Scope:        "container",
+				Policy:       "single-numa-node",
+				MaxNUMANodes: 9,
+			},
+			tmB: TopologyManager{
+				Scope:  "container",
+				Policy: "best-effort",
+			},
+			expected: false,
+		},
 	}
 
 	for _, tt := range tests {
@@ -324,13 +358,67 @@ func TestConfigFromAttributes(t *testing.T) {
 				Policy: kubeletconfig.RestrictedTopologyManagerPolicy,
 			},
 		},
+		{
+			name: "invalid-nodes-string",
+			attrs: topologyv1alpha2.AttributeList{
+				{
+					Name:  "topologyManagerMaxNUMANodes",
+					Value: "A",
+				},
+			},
+			expected: TopologyManager{},
+		},
+		{
+			name: "invalid-nodes-zero",
+			attrs: topologyv1alpha2.AttributeList{
+				{
+					Name:  "topologyManagerMaxNUMANodes",
+					Value: "0",
+				},
+			},
+			expected: TopologyManager{},
+		},
+		{
+			name: "invalid-nodes-negative",
+			attrs: topologyv1alpha2.AttributeList{
+				{
+					Name:  "topologyManagerMaxNUMANodes",
+					Value: "-2",
+				},
+			},
+			expected: TopologyManager{},
+		},
+		{
+			name: "valid-nodes",
+			attrs: topologyv1alpha2.AttributeList{
+				{
+					Name:  "topologyManagerMaxNUMANodes",
+					Value: "16",
+				},
+			},
+			expected: TopologyManager{
+				MaxNUMANodes: 16,
+			},
+		},
+		{
+			name: "valid-nodes-upper-bound",
+			attrs: topologyv1alpha2.AttributeList{
+				{
+					Name:  "topologyManagerMaxNUMANodes",
+					Value: "65535",
+				},
+			},
+			expected: TopologyManager{
+				MaxNUMANodes: LimitNUMANodes,
+			},
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			got := TopologyManager{}
 			cfg := &got // shortcut
-			cfg.updateFromAttributes(tt.attrs)
+			cfg.updateFromAttributes(klog.Background(), tt.attrs)
 			if !reflect.DeepEqual(got, tt.expected) {
 				t.Errorf("conf got=%+#v expected=%+#v", got, tt.expected)
 			}
@@ -427,8 +515,9 @@ func TestConfigFromNRT(t *testing.T) {
 				},
 			},
 			expected: TopologyManager{
-				Policy: kubeletconfig.BestEffortTopologyManagerPolicy,
-				Scope:  kubeletconfig.PodTopologyManagerScope,
+				Policy:       kubeletconfig.BestEffortTopologyManagerPolicy,
+				Scope:        kubeletconfig.PodTopologyManagerScope,
+				MaxNUMANodes: DefaultMaxNUMANodes,
 			},
 		},
 		{
@@ -440,8 +529,9 @@ func TestConfigFromNRT(t *testing.T) {
 				},
 			},
 			expected: TopologyManager{
-				Policy: kubeletconfig.RestrictedTopologyManagerPolicy,
-				Scope:  kubeletconfig.ContainerTopologyManagerScope,
+				Policy:       kubeletconfig.RestrictedTopologyManagerPolicy,
+				Scope:        kubeletconfig.ContainerTopologyManagerScope,
+				MaxNUMANodes: DefaultMaxNUMANodes,
 			},
 		},
 		{
@@ -455,8 +545,9 @@ func TestConfigFromNRT(t *testing.T) {
 				},
 			},
 			expected: TopologyManager{
-				Policy: kubeletconfig.RestrictedTopologyManagerPolicy,
-				Scope:  kubeletconfig.ContainerTopologyManagerScope,
+				Policy:       kubeletconfig.RestrictedTopologyManagerPolicy,
+				Scope:        kubeletconfig.ContainerTopologyManagerScope,
+				MaxNUMANodes: DefaultMaxNUMANodes,
 			},
 		},
 		{
@@ -473,8 +564,9 @@ func TestConfigFromNRT(t *testing.T) {
 				},
 			},
 			expected: TopologyManager{
-				Policy: kubeletconfig.BestEffortTopologyManagerPolicy,
-				Scope:  kubeletconfig.ContainerTopologyManagerScope,
+				Policy:       kubeletconfig.BestEffortTopologyManagerPolicy,
+				Scope:        kubeletconfig.ContainerTopologyManagerScope,
+				MaxNUMANodes: DefaultMaxNUMANodes,
 			},
 		},
 		{
@@ -495,8 +587,9 @@ func TestConfigFromNRT(t *testing.T) {
 				},
 			},
 			expected: TopologyManager{
-				Policy: kubeletconfig.RestrictedTopologyManagerPolicy,
-				Scope:  kubeletconfig.ContainerTopologyManagerScope,
+				Policy:       kubeletconfig.RestrictedTopologyManagerPolicy,
+				Scope:        kubeletconfig.ContainerTopologyManagerScope,
+				MaxNUMANodes: DefaultMaxNUMANodes,
 			},
 		},
 	}