From 59e90f4ee0369c8371c6b6b3341c4fd72739fd14 Mon Sep 17 00:00:00 2001
From: Alexander Constantinescu <aconstantinescu@confluent.io>
Date: Thu, 4 Aug 2022 13:37:18 +0200
Subject: [PATCH] [CCM - service controller] Remove schedulability predicate
 for LB set

---
 .../controllers/service/controller.go         |   7 -
 .../controllers/service/controller_test.go    | 645 ------------------
 2 files changed, 652 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 fafd845241a..4c9a45510bc 100644
--- a/staging/src/k8s.io/cloud-provider/controllers/service/controller.go
+++ b/staging/src/k8s.io/cloud-provider/controllers/service/controller.go
@@ -984,13 +984,11 @@ type NodeConditionPredicate func(node *v1.Node) bool
 var (
 	allNodePredicates []NodeConditionPredicate = []NodeConditionPredicate{
 		nodeIncludedPredicate,
-		nodeSchedulablePredicate,
 		nodeUnTaintedPredicate,
 		nodeReadyPredicate,
 	}
 	etpLocalNodePredicates []NodeConditionPredicate = []NodeConditionPredicate{
 		nodeIncludedPredicate,
-		nodeSchedulablePredicate,
 		nodeUnTaintedPredicate,
 	}
 )
@@ -1008,11 +1006,6 @@ func nodeIncludedPredicate(node *v1.Node) bool {
 	return !hasExcludeBalancerLabel
 }
 
-// We consider the node for load balancing only when the node is schedulable.
-func nodeSchedulablePredicate(node *v1.Node) bool {
-	return !node.Spec.Unschedulable
-}
-
 // We consider the node for load balancing only when its not tainted for deletion by the cluster autoscaler.
 func nodeUnTaintedPredicate(node *v1.Node) bool {
 	for _, taint := range node.Spec.Taints {
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 b7877004b20..c6079a1b515 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
@@ -580,7 +580,6 @@ func TestNodeChangesForExternalTrafficPolicyLocalServices(t *testing.T) {
 	node2 := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node1"}, Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}}
 	node2NotReady := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node1"}, Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionFalse}}}}
 	node2Tainted := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node1"}, Spec: v1.NodeSpec{Taints: []v1.Taint{{Key: ToBeDeletedTaint}}}, Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionFalse}}}}
-	node2Unschedulable := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node1"}, Spec: v1.NodeSpec{Unschedulable: true}, Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}}
 	node2SpuriousChange := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node1"}, Status: v1.NodeStatus{Phase: v1.NodeTerminated, Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}}
 	node2Exclude := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node1", Labels: map[string]string{v1.LabelNodeExcludeBalancers: ""}}, Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}}
 	node3 := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node73"}, Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}}
@@ -670,20 +669,6 @@ func TestNodeChangesForExternalTrafficPolicyLocalServices(t *testing.T) {
 				{Service: service3, Hosts: []*v1.Node{node1, node3}},
 			},
 		},
-		{
-			desc:         "1 node goes unschedulable",
-			initialState: []*v1.Node{node1, node2, node3},
-			stateChanges: []stateChanges{
-				{
-					nodes: []*v1.Node{node1, node2Unschedulable, node3},
-				},
-			},
-			expectedUpdateCalls: []fakecloud.UpdateBalancerCall{
-				{Service: etpLocalservice1, Hosts: []*v1.Node{node1, node3}},
-				{Service: etpLocalservice2, Hosts: []*v1.Node{node1, node3}},
-				{Service: service3, Hosts: []*v1.Node{node1, node3}},
-			},
-		},
 		{
 			desc:         "1 node goes Ready",
 			initialState: []*v1.Node{node1, node2NotReady, node3},
@@ -1777,8 +1762,6 @@ func Test_respectsPredicates(t *testing.T) {
 		{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: false, 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: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}, ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{}}}},
 		{want: false, input: &v1.Node{Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}, ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{v1.LabelNodeExcludeBalancers: ""}}}},
 
