Merge pull request #108052 from klueska/fix-topology-manager

Fix bug in TopologyManager with merging hints when NUM_NUMA > 2

pkg/kubelet/cm/topologymanager/policy.go

@@ -41,12 +41,14 @@ func mergePermutation(numaNodes []int, permutation []TopologyHint) TopologyHint
 	var numaAffinities []bitmask.BitMask
 	for _, hint := range permutation {
 		// Only consider hints that have an actual NUMANodeAffinity set.
-		if hint.NUMANodeAffinity == nil {
+		if hint.NUMANodeAffinity != nil {
-			numaAffinities = append(numaAffinities, defaultAffinity)
-		} else {
 			numaAffinities = append(numaAffinities, hint.NUMANodeAffinity)
+			// Only mark preferred if all affinities are equal.
+			if !hint.NUMANodeAffinity.IsEqual(numaAffinities[0]) {
+				preferred = false
 			}
-
+		}
+		// Only mark preferred if all affinities are preferred.
 		if !hint.Preferred {
 			preferred = false
 		}
@@ -54,8 +56,8 @@ func mergePermutation(numaNodes []int, permutation []TopologyHint) TopologyHint
 
 	// Merge the affinities using a bitwise-and operation.
 	mergedAffinity := bitmask.And(defaultAffinity, numaAffinities...)
-	// Build a mergedHint from the merged affinity mask, indicating if an
+	// Build a mergedHint from the merged affinity mask, setting preferred as
-	// preferred allocation was used to generate the affinity mask or not.
+	// appropriate based on the logic above.
 	return TopologyHint{mergedAffinity, preferred}
 }
 

pkg/kubelet/cm/topologymanager/policy_best_effort_test.go

@@ -50,7 +50,7 @@ func TestPolicyBestEffortCanAdmitPodResult(t *testing.T) {
 }
 
 func TestPolicyBestEffortMerge(t *testing.T) {
-	numaNodes := []int{0, 1}
+	numaNodes := []int{0, 1, 2, 3}
 	policy := NewBestEffortPolicy(numaNodes)
 
 	tcases := commonPolicyMergeTestCases(numaNodes)

pkg/kubelet/cm/topologymanager/policy_restricted_test.go

@@ -68,7 +68,7 @@ func TestPolicyRestrictedCanAdmitPodResult(t *testing.T) {
 }
 
 func TestPolicyRestrictedMerge(t *testing.T) {
-	numaNodes := []int{0, 1}
+	numaNodes := []int{0, 1, 2, 3}
 	policy := NewRestrictedPolicy(numaNodes)
 
 	tcases := commonPolicyMergeTestCases(numaNodes)

pkg/kubelet/cm/topologymanager/policy_single_numa_node_test.go

@@ -156,7 +156,7 @@ func TestPolicySingleNumaNodeFilterHints(t *testing.T) {
 }
 
 func TestPolicySingleNumaNodeMerge(t *testing.T) {
-	numaNodes := []int{0, 1}
+	numaNodes := []int{0, 1, 2, 3}
 	policy := NewSingleNumaNodePolicy(numaNodes)
 
 	tcases := commonPolicyMergeTestCases(numaNodes)

pkg/kubelet/cm/topologymanager/policy_test.go

@@ -315,6 +315,43 @@ func commonPolicyMergeTestCases(numaNodes []int) []policyMergeTestCase {
 
 func (p *bestEffortPolicy) mergeTestCases(numaNodes []int) []policyMergeTestCase {
 	return []policyMergeTestCase{
+		{
+			name: "Two providers, 2 hints each, same mask (some with different bits), same preferred",
+			hp: []HintProvider{
+				&mockHintProvider{
+					map[string][]TopologyHint{
+						"resource1": {
+							{
+								NUMANodeAffinity: NewTestBitMask(0, 1),
+								Preferred:        true,
+							},
+							{
+								NUMANodeAffinity: NewTestBitMask(0, 2),
+								Preferred:        true,
+							},
+						},
+					},
+				},
+				&mockHintProvider{
+					map[string][]TopologyHint{
+						"resource2": {
+							{
+								NUMANodeAffinity: NewTestBitMask(0, 1),
+								Preferred:        true,
+							},
+							{
+								NUMANodeAffinity: NewTestBitMask(0, 2),
+								Preferred:        true,
+							},
+						},
+					},
+				},
+			},
+			expected: TopologyHint{
+				NUMANodeAffinity: NewTestBitMask(0, 1),
+				Preferred:        true,
+			},
+		},
 		{
 			name: "TopologyHint not set",
 			hp:   []HintProvider{},
@@ -513,7 +550,7 @@ func (p *bestEffortPolicy) mergeTestCases(numaNodes []int) []policyMergeTestCase
 			},
 			expected: TopologyHint{
 				NUMANodeAffinity: NewTestBitMask(0),
-				Preferred:        true,
+				Preferred:        false,
 			},
 		},
 		{
@@ -550,7 +587,7 @@ func (p *bestEffortPolicy) mergeTestCases(numaNodes []int) []policyMergeTestCase
 			},
 		},
 		{
-			name: "Two providers, 1 hint each, 1 wider mask, both preferred 1/2",
+			name: "Two providers, 1 hint each, 1 wider mask, both preferred 2/2",
 			hp: []HintProvider{
 				&mockHintProvider{
 					map[string][]TopologyHint{
@@ -575,7 +612,7 @@ func (p *bestEffortPolicy) mergeTestCases(numaNodes []int) []policyMergeTestCase
 			},
 			expected: TopologyHint{
 				NUMANodeAffinity: NewTestBitMask(1),
-				Preferred:        true,
+				Preferred:        false,
 			},
 		},
 	}