Refactor filterHints:

- Restructure function
- Remove bug fix for catching {nil true} - To be fixed in later commit
- Restore unit tests to original state for testing filterHints

pkg/kubelet/cm/topologymanager/policy_single_numa_node.go

@@ -71,34 +71,20 @@ func (p *singleNumaNodePolicy) mergePermutation(permutation []TopologyHint) Topo
 	return TopologyHint{mergedAffinity, preferred}
 }
 
-// Return hints that have valid bitmasks with exactly one bit set. Also return bool
+// Return hints that have valid bitmasks with exactly one bit set.
-// which indicates whether allResourceHints only consists of {nil true} hints.
+func (p *singleNumaNodePolicy) filterHints(allResourcesHints [][]TopologyHint) [][]TopologyHint {
-func (p *singleNumaNodePolicy) filterHints(allResourcesHints [][]TopologyHint) ([][]TopologyHint, bool) {
 	var filteredResourcesHints [][]TopologyHint
-	var noAffinityPreferredHints int
+	for _, oneResourceHints := range allResourcesHints {
-	var totalHints int
+		var filtered []TopologyHint
-	if len(allResourcesHints) > 0 {
+		for _, hint := range oneResourceHints {
-		for _, oneResourceHints := range allResourcesHints {
+			if hint.NUMANodeAffinity != nil && hint.NUMANodeAffinity.Count() == 1 && hint.Preferred == true {
-			var filtered []TopologyHint
+				filtered = append(filtered, hint)
-			if len(oneResourceHints) > 0 {
-				for _, hint := range oneResourceHints {
-					totalHints++
-					if hint.NUMANodeAffinity != nil && hint.NUMANodeAffinity.Count() == 1 && hint.Preferred == true {
-						filtered = append(filtered, hint)
-					}
-					if hint.NUMANodeAffinity == nil && hint.Preferred == true {
-						noAffinityPreferredHints++
-					}
-				}
 			}
-			filteredResourcesHints = append(filteredResourcesHints, filtered)
 		}
+
+		filteredResourcesHints = append(filteredResourcesHints, filtered)
 	}
-	// Check if all resource hints only consist of nil-affinity/preferred hint: {nil true}.
+	return filteredResourcesHints
-	if noAffinityPreferredHints == totalHints {
-		return filteredResourcesHints, true
-	}
-	return filteredResourcesHints, false
 }
 
 func (p *singleNumaNodePolicy) mergeProvidersHints(providersHints []map[string][]TopologyHint) TopologyHint {
@@ -139,24 +125,17 @@ func (p *singleNumaNodePolicy) mergeProvidersHints(providersHints []map[string][
 		return TopologyHint{nil, true}
 	}
 
-	var noAffinityPreferredOnly bool
+	allResourcesHints = p.filterHints(allResourcesHints)
-	allResourcesHints, noAffinityPreferredOnly = p.filterHints(allResourcesHints)
+	// If no hints, or there is a resource with empty hints after filtering, then policy
-	// If no hints, then policy cannot be satisfied
+	// cannot be satisfied
 	if len(allResourcesHints) == 0 {
 		klog.Infof("[topologymanager] No hints that align to a single NUMA node.")
 		return TopologyHint{}
 	}
-	// If there is a resource with empty hints after filtering, then policy cannot be satisfied.
-	// In the event that the only hints that exist are {nil true} update default hint preferred
-	// to allow scheduling.
 	for _, hints := range allResourcesHints {
 		if len(hints) == 0 {
 			klog.Infof("[topologymanager] No hints that align to a single NUMA node for resource.")
-			if !noAffinityPreferredOnly {
+			return TopologyHint{}
-				return TopologyHint{}
-			} else if noAffinityPreferredOnly {
-				return TopologyHint{nil, true}
-			}
 		}
 	}
 

pkg/kubelet/cm/topologymanager/policy_single_numa_node_test.go

@@ -59,45 +59,18 @@ func TestPolicySingleNumaNodeFilterHints(t *testing.T) {
 		name              string
 		allResources      [][]TopologyHint
 		expectedResources [][]TopologyHint
-		expectedExists    bool
 	}{
 		{
 			name:              "filter empty resources",
 			allResources:      [][]TopologyHint{},
 			expectedResources: [][]TopologyHint(nil),
-			expectedExists:    false,
 		},
 		{
-			name: "filter hints with nil socket mask, preferred true",
+			name: "filter hints with nil socket mask 1/2",
-			allResources: [][]TopologyHint{
-				{
-					{NUMANodeAffinity: nil, Preferred: true},
-				},
-			},
-			expectedResources: [][]TopologyHint{
-				[]TopologyHint(nil),
-			},
-			expectedExists: true,
-		},
-
-		{
-			name: "filter hints with nil socket mask, preferred false",
 			allResources: [][]TopologyHint{
 				{
 					{NUMANodeAffinity: nil, Preferred: false},
 				},
-			},
-			expectedResources: [][]TopologyHint{
-				[]TopologyHint(nil),
-			},
-			expectedExists: false,
-		},
-		{
-			name: "filter hints with nil socket mask, preferred both true",
-			allResources: [][]TopologyHint{
-				{
-					{NUMANodeAffinity: nil, Preferred: true},
-				},
 				{
 					{NUMANodeAffinity: nil, Preferred: true},
 				},
@@ -106,43 +79,9 @@ func TestPolicySingleNumaNodeFilterHints(t *testing.T) {
 				[]TopologyHint(nil),
 				[]TopologyHint(nil),
 			},
-			expectedExists: true,
 		},
 		{
-			name: "filter hints with nil socket mask, preferred both false",
+			name: "filter hints with nil socket mask 2/2",
-			allResources: [][]TopologyHint{
-				{
-					{NUMANodeAffinity: nil, Preferred: false},
-				},
-				{
-					{NUMANodeAffinity: nil, Preferred: false},
-				},
-			},
-			expectedResources: [][]TopologyHint{
-				[]TopologyHint(nil),
-				[]TopologyHint(nil),
-			},
-			expectedExists: false,
-		},
-
-		{
-			name: "filter hints with nil socket mask, preferred true and false",
-			allResources: [][]TopologyHint{
-				{
-					{NUMANodeAffinity: nil, Preferred: true},
-				},
-				{
-					{NUMANodeAffinity: nil, Preferred: false},
-				},
-			},
-			expectedResources: [][]TopologyHint{
-				[]TopologyHint(nil),
-				[]TopologyHint(nil),
-			},
-			expectedExists: false,
-		},
-		{
-			name: "filter hints with nil socket mask",
 			allResources: [][]TopologyHint{
 				{
 					{NUMANodeAffinity: NewTestBitMask(0), Preferred: true},
@@ -161,7 +100,6 @@ func TestPolicySingleNumaNodeFilterHints(t *testing.T) {
 					{NUMANodeAffinity: NewTestBitMask(1), Preferred: true},
 				},
 			},
-			expectedExists: false,
 		},
 		{
 			name: "filter hints with empty resource socket mask",
@@ -180,7 +118,6 @@ func TestPolicySingleNumaNodeFilterHints(t *testing.T) {
 				},
 				[]TopologyHint(nil),
 			},
-			expectedExists: false,
 		},
 		{
 			name: "filter hints with wide sockemask",
@@ -212,22 +149,17 @@ func TestPolicySingleNumaNodeFilterHints(t *testing.T) {
 				[]TopologyHint(nil),
 				[]TopologyHint(nil),
 			},
-			expectedExists: false,
 		},
 	}
 
 	numaNodes := []int{0, 1, 2, 3}
 	for _, tc := range tcases {
 		policy := NewSingleNumaNodePolicy(numaNodes)
-		actual, exists := policy.(*singleNumaNodePolicy).filterHints(tc.allResources)
+		actual := policy.(*singleNumaNodePolicy).filterHints(tc.allResources)
 		if !reflect.DeepEqual(tc.expectedResources, actual) {
 			t.Errorf("Test Case: %s", tc.name)
 			t.Errorf("Expected result to be %v, got %v", tc.expectedResources, actual)
 		}
-		if !reflect.DeepEqual(tc.expectedResources, actual) {
-			t.Errorf("Test Case: %s", tc.name)
-			t.Errorf("Expected result to be %v, got %v", tc.expectedExists, exists)
-		}
 	}
 }