From a9461fe6f50dbe2f4a5fd86b40dd54df28be7e38 Mon Sep 17 00:00:00 2001 From: Aldo Culquicondor Date: Mon, 27 Apr 2020 13:18:34 -0400 Subject: [PATCH] spreading: Store ignored instead of qualifying nodes Signed-off-by: Aldo Culquicondor --- .../plugins/podtopologyspread/scoring.go | 24 +++++++++---------- .../plugins/podtopologyspread/scoring_test.go | 8 +++---- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/pkg/scheduler/framework/plugins/podtopologyspread/scoring.go b/pkg/scheduler/framework/plugins/podtopologyspread/scoring.go index 378cbd130fa..29af84a2d0c 100644 --- a/pkg/scheduler/framework/plugins/podtopologyspread/scoring.go +++ b/pkg/scheduler/framework/plugins/podtopologyspread/scoring.go @@ -35,8 +35,8 @@ const preScoreStateKey = "PreScore" + Name // Fields are exported for comparison during testing. type preScoreState struct { Constraints []topologySpreadConstraint - // NodeNameSet is a string set holding all node names which have all Constraints[*].topologyKey present. - NodeNameSet sets.String + // IgnoredNodes is a set of node names which miss some Constraints[*].topologyKey. + IgnoredNodes sets.String // TopologyPairToPodCounts is keyed with topologyPair, and valued with the number of matching pods. TopologyPairToPodCounts map[topologyPair]*int64 } @@ -48,9 +48,9 @@ func (s *preScoreState) Clone() framework.StateData { } // initPreScoreState iterates "filteredNodes" to filter out the nodes which -// don't have required topologyKey(s), and initialize two maps: +// don't have required topologyKey(s), and initialize: // 1) s.TopologyPairToPodCounts: keyed with both eligible topology pair and node names. -// 2) s.NodeNameSet: keyed with node name, and valued with a *int64 pointer for eligible node only. +// 2) s.IgnoredNodes: the set of nodes that shouldn't be scored. func (pl *PodTopologySpread) initPreScoreState(s *preScoreState, pod *v1.Pod, filteredNodes []*v1.Node) error { var err error if len(pod.Spec.TopologySpreadConstraints) > 0 { @@ -69,6 +69,9 @@ func (pl *PodTopologySpread) initPreScoreState(s *preScoreState, pod *v1.Pod, fi } for _, node := range filteredNodes { if !nodeLabelsMatchSpreadConstraints(node.Labels, s.Constraints) { + // Nodes which don't have all required topologyKeys present are ignored + // when scoring later. + s.IgnoredNodes.Insert(node.Name) continue } for _, constraint := range s.Constraints { @@ -81,9 +84,6 @@ func (pl *PodTopologySpread) initPreScoreState(s *preScoreState, pod *v1.Pod, fi s.TopologyPairToPodCounts[pair] = new(int64) } } - s.NodeNameSet.Insert(node.Name) - // For those nodes which don't have all required topologyKeys present, it's intentional to leave - // their entries absent in NodeNameSet, so that we're able to score them to 0 afterwards. } return nil } @@ -106,7 +106,7 @@ func (pl *PodTopologySpread) PreScore( } state := &preScoreState{ - NodeNameSet: make(sets.String, len(filteredNodes)), + IgnoredNodes: sets.NewString(), TopologyPairToPodCounts: make(map[topologyPair]*int64), } err = pl.initPreScoreState(state, pod, filteredNodes) @@ -168,7 +168,7 @@ func (pl *PodTopologySpread) Score(ctx context.Context, cycleState *framework.Cy } // Return if the node is not qualified. - if _, ok := s.NodeNameSet[node.Name]; !ok { + if s.IgnoredNodes.Has(node.Name) { return 0, nil } @@ -204,8 +204,8 @@ func (pl *PodTopologySpread) NormalizeScore(ctx context.Context, cycleState *fra var minScore int64 = math.MaxInt64 var total int64 for _, score := range scores { - // it's mandatory to check if is present in m.NodeNameSet - if _, ok := s.NodeNameSet[score.Name]; !ok { + // it's mandatory to check if is present in m.IgnoredNodes + if s.IgnoredNodes.Has(score.Name) { continue } total += score.Score @@ -227,7 +227,7 @@ func (pl *PodTopologySpread) NormalizeScore(ctx context.Context, cycleState *fra continue } - if _, ok := s.NodeNameSet[node.Name]; !ok { + if s.IgnoredNodes.Has(node.Name) { scores[i].Score = 0 continue } diff --git a/pkg/scheduler/framework/plugins/podtopologyspread/scoring_test.go b/pkg/scheduler/framework/plugins/podtopologyspread/scoring_test.go index d392554f1c6..3bda625739b 100644 --- a/pkg/scheduler/framework/plugins/podtopologyspread/scoring_test.go +++ b/pkg/scheduler/framework/plugins/podtopologyspread/scoring_test.go @@ -68,7 +68,7 @@ func TestPreScoreStateEmptyNodes(t *testing.T) { Selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Exists("foo").Obj()), }, }, - NodeNameSet: sets.NewString("node-a", "node-b", "node-x"), + IgnoredNodes: sets.NewString(), TopologyPairToPodCounts: map[topologyPair]*int64{ {key: "zone", value: "zone1"}: pointer.Int64Ptr(0), {key: "zone", value: "zone2"}: pointer.Int64Ptr(0), @@ -102,7 +102,7 @@ func TestPreScoreStateEmptyNodes(t *testing.T) { Selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Exists("bar").Obj()), }, }, - NodeNameSet: sets.NewString("node-a", "node-b"), + IgnoredNodes: sets.NewString("node-x"), TopologyPairToPodCounts: map[topologyPair]*int64{ {key: "zone", value: "zone1"}: pointer.Int64Ptr(0), {key: "node", value: "node-a"}: pointer.Int64Ptr(0), @@ -137,7 +137,7 @@ func TestPreScoreStateEmptyNodes(t *testing.T) { Selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Exists("foo").Obj()), }, }, - NodeNameSet: sets.NewString("node-a"), + IgnoredNodes: sets.NewString(), TopologyPairToPodCounts: map[topologyPair]*int64{ {key: "node", value: "node-a"}: pointer.Int64Ptr(0), {key: "planet", value: "mars"}: pointer.Int64Ptr(0), @@ -182,7 +182,7 @@ func TestPreScoreStateEmptyNodes(t *testing.T) { Selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Label("baz", "sup").Obj()), }, }, - NodeNameSet: sets.NewString("node-a"), + IgnoredNodes: sets.NewString(), TopologyPairToPodCounts: map[topologyPair]*int64{ {"planet", "mars"}: pointer.Int64Ptr(0), },