diff --git a/api/run.go b/api/run.go index 2c524100..08f63252 100644 --- a/api/run.go +++ b/api/run.go @@ -61,14 +61,6 @@ 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) @@ -76,16 +68,11 @@ 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 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{ homeDir: globals.DefaultHomeDir, @@ -102,7 +89,6 @@ 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 6536ae37..d43a10b8 100644 --- a/api/run_test.go +++ b/api/run_test.go @@ -19,7 +19,6 @@ import ( "context" "os" "path/filepath" - "strconv" "testing" "time" @@ -29,31 +28,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" - // 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 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{ - "--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. - }) - } + 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) } func TestRunToCompletion(t *testing.T) { @@ -72,7 +71,7 @@ func TestRunToCompletion(t *testing.T) { ctx, cancel := context.WithTimeout(ctx, 1000*time.Minute) defer cancel() - err := Run(ctx, []string{"--version"}, Out(b), HomeDir(tmpDir), EnvoyVersionsURL(envoyVersionsURL)) + err := Run(ctx, runArgs, 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 f38ae111..adeea22d 100644 --- a/internal/envoy/run.go +++ b/internal/envoy/run.go @@ -62,9 +62,6 @@ 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. - if r.opts.ExitCh != nil { - r.opts.ExitCh <- struct{}{} - } }() awaitAdminAddress(sigCtx, r) diff --git a/internal/globals/globals.go b/internal/globals/globals.go index 2c0ddd9b..68048522 100644 --- a/internal/globals/globals.go +++ b/internal/globals/globals.go @@ -34,8 +34,6 @@ 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.