Improved multi-numa alignment in Topology Manager: implement closest numa policy

Signed-off-by: PiotrProkop <pprokop@nvidia.com>

pkg/kubelet/cm/container_manager_linux.go

@@ -288,6 +288,7 @@ func NewContainerManager(mountUtil mount.Interface, cadvisorInterface cadvisor.I
 			machineInfo.Topology,
 			nodeConfig.ExperimentalTopologyManagerPolicy,
 			nodeConfig.ExperimentalTopologyManagerScope,
+			nodeConfig.ExperimentalTopologyManagerPolicyOptions,
 		)
 
 		if err != nil {

pkg/kubelet/cm/cpumanager/policy_options_test.go

@@ -162,7 +162,7 @@ func TestValidateStaticPolicyOptions(t *testing.T) {
 		t.Run(testCase.description, func(t *testing.T) {
 			topoMgrPolicy := topologymanager.NewNonePolicy()
 			if testCase.topoMgrPolicy == topologymanager.PolicySingleNumaNode {
-				topoMgrPolicy = topologymanager.NewSingleNumaNodePolicy(nil)
+				topoMgrPolicy, _ = topologymanager.NewSingleNumaNodePolicy(&topologymanager.NUMAInfo{}, map[string]string{})
 
 			}
 			topoMgrStore := topologymanager.NewFakeManagerWithPolicy(topoMgrPolicy)

pkg/kubelet/cm/topologymanager/policy.go

@@ -33,11 +33,10 @@ type Policy interface {
 // Merge a TopologyHints permutation to a single hint by performing a bitwise-AND
 // of their affinity masks. The hint shall be preferred if all hits in the permutation
 // are preferred.
-func mergePermutation(numaNodes []int, permutation []TopologyHint) TopologyHint {
+func mergePermutation(defaultAffinity bitmask.BitMask, permutation []TopologyHint) TopologyHint {
 	// Get the NUMANodeAffinity from each hint in the permutation and see if any
 	// of them encode unpreferred allocations.
 	preferred := true
-	defaultAffinity, _ := bitmask.NewBitMask(numaNodes...)
 	var numaAffinities []bitmask.BitMask
 	for _, hint := range permutation {
 		// Only consider hints that have an actual NUMANodeAffinity set.
@@ -127,7 +126,50 @@ func maxOfMinAffinityCounts(filteredHints [][]TopologyHint) int {
 	return maxOfMinCount
 }
 
-func compareHints(bestNonPreferredAffinityCount int, current *TopologyHint, candidate *TopologyHint) *TopologyHint {
+type HintMerger struct {
+	NUMAInfo *NUMAInfo
+	Hints    [][]TopologyHint
+	// Set bestNonPreferredAffinityCount to help decide which affinity mask is
+	// preferred amongst all non-preferred hints. We calculate this value as
+	// the maximum of the minimum affinity counts supplied for any given hint
+	// provider. In other words, prefer a hint that has an affinity mask that
+	// includes all of the NUMA nodes from the provider that requires the most
+	// NUMA nodes to satisfy its allocation.
+	BestNonPreferredAffinityCount int
+	CompareNUMAAffinityMasks      func(candidate *TopologyHint, current *TopologyHint) (best *TopologyHint)
+}
+
+func NewHintMerger(numaInfo *NUMAInfo, hints [][]TopologyHint, policyName string, opts PolicyOptions) HintMerger {
+	compareNumaAffinityMasks := func(current, candidate *TopologyHint) *TopologyHint {
+		// If current and candidate bitmasks are the same, prefer current hint.
+		if candidate.NUMANodeAffinity.IsEqual(current.NUMANodeAffinity) {
+			return current
+		}
+
+		// Otherwise compare the hints, based on the policy options provided
+		var best bitmask.BitMask
+		if (policyName != PolicySingleNumaNode) && opts.PreferClosestNUMA {
+			best = numaInfo.Closest(current.NUMANodeAffinity, candidate.NUMANodeAffinity)
+		} else {
+			best = numaInfo.Narrowest(current.NUMANodeAffinity, candidate.NUMANodeAffinity)
+		}
+		if best.IsEqual(current.NUMANodeAffinity) {
+			return current
+		}
+		return candidate
+	}
+
+	merger := HintMerger{
+		NUMAInfo:                      numaInfo,
+		Hints:                         hints,
+		BestNonPreferredAffinityCount: maxOfMinAffinityCounts(hints),
+		CompareNUMAAffinityMasks:      compareNumaAffinityMasks,
+	}
+
+	return merger
+}
+
+func (m HintMerger) compare(current *TopologyHint, candidate *TopologyHint) *TopologyHint {
 	// Only consider candidates that result in a NUMANodeAffinity > 0 to
 	// replace the current bestHint.
 	if candidate.NUMANodeAffinity.Count() == 0 {
@@ -146,20 +188,18 @@ func compareHints(bestNonPreferredAffinityCount int, current *TopologyHint, cand
 	}
 
 	// If the current bestHint is preferred and the candidate hint is
-	// non-preferred, never update the bestHint, regardless of the
+	// non-preferred, never update the bestHint, regardless of how
-	// candidate hint's narowness.
+	// the candidate hint's affinity mask compares to the current
+	// hint's affinity mask.
 	if current.Preferred && !candidate.Preferred {
 		return current
 	}
 
 	// If the current bestHint and the candidate hint are both preferred,
-	// then only consider candidate hints that have a narrower
+	// then only consider fitter NUMANodeAffinity
-	// NUMANodeAffinity than the NUMANodeAffinity in the current bestHint.
 	if current.Preferred && candidate.Preferred {
-		if candidate.NUMANodeAffinity.IsNarrowerThan(current.NUMANodeAffinity) {
+		return m.CompareNUMAAffinityMasks(current, candidate)
-			return candidate
+
-		}
-		return current
 	}
 
 	// The only case left is if the current best bestHint and the candidate
@@ -173,13 +213,13 @@ func compareHints(bestNonPreferredAffinityCount int, current *TopologyHint, cand
 	//   3. current.NUMANodeAffinity.Count() <  bestNonPreferredAffinityCount
 	//
 	// For case (1), the current bestHint is larger than the
-	// bestNonPreferredAffinityCount, so updating to any narrower mergeHint
+	// bestNonPreferredAffinityCount, so updating to fitter mergeHint
 	// is preferred over staying where we are.
 	//
 	// For case (2), the current bestHint is equal to the
 	// bestNonPreferredAffinityCount, so we would like to stick with what
 	// we have *unless* the candidate hint is also equal to
-	// bestNonPreferredAffinityCount and it is narrower.
+	// bestNonPreferredAffinityCount and it is fitter.
 	//
 	// For case (3), the current bestHint is less than
 	// bestNonPreferredAffinityCount, so we would like to creep back up to
@@ -216,33 +256,28 @@ func compareHints(bestNonPreferredAffinityCount int, current *TopologyHint, cand
 	// the bestNonPreferredAffinityCount.
 	//
 	// Finally, for case (3cc), we know that the current bestHint and the
-	// candidate hint are equal, so we simply choose the narrower of the 2.
+	// candidate hint are equal, so we simply choose the fitter of the 2.
 
 	// Case 1
-	if current.NUMANodeAffinity.Count() > bestNonPreferredAffinityCount {
+	if current.NUMANodeAffinity.Count() > m.BestNonPreferredAffinityCount {
-		if candidate.NUMANodeAffinity.IsNarrowerThan(current.NUMANodeAffinity) {
+		return m.CompareNUMAAffinityMasks(current, candidate)
-			return candidate
-		}
-		return current
 	}
 	// Case 2
-	if current.NUMANodeAffinity.Count() == bestNonPreferredAffinityCount {
+	if current.NUMANodeAffinity.Count() == m.BestNonPreferredAffinityCount {
-		if candidate.NUMANodeAffinity.Count() != bestNonPreferredAffinityCount {
+		if candidate.NUMANodeAffinity.Count() != m.BestNonPreferredAffinityCount {
 			return current
 		}
-		if candidate.NUMANodeAffinity.IsNarrowerThan(current.NUMANodeAffinity) {
+		return m.CompareNUMAAffinityMasks(current, candidate)
-			return candidate
-		}
-		return current
 	}
 	// Case 3a
-	if candidate.NUMANodeAffinity.Count() > bestNonPreferredAffinityCount {
+	if candidate.NUMANodeAffinity.Count() > m.BestNonPreferredAffinityCount {
 		return current
 	}
 	// Case 3b
-	if candidate.NUMANodeAffinity.Count() == bestNonPreferredAffinityCount {
+	if candidate.NUMANodeAffinity.Count() == m.BestNonPreferredAffinityCount {
 		return candidate
 	}
+
 	// Case 3ca
 	if candidate.NUMANodeAffinity.Count() > current.NUMANodeAffinity.Count() {
 		return candidate
@@ -251,35 +286,27 @@ func compareHints(bestNonPreferredAffinityCount int, current *TopologyHint, cand
 	if candidate.NUMANodeAffinity.Count() < current.NUMANodeAffinity.Count() {
 		return current
 	}
+
 	// Case 3cc
-	if candidate.NUMANodeAffinity.IsNarrowerThan(current.NUMANodeAffinity) {
+	return m.CompareNUMAAffinityMasks(current, candidate)
-		return candidate
+
-	}
-	return current
 }
 
-func mergeFilteredHints(numaNodes []int, filteredHints [][]TopologyHint) TopologyHint {
+func (m HintMerger) Merge() TopologyHint {
-	// Set bestNonPreferredAffinityCount to help decide which affinity mask is
+	defaultAffinity := m.NUMAInfo.DefaultAffinityMask()
-	// preferred amongst all non-preferred hints. We calculate this value as
-	// the maximum of the minimum affinity counts supplied for any given hint
-	// provider. In other words, prefer a hint that has an affinity mask that
-	// includes all of the NUMA nodes from the provider that requires the most
-	// NUMA nodes to satisfy its allocation.
-	bestNonPreferredAffinityCount := maxOfMinAffinityCounts(filteredHints)
 
 	var bestHint *TopologyHint
-	iterateAllProviderTopologyHints(filteredHints, func(permutation []TopologyHint) {
+	iterateAllProviderTopologyHints(m.Hints, func(permutation []TopologyHint) {
 		// Get the NUMANodeAffinity from each hint in the permutation and see if any
 		// of them encode unpreferred allocations.
-		mergedHint := mergePermutation(numaNodes, permutation)
+		mergedHint := mergePermutation(defaultAffinity, permutation)
 
 		// Compare the current bestHint with the candidate mergedHint and
 		// update bestHint if appropriate.
-		bestHint = compareHints(bestNonPreferredAffinityCount, bestHint, &mergedHint)
+		bestHint = m.compare(bestHint, &mergedHint)
 	})
 
 	if bestHint == nil {
-		defaultAffinity, _ := bitmask.NewBitMask(numaNodes...)
 		bestHint = &TopologyHint{defaultAffinity, false}
 	}
 

pkg/kubelet/cm/topologymanager/policy_best_effort.go

@@ -17,8 +17,9 @@ limitations under the License.
 package topologymanager
 
 type bestEffortPolicy struct {
-	//List of NUMA Nodes available on the underlying machine
+	// numaInfo represents list of NUMA Nodes available on the underlying machine and distances between them
-	numaNodes []int
+	numaInfo *NUMAInfo
+	opts     PolicyOptions
 }
 
 var _ Policy = &bestEffortPolicy{}
@@ -27,8 +28,13 @@ var _ Policy = &bestEffortPolicy{}
 const PolicyBestEffort string = "best-effort"
 
 // NewBestEffortPolicy returns best-effort policy.
-func NewBestEffortPolicy(numaNodes []int) Policy {
+func NewBestEffortPolicy(numaInfo *NUMAInfo, topologyPolicyOptions map[string]string) (Policy, error) {
-	return &bestEffortPolicy{numaNodes: numaNodes}
+	opts, err := NewPolicyOptions(topologyPolicyOptions)
+	if err != nil {
+		return nil, err
+	}
+
+	return &bestEffortPolicy{numaInfo: numaInfo, opts: opts}, nil
 }
 
 func (p *bestEffortPolicy) Name() string {
@@ -40,8 +46,9 @@ func (p *bestEffortPolicy) canAdmitPodResult(hint *TopologyHint) bool {
 }
 
 func (p *bestEffortPolicy) Merge(providersHints []map[string][]TopologyHint) (TopologyHint, bool) {
-	filteredProvidersHints := filterProvidersHints(providersHints)
+	filteredHints := filterProvidersHints(providersHints)
-	bestHint := mergeFilteredHints(p.numaNodes, filteredProvidersHints)
+	merger := NewHintMerger(p.numaInfo, filteredHints, p.Name(), p.opts)
+	bestHint := merger.Merge()
 	admit := p.canAdmitPodResult(&bestHint)
 	return bestHint, admit
 }

pkg/kubelet/cm/topologymanager/policy_best_effort_test.go

@@ -39,9 +39,9 @@ func TestPolicyBestEffortCanAdmitPodResult(t *testing.T) {
 	}
 
 	for _, tc := range tcases {
-		numaNodes := []int{0, 1}
+		numaInfo := commonNUMAInfoTwoNodes()
-		policy := NewBestEffortPolicy(numaNodes)
+		policy := &bestEffortPolicy{numaInfo: numaInfo}
-		result := policy.(*bestEffortPolicy).canAdmitPodResult(&tc.hint)
+		result := policy.canAdmitPodResult(&tc.hint)
 
 		if result != tc.expected {
 			t.Errorf("Expected result to be %t, got %t", tc.expected, result)
@@ -50,11 +50,26 @@ func TestPolicyBestEffortCanAdmitPodResult(t *testing.T) {
 }
 
 func TestPolicyBestEffortMerge(t *testing.T) {
-	numaNodes := []int{0, 1, 2, 3}
+	numaInfo := commonNUMAInfoFourNodes()
-	policy := NewBestEffortPolicy(numaNodes)
+	policy := &bestEffortPolicy{numaInfo: numaInfo}
 
-	tcases := commonPolicyMergeTestCases(numaNodes)
+	tcases := commonPolicyMergeTestCases(numaInfo.Nodes)
-	tcases = append(tcases, policy.(*bestEffortPolicy).mergeTestCases(numaNodes)...)
+	tcases = append(tcases, policy.mergeTestCases(numaInfo.Nodes)...)
+	tcases = append(tcases, policy.mergeTestCasesNoPolicies(numaInfo.Nodes)...)
+
+	testPolicyMerge(policy, tcases, t)
+}
+
+func TestPolicyBestEffortMergeClosestNUMA(t *testing.T) {
+	numaInfo := commonNUMAInfoEightNodes()
+	opts := PolicyOptions{
+		PreferClosestNUMA: true,
+	}
+	policy := &bestEffortPolicy{numaInfo: numaInfo, opts: opts}
+
+	tcases := commonPolicyMergeTestCases(numaInfo.Nodes)
+	tcases = append(tcases, policy.mergeTestCases(numaInfo.Nodes)...)
+	tcases = append(tcases, policy.mergeTestCasesClosestNUMA(numaInfo.Nodes)...)
 
 	testPolicyMerge(policy, tcases, t)
 }

pkg/kubelet/cm/topologymanager/policy_restricted.go

@@ -26,8 +26,13 @@ var _ Policy = &restrictedPolicy{}
 const PolicyRestricted string = "restricted"
 
 // NewRestrictedPolicy returns restricted policy.
-func NewRestrictedPolicy(numaNodes []int) Policy {
+func NewRestrictedPolicy(numaInfo *NUMAInfo, topologyPolicyOptions map[string]string) (Policy, error) {
-	return &restrictedPolicy{bestEffortPolicy{numaNodes: numaNodes}}
+	opts, err := NewPolicyOptions(topologyPolicyOptions)
+	if err != nil {
+		return nil, err
+	}
+
+	return &restrictedPolicy{bestEffortPolicy{numaInfo: numaInfo, opts: opts}}, nil
 }
 
 func (p *restrictedPolicy) Name() string {
@@ -40,7 +45,8 @@ func (p *restrictedPolicy) canAdmitPodResult(hint *TopologyHint) bool {
 
 func (p *restrictedPolicy) Merge(providersHints []map[string][]TopologyHint) (TopologyHint, bool) {
 	filteredHints := filterProvidersHints(providersHints)
-	hint := mergeFilteredHints(p.numaNodes, filteredHints)
+	merger := NewHintMerger(p.numaInfo, filteredHints, p.Name(), p.opts)
-	admit := p.canAdmitPodResult(&hint)
+	bestHint := merger.Merge()
-	return hint, admit
+	admit := p.canAdmitPodResult(&bestHint)
+	return bestHint, admit
 }

pkg/kubelet/cm/topologymanager/policy_restricted_test.go

@@ -30,8 +30,9 @@ func TestPolicyRestrictedName(t *testing.T) {
 			expected: "restricted",
 		},
 	}
+	numaInfo := commonNUMAInfoTwoNodes()
 	for _, tc := range tcases {
-		policy := NewRestrictedPolicy([]int{0, 1})
+		policy := &restrictedPolicy{bestEffortPolicy{numaInfo: numaInfo, opts: PolicyOptions{}}}
 		if policy.Name() != tc.expected {
 			t.Errorf("Expected Policy Name to be %s, got %s", tc.expected, policy.Name())
 		}
@@ -57,9 +58,9 @@ func TestPolicyRestrictedCanAdmitPodResult(t *testing.T) {
 	}
 
 	for _, tc := range tcases {
-		numaNodes := []int{0, 1}
+		numaInfo := commonNUMAInfoTwoNodes()
-		policy := NewRestrictedPolicy(numaNodes)
+		policy := &restrictedPolicy{bestEffortPolicy{numaInfo: numaInfo}}
-		result := policy.(*restrictedPolicy).canAdmitPodResult(&tc.hint)
+		result := policy.canAdmitPodResult(&tc.hint)
 
 		if result != tc.expected {
 			t.Errorf("Expected result to be %t, got %t", tc.expected, result)
@@ -68,11 +69,23 @@ func TestPolicyRestrictedCanAdmitPodResult(t *testing.T) {
 }
 
 func TestPolicyRestrictedMerge(t *testing.T) {
-	numaNodes := []int{0, 1, 2, 3}
+	numaInfo := commonNUMAInfoFourNodes()
-	policy := NewRestrictedPolicy(numaNodes)
+	policy := &restrictedPolicy{bestEffortPolicy{numaInfo: numaInfo}}
 
-	tcases := commonPolicyMergeTestCases(numaNodes)
+	tcases := commonPolicyMergeTestCases(numaInfo.Nodes)
-	tcases = append(tcases, policy.(*restrictedPolicy).mergeTestCases(numaNodes)...)
+	tcases = append(tcases, policy.mergeTestCases(numaInfo.Nodes)...)
+	tcases = append(tcases, policy.mergeTestCasesNoPolicies(numaInfo.Nodes)...)
+
+	testPolicyMerge(policy, tcases, t)
+}
+
+func TestPolicyRestrictedMergeClosestNUMA(t *testing.T) {
+	numaInfo := commonNUMAInfoEightNodes()
+	policy := &restrictedPolicy{bestEffortPolicy{numaInfo: numaInfo, opts: PolicyOptions{PreferClosestNUMA: true}}}
+
+	tcases := commonPolicyMergeTestCases(numaInfo.Nodes)
+	tcases = append(tcases, policy.mergeTestCases(numaInfo.Nodes)...)
+	tcases = append(tcases, policy.mergeTestCasesClosestNUMA(numaInfo.Nodes)...)
 
 	testPolicyMerge(policy, tcases, t)
 }

pkg/kubelet/cm/topologymanager/policy_single_numa_node.go

@@ -16,13 +16,10 @@ limitations under the License.
 
 package topologymanager
 
-import (
-	"k8s.io/kubernetes/pkg/kubelet/cm/topologymanager/bitmask"
-)
-
 type singleNumaNodePolicy struct {
-	//List of NUMA Nodes available on the underlying machine
+	// numaInfo represents list of NUMA Nodes available on the underlying machine and distances between them
-	numaNodes []int
+	numaInfo *NUMAInfo
+	opts     PolicyOptions
 }
 
 var _ Policy = &singleNumaNodePolicy{}
@@ -31,8 +28,13 @@ var _ Policy = &singleNumaNodePolicy{}
 const PolicySingleNumaNode string = "single-numa-node"
 
 // NewSingleNumaNodePolicy returns single-numa-node policy.
-func NewSingleNumaNodePolicy(numaNodes []int) Policy {
+func NewSingleNumaNodePolicy(numaInfo *NUMAInfo, topologyPolicyOptions map[string]string) (Policy, error) {
-	return &singleNumaNodePolicy{numaNodes: numaNodes}
+	opts, err := NewPolicyOptions(topologyPolicyOptions)
+	if err != nil {
+		return nil, err
+	}
+
+	return &singleNumaNodePolicy{numaInfo: numaInfo, opts: opts}, nil
 }
 
 func (p *singleNumaNodePolicy) Name() string {
@@ -65,10 +67,11 @@ func (p *singleNumaNodePolicy) Merge(providersHints []map[string][]TopologyHint)
 	filteredHints := filterProvidersHints(providersHints)
 	// Filter to only include don't cares and hints with a single NUMA node.
 	singleNumaHints := filterSingleNumaHints(filteredHints)
-	bestHint := mergeFilteredHints(p.numaNodes, singleNumaHints)
 
-	defaultAffinity, _ := bitmask.NewBitMask(p.numaNodes...)
+	merger := NewHintMerger(p.numaInfo, singleNumaHints, p.Name(), p.opts)
-	if bestHint.NUMANodeAffinity.IsEqual(defaultAffinity) {
+	bestHint := merger.Merge()
+
+	if bestHint.NUMANodeAffinity.IsEqual(p.numaInfo.DefaultAffinityMask()) {
 		bestHint = TopologyHint{nil, bestHint.Preferred}
 	}
 

pkg/kubelet/cm/topologymanager/policy_single_numa_node_test.go

@@ -33,11 +33,11 @@ func TestPolicySingleNumaNodeCanAdmitPodResult(t *testing.T) {
 			expected: false,
 		},
 	}
+	numaInfo := commonNUMAInfoTwoNodes()
 
 	for _, tc := range tcases {
-		numaNodes := []int{0, 1}
+		policy := singleNumaNodePolicy{numaInfo: numaInfo, opts: PolicyOptions{}}
-		policy := NewSingleNumaNodePolicy(numaNodes)
+		result := policy.canAdmitPodResult(&tc.hint)
-		result := policy.(*singleNumaNodePolicy).canAdmitPodResult(&tc.hint)
 
 		if result != tc.expected {
 			t.Errorf("Expected result to be %t, got %t", tc.expected, result)
@@ -156,11 +156,11 @@ func TestPolicySingleNumaNodeFilterHints(t *testing.T) {
 }
 
 func TestPolicySingleNumaNodeMerge(t *testing.T) {
-	numaNodes := []int{0, 1, 2, 3}
+	numaInfo := commonNUMAInfoFourNodes()
-	policy := NewSingleNumaNodePolicy(numaNodes)
+	policy := singleNumaNodePolicy{numaInfo: numaInfo, opts: PolicyOptions{}}
 
-	tcases := commonPolicyMergeTestCases(numaNodes)
+	tcases := commonPolicyMergeTestCases(numaInfo.Nodes)
-	tcases = append(tcases, policy.(*singleNumaNodePolicy).mergeTestCases(numaNodes)...)
+	tcases = append(tcases, policy.mergeTestCases(numaInfo.Nodes)...)
 
-	testPolicyMerge(policy, tcases, t)
+	testPolicyMerge(&policy, tcases, t)
 }

pkg/kubelet/cm/topologymanager/policy_test.go

@@ -748,6 +748,11 @@ func (p *bestEffortPolicy) mergeTestCases(numaNodes []int) []policyMergeTestCase
 				Preferred:        false,
 			},
 		},
+	}
+}
+
+func (p *bestEffortPolicy) mergeTestCasesNoPolicies(numaNodes []int) []policyMergeTestCase {
+	return []policyMergeTestCase{
 		{
 			name: "bestNonPreferredAffinityCount (5)",
 			hp: []HintProvider{
@@ -833,6 +838,167 @@ func (p *bestEffortPolicy) mergeTestCases(numaNodes []int) []policyMergeTestCase
 	}
 }
 
+func (p *bestEffortPolicy) mergeTestCasesClosestNUMA(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, 4),
+								Preferred:        true,
+							},
+							{
+								NUMANodeAffinity: NewTestBitMask(0, 2),
+								Preferred:        true,
+							},
+						},
+					},
+				},
+				&mockHintProvider{
+					map[string][]TopologyHint{
+						"resource2": {
+							{
+								NUMANodeAffinity: NewTestBitMask(0, 4),
+								Preferred:        true,
+							},
+							{
+								NUMANodeAffinity: NewTestBitMask(0, 2),
+								Preferred:        true,
+							},
+						},
+					},
+				},
+			},
+			expected: TopologyHint{
+				NUMANodeAffinity: NewTestBitMask(0, 2),
+				Preferred:        true,
+			},
+		},
+		{
+			name: "Two providers, 2 hints each, different mask",
+			hp: []HintProvider{
+				&mockHintProvider{
+					map[string][]TopologyHint{
+						"resource1": {
+							{
+								NUMANodeAffinity: NewTestBitMask(4),
+								Preferred:        true,
+							},
+							{
+								NUMANodeAffinity: NewTestBitMask(0, 2),
+								Preferred:        true,
+							},
+						},
+					},
+				},
+				&mockHintProvider{
+					map[string][]TopologyHint{
+						"resource2": {
+							{
+								NUMANodeAffinity: NewTestBitMask(4),
+								Preferred:        true,
+							},
+							{
+								NUMANodeAffinity: NewTestBitMask(0, 2),
+								Preferred:        true,
+							},
+						},
+					},
+				},
+			},
+			expected: TopologyHint{
+				NUMANodeAffinity: NewTestBitMask(4),
+				Preferred:        true,
+			},
+		},
+		{
+			name: "bestNonPreferredAffinityCount (5)",
+			hp: []HintProvider{
+				&mockHintProvider{
+					map[string][]TopologyHint{
+						"resource1": {
+							{
+								NUMANodeAffinity: NewTestBitMask(0, 1, 2, 3),
+								Preferred:        false,
+							},
+							{
+								NUMANodeAffinity: NewTestBitMask(0, 1),
+								Preferred:        false,
+							},
+						},
+					},
+				},
+				&mockHintProvider{
+					map[string][]TopologyHint{
+						"resource2": {
+							{
+								NUMANodeAffinity: NewTestBitMask(1, 2),
+								Preferred:        false,
+							},
+							{
+								NUMANodeAffinity: NewTestBitMask(2, 3),
+								Preferred:        false,
+							},
+						},
+					},
+				},
+			},
+			expected: TopologyHint{
+				NUMANodeAffinity: NewTestBitMask(2, 3),
+				Preferred:        false,
+			},
+		},
+		{
+			name: "bestNonPreferredAffinityCount (6)",
+			hp: []HintProvider{
+				&mockHintProvider{
+					map[string][]TopologyHint{
+						"resource1": {
+							{
+								NUMANodeAffinity: NewTestBitMask(0, 1, 2, 3),
+								Preferred:        false,
+							},
+							{
+								NUMANodeAffinity: NewTestBitMask(0, 1),
+								Preferred:        false,
+							},
+						},
+					},
+				},
+				&mockHintProvider{
+					map[string][]TopologyHint{
+						"resource2": {
+							{
+								NUMANodeAffinity: NewTestBitMask(1, 2, 3),
+								Preferred:        false,
+							},
+							{
+								NUMANodeAffinity: NewTestBitMask(1, 2),
+								Preferred:        false,
+							},
+							{
+								NUMANodeAffinity: NewTestBitMask(1, 3),
+								Preferred:        false,
+							},
+							{
+								NUMANodeAffinity: NewTestBitMask(2, 3),
+								Preferred:        false,
+							},
+						},
+					},
+				},
+			},
+			expected: TopologyHint{
+				NUMANodeAffinity: NewTestBitMask(2, 3),
+				Preferred:        false,
+			},
+		},
+	}
+}
+
 func (p *singleNumaNodePolicy) mergeTestCases(numaNodes []int) []policyMergeTestCase {
 	return []policyMergeTestCase{
 		{
@@ -1200,7 +1366,7 @@ func TestMaxOfMinAffinityCounts(t *testing.T) {
 	}
 }
 
-func TestCompareHints(t *testing.T) {
+func TestCompareHintsNarrowest(t *testing.T) {
 	tcases := []struct {
 		description                   string
 		bestNonPreferredAffinityCount int
@@ -1387,7 +1553,11 @@ func TestCompareHints(t *testing.T) {
 
 	for _, tc := range tcases {
 		t.Run(tc.description, func(t *testing.T) {
-			result := compareHints(tc.bestNonPreferredAffinityCount, tc.current, tc.candidate)
+			numaInfo := &NUMAInfo{}
+			merger := NewHintMerger(numaInfo, [][]TopologyHint{}, PolicyBestEffort, PolicyOptions{})
+			merger.BestNonPreferredAffinityCount = tc.bestNonPreferredAffinityCount
+
+			result := merger.compare(tc.current, tc.candidate)
 			if result != tc.current && result != tc.candidate {
 				t.Errorf("Expected result to be either 'current' or 'candidate' hint")
 			}
@@ -1400,3 +1570,41 @@ func TestCompareHints(t *testing.T) {
 		})
 	}
 }
+
+func commonNUMAInfoTwoNodes() *NUMAInfo {
+	return &NUMAInfo{
+		Nodes: []int{0, 1},
+		NUMADistances: NUMADistances{
+			{10, 11},
+			{11, 10},
+		},
+	}
+}
+
+func commonNUMAInfoFourNodes() *NUMAInfo {
+	return &NUMAInfo{
+		Nodes: []int{0, 1, 2, 3},
+		NUMADistances: NUMADistances{
+			{10, 11, 12, 12},
+			{11, 10, 12, 12},
+			{12, 12, 10, 11},
+			{12, 12, 11, 10},
+		},
+	}
+}
+
+func commonNUMAInfoEightNodes() *NUMAInfo {
+	return &NUMAInfo{
+		Nodes: []int{0, 1, 2, 3, 4, 5, 6, 7},
+		NUMADistances: NUMADistances{
+			{10, 11, 12, 12, 30, 30, 30, 30},
+			{11, 10, 12, 12, 30, 30, 30, 30},
+			{12, 12, 10, 11, 30, 30, 30, 30},
+			{12, 12, 11, 10, 30, 30, 30, 30},
+			{30, 30, 30, 30, 10, 11, 12, 12},
+			{30, 30, 30, 30, 11, 10, 12, 12},
+			{30, 30, 30, 30, 12, 12, 10, 11},
+			{30, 30, 30, 30, 12, 12, 13, 10},
+		},
+	}
+}

pkg/kubelet/cm/topologymanager/topology_manager.go

@@ -130,15 +130,15 @@ func (th *TopologyHint) LessThan(other TopologyHint) bool {
 var _ Manager = &manager{}
 
 // NewManager creates a new TopologyManager based on provided policy and scope
-func NewManager(topology []cadvisorapi.Node, topologyPolicyName string, topologyScopeName string) (Manager, error) {
+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)
 
-	var numaNodes []int
+	numaInfo, err := NewNUMAInfo(topology)
-	for _, node := range topology {
+	if err != nil {
-		numaNodes = append(numaNodes, node.Id)
+		return nil, fmt.Errorf("cannot discover NUMA topology: %w", err)
 	}
 
-	if topologyPolicyName != PolicyNone && len(numaNodes) > maxAllowableNUMANodes {
+	if topologyPolicyName != PolicyNone && len(numaInfo.Nodes) > maxAllowableNUMANodes {
 		return nil, fmt.Errorf("unsupported on machines with more than %v NUMA Nodes", maxAllowableNUMANodes)
 	}
 
@@ -149,13 +149,22 @@ func NewManager(topology []cadvisorapi.Node, topologyPolicyName string, topology
 		policy = NewNonePolicy()
 
 	case PolicyBestEffort:
-		policy = NewBestEffortPolicy(numaNodes)
+		policy, err = NewBestEffortPolicy(numaInfo, topologyPolicyOptions)
+		if err != nil {
+			return nil, err
+		}
 
 	case PolicyRestricted:
-		policy = NewRestrictedPolicy(numaNodes)
+		policy, err = NewRestrictedPolicy(numaInfo, topologyPolicyOptions)
+		if err != nil {
+			return nil, err
+		}
 
 	case PolicySingleNumaNode:
-		policy = NewSingleNumaNodePolicy(numaNodes)
+		policy, err = NewSingleNumaNodePolicy(numaInfo, topologyPolicyOptions)
+		if err != nil {
+			return nil, err
+		}
 
 	default:
 		return nil, fmt.Errorf("unknown policy: \"%s\"", topologyPolicyName)

pkg/kubelet/cm/topologymanager/topology_manager_test.go

@@ -22,6 +22,9 @@ import (
 	"testing"
 
 	"k8s.io/api/core/v1"
+
+	cadvisorapi "github.com/google/cadvisor/info/v1"
+
 	"k8s.io/kubernetes/pkg/kubelet/cm/topologymanager/bitmask"
 	"k8s.io/kubernetes/pkg/kubelet/lifecycle"
 )
@@ -37,6 +40,8 @@ func TestNewManager(t *testing.T) {
 		policyName     string
 		expectedPolicy string
 		expectedError  error
+		topologyError  error
+		policyOptions  map[string]string
 	}{
 		{
 			description:    "Policy is set to none",
@@ -63,11 +68,30 @@ func TestNewManager(t *testing.T) {
 			policyName:    "unknown",
 			expectedError: fmt.Errorf("unknown policy: \"unknown\""),
 		},
+		{
+			description:    "Unknown policy name best-effort policy",
+			policyName:     "best-effort",
+			expectedPolicy: "best-effort",
+			expectedError:  fmt.Errorf("unknown Topology Manager Policy option:"),
+			policyOptions: map[string]string{
+				"unknown-option": "true",
+			},
+		},
+		{
+			description:    "Unknown policy name restricted policy",
+			policyName:     "restricted",
+			expectedPolicy: "restricted",
+			expectedError:  fmt.Errorf("unknown Topology Manager Policy option:"),
+			policyOptions: map[string]string{
+				"unknown-option": "true",
+			},
+		},
 	}
 
 	for _, tc := range tcases {
-		mngr, err := NewManager(nil, tc.policyName, "container")
+		topology := []cadvisorapi.Node{}
 
+		mngr, err := NewManager(topology, tc.policyName, "container", tc.policyOptions)
 		if tc.expectedError != nil {
 			if !strings.Contains(err.Error(), tc.expectedError.Error()) {
 				t.Errorf("Unexpected error message. Have: %s wants %s", err.Error(), tc.expectedError.Error())
@@ -107,7 +131,7 @@ func TestManagerScope(t *testing.T) {
 	}
 
 	for _, tc := range tcases {
-		mngr, err := NewManager(nil, "best-effort", tc.scopeName)
+		mngr, err := NewManager(nil, "best-effort", tc.scopeName, nil)
 
 		if tc.expectedError != nil {
 			if !strings.Contains(err.Error(), tc.expectedError.Error()) {
@@ -179,7 +203,18 @@ func TestAddHintProvider(t *testing.T) {
 }
 
 func TestAdmit(t *testing.T) {
-	numaNodes := []int{0, 1}
+	numaInfo := &NUMAInfo{
+		Nodes: []int{0, 1},
+		NUMADistances: NUMADistances{
+			{10, 11},
+			{11, 10},
+		},
+	}
+
+	opts := map[string]string{}
+	bePolicy, _ := NewBestEffortPolicy(numaInfo, opts)
+	restrictedPolicy, _ := NewRestrictedPolicy(numaInfo, opts)
+	singleNumaPolicy, _ := NewSingleNumaNodePolicy(numaInfo, opts)
 
 	tcases := []struct {
 		name     string
@@ -206,7 +241,7 @@ func TestAdmit(t *testing.T) {
 		{
 			name:     "QOSClass set as BestEffort. single-numa-node Policy. No Hints.",
 			qosClass: v1.PodQOSBestEffort,
-			policy:   NewRestrictedPolicy(numaNodes),
+			policy:   singleNumaPolicy,
 			hp: []HintProvider{
 				&mockHintProvider{},
 			},
@@ -215,7 +250,7 @@ func TestAdmit(t *testing.T) {
 		{
 			name:     "QOSClass set as BestEffort. Restricted Policy. No Hints.",
 			qosClass: v1.PodQOSBestEffort,
-			policy:   NewRestrictedPolicy(numaNodes),
+			policy:   restrictedPolicy,
 			hp: []HintProvider{
 				&mockHintProvider{},
 			},
@@ -224,7 +259,7 @@ func TestAdmit(t *testing.T) {
 		{
 			name:     "QOSClass set as Guaranteed. BestEffort Policy. Preferred Affinity.",
 			qosClass: v1.PodQOSGuaranteed,
-			policy:   NewBestEffortPolicy(numaNodes),
+			policy:   bePolicy,
 			hp: []HintProvider{
 				&mockHintProvider{
 					map[string][]TopologyHint{
@@ -246,7 +281,7 @@ func TestAdmit(t *testing.T) {
 		{
 			name:     "QOSClass set as Guaranteed. BestEffort Policy. More than one Preferred Affinity.",
 			qosClass: v1.PodQOSGuaranteed,
-			policy:   NewBestEffortPolicy(numaNodes),
+			policy:   bePolicy,
 			hp: []HintProvider{
 				&mockHintProvider{
 					map[string][]TopologyHint{
@@ -272,7 +307,7 @@ func TestAdmit(t *testing.T) {
 		{
 			name:     "QOSClass set as Burstable. BestEffort Policy. More than one Preferred Affinity.",
 			qosClass: v1.PodQOSBurstable,
-			policy:   NewBestEffortPolicy(numaNodes),
+			policy:   bePolicy,
 			hp: []HintProvider{
 				&mockHintProvider{
 					map[string][]TopologyHint{
@@ -298,7 +333,7 @@ func TestAdmit(t *testing.T) {
 		{
 			name:     "QOSClass set as Guaranteed. BestEffort Policy. No Preferred Affinity.",
 			qosClass: v1.PodQOSGuaranteed,
-			policy:   NewBestEffortPolicy(numaNodes),
+			policy:   bePolicy,
 			hp: []HintProvider{
 				&mockHintProvider{
 					map[string][]TopologyHint{
@@ -316,7 +351,7 @@ func TestAdmit(t *testing.T) {
 		{
 			name:     "QOSClass set as Guaranteed. Restricted Policy. Preferred Affinity.",
 			qosClass: v1.PodQOSGuaranteed,
-			policy:   NewRestrictedPolicy(numaNodes),
+			policy:   restrictedPolicy,
 			hp: []HintProvider{
 				&mockHintProvider{
 					map[string][]TopologyHint{
@@ -338,7 +373,7 @@ func TestAdmit(t *testing.T) {
 		{
 			name:     "QOSClass set as Burstable. Restricted Policy. Preferred Affinity.",
 			qosClass: v1.PodQOSBurstable,
-			policy:   NewRestrictedPolicy(numaNodes),
+			policy:   restrictedPolicy,
 			hp: []HintProvider{
 				&mockHintProvider{
 					map[string][]TopologyHint{
@@ -360,7 +395,7 @@ func TestAdmit(t *testing.T) {
 		{
 			name:     "QOSClass set as Guaranteed. Restricted Policy. More than one Preferred affinity.",
 			qosClass: v1.PodQOSGuaranteed,
-			policy:   NewRestrictedPolicy(numaNodes),
+			policy:   restrictedPolicy,
 			hp: []HintProvider{
 				&mockHintProvider{
 					map[string][]TopologyHint{
@@ -386,7 +421,7 @@ func TestAdmit(t *testing.T) {
 		{
 			name:     "QOSClass set as Burstable. Restricted Policy. More than one Preferred affinity.",
 			qosClass: v1.PodQOSBurstable,
-			policy:   NewRestrictedPolicy(numaNodes),
+			policy:   restrictedPolicy,
 			hp: []HintProvider{
 				&mockHintProvider{
 					map[string][]TopologyHint{
@@ -412,7 +447,7 @@ func TestAdmit(t *testing.T) {
 		{
 			name:     "QOSClass set as Guaranteed. Restricted Policy. No Preferred affinity.",
 			qosClass: v1.PodQOSGuaranteed,
-			policy:   NewRestrictedPolicy(numaNodes),
+			policy:   restrictedPolicy,
 			hp: []HintProvider{
 				&mockHintProvider{
 					map[string][]TopologyHint{
@@ -430,7 +465,7 @@ func TestAdmit(t *testing.T) {
 		{
 			name:     "QOSClass set as Burstable. Restricted Policy. No Preferred affinity.",
 			qosClass: v1.PodQOSBurstable,
-			policy:   NewRestrictedPolicy(numaNodes),
+			policy:   restrictedPolicy,
 			hp: []HintProvider{
 				&mockHintProvider{
 					map[string][]TopologyHint{