[KCCM]: service controller - Add node.deletionTimestamp predicate

This commit is contained in:
Alexander Constantinescu 2023-03-13 12:01:06 +01:00
parent 0ccab9804a
commit 95df201d36
2 changed files with 59 additions and 14 deletions

View File

@ -943,6 +943,7 @@ var (
nodeUnTaintedPredicate,
}
stableNodeSetPredicates []NodeConditionPredicate = []NodeConditionPredicate{
nodeNotDeletedPredicate,
nodeIncludedPredicate,
// This is not perfect, but probably good enough. We won't update the
// LBs just because the taint was added (see shouldSyncUpdatedNode) but
@ -989,6 +990,10 @@ func nodeReadyPredicate(node *v1.Node) bool {
return false
}
func nodeNotDeletedPredicate(node *v1.Node) bool {
return node.DeletionTimestamp.IsZero()
}
// listWithPredicate gets nodes that matches all predicate functions.
func listWithPredicates(nodeLister corelisters.NodeLister, predicates ...NodeConditionPredicate) ([]*v1.Node, error) {
nodes, err := nodeLister.List(labels.Everything())

View File

@ -578,13 +578,13 @@ func TestUpdateNodesInExternalLoadBalancer(t *testing.T) {
func TestNodeChangesForExternalTrafficPolicyLocalServices(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StableLoadBalancerNodeSet, false)()
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}}}}
node2NotReady := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node2"}, Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionFalse}}}}
node2Tainted := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node2"}, Spec: v1.NodeSpec{Taints: []v1.Taint{{Key: ToBeDeletedTaint}}}, Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionFalse}}}}
node2SpuriousChange := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node2"}, Status: v1.NodeStatus{Phase: v1.NodeTerminated, Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}}
node2Exclude := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node2", 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: "node3"}, Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}}
node1 := makeNode(tweakName("node1"), tweakSetCondition(v1.NodeReady, v1.ConditionTrue))
node2 := makeNode(tweakName("node2"), tweakSetCondition(v1.NodeReady, v1.ConditionTrue))
node3 := makeNode(tweakName("node3"), tweakSetCondition(v1.NodeReady, v1.ConditionTrue))
node2NotReady := makeNode(tweakName("node2"), tweakSetCondition(v1.NodeReady, v1.ConditionFalse))
node2Tainted := makeNode(tweakName("node2"), tweakAddTaint(ToBeDeletedTaint), tweakSetCondition(v1.NodeReady, v1.ConditionTrue))
node2SpuriousChange := makeNode(tweakName("node2"), tweakAddTaint("Other"), tweakSetCondition(v1.NodeReady, v1.ConditionTrue))
node2Exclude := makeNode(tweakName("node2"), tweakSetLabel(v1.LabelNodeExcludeBalancers, ""), tweakSetCondition(v1.NodeReady, v1.ConditionTrue))
type stateChanges struct {
nodes []*v1.Node
@ -750,13 +750,14 @@ func TestNodeChangesForExternalTrafficPolicyLocalServices(t *testing.T) {
}
func TestNodeChangesForStableNodeSetEnabled(t *testing.T) {
node1 := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "node0"}, Status: v1.NodeStatus{Conditions: []v1.NodeCondition{{Type: v1.NodeReady, Status: v1.ConditionTrue}}}}
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}}}}
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}}}}
node1 := makeNode(tweakName("node1"), tweakSetCondition(v1.NodeReady, v1.ConditionTrue))
node2 := makeNode(tweakName("node2"), tweakSetCondition(v1.NodeReady, v1.ConditionTrue))
node3 := makeNode(tweakName("node3"), tweakSetCondition(v1.NodeReady, v1.ConditionTrue))
node2NotReady := makeNode(tweakName("node2"), tweakSetCondition(v1.NodeReady, v1.ConditionFalse))
node2Tainted := makeNode(tweakName("node2"), tweakAddTaint(ToBeDeletedTaint), tweakSetCondition(v1.NodeReady, v1.ConditionTrue))
node2SpuriousChange := makeNode(tweakName("node2"), tweakAddTaint("Other"), tweakSetCondition(v1.NodeReady, v1.ConditionTrue))
node2Exclude := makeNode(tweakName("node2"), tweakSetLabel(v1.LabelNodeExcludeBalancers, ""), tweakSetCondition(v1.NodeReady, v1.ConditionTrue))
node2Deleted := makeNode(tweakName("node2"), tweakDeleted())
type stateChanges struct {
nodes []*v1.Node
@ -879,6 +880,20 @@ func TestNodeChangesForStableNodeSetEnabled(t *testing.T) {
{Service: service3, Hosts: []*v1.Node{node1, node2}},
},
},
{
desc: "1 node marked for deletion",
initialState: []*v1.Node{node1, node2, node3},
stateChanges: []stateChanges{
{
nodes: []*v1.Node{node1, node2Deleted, 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 spurious node update",
initialState: []*v1.Node{node1, node2, node3},
@ -2084,6 +2099,12 @@ func makeNode(tweaks ...nodeTweak) *v1.Node {
return n
}
func tweakName(name string) nodeTweak {
return func(n *v1.Node) {
n.Name = name
}
}
func tweakAddTaint(key string) nodeTweak {
return func(n *v1.Node) {
n.Spec.Taints = append(n.Spec.Taints, v1.Taint{Key: key})
@ -2142,6 +2163,14 @@ func tweakUnsetCondition(condType v1.NodeConditionType) nodeTweak {
}
}
func tweakDeleted() nodeTweak {
return func(n *v1.Node) {
n.DeletionTimestamp = &metav1.Time{
Time: time.Now(),
}
}
}
func Test_shouldSyncUpdatedNode_individualPredicates(t *testing.T) {
testcases := []struct {
name string
@ -2288,6 +2317,17 @@ func Test_shouldSyncUpdatedNode_individualPredicates(t *testing.T) {
oldNode: makeNode(tweakSetCondition("Other", v1.ConditionTrue)),
newNode: makeNode(tweakSetCondition("Other", v1.ConditionFalse)),
shouldSync: false,
}, {
name: "deletionTimestamp F -> T",
oldNode: makeNode(),
newNode: makeNode(tweakDeleted()),
shouldSync: false,
}, {
name: "deletionTimestamp F -> T",
oldNode: makeNode(),
newNode: makeNode(tweakDeleted()),
shouldSync: false,
stableNodeSetEnabled: true,
}}
for _, testcase := range testcases {
t.Run(fmt.Sprintf("%s - StableLoadBalancerNodeSet: %v", testcase.name, testcase.stableNodeSetEnabled), func(t *testing.T) {