diff --git a/pkg/scheduler/algorithm/predicates/metadata.go b/pkg/scheduler/algorithm/predicates/metadata.go index 1e422849e07..cd5d0367d1a 100644 --- a/pkg/scheduler/algorithm/predicates/metadata.go +++ b/pkg/scheduler/algorithm/predicates/metadata.go @@ -104,7 +104,7 @@ type predicateMetadata struct { // from scheduler extender configuration and does not change per pod. ignoredExtendedResources sets.String // Similar to the map for pod (anti-)affinity, but imposes additional min matches info - // to describe mininum match number on each topology spread constraint + // to describe minimum match number on each topology spread constraint. topologyPairsPodSpreadMap *topologyPairsPodSpreadMap } @@ -190,8 +190,8 @@ func (pfactory *PredicateMetadataFactory) GetMetadata(pod *v1.Pod, nodeNameToInf } func getTPMapMatchingSpreadConstraints(pod *v1.Pod, nodeInfoMap map[string]*schedulernodeinfo.NodeInfo) (*topologyPairsPodSpreadMap, error) { - // we have feature gating in APIserver to strip the spec - // so don't need to re-check feature gate, just check length of constraints + // We have feature gating in APIServer to strip the spec + // so don't need to re-check feature gate, just check length of constraints. constraints := getHardTopologySpreadConstraints(pod) if len(constraints) == 0 { return nil, nil @@ -206,7 +206,7 @@ func getTPMapMatchingSpreadConstraints(pod *v1.Pod, nodeInfoMap map[string]*sche var lock sync.Mutex topologyPairsPodSpreadMap := &topologyPairsPodSpreadMap{ - // topologyKeyToMinPodsMap will be initilized with proper size later. + // topologyKeyToMinPodsMap will be initialized with proper size later. topologyPairsMaps: newTopologyPairsMaps(), } @@ -230,7 +230,7 @@ func getTPMapMatchingSpreadConstraints(pod *v1.Pod, nodeInfoMap map[string]*sche if !podMatchesNodeSelectorAndAffinityTerms(pod, node) { return } - // ensure current node's labels contains all topologyKeys in 'constraints' + // Ensure current node's labels contains all topologyKeys in 'constraints'. for _, constraint := range constraints { if _, ok := node.Labels[constraint.TopologyKey]; !ok { return @@ -240,13 +240,17 @@ func getTPMapMatchingSpreadConstraints(pod *v1.Pod, nodeInfoMap map[string]*sche nodeTopologyMaps := newTopologyPairsMaps() // nodeInfo.Pods() can be empty; or all pods don't fit for _, existingPod := range nodeInfo.Pods() { - ok, err := podMatchesAllSpreadConstraints(existingPod, pod.Namespace, constraints) - if err != nil { - errCh.SendErrorWithCancel(err, cancel) - return + if existingPod.Namespace != pod.Namespace { + continue } - if ok { - for _, constraint := range constraints { + podLabelSet := labels.Set(existingPod.Labels) + for _, constraint := range constraints { + ok, err := podMatchesSpreadConstraint(podLabelSet, constraint) + if err != nil { + errCh.SendErrorWithCancel(err, cancel) + return + } + if ok { // constraint.TopologyKey is already guaranteed to be present pair := topologyPair{key: constraint.TopologyKey, value: node.Labels[constraint.TopologyKey]} nodeTopologyMaps.addTopologyPair(pair, existingPod) @@ -254,7 +258,7 @@ func getTPMapMatchingSpreadConstraints(pod *v1.Pod, nodeInfoMap map[string]*sche } } // If needed, append topology pair without entry of pods. - // For example, on node-x, there is no pod matching spread constraints + // For example, on node-x, there is no pod matching spread constraints, // but node-x should be also considered as a match (with match number 0) // i.e. : {} for _, constraint := range constraints { @@ -281,8 +285,8 @@ func getTPMapMatchingSpreadConstraints(pod *v1.Pod, nodeInfoMap map[string]*sche topologyPairsPodSpreadMap.topologyKeyToMinPodsMap[constraint.TopologyKey] = math.MaxInt32 } for pair, podSet := range topologyPairsPodSpreadMap.topologyPairToPods { - // TODO(Huang-Wei): short circuit all portions of - // if we see 0 as min match of the topologyKey + // TODO(Huang-Wei): short circuit unvisited portions of + // if we already see 0 as min match of that topologyKey. if l := int32(len(podSet)); l < topologyPairsPodSpreadMap.topologyKeyToMinPodsMap[pair.key] { topologyPairsPodSpreadMap.topologyKeyToMinPodsMap[pair.key] = l } @@ -301,30 +305,16 @@ func getHardTopologySpreadConstraints(pod *v1.Pod) (constraints []v1.TopologySpr return } -func podMatchesAllSpreadConstraints(pod *v1.Pod, ns string, constraints []v1.TopologySpreadConstraint) (bool, error) { - if len(constraints) == 0 || pod.Namespace != ns { - return false, nil - } - return podLabelsMatchesSpreadConstraints(pod.Labels, constraints) -} - // some corner cases: -// 1. podLabels = nil, constraint.LabelSelector = nil => returns false -// 2. podLabels = nil => returns false -// 3. constraint.LabelSelector = nil => returns false -func podLabelsMatchesSpreadConstraints(podLabels map[string]string, constraints []v1.TopologySpreadConstraint) (bool, error) { - if len(constraints) == 0 { - return false, nil +// 1. podLabelSet = nil => returns (false, nil) +// 2. constraint.LabelSelector = nil => returns (false, nil) +func podMatchesSpreadConstraint(podLabelSet labels.Set, constraint v1.TopologySpreadConstraint) (bool, error) { + selector, err := metav1.LabelSelectorAsSelector(constraint.LabelSelector) + if err != nil { + return false, err } - podLabelSet := labels.Set(podLabels) - for _, constraint := range constraints { - selector, err := metav1.LabelSelectorAsSelector(constraint.LabelSelector) - if err != nil { - return false, err - } - if !selector.Matches(podLabelSet) { - return false, nil - } + if !selector.Matches(podLabelSet) { + return false, nil } return true, nil } @@ -337,9 +327,7 @@ func newTopologyPairsMaps() *topologyPairsMaps { func (m *topologyPairsMaps) addTopologyPair(pair topologyPair, pod *v1.Pod) { podFullName := schedutil.GetPodFullName(pod) - if m.topologyPairToPods[pair] == nil { - m.topologyPairToPods[pair] = make(map[*v1.Pod]struct{}) - } + m.addTopologyPairWithoutPods(pair) m.topologyPairToPods[pair][pod] = struct{}{} if m.podToTopologyPairs[podFullName] == nil { m.podToTopologyPairs[podFullName] = make(map[topologyPair]struct{}) diff --git a/pkg/scheduler/algorithm/predicates/metadata_test.go b/pkg/scheduler/algorithm/predicates/metadata_test.go index 1ca96f948e9..4a284667f60 100644 --- a/pkg/scheduler/algorithm/predicates/metadata_test.go +++ b/pkg/scheduler/algorithm/predicates/metadata_test.go @@ -24,6 +24,7 @@ import ( "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" schedulernodeinfo "k8s.io/kubernetes/pkg/scheduler/nodeinfo" st "k8s.io/kubernetes/pkg/scheduler/testing" ) @@ -825,25 +826,23 @@ func TestGetTPMapMatchingIncomingAffinityAntiAffinity(t *testing.T) { } } -func TestPodLabelsMatchesSpreadConstraints(t *testing.T) { +func TestPodMatchesSpreadConstraint(t *testing.T) { tests := []struct { - name string - podLabels map[string]string - constraints []v1.TopologySpreadConstraint - want bool - wantErr bool + name string + podLabels map[string]string + constraint v1.TopologySpreadConstraint + want bool + wantErr bool }{ { name: "normal match", podLabels: map[string]string{"foo": "", "bar": ""}, - constraints: []v1.TopologySpreadConstraint{ - { - LabelSelector: &metav1.LabelSelector{ - MatchExpressions: []metav1.LabelSelectorRequirement{ - { - Key: "foo", - Operator: metav1.LabelSelectorOpExists, - }, + constraint: v1.TopologySpreadConstraint{ + LabelSelector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "foo", + Operator: metav1.LabelSelectorOpExists, }, }, }, @@ -853,18 +852,16 @@ func TestPodLabelsMatchesSpreadConstraints(t *testing.T) { { name: "normal mismatch", podLabels: map[string]string{"foo": "", "baz": ""}, - constraints: []v1.TopologySpreadConstraint{ - { - LabelSelector: &metav1.LabelSelector{ - MatchExpressions: []metav1.LabelSelectorRequirement{ - { - Key: "foo", - Operator: metav1.LabelSelectorOpExists, - }, - { - Key: "bar", - Operator: metav1.LabelSelectorOpExists, - }, + constraint: v1.TopologySpreadConstraint{ + LabelSelector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "foo", + Operator: metav1.LabelSelectorOpExists, + }, + { + Key: "bar", + Operator: metav1.LabelSelectorOpExists, }, }, }, @@ -873,14 +870,12 @@ func TestPodLabelsMatchesSpreadConstraints(t *testing.T) { }, { name: "podLabels is nil", - constraints: []v1.TopologySpreadConstraint{ - { - LabelSelector: &metav1.LabelSelector{ - MatchExpressions: []metav1.LabelSelectorRequirement{ - { - Key: "foo", - Operator: metav1.LabelSelectorOpExists, - }, + constraint: v1.TopologySpreadConstraint{ + LabelSelector: &metav1.LabelSelector{ + MatchExpressions: []metav1.LabelSelectorRequirement{ + { + Key: "foo", + Operator: metav1.LabelSelectorOpExists, }, }, }, @@ -893,31 +888,28 @@ func TestPodLabelsMatchesSpreadConstraints(t *testing.T) { "foo": "", "bar": "", }, - constraints: []v1.TopologySpreadConstraint{ - { - MaxSkew: 1, - }, + constraint: v1.TopologySpreadConstraint{ + MaxSkew: 1, }, want: false, }, { name: "both podLabels and constraint.LabelSelector are nil", - constraints: []v1.TopologySpreadConstraint{ - { - MaxSkew: 1, - }, + constraint: v1.TopologySpreadConstraint{ + MaxSkew: 1, }, want: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := podLabelsMatchesSpreadConstraints(tt.podLabels, tt.constraints) + podLabelSet := labels.Set(tt.podLabels) + got, err := podMatchesSpreadConstraint(podLabelSet, tt.constraint) if (err != nil) != tt.wantErr { - t.Errorf("podLabelsMatchesSpreadConstraints() error = %v, wantErr %v", err, tt.wantErr) + t.Errorf("podMatchesSpreadConstraint() error = %v, wantErr %v", err, tt.wantErr) } if got != tt.want { - t.Errorf("podLabelsMatchesSpreadConstraints() = %v, want %v", got, tt.want) + t.Errorf("podMatchesSpreadConstraint() = %v, want %v", got, tt.want) } }) } @@ -996,7 +988,7 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) { }, }, { - name: "namespace mis-match doesn't count", + name: "namespace mismatch doesn't count", pod: st.MakePod().Name("p").Label("foo", "").SpreadConstraint( 1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj(), ).Obj(), @@ -1117,7 +1109,37 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) { }, }, { - name: "different labelSelectors", + name: "different labelSelectors - simple version", + pod: st.MakePod().Name("p").Label("foo", "").Label("bar", ""). + SpreadConstraint(1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()). + SpreadConstraint(1, "node", hardSpread, st.MakeLabelSelector().Exists("bar").Obj()). + Obj(), + nodes: []*v1.Node{ + st.MakeNode().Name("node-a").Label("zone", "zone1").Label("node", "node-a").Obj(), + st.MakeNode().Name("node-b").Label("zone", "zone1").Label("node", "node-b").Obj(), + st.MakeNode().Name("node-y").Label("zone", "zone2").Label("node", "node-y").Obj(), + }, + existingPods: []*v1.Pod{ + st.MakePod().Name("p-a").Node("node-a").Label("foo", "").Obj(), + }, + injectPodPointers: map[topologyPair][]int{ + {key: "zone", value: "zone1"}: {0}, + {key: "zone", value: "zone2"}: {}, + {key: "node", value: "node-a"}: {}, + {key: "node", value: "node-b"}: {}, + {key: "node", value: "node-y"}: {}, + }, + want: &topologyPairsPodSpreadMap{ + topologyKeyToMinPodsMap: map[string]int32{"zone": 0, "node": 0}, + topologyPairsMaps: &topologyPairsMaps{ + podToTopologyPairs: map[string]topologyPairSet{ + "p-a_": newPairSet("zone", "zone1"), + }, + }, + }, + }, + { + name: "different labelSelectors - complex version", pod: st.MakePod().Name("p").Label("foo", "").Label("bar", ""). SpreadConstraint(1, "zone", hardSpread, st.MakeLabelSelector().Exists("foo").Obj()). SpreadConstraint(1, "node", hardSpread, st.MakeLabelSelector().Exists("bar").Obj()). @@ -1137,18 +1159,22 @@ func TestGetTPMapMatchingSpreadConstraints(t *testing.T) { st.MakePod().Name("p-y4").Node("node-y").Label("foo", "").Label("bar", "").Obj(), }, injectPodPointers: map[topologyPair][]int{ - {key: "zone", value: "zone1"}: {1}, - {key: "zone", value: "zone2"}: {4, 6}, + {key: "zone", value: "zone1"}: {0, 1, 2}, + {key: "zone", value: "zone2"}: {3, 4, 5, 6}, {key: "node", value: "node-a"}: {1}, {key: "node", value: "node-b"}: {}, {key: "node", value: "node-y"}: {4, 6}, }, want: &topologyPairsPodSpreadMap{ - topologyKeyToMinPodsMap: map[string]int32{"zone": 1, "node": 0}, + topologyKeyToMinPodsMap: map[string]int32{"zone": 3, "node": 0}, topologyPairsMaps: &topologyPairsMaps{ podToTopologyPairs: map[string]topologyPairSet{ + "p-a1_": newPairSet("zone", "zone1"), "p-a2_": newPairSet("zone", "zone1", "node", "node-a"), + "p-b1_": newPairSet("zone", "zone1"), + "p-y1_": newPairSet("zone", "zone2"), "p-y2_": newPairSet("zone", "zone2", "node", "node-y"), + "p-y3_": newPairSet("zone", "zone2"), "p-y4_": newPairSet("zone", "zone2", "node", "node-y"), }, },