envoy: exit if envoy exits #2240
Conversation
|
Code Climate has analyzed commit 53c22e8 and detected 0 issues on this pull request. View more on Code Climate. |
| } else { | ||
| args = append(args, "--use-dynamic-base-id", "--base-id-path", baseIDPath) | ||
| } | ||
| srv.restartEpoch++ | ||
|
|
||
| cmd := exec.Command(srv.envoyPath, args...) // #nosec |
There was a problem hiding this comment.
=> exec.CommandContext for consistency
There was a problem hiding this comment.
We want the background context. The context passed to run comes from the on change handler. But envoy needs to survive past the end of this function.
| @@ -213,6 +218,13 @@ func (srv *Server) run(ctx context.Context) error { | |||
| if err != nil { | |||
| return fmt.Errorf("error starting envoy: %w", err) | |||
| } | |||
| // call Wait to avoid zombie processes | |||
| go func() { _ = cmd.Wait() }() | |||
There was a problem hiding this comment.
I'm not sure why monitorProcessCancel has to be inside Server - you can just wait for the process completion in this goroutine, and invoke monitorProcessCancel there.
There was a problem hiding this comment.
When we update envoy config we start a new envoy process using hot-reload. Once hot-reload completes it will kill the previous envoy process. If we don't cancel the previous process monitor, we will immediately exit when this happens.
We have to cancel the process monitor before the process exits.
Summary
If envoy exits unexpectedly, we should use
log.Fatalto exit as well. This PR also fixes a bug with hot-restart. Therestart-epochwas supposed to start at 1 for the first restart, not 0.Related issues
Fixes https://github.com/pomerium/internal/issues/414
Checklist
improvement/bug/ etc)