Address comments and update bazel

kubernetes · Feb 24, 2017 · e0fcc78 · e0fcc78
1 parent 98e5b77
commit e0fcc78
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 16 deletions.
diff --git a/pkg/controller/daemon/BUILD b/pkg/controller/daemon/BUILD
@@ -26,6 +26,8 @@ go_library(
         "//pkg/client/listers/core/v1:go_default_library",
         "//pkg/client/listers/extensions/v1beta1:go_default_library",
         "//pkg/controller:go_default_library",
+        "//pkg/features:go_default_library",
+        "//pkg/kubelet/types:go_default_library",
         "//pkg/util/metrics:go_default_library",
         "//plugin/pkg/scheduler/algorithm:go_default_library",
         "//plugin/pkg/scheduler/algorithm/predicates:go_default_library",
@@ -37,6 +39,7 @@ go_library(
         "//vendor:k8s.io/apimachinery/pkg/util/errors",
         "//vendor:k8s.io/apimachinery/pkg/util/runtime",
         "//vendor:k8s.io/apimachinery/pkg/util/wait",
+        "//vendor:k8s.io/apiserver/pkg/util/feature",
         "//vendor:k8s.io/client-go/kubernetes/typed/core/v1",
         "//vendor:k8s.io/client-go/pkg/api/v1",
         "//vendor:k8s.io/client-go/tools/cache",
@@ -58,11 +61,13 @@ go_test(
         "//pkg/client/clientset_generated/clientset/fake:go_default_library",
         "//pkg/client/informers/informers_generated/externalversions:go_default_library",
         "//pkg/controller:go_default_library",
+        "//pkg/kubelet/types:go_default_library",
         "//pkg/securitycontext:go_default_library",
         "//vendor:k8s.io/apimachinery/pkg/api/resource",
         "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1",
         "//vendor:k8s.io/apimachinery/pkg/runtime",
         "//vendor:k8s.io/apiserver/pkg/storage/names",
+        "//vendor:k8s.io/apiserver/pkg/util/feature",
         "//vendor:k8s.io/client-go/testing",
         "//vendor:k8s.io/client-go/tools/cache",
         "//vendor:k8s.io/client-go/tools/record",

diff --git a/pkg/controller/daemon/daemoncontroller.go b/pkg/controller/daemon/daemoncontroller.go
@@ -752,6 +752,8 @@ func (dsc *DaemonSetsController) nodeShouldRunDaemonPod(node *v1.Node, ds *exten
 		if critical {
 			break
 		}
+		// TODO: There are other node status that the DaemonSet should ideally respect too,
+		//       e.g. MemoryPressure, and DiskPressure
 		if c.Type == v1.NodeOutOfDisk && c.Status == v1.ConditionTrue {
 			// the kubelet will evict this pod if it needs to. Let kubelet
 			// decide whether to continue running this pod so leave shouldContinueRunning

diff --git a/pkg/controller/daemon/daemoncontroller_test.go b/pkg/controller/daemon/daemoncontroller_test.go
@@ -810,8 +810,8 @@ func TestInsufficientCapacityNodeDaemonLaunchesCriticalPod(t *testing.T) {
 	syncAndValidateDaemonSets(t, manager, ds, podControl, 1, 0)
 }
 
-// DaemonSets should launch a critical pod even when there are port conflicts.
-func TestPortConflictNodeDaemonLaunchesCriticalPod(t *testing.T) {
+// DaemonSets should NOT launch a critical pod when there are port conflicts.
+func TestPortConflictNodeDaemonDoesNotLaunchCriticalPod(t *testing.T) {
 	podSpec := v1.PodSpec{
 		NodeName: "port-conflict",
 		Containers: []v1.Container{{
@@ -827,17 +827,12 @@ func TestPortConflictNodeDaemonLaunchesCriticalPod(t *testing.T) {
 		Spec: podSpec,
 	})
 
-	// Without enabling critical pod annotation feature gate, we shouldn't create critical pod
-	utilfeature.DefaultFeatureGate.Set("ExperimentalCriticalPodAnnotation=False")
+	utilfeature.DefaultFeatureGate.Set("ExperimentalCriticalPodAnnotation=True")
 	ds := newDaemonSet("critical")
 	ds.Spec.Template.Spec = podSpec
 	setDaemonSetCritical(ds)
 	manager.dsStore.Add(ds)
 	syncAndValidateDaemonSets(t, manager, ds, podControl, 0, 0)
-
-	// Enabling critical pod annotation feature gate should create critical pod
-	utilfeature.DefaultFeatureGate.Set("ExperimentalCriticalPodAnnotation=True")
-	syncAndValidateDaemonSets(t, manager, ds, podControl, 1, 0)
 }
 
 func setDaemonSetCritical(ds *extensions.DaemonSet) {

diff --git a/plugin/pkg/scheduler/algorithm/predicates/predicates.go b/plugin/pkg/scheduler/algorithm/predicates/predicates.go
@@ -876,14 +876,22 @@ func noncriticalPredicates(pod *v1.Pod, meta interface{}, nodeInfo *schedulercac
 		predicateFails = append(predicateFails, reasons...)
 	}
 
-	fit, reasons, err = PodFitsHost(pod, meta, nodeInfo)
+	return len(predicateFails) == 0, predicateFails, nil
+}
+
+// EssentialPredicates are the predicates that all pods, including critical pods, need
+func EssentialPredicates(pod *v1.Pod, meta interface{}, nodeInfo *schedulercache.NodeInfo) (bool, []algorithm.PredicateFailureReason, error) {
+	var predicateFails []algorithm.PredicateFailureReason
+	fit, reasons, err := PodFitsHost(pod, meta, nodeInfo)
 	if err != nil {
 		return false, predicateFails, err
 	}
 	if !fit {
 		predicateFails = append(predicateFails, reasons...)
 	}
 
+	// TODO: PodFitsHostPorts is essential for now, but kubelet should ideally
+	//       preempt pods to free up host ports too
 	fit, reasons, err = PodFitsHostPorts(pod, meta, nodeInfo)
 	if err != nil {
 		return false, predicateFails, err
@@ -892,13 +900,7 @@ func noncriticalPredicates(pod *v1.Pod, meta interface{}, nodeInfo *schedulercac
 		predicateFails = append(predicateFails, reasons...)
 	}
 
-	return len(predicateFails) == 0, predicateFails, nil
-}
-
-// EssentialPredicates are the predicates that all pods, including critical pods, need
-func EssentialPredicates(pod *v1.Pod, meta interface{}, nodeInfo *schedulercache.NodeInfo) (bool, []algorithm.PredicateFailureReason, error) {
-	var predicateFails []algorithm.PredicateFailureReason
-	fit, reasons, err := PodSelectorMatches(pod, meta, nodeInfo)
+	fit, reasons, err = PodSelectorMatches(pod, meta, nodeInfo)
 	if err != nil {
 		return false, predicateFails, err
 	}