diff --git a/pkg/kubelet/cm/cpumanager/topology_hints_test.go b/pkg/kubelet/cm/cpumanager/topology_hints_test.go index 3f0be9a6388..f998e734159 100644 --- a/pkg/kubelet/cm/cpumanager/topology_hints_test.go +++ b/pkg/kubelet/cm/cpumanager/topology_hints_test.go @@ -29,13 +29,6 @@ import ( "k8s.io/kubernetes/pkg/kubelet/cm/topologymanager/bitmask" ) -func topologyHintLessThan(a topologymanager.TopologyHint, b topologymanager.TopologyHint) bool { - if a.Preferred != b.Preferred { - return a.Preferred == true - } - return a.NUMANodeAffinity.IsNarrowerThan(b.NUMANodeAffinity) -} - func TestGetTopologyHints(t *testing.T) { testPod1 := makePod("2", "2") testContainer1 := &testPod1.Spec.Containers[0] @@ -168,10 +161,10 @@ func TestGetTopologyHints(t *testing.T) { continue } sort.SliceStable(hints, func(i, j int) bool { - return topologyHintLessThan(hints[i], hints[j]) + return hints[i].LessThan(hints[j]) }) sort.SliceStable(tc.expectedHints, func(i, j int) bool { - return topologyHintLessThan(tc.expectedHints[i], tc.expectedHints[j]) + return tc.expectedHints[i].LessThan(tc.expectedHints[j]) }) if !reflect.DeepEqual(tc.expectedHints, hints) { t.Errorf("Expected in result to be %v , got %v", tc.expectedHints, hints) diff --git a/pkg/kubelet/cm/devicemanager/topology_hints_test.go b/pkg/kubelet/cm/devicemanager/topology_hints_test.go index 1222e042e45..e77cf3b4957 100644 --- a/pkg/kubelet/cm/devicemanager/topology_hints_test.go +++ b/pkg/kubelet/cm/devicemanager/topology_hints_test.go @@ -44,13 +44,6 @@ func makeNUMADevice(id string, numa int) pluginapi.Device { } } -func topologyHintLessThan(a topologymanager.TopologyHint, b topologymanager.TopologyHint) bool { - if a.Preferred != b.Preferred { - return a.Preferred == true - } - return a.NUMANodeAffinity.IsNarrowerThan(b.NUMANodeAffinity) -} - func makeSocketMask(sockets ...int) bitmask.BitMask { mask, _ := bitmask.NewBitMask(sockets...) return mask @@ -292,10 +285,10 @@ func TestGetTopologyHints(t *testing.T) { for r := range tc.expectedHints { sort.SliceStable(hints[r], func(i, j int) bool { - return topologyHintLessThan(hints[r][i], hints[r][j]) + return hints[r][i].LessThan(hints[r][j]) }) sort.SliceStable(tc.expectedHints[r], func(i, j int) bool { - return topologyHintLessThan(tc.expectedHints[r][i], tc.expectedHints[r][j]) + return tc.expectedHints[r][i].LessThan(tc.expectedHints[r][j]) }) if !reflect.DeepEqual(hints[r], tc.expectedHints[r]) { t.Errorf("%v: Expected result to be %v, got %v", tc.description, tc.expectedHints[r], hints[r]) diff --git a/pkg/kubelet/cm/topologymanager/BUILD b/pkg/kubelet/cm/topologymanager/BUILD index c0dd4d1249e..5d15446fd2e 100644 --- a/pkg/kubelet/cm/topologymanager/BUILD +++ b/pkg/kubelet/cm/topologymanager/BUILD @@ -47,6 +47,7 @@ go_test( "policy_none_test.go", "policy_restricted_test.go", "policy_single_numa_node_test.go", + "policy_test.go", "topology_manager_test.go", ], embed = [":go_default_library"], diff --git a/pkg/kubelet/cm/topologymanager/policy.go b/pkg/kubelet/cm/topologymanager/policy.go index 7ffa0a79f10..c93f3d135a0 100644 --- a/pkg/kubelet/cm/topologymanager/policy.go +++ b/pkg/kubelet/cm/topologymanager/policy.go @@ -20,10 +20,11 @@ import ( "k8s.io/kubernetes/pkg/kubelet/lifecycle" ) -//Policy interface for Topology Manager Pod Admit Result +// Policy interface for Topology Manager Pod Admit Result type Policy interface { - //Returns Policy Name + // Returns Policy Name Name() string - //Returns Pod Admit Handler Response based on hints and policy type - CanAdmitPodResult(hint *TopologyHint) lifecycle.PodAdmitResult + // Returns a merged TopologyHint based on input from hint providers + // and a Pod Admit Handler Response based on hints and policy type + Merge(providersHints []map[string][]TopologyHint) (TopologyHint, lifecycle.PodAdmitResult) } diff --git a/pkg/kubelet/cm/topologymanager/policy_best_effort.go b/pkg/kubelet/cm/topologymanager/policy_best_effort.go index b5f81b2fdb4..2a0d9f1d927 100644 --- a/pkg/kubelet/cm/topologymanager/policy_best_effort.go +++ b/pkg/kubelet/cm/topologymanager/policy_best_effort.go @@ -17,10 +17,15 @@ limitations under the License. package topologymanager import ( + "k8s.io/klog" + "k8s.io/kubernetes/pkg/kubelet/cm/topologymanager/bitmask" "k8s.io/kubernetes/pkg/kubelet/lifecycle" ) -type bestEffortPolicy struct{} +type bestEffortPolicy struct { + //List of NUMA Nodes available on the underlying machine + numaNodes []int +} var _ Policy = &bestEffortPolicy{} @@ -28,16 +33,167 @@ var _ Policy = &bestEffortPolicy{} const PolicyBestEffort string = "best-effort" // NewBestEffortPolicy returns best-effort policy. -func NewBestEffortPolicy() Policy { - return &bestEffortPolicy{} +func NewBestEffortPolicy(numaNodes []int) Policy { + return &bestEffortPolicy{numaNodes: numaNodes} } func (p *bestEffortPolicy) Name() string { return PolicyBestEffort } -func (p *bestEffortPolicy) CanAdmitPodResult(hint *TopologyHint) lifecycle.PodAdmitResult { +func (p *bestEffortPolicy) canAdmitPodResult(hint *TopologyHint) lifecycle.PodAdmitResult { return lifecycle.PodAdmitResult{ Admit: true, } } + +func (p *bestEffortPolicy) Merge(providersHints []map[string][]TopologyHint) (TopologyHint, lifecycle.PodAdmitResult) { + hint := mergeProvidersHints(p, p.numaNodes, providersHints) + admit := p.canAdmitPodResult(&hint) + return hint, admit +} + +// Iterate over all permutations of hints in 'allProviderHints [][]TopologyHint'. +// +// This procedure is implemented as a recursive function over the set of hints +// in 'allproviderHints[i]'. It applies the function 'callback' to each +// permutation as it is found. It is the equivalent of: +// +// for i := 0; i < len(providerHints[0]); i++ +// for j := 0; j < len(providerHints[1]); j++ +// for k := 0; k < len(providerHints[2]); k++ +// ... +// for z := 0; z < len(providerHints[-1]); z++ +// permutation := []TopologyHint{ +// providerHints[0][i], +// providerHints[1][j], +// providerHints[2][k], +// ... +// providerHints[-1][z] +// } +// callback(permutation) +func iterateAllProviderTopologyHints(allProviderHints [][]TopologyHint, callback func([]TopologyHint)) { + // Internal helper function to accumulate the permutation before calling the callback. + var iterate func(i int, accum []TopologyHint) + iterate = func(i int, accum []TopologyHint) { + // Base case: we have looped through all providers and have a full permutation. + if i == len(allProviderHints) { + callback(accum) + return + } + + // Loop through all hints for provider 'i', and recurse to build the + // the permutation of this hint with all hints from providers 'i++'. + for j := range allProviderHints[i] { + iterate(i+1, append(accum, allProviderHints[i][j])) + } + } + iterate(0, []TopologyHint{}) +} + +// Merge the hints from all hint providers to find the best one. +func mergeProvidersHints(policy Policy, numaNodes []int, providersHints []map[string][]TopologyHint) TopologyHint { + // Set the default affinity as an any-numa affinity containing the list + // of NUMA Nodes available on this machine. + defaultAffinity, _ := bitmask.NewBitMask(numaNodes...) + + // Loop through all hint providers and save an accumulated list of the + // hints returned by each hint provider. If no hints are provided, assume + // that provider has no preference for topology-aware allocation. + var allProviderHints [][]TopologyHint + for _, hints := range providersHints { + // If hints is nil, insert a single, preferred any-numa hint into allProviderHints. + if len(hints) == 0 { + klog.Infof("[topologymanager] Hint Provider has no preference for NUMA affinity with any resource") + allProviderHints = append(allProviderHints, []TopologyHint{{nil, true}}) + continue + } + + // Otherwise, accumulate the hints for each resource type into allProviderHints. + for resource := range hints { + if hints[resource] == nil { + klog.Infof("[topologymanager] Hint Provider has no preference for NUMA affinity with resource '%s'", resource) + allProviderHints = append(allProviderHints, []TopologyHint{{nil, true}}) + continue + } + + if len(hints[resource]) == 0 { + klog.Infof("[topologymanager] Hint Provider has no possible NUMA affinities for resource '%s'", resource) + allProviderHints = append(allProviderHints, []TopologyHint{{nil, false}}) + continue + } + + allProviderHints = append(allProviderHints, hints[resource]) + } + } + + // Iterate over all permutations of hints in 'allProviderHints'. Merge the + // hints in each permutation by taking the bitwise-and of their affinity masks. + // Return the hint with the narrowest NUMANodeAffinity of all merged + // permutations that have at least one NUMA ID set. If no merged mask can be + // found that has at least one NUMA ID set, return the 'defaultAffinity'. + bestHint := TopologyHint{defaultAffinity, false} + iterateAllProviderTopologyHints(allProviderHints, func(permutation []TopologyHint) { + // Get the NUMANodeAffinity from each hint in the permutation and see if any + // of them encode unpreferred allocations. + preferred := true + var numaAffinities []bitmask.BitMask + for _, hint := range permutation { + if hint.NUMANodeAffinity == nil { + numaAffinities = append(numaAffinities, defaultAffinity) + } else { + numaAffinities = append(numaAffinities, hint.NUMANodeAffinity) + } + + if !hint.Preferred { + preferred = false + } + + // Special case PolicySingleNumaNode to only prefer hints where + // all providers have a single NUMA affinity set. + if policy != nil && policy.Name() == PolicySingleNumaNode && hint.NUMANodeAffinity != nil && hint.NUMANodeAffinity.Count() > 1 { + preferred = false + } + } + + // Merge the affinities using a bitwise-and operation. + mergedAffinity, _ := bitmask.NewBitMask(numaNodes...) + mergedAffinity.And(numaAffinities...) + + // Build a mergedHintfrom the merged affinity mask, indicating if an + // preferred allocation was used to generate the affinity mask or not. + mergedHint := TopologyHint{mergedAffinity, preferred} + + // Only consider mergedHints that result in a NUMANodeAffinity > 0 to + // replace the current bestHint. + if mergedHint.NUMANodeAffinity.Count() == 0 { + return + } + + // If the current bestHint is non-preferred and the new mergedHint is + // preferred, always choose the preferred hint over the non-preferred one. + if mergedHint.Preferred && !bestHint.Preferred { + bestHint = mergedHint + return + } + + // If the current bestHint is preferred and the new mergedHint is + // non-preferred, never update bestHint, regardless of mergedHint's + // narowness. + if !mergedHint.Preferred && bestHint.Preferred { + return + } + + // If mergedHint and bestHint has the same preference, only consider + // mergedHints that have a narrower NUMANodeAffinity than the + // NUMANodeAffinity in the current bestHint. + if !mergedHint.NUMANodeAffinity.IsNarrowerThan(bestHint.NUMANodeAffinity) { + return + } + + // In all other cases, update bestHint to the current mergedHint + bestHint = mergedHint + }) + + return bestHint +} diff --git a/pkg/kubelet/cm/topologymanager/policy_best_effort_test.go b/pkg/kubelet/cm/topologymanager/policy_best_effort_test.go index 2fea3df9920..2ae7de0ecd8 100644 --- a/pkg/kubelet/cm/topologymanager/policy_best_effort_test.go +++ b/pkg/kubelet/cm/topologymanager/policy_best_effort_test.go @@ -39,11 +39,22 @@ func TestPolicyBestEffortCanAdmitPodResult(t *testing.T) { } for _, tc := range tcases { - policy := NewBestEffortPolicy() - result := policy.CanAdmitPodResult(&tc.hint) + numaNodes := []int{0, 1} + policy := NewBestEffortPolicy(numaNodes) + result := policy.(*bestEffortPolicy).canAdmitPodResult(&tc.hint) if result.Admit != tc.expected { t.Errorf("Expected Admit field in result to be %t, got %t", tc.expected, result.Admit) } } } + +func TestBestEffortPolicyMerge(t *testing.T) { + numaNodes := []int{0, 1} + policy := NewBestEffortPolicy(numaNodes) + + tcases := commonPolicyMergeTestCases(numaNodes) + tcases = append(tcases, policy.(*bestEffortPolicy).mergeTestCases(numaNodes)...) + + testPolicyMerge(policy, tcases, t) +} diff --git a/pkg/kubelet/cm/topologymanager/policy_none.go b/pkg/kubelet/cm/topologymanager/policy_none.go index aacdf19aa10..7e5010a765d 100644 --- a/pkg/kubelet/cm/topologymanager/policy_none.go +++ b/pkg/kubelet/cm/topologymanager/policy_none.go @@ -36,8 +36,12 @@ func (p *nonePolicy) Name() string { return PolicyNone } -func (p *nonePolicy) CanAdmitPodResult(hint *TopologyHint) lifecycle.PodAdmitResult { +func (p *nonePolicy) canAdmitPodResult(hint *TopologyHint) lifecycle.PodAdmitResult { return lifecycle.PodAdmitResult{ Admit: true, } } + +func (p *nonePolicy) Merge(providersHints []map[string][]TopologyHint) (TopologyHint, lifecycle.PodAdmitResult) { + return TopologyHint{}, p.canAdmitPodResult(nil) +} diff --git a/pkg/kubelet/cm/topologymanager/policy_none_test.go b/pkg/kubelet/cm/topologymanager/policy_none_test.go index 76f46e490b4..cafcaf044b6 100644 --- a/pkg/kubelet/cm/topologymanager/policy_none_test.go +++ b/pkg/kubelet/cm/topologymanager/policy_none_test.go @@ -58,7 +58,7 @@ func TestPolicyNoneCanAdmitPodResult(t *testing.T) { for _, tc := range tcases { policy := NewNonePolicy() - result := policy.CanAdmitPodResult(&tc.hint) + result := policy.(*nonePolicy).canAdmitPodResult(&tc.hint) if result.Admit != tc.expected { t.Errorf("Expected Admit field in result to be %t, got %t", tc.expected, result.Admit) diff --git a/pkg/kubelet/cm/topologymanager/policy_restricted.go b/pkg/kubelet/cm/topologymanager/policy_restricted.go index 7993675ce35..cc522980c0e 100644 --- a/pkg/kubelet/cm/topologymanager/policy_restricted.go +++ b/pkg/kubelet/cm/topologymanager/policy_restricted.go @@ -20,7 +20,9 @@ import ( "k8s.io/kubernetes/pkg/kubelet/lifecycle" ) -type restrictedPolicy struct{} +type restrictedPolicy struct { + bestEffortPolicy +} var _ Policy = &restrictedPolicy{} @@ -28,15 +30,15 @@ var _ Policy = &restrictedPolicy{} const PolicyRestricted string = "restricted" // NewRestrictedPolicy returns restricted policy. -func NewRestrictedPolicy() Policy { - return &restrictedPolicy{} +func NewRestrictedPolicy(numaNodes []int) Policy { + return &restrictedPolicy{bestEffortPolicy{numaNodes: numaNodes}} } func (p *restrictedPolicy) Name() string { return PolicyRestricted } -func (p *restrictedPolicy) CanAdmitPodResult(hint *TopologyHint) lifecycle.PodAdmitResult { +func (p *restrictedPolicy) canAdmitPodResult(hint *TopologyHint) lifecycle.PodAdmitResult { if !hint.Preferred { return lifecycle.PodAdmitResult{ Admit: false, @@ -48,3 +50,9 @@ func (p *restrictedPolicy) CanAdmitPodResult(hint *TopologyHint) lifecycle.PodAd Admit: true, } } + +func (p *restrictedPolicy) Merge(providersHints []map[string][]TopologyHint) (TopologyHint, lifecycle.PodAdmitResult) { + hint := mergeProvidersHints(p, p.numaNodes, providersHints) + admit := p.canAdmitPodResult(&hint) + return hint, admit +} diff --git a/pkg/kubelet/cm/topologymanager/policy_restricted_test.go b/pkg/kubelet/cm/topologymanager/policy_restricted_test.go index d011971632a..f9cf840708c 100644 --- a/pkg/kubelet/cm/topologymanager/policy_restricted_test.go +++ b/pkg/kubelet/cm/topologymanager/policy_restricted_test.go @@ -39,8 +39,9 @@ func TestPolicyRestrictedCanAdmitPodResult(t *testing.T) { } for _, tc := range tcases { - policy := NewRestrictedPolicy() - result := policy.CanAdmitPodResult(&tc.hint) + numaNodes := []int{0, 1} + policy := NewRestrictedPolicy(numaNodes) + result := policy.(*restrictedPolicy).canAdmitPodResult(&tc.hint) if result.Admit != tc.expected { t.Errorf("Expected Admit field in result to be %t, got %t", tc.expected, result.Admit) @@ -56,3 +57,13 @@ func TestPolicyRestrictedCanAdmitPodResult(t *testing.T) { } } } + +func TestRestrictedPolicyMerge(t *testing.T) { + numaNodes := []int{0, 1} + policy := NewRestrictedPolicy(numaNodes) + + tcases := commonPolicyMergeTestCases(numaNodes) + tcases = append(tcases, policy.(*restrictedPolicy).mergeTestCases(numaNodes)...) + + 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 e17e5ca949c..6f29e1c27b7 100644 --- a/pkg/kubelet/cm/topologymanager/policy_single_numa_node.go +++ b/pkg/kubelet/cm/topologymanager/policy_single_numa_node.go @@ -20,7 +20,10 @@ import ( "k8s.io/kubernetes/pkg/kubelet/lifecycle" ) -type singleNumaNodePolicy struct{} +type singleNumaNodePolicy struct { + //List of NUMA Nodes available on the underlying machine + numaNodes []int +} var _ Policy = &singleNumaNodePolicy{} @@ -28,15 +31,15 @@ var _ Policy = &singleNumaNodePolicy{} const PolicySingleNumaNode string = "single-numa-node" // NewSingleNumaNodePolicy returns single-numa-node policy. -func NewSingleNumaNodePolicy() Policy { - return &singleNumaNodePolicy{} +func NewSingleNumaNodePolicy(numaNodes []int) Policy { + return &singleNumaNodePolicy{numaNodes: numaNodes} } func (p *singleNumaNodePolicy) Name() string { return PolicySingleNumaNode } -func (p *singleNumaNodePolicy) CanAdmitPodResult(hint *TopologyHint) lifecycle.PodAdmitResult { +func (p *singleNumaNodePolicy) canAdmitPodResult(hint *TopologyHint) lifecycle.PodAdmitResult { if !hint.Preferred { return lifecycle.PodAdmitResult{ Admit: false, @@ -48,3 +51,9 @@ func (p *singleNumaNodePolicy) CanAdmitPodResult(hint *TopologyHint) lifecycle.P Admit: true, } } + +func (p *singleNumaNodePolicy) Merge(providersHints []map[string][]TopologyHint) (TopologyHint, lifecycle.PodAdmitResult) { + hint := mergeProvidersHints(p, p.numaNodes, providersHints) + admit := p.canAdmitPodResult(&hint) + return hint, admit +} 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 44ea8370edd..a85862ba916 100644 --- a/pkg/kubelet/cm/topologymanager/policy_single_numa_node_test.go +++ b/pkg/kubelet/cm/topologymanager/policy_single_numa_node_test.go @@ -34,8 +34,9 @@ func TestPolicySingleNumaNodeCanAdmitPodResult(t *testing.T) { } for _, tc := range tcases { - policy := NewSingleNumaNodePolicy() - result := policy.CanAdmitPodResult(&tc.hint) + numaNodes := []int{0, 1} + policy := NewSingleNumaNodePolicy(numaNodes) + result := policy.(*singleNumaNodePolicy).canAdmitPodResult(&tc.hint) if result.Admit != tc.expected { t.Errorf("Expected Admit field in result to be %t, got %t", tc.expected, result.Admit) @@ -51,3 +52,13 @@ func TestPolicySingleNumaNodeCanAdmitPodResult(t *testing.T) { } } } + +func TestSingleNumaNodePolicyMerge(t *testing.T) { + numaNodes := []int{0, 1} + policy := NewSingleNumaNodePolicy(numaNodes) + + tcases := commonPolicyMergeTestCases(numaNodes) + tcases = append(tcases, policy.(*singleNumaNodePolicy).mergeTestCases(numaNodes)...) + + testPolicyMerge(policy, tcases, t) +} diff --git a/pkg/kubelet/cm/topologymanager/policy_test.go b/pkg/kubelet/cm/topologymanager/policy_test.go new file mode 100644 index 00000000000..5e8a61b6394 --- /dev/null +++ b/pkg/kubelet/cm/topologymanager/policy_test.go @@ -0,0 +1,668 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package topologymanager + +import ( + "testing" + + "k8s.io/api/core/v1" +) + +type policyMergeTestCase struct { + name string + hp []HintProvider + expected TopologyHint +} + +func commonPolicyMergeTestCases(numaNodes []int) []policyMergeTestCase { + return []policyMergeTestCase{ + { + name: "TopologyHint not set", + hp: []HintProvider{}, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(numaNodes...), + Preferred: true, + }, + }, + { + name: "HintProvider returns empty non-nil map[string][]TopologyHint", + hp: []HintProvider{ + &mockHintProvider{ + map[string][]TopologyHint{}, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(numaNodes...), + Preferred: true, + }, + }, + { + name: "HintProvider returns -nil map[string][]TopologyHint from provider", + hp: []HintProvider{ + &mockHintProvider{ + map[string][]TopologyHint{ + "resource": nil, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(numaNodes...), + Preferred: true, + }, + }, + { + name: "HintProvider returns empty non-nil map[string][]TopologyHint from provider", + hp: []HintProvider{ + &mockHintProvider{ + map[string][]TopologyHint{ + "resource": {}, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(numaNodes...), + Preferred: false, + }, + }, + { + name: "Single TopologyHint with Preferred as true and NUMANodeAffinity as nil", + hp: []HintProvider{ + &mockHintProvider{ + map[string][]TopologyHint{ + "resource": { + { + NUMANodeAffinity: nil, + Preferred: true, + }, + }, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(numaNodes...), + Preferred: true, + }, + }, + { + name: "Single TopologyHint with Preferred as false and NUMANodeAffinity as nil", + hp: []HintProvider{ + &mockHintProvider{ + map[string][]TopologyHint{ + "resource": { + { + NUMANodeAffinity: nil, + Preferred: false, + }, + }, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(numaNodes...), + Preferred: false, + }, + }, + { + name: "Two providers, 1 hint each, same mask, both preferred 1/2", + hp: []HintProvider{ + &mockHintProvider{ + map[string][]TopologyHint{ + "resource1": { + { + NUMANodeAffinity: NewTestBitMask(0), + Preferred: true, + }, + }, + }, + }, + &mockHintProvider{ + map[string][]TopologyHint{ + "resource2": { + { + NUMANodeAffinity: NewTestBitMask(0), + Preferred: true, + }, + }, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(0), + Preferred: true, + }, + }, + { + name: "Two providers, 1 hint each, same mask, both preferred 2/2", + hp: []HintProvider{ + &mockHintProvider{ + map[string][]TopologyHint{ + "resource1": { + { + NUMANodeAffinity: NewTestBitMask(1), + Preferred: true, + }, + }, + }, + }, + &mockHintProvider{ + map[string][]TopologyHint{ + "resource2": { + { + NUMANodeAffinity: NewTestBitMask(1), + Preferred: true, + }, + }, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(1), + Preferred: true, + }, + }, + { + name: "Two providers, 1 hint each, no common mask", + hp: []HintProvider{ + &mockHintProvider{ + map[string][]TopologyHint{ + "resource1": { + { + NUMANodeAffinity: NewTestBitMask(0), + Preferred: true, + }, + }, + }, + }, + &mockHintProvider{ + map[string][]TopologyHint{ + "resource2": { + { + NUMANodeAffinity: NewTestBitMask(1), + Preferred: true, + }, + }, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(numaNodes...), + Preferred: false, + }, + }, + { + name: "Two providers, 1 hint each, same mask, 1 preferred, 1 not 1/2", + hp: []HintProvider{ + &mockHintProvider{ + map[string][]TopologyHint{ + "resource1": { + { + NUMANodeAffinity: NewTestBitMask(0), + Preferred: true, + }, + }, + }, + }, + &mockHintProvider{ + map[string][]TopologyHint{ + "resource2": { + { + NUMANodeAffinity: NewTestBitMask(0), + Preferred: false, + }, + }, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(0), + Preferred: false, + }, + }, + { + name: "Two providers, 1 hint each, same mask, 1 preferred, 1 not 2/2", + hp: []HintProvider{ + &mockHintProvider{ + map[string][]TopologyHint{ + "resource1": { + { + NUMANodeAffinity: NewTestBitMask(1), + Preferred: true, + }, + }, + }, + }, + &mockHintProvider{ + map[string][]TopologyHint{ + "resource2": { + { + NUMANodeAffinity: NewTestBitMask(1), + Preferred: false, + }, + }, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(1), + Preferred: false, + }, + }, + { + name: "Two providers, 1 no hints, 1 single hint preferred 1/2", + hp: []HintProvider{ + &mockHintProvider{}, + &mockHintProvider{ + map[string][]TopologyHint{ + "resource": { + { + NUMANodeAffinity: NewTestBitMask(0), + Preferred: true, + }, + }, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(0), + Preferred: true, + }, + }, + { + name: "Two providers, 1 no hints, 1 single hint preferred 2/2", + hp: []HintProvider{ + &mockHintProvider{}, + &mockHintProvider{ + map[string][]TopologyHint{ + "resource": { + { + NUMANodeAffinity: NewTestBitMask(1), + Preferred: true, + }, + }, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(1), + Preferred: true, + }, + }, + { + name: "Two providers, 1 with 2 hints, 1 with single hint matching 1/2", + hp: []HintProvider{ + &mockHintProvider{ + map[string][]TopologyHint{ + "resource1": { + { + NUMANodeAffinity: NewTestBitMask(0), + Preferred: true, + }, + { + NUMANodeAffinity: NewTestBitMask(1), + Preferred: true, + }, + }, + }, + }, + &mockHintProvider{ + map[string][]TopologyHint{ + "resource2": { + { + NUMANodeAffinity: NewTestBitMask(0), + Preferred: true, + }, + }, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(0), + Preferred: true, + }, + }, + { + name: "Two providers, 1 with 2 hints, 1 with single hint matching 2/2", + hp: []HintProvider{ + &mockHintProvider{ + map[string][]TopologyHint{ + "resource1": { + { + NUMANodeAffinity: NewTestBitMask(0), + Preferred: true, + }, + { + NUMANodeAffinity: NewTestBitMask(1), + Preferred: true, + }, + }, + }, + }, + &mockHintProvider{ + map[string][]TopologyHint{ + "resource2": { + { + NUMANodeAffinity: NewTestBitMask(1), + Preferred: true, + }, + }, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(1), + Preferred: true, + }, + }, + { + name: "Two providers, 1 with 2 hints, 1 with single non-preferred hint matching", + hp: []HintProvider{ + &mockHintProvider{ + map[string][]TopologyHint{ + "resource1": { + { + NUMANodeAffinity: NewTestBitMask(0), + Preferred: true, + }, + { + NUMANodeAffinity: NewTestBitMask(1), + Preferred: true, + }, + }, + }, + }, + &mockHintProvider{ + map[string][]TopologyHint{ + "resource2": { + { + NUMANodeAffinity: NewTestBitMask(0, 1), + Preferred: false, + }, + }, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(0), + Preferred: false, + }, + }, + { + name: "Two providers, both with 2 hints, matching narrower preferred hint from both", + hp: []HintProvider{ + &mockHintProvider{ + map[string][]TopologyHint{ + "resource1": { + { + NUMANodeAffinity: NewTestBitMask(0), + Preferred: true, + }, + { + NUMANodeAffinity: NewTestBitMask(1), + Preferred: true, + }, + }, + }, + }, + &mockHintProvider{ + map[string][]TopologyHint{ + "resource2": { + { + NUMANodeAffinity: NewTestBitMask(0), + Preferred: true, + }, + { + NUMANodeAffinity: NewTestBitMask(0, 1), + Preferred: false, + }, + }, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(0), + Preferred: true, + }, + }, + { + name: "Ensure less narrow preferred hints are chosen over narrower non-preferred hints", + hp: []HintProvider{ + &mockHintProvider{ + map[string][]TopologyHint{ + "resource1": { + { + NUMANodeAffinity: NewTestBitMask(1), + Preferred: true, + }, + { + NUMANodeAffinity: NewTestBitMask(0, 1), + Preferred: false, + }, + }, + }, + }, + &mockHintProvider{ + map[string][]TopologyHint{ + "resource2": { + { + NUMANodeAffinity: NewTestBitMask(0), + Preferred: true, + }, + { + NUMANodeAffinity: NewTestBitMask(1), + Preferred: true, + }, + { + NUMANodeAffinity: NewTestBitMask(0, 1), + Preferred: false, + }, + }, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(1), + Preferred: true, + }, + }, + { + name: "Multiple resources, same provider", + hp: []HintProvider{ + &mockHintProvider{ + map[string][]TopologyHint{ + "resource1": { + { + NUMANodeAffinity: NewTestBitMask(1), + Preferred: true, + }, + { + NUMANodeAffinity: NewTestBitMask(0, 1), + Preferred: false, + }, + }, + "resource2": { + { + NUMANodeAffinity: NewTestBitMask(0), + Preferred: true, + }, + { + NUMANodeAffinity: NewTestBitMask(1), + Preferred: true, + }, + { + NUMANodeAffinity: NewTestBitMask(0, 1), + Preferred: false, + }, + }, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(1), + Preferred: true, + }, + }, + } +} + +func (p *bestEffortPolicy) mergeTestCases(numaNodes []int) []policyMergeTestCase { + return []policyMergeTestCase{ + { + name: "Two providers, 1 hint each, 1 wider mask, both preferred 1/2", + hp: []HintProvider{ + &mockHintProvider{ + map[string][]TopologyHint{ + "resource1": { + { + NUMANodeAffinity: NewTestBitMask(0), + Preferred: true, + }, + }, + }, + }, + &mockHintProvider{ + map[string][]TopologyHint{ + "resource2": { + { + NUMANodeAffinity: NewTestBitMask(0, 1), + Preferred: true, + }, + }, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(0), + Preferred: true, + }, + }, + { + name: "Two providers, 1 hint each, 1 wider mask, both preferred 1/2", + hp: []HintProvider{ + &mockHintProvider{ + map[string][]TopologyHint{ + "resource1": { + { + NUMANodeAffinity: NewTestBitMask(1), + Preferred: true, + }, + }, + }, + }, + &mockHintProvider{ + map[string][]TopologyHint{ + "resource2": { + { + NUMANodeAffinity: NewTestBitMask(0, 1), + Preferred: true, + }, + }, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(1), + Preferred: true, + }, + }, + } +} + +func (p *singleNumaNodePolicy) mergeTestCases(numaNodes []int) []policyMergeTestCase { + return []policyMergeTestCase{ + { + name: "Single NUMA hint generation", + hp: []HintProvider{ + &mockHintProvider{ + map[string][]TopologyHint{ + "resource1": { + { + NUMANodeAffinity: NewTestBitMask(0, 1), + Preferred: true, + }, + }, + "resource2": { + { + NUMANodeAffinity: NewTestBitMask(0), + Preferred: true, + }, + { + NUMANodeAffinity: NewTestBitMask(1), + Preferred: true, + }, + { + NUMANodeAffinity: NewTestBitMask(0, 1), + Preferred: false, + }, + }, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(0), + Preferred: false, + }, + }, + { + name: "One no-preference provider", + hp: []HintProvider{ + &mockHintProvider{ + map[string][]TopologyHint{ + "resource1": { + { + NUMANodeAffinity: NewTestBitMask(0), + Preferred: true, + }, + { + NUMANodeAffinity: NewTestBitMask(1), + Preferred: true, + }, + { + NUMANodeAffinity: NewTestBitMask(0, 1), + Preferred: false, + }, + }, + }, + }, + &mockHintProvider{ + nil, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(0), + Preferred: true, + }, + }, + } +} + +func testPolicyMerge(policy Policy, tcases []policyMergeTestCase, t *testing.T) { + for _, tc := range tcases { + var providersHints []map[string][]TopologyHint + for _, provider := range tc.hp { + hints := provider.GetTopologyHints(v1.Pod{}, v1.Container{}) + providersHints = append(providersHints, hints) + } + + actual, _ := policy.Merge(providersHints) + if !actual.NUMANodeAffinity.IsEqual(tc.expected.NUMANodeAffinity) { + t.Errorf("%v: Expected NUMANodeAffinity in result to be %v, got %v", tc.name, tc.expected.NUMANodeAffinity, actual.NUMANodeAffinity) + } + if actual.Preferred != tc.expected.Preferred { + t.Errorf("%v: Expected Affinity preference in result to be %v, got %v", tc.name, tc.expected.Preferred, actual.Preferred) + } + } +} diff --git a/pkg/kubelet/cm/topologymanager/topology_manager.go b/pkg/kubelet/cm/topologymanager/topology_manager.go index e8cb592fac5..4e452d71a00 100644 --- a/pkg/kubelet/cm/topologymanager/topology_manager.go +++ b/pkg/kubelet/cm/topologymanager/topology_manager.go @@ -63,8 +63,6 @@ type manager struct { podMap map[string]string //Topology Manager Policy policy Policy - //List of NUMA Nodes available on the underlying machine - numaNodes []int } // HintProvider is an interface for components that want to collaborate to @@ -94,30 +92,32 @@ type TopologyHint struct { Preferred bool } +// IsEqual checks if TopologyHint are equal +func (th *TopologyHint) IsEqual(topologyHint TopologyHint) bool { + if th.Preferred == topologyHint.Preferred { + if th.NUMANodeAffinity == nil || topologyHint.NUMANodeAffinity == nil { + return th.NUMANodeAffinity == topologyHint.NUMANodeAffinity + } + return th.NUMANodeAffinity.IsEqual(topologyHint.NUMANodeAffinity) + } + return false +} + +// LessThan checks if TopologyHint `a` is less than TopologyHint `b` +// this means that either `a` is a preferred hint and `b` is not +// or `a` NUMANodeAffinity attribute is narrower than `b` NUMANodeAffinity attribute. +func (th *TopologyHint) LessThan(other TopologyHint) bool { + if th.Preferred != other.Preferred { + return th.Preferred == true + } + return th.NUMANodeAffinity.IsNarrowerThan(other.NUMANodeAffinity) +} + var _ Manager = &manager{} //NewManager creates a new TopologyManager based on provided policy func NewManager(numaNodeInfo cputopology.NUMANodeInfo, topologyPolicyName string) (Manager, error) { klog.Infof("[topologymanager] Creating topology manager with %s policy", topologyPolicyName) - var policy Policy - - switch topologyPolicyName { - - case PolicyNone: - policy = NewNonePolicy() - - case PolicyBestEffort: - policy = NewBestEffortPolicy() - - case PolicyRestricted: - policy = NewRestrictedPolicy() - - case PolicySingleNumaNode: - policy = NewSingleNumaNodePolicy() - - default: - return nil, fmt.Errorf("unknown policy: \"%s\"", topologyPolicyName) - } var numaNodes []int for node := range numaNodeInfo { @@ -128,6 +128,25 @@ func NewManager(numaNodeInfo cputopology.NUMANodeInfo, topologyPolicyName string return nil, fmt.Errorf("unsupported on machines with more than %v NUMA Nodes", maxAllowableNUMANodes) } + var policy Policy + switch topologyPolicyName { + + case PolicyNone: + policy = NewNonePolicy() + + case PolicyBestEffort: + policy = NewBestEffortPolicy(numaNodes) + + case PolicyRestricted: + policy = NewRestrictedPolicy(numaNodes) + + case PolicySingleNumaNode: + policy = NewSingleNumaNodePolicy(numaNodes) + + default: + return nil, fmt.Errorf("unknown policy: \"%s\"", topologyPolicyName) + } + var hp []HintProvider pth := make(map[string]map[string]TopologyHint) pm := make(map[string]string) @@ -136,7 +155,6 @@ func NewManager(numaNodeInfo cputopology.NUMANodeInfo, topologyPolicyName string podTopologyHints: pth, podMap: pm, policy: policy, - numaNodes: numaNodes, } return manager, nil @@ -146,154 +164,24 @@ func (m *manager) GetAffinity(podUID string, containerName string) TopologyHint return m.podTopologyHints[podUID][containerName] } -// Iterate over all permutations of hints in 'allProviderHints [][]TopologyHint'. -// -// This procedure is implemented as a recursive function over the set of hints -// in 'allproviderHints[i]'. It applies the function 'callback' to each -// permutation as it is found. It is the equivalent of: -// -// for i := 0; i < len(providerHints[0]); i++ -// for j := 0; j < len(providerHints[1]); j++ -// for k := 0; k < len(providerHints[2]); k++ -// ... -// for z := 0; z < len(providerHints[-1]); z++ -// permutation := []TopologyHint{ -// providerHints[0][i], -// providerHints[1][j], -// providerHints[2][k], -// ... -// provideryHints[-1][z] -// } -// callback(permutation) -func (m *manager) iterateAllProviderTopologyHints(allProviderHints [][]TopologyHint, callback func([]TopologyHint)) { - // Internal helper function to accumulate the permutation before calling the callback. - var iterate func(i int, accum []TopologyHint) - iterate = func(i int, accum []TopologyHint) { - // Base case: we have looped through all providers and have a full permutation. - if i == len(allProviderHints) { - callback(accum) - return - } - - // Loop through all hints for provider 'i', and recurse to build the - // the permutation of this hint with all hints from providers 'i++'. - for j := range allProviderHints[i] { - iterate(i+1, append(accum, allProviderHints[i][j])) - } - } - iterate(0, []TopologyHint{}) -} - -// Merge the hints from all hint providers to find the best one. -func (m *manager) calculateAffinity(pod v1.Pod, container v1.Container) TopologyHint { - // Set the default affinity as an any-numa affinity containing the list - // of NUMA Nodes available on this machine. - defaultAffinity, _ := bitmask.NewBitMask(m.numaNodes...) - +func (m *manager) accumulateProvidersHints(pod v1.Pod, container v1.Container) (providersHints []map[string][]TopologyHint) { // Loop through all hint providers and save an accumulated list of the - // hints returned by each hint provider. If no hints are provided, assume - // that provider has no preference for topology-aware allocation. - var allProviderHints [][]TopologyHint + // hints returned by each hint provider. for _, provider := range m.hintProviders { // Get the TopologyHints from a provider. hints := provider.GetTopologyHints(pod, container) - - // If hints is nil, insert a single, preferred any-numa hint into allProviderHints. - if len(hints) == 0 { - klog.Infof("[topologymanager] Hint Provider has no preference for NUMA affinity with any resource") - allProviderHints = append(allProviderHints, []TopologyHint{{nil, true}}) - continue - } - - // Otherwise, accumulate the hints for each resource type into allProviderHints. - for resource := range hints { - if hints[resource] == nil { - klog.Infof("[topologymanager] Hint Provider has no preference for NUMA affinity with resource '%s'", resource) - allProviderHints = append(allProviderHints, []TopologyHint{{nil, true}}) - continue - } - - if len(hints[resource]) == 0 { - klog.Infof("[topologymanager] Hint Provider has no possible NUMA affinities for resource '%s'", resource) - allProviderHints = append(allProviderHints, []TopologyHint{{nil, false}}) - continue - } - - allProviderHints = append(allProviderHints, hints[resource]) - } + providersHints = append(providersHints, hints) + klog.Infof("[topologymanager] TopologyHints for pod '%v', container '%v': %v", pod.Name, container.Name, hints) } + return providersHints +} - // Iterate over all permutations of hints in 'allProviderHints'. Merge the - // hints in each permutation by taking the bitwise-and of their affinity masks. - // Return the hint with the narrowest NUMANodeAffinity of all merged - // permutations that have at least one NUMA ID set. If no merged mask can be - // found that has at least one NUMA ID set, return the 'defaultAffinity'. - bestHint := TopologyHint{defaultAffinity, false} - m.iterateAllProviderTopologyHints(allProviderHints, func(permutation []TopologyHint) { - // Get the NUMANodeAffinity from each hint in the permutation and see if any - // of them encode unpreferred allocations. - preferred := true - var numaAffinities []bitmask.BitMask - for _, hint := range permutation { - if hint.NUMANodeAffinity == nil { - numaAffinities = append(numaAffinities, defaultAffinity) - } else { - numaAffinities = append(numaAffinities, hint.NUMANodeAffinity) - } - - if !hint.Preferred { - preferred = false - } - - // Special case PolicySingleNumaNode to only prefer hints where - // all providers have a single NUMA affinity set. - if m.policy != nil && m.policy.Name() == PolicySingleNumaNode && hint.NUMANodeAffinity != nil && hint.NUMANodeAffinity.Count() > 1 { - preferred = false - } - } - - // Merge the affinities using a bitwise-and operation. - mergedAffinity, _ := bitmask.NewBitMask(m.numaNodes...) - mergedAffinity.And(numaAffinities...) - - // Build a mergedHintfrom the merged affinity mask, indicating if an - // preferred allocation was used to generate the affinity mask or not. - mergedHint := TopologyHint{mergedAffinity, preferred} - - // Only consider mergedHints that result in a NUMANodeAffinity > 0 to - // replace the current bestHint. - if mergedHint.NUMANodeAffinity.Count() == 0 { - return - } - - // If the current bestHint is non-preferred and the new mergedHint is - // preferred, always choose the preferred hint over the non-preferred one. - if mergedHint.Preferred && !bestHint.Preferred { - bestHint = mergedHint - return - } - - // If the current bestHint is preferred and the new mergedHint is - // non-preferred, never update bestHint, regardless of mergedHint's - // narowness. - if !mergedHint.Preferred && bestHint.Preferred { - return - } - - // If mergedHint and bestHint has the same preference, only consider - // mergedHints that have a narrower NUMANodeAffinity than the - // NUMANodeAffinity in the current bestHint. - if !mergedHint.NUMANodeAffinity.IsNarrowerThan(bestHint.NUMANodeAffinity) { - return - } - - // In all other cases, update bestHint to the current mergedHint - bestHint = mergedHint - }) - +// Collect Hints from hint providers and pass to policy to retrieve the best one. +func (m *manager) calculateAffinity(pod v1.Pod, container v1.Container) (TopologyHint, lifecycle.PodAdmitResult) { + providersHints := m.accumulateProvidersHints(pod, container) + bestHint, admit := m.policy.Merge(providersHints) klog.Infof("[topologymanager] ContainerTopologyHint: %v", bestHint) - - return bestHint + return bestHint, admit } func (m *manager) AddHintProvider(h HintProvider) { @@ -325,8 +213,7 @@ func (m *manager) Admit(attrs *lifecycle.PodAdmitAttributes) lifecycle.PodAdmitR c := make(map[string]TopologyHint) for _, container := range append(pod.Spec.InitContainers, pod.Spec.Containers...) { - result := m.calculateAffinity(*pod, container) - admitPod := m.policy.CanAdmitPodResult(&result) + result, admitPod := m.calculateAffinity(*pod, container) if !admitPod.Admit { return admitPod } diff --git a/pkg/kubelet/cm/topologymanager/topology_manager_test.go b/pkg/kubelet/cm/topologymanager/topology_manager_test.go index 50c122cf460..9fc760413da 100644 --- a/pkg/kubelet/cm/topologymanager/topology_manager_test.go +++ b/pkg/kubelet/cm/topologymanager/topology_manager_test.go @@ -104,641 +104,6 @@ func TestGetAffinity(t *testing.T) { } } -func TestCalculateAffinity(t *testing.T) { - numaNodes := []int{0, 1} - - tcases := []struct { - name string - hp []HintProvider - expected TopologyHint - policy Policy - }{ - { - name: "TopologyHint not set", - hp: []HintProvider{}, - expected: TopologyHint{ - NUMANodeAffinity: NewTestBitMask(numaNodes...), - Preferred: true, - }, - }, - { - name: "HintProvider returns empty non-nil map[string][]TopologyHint", - hp: []HintProvider{ - &mockHintProvider{ - map[string][]TopologyHint{}, - }, - }, - expected: TopologyHint{ - NUMANodeAffinity: NewTestBitMask(numaNodes...), - Preferred: true, - }, - }, - { - name: "HintProvider returns -nil map[string][]TopologyHint from provider", - hp: []HintProvider{ - &mockHintProvider{ - map[string][]TopologyHint{ - "resource": nil, - }, - }, - }, - expected: TopologyHint{ - NUMANodeAffinity: NewTestBitMask(numaNodes...), - Preferred: true, - }, - }, - { - name: "HintProvider returns empty non-nil map[string][]TopologyHint from provider", - hp: []HintProvider{ - &mockHintProvider{ - map[string][]TopologyHint{ - "resource": {}, - }, - }, - }, - expected: TopologyHint{ - NUMANodeAffinity: NewTestBitMask(numaNodes...), - Preferred: false, - }, - }, - { - name: "Single TopologyHint with Preferred as true and NUMANodeAffinity as nil", - hp: []HintProvider{ - &mockHintProvider{ - map[string][]TopologyHint{ - "resource": { - { - NUMANodeAffinity: nil, - Preferred: true, - }, - }, - }, - }, - }, - expected: TopologyHint{ - NUMANodeAffinity: NewTestBitMask(numaNodes...), - Preferred: true, - }, - }, - { - name: "Single TopologyHint with Preferred as false and NUMANodeAffinity as nil", - hp: []HintProvider{ - &mockHintProvider{ - map[string][]TopologyHint{ - "resource": { - { - NUMANodeAffinity: nil, - Preferred: false, - }, - }, - }, - }, - }, - expected: TopologyHint{ - NUMANodeAffinity: NewTestBitMask(numaNodes...), - Preferred: false, - }, - }, - { - name: "Two providers, 1 hint each, same mask, both preferred 1/2", - hp: []HintProvider{ - &mockHintProvider{ - map[string][]TopologyHint{ - "resource1": { - { - NUMANodeAffinity: NewTestBitMask(0), - Preferred: true, - }, - }, - }, - }, - &mockHintProvider{ - map[string][]TopologyHint{ - "resource2": { - { - NUMANodeAffinity: NewTestBitMask(0), - Preferred: true, - }, - }, - }, - }, - }, - expected: TopologyHint{ - NUMANodeAffinity: NewTestBitMask(0), - Preferred: true, - }, - }, - { - name: "Two providers, 1 hint each, same mask, both preferred 2/2", - hp: []HintProvider{ - &mockHintProvider{ - map[string][]TopologyHint{ - "resource1": { - { - NUMANodeAffinity: NewTestBitMask(1), - Preferred: true, - }, - }, - }, - }, - &mockHintProvider{ - map[string][]TopologyHint{ - "resource2": { - { - NUMANodeAffinity: NewTestBitMask(1), - Preferred: true, - }, - }, - }, - }, - }, - expected: TopologyHint{ - NUMANodeAffinity: NewTestBitMask(1), - Preferred: true, - }, - }, - { - name: "Two providers, 1 hint each, 1 wider mask, both preferred 1/2", - hp: []HintProvider{ - &mockHintProvider{ - map[string][]TopologyHint{ - "resource1": { - { - NUMANodeAffinity: NewTestBitMask(0), - Preferred: true, - }, - }, - }, - }, - &mockHintProvider{ - map[string][]TopologyHint{ - "resource2": { - { - NUMANodeAffinity: NewTestBitMask(0, 1), - Preferred: true, - }, - }, - }, - }, - }, - expected: TopologyHint{ - NUMANodeAffinity: NewTestBitMask(0), - Preferred: true, - }, - }, - { - name: "Two providers, 1 hint each, 1 wider mask, both preferred 1/2", - hp: []HintProvider{ - &mockHintProvider{ - map[string][]TopologyHint{ - "resource1": { - { - NUMANodeAffinity: NewTestBitMask(1), - Preferred: true, - }, - }, - }, - }, - &mockHintProvider{ - map[string][]TopologyHint{ - "resource2": { - { - NUMANodeAffinity: NewTestBitMask(0, 1), - Preferred: true, - }, - }, - }, - }, - }, - expected: TopologyHint{ - NUMANodeAffinity: NewTestBitMask(1), - Preferred: true, - }, - }, - { - name: "Two providers, 1 hint each, no common mask", - hp: []HintProvider{ - &mockHintProvider{ - map[string][]TopologyHint{ - "resource1": { - { - NUMANodeAffinity: NewTestBitMask(0), - Preferred: true, - }, - }, - }, - }, - &mockHintProvider{ - map[string][]TopologyHint{ - "resource2": { - { - NUMANodeAffinity: NewTestBitMask(1), - Preferred: true, - }, - }, - }, - }, - }, - expected: TopologyHint{ - NUMANodeAffinity: NewTestBitMask(numaNodes...), - Preferred: false, - }, - }, - { - name: "Two providers, 1 hint each, same mask, 1 preferred, 1 not 1/2", - hp: []HintProvider{ - &mockHintProvider{ - map[string][]TopologyHint{ - "resource1": { - { - NUMANodeAffinity: NewTestBitMask(0), - Preferred: true, - }, - }, - }, - }, - &mockHintProvider{ - map[string][]TopologyHint{ - "resource2": { - { - NUMANodeAffinity: NewTestBitMask(0), - Preferred: false, - }, - }, - }, - }, - }, - expected: TopologyHint{ - NUMANodeAffinity: NewTestBitMask(0), - Preferred: false, - }, - }, - { - name: "Two providers, 1 hint each, same mask, 1 preferred, 1 not 2/2", - hp: []HintProvider{ - &mockHintProvider{ - map[string][]TopologyHint{ - "resource1": { - { - NUMANodeAffinity: NewTestBitMask(1), - Preferred: true, - }, - }, - }, - }, - &mockHintProvider{ - map[string][]TopologyHint{ - "resource2": { - { - NUMANodeAffinity: NewTestBitMask(1), - Preferred: false, - }, - }, - }, - }, - }, - expected: TopologyHint{ - NUMANodeAffinity: NewTestBitMask(1), - Preferred: false, - }, - }, - { - name: "Two providers, 1 no hints, 1 single hint preferred 1/2", - hp: []HintProvider{ - &mockHintProvider{}, - &mockHintProvider{ - map[string][]TopologyHint{ - "resource": { - { - NUMANodeAffinity: NewTestBitMask(0), - Preferred: true, - }, - }, - }, - }, - }, - expected: TopologyHint{ - NUMANodeAffinity: NewTestBitMask(0), - Preferred: true, - }, - }, - { - name: "Two providers, 1 no hints, 1 single hint preferred 2/2", - hp: []HintProvider{ - &mockHintProvider{}, - &mockHintProvider{ - map[string][]TopologyHint{ - "resource": { - { - NUMANodeAffinity: NewTestBitMask(1), - Preferred: true, - }, - }, - }, - }, - }, - expected: TopologyHint{ - NUMANodeAffinity: NewTestBitMask(1), - Preferred: true, - }, - }, - { - name: "Two providers, 1 with 2 hints, 1 with single hint matching 1/2", - hp: []HintProvider{ - &mockHintProvider{ - map[string][]TopologyHint{ - "resource1": { - { - NUMANodeAffinity: NewTestBitMask(0), - Preferred: true, - }, - { - NUMANodeAffinity: NewTestBitMask(1), - Preferred: true, - }, - }, - }, - }, - &mockHintProvider{ - map[string][]TopologyHint{ - "resource2": { - { - NUMANodeAffinity: NewTestBitMask(0), - Preferred: true, - }, - }, - }, - }, - }, - expected: TopologyHint{ - NUMANodeAffinity: NewTestBitMask(0), - Preferred: true, - }, - }, - { - name: "Two providers, 1 with 2 hints, 1 with single hint matching 2/2", - hp: []HintProvider{ - &mockHintProvider{ - map[string][]TopologyHint{ - "resource1": { - { - NUMANodeAffinity: NewTestBitMask(0), - Preferred: true, - }, - { - NUMANodeAffinity: NewTestBitMask(1), - Preferred: true, - }, - }, - }, - }, - &mockHintProvider{ - map[string][]TopologyHint{ - "resource2": { - { - NUMANodeAffinity: NewTestBitMask(1), - Preferred: true, - }, - }, - }, - }, - }, - expected: TopologyHint{ - NUMANodeAffinity: NewTestBitMask(1), - Preferred: true, - }, - }, - { - name: "Two providers, 1 with 2 hints, 1 with single non-preferred hint matching", - hp: []HintProvider{ - &mockHintProvider{ - map[string][]TopologyHint{ - "resource1": { - { - NUMANodeAffinity: NewTestBitMask(0), - Preferred: true, - }, - { - NUMANodeAffinity: NewTestBitMask(1), - Preferred: true, - }, - }, - }, - }, - &mockHintProvider{ - map[string][]TopologyHint{ - "resource2": { - { - NUMANodeAffinity: NewTestBitMask(0, 1), - Preferred: false, - }, - }, - }, - }, - }, - expected: TopologyHint{ - NUMANodeAffinity: NewTestBitMask(0), - Preferred: false, - }, - }, - { - name: "Two providers, both with 2 hints, matching narrower preferred hint from both", - hp: []HintProvider{ - &mockHintProvider{ - map[string][]TopologyHint{ - "resource1": { - { - NUMANodeAffinity: NewTestBitMask(0), - Preferred: true, - }, - { - NUMANodeAffinity: NewTestBitMask(1), - Preferred: true, - }, - }, - }, - }, - &mockHintProvider{ - map[string][]TopologyHint{ - "resource2": { - { - NUMANodeAffinity: NewTestBitMask(0), - Preferred: true, - }, - { - NUMANodeAffinity: NewTestBitMask(0, 1), - Preferred: false, - }, - }, - }, - }, - }, - expected: TopologyHint{ - NUMANodeAffinity: NewTestBitMask(0), - Preferred: true, - }, - }, - { - name: "Ensure less narrow preferred hints are chosen over narrower non-preferred hints", - hp: []HintProvider{ - &mockHintProvider{ - map[string][]TopologyHint{ - "resource1": { - { - NUMANodeAffinity: NewTestBitMask(1), - Preferred: true, - }, - { - NUMANodeAffinity: NewTestBitMask(0, 1), - Preferred: false, - }, - }, - }, - }, - &mockHintProvider{ - map[string][]TopologyHint{ - "resource2": { - { - NUMANodeAffinity: NewTestBitMask(0), - Preferred: true, - }, - { - NUMANodeAffinity: NewTestBitMask(1), - Preferred: true, - }, - { - NUMANodeAffinity: NewTestBitMask(0, 1), - Preferred: false, - }, - }, - }, - }, - }, - expected: TopologyHint{ - NUMANodeAffinity: NewTestBitMask(1), - Preferred: true, - }, - }, - { - name: "Multiple resources, same provider", - hp: []HintProvider{ - &mockHintProvider{ - map[string][]TopologyHint{ - "resource1": { - { - NUMANodeAffinity: NewTestBitMask(1), - Preferred: true, - }, - { - NUMANodeAffinity: NewTestBitMask(0, 1), - Preferred: false, - }, - }, - "resource2": { - { - NUMANodeAffinity: NewTestBitMask(0), - Preferred: true, - }, - { - NUMANodeAffinity: NewTestBitMask(1), - Preferred: true, - }, - { - NUMANodeAffinity: NewTestBitMask(0, 1), - Preferred: false, - }, - }, - }, - }, - }, - expected: TopologyHint{ - NUMANodeAffinity: NewTestBitMask(1), - Preferred: true, - }, - }, - { - name: "Special cased PolicySingleNumaNode for single NUMA hint generation", - policy: NewSingleNumaNodePolicy(), - hp: []HintProvider{ - &mockHintProvider{ - map[string][]TopologyHint{ - "resource1": { - { - NUMANodeAffinity: NewTestBitMask(0, 1), - Preferred: true, - }, - }, - "resource2": { - { - NUMANodeAffinity: NewTestBitMask(0), - Preferred: true, - }, - { - NUMANodeAffinity: NewTestBitMask(1), - Preferred: true, - }, - { - NUMANodeAffinity: NewTestBitMask(0, 1), - Preferred: false, - }, - }, - }, - }, - }, - expected: TopologyHint{ - NUMANodeAffinity: NewTestBitMask(0), - Preferred: false, - }, - }, - { - name: "Special cased PolicySingleNumaNode with one no-preference provider", - policy: NewSingleNumaNodePolicy(), - hp: []HintProvider{ - &mockHintProvider{ - map[string][]TopologyHint{ - "resource1": { - { - NUMANodeAffinity: NewTestBitMask(0), - Preferred: true, - }, - { - NUMANodeAffinity: NewTestBitMask(1), - Preferred: true, - }, - { - NUMANodeAffinity: NewTestBitMask(0, 1), - Preferred: false, - }, - }, - }, - }, - &mockHintProvider{ - nil, - }, - }, - expected: TopologyHint{ - NUMANodeAffinity: NewTestBitMask(0), - Preferred: true, - }, - }, - } - - for _, tc := range tcases { - mngr := manager{ - policy: tc.policy, - hintProviders: tc.hp, - numaNodes: numaNodes, - } - actual := mngr.calculateAffinity(v1.Pod{}, v1.Container{}) - if !actual.NUMANodeAffinity.IsEqual(tc.expected.NUMANodeAffinity) { - t.Errorf("Expected NUMANodeAffinity in result to be %v, got %v", tc.expected.NUMANodeAffinity, actual.NUMANodeAffinity) - } - if actual.Preferred != tc.expected.Preferred { - t.Errorf("Expected Affinity preference in result to be %v, got %v", tc.expected.Preferred, actual.Preferred) - } - } -} - func TestAddContainer(t *testing.T) { testCases := []struct { name string @@ -835,6 +200,8 @@ func TestAddHintProvider(t *testing.T) { } func TestAdmit(t *testing.T) { + numaNodes := []int{0, 1} + tcases := []struct { name string result lifecycle.PodAdmitResult @@ -860,7 +227,7 @@ func TestAdmit(t *testing.T) { { name: "QOSClass set as BestEffort. single-numa-node Policy. No Hints.", qosClass: v1.PodQOSBestEffort, - policy: NewRestrictedPolicy(), + policy: NewRestrictedPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{}, }, @@ -869,7 +236,7 @@ func TestAdmit(t *testing.T) { { name: "QOSClass set as BestEffort. Restricted Policy. No Hints.", qosClass: v1.PodQOSBestEffort, - policy: NewRestrictedPolicy(), + policy: NewRestrictedPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{}, }, @@ -878,7 +245,7 @@ func TestAdmit(t *testing.T) { { name: "QOSClass set as Guaranteed. BestEffort Policy. Preferred Affinity.", qosClass: v1.PodQOSGuaranteed, - policy: NewBestEffortPolicy(), + policy: NewBestEffortPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{ map[string][]TopologyHint{ @@ -900,7 +267,7 @@ func TestAdmit(t *testing.T) { { name: "QOSClass set as Guaranteed. BestEffort Policy. More than one Preferred Affinity.", qosClass: v1.PodQOSGuaranteed, - policy: NewBestEffortPolicy(), + policy: NewBestEffortPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{ map[string][]TopologyHint{ @@ -926,7 +293,7 @@ func TestAdmit(t *testing.T) { { name: "QOSClass set as Burstable. BestEffort Policy. More than one Preferred Affinity.", qosClass: v1.PodQOSBurstable, - policy: NewBestEffortPolicy(), + policy: NewBestEffortPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{ map[string][]TopologyHint{ @@ -952,7 +319,7 @@ func TestAdmit(t *testing.T) { { name: "QOSClass set as Guaranteed. BestEffort Policy. No Preferred Affinity.", qosClass: v1.PodQOSGuaranteed, - policy: NewBestEffortPolicy(), + policy: NewBestEffortPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{ map[string][]TopologyHint{ @@ -970,7 +337,7 @@ func TestAdmit(t *testing.T) { { name: "QOSClass set as Guaranteed. Restricted Policy. Preferred Affinity.", qosClass: v1.PodQOSGuaranteed, - policy: NewRestrictedPolicy(), + policy: NewRestrictedPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{ map[string][]TopologyHint{ @@ -992,7 +359,7 @@ func TestAdmit(t *testing.T) { { name: "QOSClass set as Burstable. Restricted Policy. Preferred Affinity.", qosClass: v1.PodQOSBurstable, - policy: NewRestrictedPolicy(), + policy: NewRestrictedPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{ map[string][]TopologyHint{ @@ -1014,7 +381,7 @@ func TestAdmit(t *testing.T) { { name: "QOSClass set as Guaranteed. Restricted Policy. More than one Preferred affinity.", qosClass: v1.PodQOSGuaranteed, - policy: NewRestrictedPolicy(), + policy: NewRestrictedPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{ map[string][]TopologyHint{ @@ -1040,7 +407,7 @@ func TestAdmit(t *testing.T) { { name: "QOSClass set as Burstable. Restricted Policy. More than one Preferred affinity.", qosClass: v1.PodQOSBurstable, - policy: NewRestrictedPolicy(), + policy: NewRestrictedPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{ map[string][]TopologyHint{ @@ -1066,7 +433,7 @@ func TestAdmit(t *testing.T) { { name: "QOSClass set as Guaranteed. Restricted Policy. No Preferred affinity.", qosClass: v1.PodQOSGuaranteed, - policy: NewRestrictedPolicy(), + policy: NewRestrictedPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{ map[string][]TopologyHint{ @@ -1084,7 +451,7 @@ func TestAdmit(t *testing.T) { { name: "QOSClass set as Burstable. Restricted Policy. No Preferred affinity.", qosClass: v1.PodQOSBurstable, - policy: NewRestrictedPolicy(), + policy: NewRestrictedPolicy(numaNodes), hp: []HintProvider{ &mockHintProvider{ map[string][]TopologyHint{ @@ -1105,7 +472,6 @@ func TestAdmit(t *testing.T) { policy: tc.policy, podTopologyHints: make(map[string]map[string]TopologyHint), hintProviders: tc.hp, - numaNodes: []int{0, 1}, } pod := &v1.Pod{