Merge pull request #130 from johananl/bugfix/mv_terraform_error

Improve error handling

bootstrap/bootstrap.go

@@ -24,6 +24,7 @@ import (
 	"os/signal"
 	"runtime"
 	"strings"
+	"sync"
 	"syscall"
 	"time"
 
@@ -105,12 +106,10 @@ Follow these instructions to create a token (we don't store any tokens):
 		colorstring.Println("[green]=> downloaded terraform successfully!")
 		s.Stop()
 
-		var terraformCmd *exec.Cmd
-		terraformCmd, err = executeCmd("mv", []string{"/tmp/terraform", "/usr/local/bin/"})
+		err = executeCmd("mv", "/tmp/terraform", "/usr/local/bin/")
 		if err != nil {
 			return errors.Wrapf(err, "moving terraform binary into /usr/local/bin")
 		}
-		terraformCmd.Wait()
 		colorstring.Println("[green]=> installed terraform successfully at /usr/local/bin")
 	} else {
 		colorstring.Println("[green]=> terraform found in $PATH!")
@@ -152,17 +151,17 @@ tunnels:
 		return errors.Wrap(err, "writing ngrok config file")
 	}
 
-	ngrokCmd, err := executeCmd("/tmp/ngrok", []string{"start", "atlantis", "--config", ngrokConfigFile.Name()})
+	// Used to ensure proper termination of all background commands.
+	var wg sync.WaitGroup
+	defer wg.Wait()
+
+	cancelNgrok, ngrokErrors, err := executeBackgroundCmd(&wg, "/tmp/ngrok", "start", "atlantis", "--config", ngrokConfigFile.Name())
+	// Check if we got a fast error. Move on if we haven't (the command is still running).
 	if err != nil {
-		return errors.Wrapf(err, "creating ngrok tunnel")
+		return errors.Wrap(err, "creating ngrok tunnel")
 	}
-
-	ngrokErrChan := make(chan error, 10)
-	go func() {
-		ngrokErrChan <- ngrokCmd.Wait()
-	}()
 	// When this function returns, ngrok tunnel should be stopped.
-	defer ngrokCmd.Process.Kill()
+	defer cancelNgrok()
 
 	// Wait for the tunnel to be up.
 	time.Sleep(2 * time.Second)
@@ -177,17 +176,14 @@ tunnels:
 	// Start atlantis server.
 	colorstring.Println("[white]=> starting atlantis server")
 	s.Start()
-	atlantisCmd, err := executeCmd(os.Args[0], []string{"server", "--gh-user", githubUsername, "--gh-token", githubToken, "--data-dir", "/tmp/atlantis/data", "--atlantis-url", tunnelURL, "--repo-whitelist", fmt.Sprintf("github.com/%s/%s", githubUsername, terraformExampleRepo)})
+	cancelAtlantis, atlantisErrors, err := executeBackgroundCmd(&wg, os.Args[0], "server", "--gh-user", githubUsername, "--gh-token", githubToken, "--data-dir", "/tmp/atlantis/data", "--atlantis-url", tunnelURL, "--repo-whitelist", fmt.Sprintf("github.com/%s/%s", githubUsername, terraformExampleRepo))
+	// Check if we got a fast error. Move on if we haven't (the command is still running).
 	if err != nil {
-		return errors.Wrapf(err, "creating atlantis server")
+		return errors.Wrap(err, "creating atlantis server")
 	}
-
-	atlantisErrChan := make(chan error, 10)
-	go func() {
-		atlantisErrChan <- atlantisCmd.Wait()
-	}()
 	// When this function returns atlantis server should be stopped.
-	defer atlantisCmd.Process.Kill()
+	defer cancelAtlantis()
+
 	colorstring.Printf("[green]=> atlantis server is now securely exposed at [bold][underline]%s\n", tunnelURL)
 	fmt.Println("")
 
@@ -215,9 +211,9 @@ tunnels:
 	colorstring.Println("[white]=> opening pull request")
 	s.Start()
 	time.Sleep(2 * time.Second)
-	_, err = executeCmd("open", []string{pullRequestURL})
+	err = executeCmd("open", pullRequestURL)
 	if err != nil {
-		colorstring.Printf("[red]=> opening pull request failed. please go to: %s on the browser", pullRequestURL)
+		colorstring.Printf("[red]=> opening pull request failed. please go to: %s on the browser\n", pullRequestURL)
 	}
 	s.Stop()
 
@@ -230,8 +226,16 @@ tunnels:
 	// bootstrap process and want's to stop.
 	signalChan := make(chan os.Signal, 1)
 	signal.Notify(signalChan, syscall.SIGINT, syscall.SIGTERM)
-	<-signalChan
-	colorstring.Println("\n[red]shutdown signal received, exiting....")
-	colorstring.Println("\n[green]Thank you for using atlantis :) \n[white]For more information about how to use atlantis in production go to: https://github.com/runatlantis/atlantis")
-	return nil
+
+	// Keep checking for errors from ngrok or atlantis server. Exit normally on shutdown signal.
+	select {
+	case <-signalChan:
+		colorstring.Println("\n[red]shutdown signal received, exiting....")
+		colorstring.Println("\n[green]Thank you for using atlantis :) \n[white]For more information about how to use atlantis in production go to: https://github.com/runatlantis/atlantis")
+		return nil
+	case err := <-ngrokErrors:
+		return errors.Wrap(err, "ngrok tunnel")
+	case err := <-atlantisErrors:
+		return errors.Wrap(err, "atlantis server")
+	}
 }

bootstrap/utils.go

@@ -15,13 +15,15 @@ package bootstrap
 
 import (
 	"archive/zip"
+	"context"
 	"encoding/json"
 	"fmt"
 	"io"
 	"net/http"
 	"os"
 	"os/exec"
 	"path/filepath"
+	"sync"
 	"syscall"
 
 	"github.com/pkg/errors"
@@ -134,11 +136,38 @@ func downloadAndUnzip(url string, path string, target string) error {
 	return unzip(path, target)
 }
 
-func executeCmd(cmd string, args []string) (*exec.Cmd, error) {
+// executeCmd executes a command, waits for it to finish and returns any errors.
+func executeCmd(cmd string, args ...string) error {
 	command := exec.Command(cmd, args...) // #nosec
+	bytes, err := command.CombinedOutput()
+	if err != nil {
+		return fmt.Errorf("%s: %s", err, bytes)
+	}
+	return nil
+}
+
+// executeBackgroundCmd executes a long-running command in the background. The function returns a
+// context so that the caller may cancel the command prematurely if necessary, as well as an errors
+// channel.
+//
+// The function returns an error if the command could not start successfully.
+func executeBackgroundCmd(wg *sync.WaitGroup, cmd string, args ...string) (context.CancelFunc, <-chan error, error) {
+	ctx, cancel := context.WithCancel(context.Background())
+	command := exec.CommandContext(ctx, cmd, args...) // #nosec
+
+	errChan := make(chan error, 1)
+
 	err := command.Start()
 	if err != nil {
-		return nil, err
+		return cancel, errChan, fmt.Errorf("starting command: %v", err)
 	}
-	return command, nil
+
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		err := command.Wait()
+		errChan <- err
+	}()
+
+	return cancel, errChan, nil
 }