Don't require failureDomains in PodAffinityChecker

failureDomains are only used for PreferredDuringScheduling pod
anti-affinity, which is ignored by PodAffinityChecker.
This unnecessary requirement was making it hard to move
PodAffinityChecker to GeneralPredicates because that would require
passing --failure-domains to both kubelet and kube-controller-manager.

plugin/pkg/scheduler/algorithm/predicates/BUILD

@@ -51,7 +51,6 @@ go_test(
         "//pkg/labels:go_default_library",
         "//pkg/util/codeinspector: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/gengo/parser",
         "//vendor:k8s.io/gengo/types",

plugin/pkg/scheduler/algorithm/predicates/predicates.go

@@ -17,6 +17,7 @@ limitations under the License.
 package predicates
 
 import (
+	"errors"
 	"fmt"
 	"math/rand"
 	"strconv"
@@ -859,16 +860,14 @@ func GeneralPredicates(pod *v1.Pod, meta interface{}, nodeInfo *schedulercache.N
 }
 
 type PodAffinityChecker struct {
-	info           NodeInfo
+	info      NodeInfo
-	podLister      algorithm.PodLister
+	podLister algorithm.PodLister
-	failureDomains priorityutil.Topologies
 }
 
-func NewPodAffinityPredicate(info NodeInfo, podLister algorithm.PodLister, failureDomains []string) algorithm.FitPredicate {
+func NewPodAffinityPredicate(info NodeInfo, podLister algorithm.PodLister) algorithm.FitPredicate {
 	checker := &PodAffinityChecker{
-		info:           info,
+		info:      info,
-		podLister:      podLister,
+		podLister: podLister,
-		failureDomains: priorityutil.Topologies{DefaultKeys: failureDomains},
 	}
 	return checker.InterPodAffinityMatches
 }
@@ -903,11 +902,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
 	for _, existingPod := range allPods {
 		match, err := priorityutil.PodMatchesTermsNamespaceAndSelector(existingPod, pod, term)
@@ -920,7 +922,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
 			}
 		}
@@ -1056,7 +1058,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

plugin/pkg/scheduler/algorithm/predicates/predicates_test.go

@@ -21,7 +21,6 @@ import (
 	"os/exec"
 	"path/filepath"
 	"reflect"
-	"strings"
 	"testing"
 
 	"k8s.io/gengo/parser"
@@ -30,7 +29,6 @@ import (
 	"k8s.io/kubernetes/pkg/api/v1"
 	"k8s.io/kubernetes/pkg/util/codeinspector"
 	"k8s.io/kubernetes/plugin/pkg/scheduler/algorithm"
-	priorityutil "k8s.io/kubernetes/plugin/pkg/scheduler/algorithm/priorities/util"
 	"k8s.io/kubernetes/plugin/pkg/scheduler/schedulercache"
 )
 
@@ -2463,9 +2461,8 @@ func TestInterPodAffinity(t *testing.T) {
 		}
 
 		fit := PodAffinityChecker{
-			info:           FakeNodeInfo(*node),
+			info:      FakeNodeInfo(*node),
-			podLister:      algorithm.FakePodLister(test.pods),
+			podLister: algorithm.FakePodLister(test.pods),
-			failureDomains: priorityutil.Topologies{DefaultKeys: strings.Split(v1.DefaultFailureDomains, ",")},
 		}
 		nodeInfo := schedulercache.NewNodeInfo(podsOnNode...)
 		nodeInfo.SetNode(test.node)
@@ -2708,9 +2705,8 @@ func TestInterPodAffinityWithMultipleNodes(t *testing.T) {
 			}
 
 			testFit := PodAffinityChecker{
-				info:           nodeListInfo,
+				info:      nodeListInfo,
-				podLister:      algorithm.FakePodLister(test.pods),
+				podLister: algorithm.FakePodLister(test.pods),
-				failureDomains: priorityutil.Topologies{DefaultKeys: strings.Split(v1.DefaultFailureDomains, ",")},
 			}
 			nodeInfo := schedulercache.NewNodeInfo(podsOnNode...)
 			nodeInfo.SetNode(&node)

plugin/pkg/scheduler/algorithm/priorities/util/topologies.go

@@ -52,8 +52,12 @@ func PodMatchesTermsNamespaceAndSelector(pod *v1.Pod, affinityPod *v1.Pod, term
 	return true, nil
 }
 
-// nodesHaveSameTopologyKeyInternal checks if nodeA and nodeB have same label value with given topologyKey as label key.
+// NodesHaveSameTopologyKey checks if nodeA and nodeB have same label value with given topologyKey as label key.
-func nodesHaveSameTopologyKeyInternal(nodeA, nodeB *v1.Node, topologyKey string) bool {
+// 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]
 }
 
@@ -67,12 +71,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)
 	}
 }

plugin/pkg/scheduler/algorithmprovider/defaults/defaults.go

@@ -139,7 +139,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)
 			},
 		),