diff --git a/pkg/scheduler/factory/BUILD b/pkg/scheduler/factory/BUILD index 9c3a25dd73..dfade874b3 100644 --- a/pkg/scheduler/factory/BUILD +++ b/pkg/scheduler/factory/BUILD @@ -62,6 +62,7 @@ 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", @@ -72,6 +73,7 @@ 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", diff --git a/pkg/scheduler/factory/factory.go b/pkg/scheduler/factory/factory.go index 2300e43ca7..2d0801480f 100644 --- a/pkg/scheduler/factory/factory.go +++ b/pkg/scheduler/factory/factory.go @@ -983,7 +983,10 @@ func (c *configFactory) updateNodeInCache(oldObj, newObj interface{}) { } c.invalidateCachedPredicatesOnNodeUpdate(newNode, oldNode) - c.podQueue.MoveAllToActiveQueue() + // 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) { @@ -1055,6 +1058,64 @@ 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) { diff --git a/pkg/scheduler/factory/factory_test.go b/pkg/scheduler/factory/factory_test.go index 346d964110..2e7d4256e7 100644 --- a/pkg/scheduler/factory/factory_test.go +++ b/pkg/scheduler/factory/factory_test.go @@ -24,6 +24,7 @@ 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" @@ -34,6 +35,7 @@ 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" @@ -634,3 +636,130 @@ 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) + } + } +}