github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-08-31 08:36:16 +00:00
Code Issues Projects Releases Wiki Activity

KCCM: add providerID predicate to service controller

Browse Source
This commit is contained in:
Alexander Constantinescu
2023-04-16 01:00:05 +02:00
parent d3c103edce
commit 9c0153aef9
2 changed files with 102 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
staging/src/k8s.io/cloud-provider/controllers/service/controller.go
View File

@@ -674,6 +674,10 @@ func shouldSyncUpdatedNode(oldNode, newNode *v1.Node) bool {
if respectsPredicates(oldNode, nodeIncludedPredicate) != respectsPredicates(newNode, nodeIncludedPredicate) {
return true
}
// For the same reason as above, also check for changes to the providerID
if respectsPredicates(oldNode, nodeHasProviderIDPredicate) != respectsPredicates(newNode, nodeHasProviderIDPredicate) {
return true
}
if !utilfeature.DefaultFeatureGate.Enabled(features.StableLoadBalancerNodeSet) {
return respectsPredicates(oldNode, allNodePredicates...) != respectsPredicates(newNode, allNodePredicates...)
}
@@ -935,14 +939,17 @@ var (
nodeIncludedPredicate,
nodeUnTaintedPredicate,
nodeReadyPredicate,
nodeHasProviderIDPredicate,
}
etpLocalNodePredicates []NodeConditionPredicate = []NodeConditionPredicate{
nodeIncludedPredicate,
nodeUnTaintedPredicate,
nodeHasProviderIDPredicate,
}
stableNodeSetPredicates []NodeConditionPredicate = []NodeConditionPredicate{
nodeNotDeletedPredicate,
nodeIncludedPredicate,
nodeHasProviderIDPredicate,
// This is not perfect, but probably good enough. We won't update the
// LBs just because the taint was added (see shouldSyncUpdatedNode) but
// if any other situation causes an LB sync, tainted nodes will be
@@ -968,6 +975,10 @@ func nodeIncludedPredicate(node *v1.Node) bool {
return !hasExcludeBalancerLabel
}
func nodeHasProviderIDPredicate(node *v1.Node) bool {
return node.Spec.ProviderID != ""
}
// 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 {

101
staging/src/k8s.io/cloud-provider/controllers/service/controller_test.go
View File

@@ -471,9 +471,9 @@ func TestSyncLoadBalancerIfNeeded(t *testing.T) {
// TODO: Finish converting and update comments
func TestUpdateNodesInExternalLoadBalancer(t *testing.T) {
nodes := []*v1.Node{
{ObjectMeta: metav1.ObjectMeta{Name: "node0"}, Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}},
{ObjectMeta: metav1.ObjectMeta{Name: "node1"}, Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}},
{ObjectMeta: metav1.ObjectMeta{Name: "node73"}, Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}},
makeNode(tweakName("node1")),
makeNode(tweakName("node2")),
makeNode(tweakName("node3")),
}
table := []struct {
desc string
@@ -933,10 +933,10 @@ func TestNodeChangesForStableNodeSetEnabled(t *testing.T) {
}
func TestNodeChangesInExternalLoadBalancer(t *testing.T) {
node1 := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node1"}, Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}}
node2 := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node2"}, Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}}
node3 := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node3"}, Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}}
node4 := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node4"}, Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}}
node1 := makeNode(tweakName("node1"))
node2 := makeNode(tweakName("node2"))
node3 := makeNode(tweakName("node3"))
node4 := makeNode(tweakName("node4"))
services := []*v1.Service{
newService("s0", "777", v1.ServiceTypeLoadBalancer),
@@ -1992,9 +1992,10 @@ func Test_respectsPredicates(t *testing.T) {
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: true, input: &v1.Node{Spec: v1.NodeSpec{ProviderID: providerID}, 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.ConditionTrue}}}}},
{want: false, input: &v1.Node{Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionFalse}}}}},
{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: true, input: &v1.Node{Spec: v1.NodeSpec{ProviderID: providerID}, 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: ""}}}},
{want: false, input: &v1.Node{Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}},
@@ -2073,6 +2074,8 @@ func TestListWithPredicate(t *testing.T) {
}
}
var providerID = "providerID"
type nodeTweak func(n *v1.Node)
// TODO: use this pattern in all the tests above.
@@ -2083,7 +2086,8 @@ func makeNode(tweaks ...nodeTweak) *v1.Node {
Labels: map[string]string{},
},
Spec: v1.NodeSpec{
Taints: []v1.Taint{},
Taints: []v1.Taint{},
ProviderID: providerID,
},
Status: v1.NodeStatus{
Conditions: []v1.NodeCondition{{
@@ -2171,6 +2175,12 @@ func tweakDeleted() nodeTweak {
}
}
func tweakUnsetProviderID() nodeTweak {
return func(n *v1.Node) {
n.Spec.ProviderID = ""
}
}
func Test_shouldSyncUpdatedNode_individualPredicates(t *testing.T) {
testcases := []struct {
name string
@@ -2328,6 +2338,17 @@ func Test_shouldSyncUpdatedNode_individualPredicates(t *testing.T) {
newNode: makeNode(tweakDeleted()),
shouldSync: false,
stableNodeSetEnabled: true,
}, {
name: "providerID set F -> T",
oldNode: makeNode(tweakUnsetProviderID()),
newNode: makeNode(),
shouldSync: true,
}, {
name: "providerID set F -> T",
oldNode: makeNode(tweakUnsetProviderID()),
newNode: makeNode(),
shouldSync: true,
stableNodeSetEnabled: true,
}}
for _, testcase := range testcases {
t.Run(fmt.Sprintf("%s - StableLoadBalancerNodeSet: %v", testcase.name, testcase.stableNodeSetEnabled), func(t *testing.T) {
@@ -2358,6 +2379,16 @@ func Test_shouldSyncUpdatedNode_compoundedPredicates(t *testing.T) {
oldNode: makeNode(tweakAddTaint(ToBeDeletedTaint), tweakSetLabel(v1.LabelNodeExcludeBalancers, "")),
newNode: makeNode(tweakAddTaint(ToBeDeletedTaint)),
shouldSync: true,
}, {
name: "tainted T, providerID set F->T",
oldNode: makeNode(tweakAddTaint(ToBeDeletedTaint), tweakUnsetProviderID()),
newNode: makeNode(tweakAddTaint(ToBeDeletedTaint)),
shouldSync: true,
}, {
name: "tainted T, providerID set T->F",
oldNode: makeNode(tweakAddTaint(ToBeDeletedTaint)),
newNode: makeNode(tweakAddTaint(ToBeDeletedTaint), tweakUnsetProviderID()),
shouldSync: true,
}, {
name: "tainted T, ready F->T",
oldNode: makeNode(tweakAddTaint(ToBeDeletedTaint), tweakSetReady(false)),
@@ -2388,6 +2419,16 @@ func Test_shouldSyncUpdatedNode_compoundedPredicates(t *testing.T) {
oldNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, "")),
newNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, ""), tweakSetReady(false)),
shouldSync: false,
}, {
name: "excluded T, providerID set F->T",
oldNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, ""), tweakUnsetProviderID()),
newNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, "")),
shouldSync: true,
}, {
name: "excluded T, providerID set T->F",
oldNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, "")),
newNode: makeNode(tweakSetLabel(v1.LabelNodeExcludeBalancers, ""), tweakUnsetProviderID()),
shouldSync: true,
}, {
name: "ready F, tainted F->T",
oldNode: makeNode(tweakSetReady(false)),
@@ -2408,6 +2449,46 @@ func Test_shouldSyncUpdatedNode_compoundedPredicates(t *testing.T) {
oldNode: makeNode(tweakSetReady(false), tweakSetLabel(v1.LabelNodeExcludeBalancers, "")),
newNode: makeNode(tweakSetReady(false)),
shouldSync: true,
}, {
name: "ready F, providerID set F->T",
oldNode: makeNode(tweakSetReady(false), tweakUnsetProviderID()),
newNode: makeNode(tweakSetReady(false)),
shouldSync: true,
}, {
name: "ready F, providerID set T->F",
oldNode: makeNode(tweakSetReady(false)),
newNode: makeNode(tweakSetReady(false), tweakUnsetProviderID()),
shouldSync: true,
}, {
name: "providerID unset, tainted F->T",
oldNode: makeNode(tweakUnsetProviderID()),
newNode: makeNode(tweakUnsetProviderID(), tweakAddTaint(ToBeDeletedTaint)),
shouldSync: false,
}, {
name: "providerID unset, tainted T->F",
oldNode: makeNode(tweakUnsetProviderID(), tweakAddTaint(ToBeDeletedTaint)),
newNode: makeNode(tweakUnsetProviderID()),
shouldSync: false,
}, {
name: "providerID unset, excluded F->T",
oldNode: makeNode(tweakUnsetProviderID()),
newNode: makeNode(tweakUnsetProviderID(), tweakSetLabel(v1.LabelNodeExcludeBalancers, "")),
shouldSync: true,
}, {
name: "providerID unset, excluded T->F",
oldNode: makeNode(tweakUnsetProviderID(), tweakSetLabel(v1.LabelNodeExcludeBalancers, "")),
newNode: makeNode(tweakUnsetProviderID()),
shouldSync: true,
}, {
name: "providerID unset, ready F->T",
oldNode: makeNode(tweakUnsetProviderID()),
newNode: makeNode(tweakUnsetProviderID(), tweakSetReady(false)),
shouldSync: false,
}, {
name: "providerID unset, ready T->F",
oldNode: makeNode(tweakUnsetProviderID()),
newNode: makeNode(tweakUnsetProviderID(), tweakSetReady(true)),
shouldSync: false,
}}
for _, testcase := range testcases {
t.Run(fmt.Sprintf("%s - StableLoadBalancerNodeSet: %v", testcase.name, fgEnabled), func(t *testing.T) {