From b5228644e775dcf4e6db46aa746009d4e020f452 Mon Sep 17 00:00:00 2001
From: Clay Kauzlaric <ckauzlaric@vmware.com>
Date: Tue, 23 Apr 2024 11:33:36 -0400
Subject: [PATCH] refactor: don't error if gateway not found

* don't want to trigger re-reconciling if it doesn't exist
---
 pkg/reconciler/ingress/ingress.go      | 38 +++++++++++++++++---------
 pkg/reconciler/ingress/ingress_test.go |  5 ----
 2 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/pkg/reconciler/ingress/ingress.go b/pkg/reconciler/ingress/ingress.go
index aeaab80a9..87f3b7e8f 100644
--- a/pkg/reconciler/ingress/ingress.go
+++ b/pkg/reconciler/ingress/ingress.go
@@ -42,6 +42,14 @@ const (
 	notReconciledMessage = "Ingress reconciliation failed"
 )
 
+type GatewayNotFoundError struct {
+	Err error
+}
+
+func (gnfe *GatewayNotFoundError) Error() string {
+	return gnfe.Err.Error()
+}
+
 // Reconciler implements controller.Reconciler for Route resources.
 type Reconciler struct {
 	statusManager status.Manager
@@ -148,13 +156,19 @@ func (c *Reconciler) reconcileIngress(ctx context.Context, ing *v1alpha1.Ingress
 
 	// TODO: check Gateway readiness before reporting Ingress ready
 	if routesReady {
-		lbs, err := c.lookUpLoadBalancers(ing, pluginConfig)
-
+		externalLBs, internalLBs, err := c.lookUpLoadBalancers(ing, pluginConfig)
 		if err != nil {
 			ing.Status.MarkLoadBalancerNotReady()
-			return err
+			if _, ok := err.(*GatewayNotFoundError); ok {
+				// if we can't find a Gateway, we mark it as failed, and
+				// return no error, since there is no point in retrying
+				return nil
+			} else {
+				return err
+			}
 		}
-		ing.Status.MarkLoadBalancerReady(lbs[v1alpha1.IngressVisibilityExternalIP], lbs[v1alpha1.IngressVisibilityClusterLocal])
+
+		ing.Status.MarkLoadBalancerReady(externalLBs, internalLBs)
 	} else {
 		ing.Status.MarkLoadBalancerNotReady()
 	}
@@ -164,26 +178,23 @@ func (c *Reconciler) reconcileIngress(ctx context.Context, ing *v1alpha1.Ingress
 
 // lookUpLoadBalancers will return a map of visibilites to
 // LoadBalancerIngressStatuses for the current Gateways in use.
-func (c *Reconciler) lookUpLoadBalancers(ing *v1alpha1.Ingress, gpc *config.GatewayPlugin) (map[v1alpha1.IngressVisibility][]v1alpha1.LoadBalancerIngressStatus, error) {
+func (c *Reconciler) lookUpLoadBalancers(ing *v1alpha1.Ingress, gpc *config.GatewayPlugin) ([]v1alpha1.LoadBalancerIngressStatus, []v1alpha1.LoadBalancerIngressStatus, error) {
 	externalStatuses, err := c.collectLBIngressStatus(ing, gpc.ExternalGateway())
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
 	internalStatuses, err := c.collectLBIngressStatus(ing, gpc.LocalGateway())
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
-	return map[v1alpha1.IngressVisibility][]v1alpha1.LoadBalancerIngressStatus{
-		v1alpha1.IngressVisibilityExternalIP:   externalStatuses,
-		v1alpha1.IngressVisibilityClusterLocal: internalStatuses,
-	}, nil
+	return externalStatuses, internalStatuses, nil
 }
 
 // collectLBIngressStatus will return LoadBalancerIngressStatuses for the
 // provided single Gateway config. If a service is available on a Gateway, it will
-// return the address of the service.  Otherwise, it will return the first
+// return the address of the service. Otherwise, it will return the first
 // address in the Gateway status.
 func (c *Reconciler) collectLBIngressStatus(ing *v1alpha1.Ingress, gwc config.Gateway) ([]v1alpha1.LoadBalancerIngressStatus, error) {
 	statuses := []v1alpha1.LoadBalancerIngressStatus{}
@@ -207,8 +218,9 @@ func (c *Reconciler) collectLBIngressStatus(ing *v1alpha1.Ingress, gwc config.Ga
 						gwc.Name,
 					),
 				)
+				return nil, &GatewayNotFoundError{Err: err}
 			}
-			return nil, fmt.Errorf("could not find Gateway \"%s/%s\": %w", gwc.Namespace, gwc.Name, err)
+			return nil, fmt.Errorf("failed to get Gateway \"%s/%s\": %w", gwc.Namespace, gwc.Name, err)
 		}
 
 		if len(gw.Status.Addresses) > 0 {
diff --git a/pkg/reconciler/ingress/ingress_test.go b/pkg/reconciler/ingress/ingress_test.go
index 217a702eb..a9a2ec9e1 100644
--- a/pkg/reconciler/ingress/ingress_test.go
+++ b/pkg/reconciler/ingress/ingress_test.go
@@ -2309,7 +2309,6 @@ func TestReconcileProbingOffClusterGateway(t *testing.T) {
 			ing(withBasicSpec, withGatewayAPIclass, withFinalizer, withInitialConditions),
 			httpRoute(t, ing(withBasicSpec, withGatewayAPIclass), httpRouteReady),
 		}, servicesAndEndpoints...),
-		WantErr: true,
 		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{Object: ing(
 			withBasicSpec,
 			withGatewayAPIClass,
@@ -2318,11 +2317,7 @@ func TestReconcileProbingOffClusterGateway(t *testing.T) {
 				i.Status.InitializeConditions()
 				i.Status.MarkLoadBalancerNotReady()
 				i.Status.MarkNetworkConfigured()
-				i.Status.MarkIngressNotReady("ReconcileIngressFailed", "Ingress reconciliation failed")
 			})}},
-		WantEvents: []string{
-			Eventf(corev1.EventTypeWarning, "InternalError", `could not find Gateway "istio-system/istio-gateway": gateway.gateway.networking.k8s.io "istio-gateway" not found`),
-		},
 	}}
 
 	table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher) controller.Reconciler {