Skip to content

Commit 6e572b5

Browse files
08volt08volt
committed
Enhance LoadBalancer status updates with conditions handling and refactor related functions
1 parent b97dd1f commit 6e572b5

File tree

16 files changed

+729
-36
lines changed

16 files changed

+729
-36
lines changed

‎cmd/glbc/main.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,7 @@ func main() {
365365
EnableL4ILBZonalAffinity: flags.F.EnableL4ILBZonalAffinity,
366366
EnableL4NetLBForwardingRulesOptimizations: flags.F.EnableL4NetLBForwardingRulesOptimizations,
367367
ReadOnlyMode: flags.F.ReadOnlyMode,
368+
EnableL4LBConditions: flags.F.EnableL4LBConditions,
368369
}
369370
ctx, err := ingctx.NewControllerContext(kubeClient, backendConfigClient, frontendConfigClient, firewallCRClient, svcNegClient, svcAttachmentClient, networkClient, nodeTopologyClient, eventRecorderKubeClient, cloud, namer, kubeSystemUID, ctxConfig, rootLogger)
370371
if err != nil {

‎pkg/context/context.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ type ControllerContextConfig struct {
138138
EnableL4ILBMixedProtocol bool
139139
EnableL4NetLBMixedProtocol bool
140140
EnableL4NetLBForwardingRulesOptimizations bool
141+
EnableL4LBConditions bool
141142
}
142143

143144
// NewControllerContext returns a new shared set of informers.

‎pkg/flags/flags.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,7 @@ var F = struct {
149149
EnableNEGsForIngress bool
150150
L4ILBLegacyHeadStartTime time.Duration
151151
EnableIPv6NodeNEGEndpoints bool
152+
EnableL4LBConditions bool
152153

153154
// ===============================
154155
// DEPRECATED FLAGS
@@ -362,6 +363,7 @@ L7 load balancing. CSV values accepted. Example: -node-port-ranges=80,8080,400-5
362363
flag.BoolVar(&F.EnableNEGsForIngress, "enable-negs-for-ingress", true, "Allow the NEG controller to create NEGs for Ingress services.")
363364
flag.DurationVar(&F.L4ILBLegacyHeadStartTime, "prevent-legacy-race-l4-ilb", 0*time.Second, "Delay before processing new L4 ILB services without existing finalizers. This gives the legacy controller a head start to claim the service, preventing a race condition upon service creation.")
364365
flag.BoolVar(&F.EnableIPv6NodeNEGEndpoints, "enable-ipv6-node-neg-endpoints", false, "Enable populating IPv6 addresses for Node IPs in GCE_VM_IP NEGs.")
366+
flag.BoolVar(&F.EnableL4LBConditions, "enable-l4lb-svc-conditions", false, "Enable Conditions on L4 LB Service status to reflect provisioned GCP resources.")
365367
}
366368

367369
func Validate() {

‎pkg/l4conditions/conditions.go‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
package l4conditions
2+
3+
import (
4+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
5+
)
6+
7+
const (
8+
BackendServiceConditionType = "ServiceLoadBalancerBackendService"
9+
10+
TCPForwardingRuleConditionType = "ServiceLoadBalancerTCPForwardingRule"
11+
UDPForwardingRuleConditionType = "ServiceLoadBalancerUDPForwardingRule"
12+
L3ForwardingRuleConditionType = "ServiceLoadBalancerL3ForwardingRule"
13+
14+
TCPIPv6ForwardingRuleConditionType = "ServiceLoadBalancerTCPIPv6ForwardingRule"
15+
UDPIPv6ForwardingRuleConditionType = "ServiceLoadBalancerUDPIPv6ForwardingRule"
16+
L3IPv6ForwardingRuleConditionType = "ServiceLoadBalancerL3IPv6ForwardingRule"
17+
18+
HealthCheckConditionType = "ServiceLoadBalancerHealthCheck"
19+
FirewallRuleConditionType = "ServiceLoadBalancerFirewallRule"
20+
IPv6FirewallRuleConditionType = "ServiceLoadBalancerIPv6FirewallRule"
21+
FirewallHealthCheckConditionType = "ServiceLoadBalancerFirewallRuleForHealthCheck"
22+
FirewallHealthCheckIPv6ConditionType = "ServiceLoadBalancerFirewallRuleForHealthCheckIPv6"
23+
24+
ConditionReason = "GCEResourceAllocated"
25+
)
26+
27+
func NewConditionResourceAllocated(conditionType string, resourceName string) metav1.Condition {
28+
return metav1.Condition{
29+
LastTransitionTime: metav1.Now(),
30+
Type: conditionType,
31+
Status: metav1.ConditionTrue,
32+
Reason: ConditionReason,
33+
Message: resourceName,
34+
}
35+
}
36+
37+
// NewBackendServiceCondition creates a condition for the backend service.
38+
func NewBackendServiceCondition(bsName string) metav1.Condition {
39+
return NewConditionResourceAllocated(BackendServiceConditionType, bsName)
40+
}
41+
42+
// NewTCPForwardingRuleCondition creates a condition for the TCP forwarding rule.
43+
func NewTCPForwardingRuleCondition(frName string) metav1.Condition {
44+
return NewConditionResourceAllocated(TCPForwardingRuleConditionType, frName)
45+
}
46+
47+
// NewUDPForwardingRuleCondition creates a condition for the UDP forwarding rule.
48+
func NewUDPForwardingRuleCondition(frName string) metav1.Condition {
49+
return NewConditionResourceAllocated(UDPForwardingRuleConditionType, frName)
50+
}
51+
52+
// NewL3ForwardingRuleCondition creates a condition for the L3 forwarding rule.
53+
func NewL3ForwardingRuleCondition(frName string) metav1.Condition {
54+
return NewConditionResourceAllocated(L3ForwardingRuleConditionType, frName)
55+
}
56+
57+
// NewTCPIPv6ForwardingRuleCondition creates a condition for the TCP forwarding rule.
58+
func NewTCPIPv6ForwardingRuleCondition(frName string) metav1.Condition {
59+
return NewConditionResourceAllocated(TCPIPv6ForwardingRuleConditionType, frName)
60+
}
61+
62+
// NewUDPIPv6ForwardingRuleCondition creates a condition for the UDP forwarding rule.
63+
func NewUDPIPv6ForwardingRuleCondition(frName string) metav1.Condition {
64+
return NewConditionResourceAllocated(UDPIPv6ForwardingRuleConditionType, frName)
65+
}
66+
67+
// NewL3IPv6ForwardingRuleCondition creates a condition for the L3 forwarding rule.
68+
func NewL3IPv6ForwardingRuleCondition(frName string) metav1.Condition {
69+
return NewConditionResourceAllocated(L3IPv6ForwardingRuleConditionType, frName)
70+
}
71+
72+
// NewHealthCheckCondition creates a condition for the health check.
73+
func NewHealthCheckCondition(hcName string) metav1.Condition {
74+
return NewConditionResourceAllocated(HealthCheckConditionType, hcName)
75+
}
76+
77+
// NewFirewallCondition creates a condition for the firewall.
78+
func NewFirewallCondition(fwName string) metav1.Condition {
79+
return NewConditionResourceAllocated(FirewallRuleConditionType, fwName)
80+
}
81+
82+
// NewIPv6FirewallCondition creates a condition for the IPv6 firewall.
83+
func NewIPv6FirewallCondition(fwName string) metav1.Condition {
84+
return NewConditionResourceAllocated(IPv6FirewallRuleConditionType, fwName)
85+
}
86+
87+
// NewFirewallHealthCheckCondition creates a condition for the firewall health check.
88+
func NewFirewallHealthCheckCondition(fwName string) metav1.Condition {
89+
return NewConditionResourceAllocated(FirewallHealthCheckConditionType, fwName)
90+
}
91+
92+
// NewFirewallHealthCheckIPv6Condition creates a condition for the firewall health check IPv6.
93+
func NewFirewallHealthCheckIPv6Condition(fwName string) metav1.Condition {
94+
return NewConditionResourceAllocated(FirewallHealthCheckIPv6ConditionType, fwName)
95+
}

‎pkg/l4lb/l4controller.go‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud"
2929
"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
3030
v1 "k8s.io/api/core/v1"
31+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3132
"k8s.io/apimachinery/pkg/types"
3233
"k8s.io/apimachinery/pkg/util/wait"
3334
"k8s.io/client-go/kubernetes"
@@ -390,7 +391,11 @@ func (l4c *L4Controller) processServiceCreateOrUpdate(service *v1.Service, svcLo
390391
syncResult.Error = err
391392
return syncResult
392393
}
393-
err = updateServiceStatus(l4c.ctx, service, syncResult.Status, svcLogger)
394+
expectedConditions := []metav1.Condition{}
395+
if l4c.ctx.EnableL4LBConditions {
396+
expectedConditions = syncResult.Conditions
397+
}
398+
err = updateServiceStatus(l4c.ctx, service, syncResult.Status, expectedConditions, svcLogger)
394399
if err != nil {
395400
l4c.ctx.Recorder(service.Namespace).Eventf(service, v1.EventTypeWarning, "SyncLoadBalancerFailed",
396401
"Error updating load balancer status: %v", err)
@@ -460,7 +465,7 @@ func (l4c *L4Controller) processServiceDeletion(key string, svc *v1.Service, svc
460465
// Reset the loadbalancer status first, before resetting annotations.
461466
// Other controllers(like service-controller) will process the service update if annotations change, but will ignore a service status change.
462467
// Following this order avoids a race condition when a service is changed from LoadBalancer type Internal to External.
463-
if err := updateServiceStatus(l4c.ctx, svc, &v1.LoadBalancerStatus{}, svcLogger); err != nil {
468+
if err := updateServiceStatus(l4c.ctx, svc, &v1.LoadBalancerStatus{}, []metav1.Condition{}, svcLogger); err != nil {
464469
l4c.ctx.Recorder(svc.Namespace).Eventf(svc, v1.EventTypeWarning, "DeleteLoadBalancer",
465470
"Error resetting load balancer status to empty: %v", err)
466471
result.Error = fmt.Errorf("failed to reset ILB status, err: %w", err)

‎pkg/l4lb/l4lbcommon.go‎

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -100,13 +100,21 @@ func deleteAnnotation(ctx *context.ControllerContext, svc *v1.Service, annotatio
100100
}
101101

102102
// updateServiceStatus this faction checks if LoadBalancer status changed and patch service if needed.
103-
func updateServiceStatus(ctx *context.ControllerContext, svc *v1.Service, newStatus *v1.LoadBalancerStatus, svcLogger klog.Logger) error {
104-
svcLogger.V(2).Info("Updating service status", "newStatus", fmt.Sprintf("%+v", newStatus))
105-
if helpers.LoadBalancerStatusEqual(&svc.Status.LoadBalancer, newStatus) {
106-
svcLogger.V(2).Info("New and old statuses are equal, skipping patch")
107-
return nil
103+
func updateServiceStatus(ctx *context.ControllerContext, svc *v1.Service, newStatus *v1.LoadBalancerStatus, newConditions []metav1.Condition, svcLogger klog.Logger) error {
104+
svcLogger.V(2).Info("Updating service status and conditions", "newStatus", fmt.Sprintf("%+v", newStatus), "newConditions", fmt.Sprintf("%+v", newConditions))
105+
106+
lbStatusEqual := helpers.LoadBalancerStatusEqual(&svc.Status.LoadBalancer, newStatus)
107+
lbConditionsEqual := conditionsEqual(svc.Status.Conditions, newConditions)
108+
109+
if !lbStatusEqual || !lbConditionsEqual {
110+
svcLogger.V(2).Info("Patching LoadBalancer status and Conditions", "newStatus", fmt.Sprintf("%+v", newStatus), "newConditions", fmt.Sprintf("%+v", newConditions))
111+
return patch.PatchServiceStatus(ctx.KubeClient.CoreV1(), svc, v1.ServiceStatus{
112+
LoadBalancer: *newStatus,
113+
Conditions: newConditions,
114+
})
108115
}
109-
return patch.PatchServiceLoadBalancerStatus(ctx.KubeClient.CoreV1(), svc, *newStatus)
116+
svcLogger.V(3).Info("Service status not changed, skipping patch for service")
117+
return nil
110118
}
111119

112120
// isHealthCheckDeleted checks if given health check exists in GCE
@@ -155,3 +163,24 @@ func finalizerWasRemovedUnexpectedly(oldService, newService *v1.Service, finaliz
155163
svcToBeDeleted := newService.ObjectMeta.DeletionTimestamp != nil
156164
return oldSvcHasLegacyFinalizer && !newSvcHasLegacyFinalizer && !svcToBeDeleted
157165
}
166+
167+
// conditionsEqual checks if load balancer conditions are equal
168+
func conditionsEqual(l, r []metav1.Condition) bool {
169+
if len(l) != len(r) {
170+
return false
171+
}
172+
lMap := make(map[string]metav1.Condition)
173+
for _, cond := range l {
174+
lMap[cond.Type] = cond
175+
}
176+
for _, condR := range r {
177+
condL, found := lMap[condR.Type]
178+
if !found {
179+
return false
180+
}
181+
if condL.Status != condR.Status || condL.Reason != condR.Reason || condL.Message != condR.Message {
182+
return false
183+
}
184+
}
185+
return true
186+
}

‎pkg/l4lb/l4lbcommon_test.go‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,3 +141,118 @@ func TestFinalizerWasRemovedUnexpectedly(t *testing.T) {
141141
})
142142
}
143143
}
144+
145+
func TestConditionsEqual(t *testing.T) {
146+
testCases := []struct {
147+
desc string
148+
left []metav1.Condition
149+
right []metav1.Condition
150+
expectedResult bool
151+
}{
152+
{
153+
desc: "Both empty",
154+
left: []metav1.Condition{},
155+
right: []metav1.Condition{},
156+
expectedResult: true,
157+
},
158+
{
159+
desc: "One nil, one empty",
160+
left: nil,
161+
right: []metav1.Condition{},
162+
expectedResult: true,
163+
},
164+
{
165+
desc: "Different lengths",
166+
left: []metav1.Condition{
167+
{Type: "Ready", Status: metav1.ConditionTrue},
168+
},
169+
right: []metav1.Condition{},
170+
expectedResult: false,
171+
},
172+
{
173+
desc: "Same conditions, same order",
174+
left: []metav1.Condition{
175+
{Type: "Ready", Status: metav1.ConditionTrue, Reason: "ReasonA", Message: "MsgA"},
176+
{Type: "Synced", Status: metav1.ConditionFalse, Reason: "ReasonB", Message: "MsgB"},
177+
},
178+
right: []metav1.Condition{
179+
{Type: "Ready", Status: metav1.ConditionTrue, Reason: "ReasonA", Message: "MsgA"},
180+
{Type: "Synced", Status: metav1.ConditionFalse, Reason: "ReasonB", Message: "MsgB"},
181+
},
182+
expectedResult: true,
183+
},
184+
{
185+
desc: "Same conditions, different order",
186+
left: []metav1.Condition{
187+
{Type: "Ready", Status: metav1.ConditionTrue},
188+
{Type: "Synced", Status: metav1.ConditionFalse},
189+
},
190+
right: []metav1.Condition{
191+
{Type: "Synced", Status: metav1.ConditionFalse},
192+
{Type: "Ready", Status: metav1.ConditionTrue},
193+
},
194+
expectedResult: true,
195+
},
196+
{
197+
desc: "Missing condition type in right",
198+
left: []metav1.Condition{
199+
{Type: "Ready", Status: metav1.ConditionTrue},
200+
{Type: "Synced", Status: metav1.ConditionTrue},
201+
},
202+
right: []metav1.Condition{
203+
{Type: "Ready", Status: metav1.ConditionTrue},
204+
{Type: "Other", Status: metav1.ConditionTrue},
205+
},
206+
expectedResult: false,
207+
},
208+
{
209+
desc: "Different Status",
210+
left: []metav1.Condition{
211+
{Type: "Ready", Status: metav1.ConditionTrue},
212+
},
213+
right: []metav1.Condition{
214+
{Type: "Ready", Status: metav1.ConditionFalse},
215+
},
216+
expectedResult: false,
217+
},
218+
{
219+
desc: "Different Reason",
220+
left: []metav1.Condition{
221+
{Type: "Ready", Status: metav1.ConditionTrue, Reason: "Foo"},
222+
},
223+
right: []metav1.Condition{
224+
{Type: "Ready", Status: metav1.ConditionTrue, Reason: "Bar"},
225+
},
226+
expectedResult: false,
227+
},
228+
{
229+
desc: "Different Message",
230+
left: []metav1.Condition{
231+
{Type: "Ready", Status: metav1.ConditionTrue, Message: "Foo"},
232+
},
233+
right: []metav1.Condition{
234+
{Type: "Ready", Status: metav1.ConditionTrue, Message: "Bar"},
235+
},
236+
expectedResult: false,
237+
},
238+
{
239+
desc: "Extra fields in left ignored (LastTransitionTime is not checked)",
240+
left: []metav1.Condition{
241+
{Type: "Ready", Status: metav1.ConditionTrue, LastTransitionTime: metav1.Now()},
242+
},
243+
right: []metav1.Condition{
244+
{Type: "Ready", Status: metav1.ConditionTrue},
245+
},
246+
expectedResult: true,
247+
},
248+
}
249+
250+
for _, tc := range testCases {
251+
t.Run(tc.desc, func(t *testing.T) {
252+
gotResult := conditionsEqual(tc.left, tc.right)
253+
if gotResult != tc.expectedResult {
254+
t.Errorf("conditionsEqual() = %v, expected %v", gotResult, tc.expectedResult)
255+
}
256+
})
257+
}
258+
}

‎pkg/l4lb/l4netlbcontroller.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929
"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
3030
"github.com/google/go-cmp/cmp"
3131
v1 "k8s.io/api/core/v1"
32+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3233
"k8s.io/apimachinery/pkg/types"
3334
"k8s.io/apimachinery/pkg/util/wait"
3435
"k8s.io/client-go/tools/cache"
@@ -685,7 +686,12 @@ func (lc *L4NetLBController) syncInternal(service *v1.Service, svcLogger klog.Lo
685686
return syncResult
686687
}
687688

688-
err = updateServiceStatus(lc.ctx, service, syncResult.Status, svcLogger)
689+
expectedConditions := []metav1.Condition{}
690+
if lc.ctx.EnableL4LBConditions {
691+
expectedConditions = syncResult.Conditions
692+
}
693+
694+
err = updateServiceStatus(lc.ctx, service, syncResult.Status, expectedConditions, svcLogger)
689695
if err != nil {
690696
lc.ctx.Recorder(service.Namespace).Eventf(service, v1.EventTypeWarning, "SyncExternalLoadBalancerFailed",
691697
"Error updating L4 External LoadBalancer, err: %v", err)
@@ -894,7 +900,7 @@ func (lc *L4NetLBController) garbageCollectRBSNetLB(key string, svc *v1.Service,
894900
return result
895901
}
896902

897-
if err := updateServiceStatus(lc.ctx, svc, &v1.LoadBalancerStatus{}, svcLogger); err != nil {
903+
if err := updateServiceStatus(lc.ctx, svc, &v1.LoadBalancerStatus{}, []metav1.Condition{}, svcLogger); err != nil {
898904
lc.ctx.Recorder(svc.Namespace).Eventf(svc, v1.EventTypeWarning, "DeleteLoadBalancer",
899905
"Error resetting L4 External LoadBalancer status to empty, err: %v", err)
900906
result.Error = fmt.Errorf("Failed to reset L4 External LoadBalancer status, err: %w", err)

0 commit comments

Comments
 (0)