From 623e7dfa39ce71a511ab6383f4fb80112b3012f3 Mon Sep 17 00:00:00 2001 From: Abhishek Gupta Date: Fri, 3 Jun 2016 14:35:50 -0700 Subject: [PATCH 1/3] Considering all nodes for the scheduler cache to allow lookups --- plugin/pkg/scheduler/factory/factory.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/plugin/pkg/scheduler/factory/factory.go b/plugin/pkg/scheduler/factory/factory.go index 1a93c2b19fb..f02d7fa877b 100644 --- a/plugin/pkg/scheduler/factory/factory.go +++ b/plugin/pkg/scheduler/factory/factory.go @@ -449,6 +449,11 @@ func getNodeConditionPredicate() cache.NodeConditionPredicate { return false } } + // Ignore nodes that are marked unschedulable + if node.Spec.Unschedulable { + glog.V(4).Infof("Ignoring node %v since it is unschedulable", node.Name) + return false + } return true } } @@ -470,9 +475,10 @@ func (factory *ConfigFactory) createAssignedNonTerminatedPodLW() *cache.ListWatc // createNodeLW returns a cache.ListWatch that gets all changes to nodes. func (factory *ConfigFactory) createNodeLW() *cache.ListWatch { - // TODO: Filter out nodes that doesn't have NodeReady condition. - fields := fields.Set{api.NodeUnschedulableField: "false"}.AsSelector() - return cache.NewListWatchFromClient(factory.Client, "nodes", api.NamespaceAll, fields) + // all nodes are considered to ensure that the scheduler cache has access to all nodes for lookups + // the NodeCondition is used to filter out the nodes that are not ready or unschedulable + // the filtered list is used as the super set of nodes to consider for scheduling + return cache.NewListWatchFromClient(factory.Client, "nodes", api.NamespaceAll, fields.ParseSelectorOrDie("")) } // createPersistentVolumeLW returns a cache.ListWatch that gets all changes to persistentVolumes. From bc9c461402ea064501890d5d382c3270b15b5235 Mon Sep 17 00:00:00 2001 From: Abhishek Gupta Date: Tue, 7 Jun 2016 11:29:38 -0700 Subject: [PATCH 2/3] Adding test case for scheduler NodeConditionPredicate --- plugin/pkg/scheduler/factory/factory_test.go | 43 ++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/plugin/pkg/scheduler/factory/factory_test.go b/plugin/pkg/scheduler/factory/factory_test.go index 4369b40eb55..8439a5540b5 100644 --- a/plugin/pkg/scheduler/factory/factory_test.go +++ b/plugin/pkg/scheduler/factory/factory_test.go @@ -430,3 +430,46 @@ func TestInvalidFactoryArgs(t *testing.T) { } } + +func TestNodeConditionPredicate(t *testing.T) { + nodeFunc := getNodeConditionPredicate() + nodeList := &api.NodeList{ + Items: []api.Node{ + // node1 considered + {ObjectMeta: api.ObjectMeta{Name: "node1"}, Status: api.NodeStatus{Conditions: []api.NodeCondition{{Type: api.NodeReady, Status: api.ConditionTrue}}}}, + // node2 ignored - node not Ready + {ObjectMeta: api.ObjectMeta{Name: "node2"}, Status: api.NodeStatus{Conditions: []api.NodeCondition{{Type: api.NodeReady, Status: api.ConditionFalse}}}}, + // node3 ignored - node out of disk + {ObjectMeta: api.ObjectMeta{Name: "node3"}, Status: api.NodeStatus{Conditions: []api.NodeCondition{{Type: api.NodeOutOfDisk, Status: api.ConditionTrue}}}}, + // node4 considered + {ObjectMeta: api.ObjectMeta{Name: "node4"}, Status: api.NodeStatus{Conditions: []api.NodeCondition{{Type: api.NodeOutOfDisk, Status: api.ConditionFalse}}}}, + + // node5 ignored - node out of disk + {ObjectMeta: api.ObjectMeta{Name: "node5"}, Status: api.NodeStatus{Conditions: []api.NodeCondition{{Type: api.NodeReady, Status: api.ConditionTrue}, {Type: api.NodeOutOfDisk, Status: api.ConditionTrue}}}}, + // node6 considered + {ObjectMeta: api.ObjectMeta{Name: "node6"}, Status: api.NodeStatus{Conditions: []api.NodeCondition{{Type: api.NodeReady, Status: api.ConditionTrue}, {Type: api.NodeOutOfDisk, Status: api.ConditionFalse}}}}, + // node7 ignored - node out of disk, node not Ready + {ObjectMeta: api.ObjectMeta{Name: "node7"}, Status: api.NodeStatus{Conditions: []api.NodeCondition{{Type: api.NodeReady, Status: api.ConditionFalse}, {Type: api.NodeOutOfDisk, Status: api.ConditionTrue}}}}, + // node8 ignored - node not Ready + {ObjectMeta: api.ObjectMeta{Name: "node8"}, Status: api.NodeStatus{Conditions: []api.NodeCondition{{Type: api.NodeReady, Status: api.ConditionFalse}, {Type: api.NodeOutOfDisk, Status: api.ConditionFalse}}}}, + + // node9 ignored - node unschedulable + {ObjectMeta: api.ObjectMeta{Name: "node9"}, Spec: api.NodeSpec{Unschedulable: true}}, + // node10 considered + {ObjectMeta: api.ObjectMeta{Name: "node10"}, Spec: api.NodeSpec{Unschedulable: false}}, + // node11 considered + {ObjectMeta: api.ObjectMeta{Name: "node11"}}, + }, + } + + nodeNames := []string{} + for _, node := range nodeList.Items { + if nodeFunc(node) { + nodeNames = append(nodeNames, node.Name) + } + } + expectedNodes := []string{"node1", "node4", "node6", "node10", "node11"} + if !reflect.DeepEqual(expectedNodes, nodeNames) { + t.Errorf("expected: %v, got %v", expectedNodes, nodeNames) + } +} From f12f7c51ce72dec77147c65676aa022a8c66436b Mon Sep 17 00:00:00 2001 From: Abhishek Gupta Date: Wed, 8 Jun 2016 12:07:12 -0700 Subject: [PATCH 3/3] Modifying scheduler integration test for unschedulable nodes --- test/integration/scheduler_test.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test/integration/scheduler_test.go b/test/integration/scheduler_test.go index ff7b64b44e9..81883c5f3b4 100644 --- a/test/integration/scheduler_test.go +++ b/test/integration/scheduler_test.go @@ -170,8 +170,11 @@ func DoTestUnschedulableNodes(t *testing.T, restClient *client.Client, nodeStore t.Fatalf("Failed to update node with unschedulable=true: %v", err) } err = waitForReflection(t, s, nodeKey, func(node interface{}) bool { - // An unschedulable node should get deleted from the store - return node == nil + // An unschedulable node should still be present in the store + // Nodes that are unschedulable or that are not ready or + // have their disk full (Node.Spec.Conditions) are exluded + // based on NodeConditionPredicate, a separate check + return node != nil && node.(*api.Node).Spec.Unschedulable == true }) if err != nil { t.Fatalf("Failed to observe reflected update for setting unschedulable=true: %v", err)