Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Improve *-all error message output #722

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
2 changes: 1 addition & 1 deletion cli/cli_app.go
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,7 @@ func prepareInitOptions(terragruntOptions *options.TerragruntOptions, terraformS
initOptions.TerraformCommand = CMD_INIT

// Don't pollute stdout with the stdout from Auto Init
initOptions.Writer = initOptions.ErrWriter
initOptions.Writer = io.MultiWriter(initOptions.ErrWriter, &terragruntOptions.InitStream)

return initOptions, nil
}
Expand Down
48 changes: 44 additions & 4 deletions configstack/running_module.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package configstack

import (
"bytes"
"fmt"
"io"
"strings"
"sync"

Expand All @@ -27,6 +29,9 @@ type runningModule struct {
Dependencies map[string]*runningModule
NotifyWhenDone []*runningModule
FlagExcluded bool
OutStream bytes.Buffer
ErrStream bytes.Buffer
Writer io.Writer
}

// This controls in what order dependencies should be enforced between modules
Expand All @@ -37,6 +42,13 @@ const (
ReverseOrder
)

var (
// OutputMessageSeparator is the string used for separating the different module outputs
OutputMessageSeparator = strings.Repeat("-", 132)
// DetailedErrorMessageDivider is the string used for diving the detailed error output at the end of the execution from the rest of the output
DetailedErrorMessageDivider = strings.Repeat("=", 132)
)

// Create a new RunningModule struct for the given module. This will initialize all fields to reasonable defaults,
// except for the Dependencies and NotifyWhenDone, both of which will be empty. You should fill these using a
// function such as crossLinkDependencies.
Expand All @@ -48,6 +60,7 @@ func newRunningModule(module *TerraformModule) *runningModule {
Dependencies: map[string]*runningModule{},
NotifyWhenDone: []*runningModule{},
FlagExcluded: module.FlagExcluded,
Writer: module.TerragruntOptions.Writer,
}
}

Expand Down Expand Up @@ -130,6 +143,7 @@ func removeFlagExcluded(modules map[string]*runningModule) map[string]*runningMo
Err: module.Err,
NotifyWhenDone: module.NotifyWhenDone,
Status: module.Status,
Writer: module.Module.TerragruntOptions.Writer,
}

