Merge pull request #141 from asobrien/bugfix/shell_signals

Wait for shell when forwarding signals; shell package test coverage

Author: brikis98

main.go

@@ -2,8 +2,7 @@ package main
 
 import (
 	"os"
-	"os/exec"
-	"syscall"
+	"github.com/gruntwork-io/terragrunt/shell"
 	"github.com/gruntwork-io/terragrunt/util"
 	"github.com/gruntwork-io/terragrunt/cli"
 	"github.com/gruntwork-io/terragrunt/errors"
@@ -35,12 +34,12 @@ func checkForErrorsAndExit(err error) {
 			logger.Println(err)
 		}
 		// exit with the underlying error code
-		var retCode int = 1
-		if exiterr, ok := errors.Unwrap(err).(*exec.ExitError); ok {
-			status := exiterr.Sys().(syscall.WaitStatus)
-			retCode = status.ExitStatus()
+		exitCode, exitCodeErr := shell.GetExitCode(err)
+		if exitCodeErr != nil {
+			exitCode = 1
+			logger.Println("Unable to determine underlying exit code, so Terragrunt will exit with error code 1")
 		}
-		os.Exit(retCode)
+		os.Exit(exitCode)
 	}
 
 }

shell/run_shell_cmd.go

@@ -7,6 +7,7 @@ import (
 	"os/signal"
 	"reflect"
 	"strings"
+	"syscall"
 
 	"github.com/gruntwork-io/terragrunt/errors"
 	"github.com/gruntwork-io/terragrunt/options"
@@ -33,30 +34,45 @@ func RunShellCommand(terragruntOptions *options.TerragruntOptions, command strin
 
 	cmd.Dir = terragruntOptions.WorkingDir
 
-	signalChannel := NewSignalsForwarder(forwardSignals, cmd.Process, terragruntOptions.Logger)
+	cmdChannel := make(chan error)
+	signalChannel := NewSignalsForwarder(forwardSignals, cmd, terragruntOptions.Logger, cmdChannel)
 	defer signalChannel.Close()
 
-	return errors.WithStackTrace(cmd.Run())
+	err := cmd.Run()
+	cmdChannel <- err
+
+	return errors.WithStackTrace(err)
+}
+
+
+// Return the exit code of a command. If the error is not an exec.ExitError type,
+// the error is returned.
+func GetExitCode(err error) (int, error) {
+	if exiterr, ok := errors.Unwrap(err).(*exec.ExitError); ok {
+		status := exiterr.Sys().(syscall.WaitStatus)
+		return status.ExitStatus(), nil
+	}
+	return 0, err
 }
 
 type SignalsForwarder chan os.Signal
 
-func NewSignalsForwarder(signals []os.Signal, p *os.Process, logger *log.Logger) SignalsForwarder {
+// Fowards signals to a command, waiting for the command to finish.
+func NewSignalsForwarder(signals []os.Signal, c *exec.Cmd, logger *log.Logger, cmdChannel chan error) SignalsForwarder {
 	signalChannel := make(chan os.Signal, 1)
 	signal.Notify(signalChannel, signals...)
 
 	go func() {
 		for {
 			select {
 			case s := <-signalChannel:
-				if s == nil {
-					return
-				}
 				logger.Printf("Forward signal %s to terraform.", s.String())
-				err := p.Signal(s)
+				err := c.Process.Signal(s)
 				if err != nil {
 					logger.Printf("Error forwarding signal: %v", err)
 				}
+			case <- cmdChannel:
+				return
 			}
 		}
 	}()

shell/run_shell_cmd_test.go

@@ -0,0 +1,123 @@
+package shell
+
+import (
+	goerrors "errors"
+	"github.com/gruntwork-io/terragrunt/options"
+	"github.com/stretchr/testify/assert"
+	"os"
+	"os/exec"
+	"strconv"
+	"testing"
+	"time"
+)
+
+func TestRunShellCommand(t *testing.T) {
+	t.Parallel()
+
+	terragruntOptions := options.NewTerragruntOptionsForTest("")
+	cmd := RunShellCommand(terragruntOptions, "/bin/bash", "-c", "true")
+	assert.Nil(t, cmd)
+
+	cmd = RunShellCommand(terragruntOptions, "/bin/bash", "-c", "false")
+	assert.Error(t, cmd)
+}
+
+func TestExitCode(t *testing.T) {
+	t.Parallel()
+
+	for i := 0; i <= 255; i++ {
+		cmd := exec.Command("../testdata/test_exit_code.sh", strconv.Itoa(i))
+		err := cmd.Run()
+
+		if i == 0 {
+			assert.Nil(t, err)
+		} else {
+			assert.Error(t, err)
+		}
+		retCode, err := GetExitCode(err)
+		assert.Nil(t, err)
+		assert.Equal(t, i, retCode)
+	}
+
+	// assert a non exec.ExitError returns an error
+	err := goerrors.New("This is an explicit error")
+	retCode, retErr := GetExitCode(err)
+	assert.Error(t, retErr, "An error was expected")
+	assert.Equal(t, err, retErr)
+	assert.Equal(t, 0, retCode)
+}
+
+func TestNewSignalsForwarderWait(t *testing.T) {
+	t.Parallel()
+
+	expectedWait := 5
+
+	terragruntOptions := options.NewTerragruntOptionsForTest("")
+	cmd := exec.Command("../testdata/test_sigint_wait.sh", strconv.Itoa(expectedWait))
+
+	cmdChannel := make(chan error)
+	runChannel := make(chan error)
+
+	signalChannel := NewSignalsForwarder(forwardSignals, cmd, terragruntOptions.Logger, cmdChannel)
+	defer signalChannel.Close()
+
+	go func() {
+		runChannel <- cmd.Run()
+	}()
+
+	time.Sleep(1000 * time.Millisecond)
+	start := time.Now()
+	cmd.Process.Signal(os.Interrupt)
+	err := <-runChannel
+	cmdChannel <- err
+	assert.Error(t, err)
+	retCode, err := GetExitCode(err)
+	assert.Nil(t, err)
+	assert.Equal(t, retCode, expectedWait)
+	assert.WithinDuration(t, time.Now(), start.Add(time.Duration(expectedWait)*time.Second), time.Second,
+		"Expected to wait 5 (+/-1) seconds after SIGINT")
+
+}
+
+func TestNewSignalsForwarderMultiple(t *testing.T) {
+	t.Parallel()
+
+	expectedInterrupts := 10
+	terragruntOptions := options.NewTerragruntOptionsForTest("")
+	cmd := exec.Command("../testdata/test_sigint_multiple.sh", strconv.Itoa(expectedInterrupts))
+
+	cmdChannel := make(chan error)
+	runChannel := make(chan error)
+
+	signalChannel := NewSignalsForwarder(forwardSignals, cmd, terragruntOptions.Logger, cmdChannel)
+	defer signalChannel.Close()
+
+	go func() {
+		runChannel <- cmd.Run()
+	}()
+
+	time.Sleep(1000 * time.Millisecond)
+
+	interruptAndWaitForProcess := func() (int, error) {
+		var interrupts int
+		var err error
+		for {
+			time.Sleep(500 * time.Millisecond)
+			select {
+			case err = <-runChannel:
+				return interrupts, err
+			default:
+				cmd.Process.Signal(os.Interrupt)
+				interrupts++
+			}
+		}
+	}
+
+	interrupts, err := interruptAndWaitForProcess()
+	cmdChannel <- err
+	assert.Error(t, err)
+	retCode, err := GetExitCode(err)
+	assert.Nil(t, err)
+	assert.Equal(t, retCode, interrupts)
+
+}

shell/signal_unix.go

@@ -7,4 +7,4 @@ import (
 	"syscall"
 )
 
-var forwardSignals []os.Signal = []os.Signal{syscall.SIGTERM}
+var forwardSignals []os.Signal = []os.Signal{syscall.SIGTERM, syscall.SIGINT}

testdata/test_exit_code.sh

@@ -0,0 +1,3 @@
+#!/bin/bash -e
+
+exit $1

testdata/test_sigint_multiple.sh

@@ -0,0 +1,16 @@
+#!/bin/bash -e
+
+INT_REQUIRED=$1
+INT_COUNTER=0
+
+trap int_handler INT
+
+function int_handler() {
+    INT_COUNTER=$((INT_COUNTER + 1))
+}
+
+while [ $INT_COUNTER -lt $INT_REQUIRED ]
+    do sleep 0.1
+done
+
+exit $INT_COUNTER

testdata/test_sigint_wait.sh

@@ -0,0 +1,12 @@
+#!/bin/bash -e
+
+WAIT_TIME=$1
+
+trap int_handler INT
+
+function int_handler() {
+        sleep $WAIT_TIME
+        exit $WAIT_TIME
+}
+
+while true; do sleep 0.1; done