diff --git a/pkg/kubelet/cm/topologymanager/policy.go b/pkg/kubelet/cm/topologymanager/policy.go index 231d9e3f8aa..40255ed95cc 100644 --- a/pkg/kubelet/cm/topologymanager/policy.go +++ b/pkg/kubelet/cm/topologymanager/policy.go @@ -94,51 +94,196 @@ func filterProvidersHints(providersHints []map[string][]TopologyHint) [][]Topolo return allProviderHints } -func mergeFilteredHints(numaNodes []int, filteredHints [][]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...) +func narrowestHint(hints []TopologyHint) *TopologyHint { + if len(hints) == 0 { + return nil + } + var narrowestHint *TopologyHint + for i := range hints { + if hints[i].NUMANodeAffinity == nil { + continue + } + if narrowestHint == nil { + narrowestHint = &hints[i] + } + if hints[i].NUMANodeAffinity.IsNarrowerThan(narrowestHint.NUMANodeAffinity) { + narrowestHint = &hints[i] + } + } + return narrowestHint +} - // Set the bestHint to return from this function as {nil false}. - // This will only be returned if no better hint can be found when - // merging hints from each hint provider. - bestHint := TopologyHint{defaultAffinity, false} +func maxOfMinAffinityCounts(filteredHints [][]TopologyHint) int { + maxOfMinCount := 0 + for _, resourceHints := range filteredHints { + narrowestHint := narrowestHint(resourceHints) + if narrowestHint == nil { + continue + } + if narrowestHint.NUMANodeAffinity.Count() > maxOfMinCount { + maxOfMinCount = narrowestHint.NUMANodeAffinity.Count() + } + } + return maxOfMinCount +} + +func compareHints(bestNonPreferredAffinityCount int, current *TopologyHint, candidate *TopologyHint) *TopologyHint { + // Only consider candidates that result in a NUMANodeAffinity > 0 to + // replace the current bestHint. + if candidate.NUMANodeAffinity.Count() == 0 { + return current + } + + // If no current bestHint is set, return the candidate as the bestHint. + if current == nil { + return candidate + } + + // If the current bestHint is non-preferred and the candidate hint is + // preferred, always choose the preferred hint over the non-preferred one. + if !current.Preferred && candidate.Preferred { + return candidate + } + + // If the current bestHint is preferred and the candidate hint is + // non-preferred, never update the bestHint, regardless of the + // candidate hint's narowness. + 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. + if current.Preferred && candidate.Preferred { + if candidate.NUMANodeAffinity.IsNarrowerThan(current.NUMANodeAffinity) { + return candidate + } + return current + } + + // The only case left is if the current best bestHint and the candidate + // hint are both non-preferred. In this case, try and find a hint whose + // affinity count is as close to (but not higher than) the + // bestNonPreferredAffinityCount as possible. To do this we need to + // consider the following cases and react accordingly: + // + // 1. current.NUMANodeAffinity.Count() > bestNonPreferredAffinityCount + // 2. current.NUMANodeAffinity.Count() == bestNonPreferredAffinityCount + // 3. current.NUMANodeAffinity.Count() < bestNonPreferredAffinityCount + // + // For case (1), the current bestHint is larger than the + // bestNonPreferredAffinityCount, so updating to any narrower 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. + // + // For case (3), the current bestHint is less than + // bestNonPreferredAffinityCount, so we would like to creep back up to + // bestNonPreferredAffinityCount as close as we can. There are three + // cases to consider here: + // + // 3a. candidate.NUMANodeAffinity.Count() > bestNonPreferredAffinityCount + // 3b. candidate.NUMANodeAffinity.Count() == bestNonPreferredAffinityCount + // 3c. candidate.NUMANodeAffinity.Count() < bestNonPreferredAffinityCount + // + // For case (3a), we just want to stick with the current bestHint + // because choosing a new hint that is greater than + // bestNonPreferredAffinityCount would be counter-productive. + // + // For case (3b), we want to immediately update bestHint to the + // candidate hint, making it now equal to bestNonPreferredAffinityCount. + // + // For case (3c), we know that *both* the current bestHint and the + // candidate hint are less than bestNonPreferredAffinityCount, so we + // want to choose one that brings us back up as close to + // bestNonPreferredAffinityCount as possible. There are three cases to + // consider here: + // + // 3ca. candidate.NUMANodeAffinity.Count() > current.NUMANodeAffinity.Count() + // 3cb. candidate.NUMANodeAffinity.Count() < current.NUMANodeAffinity.Count() + // 3cc. candidate.NUMANodeAffinity.Count() == current.NUMANodeAffinity.Count() + // + // For case (3ca), we want to immediately update bestHint to the + // candidate hint because that will bring us closer to the (higher) + // value of bestNonPreferredAffinityCount. + // + // For case (3cb), we want to stick with the current bestHint because + // choosing the candidate hint would strictly move us further away from + // 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. + + // Case 1 + if current.NUMANodeAffinity.Count() > bestNonPreferredAffinityCount { + if candidate.NUMANodeAffinity.IsNarrowerThan(current.NUMANodeAffinity) { + return candidate + } + return current + } + // Case 2 + if current.NUMANodeAffinity.Count() == bestNonPreferredAffinityCount { + if candidate.NUMANodeAffinity.Count() != bestNonPreferredAffinityCount { + return current + } + if candidate.NUMANodeAffinity.IsNarrowerThan(current.NUMANodeAffinity) { + return candidate + } + return current + } + // Case 3a + if candidate.NUMANodeAffinity.Count() > bestNonPreferredAffinityCount { + return current + } + // Case 3b + if candidate.NUMANodeAffinity.Count() == bestNonPreferredAffinityCount { + return candidate + } + // Case 3ca + if candidate.NUMANodeAffinity.Count() > current.NUMANodeAffinity.Count() { + return candidate + } + // Case 3cb + if candidate.NUMANodeAffinity.Count() < current.NUMANodeAffinity.Count() { + return current + } + // Case 3cc + if candidate.NUMANodeAffinity.IsNarrowerThan(current.NUMANodeAffinity) { + return candidate + } + return current +} + +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) + + var bestHint *TopologyHint iterateAllProviderTopologyHints(filteredHints, 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) - // 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 + // Compare the current bestHint with the candidate mergedHint and + // update bestHint if appropriate. + bestHint = compareHints(bestNonPreferredAffinityCount, bestHint, &mergedHint) }) - return bestHint + if bestHint == nil { + defaultAffinity, _ := bitmask.NewBitMask(numaNodes...) + bestHint = &TopologyHint{defaultAffinity, false} + } + + return *bestHint } // Iterate over all permutations of hints in 'allProviderHints [][]TopologyHint'. diff --git a/pkg/kubelet/cm/topologymanager/policy_test.go b/pkg/kubelet/cm/topologymanager/policy_test.go index 1cc4f21d0cf..df9f7e8d5f6 100644 --- a/pkg/kubelet/cm/topologymanager/policy_test.go +++ b/pkg/kubelet/cm/topologymanager/policy_test.go @@ -21,6 +21,7 @@ import ( "testing" "k8s.io/api/core/v1" + "k8s.io/kubernetes/pkg/kubelet/cm/topologymanager/bitmask" ) type policyMergeTestCase struct { @@ -615,6 +616,220 @@ func (p *bestEffortPolicy) mergeTestCases(numaNodes []int) []policyMergeTestCase Preferred: false, }, }, + { + name: "bestNonPreferredAffinityCount (1)", + 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(0, 1), + Preferred: false, + }, + }, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(0, 1), + Preferred: false, + }, + }, + { + name: "bestNonPreferredAffinityCount (2)", + 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(0, 3), + Preferred: false, + }, + }, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(0, 3), + Preferred: false, + }, + }, + { + name: "bestNonPreferredAffinityCount (3)", + 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, + }, + }, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(1, 2), + Preferred: false, + }, + }, + { + name: "bestNonPreferredAffinityCount (4)", + 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(2, 3), + Preferred: false, + }, + }, + }, + }, + }, + expected: TopologyHint{ + NUMANodeAffinity: NewTestBitMask(2, 3), + Preferred: false, + }, + }, + { + 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(1, 2), + 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(1, 2), + Preferred: false, + }, + }, } } @@ -905,3 +1120,283 @@ func testPolicyMerge(policy Policy, tcases []policyMergeTestCase, t *testing.T) } } } + +func TestMaxOfMinAffinityCounts(t *testing.T) { + tcases := []struct { + hints [][]TopologyHint + expected int + }{ + { + [][]TopologyHint{}, + 0, + }, + { + [][]TopologyHint{ + { + TopologyHint{NewTestBitMask(), true}, + }, + }, + 0, + }, + { + [][]TopologyHint{ + { + TopologyHint{NewTestBitMask(0), true}, + }, + }, + 1, + }, + { + [][]TopologyHint{ + { + TopologyHint{NewTestBitMask(0, 1), true}, + }, + }, + 2, + }, + { + [][]TopologyHint{ + { + TopologyHint{NewTestBitMask(0, 1), true}, + TopologyHint{NewTestBitMask(0, 1, 2), true}, + }, + }, + 2, + }, + { + [][]TopologyHint{ + { + TopologyHint{NewTestBitMask(0, 1), true}, + TopologyHint{NewTestBitMask(0, 1, 2), true}, + }, + { + TopologyHint{NewTestBitMask(0, 1, 2), true}, + }, + }, + 3, + }, + { + [][]TopologyHint{ + { + TopologyHint{NewTestBitMask(0, 1), true}, + TopologyHint{NewTestBitMask(0, 1, 2), true}, + }, + { + TopologyHint{NewTestBitMask(0, 1, 2), true}, + TopologyHint{NewTestBitMask(0, 1, 2, 3), true}, + }, + }, + 3, + }, + } + + for _, tc := range tcases { + t.Run("", func(t *testing.T) { + result := maxOfMinAffinityCounts(tc.hints) + if result != tc.expected { + t.Errorf("Expected result to be %v, got %v", tc.expected, result) + } + }) + } +} + +func TestCompareHints(t *testing.T) { + tcases := []struct { + description string + bestNonPreferredAffinityCount int + current *TopologyHint + candidate *TopologyHint + expected string + }{ + { + "candidate.NUMANodeAffinity.Count() == 0 (1)", + -1, + nil, + &TopologyHint{bitmask.NewEmptyBitMask(), false}, + "current", + }, + { + "candidate.NUMANodeAffinity.Count() == 0 (2)", + -1, + &TopologyHint{NewTestBitMask(), true}, + &TopologyHint{NewTestBitMask(), false}, + "current", + }, + { + "current == nil (1)", + -1, + nil, + &TopologyHint{NewTestBitMask(0), true}, + "candidate", + }, + { + "current == nil (2)", + -1, + nil, + &TopologyHint{NewTestBitMask(0), false}, + "candidate", + }, + { + "!current.Preferred && candidate.Preferred", + -1, + &TopologyHint{NewTestBitMask(0), false}, + &TopologyHint{NewTestBitMask(0), true}, + "candidate", + }, + { + "current.Preferred && !candidate.Preferred", + -1, + &TopologyHint{NewTestBitMask(0), true}, + &TopologyHint{NewTestBitMask(0), false}, + "current", + }, + { + "current.Preferred && candidate.Preferred (1)", + -1, + &TopologyHint{NewTestBitMask(0), true}, + &TopologyHint{NewTestBitMask(0), true}, + "current", + }, + { + "current.Preferred && candidate.Preferred (2)", + -1, + &TopologyHint{NewTestBitMask(0, 1), true}, + &TopologyHint{NewTestBitMask(0), true}, + "candidate", + }, + { + "current.Preferred && candidate.Preferred (3)", + -1, + &TopologyHint{NewTestBitMask(0), true}, + &TopologyHint{NewTestBitMask(0, 1), true}, + "current", + }, + { + "!current.Preferred && !candidate.Preferred (1.1)", + 1, + &TopologyHint{NewTestBitMask(0, 1), false}, + &TopologyHint{NewTestBitMask(0, 1), false}, + "current", + }, + { + "!current.Preferred && !candidate.Preferred (1.2)", + 1, + &TopologyHint{NewTestBitMask(1, 2), false}, + &TopologyHint{NewTestBitMask(0, 1), false}, + "candidate", + }, + { + "!current.Preferred && !candidate.Preferred (1.3)", + 1, + &TopologyHint{NewTestBitMask(0, 1), false}, + &TopologyHint{NewTestBitMask(1, 2), false}, + "current", + }, + { + "!current.Preferred && !candidate.Preferred (2.1)", + 2, + &TopologyHint{NewTestBitMask(0, 1), false}, + &TopologyHint{NewTestBitMask(0), false}, + "current", + }, + { + "!current.Preferred && !candidate.Preferred (2.2)", + 2, + &TopologyHint{NewTestBitMask(0, 1), false}, + &TopologyHint{NewTestBitMask(0, 1), false}, + "current", + }, + { + "!current.Preferred && !candidate.Preferred (2.3)", + 2, + &TopologyHint{NewTestBitMask(1, 2), false}, + &TopologyHint{NewTestBitMask(0, 1), false}, + "candidate", + }, + { + "!current.Preferred && !candidate.Preferred (2.4)", + 2, + &TopologyHint{NewTestBitMask(0, 1), false}, + &TopologyHint{NewTestBitMask(1, 2), false}, + "current", + }, + { + "!current.Preferred && !candidate.Preferred (3a)", + 2, + &TopologyHint{NewTestBitMask(0), false}, + &TopologyHint{NewTestBitMask(0, 1, 2), false}, + "current", + }, + { + "!current.Preferred && !candidate.Preferred (3b)", + 2, + &TopologyHint{NewTestBitMask(0), false}, + &TopologyHint{NewTestBitMask(0, 1), false}, + "candidate", + }, + { + "!current.Preferred && !candidate.Preferred (3ca.1)", + 3, + &TopologyHint{NewTestBitMask(0), false}, + &TopologyHint{NewTestBitMask(0, 1), false}, + "candidate", + }, + { + "!current.Preferred && !candidate.Preferred (3ca.2)", + 3, + &TopologyHint{NewTestBitMask(0), false}, + &TopologyHint{NewTestBitMask(1, 2), false}, + "candidate", + }, + { + "!current.Preferred && !candidate.Preferred (3ca.3)", + 4, + &TopologyHint{NewTestBitMask(0, 1), false}, + &TopologyHint{NewTestBitMask(1, 2, 3), false}, + "candidate", + }, + { + "!current.Preferred && !candidate.Preferred (3cb)", + 4, + &TopologyHint{NewTestBitMask(1, 2, 3), false}, + &TopologyHint{NewTestBitMask(0, 1), false}, + "current", + }, + { + "!current.Preferred && !candidate.Preferred (3cc.1)", + 4, + &TopologyHint{NewTestBitMask(0, 1, 2), false}, + &TopologyHint{NewTestBitMask(0, 1, 2), false}, + "current", + }, + { + "!current.Preferred && !candidate.Preferred (3cc.2)", + 4, + &TopologyHint{NewTestBitMask(0, 1, 2), false}, + &TopologyHint{NewTestBitMask(1, 2, 3), false}, + "current", + }, + { + "!current.Preferred && !candidate.Preferred (3cc.3)", + 4, + &TopologyHint{NewTestBitMask(1, 2, 3), false}, + &TopologyHint{NewTestBitMask(0, 1, 2), false}, + "candidate", + }, + } + + for _, tc := range tcases { + t.Run(tc.description, func(t *testing.T) { + result := compareHints(tc.bestNonPreferredAffinityCount, tc.current, tc.candidate) + if result != tc.current && result != tc.candidate { + t.Errorf("Expected result to be either 'current' or 'candidate' hint") + } + if tc.expected == "current" && result != tc.current { + t.Errorf("Expected result to be %v, got %v", tc.current, result) + } + if tc.expected == "candidate" && result != tc.candidate { + t.Errorf("Expected result to be %v, got %v", tc.candidate, result) + } + }) + } +}