envoyproxy / envoyproxy/gateway

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

オープン
#5,447 コメント 8 件 リアクション 0 件 担当者 1 名 @nareddyt が担当を希望しています GitHub で見る
kind/bug stale
主要言語
Go
スター
3k
フォーク
864
平均マージ
2日 2時間
マージ済み PR(30日)
140

説明

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

コントリビューションガイド

このリポジトリのコントリビューションガイドは索引されていません

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。