@@ -1865,150 +1848,6 @@ func Test_shouldSyncUpdatedNode_individualPredicates(t *testing.T) {
 		newNode    *v1.Node
 		shouldSync bool
 	}{
-		{
-			name: "unschedulable F->T",
-			oldNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: false,
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionTrue,
-						},
-					},
-				},
-			},
-			newNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: true,
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionTrue,
-						},
-					},
-				},
-			},
-			shouldSync: true,
-		},
-		{
-			name: "unschedable T->F",
-			oldNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: true,
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionTrue,
-						},
-					},
-				},
-			},
-			newNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: false,
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionTrue,
-						},
-					},
-				},
-			},
-			shouldSync: true,
-		},
-		{
-			name: "unschedable T->T",
-			oldNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: true,
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionTrue,
-						},
-					},
-				},
-			},
-			newNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: true,
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionTrue,
-						},
-					},
-				},
-			},
-			shouldSync: false,
-		},
-		{
-			name: "unschedable F->F",
-			oldNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: false,
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionTrue,
-						},
-					},
-				},
-			},
-			newNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: false,
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionTrue,
-						},
-					},
-				},
-			},
-			shouldSync: false,
-		},
 		{
 			name: "taint F->T",
 			oldNode: &v1.Node{
@@ -2863,334 +2702,6 @@ func Test_shouldSyncUpdatedNode_compoundedPredicates(t *testing.T) {
 		newNode    *v1.Node
 		shouldSync bool
 	}{
-		{
-			name: "unschedable T, tainted F->T",
-			oldNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: true,
-					Taints:        []v1.Taint{},
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionTrue,
-						},
-					},
-				},
-			},
-			newNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: true,
-					Taints: []v1.Taint{
-						{
-							Key: ToBeDeletedTaint,
-						},
-					},
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionTrue,
-						},
-					},
-				},
-			},
-			shouldSync: false,
-		},
-		{
-			name: "unschedable T, tainted T->F",
-			oldNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: true,
-					Taints: []v1.Taint{
-						{
-							Key: ToBeDeletedTaint,
-						},
-					},
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionTrue,
-						},
-					},
-				},
-			},
-			newNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: true,
-					Taints:        []v1.Taint{},
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionTrue,
-						},
-					},
-				},
-			},
-			shouldSync: false,
-		},
-		{
-			name: "unschedable T, excluded T->F",
-			oldNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-					Labels: map[string]string{
-						v1.LabelNodeExcludeBalancers: "",
-					},
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: true,
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionTrue,
-						},
-					},
-				},
-			},
-			newNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:   "node",
-					Labels: map[string]string{},
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: true,
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionTrue,
-						},
-					},
-				},
-			},
-			shouldSync: true,
-		},
-		{
-			name: "unschedable T, excluded F->T",
-			oldNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:   "node",
-					Labels: map[string]string{},
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: true,
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionTrue,
-						},
-					},
-				},
-			},
-			newNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-					Labels: map[string]string{
-						v1.LabelNodeExcludeBalancers: "",
-					},
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: true,
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionTrue,
-						},
-					},
-				},
-			},
-			shouldSync: true,
-		},
-		{
-			name: "unschedable T, ready F->T",
-			oldNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: true,
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionFalse,
-						},
-					},
-				},
-			},
-			newNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: true,
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionTrue,
-						},
-					},
-				},
-			},
-			shouldSync: false,
-		},
-		{
-			name: "unschedable T, ready T->F",
-			oldNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: true,
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionTrue,
-						},
-					},
-				},
-			},
-			newNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: true,
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionFalse,
-						},
-					},
-				},
-			},
-			shouldSync: false,
-		},
-		{
-			name: "tainted T, unschedulable F->T",
-			oldNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: false,
-					Taints: []v1.Taint{
-						{
-							Key: ToBeDeletedTaint,
-						},
-					},
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionTrue,
-						},
-					},
-				},
-			},
-			newNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: true,
-					Taints: []v1.Taint{
-						{
-							Key: ToBeDeletedTaint,
-						},
-					},
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionTrue,
-						},
-					},
-				},
-			},
-			shouldSync: false,
-		},
-		{
-			name: "tainted T, unschedulable T->F",
-			oldNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: true,
-					Taints: []v1.Taint{
-						{
-							Key: ToBeDeletedTaint,
-						},
-					},
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionTrue,
-						},
-					},
-				},
-			},
-			newNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: false,
-					Taints: []v1.Taint{
-						{
-							Key: ToBeDeletedTaint,
-						},
-					},
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionTrue,
-						},
-					},
-				},
-			},
-			shouldSync: false,
-		},
 		{
 			name: "tainted T, excluded F->T",
 			oldNode: &v1.Node{
@@ -3375,90 +2886,6 @@ func Test_shouldSyncUpdatedNode_compoundedPredicates(t *testing.T) {
 			},
 			shouldSync: false,
 		},
-		{
-			name: "excluded T, unschedulable F->T",
-			oldNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-					Labels: map[string]string{
-						v1.LabelNodeExcludeBalancers: "",
-					},
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: false,
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionTrue,
-						},
-					},
-				},
-			},
-			newNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-					Labels: map[string]string{
-						v1.LabelNodeExcludeBalancers: "",
-					},
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: true,
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionTrue,
-						},
-					},
-				},
-			},
-			shouldSync: false,
-		},
-		{
-			name: "excluded T, unschedulable T->F",
-			oldNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-					Labels: map[string]string{
-						v1.LabelNodeExcludeBalancers: "",
-					},
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: true,
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionTrue,
-						},
-					},
-				},
-			},
-			newNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-					Labels: map[string]string{
-						v1.LabelNodeExcludeBalancers: "",
-					},
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: false,
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionTrue,
-						},
-					},
-				},
-			},
-			shouldSync: false,
-		},
 		{
 			name: "excluded T, tainted F->T",
 			oldNode: &v1.Node{
@@ -3623,78 +3050,6 @@ func Test_shouldSyncUpdatedNode_compoundedPredicates(t *testing.T) {
 			},
 			shouldSync: false,
 		},
-		{
-			name: "ready F, unschedulable F->T",
-			oldNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: false,
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionFalse,
-						},
-					},
-				},
-			},
-			newNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: true,
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionFalse,
-						},
-					},
-				},
-			},
-			shouldSync: false,
-		},
-		{
-			name: "ready F, unschedulable T->F",
-			oldNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: true,
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionFalse,
-						},
-					},
-				},
-			},
-			newNode: &v1.Node{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: "node",
-				},
-				Spec: v1.NodeSpec{
-					Unschedulable: false,
-				},
-				Status: v1.NodeStatus{
-					Conditions: []v1.NodeCondition{
-						{
-							Type:   v1.NodeReady,
-							Status: v1.ConditionFalse,
-						},
-					},
-				},
-			},
-			shouldSync: false,
-		},
 		{
 			name: "ready F, tainted F->T",
 			oldNode: &v1.Node{