From 75bb437a6b3a992e60d2422f8d390280d42bba59 Mon Sep 17 00:00:00 2001 From: PiotrProkop Date: Thu, 3 Nov 2022 10:44:15 +0100 Subject: [PATCH] Improved multi-numa alignment in Topology Manager: implement closest numa policy Signed-off-by: PiotrProkop --- pkg/kubelet/cm/container_manager_linux.go | 1 + .../cm/cpumanager/policy_options_test.go | 2 +- pkg/kubelet/cm/topologymanager/policy.go | 113 ++++++---- .../cm/topologymanager/policy_best_effort.go | 19 +- .../policy_best_effort_test.go | 29 ++- .../cm/topologymanager/policy_restricted.go | 16 +- .../topologymanager/policy_restricted_test.go | 29 ++- .../policy_single_numa_node.go | 25 ++- .../policy_single_numa_node_test.go | 16 +- pkg/kubelet/cm/topologymanager/policy_test.go | 212 +++++++++++++++++- .../cm/topologymanager/topology_manager.go | 25 ++- .../topologymanager/topology_manager_test.go | 65 ++++-- 12 files changed, 438 insertions(+), 114 deletions(-) diff --git a/pkg/kubelet/cm/container_manager_linux.go b/pkg/kubelet/cm/container_manager_linux.go index a7d6c89342f..cfd629da953 100644 --- a/pkg/kubelet/cm/container_manager_linux.go +++ b/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 { diff --git a/pkg/kubelet/cm/cpumanager/policy_options_test.go b/pkg/kubelet/cm/cpumanager/policy_options_test.go index 8fc3bb8bdc3..60917d06f4f 100644 --- a/pkg/kubelet/cm/cpumanager/policy_options_test.go +++ b/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) diff --git a/pkg/kubelet/cm/topologymanager/policy.go b/pkg/kubelet/cm/topologymanager/policy.go index 38ad9792c5c..09f1c7bc2cf 100644 --- a/pkg/kubelet/cm/topologymanager/policy.go +++ b/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 - // candidate hint's narowness. + // non-preferred, never update the bestHint, regardless of how + // 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 - // NUMANodeAffinity than the NUMANodeAffinity in the current bestHint. + // then only consider fitter NUMANodeAffinity if current.Preferred && candidate.Preferred { - if candidate.NUMANodeAffinity.IsNarrowerThan(current.NUMANodeAffinity) { - return candidate - } - return current + return m.CompareNUMAAffinityMasks(current, candidate) + } // 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 candidate.NUMANodeAffinity.IsNarrowerThan(current.NUMANodeAffinity) { - return candidate - } - return current + if current.NUMANodeAffinity.Count() > m.BestNonPreferredAffinityCount { + return m.CompareNUMAAffinityMasks(current, candidate) } // Case 2 - if current.NUMANodeAffinity.Count() == bestNonPreferredAffinityCount { - if candidate.NUMANodeAffinity.Count() != bestNonPreferredAffinityCount { + if current.NUMANodeAffinity.Count() == m.BestNonPreferredAffinityCount { + if candidate.NUMANodeAffinity.Count() != m.BestNonPreferredAffinityCount { return current } - if candidate.NUMANodeAffinity.IsNarrowerThan(current.NUMANodeAffinity) { - return candidate - } - return current + return m.CompareNUMAAffinityMasks(current, candidate) } // 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 candidate - } - return current + return m.CompareNUMAAffinityMasks(current, candidate) + } -func mergeFilteredHints(numaNodes []int, filteredHints [][]TopologyHint) 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 := maxOfMinAffinityCounts(filteredHints) +func (m HintMerger) Merge() TopologyHint { + defaultAffinity := m.NUMAInfo.DefaultAffinityMask() 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} } diff --git a/pkg/kubelet/cm/topologymanager/policy_best_effort.go b/pkg/kubelet/cm/topologymanager/policy_best_effort.go index 651f3a76572..186e622fc1c 100644 --- a/pkg/kubelet/cm/topologymanager/policy_best_effort.go +++ b/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 - numaNodes []int + // numaInfo represents list of NUMA Nodes available on the underlying machine and distances between them + 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 { - return &bestEffortPolicy{numaNodes: numaNodes} +func NewBestEffortPolicy(numaInfo *NUMAInfo, topologyPolicyOptions map[string]string) (Policy, error) { + 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) - bestHint := mergeFilteredHints(p.numaNodes, filteredProvidersHints) + filteredHints := filterProvidersHints(providersHints) + merger := NewHintMerger(p.numaInfo, filteredHints, p.Name(), p.opts) + bestHint := merger.Merge() admit := p.canAdmitPodResult(&bestHint) return bestHint, admit } diff --git a/pkg/kubelet/cm/topologymanager/policy_best_effort_test.go b/pkg/kubelet/cm/topologymanager/policy_best_effort_test.go index 43bcc6ca933..70b59a4b7f0 100644 --- a/pkg/kubelet/cm/topologymanager/policy_best_effort_test.go +++ b/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} - policy := NewBestEffortPolicy(numaNodes) - result := policy.(*bestEffortPolicy).canAdmitPodResult(&tc.hint) + numaInfo := commonNUMAInfoTwoNodes() + policy := &bestEffortPolicy{numaInfo: numaInfo} + 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} - policy := NewBestEffortPolicy(numaNodes) + numaInfo := commonNUMAInfoFourNodes() + policy := &bestEffortPolicy{numaInfo: numaInfo} - tcases := commonPolicyMergeTestCases(numaNodes) - tcases = append(tcases, policy.(*bestEffortPolicy).mergeTestCases(numaNodes)...) + tcases := commonPolicyMergeTestCases(numaInfo.Nodes) + 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) } diff --git a/pkg/kubelet/cm/topologymanager/policy_restricted.go b/pkg/kubelet/cm/topologymanager/policy_restricted.go index 5ee2f245d63..8d515e3d4c9 100644 --- a/pkg/kubelet/cm/topologymanager/policy_restricted.go +++ b/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 { - return &restrictedPolicy{bestEffortPolicy{numaNodes: numaNodes}} +func NewRestrictedPolicy(numaInfo *NUMAInfo, topologyPolicyOptions map[string]string) (Policy, error) { + 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) - admit := p.canAdmitPodResult(&hint) - return hint, admit + merger := NewHintMerger(p.numaInfo, filteredHints, p.Name(), p.opts) + bestHint := merger.Merge() + admit := p.canAdmitPodResult(&bestHint) + return bestHint, admit } diff --git a/pkg/kubelet/cm/topologymanager/policy_restricted_test.go b/pkg/kubelet/cm/topologymanager/policy_restricted_test.go index d2623199208..b17fd147696 100644 --- a/pkg/kubelet/cm/topologymanager/policy_restricted_test.go +++ b/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} - policy := NewRestrictedPolicy(numaNodes) - result := policy.(*restrictedPolicy).canAdmitPodResult(&tc.hint) + numaInfo := commonNUMAInfoTwoNodes() + policy := &restrictedPolicy{bestEffortPolicy{numaInfo: numaInfo}} + 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} - policy := NewRestrictedPolicy(numaNodes) + numaInfo := commonNUMAInfoFourNodes() + policy := &restrictedPolicy{bestEffortPolicy{numaInfo: numaInfo}} - tcases := commonPolicyMergeTestCases(numaNodes) - tcases = append(tcases, policy.(*restrictedPolicy).mergeTestCases(numaNodes)...) + tcases := commonPolicyMergeTestCases(numaInfo.Nodes) + 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) } diff --git a/pkg/kubelet/cm/topologymanager/policy_single_numa_node.go b/pkg/kubelet/cm/topologymanager/policy_single_numa_node.go index 7745951c085..54395b5ec1e 100644 --- a/pkg/kubelet/cm/topologymanager/policy_single_numa_node.go +++ b/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 - numaNodes []int + // numaInfo represents list of NUMA Nodes available on the underlying machine and distances between them + 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 { - return &singleNumaNodePolicy{numaNodes: numaNodes} +func NewSingleNumaNodePolicy(numaInfo *NUMAInfo, topologyPolicyOptions map[string]string) (Policy, error) { + 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...) - if bestHint.NUMANodeAffinity.IsEqual(defaultAffinity) { + merger := NewHintMerger(p.numaInfo, singleNumaHints, p.Name(), p.opts) + bestHint := merger.Merge() + + if bestHint.NUMANodeAffinity.IsEqual(p.numaInfo.DefaultAffinityMask()) { bestHint = TopologyHint{nil, bestHint.Preferred} } diff --git a/pkg/kubelet/cm/topologymanager/policy_single_numa_node_test.go b/pkg/kubelet/cm/topologymanager/policy_single_numa_node_test.go index f2303e72a80..39b1fe33c68 100644 --- a/pkg/kubelet/cm/topologymanager/policy_single_numa_node_test.go +++ b/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 := NewSingleNumaNodePolicy(numaNodes) - result := policy.(*singleNumaNodePolicy).canAdmitPodResult(&tc.hint) + policy := singleNumaNodePolicy{numaInfo: numaInfo, opts: PolicyOptions{}} + result := policy.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} - policy := NewSingleNumaNodePolicy(numaNodes) + numaInfo := commonNUMAInfoFourNodes() + policy := singleNumaNodePolicy{numaInfo: numaInfo, opts: PolicyOptions{}} - tcases := commonPolicyMergeTestCases(numaNodes) - tcases = append(tcases, policy.(*singleNumaNodePolicy).mergeTestCases(numaNodes)...) + tcases := commonPolicyMergeTestCases(numaInfo.Nodes) + tcases = append(tcases, policy.mergeTestCases(numaInfo.Nodes)...) - testPolicyMerge(policy, tcases, t) + testPolicyMerge(&policy, tcases, t) } diff --git a/pkg/kubelet/cm/topologymanager/policy_test.go b/pkg/kubelet/cm/topologymanager/policy_test.go index df9f7e8d5f6..e0924507f80 100644 --- a/pkg/kubelet/cm/topologymanager/policy_test.go +++ b/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}, + }, + } +} diff --git a/pkg/kubelet/cm/topologymanager/topology_manager.go b/pkg/kubelet/cm/topologymanager/topology_manager.go index ea5ac91560d..1484af3bd42 100644 --- a/pkg/kubelet/cm/topologymanager/topology_manager.go +++ b/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 - for _, node := range topology { - numaNodes = append(numaNodes, node.Id) + numaInfo, err := NewNUMAInfo(topology) + if err != nil { + 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) diff --git a/pkg/kubelet/cm/topologymanager/topology_manager_test.go b/pkg/kubelet/cm/topologymanager/topology_manager_test.go index c0cc0980c65..3573b045a58 100644 --- a/pkg/kubelet/cm/topologymanager/topology_manager_test.go +++ b/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{