diff --git a/pkg/binary/envoy/run.go b/pkg/binary/envoy/run.go index b465089d..c749af8d 100644 --- a/pkg/binary/envoy/run.go +++ b/pkg/binary/envoy/run.go @@ -33,8 +33,8 @@ func (r *Runtime) Run(ctx context.Context, args []string) error { // handlers, and they expect the process to still be running. For example, this allows admin API hooks. cmd := exec.Command(r.opts.EnvoyPath, args...) // #nosec -> users can run whatever binary they like! cmd.Dir = r.opts.WorkingDir - cmd.Stdout = r.IO.Out - cmd.Stderr = r.IO.Err + cmd.Stdout = r.Out + cmd.Stderr = r.Err cmd.SysProcAttr = sysProcAttr() r.cmd = cmd diff --git a/pkg/binary/envoy/runtime.go b/pkg/binary/envoy/runtime.go index 7526ce9e..6c1d0174 100644 --- a/pkg/binary/envoy/runtime.go +++ b/pkg/binary/envoy/runtime.go @@ -18,12 +18,12 @@ import ( "context" "errors" "fmt" + "io" "net" "os" "os/exec" "github.com/tetratelabs/getenvoy/pkg/globals" - ioutil "github.com/tetratelabs/getenvoy/pkg/util/io" ) // NewRuntime creates a new Runtime that runs envoy in globals.RunOpts WorkingDir @@ -37,7 +37,8 @@ type Runtime struct { opts *globals.RunOpts cmd *exec.Cmd - IO ioutil.StdStreams + Out io.Writer + Err io.Writer adminAddress, adminAddressPath string diff --git a/pkg/cmd/root.go b/pkg/cmd/root.go index 04b3a935..be4ed201 100644 --- a/pkg/cmd/root.go +++ b/pkg/cmd/root.go @@ -24,7 +24,6 @@ import ( "github.com/spf13/cobra" "github.com/tetratelabs/getenvoy/pkg/globals" - "github.com/tetratelabs/getenvoy/pkg/util/exec" "github.com/tetratelabs/getenvoy/pkg/version" ) @@ -106,15 +105,10 @@ func handleFlagOverrides(o *globals.GlobalOpts, homeDirFlag, manifestURLFlag str func Execute(cmd *cobra.Command) error { actualCmd, err := cmd.ExecuteC() if actualCmd != nil && err != nil { // both are always true on error - var serr exec.ShutdownError - if errors.As(err, &serr) { // in case of ShutdownError, we want to avoid any wrapper messages - cmd.PrintErrln("NOTE:", serr.Error()) - } else { - cmd.PrintErrln("Error:", err.Error()) - // actualCmd ensures command path includes the subcommand (ex "extension run") - cmd.PrintErrf("\nRun '%v --help' for usage.\n", actualCmd.CommandPath()) - return err - } + cmd.PrintErrln("Error:", err.Error()) + // actualCmd ensures command path includes the subcommand (ex "extension run") + cmd.PrintErrf("\nRun '%v --help' for usage.\n", actualCmd.CommandPath()) + return err } return nil } diff --git a/pkg/cmd/root_test.go b/pkg/cmd/root_test.go index 5ddf8cde..15a62ede 100644 --- a/pkg/cmd/root_test.go +++ b/pkg/cmd/root_test.go @@ -15,15 +15,16 @@ package cmd_test import ( + "bytes" "fmt" "os" "testing" + "github.com/spf13/cobra" "github.com/stretchr/testify/require" rootcmd "github.com/tetratelabs/getenvoy/pkg/cmd" "github.com/tetratelabs/getenvoy/pkg/globals" - cmdtest "github.com/tetratelabs/getenvoy/pkg/test/cmd" ) func TestGetEnvoyValidateArgs(t *testing.T) { @@ -55,7 +56,7 @@ func TestGetEnvoyValidateArgs(t *testing.T) { test := test // pin! see https://github.com/kyoh86/scopelint for why t.Run(test.name, func(t *testing.T) { - c, stdout, stderr := cmdtest.NewRootCommand(o) + c, stdout, stderr := newRootCommand(o) c.SetArgs(test.args) err := rootcmd.Execute(c) @@ -116,7 +117,7 @@ func TestGetEnvoyHomeDir(t *testing.T) { } o := &globals.GlobalOpts{} - c, stdout, stderr := cmdtest.NewRootCommand(o) + c, stdout, stderr := newRootCommand(o) c.SetArgs(test.args) err := rootcmd.Execute(c) @@ -175,7 +176,7 @@ func TestGetEnvoyManifest(t *testing.T) { } o := &globals.GlobalOpts{} - c, stdout, stderr := cmdtest.NewRootCommand(o) + c, stdout, stderr := newRootCommand(o) c.SetArgs(append(test.args, "help")) err := rootcmd.Execute(c) @@ -198,3 +199,13 @@ func requireSetenv(t *testing.T, key, value string) func() { require.NoError(t, e, `error reverting env variable %s=%s`, key, previous) } } + +// newRootCommand initializes a command with buffers for stdout and stderr. +func newRootCommand(o *globals.GlobalOpts) (c *cobra.Command, stdout, stderr *bytes.Buffer) { + stdout = new(bytes.Buffer) + stderr = new(bytes.Buffer) + c = rootcmd.NewRoot(o) + c.SetOut(stdout) + c.SetErr(stderr) + return c, stdout, stderr +} diff --git a/pkg/cmd/run.go b/pkg/cmd/run.go index 87c716f6..0a8e0fa1 100644 --- a/pkg/cmd/run.go +++ b/pkg/cmd/run.go @@ -28,7 +28,6 @@ import ( "github.com/tetratelabs/getenvoy/pkg/binary/envoy" "github.com/tetratelabs/getenvoy/pkg/binary/envoy/debug" "github.com/tetratelabs/getenvoy/pkg/globals" - ioutil "github.com/tetratelabs/getenvoy/pkg/util/io" ) // NewRunCmd create a command responsible for starting an Envoy process @@ -95,11 +94,8 @@ func InitializeRunOpts(o *globals.GlobalOpts, reference string) error { // This is exposed for re-use in "getenvoy extension run" func Run(o *globals.GlobalOpts, cmd *cobra.Command, args []string) error { r := envoy.NewRuntime(&o.RunOpts) - r.IO = ioutil.StdStreams{ - In: cmd.InOrStdin(), - Out: cmd.OutOrStdout(), - Err: cmd.ErrOrStderr(), - } + r.Out = cmd.OutOrStdout() + r.Err = cmd.ErrOrStderr() debug.EnableAll(r) return r.Run(cmd.Context(), args) } diff --git a/pkg/cmd/run_test.go b/pkg/cmd/run_test.go index 9a12b317..95ce8258 100644 --- a/pkg/cmd/run_test.go +++ b/pkg/cmd/run_test.go @@ -26,7 +26,6 @@ import ( rootcmd "github.com/tetratelabs/getenvoy/pkg/cmd" "github.com/tetratelabs/getenvoy/pkg/globals" "github.com/tetratelabs/getenvoy/pkg/manifest" - "github.com/tetratelabs/getenvoy/pkg/test/cmd" manifesttest "github.com/tetratelabs/getenvoy/pkg/test/manifest" "github.com/tetratelabs/getenvoy/pkg/test/morerequire" ) @@ -54,7 +53,7 @@ func TestGetEnvoyRunValidateFlag(t *testing.T) { t.Run(test.name, func(t *testing.T) { // Run "getenvoy run" - c, stdout, stderr := cmd.NewRootCommand(&globals.GlobalOpts{}) + c, stdout, stderr := newRootCommand(&globals.GlobalOpts{}) c.SetArgs(test.args) err := rootcmd.Execute(c) @@ -72,7 +71,7 @@ func TestGetEnvoyRun(t *testing.T) { defer cleanup() // Run "getenvoy run standard:1.17.1 -- -c envoy.yaml" - c, stdout, stderr := cmd.NewRootCommand(&o.GlobalOpts) + c, stdout, stderr := newRootCommand(&o.GlobalOpts) c.SetArgs([]string{"run", reference.Latest, "--", "-c", "envoy.yaml"}) err := rootcmd.Execute(c) @@ -92,7 +91,7 @@ func TestGetEnvoyRunFailWithUnknownVersion(t *testing.T) { defer cleanup() o.EnvoyPath = "" // force lookup of version flag - c, _, stderr := cmd.NewRootCommand(&o.GlobalOpts) + c, _, stderr := newRootCommand(&o.GlobalOpts) // Run "getenvoy run unknown" version := "unknown" diff --git a/pkg/test/cmd/command.go b/pkg/test/cmd/command.go deleted file mode 100644 index 2ac5084d..00000000 --- a/pkg/test/cmd/command.go +++ /dev/null @@ -1,34 +0,0 @@ -// Copyright 2021 Tetrate -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package cmd - -import ( - "bytes" - - "github.com/spf13/cobra" - - "github.com/tetratelabs/getenvoy/pkg/cmd" - "github.com/tetratelabs/getenvoy/pkg/globals" -) - -// NewRootCommand initializes a command with buffers for stdout and stderr. -func NewRootCommand(o *globals.GlobalOpts) (c *cobra.Command, stdout, stderr *bytes.Buffer) { - stdout = new(bytes.Buffer) - stderr = new(bytes.Buffer) - c = cmd.NewRoot(o) - c.SetOut(stdout) - c.SetErr(stderr) - return c, stdout, stderr -} diff --git a/pkg/test/morerequire/morerequire.go b/pkg/test/morerequire/morerequire.go index 41c4a1ac..3d31b141 100644 --- a/pkg/test/morerequire/morerequire.go +++ b/pkg/test/morerequire/morerequire.go @@ -60,10 +60,3 @@ func RequireCaptureScript(t *testing.T, name string) (string, func()) { } return path, cleanup } - -// RequireAbs runs filepath.Abs and ensures there are no errors. -func RequireAbs(t *testing.T, f string) string { - f, err := filepath.Abs(f) - require.NoError(t, err, `error determining absolute path: %v`, f) - return f -} diff --git a/pkg/util/exec/exec.go b/pkg/util/exec/exec.go deleted file mode 100644 index a156b81f..00000000 --- a/pkg/util/exec/exec.go +++ /dev/null @@ -1,167 +0,0 @@ -// Copyright 2020 Tetrate -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package exec - -import ( - "context" - "fmt" - "os" - "os/exec" - "syscall" - "time" - - "github.com/tetratelabs/log" - - ioutil "github.com/tetratelabs/getenvoy/pkg/util/io" - osutil "github.com/tetratelabs/getenvoy/pkg/util/os" -) - -var ( - // killTimeout represents a delay between SIGTERM and SIGKILL signals - // that are sent to an external command when getenvoy process itself - // gets terminated. - killTimeout = 60 * time.Second -) - -var ( - setupSignalHandler = osutil.SetupSignalHandler -) - -func newRunError(cmd fmt.Stringer, cause error) *RunError { - return &RunError{ - cmd: cmd.String(), - cause: cause, - } -} - -// RunError represents an error to run an external command. -type RunError struct { - cmd string - cause error -} - -// Cmd returns a failed command. -func (e *RunError) Cmd() string { - return e.cmd -} - -// Cause returns a cause. -func (e *RunError) Cause() error { - return e.cause -} - -func (e *RunError) Error() string { - return fmt.Sprintf("failed to execute an external command %q: %v", e.Cmd(), e.Cause()) -} - -func newShutdownError(signal os.Signal) ShutdownError { - return ShutdownError{signal: signal} -} - -// ShutdownError represents an error caused by a shutdown signal. -type ShutdownError struct { - signal os.Signal -} - -// Error returns the error message. -func (e ShutdownError) Error() string { - return fmt.Sprintf("Shutting down early because a Ctrl-C (%q) was received.", e.signal) -} - -// Run executes a given command. -func Run(cmd *exec.Cmd, streams ioutil.StdStreams) error { - log.Debugf("running: %s", cmd) - - // configure standard I/O of the external process - cmd.Stdin = streams.In - cmd.Stdout = streams.Out - cmd.Stderr = streams.Err - - // configure system attributes of the external process - if cmd.SysProcAttr == nil { - cmd.SysProcAttr = new(syscall.SysProcAttr) - } - // give OS a hint to automatically kill a child process whenever its parent dies. - parentDeathAttr.Set(cmd.SysProcAttr, syscall.SIGTERM) - - // start the specified command in a separate step to ensure that - // cmd.Process field is always set by the time we need it - if err := cmd.Start(); err != nil { - return newRunError(cmd, err) - } - - // use a dedicated goroutine to wait for the command to complete - errCh := make(chan error) - go func() { - defer close(errCh) - if err := cmd.Wait(); err != nil { - errCh <- err - } - }() - - // setup a cancelable handler for stop signals - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - stopCh := setupSignalHandler(ctx) - - // wait for the external command to complete or the current process to get stopped - select { - case err := <-errCh: - if err != nil { - return newRunError(cmd, err) - } - return nil - case sig := <-stopCh: - terminate(cmd, errCh) - return newShutdownError(sig) - } -} - -func terminate(cmd *exec.Cmd, errCh <-chan error) { - defer func() { - // if the external process didn't exit gracefully, kill it - if isProcessRunning(cmd.Process.Pid) { - if e := cmd.Process.Kill(); e != nil { - log.Warnf("failed to send SIGKILL to the external process %q: %v", cmd, e) - } - } - }() - - if isProcessRunning(cmd.Process.Pid) { - // First, give the external process a chance to exit gracefully. - if err := cmd.Process.Signal(syscall.SIGTERM); err != nil { - log.Warnf("failed to send SIGTERM to the external process %q: %v", cmd, err) - } - } - - // wait for the external process to exit or a timeout to expire - select { - case <-errCh: - // don't warn about errors in response to SIGTERM - case <-time.After(killTimeout): - log.Warnf("external process didn't exit gracefully within %s: %q", killTimeout, cmd) - } - _ = cmd.Process.Release() -} - -// Same as cgi.isProcessRunning. -// See https://github.com/golang/go/issues/34396 for tracking long term solution. -func isProcessRunning(pid int) bool { - p, err := os.FindProcess(pid) - if err != nil { - return false - } - return p.Signal(syscall.Signal(0)) == nil -} diff --git a/pkg/util/exec/exec_test.go b/pkg/util/exec/exec_test.go deleted file mode 100644 index ee2d26e1..00000000 --- a/pkg/util/exec/exec_test.go +++ /dev/null @@ -1,191 +0,0 @@ -// +build !windows - -// Copyright 2020 Tetrate -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package exec - -import ( - "bytes" - "context" - "fmt" - "os" - "os/exec" - "strings" - "syscall" - "testing" - "time" - - "github.com/stretchr/testify/require" - - "github.com/tetratelabs/getenvoy/pkg/test/morerequire" - ioutil "github.com/tetratelabs/getenvoy/pkg/util/io" -) - -func TestRunPipesStdoutAndStderr(t *testing.T) { - stdin, stdout, stderr := new(bytes.Buffer), new(bytes.Buffer), new(bytes.Buffer) - stdin.WriteString("input to stdin\n") - - testScript, removeTestScript := morerequire.RequireCaptureScript(t, "test") - defer removeTestScript() - - cmd := exec.Command(testScript, "text_exit=0") - err := Run(cmd, ioutil.StdStreams{In: stdin, Out: stdout, Err: stderr}) - - require.NoError(t, err, `error running [%v]`, cmd) - expectedStdout := fmt.Sprintf("test wd: %s\ntest bin: %s\ntest args: text_exit=0\n", morerequire.RequireAbs(t, "."), testScript) - require.Equal(t, expectedStdout, stdout.String(), `invalid stdout running [%v]`, cmd) - require.Equal(t, "test stderr\n", stderr.String(), `invalid stderr running [%v]`, cmd) -} - -func TestRunErrorWrapsCause(t *testing.T) { - testScript, removeTestScript := morerequire.RequireCaptureScript(t, "test") - defer removeTestScript() - - tests := []struct { - name string - path string - expectedErr string - expectedErrTarget error - }{ - { - name: "invalid path", - path: "/invalid/path", - expectedErr: `failed to execute an external command "/invalid/path test_exit=123": fork/exec /invalid/path: no such file or directory`, - expectedErrTarget: new(os.PathError), - }, - { - name: "exit status", - path: testScript, - expectedErr: fmt.Sprintf(`failed to execute an external command "%s test_exit=123": exit status 123`, testScript), - expectedErrTarget: new(exec.ExitError), - }, - } - - for _, test := range tests { - tt := test // pin! see https://github.com/kyoh86/scopelint for why - - t.Run(tt.name, func(t *testing.T) { - stdin, stdout, stderr := new(bytes.Buffer), new(bytes.Buffer), new(bytes.Buffer) - - cmd := exec.Command(tt.path, "test_exit=123") - err := Run(cmd, ioutil.StdStreams{In: stdin, Out: stdout, Err: stderr}) - - // Verify the command failed with the expected error. - require.EqualError(t, err, tt.expectedErr, `expected an error running [%v]`, cmd) - - var runErr *RunError - require.ErrorAs(t, err, &runErr, `expected a RunError running [%v]`, cmd) - require.Equal(t, tt.path+" test_exit=123", runErr.Cmd(), `expected RunError.Cmd() to contain path and args`) - require.ErrorAs(t, runErr.Cause(), &tt.expectedErrTarget, `expected RunError.Cause() to wrap the original error`) - }) - } -} - -func TestRunShutdownError(t *testing.T) { - for _, s := range []os.Signal{syscall.SIGINT, syscall.SIGTERM} { - s := s // pin! see https://github.com/kyoh86/scopelint for why - - t.Run(s.String(), func(t *testing.T) { - cmd := exec.Command("sleep", "1") - - // Fake the actual signal handler. Signals sent to stopCh aren't actually sent to the process. - stopCh := make(chan os.Signal, 1) - revertSetupSignalHandler := overrideSetupSignalHandler(func(ctx context.Context) <-chan os.Signal { - return stopCh - }) - defer revertSetupSignalHandler() - - // Since we are only testing signals manifest in errors, kill quickly. - revertKillTimeout := overrideKillTimeout(1 * time.Millisecond) - defer revertKillTimeout() - - // We run the command in a goroutine as we need to signal the process, which results in an error. - errCh := make(chan error) - go func() { - defer close(errCh) - errCh <- Run(cmd, ioutil.StdStreams{In: new(bytes.Buffer), Out: new(bytes.Buffer), Err: new(bytes.Buffer)}) - }() - - stopCh <- s // Trigger exec.terminate() - close(stopCh) - - // Verify the process shutdown and raised an error due to the signal we caught - for err := range errCh { - require.Equal(t, newShutdownError(s), err) - } - }) - } -} - -func TestRunSendsSIGTERMIfProcessStillRunningAfterStopSignal(t *testing.T) { - stdout, stderr := new(bytes.Buffer), new(bytes.Buffer) - stdio := ioutil.StdStreams{In: new(bytes.Buffer), Out: stdout, Err: stderr} - - sleepScript, removeSleepScript := morerequire.RequireCaptureScript(t, "sleep") - defer removeSleepScript() - cmd := exec.Command(sleepScript) - - // Fake the actual signal handler. Signals sent to stopCh aren't actually sent to the process. - stopCh := make(chan os.Signal, 1) - revertSetupSignalHandler := overrideSetupSignalHandler(func(ctx context.Context) <-chan os.Signal { - return stopCh - }) - defer revertSetupSignalHandler() - - // We run the command in a goroutine as we need to SIGTERM the process, which results in an error. - errCh := make(chan error) - go func() { - errCh <- Run(cmd, stdio) - close(errCh) - }() - - // Wait for shell script to start - require.Eventually(t, func() bool { - return strings.Contains(stdout.String(), "sleep wd") - }, 1*time.Second, 10*time.Millisecond) - - // Send a fake signal to our signal handler which invokes the stop channel - fakeSignal := syscall.Signal(0) - stopCh <- fakeSignal - close(stopCh) - - // Wait for the err channel which means the process shutdown. - for err := range errCh { - require.Equal(t, newShutdownError(fakeSignal), err) - } - - // Verify the program received a SIGTERM because the process was still alive upon exec.terminate() - require.Equal(t, "sleep stderr\nSIGTERM caught!\n", stderr.String(), `invalid stderr running [%v]`, cmd) -} - -// overrideSetupSignalHandler sets setupSignalHandler to a function. Doing so prevents the process from seeing signals. -// The function returned reverts the original value. -func overrideSetupSignalHandler(override func(ctx context.Context) <-chan os.Signal) func() { - previous := setupSignalHandler - setupSignalHandler = override - return func() { - setupSignalHandler = previous - } -} - -// overrideKillTimeout sets killTimeout to a short value as when we override setupSignalHandler, we need to kill faster. -// The function returned reverts the original value. -func overrideKillTimeout(override time.Duration) func() { - previous := killTimeout - killTimeout = override - return func() { - killTimeout = previous - } -} diff --git a/pkg/util/exec/sys.go b/pkg/util/exec/sys.go deleted file mode 100644 index 0a371df9..00000000 --- a/pkg/util/exec/sys.go +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright 2020 Tetrate -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package exec - -import "syscall" - -// parentDeathAttrFn represents an OS-specific field on syscall.SysProcAttr. -type parentDeathAttrFn func(attr *syscall.SysProcAttr, signal syscall.Signal) - -func (fn parentDeathAttrFn) Supported() bool { - return fn != nil -} - -func (fn parentDeathAttrFn) Set(attr *syscall.SysProcAttr, signal syscall.Signal) { - if fn.Supported() { - fn(attr, signal) - } -} diff --git a/pkg/util/exec/sys_linux.go b/pkg/util/exec/sys_linux.go deleted file mode 100644 index aa8102c6..00000000 --- a/pkg/util/exec/sys_linux.go +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright 2020 Tetrate -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package exec - -import "syscall" - -var ( - // parentDeathAttr is supported on Linux. - parentDeathAttr = parentDeathAttrFn(func(attr *syscall.SysProcAttr, signal syscall.Signal) { - attr.Pdeathsig = signal - }) -) diff --git a/pkg/util/exec/sys_other.go b/pkg/util/exec/sys_other.go deleted file mode 100644 index d75f363e..00000000 --- a/pkg/util/exec/sys_other.go +++ /dev/null @@ -1,22 +0,0 @@ -// +build !linux - -// Copyright 2020 Tetrate -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package exec - -var ( - // parentDeathAttr is not supported on OS other than Linux. - parentDeathAttr = parentDeathAttrFn(nil) -) diff --git a/pkg/util/io/io.go b/pkg/util/io/io.go deleted file mode 100644 index d80807b9..00000000 --- a/pkg/util/io/io.go +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright 2020 Tetrate -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package io - -import "io" - -// StdStreams represents a combination of standard I/O streams. -type StdStreams struct { - In io.Reader - Out io.Writer - Err io.Writer -} diff --git a/pkg/util/os/signals.go b/pkg/util/os/signals.go deleted file mode 100644 index e40c11e7..00000000 --- a/pkg/util/os/signals.go +++ /dev/null @@ -1,69 +0,0 @@ -// Copyright 2020 Tetrate -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package os - -import ( - "context" - "os" - "os/signal" - "syscall" -) - -var ( - // TODO(yskopets): at the moment, we only support Linux and Mac - shutdownSignals = []os.Signal{os.Interrupt, syscall.SIGTERM} - - terminate = func() { - os.Exit(1) - } -) - -// SetupSignalHandler registers for SIGTERM and SIGINT signals, and returns a -// channel that will be closed as soon as one of those signals is received. -// If a signal is received for the second time, the program will be terminated -// with exit code 1. -// -// The provided context is used to undo registration for signals if the context -// becomes done before a signal is received for the second time. -// Consequently, the returned channel might never get closed if the context -// becomes done prior to the first signal. -func SetupSignalHandler(ctx context.Context) <-chan os.Signal { - stopCh := make(chan os.Signal, 1) - - signalCh := make(chan os.Signal, 2) - signal.Notify(signalCh, shutdownSignals...) - go func() { - defer signal.Stop(signalCh) - - // on first signal, close the stop channel - select { - case <-ctx.Done(): - return - case sig := <-signalCh: - stopCh <- sig - close(stopCh) - } - - // on second signal, terminate the program - select { - case <-ctx.Done(): - return - case <-signalCh: - terminate() - } - }() - - return stopCh -} diff --git a/pkg/util/os/signals_test.go b/pkg/util/os/signals_test.go deleted file mode 100644 index 3a2cb93e..00000000 --- a/pkg/util/os/signals_test.go +++ /dev/null @@ -1,178 +0,0 @@ -// Copyright 2020 Tetrate -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package os - -import ( - "context" - "os" - "syscall" - "testing" - "time" - - "github.com/stretchr/testify/require" -) - -// Signal response can be slow in CI, so use a period larger than what makes sense locally -const waitFor = 100 * time.Millisecond - -func TestShutdownSignals(t *testing.T) { - // This is a base-case, just verifying the default values - require.Equal(t, []os.Signal{syscall.SIGINT, syscall.SIGTERM}, shutdownSignals) -} - -func TestSetupSignalHandlerCatchesRelevantSignalAndClosesTheChannel(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - stopCh := SetupSignalHandler(ctx) - - // send a relevant signal to the process - err := syscall.Kill(syscall.Getpid(), syscall.SIGINT) - require.NoError(t, err) - - requireSignal(t, syscall.SIGINT, stopCh) - requireChannelClosed(t, stopCh) -} - -func TestSetupSignalHandlerIgnoresWhenContextCanceledBeforeSignal(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - stopCh := SetupSignalHandler(ctx) - - cancel() // context canceled - - // send a relevant signal to the process - err := syscall.Kill(syscall.Getpid(), syscall.SIGINT) - require.NoError(t, err) - - // The signal is ignored because the context closed prior to receiving it - requireNoSignal(t, stopCh) -} - -func TestSetupSignalHandlerIgnoresIrrelevantSignal(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - stopCh := SetupSignalHandler(ctx) - - // send an irrelevant signal to the process - err := syscall.Kill(syscall.Getpid(), syscall.SIGUSR2) - require.NoError(t, err) - - requireNoSignal(t, stopCh) -} - -func requireSignal(t *testing.T, expected os.Signal, ch <-chan os.Signal) { - require.Eventually(t, func() bool { - select { - case s, ok := <-ch: - return s == expected && ok - default: - return false - } - }, waitFor, 10*time.Millisecond) -} - -func requireChannelClosed(t *testing.T, ch <-chan os.Signal) { - select { - case _, ok := <-ch: - require.False(t, ok) - default: - t.Fatal() - } -} - -// requireNoSignal ensures the channel is open, but there are no signals in it. -func requireNoSignal(t *testing.T, ch <-chan os.Signal) { - require.Never(t, func() bool { - select { - case v, ok := <-ch: - return v != nil || ok - default: - return false - } - }, waitFor, 10*time.Millisecond) -} - -// overrideTerminateWithBool returns a boolean made true on terminate. The function returned reverts the original. -func overrideTerminateWithBool() (*bool, func()) { - previous := terminate - closed := false - terminate = func() { - closed = true - } - return &closed, func() { - terminate = previous - } -} - -func TestSetupSignalHandlerTerminatesOnSecondRelevantSignal(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - terminated, revertTerminate := overrideTerminateWithBool() - defer revertTerminate() - - stopCh := SetupSignalHandler(ctx) - - // send a relevant signal to the process - err := syscall.Kill(syscall.Getpid(), syscall.SIGINT) - require.NoError(t, err) - - // First relevant signal closes the channel, but doesn't terminate - requireSignal(t, syscall.SIGINT, stopCh) - requireChannelClosed(t, stopCh) - require.False(t, *terminated) - - // send another relevant signal to the process - err = syscall.Kill(syscall.Getpid(), syscall.SIGINT) - require.NoError(t, err) - - // Second relevant signal terminates the process - require.Eventually(t, func() bool { - return *terminated - }, waitFor, 10*time.Millisecond) -} - -func TestSetupSignalHandlerDoesntTerminateWhenContextCanceledBeforeSecondRelevantSignal(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - terminated, revertTerminate := overrideTerminateWithBool() - defer revertTerminate() - - stopCh := SetupSignalHandler(ctx) - - // send a relevant signal to the process - err := syscall.Kill(syscall.Getpid(), syscall.SIGINT) - require.NoError(t, err) - - // First relevant signal closes the channel, but doesn't terminate - requireSignal(t, syscall.SIGINT, stopCh) - requireChannelClosed(t, stopCh) - require.False(t, *terminated) - - cancel() // context canceled - - // send another relevant signal to the process - err = syscall.Kill(syscall.Getpid(), syscall.SIGINT) - require.NoError(t, err) - - // Second relevant signal doesn't terminate the process - require.Never(t, func() bool { - return *terminated - }, waitFor, 10*time.Millisecond) -} diff --git a/pkg/util/args/args.go b/test/e2e/util/args.go similarity index 98% rename from pkg/util/args/args.go rename to test/e2e/util/args.go index 86c3e772..6ba340b6 100644 --- a/pkg/util/args/args.go +++ b/test/e2e/util/args.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package args +package util import ( "fmt" diff --git a/pkg/util/args/args_test.go b/test/e2e/util/args_test.go similarity index 99% rename from pkg/util/args/args_test.go rename to test/e2e/util/args_test.go index 6f7fccb2..48fb97c1 100644 --- a/pkg/util/args/args_test.go +++ b/test/e2e/util/args_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package args +package util import ( "testing" diff --git a/test/e2e/util/command.go b/test/e2e/util/command.go index edd33d58..1e15df7a 100644 --- a/test/e2e/util/command.go +++ b/test/e2e/util/command.go @@ -26,8 +26,6 @@ import ( "time" "github.com/stretchr/testify/require" - - argutil "github.com/tetratelabs/getenvoy/pkg/util/args" ) var ( @@ -42,7 +40,7 @@ type cmdBuilder struct { // GetEnvoy returns a new command builder. func GetEnvoy(cmdline string) *cmdBuilder { //nolint:golint - args, err := argutil.SplitCommandLine(cmdline) + args, err := SplitCommandLine(cmdline) if err != nil { panic(err) }