envoyproxy / envoyproxy/gateway

Correctly handle errors on `Runner::Start()`

Abierto
#5,447 8 comentarios 0 reacciones 1 asignado Reclamado por @nareddyt Ver en GitHub
kind/bug stale
Lenguaje dominante
Go
Estrellas
3k
Forks
864
Merge medio
2 d 2 h
PR fusionados (30 d)
140

Descripción

## Problem

During Envoy Gateway startup, some `Runners` that start via goroutines don't handle startup errors correctly. They do error checks but only log the error. This results in the Envoy Gateway container starting, but with some runners not started, so Envoy Gateway's functionality is degraded.

Some examples of this pattern are below

#### Kubernetes Provider Runner

Example: Envoy Gateway pod doesn't have correct `RoleBindings` on resources. This goroutine will exit with the error, Kubernetes provider never starts successfully, but the error is masked and Envoy Gateway continues to run.

https://github.com/envoyproxy/gateway/blob/8b6d3ea71d455bd556a7c1901619a9226dd845ec/internal/provider/runner/runner.go#L63-L67

#### xDS Server Runner

Example: xDS server fails to listen on port because of a port conflict. The xDS server will never start up, but Envoy Gateway container will continue to run.

- Error source: https://github.com/envoyproxy/gateway/blob/main/internal/xds/server/runner/runner.go#L112
- Error masked: https://github.com/envoyproxy/gateway/blob/main/internal/xds/server/runner/runner.go#L100

#### Gateway API Runner - WASM

Any error in `startWasmCache` is masked here.

https://github.com/envoyproxy/gateway/blob/8b6d3ea71d455bd556a7c1901619a9226dd845ec/internal/gatewayapi/runner/runner.go#L84

## Proposed Solution

Currently, `Start()` only takes in a `context.Context`. We should pass around an additional `errgroup.Group` that each runner can use to run functions in a goroutine.

```
func (r *Runner) Start(ctx context.Context, g errgroup.Group) (err error) {
g.Go(func() error {
return r.serveXdsServer(ctx)
})
return nil
}

func (r *Runner) serveXdsServer(ctx context.Context) error {
addr := net.JoinHostPort(XdsServerAddress, strconv.Itoa(bootstrap.DefaultXdsServerPort))
l, err := net.Listen("tcp", addr)
if err != nil {
return fmt.Errorf("failed to listen on address %q: %v", addr, err)
}
...
}
```

Caller can use `Wait()` functionality in the errorgroup

```
func setupRunners(ctx context.Context, cfg *config.Server) (err error) {
g, ctx := errorgroup.WithContext(ctx)
...
if err := xdsRunner.Start(ctx, g); err != nil { return err }
...
// Instead of: <-ctx.Done()
if err != g.Wait(); err != nil { return err }
}
```

Guía de contribución

No hay ninguna guía de contribución indexada para este repositorio

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.