From 1f010cc42bb1e26d20e6c9729c78c9820f5519b2 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Wed, 6 May 2020 18:22:17 -0400 Subject: [PATCH] Service load balancers should include unschedulable nodes In 16443 a set of changes were made to enable service load balancers for Kube 1.1 and GCP to exclude unschedulable nodes from load balancer pools. However, Unschedulable has nothing to do with whether nodes should host service load balancer pods - the act of preventing new pods from landing on a node has no impact on existing workloads. In general nodes are not special, and a user who wants to exclude otherwise healthy nodes can use the beta service load balancer label to exclude the node from the LB pool. This commit removes the check for unschedulable nodes from the LB pool. --- .../k8s.io/cloud-provider/controllers/service/controller.go | 6 ------ .../cloud-provider/controllers/service/controller_test.go | 5 +++++ 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/staging/src/k8s.io/cloud-provider/controllers/service/controller.go b/staging/src/k8s.io/cloud-provider/controllers/service/controller.go index 05695475662..797553a8cad 100644 --- a/staging/src/k8s.io/cloud-provider/controllers/service/controller.go +++ b/staging/src/k8s.io/cloud-provider/controllers/service/controller.go @@ -635,12 +635,6 @@ func nodeSlicesEqualForLB(x, y []*v1.Node) bool { func (s *Controller) getNodeConditionPredicate() NodeConditionPredicate { return func(node *v1.Node) bool { - // We add the master to the node list, but its unschedulable. So we use this to filter - // the master. - if node.Spec.Unschedulable { - return false - } - if s.legacyNodeRoleFeatureEnabled { // As of 1.6, we will taint the master, but not necessarily mark it unschedulable. // Recognize nodes labeled as master, and filter them also, as we were doing previously. diff --git a/staging/src/k8s.io/cloud-provider/controllers/service/controller_test.go b/staging/src/k8s.io/cloud-provider/controllers/service/controller_test.go index b893c0d64df..353b47f2abd 100644 --- a/staging/src/k8s.io/cloud-provider/controllers/service/controller_test.go +++ b/staging/src/k8s.io/cloud-provider/controllers/service/controller_test.go @@ -1379,6 +1379,11 @@ func Test_getNodeConditionPredicate(t *testing.T) { input *v1.Node want bool }{ + {want: false, input: &v1.Node{}}, + {want: true, input: &v1.Node{Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}}}, + {want: false, input: &v1.Node{Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionFalse}}}}}, + {want: true, input: &v1.Node{Spec: v1.NodeSpec{Unschedulable: true}, Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}}}, + {want: true, input: &v1.Node{Status: validNodeStatus, ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{}}}}, {want: true, input: &v1.Node{Status: validNodeStatus, ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{labelNodeRoleMaster: ""}}}}, {want: true, input: &v1.Node{Status: validNodeStatus, ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{labelNodeRoleExcludeBalancer: ""}}}},