Revert "Activate unschedulable pods only if the node became more schedulable"

pkg/scheduler/factory/BUILD

@@ -62,7 +62,6 @@ go_test(
     embed = [":go_default_library"],
     deps = [
         "//pkg/api/testing:go_default_library",
-        "//pkg/apis/core:go_default_library",
         "//pkg/scheduler/algorithm:go_default_library",
         "//pkg/scheduler/algorithm/priorities:go_default_library",
         "//pkg/scheduler/api:go_default_library",
@@ -73,7 +72,6 @@ go_test(
         "//pkg/scheduler/testing:go_default_library",
         "//pkg/scheduler/util:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",
-        "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",

pkg/scheduler/factory/factory.go

@@ -984,11 +984,8 @@ func (c *configFactory) updateNodeInCache(oldObj, newObj interface{}) {
 	}
 
 	c.invalidateCachedPredicatesOnNodeUpdate(newNode, oldNode)
-	// Only activate unschedulable pods if the node became more schedulable.
-	if nodeSchedulingPropertiesChanged(newNode, oldNode) {
 	c.podQueue.MoveAllToActiveQueue()
 }
-}
 
 func (c *configFactory) invalidateCachedPredicatesOnNodeUpdate(newNode *v1.Node, oldNode *v1.Node) {
 	if c.enableEquivalenceClassCache {
@@ -1059,64 +1056,6 @@ func (c *configFactory) invalidateCachedPredicatesOnNodeUpdate(newNode *v1.Node,
 	}
 }
 
-func nodeSchedulingPropertiesChanged(newNode *v1.Node, oldNode *v1.Node) bool {
-	if nodeAllocatableChanged(newNode, oldNode) {
-		glog.V(4).Infof("Allocatable resource of node %s changed", newNode.Name)
-		return true
-	}
-	if nodeLabelsChanged(newNode, oldNode) {
-		glog.V(4).Infof("Labels of node %s changed", newNode.Name)
-		return true
-	}
-	if nodeTaintsChanged(newNode, oldNode) {
-		glog.V(4).Infof("Taints of node %s changed", newNode.Name)
-		return true
-	}
-	if nodeConditionsChanged(newNode, oldNode) {
-		glog.V(4).Infof("Conditions of node %s changed", newNode.Name)
-		return true
-	}
-	if newNode.Spec.Unschedulable != oldNode.Spec.Unschedulable && newNode.Spec.Unschedulable == false {
-		glog.V(4).Infof("Node %s changed to schedulable", newNode.Name)
-		return true
-	}
-	return false
-}
-
-func nodeAllocatableChanged(newNode *v1.Node, oldNode *v1.Node) bool {
-	return !reflect.DeepEqual(oldNode.Status.Allocatable, newNode.Status.Allocatable)
-}
-
-func nodeLabelsChanged(newNode *v1.Node, oldNode *v1.Node) bool {
-	return !reflect.DeepEqual(oldNode.GetLabels(), newNode.GetLabels())
-}
-
-func nodeTaintsChanged(newNode *v1.Node, oldNode *v1.Node) bool {
-	if !reflect.DeepEqual(newNode.Spec.Taints, oldNode.Spec.Taints) {
-		return true
-	}
-	oldTaints, oldErr := helper.GetTaintsFromNodeAnnotations(oldNode.GetAnnotations())
-	if oldErr != nil {
-		// If parse old node's taint annotation failed, we assume node's taint changed.
-		glog.Errorf("Failed to get taints from annotation of old node %s: %v", oldNode.Name, oldErr)
-		return true
-	}
-	newTaints, newErr := helper.GetTaintsFromNodeAnnotations(newNode.GetAnnotations())
-	if newErr != nil {
-		// If parse new node's taint annotation failed, we assume node's taint changed.
-		glog.Errorf("Failed to get taints from annotation of new node %s: %v", newNode.Name, newErr)
-		return true
-	}
-	if !reflect.DeepEqual(oldTaints, newTaints) {
-		return true
-	}
-	return false
-}
-
-func nodeConditionsChanged(newNode *v1.Node, oldNode *v1.Node) bool {
-	return !reflect.DeepEqual(oldNode.Status.Conditions, newNode.Status.Conditions)
-}
-
 func (c *configFactory) deleteNodeFromCache(obj interface{}) {
 	var node *v1.Node
 	switch t := obj.(type) {

pkg/scheduler/factory/factory_test.go

@@ -24,7 +24,6 @@ import (
 	"time"
 
 	"k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/sets"
@@ -35,7 +34,6 @@ import (
 	clienttesting "k8s.io/client-go/testing"
 	"k8s.io/client-go/tools/cache"
 	apitesting "k8s.io/kubernetes/pkg/api/testing"
-	"k8s.io/kubernetes/pkg/apis/core"
 	"k8s.io/kubernetes/pkg/scheduler/algorithm"
 	schedulerapi "k8s.io/kubernetes/pkg/scheduler/api"
 	latestschedulerapi "k8s.io/kubernetes/pkg/scheduler/api/latest"
@@ -636,130 +634,3 @@ func testGetBinderFunc(expectedBinderType, podName string, extenders []algorithm
 		t.Errorf("Expected binder %q but got %q", expectedBinderType, binderType)
 	}
 }
-
-func TestNodeAllocatableChanged(t *testing.T) {
-	newQuantity := func(value int64) resource.Quantity {
-		return *resource.NewQuantity(value, resource.BinarySI)
-	}
-	for _, c := range []struct {
-		Changed        bool
-		OldAllocatable v1.ResourceList
-		NewAllocatable v1.ResourceList
-	}{
-		// No allocatable resources changed.
-		{
-			Changed:        false,
-			OldAllocatable: v1.ResourceList{v1.ResourceMemory: newQuantity(1024)},
-			NewAllocatable: v1.ResourceList{v1.ResourceMemory: newQuantity(1024)},
-		},
-		// New node has more allocatable resources.
-		{
-			Changed:        true,
-			OldAllocatable: v1.ResourceList{v1.ResourceMemory: newQuantity(1024)},
-			NewAllocatable: v1.ResourceList{v1.ResourceMemory: newQuantity(1024), v1.ResourceStorage: newQuantity(1024)},
-		},
-	} {
-		oldNode := &v1.Node{Status: v1.NodeStatus{Allocatable: c.OldAllocatable}}
-		newNode := &v1.Node{Status: v1.NodeStatus{Allocatable: c.NewAllocatable}}
-		changed := nodeAllocatableChanged(newNode, oldNode)
-		if changed != c.Changed {
-			t.Errorf("nodeAllocatableChanged should be %t, got %t", c.Changed, changed)
-		}
-	}
-}
-
-func TestNodeLabelsChanged(t *testing.T) {
-	for _, c := range []struct {
-		Changed   bool
-		OldLabels map[string]string
-		NewLabels map[string]string
-	}{
-		// No labels changed.
-		{Changed: false, OldLabels: map[string]string{"foo": "bar"}, NewLabels: map[string]string{"foo": "bar"}},
-		// Labels changed.
-		{Changed: true, OldLabels: map[string]string{"foo": "bar"}, NewLabels: map[string]string{"foo": "bar", "test": "value"}},
-	} {
-		oldNode := &v1.Node{ObjectMeta: metav1.ObjectMeta{Labels: c.OldLabels}}
-		newNode := &v1.Node{ObjectMeta: metav1.ObjectMeta{Labels: c.NewLabels}}
-		changed := nodeLabelsChanged(newNode, oldNode)
-		if changed != c.Changed {
-			t.Errorf("nodeLabelsChanged should be %t, got %t", c.Changed, changed)
-		}
-	}
-}
-
-func TestNodeTaintsChanged(t *testing.T) {
-	for _, c := range []struct {
-		Changed        bool
-		OldTaints      []v1.Taint
-		NewTaints      []v1.Taint
-		OldAnnotations map[string]string
-		NewAnnotations map[string]string
-	}{
-		// Taints use annotation and no change.
-		{
-			Changed:        false,
-			OldAnnotations: map[string]string{core.TaintsAnnotationKey: `[{"key":"value"}]`},
-			NewAnnotations: map[string]string{core.TaintsAnnotationKey: `[{"key":"value"}]`},
-		},
-		// Taints use annotation and changed.
-		{
-			Changed:        true,
-			OldAnnotations: map[string]string{core.TaintsAnnotationKey: `[{"key":"value1"}]`},
-			NewAnnotations: map[string]string{core.TaintsAnnotationKey: `[{"key":"value2"}]`},
-		},
-		// Taints use Spec.Taints and no change.
-		{
-			Changed:   false,
-			OldTaints: []v1.Taint{{Key: "key", Value: "value"}},
-			NewTaints: []v1.Taint{{Key: "key", Value: "value"}},
-		},
-		// Taints use Spec.Taints and changed.
-		{
-			Changed:   true,
-			OldTaints: []v1.Taint{{Key: "key", Value: "value1"}},
-			NewTaints: []v1.Taint{{Key: "key", Value: "value2"}},
-		},
-	} {
-		oldNode := &v1.Node{ObjectMeta: metav1.ObjectMeta{Annotations: c.OldAnnotations}, Spec: v1.NodeSpec{Taints: c.OldTaints}}
-		newNode := &v1.Node{ObjectMeta: metav1.ObjectMeta{Annotations: c.NewAnnotations}, Spec: v1.NodeSpec{Taints: c.NewTaints}}
-		changed := nodeTaintsChanged(newNode, oldNode)
-		if changed != c.Changed {
-			t.Errorf("nodeTaintsChanged should be %t, not %t", c.Changed, changed)
-		}
-	}
-}
-
-func TestNodeConditionsChanged(t *testing.T) {
-	for _, c := range []struct {
-		Changed       bool
-		OldConditions []v1.NodeCondition
-		NewConditions []v1.NodeCondition
-	}{
-		// No conditions changed.
-		{
-			Changed:       false,
-			OldConditions: []v1.NodeCondition{{Type: v1.NodeOutOfDisk, Status: v1.ConditionTrue}},
-			NewConditions: []v1.NodeCondition{{Type: v1.NodeOutOfDisk, Status: v1.ConditionTrue}},
-		},
-		// New node has more healthy conditions.
-		{
-			Changed:       true,
-			OldConditions: []v1.NodeCondition{},
-			NewConditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}},
-		},
-		// NodeReady False -> True
-		{
-			Changed:       true,
-			OldConditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionFalse}},
-			NewConditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}},
-		},
-	} {
-		oldNode := &v1.Node{Status: v1.NodeStatus{Conditions: c.OldConditions}}
-		newNode := &v1.Node{Status: v1.NodeStatus{Conditions: c.NewConditions}}
-		changed := nodeConditionsChanged(newNode, oldNode)
-		if changed != c.Changed {
-			t.Errorf("nodeConditionsChanged should be %t, got %t", c.Changed, changed)
-		}
-	}
-}