ExitChannel to allow to wait for envoy process to exit

api/run.go

@@ -61,18 +61,31 @@ 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)
 
 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,
@@ -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)

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 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.
+		})
+	}
 }
 
 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())

internal/envoy/run.go

@@ -62,6 +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.
+		if r.opts.ExitCh != nil {
+			r.opts.ExitCh <- struct{}{}
+		}
 	}()
 
 	awaitAdminAddress(sigCtx, r)

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.