From 3882c45190de8717e93ffe73bb51779772ce7480 Mon Sep 17 00:00:00 2001 From: Takeshi Yoneda Date: Tue, 18 Mar 2025 14:04:45 -0700 Subject: [PATCH 1/2] ExitChannel to allow to wait for envoy process to exit Signed-off-by: Takeshi Yoneda --- api/run.go | 14 +++++++++++++ api/run_test.go | 41 +++++++++++++++++++------------------ internal/envoy/run.go | 1 + internal/globals/globals.go | 2 ++ 4 files changed, 38 insertions(+), 20 deletions(-) diff --git a/api/run.go b/api/run.go index 08f63252..ccd250a7 100644 --- a/api/run.go +++ b/api/run.go @@ -61,6 +61,14 @@ func Out(out io.Writer) RunOption { } } +// ExitChannel is a channel that a goroutine running "envoy" will send an empty struct to when it exits. +// This can be used to synchronize with the envoy process to perform additional cleanup. +func ExitChannel(exitCh chan struct{}) RunOption { + return func(o *runOpts) { + o.exitCh = exitCh + } +} + // RunOption is configuration for Run. type RunOption func(*runOpts) @@ -68,11 +76,16 @@ type runOpts struct { homeDir string envoyVersion string envoyVersionsURL string + exitCh chan struct{} out io.Writer } // Run downloads Envoy and runs it as a process with the arguments // passed to it. Use RunOption for configuration options. +// +// This will block until the process exits or the context is done. However, +// the Envoy process itself is running another goroutine, so to wait for the +// process to exit, use ExitChannel to synchronize with the envoy process exit. func Run(ctx context.Context, args []string, options ...RunOption) error { ro := &runOpts{ homeDir: globals.DefaultHomeDir, @@ -89,6 +102,7 @@ func Run(ctx context.Context, args []string, options ...RunOption) error { EnvoyVersion: version.PatchVersion(ro.envoyVersion), EnvoyVersionsURL: ro.envoyVersion, Out: ro.out, + RunOpts: globals.RunOpts{ExitCh: ro.exitCh}, } funcECmd := cmd.NewApp(&o) diff --git a/api/run_test.go b/api/run_test.go index d43a10b8..1fe5ba5a 100644 --- a/api/run_test.go +++ b/api/run_test.go @@ -19,6 +19,7 @@ import ( "context" "os" "path/filepath" + "strconv" "testing" "time" @@ -28,31 +29,31 @@ import ( "github.com/tetratelabs/func-e/internal/version" ) -var ( - runArgs = []string{"--version"} -) - func TestRunWithCtxDone(t *testing.T) { - tmpDir := t.TempDir() envoyVersion := version.LastKnownEnvoy versionsServer := test.RequireEnvoyVersionsTestServer(t, envoyVersion) defer versionsServer.Close() envoyVersionsURL := versionsServer.URL + "/envoy-versions.json" - b := bytes.NewBufferString("") - - require.Equal(t, 0, b.Len()) - - ctx := context.Background() - // Use a very small ctx timeout - ctx, cancel := context.WithTimeout(ctx, 1*time.Second) - defer cancel() - err := Run(ctx, runArgs, Out(b), HomeDir(tmpDir), EnvoyVersionsURL(envoyVersionsURL)) - require.NoError(t, err) - - require.NotEqual(t, 0, b.Len()) - _, err = os.Stat(filepath.Join(tmpDir, "versions")) - require.NoError(t, err) + // Run the same test multiple times to ensure that the Envoy process is cleaned up properly with the context cancellation + // in conjunction with the exit channel. + for i := range 5 { + t.Run(strconv.Itoa(i), func(t *testing.T) { + exitCh := make(chan struct{}, 1) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + out := &bytes.Buffer{} + // This will return right after the context is done, but the Envoy process itself is running another goroutine, + // so without using the exit channel, we might end up existing the test (or main program) before the Envoy process receives + // the signal to exit, hence it might end up being a zombie process. + err := Run(ctx, []string{ + "--config-yaml", "admin: {address: {socket_address: {address: '127.0.0.1', port_value: 9901}}}", + }, Out(out), HomeDir(tmpDir), EnvoyVersionsURL(envoyVersionsURL), ExitChannel(exitCh)) + require.NoError(t, err) + require.NotContains(t, out.String(), "Address already in use") + <-exitCh // Wait for the Envoy process to completely exit. + }) + } } func TestRunToCompletion(t *testing.T) { @@ -71,7 +72,7 @@ func TestRunToCompletion(t *testing.T) { ctx, cancel := context.WithTimeout(ctx, 1000*time.Minute) defer cancel() - err := Run(ctx, runArgs, Out(b), HomeDir(tmpDir), EnvoyVersionsURL(envoyVersionsURL)) + err := Run(ctx, []string{"--version"}, Out(b), HomeDir(tmpDir), EnvoyVersionsURL(envoyVersionsURL)) require.NoError(t, err) require.NotEqual(t, 0, b.Len()) diff --git a/internal/envoy/run.go b/internal/envoy/run.go index adeea22d..82df9c6a 100644 --- a/internal/envoy/run.go +++ b/internal/envoy/run.go @@ -62,6 +62,7 @@ func (r *Runtime) Run(ctx context.Context, args []string) error { go func() { defer waitCancel() _ = r.cmd.Wait() // Envoy logs like "caught SIGINT" or "caught ENVOY_SIGTERM", so we don't repeat logging here. + r.opts.ExitCh <- struct{}{} }() awaitAdminAddress(sigCtx, r) diff --git a/internal/globals/globals.go b/internal/globals/globals.go index 68048522..2c0ddd9b 100644 --- a/internal/globals/globals.go +++ b/internal/globals/globals.go @@ -34,6 +34,8 @@ type RunOpts struct { RunDir string // DontArchiveRunDir is used in testing and prevents archiving the RunDir DontArchiveRunDir bool + // ExitCh is a channel that a goroutine running "envoy" will send an empty struct to when it exits. + ExitCh chan struct{} } // GlobalOpts represents options that affect more than one func-e commands. From 66db1463a1527f89b98ff0674b74f587acfb6383 Mon Sep 17 00:00:00 2001 From: Takeshi Yoneda Date: Tue, 18 Mar 2025 14:08:11 -0700 Subject: [PATCH 2/2] more Signed-off-by: Takeshi Yoneda --- api/run.go | 2 +- api/run_test.go | 2 +- internal/envoy/run.go | 4 +++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/api/run.go b/api/run.go index ccd250a7..2c524100 100644 --- a/api/run.go +++ b/api/run.go @@ -84,7 +84,7 @@ type runOpts struct { // passed to it. Use RunOption for configuration options. // // This will block until the process exits or the context is done. However, -// the Envoy process itself is running another goroutine, so to wait for the +// the Envoy process itself is running in another goroutine, so to wait for the // process to exit, use ExitChannel to synchronize with the envoy process exit. func Run(ctx context.Context, args []string, options ...RunOption) error { ro := &runOpts{ diff --git a/api/run_test.go b/api/run_test.go index 1fe5ba5a..6536ae37 100644 --- a/api/run_test.go +++ b/api/run_test.go @@ -43,7 +43,7 @@ func TestRunWithCtxDone(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() out := &bytes.Buffer{} - // This will return right after the context is done, but the Envoy process itself is running another goroutine, + // This will return right after the context is done, but the Envoy process itself is running in another goroutine, // so without using the exit channel, we might end up existing the test (or main program) before the Envoy process receives // the signal to exit, hence it might end up being a zombie process. err := Run(ctx, []string{ diff --git a/internal/envoy/run.go b/internal/envoy/run.go index 82df9c6a..f38ae111 100644 --- a/internal/envoy/run.go +++ b/internal/envoy/run.go @@ -62,7 +62,9 @@ func (r *Runtime) Run(ctx context.Context, args []string) error { go func() { defer waitCancel() _ = r.cmd.Wait() // Envoy logs like "caught SIGINT" or "caught ENVOY_SIGTERM", so we don't repeat logging here. - r.opts.ExitCh <- struct{}{} + if r.opts.ExitCh != nil { + r.opts.ExitCh <- struct{}{} + } }() awaitAdminAddress(sigCtx, r)