From 85ba486781744f60c5ae96b55dac998fb6ab1a1a Mon Sep 17 00:00:00 2001 From: Tam Mach Date: Sun, 10 Mar 2024 03:42:39 +0000 Subject: [PATCH] gateway-api: Retrieve LB service from same namespace [ upstream commit e8bed8d8343731422dc65b706c3617a40d0a8058 ] [ backporter notes: conflicts in unit tests. Had to change introduce the `NoError` check instead of expecting a specific error ] This commit is to add the same namespace while listing generated LB service, the main reason is to avoid wrong status update for gateways having the same name, but belonged to different namespace. Testing was done locally as per below: Before the fix: ``` $ kg gateway -A NAMESPACE NAME CLASS ADDRESS PROGRAMMED AGE another my-gateway cilium 10.110.222.237 True 4s default my-gateway cilium 10.110.222.237 True 56s ``` After the fix: ``` $ kg gateway -A NAMESPACE NAME CLASS ADDRESS PROGRAMMED AGE another my-gateway cilium 10.102.170.180 True 14m default my-gateway cilium 10.110.222.237 True 14m $ kg services -A NAMESPACE NAME TYPE CLUSTER-IP EXTERNAL-IP PORT(S) AGE another cilium-gateway-my-gateway LoadBalancer 10.102.170.180 10.102.170.180 80:31424/TCP 15m default cilium-gateway-my-gateway LoadBalancer 10.110.222.237 10.110.222.237 80:31889/TCP 15m ... ``` Fixes: https://github.com/cilium/cilium/issues/31270 Signed-off-by: Tam Mach Signed-off-by: Sebastian Wicki --- operator/pkg/gateway-api/gateway_reconcile.go | 2 +- .../pkg/gateway-api/gateway_reconcile_test.go | 69 +++++++++++-------- 2 files changed, 40 insertions(+), 31 deletions(-) diff --git a/operator/pkg/gateway-api/gateway_reconcile.go b/operator/pkg/gateway-api/gateway_reconcile.go index bc0050dd1f8b1..0c36a60ddd203 100644 --- a/operator/pkg/gateway-api/gateway_reconcile.go +++ b/operator/pkg/gateway-api/gateway_reconcile.go @@ -310,7 +310,7 @@ func (r *gatewayReconciler) setAddressStatus(ctx context.Context, gw *gatewayv1. svcList := &corev1.ServiceList{} if err := r.Client.List(ctx, svcList, client.MatchingLabels{ owningGatewayLabel: gw.GetName(), - }); err != nil { + }, client.InNamespace(gw.GetNamespace())); err != nil { return err } diff --git a/operator/pkg/gateway-api/gateway_reconcile_test.go b/operator/pkg/gateway-api/gateway_reconcile_test.go index d188275301442..13e77a491e6bf 100644 --- a/operator/pkg/gateway-api/gateway_reconcile_test.go +++ b/operator/pkg/gateway-api/gateway_reconcile_test.go @@ -39,7 +39,30 @@ var gwFixture = []client.Object{ Namespace: "default", }, }, - + &corev1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cilium-gateway-valid-gateway", + Namespace: "another-namespace", + Annotations: map[string]string{ + "pre-existing-annotation": "true", + }, + }, + Status: corev1.ServiceStatus{ + LoadBalancer: corev1.LoadBalancerStatus{ + Ingress: []corev1.LoadBalancerIngress{ + { + IP: "10.10.10.11", + Ports: []corev1.PortStatus{ + { + Port: 80, + Protocol: "TCP", + }, + }, + }, + }, + }, + }, + }, &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: "cilium-gateway-valid-gateway", @@ -48,6 +71,21 @@ var gwFixture = []client.Object{ "pre-existing-annotation": "true", }, }, + Status: corev1.ServiceStatus{ + LoadBalancer: corev1.LoadBalancerStatus{ + Ingress: []corev1.LoadBalancerIngress{ + { + IP: "10.10.10.10", + Ports: []corev1.PortStatus{ + { + Port: 80, + Protocol: "TCP", + }, + }, + }, + }, + }, + }, }, // Service in another namespace @@ -278,35 +316,6 @@ func Test_gatewayReconciler_Reconcile(t *testing.T) { result, err := r.Reconcile(context.Background(), ctrl.Request{NamespacedName: key}) // First reconcile should wait for LB status - require.Error(t, err) - require.Equal(t, "load balancer status is not ready", err.Error()) - require.Equal(t, ctrl.Result{}, result) - - // Simulate LB service update - lb := &corev1.Service{} - err = c.Get(context.Background(), client.ObjectKey{Namespace: "default", Name: "cilium-gateway-valid-gateway"}, lb) - require.NoError(t, err) - require.Equal(t, corev1.ServiceTypeLoadBalancer, lb.Spec.Type) - require.Equal(t, "valid-gateway", lb.Labels["io.cilium.gateway/owning-gateway"]) - require.Equal(t, "true", lb.Annotations["pre-existing-annotation"]) - - // Update LB status - lb.Status.LoadBalancer.Ingress = []corev1.LoadBalancerIngress{ - { - IP: "10.10.10.10", - Ports: []corev1.PortStatus{ - { - Port: 80, - Protocol: "TCP", - }, - }, - }, - } - err = c.Status().Update(context.Background(), lb) - require.NoError(t, err) - - // Perform second reconciliation - result, err = r.Reconcile(context.Background(), ctrl.Request{NamespacedName: key}) require.NoError(t, err) require.Equal(t, ctrl.Result{}, result)