Skip to content

Commit

Permalink
refactor: don't error if gateway not found
Browse files Browse the repository at this point in the history
* don't want to trigger re-reconciling if it doesn't exist
  • Loading branch information
KauzClay committed Apr 23, 2024
1 parent aea6526 commit b522864
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 18 deletions.
38 changes: 25 additions & 13 deletions pkg/reconciler/ingress/ingress.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,14 @@ const (
notReconciledMessage = "Ingress reconciliation failed"
)

type GatewayNotFoundError struct {
Err error
}

func (gnfe *GatewayNotFoundError) Error() string {
return gnfe.Err.Error()

Check warning on line 50 in pkg/reconciler/ingress/ingress.go

View check run for this annotation

Codecov / codecov/patch

pkg/reconciler/ingress/ingress.go#L49-L50

Added lines #L49 - L50 were not covered by tests
}

// Reconciler implements controller.Reconciler for Route resources.
type Reconciler struct {
statusManager status.Manager
Expand Down Expand Up @@ -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 {

Check failure on line 162 in pkg/reconciler/ingress/ingress.go

View workflow job for this annotation

GitHub Actions / style / Golang / Lint

type assertion on error will fail on wrapped errors. Use errors.As to check for specific errors (errorlint)
// 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 {

Check warning on line 166 in pkg/reconciler/ingress/ingress.go

View workflow job for this annotation

GitHub Actions / style / Golang / Lint

indent-error-flow: if block ends with a return statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary) (revive)
return err
}
}
ing.Status.MarkLoadBalancerReady(lbs[v1alpha1.IngressVisibilityExternalIP], lbs[v1alpha1.IngressVisibilityClusterLocal])

ing.Status.MarkLoadBalancerReady(externalLBs, internalLBs)
} else {
ing.Status.MarkLoadBalancerNotReady()
}
Expand All @@ -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

Check warning on line 189 in pkg/reconciler/ingress/ingress.go

View check run for this annotation

Codecov / codecov/patch

pkg/reconciler/ingress/ingress.go#L189

Added line #L189 was not covered by tests
}

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{}
Expand All @@ -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)

Check warning on line 223 in pkg/reconciler/ingress/ingress.go

View check run for this annotation

Codecov / codecov/patch

pkg/reconciler/ingress/ingress.go#L223

Added line #L223 was not covered by tests
}

if len(gw.Status.Addresses) > 0 {
Expand Down
5 changes: 0 additions & 5 deletions pkg/reconciler/ingress/ingress_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 {
Expand Down

0 comments on commit b522864

Please sign in to comment.