Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 1 commit into from
Nov 8, 2018
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 0 additions & 2 deletions pkg/scheduler/factory/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down
63 changes: 1 addition & 62 deletions pkg/scheduler/factory/factory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -984,10 +984,7 @@ 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()
}
c.podQueue.MoveAllToActiveQueue()
}

func (c *configFactory) invalidateCachedPredicatesOnNodeUpdate(newNode *v1.Node, oldNode *v1.Node) {
Expand Down Expand Up @@ -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) {
Expand Down
129 changes: 0 additions & 129 deletions pkg/scheduler/factory/factory_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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)
}
}
}