From 62f7526db4fa5aa6e8fa6d40f3221e3ab973988c Mon Sep 17 00:00:00 2001 From: Jeff Knurek Date: Thu, 16 Aug 2018 16:17:57 +0200 Subject: [PATCH 1/2] INIT: if the init cmd fails providing more info in the logs is helpful --- server/events/runtime/init_step_runner.go | 14 +++++----- .../events/runtime/init_step_runner_test.go | 27 ++++++++++++++++++- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/server/events/runtime/init_step_runner.go b/server/events/runtime/init_step_runner.go index d5a4a4b19..4267ce045 100644 --- a/server/events/runtime/init_step_runner.go +++ b/server/events/runtime/init_step_runner.go @@ -16,14 +16,16 @@ func (i *InitStepRunner) Run(ctx models.ProjectCommandContext, extraArgs []strin if ctx.ProjectConfig != nil && ctx.ProjectConfig.TerraformVersion != nil { tfVersion = ctx.ProjectConfig.TerraformVersion } + terraformInitCmd := append([]string{"init", "-no-color"}, extraArgs...) // If we're running < 0.9 we have to use `terraform get` instead of `init`. if MustConstraint("< 0.9.0").Check(tfVersion) { ctx.Log.Info("running terraform version %s so will use `get` instead of `init`", tfVersion) - terraformGetCmd := append([]string{"get", "-no-color"}, extraArgs...) - _, err := i.TerraformExecutor.RunCommandWithVersion(ctx.Log, path, terraformGetCmd, tfVersion, ctx.Workspace) - return "", err - } else { - _, err := i.TerraformExecutor.RunCommandWithVersion(ctx.Log, path, append([]string{"init", "-no-color"}, extraArgs...), tfVersion, ctx.Workspace) - return "", err + terraformInitCmd = append([]string{"get", "-no-color"}, extraArgs...) } + out, err := i.TerraformExecutor.RunCommandWithVersion(ctx.Log, path, terraformInitCmd, tfVersion, ctx.Workspace) + // if there was an error in `init`, than the output is interesting + if err != nil { + return out, err + } + return "", nil } diff --git a/server/events/runtime/init_step_runner_test.go b/server/events/runtime/init_step_runner_test.go index ff0ec9f08..f1d727ff5 100644 --- a/server/events/runtime/init_step_runner_test.go +++ b/server/events/runtime/init_step_runner_test.go @@ -1,6 +1,7 @@ package runtime_test import ( + "errors" "testing" version "github.com/hashicorp/go-version" @@ -57,10 +58,34 @@ func TestRun_UsesGetOrInitForRightVersion(t *testing.T) { RepoRelDir: ".", }, []string{"extra", "args"}, "/path") Ok(t, err) - // Shouldn't return output since we don't print init output to PR. + // When there is no error, should not return init output to PR. Equals(t, "", output) terraform.VerifyWasCalledOnce().RunCommandWithVersion(logger, "/path", []string{c.expCmd, "-no-color", "extra", "args"}, tfVersion, "workspace") }) + + t.Run(c.version, func(t *testing.T) { + terraform := mocks.NewMockClient() + + tfVersion, _ := version.NewVersion(c.version) + logger := logging.NewNoopLogger() + iso := runtime.InitStepRunner{ + TerraformExecutor: terraform, + DefaultTFVersion: tfVersion, + } + When(terraform.RunCommandWithVersion(matchers.AnyPtrToLoggingSimpleLogger(), AnyString(), AnyStringSlice(), matchers2.AnyPtrToGoVersionVersion(), AnyString())). + ThenReturn("output", errors.New("err")) + + output, err := iso.Run(models.ProjectCommandContext{ + Log: logger, + Workspace: "workspace", + RepoRelDir: ".", + }, []string{"extra", "args"}, "/path") + // But if there is an error, the init output should be returned. + ErrEquals(t, "err", err) + Equals(t, "output", output) + + terraform.VerifyWasCalledOnce().RunCommandWithVersion(logger, "/path", []string{c.expCmd, "-no-color", "extra", "args"}, tfVersion, "workspace") + }) } } From 13f79f396376d9394a809db474876eab51365932 Mon Sep 17 00:00:00 2001 From: Luke Kysow Date: Fri, 17 Aug 2018 13:52:43 -1000 Subject: [PATCH 2/2] Split init step test into its own. --- server/events/runtime/init_step_runner.go | 5 +- .../events/runtime/init_step_runner_test.go | 47 +++++++++---------- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/server/events/runtime/init_step_runner.go b/server/events/runtime/init_step_runner.go index 4267ce045..1e8343f8f 100644 --- a/server/events/runtime/init_step_runner.go +++ b/server/events/runtime/init_step_runner.go @@ -17,13 +17,16 @@ func (i *InitStepRunner) Run(ctx models.ProjectCommandContext, extraArgs []strin tfVersion = ctx.ProjectConfig.TerraformVersion } terraformInitCmd := append([]string{"init", "-no-color"}, extraArgs...) + // If we're running < 0.9 we have to use `terraform get` instead of `init`. if MustConstraint("< 0.9.0").Check(tfVersion) { ctx.Log.Info("running terraform version %s so will use `get` instead of `init`", tfVersion) terraformInitCmd = append([]string{"get", "-no-color"}, extraArgs...) } + out, err := i.TerraformExecutor.RunCommandWithVersion(ctx.Log, path, terraformInitCmd, tfVersion, ctx.Workspace) - // if there was an error in `init`, than the output is interesting + // Only include the init output if there was an error. Otherwise it's + // unnecessary and lengthens the comment. if err != nil { return out, err } diff --git a/server/events/runtime/init_step_runner_test.go b/server/events/runtime/init_step_runner_test.go index f1d727ff5..dc4221c5e 100644 --- a/server/events/runtime/init_step_runner_test.go +++ b/server/events/runtime/init_step_runner_test.go @@ -1,11 +1,11 @@ package runtime_test import ( - "errors" "testing" version "github.com/hashicorp/go-version" . "github.com/petergtz/pegomock" + "github.com/pkg/errors" "github.com/runatlantis/atlantis/server/events/mocks/matchers" "github.com/runatlantis/atlantis/server/events/models" "github.com/runatlantis/atlantis/server/events/runtime" @@ -63,29 +63,26 @@ func TestRun_UsesGetOrInitForRightVersion(t *testing.T) { terraform.VerifyWasCalledOnce().RunCommandWithVersion(logger, "/path", []string{c.expCmd, "-no-color", "extra", "args"}, tfVersion, "workspace") }) - - t.Run(c.version, func(t *testing.T) { - terraform := mocks.NewMockClient() - - tfVersion, _ := version.NewVersion(c.version) - logger := logging.NewNoopLogger() - iso := runtime.InitStepRunner{ - TerraformExecutor: terraform, - DefaultTFVersion: tfVersion, - } - When(terraform.RunCommandWithVersion(matchers.AnyPtrToLoggingSimpleLogger(), AnyString(), AnyStringSlice(), matchers2.AnyPtrToGoVersionVersion(), AnyString())). - ThenReturn("output", errors.New("err")) - - output, err := iso.Run(models.ProjectCommandContext{ - Log: logger, - Workspace: "workspace", - RepoRelDir: ".", - }, []string{"extra", "args"}, "/path") - // But if there is an error, the init output should be returned. - ErrEquals(t, "err", err) - Equals(t, "output", output) - - terraform.VerifyWasCalledOnce().RunCommandWithVersion(logger, "/path", []string{c.expCmd, "-no-color", "extra", "args"}, tfVersion, "workspace") - }) } } + +func TestRun_ShowInitOutputOnError(t *testing.T) { + // If there was an error during init then we want the output to be returned. + RegisterMockTestingT(t) + tfClient := mocks.NewMockClient() + When(tfClient.RunCommandWithVersion(matchers.AnyPtrToLoggingSimpleLogger(), AnyString(), AnyStringSlice(), matchers2.AnyPtrToGoVersionVersion(), AnyString())). + ThenReturn("output", errors.New("error")) + + tfVersion, _ := version.NewVersion("0.11.0") + iso := runtime.InitStepRunner{ + TerraformExecutor: tfClient, + DefaultTFVersion: tfVersion, + } + + output, err := iso.Run(models.ProjectCommandContext{ + Workspace: "workspace", + RepoRelDir: ".", + }, nil, "/path") + ErrEquals(t, "error", err) + Equals(t, "output", output) +}