From 9e9808d0ab2e72235e9241e92ba65cdd2d565129 Mon Sep 17 00:00:00 2001 From: Wei Huang Date: Tue, 30 Apr 2019 16:29:24 -0700 Subject: [PATCH] EvenPodsSpread: Supports Preemption (removePod) - add removePod() for podSpreadMap --- .../algorithm/predicates/metadata.go | 27 +++ .../algorithm/predicates/metadata_test.go | 220 ++++++++++++++++++ .../algorithm/predicates/predicates.go | 1 - 3 files changed, 247 insertions(+), 1 deletion(-) diff --git a/pkg/scheduler/algorithm/predicates/metadata.go b/pkg/scheduler/algorithm/predicates/metadata.go index cd5d0367d1a..1c41204f569 100644 --- a/pkg/scheduler/algorithm/predicates/metadata.go +++ b/pkg/scheduler/algorithm/predicates/metadata.go @@ -374,6 +374,31 @@ func (m *topologyPairsMaps) clone() *topologyPairsMaps { return copy } +func (m *topologyPairsPodSpreadMap) removePod(deletedPod *v1.Pod) { + if m == nil || deletedPod == nil { + return + } + + deletedPodFullName := schedutil.GetPodFullName(deletedPod) + pairSet, ok := m.podToTopologyPairs[deletedPodFullName] + if !ok { + return + } + topologyPairToPods := m.topologyPairToPods + for pair := range pairSet { + delete(topologyPairToPods[pair], deletedPod) + // if topologyPairToPods[pair] is empty after deletion + // don't clean it up as that topology counts as a match now + + // removal of the deletedPod would probably genereate a smaller matching number + // so re-calculate minMatch to a smaller value if possible + if l := int32(len(topologyPairToPods[pair])); l < m.topologyKeyToMinPodsMap[pair.key] { + m.topologyKeyToMinPodsMap[pair.key] = l + } + } + delete(m.podToTopologyPairs, deletedPodFullName) +} + func (m *topologyPairsPodSpreadMap) clone() *topologyPairsPodSpreadMap { // m could be nil when EvenPodsSpread feature is disabled if m == nil { @@ -400,6 +425,8 @@ func (meta *predicateMetadata) RemovePod(deletedPod *v1.Pod) error { // Delete pod from the matching affinity or anti-affinity topology pairs maps. meta.topologyPairsPotentialAffinityPods.removePod(deletedPod) meta.topologyPairsPotentialAntiAffinityPods.removePod(deletedPod) + // Delete pod from the pod spread topology maps. + meta.topologyPairsPodSpreadMap.removePod(deletedPod) // All pods in the serviceAffinityMatchingPodList are in the same namespace. // So, if the namespace of the first one is not the same as the namespace of the // deletedPod, we don't need to check the list, as deletedPod isn't in the list. diff --git a/pkg/scheduler/algorithm/predicates/metadata_test.go b/pkg/scheduler/algorithm/predicates/metadata_test.go index 4a284667f60..c243bc5c6b4 100644 --- a/pkg/scheduler/algorithm/predicates/metadata_test.go +++ b/pkg/scheduler/algorithm/predicates/metadata_test.go @@ -1243,6 +1243,226 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) { } } +func TestPodSpreadMap_RemovePod(t *testing.T) { + tests := []struct { + name string + preemptor *v1.Pod // preemptor pod + nodes []*v1.Node + existingPods []*v1.Pod + deletedPodIdx int // need to reuse *Pod of exsitingPods[i] + deletedPod *v1.Pod // if deletedPodIdx is invalid, this field is bypassed + injectPodPointers map[topologyPair][]int + want *topologyPairsPodSpreadMap + }{ + { + // A high priority pod may not be scheduled due to node taints or resource shortage. + // So preemption is triggered. + name: "one spreadConstraint on zone, minMatchMap unchanged", + preemptor: u.MakePod().Name("p").Label("foo", ""). + SpreadConstraint(1, "zone", hardSpread, u.MakeLabelSelector().Exists("foo").Obj()). + Obj(), + nodes: []*v1.Node{ + u.MakeNode().Name("node-a").Label("zone", "zone1").Label("node", "node-a").Obj(), + u.MakeNode().Name("node-b").Label("zone", "zone1").Label("node", "node-b").Obj(), + u.MakeNode().Name("node-x").Label("zone", "zone2").Label("node", "node-x").Obj(), + }, + existingPods: []*v1.Pod{ + u.MakePod().Name("p-a1").Node("node-a").Label("foo", "").Obj(), + u.MakePod().Name("p-b1").Node("node-b").Label("foo", "").Obj(), + u.MakePod().Name("p-x1").Node("node-x").Label("foo", "").Obj(), + }, + deletedPodIdx: 0, // remove pod "p-a1" + injectPodPointers: map[topologyPair][]int{ + {key: "zone", value: "zone1"}: {1}, + {key: "zone", value: "zone2"}: {2}, + }, + want: &topologyPairsPodSpreadMap{ + // minMatchMap actually doesn't change + minMatchMap: map[string]int32{"zone": 1}, + topologyPairsMaps: &topologyPairsMaps{ + podToTopologyPairs: map[string]topologyPairSet{ + "p-b1_": newPairSet("zone", "zone1"), + "p-x1_": newPairSet("zone", "zone2"), + }, + }, + }, + }, + { + name: "one spreadConstraint on node, minMatchMap changed", + preemptor: u.MakePod().Name("p").Label("foo", ""). + SpreadConstraint(1, "zone", hardSpread, u.MakeLabelSelector().Exists("foo").Obj()). + Obj(), + nodes: []*v1.Node{ + u.MakeNode().Name("node-a").Label("zone", "zone1").Label("node", "node-a").Obj(), + u.MakeNode().Name("node-b").Label("zone", "zone1").Label("node", "node-b").Obj(), + u.MakeNode().Name("node-x").Label("zone", "zone2").Label("node", "node-x").Obj(), + u.MakeNode().Name("node-y").Label("zone", "zone2").Label("node", "node-y").Obj(), + }, + existingPods: []*v1.Pod{ + u.MakePod().Name("p-a1").Node("node-a").Label("foo", "").Obj(), + u.MakePod().Name("p-b1").Node("node-b").Label("foo", "").Obj(), + u.MakePod().Name("p-x1").Node("node-x").Label("foo", "").Obj(), + u.MakePod().Name("p-y1").Node("node-y").Label("foo", "").Obj(), + }, + deletedPodIdx: 0, // remove pod "p-a1" + injectPodPointers: map[topologyPair][]int{ + {key: "zone", value: "zone1"}: {1}, + {key: "zone", value: "zone2"}: {2, 3}, + }, + want: &topologyPairsPodSpreadMap{ + // minMatchMap is expected to be re-calculated from {"zone": 2} + // to {"zone": 1} + minMatchMap: map[string]int32{"zone": 1}, + topologyPairsMaps: &topologyPairsMaps{ + podToTopologyPairs: map[string]topologyPairSet{ + "p-b1_": newPairSet("zone", "zone1"), + "p-x1_": newPairSet("zone", "zone2"), + "p-y1_": newPairSet("zone", "zone2"), + }, + }, + }, + }, + { + name: "delete an irrelevant pod won't help", + preemptor: u.MakePod().Name("p").Label("foo", ""). + SpreadConstraint(1, "zone", hardSpread, u.MakeLabelSelector().Exists("foo").Obj()). + Obj(), + nodes: []*v1.Node{ + u.MakeNode().Name("node-a").Label("zone", "zone1").Label("node", "node-a").Obj(), + u.MakeNode().Name("node-b").Label("zone", "zone1").Label("node", "node-b").Obj(), + u.MakeNode().Name("node-x").Label("zone", "zone2").Label("node", "node-x").Obj(), + u.MakeNode().Name("node-y").Label("zone", "zone2").Label("node", "node-y").Obj(), + }, + existingPods: []*v1.Pod{ + u.MakePod().Name("p-a0").Node("node-a").Label("bar", "").Obj(), + u.MakePod().Name("p-a1").Node("node-a").Label("foo", "").Obj(), + u.MakePod().Name("p-b1").Node("node-b").Label("foo", "").Obj(), + u.MakePod().Name("p-x1").Node("node-x").Label("foo", "").Obj(), + u.MakePod().Name("p-y1").Node("node-y").Label("foo", "").Obj(), + }, + deletedPodIdx: 0, // remove pod "p-a0" + injectPodPointers: map[topologyPair][]int{ + {key: "zone", value: "zone1"}: {1, 2}, + {key: "zone", value: "zone2"}: {3, 4}, + }, + want: &topologyPairsPodSpreadMap{ + // minMatchMap is unchanged + minMatchMap: map[string]int32{"zone": 2}, + topologyPairsMaps: &topologyPairsMaps{ + podToTopologyPairs: map[string]topologyPairSet{ + "p-a1_": newPairSet("zone", "zone1"), + "p-b1_": newPairSet("zone", "zone1"), + "p-x1_": newPairSet("zone", "zone2"), + "p-y1_": newPairSet("zone", "zone2"), + }, + }, + }, + }, + { + name: "delete a non-existing pod won't help", + preemptor: u.MakePod().Name("p").Label("foo", ""). + SpreadConstraint(1, "zone", hardSpread, u.MakeLabelSelector().Exists("foo").Obj()). + Obj(), + nodes: []*v1.Node{ + u.MakeNode().Name("node-a").Label("zone", "zone1").Label("node", "node-a").Obj(), + u.MakeNode().Name("node-b").Label("zone", "zone1").Label("node", "node-b").Obj(), + u.MakeNode().Name("node-x").Label("zone", "zone2").Label("node", "node-x").Obj(), + u.MakeNode().Name("node-y").Label("zone", "zone2").Label("node", "node-y").Obj(), + }, + existingPods: []*v1.Pod{ + u.MakePod().Name("p-a1").Node("node-a").Label("foo", "").Obj(), + u.MakePod().Name("p-b1").Node("node-b").Label("foo", "").Obj(), + u.MakePod().Name("p-x1").Node("node-x").Label("foo", "").Obj(), + u.MakePod().Name("p-y1").Node("node-y").Label("foo", "").Obj(), + }, + deletedPodIdx: -1, + deletedPod: u.MakePod().Name("p-a0").Node("node-a").Label("bar", "").Obj(), + injectPodPointers: map[topologyPair][]int{ + {key: "zone", value: "zone1"}: {0, 1}, + {key: "zone", value: "zone2"}: {2, 3}, + }, + want: &topologyPairsPodSpreadMap{ + // minMatchMap is unchanged + minMatchMap: map[string]int32{"zone": 2}, + topologyPairsMaps: &topologyPairsMaps{ + podToTopologyPairs: map[string]topologyPairSet{ + "p-a1_": newPairSet("zone", "zone1"), + "p-b1_": newPairSet("zone", "zone1"), + "p-x1_": newPairSet("zone", "zone2"), + "p-y1_": newPairSet("zone", "zone2"), + }, + }, + }, + }, + { + name: "two spreadConstraints", + preemptor: u.MakePod().Name("p").Label("foo", ""). + SpreadConstraint(1, "zone", hardSpread, u.MakeLabelSelector().Exists("foo").Obj()). + SpreadConstraint(1, "node", hardSpread, u.MakeLabelSelector().Exists("foo").Obj()). + Obj(), + nodes: []*v1.Node{ + u.MakeNode().Name("node-a").Label("zone", "zone1").Label("node", "node-a").Obj(), + u.MakeNode().Name("node-b").Label("zone", "zone1").Label("node", "node-b").Obj(), + u.MakeNode().Name("node-x").Label("zone", "zone2").Label("node", "node-x").Obj(), + }, + existingPods: []*v1.Pod{ + u.MakePod().Name("p-a1").Node("node-a").Label("foo", "").Obj(), + u.MakePod().Name("p-a2").Node("node-a").Label("foo", "").Obj(), + u.MakePod().Name("p-b1").Node("node-b").Label("foo", "").Obj(), + u.MakePod().Name("p-x1").Node("node-x").Label("foo", "").Obj(), + u.MakePod().Name("p-x2").Node("node-x").Label("foo", "").Obj(), + }, + deletedPodIdx: 3, // remove pod "p-x1" + injectPodPointers: map[topologyPair][]int{ + {key: "zone", value: "zone1"}: {0, 1, 2}, + {key: "zone", value: "zone2"}: {4}, + {key: "node", value: "node-a"}: {0, 1}, + {key: "node", value: "node-b"}: {2}, + {key: "node", value: "node-x"}: {4}, + }, + want: &topologyPairsPodSpreadMap{ + // minMatchMap is expected to be re-calculated from {"zone": 2, "node": 1} + // to {"zone": 1, "node": 1} + minMatchMap: map[string]int32{"zone": 1, "node": 1}, + topologyPairsMaps: &topologyPairsMaps{ + podToTopologyPairs: map[string]topologyPairSet{ + "p-a1_": newPairSet("zone", "zone1", "node", "node-a"), + "p-a2_": newPairSet("zone", "zone1", "node", "node-a"), + "p-b1_": newPairSet("zone", "zone1", "node", "node-b"), + "p-x2_": newPairSet("zone", "zone2", "node", "node-x"), + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tt.want.topologyPairToPods = make(map[topologyPair]podSet) + for pair, indice := range tt.injectPodPointers { + pSet := make(podSet) + for _, i := range indice { + pSet[tt.existingPods[i]] = struct{}{} + } + tt.want.topologyPairToPods[pair] = pSet + } + + nodeInfoMap := schedulernodeinfo.CreateNodeNameToInfoMap(tt.existingPods, tt.nodes) + podSpreadMap := getTPMapMatchingSpreadConstraints(tt.preemptor, nodeInfoMap) + + var deletedPod *v1.Pod + if tt.deletedPodIdx < len(tt.existingPods) && tt.deletedPodIdx >= 0 { + deletedPod = tt.existingPods[tt.deletedPodIdx] + } else { + deletedPod = tt.deletedPod + } + podSpreadMap.removePod(deletedPod) + if !reflect.DeepEqual(podSpreadMap, tt.want) { + t.Errorf("podSpreadMap#removePod() = %v, want %v", podSpreadMap, tt.want) + } + }) + } +} + var ( hardSpread = v1.DoNotSchedule softSpread = v1.ScheduleAnyway diff --git a/pkg/scheduler/algorithm/predicates/predicates.go b/pkg/scheduler/algorithm/predicates/predicates.go index 4a47662c2ca..059c11220a6 100644 --- a/pkg/scheduler/algorithm/predicates/predicates.go +++ b/pkg/scheduler/algorithm/predicates/predicates.go @@ -1722,7 +1722,6 @@ func EvenPodsSpreadPredicate(pod *v1.Pod, meta PredicateMetadata, nodeInfo *sche if node == nil { return false, nil, fmt.Errorf("node not found") } - constraints := getHardTopologySpreadConstraints(pod) if len(constraints) == 0 { return true, nil, nil