Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug in TopologyManager with merging hints when NUM_NUMA > 2 #108052

Merged
merged 1 commit into from Feb 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 8 additions & 6 deletions pkg/kubelet/cm/topologymanager/policy.go
Expand Up @@ -41,21 +41,23 @@ func mergePermutation(numaNodes []int, permutation []TopologyHint) TopologyHint
var numaAffinities []bitmask.BitMask
for _, hint := range permutation {
// Only consider hints that have an actual NUMANodeAffinity set.
if hint.NUMANodeAffinity == nil {
numaAffinities = append(numaAffinities, defaultAffinity)
} else {
if hint.NUMANodeAffinity != nil {
numaAffinities = append(numaAffinities, hint.NUMANodeAffinity)
// Only mark preferred if all affinities are equal.
if !hint.NUMANodeAffinity.IsEqual(numaAffinities[0]) {
preferred = false
}
}

// Only mark preferred if all affinities are preferred.
if !hint.Preferred {
preferred = false
}
}

// Merge the affinities using a bitwise-and operation.
mergedAffinity := bitmask.And(defaultAffinity, numaAffinities...)
// Build a mergedHint from the merged affinity mask, indicating if an
// preferred allocation was used to generate the affinity mask or not.
// Build a mergedHint from the merged affinity mask, setting preferred as
// appropriate based on the logic above.
return TopologyHint{mergedAffinity, preferred}
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/kubelet/cm/topologymanager/policy_best_effort_test.go
Expand Up @@ -50,7 +50,7 @@ func TestPolicyBestEffortCanAdmitPodResult(t *testing.T) {
}

func TestPolicyBestEffortMerge(t *testing.T) {
numaNodes := []int{0, 1}
numaNodes := []int{0, 1, 2, 3}
policy := NewBestEffortPolicy(numaNodes)

tcases := commonPolicyMergeTestCases(numaNodes)
Expand Down
2 changes: 1 addition & 1 deletion pkg/kubelet/cm/topologymanager/policy_restricted_test.go
Expand Up @@ -68,7 +68,7 @@ func TestPolicyRestrictedCanAdmitPodResult(t *testing.T) {
}

func TestPolicyRestrictedMerge(t *testing.T) {
numaNodes := []int{0, 1}
numaNodes := []int{0, 1, 2, 3}
policy := NewRestrictedPolicy(numaNodes)

tcases := commonPolicyMergeTestCases(numaNodes)
Expand Down
Expand Up @@ -156,7 +156,7 @@ func TestPolicySingleNumaNodeFilterHints(t *testing.T) {
}

func TestPolicySingleNumaNodeMerge(t *testing.T) {
numaNodes := []int{0, 1}
numaNodes := []int{0, 1, 2, 3}
policy := NewSingleNumaNodePolicy(numaNodes)

tcases := commonPolicyMergeTestCases(numaNodes)
Expand Down
43 changes: 40 additions & 3 deletions pkg/kubelet/cm/topologymanager/policy_test.go
Expand Up @@ -315,6 +315,43 @@ func commonPolicyMergeTestCases(numaNodes []int) []policyMergeTestCase {

func (p *bestEffortPolicy) mergeTestCases(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, 1),
Preferred: true,
},
{
NUMANodeAffinity: NewTestBitMask(0, 2),
Preferred: true,
},
},
},
},
&mockHintProvider{
map[string][]TopologyHint{
"resource2": {
{
NUMANodeAffinity: NewTestBitMask(0, 1),
Preferred: true,
},
{
NUMANodeAffinity: NewTestBitMask(0, 2),
Preferred: true,
},
},
},
},
},
expected: TopologyHint{
NUMANodeAffinity: NewTestBitMask(0, 1),
Preferred: true,
},
},
{
name: "TopologyHint not set",
hp: []HintProvider{},
Expand Down Expand Up @@ -513,7 +550,7 @@ func (p *bestEffortPolicy) mergeTestCases(numaNodes []int) []policyMergeTestCase
},
expected: TopologyHint{
NUMANodeAffinity: NewTestBitMask(0),
Preferred: true,
Preferred: false,
},
},
{
Expand Down Expand Up @@ -550,7 +587,7 @@ func (p *bestEffortPolicy) mergeTestCases(numaNodes []int) []policyMergeTestCase
},
},
{
name: "Two providers, 1 hint each, 1 wider mask, both preferred 1/2",
name: "Two providers, 1 hint each, 1 wider mask, both preferred 2/2",
hp: []HintProvider{
&mockHintProvider{
map[string][]TopologyHint{
Expand All @@ -575,7 +612,7 @@ func (p *bestEffortPolicy) mergeTestCases(numaNodes []int) []policyMergeTestCase
},
expected: TopologyHint{
NUMANodeAffinity: NewTestBitMask(1),
Preferred: true,
Preferred: false,
},
},
}
Expand Down