envoyproxy / envoyproxy/gateway

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

Aperta
#5,447 8 commenti 0 reazioni 1 assegnatario Rivendicata da @nareddyt Vedi su GitHub
kind/bug stale
Lingua principale
Go
Stelle
3k
Fork
864
Merge medio
2g 2h
PR unite (30g)
140

Descrizione

## 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 }
}
```

Guida per i contributori

Nessuna guida per i contributori indicizzata per questo repository

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.