// Only add dependencies that should not be excluded
Expand All @@ -154,6 +168,8 @@ func runModules(modules map[string]*runningModule) error {
waitGroup.Add(1)
go func(module *runningModule) {
defer waitGroup.Done()
module.Module.TerragruntOptions.ErrWriter = io.MultiWriter(&module.OutStream, &module.ErrStream)
module.Module.TerragruntOptions.Writer = &module.OutStream
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused by this... You're overwriting TerragruntOptions.ErrWriter to write to both module.OutStream and module.ErrStream... But what was the Terragrunt.LOptions.ErrWriter value set to before that? Are module.OutStream and module.ErrStream initialized to anything? Will this buffer those errors until the very end or stream to stdout / stderr?

Same questions go for TerragruntOptions.Writer, with the additional one of what happens when you point a second item to module.OutStream?

module.runModuleWhenReady()
}(module)
}
Expand All @@ -167,17 +183,37 @@ func runModules(modules map[string]*runningModule) error {
// occurred
func collectErrors(modules map[string]*runningModule) error {
errs := []error{}
detailedErrs := []error{}

for _, module := range modules {
if module.Err != nil {
errs = append(errs, module.Err)
detailedErrs = append(detailedErrs, generateDetailedErrorMessage(module))
}
}

if len(errs) == 0 {
return nil
}

return errors.WithStackTrace(MultiError{Errors: errs, DetailedErrors: detailedErrs})
}

// generateDetailedErrorMessage extracts the clean stderr from a module and formats it for printing
func generateDetailedErrorMessage(module *runningModule) error {
// remove the auto-init pollution from the error stream
cleanErrorOutput := strings.Replace(module.ErrStream.String(), module.Module.TerragruntOptions.InitStream.String(), "", -1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, string replacement feels... A bit hacky. Is there any way to have the AutoInit write to stdout / stderr, but to not write to module.ErrStream? That way, you could use module.ErrStream directly, without having to clean anything out...


// determine whether the error is a dependency error
var errorType string

if _, isDepErr := module.Err.(DependencyFinishedWithError); isDepErr {
errorType = "(dependency error)"
} else {
return errors.WithStackTrace(MultiError{Errors: errs})
errorType = "(root error)"
}

return fmt.Errorf("%s %s: \n\n%s \n\n%s\n%s", module.Module.Path, errorType, module.Err, cleanErrorOutput, OutputMessageSeparator)
}

// Run a module once all of its dependencies have finished executing.
Expand Down Expand Up @@ -223,6 +259,7 @@ func (module *runningModule) runNow() error {
module.Module.TerragruntOptions.Logger.Printf("Running module %s now", module.Module.Path)
return module.Module.TerragruntOptions.RunTerragrunt(module.Module.TerragruntOptions)
}

}

// Record that a module has finished executing and notify all of this module's dependencies
Expand All @@ -233,6 +270,8 @@ func (module *runningModule) moduleFinished(moduleErr error) {
module.Module.TerragruntOptions.Logger.Printf("Module %s has finished with an error: %v", module.Module.Path, moduleErr)
}

fmt.Fprintf(module.Writer, "%s\n%v\n\n%v\n", OutputMessageSeparator, module.Module.Path, module.OutStream.String())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why fmt.Fprintf here?


module.Status = Finished
module.Err = moduleErr

Expand Down Expand Up @@ -261,15 +300,16 @@ func (this DependencyFinishedWithError) ExitStatus() (int, error) {
}

type MultiError struct {
Errors []error
Errors []error
DetailedErrors []error
}

func (err MultiError) Error() string {
errorStrings := []string{}
for _, err := range err.Errors {
for _, err := range err.DetailedErrors {
errorStrings = append(errorStrings, err.Error())
}
return fmt.Sprintf("Encountered the following errors:\n%s", strings.Join(errorStrings, "\n"))
return fmt.Sprintf("Encountered the following errors:\n%s\n%s", DetailedErrorMessageDivider, strings.Join(errorStrings, "\n"))
}

func (this MultiError) ExitStatus() (int, error) {
Expand Down
39 changes: 39 additions & 0 deletions configstack/running_module_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package configstack

import (
"bytes"
"fmt"
"testing"

Expand Down Expand Up @@ -1073,3 +1074,41 @@ func TestRunModulesReverseOrderMultipleModulesWithDependenciesLargeGraphPartialF
assert.True(t, eRan)
assert.True(t, fRan)
}

func TestGenerateDetailedErrorMessage(t *testing.T) {

const (
initText = "Terraform has been successfully initialized!"
argMissingErr = "Error: Missing required argument"
exitCodeErr = "Hit multiple errors:\nexit status 1"
modulePath = "a"
)

var initBuffer bytes.Buffer
initBuffer.WriteString(initText)

module := &TerraformModule{
Path: modulePath,
Dependencies: []*TerraformModule{},
Config: config.TerragruntConfig{},
TerragruntOptions: &options.TerragruntOptions{InitStream: initBuffer, ExcludeDirs: []string{}},
}

runningModule, err := toRunningModules([]*TerraformModule{module}, NormalOrder)
assert.Nil(t, err)

var errBuffer bytes.Buffer
errBuffer.WriteString(fmt.Sprintf("%s\n%s", initText, argMissingErr))

runningModule[modulePath].Err = fmt.Errorf(exitCodeErr)
runningModule[modulePath].ErrStream = errBuffer

detailedErrorMessage := generateDetailedErrorMessage(runningModule[modulePath])
fmt.Printf("DETAILED ERROR:\n%s\n", detailedErrorMessage)

// ensure terraform init output is removed from detailed error messages
assert.NotContains(t, detailedErrorMessage.Error(), initText)
assert.Contains(t, detailedErrorMessage.Error(), fmt.Sprintf("%s (root error)", modulePath))
assert.Contains(t, detailedErrorMessage.Error(), exitCodeErr)
assert.Contains(t, detailedErrorMessage.Error(), argMissingErr)
}
3 changes: 2 additions & 1 deletion configstack/stack.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ import (
"fmt"
"strings"

"sort"

"github.com/gruntwork-io/terragrunt/config"
"github.com/gruntwork-io/terragrunt/errors"
"github.com/gruntwork-io/terragrunt/options"
"github.com/gruntwork-io/terragrunt/util"
"sort"
)

// Represents a stack of Terraform modules (i.e. folders with Terraform templates) that you can "spin up" or
Expand Down
4 changes: 4 additions & 0 deletions options/options.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package options

import (
"bytes"
"fmt"
"io"
"log"
Expand Down Expand Up @@ -80,6 +81,9 @@ type TerragruntOptions struct {
// If you want stderr to go somewhere other than os.stderr
ErrWriter io.Writer

// Stores output of auto-init so it can be removed later form other streams
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Stores output of auto-init so it can be removed later form other streams
// Stores output of auto-init so it can be removed later from other streams

InitStream bytes.Buffer
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps there should be an AutoInitWriter and AutoInitErrWriter here instead? By default, these would be set to the same as Writer and ErrWriter. However, for xxx-all commands, these could be set to separate values that write to stdout / stderr but are not stored in those streams later used to print clean error messages?

Also, should this value be in the Clone method? If not, explicitly add a comment there explaining why.


// When searching the directory tree, this is the max folders to check before exiting with an error. This is
// exposed here primarily so we can set it to a low value at test time.
MaxFoldersToCheck int
Expand Down
30 changes: 30 additions & 0 deletions test/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,36 @@ func TestTerragruntReportsTerraformErrorsWithPlanAll(t *testing.T) {
assert.True(t, strings.Contains(errOutput, "missingvar2") || strings.Contains(output, "missingvar2"))
}

func TestTerragruntReportsDetailedErrorsWithPlanAll(t *testing.T) {

cleanupTerraformFolder(t, TEST_FIXTURE_FAILED_TERRAFORM)
tmpEnvPath := copyEnvironment(t, TEST_FIXTURE_FAILED_TERRAFORM)

rootTerragruntConfigPath := util.JoinPath(tmpEnvPath, "fixture-failure")

cmd := fmt.Sprintf("terragrunt plan-all --terragrunt-non-interactive --terragrunt-working-dir %s", rootTerragruntConfigPath)
var (
stdout bytes.Buffer
stderr bytes.Buffer
)
// Call runTerragruntCommand directly because this command contains failures (which causes runTerragruntRedirectOutput to abort) but we don't care.

detailedErr := runTerragruntCommand(t, cmd, &stdout, &stderr)

if detailedErr == nil {
t.Fatalf("Failed to properly fail command: %v. The terraform should be bad", cmd)
}

fmt.Printf("DETAILED ERR is %s", detailedErr)

assert.Contains(t, detailedErr.Error(), "missingvar1")
assert.Contains(t, detailedErr.Error(), "missingvar2")
assert.Contains(t, detailedErr.Error(), configstack.DetailedErrorMessageDivider)

// ensure terraform init output is removed from detailed error messages
assert.NotContains(t, detailedErr.Error(), "Terraform has been successfully initialized!")
}

func TestTerragruntOutputAllCommand(t *testing.T) {
t.Parallel()

Expand Down