Fixed bug in TopologyManager with SingleNUMANode Policy

This patch fixes an issue in the TopologyManager that wouldn't allow
pods to be admitted if pods were launched with the SingleNUMANode policy
and any of the hint providers had no NUMA preferences.

This is due to 2 factors:

1) Any hint provider that passes back a `nil` as its hints, has its hint
automatically transformed into a single {11 true} hint before merging

2) We added a special casing for the SingleNumaNodePolicy() in the
TopologyManager that essentially turns these hints into a
{11 false} anytime a {11 true} is seen.

The current patch reworks this logic so the that TopologyManager can
tell the difference between a "don't care" hint and a true "{11 true}"
hint returned by the hint provider. Only true "{11 true}" hints will be
converted by the special casing for the SingleNumaNodePolicy(), while
"don't care" hints will not.

This is a short term fix for this issue until we do a larger refactoring
of this code for the 1.17 release.
This commit is contained in:
Kevin Klues 2019-10-09 15:47:27 -07:00
parent 0956acbed1
commit 5501f542cd
2 changed files with 48 additions and 14 deletions

View File

@ -201,7 +201,7 @@ func (m *manager) calculateAffinity(pod v1.Pod, container v1.Container) Topology
// If hints is nil, insert a single, preferred any-numa hint into allProviderHints. // If hints is nil, insert a single, preferred any-numa hint into allProviderHints.
if len(hints) == 0 { if len(hints) == 0 {
klog.Infof("[topologymanager] Hint Provider has no preference for NUMA affinity with any resource") klog.Infof("[topologymanager] Hint Provider has no preference for NUMA affinity with any resource")
allProviderHints = append(allProviderHints, []TopologyHint{{defaultAffinity, true}}) allProviderHints = append(allProviderHints, []TopologyHint{{nil, true}})
continue continue
} }
@ -209,13 +209,13 @@ func (m *manager) calculateAffinity(pod v1.Pod, container v1.Container) Topology
for resource := range hints { for resource := range hints {
if hints[resource] == nil { if hints[resource] == nil {
klog.Infof("[topologymanager] Hint Provider has no preference for NUMA affinity with resource '%s'", resource) klog.Infof("[topologymanager] Hint Provider has no preference for NUMA affinity with resource '%s'", resource)
allProviderHints = append(allProviderHints, []TopologyHint{{defaultAffinity, true}}) allProviderHints = append(allProviderHints, []TopologyHint{{nil, true}})
continue continue
} }
if len(hints[resource]) == 0 { if len(hints[resource]) == 0 {
klog.Infof("[topologymanager] Hint Provider has no possible NUMA affinities for resource '%s'", resource) klog.Infof("[topologymanager] Hint Provider has no possible NUMA affinities for resource '%s'", resource)
allProviderHints = append(allProviderHints, []TopologyHint{{defaultAffinity, false}}) allProviderHints = append(allProviderHints, []TopologyHint{{nil, false}})
continue continue
} }
@ -235,18 +235,21 @@ func (m *manager) calculateAffinity(pod v1.Pod, container v1.Container) Topology
preferred := true preferred := true
var numaAffinities []bitmask.BitMask var numaAffinities []bitmask.BitMask
for _, hint := range permutation { for _, hint := range permutation {
// Only consider hints that have an actual NUMANodeAffinity set. if hint.NUMANodeAffinity == nil {
if hint.NUMANodeAffinity != nil { numaAffinities = append(numaAffinities, defaultAffinity)
if !hint.Preferred { } else {
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.Count() > 1 {
preferred = false
}
numaAffinities = append(numaAffinities, hint.NUMANodeAffinity) 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. // Merge the affinities using a bitwise-and operation.

View File

@ -196,7 +196,7 @@ func TestCalculateAffinity(t *testing.T) {
}, },
expected: TopologyHint{ expected: TopologyHint{
NUMANodeAffinity: NewTestBitMask(numaNodes...), NUMANodeAffinity: NewTestBitMask(numaNodes...),
Preferred: true, Preferred: false,
}, },
}, },
{ {
@ -690,6 +690,37 @@ func TestCalculateAffinity(t *testing.T) {
Preferred: false, 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 { for _, tc := range tcases {