Skip to content

Commit 779508f

Browse files
M00nF1shM00nF1sh
authored
add support for fail-open (kubernetes-sigs#2546)
1 parent 8bd91f7 commit 779508f

File tree

12 files changed

+2384
-1389
lines changed

12 files changed

+2384
-1389
lines changed

‎controllers/elbv2/eventhandlers/node.go‎

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -55,47 +55,54 @@ func (h *enqueueRequestsForNodeEvent) Generic(e event.GenericEvent, queue workqu
5555
// nothing to do here
5656
}
5757

58-
// enqueueImpactedEndpointBindings will enqueue all impacted TargetGroupBindings for node events.
58+
// enqueueImpactedTargetGroupBindings will enqueue all impacted TargetGroupBindings for node events.
5959
func (h *enqueueRequestsForNodeEvent) enqueueImpactedTargetGroupBindings(queue workqueue.RateLimitingInterface, nodeOld *corev1.Node, nodeNew *corev1.Node) {
6060
var nodeKey types.NamespacedName
61-
nodeOldIsReady := false
62-
nodeNewIsReady := false
61+
nodeOldSuitableAsTrafficProxy := false
62+
nodeNewSuitableAsTrafficProxy := false
63+
nodeOldReadyCondStatus := corev1.ConditionFalse
64+
nodeNewReadyCondStatus := corev1.ConditionFalse
6365
if nodeOld != nil {
6466
nodeKey = k8s.NamespacedName(nodeOld)
65-
nodeOldIsReady = k8s.IsNodeSuitableAsTrafficProxy(nodeOld)
67+
nodeOldSuitableAsTrafficProxy = backend.IsNodeSuitableAsTrafficProxy(nodeOld)
68+
if readyCond := k8s.GetNodeCondition(nodeOld, corev1.NodeReady); readyCond != nil {
69+
nodeOldReadyCondStatus = readyCond.Status
70+
}
6671
}
6772
if nodeNew != nil {
6873
nodeKey = k8s.NamespacedName(nodeNew)
69-
nodeNewIsReady = k8s.IsNodeSuitableAsTrafficProxy(nodeNew)
74+
nodeNewSuitableAsTrafficProxy = backend.IsNodeSuitableAsTrafficProxy(nodeNew)
75+
if readyCond := k8s.GetNodeCondition(nodeNew, corev1.NodeReady); readyCond != nil {
76+
nodeNewReadyCondStatus = readyCond.Status
77+
}
7078
}
7179

7280
tgbList := &elbv2api.TargetGroupBindingList{}
7381
if err := h.k8sClient.List(context.Background(), tgbList); err != nil {
74-
h.logger.Error(err, "failed to fetch targetGroupBindings")
82+
h.logger.Error(err, "[this should never happen] failed to fetch targetGroupBindings")
7583
return
7684
}
7785

7886
for _, tgb := range tgbList.Items {
7987
if tgb.Spec.TargetType == nil || (*tgb.Spec.TargetType) != elbv2api.TargetTypeInstance {
8088
continue
8189
}
82-
8390
nodeSelector, err := backend.GetTrafficProxyNodeSelector(&tgb)
8491
if err != nil {
8592
h.logger.Error(err, "failed to get nodeSelector", "TargetGroupBinding", tgb)
86-
return
93+
continue
8794
}
8895

89-
nodeOldIsTrafficProxy := false
90-
nodeNewIsTrafficProxy := false
96+
nodeOldSuitableAsTrafficProxyForTGB := false
97+
nodeNewSuitableAsTrafficProxyForTGB := false
9198
if nodeOld != nil {
92-
nodeOldIsTrafficProxy = nodeOldIsReady && nodeSelector.Matches(labels.Set(nodeOld.Labels))
99+
nodeOldSuitableAsTrafficProxyForTGB = nodeOldSuitableAsTrafficProxy && nodeSelector.Matches(labels.Set(nodeOld.Labels))
93100
}
94101
if nodeNew != nil {
95-
nodeNewIsTrafficProxy = nodeNewIsReady && nodeSelector.Matches(labels.Set(nodeNew.Labels))
102+
nodeNewSuitableAsTrafficProxyForTGB = nodeNewSuitableAsTrafficProxy && nodeSelector.Matches(labels.Set(nodeNew.Labels))
96103
}
97104

98-
if nodeOldIsTrafficProxy != nodeNewIsTrafficProxy {
105+
if h.shouldEnqueueTGBDueToNodeEvent(nodeOldSuitableAsTrafficProxyForTGB, nodeOldReadyCondStatus, nodeNewSuitableAsTrafficProxyForTGB, nodeNewReadyCondStatus) {
99106
h.logger.V(1).Info("enqueue targetGroupBinding for node event",
100107
"node", nodeKey,
101108
"targetGroupBinding", k8s.NamespacedName(&tgb),
@@ -109,3 +116,22 @@ func (h *enqueueRequestsForNodeEvent) enqueueImpactedTargetGroupBindings(queue w
109116
}
110117
}
111118
}
119+
120+
// shouldEnqueueTGBDueToNodeEvent checks whether a TGB should be queued due to node event.
121+
func (h *enqueueRequestsForNodeEvent) shouldEnqueueTGBDueToNodeEvent(
122+
nodeOldSuitableAsTrafficProxyForTGB bool, nodeOldReadyCondStatus corev1.ConditionStatus,
123+
nodeNewSuitableAsTrafficProxyForTGB bool, nodeNewReadyCondStatus corev1.ConditionStatus) bool {
124+
if nodeOldSuitableAsTrafficProxyForTGB == false && nodeNewSuitableAsTrafficProxyForTGB == false {
125+
return false
126+
}
127+
if nodeOldSuitableAsTrafficProxyForTGB == true && nodeNewSuitableAsTrafficProxyForTGB == true {
128+
return nodeOldReadyCondStatus != nodeNewReadyCondStatus
129+
}
130+
if nodeOldSuitableAsTrafficProxyForTGB == true && nodeNewSuitableAsTrafficProxyForTGB == false {
131+
return nodeOldReadyCondStatus != corev1.ConditionFalse
132+
}
133+
if nodeOldSuitableAsTrafficProxyForTGB == false && nodeNewSuitableAsTrafficProxyForTGB == true {
134+
return nodeNewReadyCondStatus != corev1.ConditionFalse
135+
}
136+
return false
137+
}

‎controllers/elbv2/eventhandlers/node_test.go‎

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
package eventhandlers
2+
3+
import (
4+
"github.com/stretchr/testify/assert"
5+
corev1 "k8s.io/api/core/v1"
6+
"testing"
7+
)
8+
9+
func Test_enqueueRequestsForNodeEvent_shouldEnqueueTGBDueToNodeEvent(t *testing.T) {
10+
type args struct {
11+
nodeOldSuitableAsTrafficProxyForTGB bool
12+
nodeOldReadyCondStatus corev1.ConditionStatus
13+
nodeNewSuitableAsTrafficProxyForTGB bool
14+
nodeNewReadyCondStatus corev1.ConditionStatus
15+
}
16+
tests := []struct {
17+
name string
18+
args args
19+
want bool
20+
}{
21+
{
22+
name: "suitable node changed from ready to notReady",
23+
args: args{
24+
nodeOldSuitableAsTrafficProxyForTGB: true,
25+
nodeOldReadyCondStatus: corev1.ConditionTrue,
26+
nodeNewSuitableAsTrafficProxyForTGB: true,
27+
nodeNewReadyCondStatus: corev1.ConditionFalse,
28+
},
29+
want: true,
30+
},
31+
{
32+
name: "suitable node changed from ready to unknown",
33+
args: args{
34+
nodeOldSuitableAsTrafficProxyForTGB: true,
35+
nodeOldReadyCondStatus: corev1.ConditionTrue,
36+
nodeNewSuitableAsTrafficProxyForTGB: true,
37+
nodeNewReadyCondStatus: corev1.ConditionUnknown,
38+
},
39+
want: true,
40+
},
41+
{
42+
name: "suitable node changed from notReady to ready",
43+
args: args{
44+
nodeOldSuitableAsTrafficProxyForTGB: true,
45+
nodeOldReadyCondStatus: corev1.ConditionFalse,
46+
nodeNewSuitableAsTrafficProxyForTGB: true,
47+
nodeNewReadyCondStatus: corev1.ConditionTrue,
48+
},
49+
want: true,
50+
},
51+
{
52+
name: "suitable node changed from notReady to unknown",
53+
args: args{
54+
nodeOldSuitableAsTrafficProxyForTGB: true,
55+
nodeOldReadyCondStatus: corev1.ConditionFalse,
56+
nodeNewSuitableAsTrafficProxyForTGB: true,
57+
nodeNewReadyCondStatus: corev1.ConditionUnknown,
58+
},
59+
want: true,
60+
},
61+
{
62+
name: "suitable node changed from unknown to ready",
63+
args: args{
64+
nodeOldSuitableAsTrafficProxyForTGB: true,
65+
nodeOldReadyCondStatus: corev1.ConditionUnknown,
66+
nodeNewSuitableAsTrafficProxyForTGB: true,
67+
nodeNewReadyCondStatus: corev1.ConditionTrue,
68+
},
69+
want: true,
70+
},
71+
{
72+
name: "suitable node changed from unknown to notReady",
73+
args: args{
74+
nodeOldSuitableAsTrafficProxyForTGB: true,
75+
nodeOldReadyCondStatus: corev1.ConditionUnknown,
76+
nodeNewSuitableAsTrafficProxyForTGB: true,
77+
nodeNewReadyCondStatus: corev1.ConditionFalse,
78+
},
79+
want: true,
80+
},
81+
{
82+
name: "suitable node remains ready",
83+
args: args{
84+
nodeOldSuitableAsTrafficProxyForTGB: true,
85+
nodeOldReadyCondStatus: corev1.ConditionTrue,
86+
nodeNewSuitableAsTrafficProxyForTGB: true,
87+
nodeNewReadyCondStatus: corev1.ConditionTrue,
88+
},
89+
want: false,
90+
},
91+
{
92+
name: "suitable node remains notReady",
93+
args: args{
94+
nodeOldSuitableAsTrafficProxyForTGB: true,
95+
nodeOldReadyCondStatus: corev1.ConditionFalse,
96+
nodeNewSuitableAsTrafficProxyForTGB: true,
97+
nodeNewReadyCondStatus: corev1.ConditionFalse,
98+
},
99+
want: false,
100+
},
101+
{
102+
name: "suitable node remains unknown",
103+
args: args{
104+
nodeOldSuitableAsTrafficProxyForTGB: true,
105+
nodeOldReadyCondStatus: corev1.ConditionUnknown,
106+
nodeNewSuitableAsTrafficProxyForTGB: true,
107+
nodeNewReadyCondStatus: corev1.ConditionUnknown,
108+
},
109+
want: false,
110+
},
111+
{
112+
name: "non-suitable node changed from ready to notReady",
113+
args: args{
114+
nodeOldSuitableAsTrafficProxyForTGB: false,
115+
nodeOldReadyCondStatus: corev1.ConditionTrue,
116+
nodeNewSuitableAsTrafficProxyForTGB: false,
117+
nodeNewReadyCondStatus: corev1.ConditionFalse,
118+
},
119+
want: false,
120+
},
121+
{
122+
name: "node became suitable while remains ready",
123+
args: args{
124+
nodeOldSuitableAsTrafficProxyForTGB: false,
125+
nodeOldReadyCondStatus: corev1.ConditionTrue,
126+
nodeNewSuitableAsTrafficProxyForTGB: true,
127+
nodeNewReadyCondStatus: corev1.ConditionTrue,
128+
},
129+
want: true,
130+
},
131+
{
132+
name: "node became suitable while remains notReady",
133+
args: args{
134+
nodeOldSuitableAsTrafficProxyForTGB: false,
135+
nodeOldReadyCondStatus: corev1.ConditionFalse,
136+
nodeNewSuitableAsTrafficProxyForTGB: true,
137+
nodeNewReadyCondStatus: corev1.ConditionFalse,
138+
},
139+
want: false,
140+
},
141+
{
142+
name: "node became non-suitable while remains ready",
143+
args: args{
144+
nodeOldSuitableAsTrafficProxyForTGB: true,
145+
nodeOldReadyCondStatus: corev1.ConditionTrue,
146+
nodeNewSuitableAsTrafficProxyForTGB: false,
147+
nodeNewReadyCondStatus: corev1.ConditionTrue,
148+
},
149+
want: true,
150+
},
151+
{
152+
name: "node became non-suitable while remains notReady",
153+
args: args{
154+
nodeOldSuitableAsTrafficProxyForTGB: true,
155+
nodeOldReadyCondStatus: corev1.ConditionFalse,
156+
nodeNewSuitableAsTrafficProxyForTGB: false,
157+
nodeNewReadyCondStatus: corev1.ConditionFalse,
158+
},
159+
want: false,
160+
},
161+
}
162+
for _, tt := range tests {
163+
t.Run(tt.name, func(t *testing.T) {
164+
h := &enqueueRequestsForNodeEvent{}
165+
got := h.shouldEnqueueTGBDueToNodeEvent(tt.args.nodeOldSuitableAsTrafficProxyForTGB, tt.args.nodeOldReadyCondStatus,
166+
tt.args.nodeNewSuitableAsTrafficProxyForTGB, tt.args.nodeNewReadyCondStatus)
167+
assert.Equal(t, tt.want, got)
168+
})
169+
}
170+
}

‎main.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,9 @@ func main() {
106106
vpcInfoProvider := networking.NewDefaultVPCInfoProvider(cloud.EC2(), ctrl.Log.WithName("vpc-info-provider"))
107107
subnetResolver := networking.NewDefaultSubnetsResolver(azInfoProvider, cloud.EC2(), cloud.VpcID(), controllerCFG.ClusterName, ctrl.Log.WithName("subnets-resolver"))
108108
tgbResManager := targetgroupbinding.NewDefaultResourceManager(mgr.GetClient(), cloud.ELBV2(), cloud.EC2(),
109-
podInfoRepo, sgManager, sgReconciler, cloud.VpcID(), controllerCFG.ClusterName, mgr.GetEventRecorderFor("targetGroupBinding"), ctrl.Log, controllerCFG.EnableEndpointSlices, controllerCFG.DisableRestrictedSGRules, vpcInfoProvider)
109+
podInfoRepo, sgManager, sgReconciler, vpcInfoProvider,
110+
cloud.VpcID(), controllerCFG.ClusterName, controllerCFG.FeatureGates.Enabled(config.EndpointsFailOpen), controllerCFG.EnableEndpointSlices, controllerCFG.DisableRestrictedSGRules,
111+
mgr.GetEventRecorderFor("targetGroupBinding"), ctrl.Log)
110112
backendSGProvider := networking.NewBackendSGProvider(controllerCFG.ClusterName, controllerCFG.BackendSecurityGroup,
111113
cloud.VpcID(), cloud.EC2(), mgr.GetClient(), controllerCFG.DefaultTags, ctrl.Log.WithName("backend-sg-provider"))
112114
ingGroupReconciler := ingress.NewGroupReconciler(cloud, mgr.GetClient(), mgr.GetEventRecorderFor("ingress"),

0 commit comments

Comments
 (0)