diff --git a/plugin/pkg/scheduler/algorithm/predicates/BUILD b/plugin/pkg/scheduler/algorithm/predicates/BUILD index 0c68819b94d..1cc5f3cfe74 100644 --- a/plugin/pkg/scheduler/algorithm/predicates/BUILD +++ b/plugin/pkg/scheduler/algorithm/predicates/BUILD @@ -43,7 +43,6 @@ go_test( deps = [ "//pkg/api/v1:go_default_library", "//plugin/pkg/scheduler/algorithm:go_default_library", - "//plugin/pkg/scheduler/algorithm/priorities/util:go_default_library", "//plugin/pkg/scheduler/schedulercache:go_default_library", "//vendor:k8s.io/apimachinery/pkg/api/resource", "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1", diff --git a/plugin/pkg/scheduler/algorithm/predicates/predicates.go b/plugin/pkg/scheduler/algorithm/predicates/predicates.go index c443964cb53..2ab01245eb3 100644 --- a/plugin/pkg/scheduler/algorithm/predicates/predicates.go +++ b/plugin/pkg/scheduler/algorithm/predicates/predicates.go @@ -17,6 +17,7 @@ limitations under the License. package predicates import ( + "errors" "fmt" "math/rand" "strconv" @@ -874,16 +875,14 @@ func GeneralPredicates(pod *v1.Pod, meta interface{}, nodeInfo *schedulercache.N } type PodAffinityChecker struct { - info NodeInfo - podLister algorithm.PodLister - failureDomains priorityutil.Topologies + info NodeInfo + podLister algorithm.PodLister } -func NewPodAffinityPredicate(info NodeInfo, podLister algorithm.PodLister, failureDomains []string) algorithm.FitPredicate { +func NewPodAffinityPredicate(info NodeInfo, podLister algorithm.PodLister) algorithm.FitPredicate { checker := &PodAffinityChecker{ - info: info, - podLister: podLister, - failureDomains: priorityutil.Topologies{DefaultKeys: failureDomains}, + info: info, + podLister: podLister, } return checker.InterPodAffinityMatches } @@ -915,11 +914,14 @@ func (c *PodAffinityChecker) InterPodAffinityMatches(pod *v1.Pod, meta interface return true, nil, nil } -// AnyPodMatchesPodAffinityTerm checks if any of given pods can match the specific podAffinityTerm. +// anyPodMatchesPodAffinityTerm checks if any of given pods can match the specific podAffinityTerm. // First return value indicates whether a matching pod exists on a node that matches the topology key, // while the second return value indicates whether a matching pod exists anywhere. // TODO: Do we really need any pod matching, or all pods matching? I think the latter. func (c *PodAffinityChecker) anyPodMatchesPodAffinityTerm(pod *v1.Pod, allPods []*v1.Pod, node *v1.Node, term *v1.PodAffinityTerm) (bool, bool, error) { + if len(term.TopologyKey) == 0 { + return false, false, errors.New("Empty topologyKey is not allowed except for PreferredDuringScheduling pod anti-affinity") + } matchingPodExists := false namespaces := priorityutil.GetNamespacesFromPodAffinityTerm(pod, term) selector, err := metav1.LabelSelectorAsSelector(term.LabelSelector) @@ -934,7 +936,7 @@ func (c *PodAffinityChecker) anyPodMatchesPodAffinityTerm(pod *v1.Pod, allPods [ if err != nil { return false, matchingPodExists, err } - if c.failureDomains.NodesHaveSameTopologyKey(node, existingPodNode, term.TopologyKey) { + if priorityutil.NodesHaveSameTopologyKey(node, existingPodNode, term.TopologyKey) { return true, matchingPodExists, nil } } @@ -1067,7 +1069,11 @@ func (c *PodAffinityChecker) satisfiesExistingPodsAntiAffinity(pod *v1.Pod, meta } } for _, term := range matchingTerms { - if c.failureDomains.NodesHaveSameTopologyKey(node, term.node, term.term.TopologyKey) { + if len(term.term.TopologyKey) == 0 { + glog.V(10).Infof("Empty topologyKey is not allowed except for PreferredDuringScheduling pod anti-affinity") + return false + } + if priorityutil.NodesHaveSameTopologyKey(node, term.node, term.term.TopologyKey) { glog.V(10).Infof("Cannot schedule pod %+v onto node %v,because of PodAntiAffinityTerm %v", podName(pod), node.Name, term.term) return false diff --git a/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go b/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go index a5bb47ba1b3..f5dec1e4545 100644 --- a/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go +++ b/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go @@ -19,14 +19,12 @@ package predicates import ( "fmt" "reflect" - "strings" "testing" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/kubernetes/pkg/api/v1" "k8s.io/kubernetes/plugin/pkg/scheduler/algorithm" - priorityutil "k8s.io/kubernetes/plugin/pkg/scheduler/algorithm/priorities/util" "k8s.io/kubernetes/plugin/pkg/scheduler/schedulercache" ) @@ -2572,9 +2570,8 @@ func TestInterPodAffinity(t *testing.T) { } fit := PodAffinityChecker{ - info: FakeNodeInfo(*node), - podLister: algorithm.FakePodLister(test.pods), - failureDomains: priorityutil.Topologies{DefaultKeys: strings.Split(v1.DefaultFailureDomains, ",")}, + info: FakeNodeInfo(*node), + podLister: algorithm.FakePodLister(test.pods), } nodeInfo := schedulercache.NewNodeInfo(podsOnNode...) nodeInfo.SetNode(test.node) @@ -2902,9 +2899,8 @@ func TestInterPodAffinityWithMultipleNodes(t *testing.T) { } testFit := PodAffinityChecker{ - info: nodeListInfo, - podLister: algorithm.FakePodLister(test.pods), - failureDomains: priorityutil.Topologies{DefaultKeys: strings.Split(v1.DefaultFailureDomains, ",")}, + info: nodeListInfo, + podLister: algorithm.FakePodLister(test.pods), } nodeInfo := schedulercache.NewNodeInfo(podsOnNode...) nodeInfo.SetNode(&node) diff --git a/plugin/pkg/scheduler/algorithm/priorities/util/topologies.go b/plugin/pkg/scheduler/algorithm/priorities/util/topologies.go index 3541707095d..fe6ea43fcc9 100644 --- a/plugin/pkg/scheduler/algorithm/priorities/util/topologies.go +++ b/plugin/pkg/scheduler/algorithm/priorities/util/topologies.go @@ -49,8 +49,12 @@ func PodMatchesTermsNamespaceAndSelector(pod *v1.Pod, namespaces sets.String, se return true } -// nodesHaveSameTopologyKeyInternal checks if nodeA and nodeB have same label value with given topologyKey as label key. -func nodesHaveSameTopologyKeyInternal(nodeA, nodeB *v1.Node, topologyKey string) bool { +// NodesHaveSameTopologyKey checks if nodeA and nodeB have same label value with given topologyKey as label key. +// Returns false if topologyKey is empty. +func NodesHaveSameTopologyKey(nodeA, nodeB *v1.Node, topologyKey string) bool { + if len(topologyKey) == 0 { + return false + } return nodeA.Labels != nil && nodeB.Labels != nil && len(nodeA.Labels[topologyKey]) > 0 && nodeA.Labels[topologyKey] == nodeB.Labels[topologyKey] } @@ -64,12 +68,12 @@ func (tps *Topologies) NodesHaveSameTopologyKey(nodeA, nodeB *v1.Node, topologyK if len(topologyKey) == 0 { // assumes this is allowed only for PreferredDuringScheduling pod anti-affinity (ensured by api/validation) for _, defaultKey := range tps.DefaultKeys { - if nodesHaveSameTopologyKeyInternal(nodeA, nodeB, defaultKey) { + if NodesHaveSameTopologyKey(nodeA, nodeB, defaultKey) { return true } } return false } else { - return nodesHaveSameTopologyKeyInternal(nodeA, nodeB, topologyKey) + return NodesHaveSameTopologyKey(nodeA, nodeB, topologyKey) } } diff --git a/plugin/pkg/scheduler/algorithmprovider/defaults/defaults.go b/plugin/pkg/scheduler/algorithmprovider/defaults/defaults.go index 0f124ef290c..a97da60e788 100644 --- a/plugin/pkg/scheduler/algorithmprovider/defaults/defaults.go +++ b/plugin/pkg/scheduler/algorithmprovider/defaults/defaults.go @@ -140,7 +140,7 @@ func defaultPredicates() sets.String { factory.RegisterFitPredicateFactory( "MatchInterPodAffinity", func(args factory.PluginFactoryArgs) algorithm.FitPredicate { - return predicates.NewPodAffinityPredicate(args.NodeInfo, args.PodLister, args.FailureDomains) + return predicates.NewPodAffinityPredicate(args.NodeInfo, args.PodLister) }, ),