diff --git a/pkg/controller/service/servicecontroller.go b/pkg/controller/service/servicecontroller.go index ffe5972c562..8f238657487 100644 --- a/pkg/controller/service/servicecontroller.go +++ b/pkg/controller/service/servicecontroller.go @@ -581,13 +581,40 @@ func stringSlicesEqual(x, y []string) bool { } func hostsFromNodeList(list *api.NodeList) []string { - result := make([]string, len(list.Items)) + result := []string{} for ix := range list.Items { - result[ix] = list.Items[ix].Name + if list.Items[ix].Spec.Unschedulable { + continue + } + result = append(result, list.Items[ix].Name) } return result } +func getNodeConditionPredicate() cache.NodeConditionPredicate { + return func(node api.Node) bool { + // We add the master to the node list, but its unschedulable. So we use this to filter + // the master. + // TODO: Use a node annotation to indicate the master + if node.Spec.Unschedulable { + return false + } + // If we have no info, don't accept + if len(node.Status.Conditions) == 0 { + return false + } + for _, cond := range node.Status.Conditions { + // We consider the node for load balancing only when its NodeReady condition status + // is ConditionTrue + if cond.Type == api.NodeReady && cond.Status != api.ConditionTrue { + glog.V(4).Infof("Ignoring node %v with %v condition status %v", node.Name, cond.Type, cond.Status) + return false + } + } + return true + } +} + // nodeSyncLoop handles updating the hosts pointed to by all load // balancers whenever the set of nodes in the cluster changes. func (s *ServiceController) nodeSyncLoop(period time.Duration) { @@ -598,7 +625,7 @@ func (s *ServiceController) nodeSyncLoop(period time.Duration) { // something to compile, and gofmt1.4 complains about using `_ = range`. for now := range time.Tick(period) { _ = now - nodes, err := s.nodeLister.List() + nodes, err := s.nodeLister.NodeCondition(getNodeConditionPredicate()).List() if err != nil { glog.Errorf("Failed to retrieve current set of nodes from node lister: %v", err) continue diff --git a/pkg/controller/service/servicecontroller_test.go b/pkg/controller/service/servicecontroller_test.go index 4a03096b41b..54e7a119073 100644 --- a/pkg/controller/service/servicecontroller_test.go +++ b/pkg/controller/service/servicecontroller_test.go @@ -228,4 +228,102 @@ func TestUpdateNodesInExternalLoadBalancer(t *testing.T) { } } +func TestHostsFromNodeList(t *testing.T) { + tests := []struct { + nodes *api.NodeList + expectedHosts []string + }{ + { + nodes: &api.NodeList{}, + expectedHosts: []string{}, + }, + { + nodes: &api.NodeList{ + Items: []api.Node{ + { + ObjectMeta: api.ObjectMeta{Name: "foo"}, + Status: api.NodeStatus{Phase: api.NodeRunning}, + }, + { + ObjectMeta: api.ObjectMeta{Name: "bar"}, + Status: api.NodeStatus{Phase: api.NodeRunning}, + }, + }, + }, + expectedHosts: []string{"foo", "bar"}, + }, + { + nodes: &api.NodeList{ + Items: []api.Node{ + { + ObjectMeta: api.ObjectMeta{Name: "foo"}, + Status: api.NodeStatus{Phase: api.NodeRunning}, + }, + { + ObjectMeta: api.ObjectMeta{Name: "bar"}, + Status: api.NodeStatus{Phase: api.NodeRunning}, + }, + { + ObjectMeta: api.ObjectMeta{Name: "unschedulable"}, + Spec: api.NodeSpec{Unschedulable: true}, + Status: api.NodeStatus{Phase: api.NodeRunning}, + }, + }, + }, + expectedHosts: []string{"foo", "bar"}, + }, + } + + for _, test := range tests { + hosts := hostsFromNodeList(test.nodes) + if !reflect.DeepEqual(hosts, test.expectedHosts) { + t.Errorf("expected: %v, saw: %v", test.expectedHosts, hosts) + } + } +} + +func TestGetNodeConditionPredicate(t *testing.T) { + tests := []struct { + node api.Node + expectAccept bool + name string + }{ + { + node: api.Node{}, + expectAccept: false, + name: "empty", + }, + { + node: api.Node{ + Status: api.NodeStatus{ + Conditions: []api.NodeCondition{ + {Type: api.NodeReady, Status: api.ConditionTrue}, + }, + }, + }, + expectAccept: true, + name: "basic", + }, + { + node: api.Node{ + Spec: api.NodeSpec{Unschedulable: true}, + Status: api.NodeStatus{ + Conditions: []api.NodeCondition{ + {Type: api.NodeReady, Status: api.ConditionTrue}, + }, + }, + }, + expectAccept: false, + name: "unschedulable", + }, + } + pred := getNodeConditionPredicate() + for _, test := range tests { + accept := pred(test.node) + if accept != test.expectAccept { + t.Errorf("Test failed for %s, expected %v, saw %v", test.name, test.expectAccept, accept) + } + } +} + // TODO(a-robinson): Add tests for update/sync/delete.