Changed RunTerraformCommandAndIgnoreOutput() by RunTerraformCommandAn…

…dRedirectOutputToLogger() to never get rid of any output, but still isolating the trully desired output from the artefacts of other commands. Changed exit code numeric value from int to const Implemented a specialized version of MultiError for plan Some other recommendation changes from @brikis98
coveooss · Apr 29, 2017 · 3774b55 · 3774b55
1 parent a3ec44c
commit 3774b55
Show file tree

Hide file tree

Showing 6 changed files with 64 additions and 25 deletions.
diff --git a/cli/cli_app.go b/cli/cli_app.go
@@ -233,7 +233,7 @@ func downloadModules(terragruntOptions *options.TerragruntOptions) error {
 			return err
 		}
 		if shouldDownload {
-			return shell.RunTerraformCommandAndIgnoreOutput(terragruntOptions, "get", "-update")
+			return shell.RunTerraformCommandAndRedirectOutputToLogger(terragruntOptions, "get", "-update")
 		}
 	}
 

diff --git a/cli/download_source.go b/cli/download_source.go
@@ -350,5 +350,5 @@ func terraformInit(terraformSource *TerraformSource, terragruntOptions *options.
 	terragruntOptions.Logger.Printf("Downloading Terraform configurations from %s into %s", terraformSource.CanonicalSourceURL, terraformSource.DownloadDir)
 
 	// Backend and get configuration will be handled separately
-	return shell.RunTerraformCommandAndIgnoreOutput(terragruntOptions, "init", "-backend=false", "-get=false", terraformSource.CanonicalSourceURL.String(), terraformSource.DownloadDir)
+	return shell.RunTerraformCommandAndRedirectOutputToLogger(terragruntOptions, "init", "-backend=false", "-get=false", terraformSource.CanonicalSourceURL.String(), terraformSource.DownloadDir)
 }
