envoyproxy / envoyproxy/gateway
Correctly handle errors on `Runner::Start()`
- Dominant language
- Go
- Stars
- 3k
- Forks
- 864
- Avg merge
- 2d 2h
- Merged PRs (30d)
- 140
Description
## 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 }
}
```
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.