Skip to content

Commit

Permalink
gateway-api: Retrieve LB service from same namespace
Browse files Browse the repository at this point in the history
[ upstream commit e8bed8d ]
[ 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: #31270
Signed-off-by: Tam Mach <tam.mach@cilium.io>
Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
  • Loading branch information
sayboras authored and gandro committed Mar 20, 2024
1 parent 40f4e73 commit 85ba486
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 31 deletions.
2 changes: 1 addition & 1 deletion operator/pkg/gateway-api/gateway_reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
69 changes: 39 additions & 30 deletions operator/pkg/gateway-api/gateway_reconcile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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
Expand Down Expand Up @@ -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)

Expand Down

0 comments on commit 85ba486

Please sign in to comment.