diff --git a/configstack/running_module.go b/configstack/running_module.go
@@ -13,12 +13,22 @@ import (
 // Represents the status of a module that we are trying to apply as part of the apply-all or destroy-all command
 type ModuleStatus int
 
+const NORMAL_EXIT_CODE = 0
+const ERROR_EXIT_CODE = 1
+const UNDEFINED_EXIT_CODE = -1
+
 const (
 	Waiting ModuleStatus = iota
 	Running
 	Finished
 )
 
+// Using CreateMultiErrors as a variable instead of a function allows us to override the function used to compose multi error object.
+// It is used if a command wants to change the default behaviour of the severity analysis that is implemented by default.
+var CreateMultiErrors = func(errs []error) error {
+	return MultiError{Errors: errs}
+}
+
 // Represents a module we are trying to "run" (i.e. apply or destroy) as part of the apply-all or destroy-all command
 type runningModule struct {
 	Module         *TerraformModule
@@ -157,7 +167,7 @@ func collectErrors(modules map[string]*runningModule) error {
 	if len(errs) == 0 {
 		return nil
 	} else {
-		return errors.WithStackTrace(MultiError{Errors: errs})
+		return errors.WithStackTrace(CreateMultiErrors(errs))
 	}
 }
 
@@ -260,13 +270,11 @@ func (err MultiError) Error() string {
 }
 
 func (this MultiError) ExitStatus() (int, error) {
-	exitCode := 0
+	exitCode := NORMAL_EXIT_CODE
 	for i := range this.Errors {
 		if code, err := shell.GetExitCode(this.Errors[i]); err != nil {
-			return -1, this
-		} else if code == 1 || code == 2 && exitCode == 0 {
-			// The exit code 1 is more significant that the exit code 2 because it represents an error
-			// while 2 represent a warning.
+			return UNDEFINED_EXIT_CODE, this
+		} else if code > exitCode {
 			exitCode = code
 		}
 	}

diff --git a/configstack/stack_plan.go b/configstack/stack_plan.go
@@ -18,7 +18,17 @@ type moduleResult struct {
 	NbChanges int
 }
 
+const CHANGE_EXIT_CODE = 2
+
+var planResultRegex = regexp.MustCompile(`(\d+) to add, (\d+) to change, (\d+) to destroy.`)
+
 func (stack *Stack) planWithSummary(terragruntOptions *options.TerragruntOptions) error {
+	// We override the multi errors creator to use a specialized error type for plan
+	// because error severity in plan is not standard (i.e. exit code 2 is less significant that exit code 1).
+	CreateMultiErrors = func(errs []error) error {
+		return PlanMultiError{MultiError{errs}}
+	}
+
 	// We do a special treatment for -detailed-exitcode since we do not want to interrupt the processing of dependant
 	// stacks if one dependency has changes
 	detailedExitCode := util.ListContainsElement(terragruntOptions.TerraformCliArgs, "-detailed-exitcode")
@@ -45,8 +55,11 @@ func (stack *Stack) planWithSummary(terragruntOptions *options.TerragruntOptions
 // Returns the handler that will be executed after each completion of `terraform plan`
 func getResultHandler(detailedExitCode bool, results *[]moduleResult) ModuleHandler {
 	return func(module TerraformModule, output string, err error) error {
-		inspectPlanResult(module, output)
-		if exitCode, convErr := shell.GetExitCode(err); convErr == nil && detailedExitCode && exitCode == 2 {
+		warnAboutMissingDependencies(module, output)
+		if exitCode, convErr := shell.GetExitCode(err); convErr == nil && detailedExitCode && exitCode == CHANGE_EXIT_CODE {
+			// We do not want to consider CHANGE_EXIT_CODE as an error and not execute the dependants because there is an "error" in the dependencies.
+			// CHANGE_EXIT_CODE is not an error in this case, it is simply a status. We will reintroduce the exit code at the very end to mimic the behaviour
+			// of the native terrafrom plan -detailed-exitcode to exit with CHANGE_EXIT_CODE if there are changes in any of the module in the stack.
 			err = nil
 		}
 
@@ -73,7 +86,7 @@ func printSummary(terragruntOptions *options.TerragruntOptions, results []module
 }
 
 // Check the output message
-func inspectPlanResult(module TerraformModule, output string) {
+func warnAboutMissingDependencies(module TerraformModule, output string) {
 	if strings.Contains(output, ": Resource 'data.terraform_remote_state.") {
 		var dependenciesMsg string
 		if len(module.Dependencies) > 0 {
@@ -94,8 +107,7 @@ func extractSummaryResultFromPlan(output string) (string, int) {
 		return "No change", 0
 	}
 
-	re := regexp.MustCompile(`(\d+) to add, (\d+) to change, (\d+) to destroy.`)
-	result := re.FindStringSubmatch(output)
+	result := planResultRegex.FindStringSubmatch(output)
 	if len(result) == 0 {
 		return "Unable to determine the plan status", -1
 	}
@@ -119,11 +131,11 @@ func extractSummaryResultFromPlan(output string) (string, int) {
 type countError struct{ count int }
 
 func (err countError) Error() string {
-	article, s := "is", ""
+	article, plural := "is", ""
 	if err.count > 1 {
-		article, s = "are", "s"
+		article, plural = "are", "s"
 	}
-	return fmt.Sprintf("There %s %v change%s to apply", article, err.count, s)
+	return fmt.Sprintf("There %s %v change%s to apply", article, err.count, plural)
 }
 
 // If there are changes, the exit status must be = 2
@@ -133,3 +145,23 @@ func (err countError) ExitStatus() (int, error) {
 	}
 	return 0, nil
 }
+
+// This is a specialized version of MultiError type
+// It handles the exit code differently from the base implementation
+type PlanMultiError struct {
+	MultiError
+}
+
+func (this PlanMultiError) ExitStatus() (int, error) {
+	exitCode := NORMAL_EXIT_CODE
+	for i := range this.Errors {
+		if code, err := shell.GetExitCode(this.Errors[i]); err != nil {
+			return UNDEFINED_EXIT_CODE, this
+		} else if code == ERROR_EXIT_CODE || code == CHANGE_EXIT_CODE && exitCode == NORMAL_EXIT_CODE {
+			// The exit code 1 is more significant that the exit code 2 because it represents an error
+			// while 2 represent a warning.
+			return UNDEFINED_EXIT_CODE, this
+		}
+	}
+	return exitCode, nil
+}
diff --git a/remote/remote_state.go b/remote/remote_state.go
@@ -66,7 +66,7 @@ func (remoteState RemoteState) ConfigureRemoteState(terragruntOptions *options.T
 		}
 
 		terragruntOptions.Logger.Printf("Configuring remote state for the %s backend", remoteState.Backend)
-		return shell.RunTerraformCommandAndIgnoreOutput(terragruntOptions, initCommand(remoteState)...)
+		return shell.RunTerraformCommandAndRedirectOutputToLogger(terragruntOptions, initCommand(remoteState)...)
 	}
 
 	return nil

diff --git a/shell/run_shell_cmd.go b/shell/run_shell_cmd.go
@@ -1,6 +1,7 @@
 package shell
 
 import (
+	"bytes"
 	"log"
 	"os"
 	"os/exec"
@@ -9,8 +10,6 @@ import (
 	"strings"
 	"syscall"
 
-	"bytes"
-	"fmt"
 	"github.com/gruntwork-io/terragrunt/errors"
 	"github.com/gruntwork-io/terragrunt/options"
 )
@@ -20,13 +19,13 @@ func RunTerraformCommand(terragruntOptions *options.TerragruntOptions, args ...s
 	return RunShellCommand(terragruntOptions, terragruntOptions.TerraformPath, args...)
 }
 
-// Run the given Terraform command and ignore the output unless there is an error
-func RunTerraformCommandAndIgnoreOutput(terragruntOptions *options.TerragruntOptions, args ...string) error {
+// Run the given Terraform command but redirect all outputs (both stdout and stderr) to the logger instead of
+// the default stream. This allows us to isolate the true output of terrraform command from the artefact of commands
+// like init and get during the preparation steps.
+// If the user redirect the stdout, he will only get the output for the terraform desired command.
+func RunTerraformCommandAndRedirectOutputToLogger(terragruntOptions *options.TerragruntOptions, args ...string) error {
 	output, err := runShellCommandAndCaptureOutput(terragruntOptions, true, terragruntOptions.TerraformPath, args...)
-	if err != nil {
-		fmt.Fprintln(terragruntOptions.ErrWriter, output)
-	}
-
+	terragruntOptions.Logger.Println(output)
 	return err
 }