diff --git a/cmd/cmd.go b/cmd/cmd.go index 25a121fd..5ef9e717 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -7,9 +7,7 @@ import ( "github.com/roboll/helmfile/state" "github.com/urfave/cli" "go.uber.org/zap" - "os/exec" "strings" - "syscall" ) func VisitAllDesiredStates(c *cli.Context, converge func(*state.HelmState, helmexec.Interface, app.Context) (bool, []error)) error { @@ -86,14 +84,10 @@ func toCliError(err error) error { switch e := err.(type) { case *app.NoMatchingHelmfileError: return cli.NewExitError(e.Error(), 2) - case *exec.ExitError: - // Propagate any non-zero exit status from the external command like `helm` that is failed under the hood - status := e.Sys().(syscall.WaitStatus) - return cli.NewExitError(e.Error(), status.ExitStatus()) - case *state.DiffError: - return cli.NewExitError(e.Error(), e.Code) + case *app.Error: + return cli.NewExitError(e.Error(), e.Code()) default: - return cli.NewExitError(e.Error(), 1) + panic(fmt.Errorf("BUG: please file an github issue for this unhandled error: %T: %v", e, e)) } } return err diff --git a/helmexec/exec.go b/helmexec/exec.go index 15bdc52d..40c00719 100644 --- a/helmexec/exec.go +++ b/helmexec/exec.go @@ -6,7 +6,6 @@ import ( "io/ioutil" "os" "strings" - "sync" "go.uber.org/zap" @@ -183,7 +182,26 @@ func (helm *execer) DiffRelease(context HelmContext, name, chart string, flags . preArgs := context.GetTillerlessArgs(helm.helmBinary) env := context.getTillerlessEnv() out, err := helm.exec(append(append(preArgs, "diff", "upgrade", "--allow-unreleased", name, chart), flags...), env) - helm.write(out) + // Do our best to write STDOUT only when diff existed + // Unfortunately, this works only when you run helmfile with `--detailed-exitcode` + detailedExitcodeEnabled := false + for _, f := range flags { + if strings.Contains(f, "detailed-exitcode") { + detailedExitcodeEnabled = true + break + } + } + if detailedExitcodeEnabled { + switch e := err.(type) { + case ExitError: + if e.ExitStatus() == 2 { + helm.write(out) + return err + } + } + } else { + helm.write(out) + } return err } @@ -227,8 +245,11 @@ func (helm *execer) exec(args []string, env map[string]string) ([]byte, error) { if helm.kubeContext != "" { cmdargs = append(cmdargs, "--kube-context", helm.kubeContext) } - helm.logger.Debugf("exec: %s %s", helm.helmBinary, strings.Join(cmdargs, " ")) - return helm.runner.Execute(helm.helmBinary, cmdargs, env) + cmd := fmt.Sprintf("exec: %s %s", helm.helmBinary, strings.Join(cmdargs, " ")) + helm.logger.Debug(cmd) + bytes, err := helm.runner.Execute(helm.helmBinary, cmdargs, env) + helm.logger.Debugf("%s: %s", cmd, bytes) + return bytes, err } func (helm *execer) info(out []byte) { diff --git a/helmexec/exec_test.go b/helmexec/exec_test.go index 679d9af1..9db0aeb7 100644 --- a/helmexec/exec_test.go +++ b/helmexec/exec_test.go @@ -78,6 +78,7 @@ func Test_AddRepo(t *testing.T) { helm.AddRepo("myRepo", "https://repo.example.com/", "cert.pem", "key.pem", "", "") expected := `Adding repo myRepo https://repo.example.com/ exec: helm repo add myRepo https://repo.example.com/ --cert-file cert.pem --key-file key.pem --kube-context dev +exec: helm repo add myRepo https://repo.example.com/ --cert-file cert.pem --key-file key.pem --kube-context dev: ` if buffer.String() != expected { t.Errorf("helmexec.AddRepo()\nactual = %v\nexpect = %v", buffer.String(), expected) @@ -87,6 +88,7 @@ exec: helm repo add myRepo https://repo.example.com/ --cert-file cert.pem --key- helm.AddRepo("myRepo", "https://repo.example.com/", "", "", "", "") expected = `Adding repo myRepo https://repo.example.com/ exec: helm repo add myRepo https://repo.example.com/ --kube-context dev +exec: helm repo add myRepo https://repo.example.com/ --kube-context dev: ` if buffer.String() != expected { t.Errorf("helmexec.AddRepo()\nactual = %v\nexpect = %v", buffer.String(), expected) @@ -96,6 +98,7 @@ exec: helm repo add myRepo https://repo.example.com/ --kube-context dev helm.AddRepo("myRepo", "https://repo.example.com/", "", "", "example_user", "example_password") expected = `Adding repo myRepo https://repo.example.com/ exec: helm repo add myRepo https://repo.example.com/ --username example_user --password example_password --kube-context dev +exec: helm repo add myRepo https://repo.example.com/ --username example_user --password example_password --kube-context dev: ` if buffer.String() != expected { t.Errorf("helmexec.AddRepo()\nactual = %v\nexpect = %v", buffer.String(), expected) @@ -109,6 +112,7 @@ func Test_UpdateRepo(t *testing.T) { helm.UpdateRepo() expected := `Updating repo exec: helm repo update --kube-context dev +exec: helm repo update --kube-context dev: ` if buffer.String() != expected { t.Errorf("helmexec.UpdateRepo()\nactual = %v\nexpect = %v", buffer.String(), expected) @@ -122,6 +126,7 @@ func Test_SyncRelease(t *testing.T) { helm.SyncRelease(HelmContext{}, "release", "chart", "--timeout 10", "--wait") expected := `Upgrading chart exec: helm upgrade --install --reset-values release chart --timeout 10 --wait --kube-context dev +exec: helm upgrade --install --reset-values release chart --timeout 10 --wait --kube-context dev: ` if buffer.String() != expected { t.Errorf("helmexec.SyncRelease()\nactual = %v\nexpect = %v", buffer.String(), expected) @@ -131,6 +136,7 @@ exec: helm upgrade --install --reset-values release chart --timeout 10 --wait -- helm.SyncRelease(HelmContext{}, "release", "chart") expected = `Upgrading chart exec: helm upgrade --install --reset-values release chart --kube-context dev +exec: helm upgrade --install --reset-values release chart --kube-context dev: ` if buffer.String() != expected { t.Errorf("helmexec.SyncRelease()\nactual = %v\nexpect = %v", buffer.String(), expected) @@ -145,6 +151,7 @@ func Test_SyncReleaseTillerless(t *testing.T) { "--timeout 10", "--wait") expected := `Upgrading chart exec: helm tiller run foo -- helm upgrade --install --reset-values release chart --timeout 10 --wait --kube-context dev +exec: helm tiller run foo -- helm upgrade --install --reset-values release chart --timeout 10 --wait --kube-context dev: ` if buffer.String() != expected { t.Errorf("helmexec.SyncRelease()\nactual = %v\nexpect = %v", buffer.String(), expected) @@ -158,6 +165,7 @@ func Test_UpdateDeps(t *testing.T) { helm.UpdateDeps("./chart/foo") expected := `Updating dependency ./chart/foo exec: helm dependency update ./chart/foo --kube-context dev +exec: helm dependency update ./chart/foo --kube-context dev: ` if buffer.String() != expected { t.Errorf("helmexec.UpdateDeps()\nactual = %v\nexpect = %v", buffer.String(), expected) @@ -168,6 +176,7 @@ exec: helm dependency update ./chart/foo --kube-context dev helm.UpdateDeps("./chart/foo") expected = `Updating dependency ./chart/foo exec: helm dependency update ./chart/foo --verify --kube-context dev +exec: helm dependency update ./chart/foo --verify --kube-context dev: ` if buffer.String() != expected { t.Errorf("helmexec.AddRepo()\nactual = %v\nexpect = %v", buffer.String(), expected) @@ -181,6 +190,7 @@ func Test_BuildDeps(t *testing.T) { helm.BuildDeps("./chart/foo") expected := `Building dependency ./chart/foo exec: helm dependency build ./chart/foo --kube-context dev +exec: helm dependency build ./chart/foo --kube-context dev: ` if buffer.String() != expected { t.Errorf("helmexec.BuildDeps()\nactual = %v\nexpect = %v", buffer.String(), expected) @@ -191,6 +201,7 @@ exec: helm dependency build ./chart/foo --kube-context dev helm.BuildDeps("./chart/foo") expected = `Building dependency ./chart/foo exec: helm dependency build ./chart/foo --verify --kube-context dev +exec: helm dependency build ./chart/foo --verify --kube-context dev: ` if buffer.String() != expected { t.Errorf("helmexec.BuildDeps()\nactual = %v\nexpect = %v", buffer.String(), expected) @@ -204,6 +215,7 @@ func Test_DecryptSecret(t *testing.T) { helm.DecryptSecret(HelmContext{}, "secretName") expected := `Decrypting secret secretName exec: helm secrets dec secretName --kube-context dev +exec: helm secrets dec secretName --kube-context dev: ` if buffer.String() != expected { t.Errorf("helmexec.DecryptSecret()\nactual = %v\nexpect = %v", buffer.String(), expected) @@ -217,6 +229,7 @@ func Test_DiffRelease(t *testing.T) { helm.DiffRelease(HelmContext{}, "release", "chart", "--timeout 10", "--wait") expected := `Comparing release chart exec: helm diff upgrade --allow-unreleased release chart --timeout 10 --wait --kube-context dev +exec: helm diff upgrade --allow-unreleased release chart --timeout 10 --wait --kube-context dev: ` if buffer.String() != expected { t.Errorf("helmexec.DiffRelease()\nactual = %v\nexpect = %v", buffer.String(), expected) @@ -226,6 +239,7 @@ exec: helm diff upgrade --allow-unreleased release chart --timeout 10 --wait --k helm.DiffRelease(HelmContext{}, "release", "chart") expected = `Comparing release chart exec: helm diff upgrade --allow-unreleased release chart --kube-context dev +exec: helm diff upgrade --allow-unreleased release chart --kube-context dev: ` if buffer.String() != expected { t.Errorf("helmexec.DiffRelease()\nactual = %v\nexpect = %v", buffer.String(), expected) @@ -239,6 +253,7 @@ func Test_DiffReleaseTillerless(t *testing.T) { helm.DiffRelease(HelmContext{Tillerless: true}, "release", "chart", "--timeout 10", "--wait") expected := `Comparing release chart exec: helm tiller run -- helm diff upgrade --allow-unreleased release chart --timeout 10 --wait --kube-context dev +exec: helm tiller run -- helm diff upgrade --allow-unreleased release chart --timeout 10 --wait --kube-context dev: ` if buffer.String() != expected { t.Errorf("helmexec.DiffRelease()\nactual = %v\nexpect = %v", buffer.String(), expected) @@ -252,6 +267,7 @@ func Test_DeleteRelease(t *testing.T) { helm.DeleteRelease(HelmContext{}, "release") expected := `Deleting release exec: helm delete release --kube-context dev +exec: helm delete release --kube-context dev: ` if buffer.String() != expected { t.Errorf("helmexec.DeleteRelease()\nactual = %v\nexpect = %v", buffer.String(), expected) @@ -264,6 +280,7 @@ func Test_DeleteRelease_Flags(t *testing.T) { helm.DeleteRelease(HelmContext{}, "release", "--purge") expected := `Deleting release exec: helm delete release --purge --kube-context dev +exec: helm delete release --purge --kube-context dev: ` if buffer.String() != expected { t.Errorf("helmexec.DeleteRelease()\nactual = %v\nexpect = %v", buffer.String(), expected) @@ -277,6 +294,7 @@ func Test_TestRelease(t *testing.T) { helm.TestRelease(HelmContext{}, "release") expected := `Testing release exec: helm test release --kube-context dev +exec: helm test release --kube-context dev: ` if buffer.String() != expected { t.Errorf("helmexec.TestRelease()\nactual = %v\nexpect = %v", buffer.String(), expected) @@ -289,6 +307,7 @@ func Test_TestRelease_Flags(t *testing.T) { helm.TestRelease(HelmContext{}, "release", "--cleanup", "--timeout", "60") expected := `Testing release exec: helm test release --cleanup --timeout 60 --kube-context dev +exec: helm test release --cleanup --timeout 60 --kube-context dev: ` if buffer.String() != expected { t.Errorf("helmexec.TestRelease()\nactual = %v\nexpect = %v", buffer.String(), expected) @@ -302,6 +321,7 @@ func Test_ReleaseStatus(t *testing.T) { helm.ReleaseStatus(HelmContext{}, "myRelease") expected := `Getting status myRelease exec: helm status myRelease --kube-context dev +exec: helm status myRelease --kube-context dev: ` if buffer.String() != expected { t.Errorf("helmexec.ReleaseStatus()\nactual = %v\nexpect = %v", buffer.String(), expected) @@ -314,7 +334,9 @@ func Test_exec(t *testing.T) { helm := MockExecer(logger, "") env := map[string]string{} helm.exec([]string{"version"}, env) - expected := "exec: helm version\n" + expected := `exec: helm version +exec: helm version: +` if buffer.String() != expected { t.Errorf("helmexec.exec()\nactual = %v\nexpect = %v", buffer.String(), expected) } @@ -328,14 +350,18 @@ func Test_exec(t *testing.T) { buffer.Reset() helm = MockExecer(logger, "dev") helm.exec([]string{"diff", "release", "chart", "--timeout 10", "--wait"}, env) - expected = "exec: helm diff release chart --timeout 10 --wait --kube-context dev\n" + expected = `exec: helm diff release chart --timeout 10 --wait --kube-context dev +exec: helm diff release chart --timeout 10 --wait --kube-context dev: +` if buffer.String() != expected { t.Errorf("helmexec.exec()\nactual = %v\nexpect = %v", buffer.String(), expected) } buffer.Reset() helm.exec([]string{"version"}, env) - expected = "exec: helm version --kube-context dev\n" + expected = `exec: helm version --kube-context dev +exec: helm version --kube-context dev: +` if buffer.String() != expected { t.Errorf("helmexec.exec()\nactual = %v\nexpect = %v", buffer.String(), expected) } @@ -343,7 +369,9 @@ func Test_exec(t *testing.T) { buffer.Reset() helm.SetExtraArgs("foo") helm.exec([]string{"version"}, env) - expected = "exec: helm version foo --kube-context dev\n" + expected = `exec: helm version foo --kube-context dev +exec: helm version foo --kube-context dev: +` if buffer.String() != expected { t.Errorf("helmexec.exec()\nactual = %v\nexpect = %v", buffer.String(), expected) } @@ -352,7 +380,9 @@ func Test_exec(t *testing.T) { helm = MockExecer(logger, "") helm.SetHelmBinary("overwritten") helm.exec([]string{"version"}, env) - expected = "exec: overwritten version\n" + expected = `exec: overwritten version +exec: overwritten version: +` if buffer.String() != expected { t.Errorf("helmexec.exec()\nactual = %v\nexpect = %v", buffer.String(), expected) } @@ -365,6 +395,7 @@ func Test_Lint(t *testing.T) { helm.Lint("path/to/chart", "--values", "file.yml") expected := `Linting path/to/chart exec: helm lint path/to/chart --values file.yml --kube-context dev +exec: helm lint path/to/chart --values file.yml --kube-context dev: ` if buffer.String() != expected { t.Errorf("helmexec.Lint()\nactual = %v\nexpect = %v", buffer.String(), expected) @@ -378,6 +409,7 @@ func Test_Fetch(t *testing.T) { helm.Fetch("chart", "--version", "1.2.3", "--untar", "--untardir", "/tmp/dir") expected := `Fetching chart exec: helm fetch chart --version 1.2.3 --untar --untardir /tmp/dir --kube-context dev +exec: helm fetch chart --version 1.2.3 --untar --untardir /tmp/dir --kube-context dev: ` if buffer.String() != expected { t.Errorf("helmexec.Lint()\nactual = %v\nexpect = %v", buffer.String(), expected) @@ -387,6 +419,7 @@ exec: helm fetch chart --version 1.2.3 --untar --untardir /tmp/dir --kube-contex var logLevelTests = map[string]string{ "debug": `Adding repo myRepo https://repo.example.com/ exec: helm repo add myRepo https://repo.example.com/ --username example_user --password example_password +exec: helm repo add myRepo https://repo.example.com/ --username example_user --password example_password: `, "info": `Adding repo myRepo https://repo.example.com/ `, diff --git a/helmexec/exit_error.go b/helmexec/exit_error.go new file mode 100644 index 00000000..fd01132e --- /dev/null +++ b/helmexec/exit_error.go @@ -0,0 +1,36 @@ +package helmexec + +import ( + "fmt" + "path/filepath" + "strings" +) + +func newExitError(helmCmdPath string, exitStatus int, errorMessage string) ExitError { + return ExitError{ + msg: fmt.Sprintf("%s exited with status %d:\n%s", filepath.Base(helmCmdPath), exitStatus, indent(strings.TrimSpace(errorMessage))), + exitStatus: exitStatus, + } +} + +func indent(text string) string { + lines := strings.Split(text, "\n") + for i := range lines { + lines[i] = " " + lines[i] + } + return strings.Join(lines, "\n") +} + +// ExitError is created whenever your shell command exits with a non-zero exit status +type ExitError struct { + msg string + exitStatus int +} + +func (e ExitError) Error() string { + return e.msg +} + +func (e ExitError) ExitStatus() int { + return e.exitStatus +} diff --git a/helmexec/runner.go b/helmexec/runner.go index 59a75021..de2fe595 100644 --- a/helmexec/runner.go +++ b/helmexec/runner.go @@ -8,6 +8,7 @@ import ( "os" "os/exec" "strings" + "syscall" ) const ( @@ -51,17 +52,22 @@ func combinedOutput(c *exec.Cmd, logger *zap.SugaredLogger) ([]byte, error) { o := stdout.Bytes() e := stderr.Bytes() - if len(e) > 0 { - logger.Debugf("%s\n", e) - } - if err != nil { // TrimSpace is necessary, because otherwise helmfile prints the redundant new-lines after each error like: // // err: release "envoy2" in "helmfile.yaml" failed: exit status 1: Error: could not find a ready tiller pod // // err: release "envoy" in "helmfile.yaml" failed: exit status 1: Error: could not find a ready tiller pod - err = fmt.Errorf("%v: %s", err, strings.TrimSpace(string(e))) + switch ee := err.(type) { + case *exec.ExitError: + // Propagate any non-zero exit status from the external command, rather than throwing it away, + // so that helmfile could return its own exit code accordingly + waitStatus := ee.Sys().(syscall.WaitStatus) + exitStatus := waitStatus.ExitStatus() + err = newExitError(c.Path, exitStatus, string(e)) + default: + panic(fmt.Sprintf("unexpected error: %v", err)) + } } return o, err diff --git a/main.go b/main.go index 00a4352e..2e136d21 100644 --- a/main.go +++ b/main.go @@ -359,13 +359,19 @@ func main() { errs = append(errs, err) } + fatalErrs := []error{} + noError := true for _, e := range errs { switch err := e.(type) { - case *state.DiffError: - noError = noError && err.Code == 2 + case *state.ReleaseError: + if err.Code != 2 { + noError = false + fatalErrs = append(fatalErrs, e) + } default: noError = false + fatalErrs = append(fatalErrs, e) } } @@ -408,7 +414,7 @@ Do you really want to apply? } } - return errs + return fatalErrs }) affectedReleases.DisplayAffectedReleases(c.App.Metadata["logger"].(*zap.SugaredLogger)) return errs diff --git a/pkg/app/app.go b/pkg/app/app.go index 4e6d672e..ad61ce7f 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -6,6 +6,7 @@ import ( "log" "os" "os/signal" + "strings" "github.com/roboll/helmfile/helmexec" "github.com/roboll/helmfile/state" @@ -83,7 +84,7 @@ func (a *App) within(dir string, do func() error) error { func (a *App) visitStateFiles(fileOrDir string, do func(string) error) error { desiredStateFiles, err := a.findDesiredStateFiles(fileOrDir) if err != nil { - return err + return appError("", err) } for _, relPath := range desiredStateFiles { @@ -103,7 +104,7 @@ func (a *App) visitStateFiles(fileOrDir string, do func(string) error) error { return do(file) }) if err != nil { - return err + return appError(fmt.Sprintf("in %s/%s", dir, file), err) } } @@ -139,6 +140,8 @@ func (a *App) VisitDesiredStates(fileOrDir string, selector []string, converge f a.Env, ) + ctx := context{a, st} + helm := helmexec.New(a.Logger, a.KubeContext) if err != nil { @@ -149,17 +152,17 @@ func (a *App) VisitDesiredStates(fileOrDir string, selector []string, converge f case *state.UndefinedEnvError: return nil default: - return err + return ctx.wrapErrs(err) } default: - return err + return ctx.wrapErrs(err) } } st.Selectors = selector if len(st.Helmfiles) > 0 { noMatchInSubHelmfiles := true - for _, m := range st.Helmfiles { + for i, m := range st.Helmfiles { //assign parent selector to sub helm selector in legacy mode or do not inherit in experimental mode if (m.Selectors == nil && !isExplicitSelectorInheritanceEnabled()) || m.SelectorsInherited { m.Selectors = selector @@ -169,7 +172,7 @@ func (a *App) VisitDesiredStates(fileOrDir string, selector []string, converge f case *NoMatchingHelmfileError: default: - return fmt.Errorf("failed processing %s: %v", m.Path, err) + return appError(fmt.Sprintf("in .helmfiles[%d]", i), err) } } else { noMatchInSubHelmfiles = false @@ -180,11 +183,11 @@ func (a *App) VisitDesiredStates(fileOrDir string, selector []string, converge f templated, tmplErr := st.ExecuteTemplates() if tmplErr != nil { - return fmt.Errorf("failed executing release templates in \"%s\": %v", f, tmplErr) + return appError(fmt.Sprintf("failed executing release templates in \"%s\"", f), tmplErr) } processed, errs := converge(templated, helm) noMatchInHelmfiles = noMatchInHelmfiles && !processed - return clean(templated, errs) + return context{a, templated}.clean(errs) }) if err != nil { @@ -369,7 +372,7 @@ func (a *App) loadDesiredStateFromYaml(yaml []byte, file string, namespace strin sig := <-sigs errs := []error{fmt.Errorf("Received [%s] to shutdown ", sig)} - _ = clean(st, errs) + _ = context{a, st}.clean(errs) // See http://tldp.org/LDP/abs/html/exitcodes.html switch sig { case syscall.SIGINT: @@ -382,26 +385,101 @@ func (a *App) loadDesiredStateFromYaml(yaml []byte, file string, namespace strin return st, nil } -func clean(st *state.HelmState, errs []error) error { +type Error struct { + msg string + + Errors []error +} + +func (e *Error) Error() string { + var cause string + if e.Errors == nil { + return e.msg + } + if len(e.Errors) == 1 { + if e.Errors[0] == nil { + panic(fmt.Sprintf("[bug] assertion error: unexpected state: e.Errors: %v", e.Errors)) + } + cause = e.Errors[0].Error() + } else { + msgs := []string{} + for i, err := range e.Errors { + msgs = append(msgs, fmt.Sprintf("err %d: %v", i, err.Error())) + } + cause = fmt.Sprintf("%d errors:\n%s", len(e.Errors), strings.Join(msgs, "\n")) + } + msg := "" + if e.msg != "" { + msg = fmt.Sprintf("%s: %s", e.msg, cause) + } else { + msg = cause + } + return msg +} + +func (e *Error) Code() int { + allDiff := false + anyNonZero := false + for _, err := range e.Errors { + switch ee := err.(type) { + case *state.ReleaseError: + if anyNonZero { + allDiff = allDiff && ee.Code == 2 + } else { + allDiff = ee.Code == 2 + } + case *Error: + if anyNonZero { + allDiff = allDiff && ee.Code() == 2 + } else { + allDiff = ee.Code() == 2 + } + } + anyNonZero = true + } + + if anyNonZero { + if allDiff { + return 2 + } + return 1 + } + panic(fmt.Sprintf("[bug] assertion error: unexpected state: unable to handle errors: %v", e.Errors)) +} + +func appError(msg string, err error) error { + return &Error{msg, []error{err}} +} + +func (c context) clean(errs []error) error { if errs == nil { errs = []error{} } - cleanErrs := st.Clean() + cleanErrs := c.st.Clean() if cleanErrs != nil { errs = append(errs, cleanErrs...) } + return c.wrapErrs(errs...) +} + +type context struct { + app *App + st *state.HelmState +} + +func (c context) wrapErrs(errs ...error) error { if errs != nil && len(errs) > 0 { for _, err := range errs { switch e := err.(type) { case *state.ReleaseError: - fmt.Fprintf(os.Stderr, "err: release \"%s\" in \"%s\" failed: %v\n", e.Name, st.FilePath, e) + c.app.Logger.Debugf("err: release \"%s\" in \"%s\" failed: %v", e.Name, c.st.FilePath, e) default: - fmt.Fprintf(os.Stderr, "err: %v\n", e) + c.app.Logger.Debugf("err: %v", e) } } - return errs[0] + return &Error{Errors: errs} } return nil } diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index 99fe80be..5335e317 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -282,11 +282,11 @@ releases: errMsg string }{ {label: "name=prometheus", expectedCount: 1, expectErr: false}, - {label: "name=", expectedCount: 0, expectErr: true, errMsg: "failed processing /path/to/helmfile.d/a1.yaml: Malformed label: name=. Expected label in form k=v or k!=v"}, - {label: "name!=", expectedCount: 0, expectErr: true, errMsg: "failed processing /path/to/helmfile.d/a1.yaml: Malformed label: name!=. Expected label in form k=v or k!=v"}, - {label: "name", expectedCount: 0, expectErr: true, errMsg: "failed processing /path/to/helmfile.d/a1.yaml: Malformed label: name. Expected label in form k=v or k!=v"}, + {label: "name=", expectedCount: 0, expectErr: true, errMsg: "in ./helmfile.yaml: in .helmfiles[0]: in /path/to/helmfile.d/a1.yaml: Malformed label: name=. Expected label in form k=v or k!=v"}, + {label: "name!=", expectedCount: 0, expectErr: true, errMsg: "in ./helmfile.yaml: in .helmfiles[0]: in /path/to/helmfile.d/a1.yaml: Malformed label: name!=. Expected label in form k=v or k!=v"}, + {label: "name", expectedCount: 0, expectErr: true, errMsg: "in ./helmfile.yaml: in .helmfiles[0]: in /path/to/helmfile.d/a1.yaml: Malformed label: name. Expected label in form k=v or k!=v"}, // See https://github.com/roboll/helmfile/issues/193 - {label: "duplicated=yes", expectedCount: 0, expectErr: true, errMsg: "failed processing /path/to/helmfile.d/b.yaml: duplicate release \"foo\" found in \"zoo\": there were 2 releases named \"foo\" matching specified selector"}, + {label: "duplicated=yes", expectedCount: 0, expectErr: true, errMsg: "in ./helmfile.yaml: in .helmfiles[2]: in /path/to/helmfile.d/b.yaml: duplicate release \"foo\" found in \"zoo\": there were 2 releases named \"foo\" matching specified selector"}, {label: "duplicatedOK=yes", expectedCount: 2, expectErr: false}, } diff --git a/pkg/app/constants_test.go b/pkg/app/constants_test.go index c5137804..6be3529d 100644 --- a/pkg/app/constants_test.go +++ b/pkg/app/constants_test.go @@ -1,7 +1,7 @@ package app import ( - . "gotest.tools/assert" + "gotest.tools/assert" is "gotest.tools/assert/cmp" "gotest.tools/env" "os" @@ -10,18 +10,18 @@ import ( func TestIsExplicitSelectorInheritanceEnabled(t *testing.T) { //env var ExperimentalEnvVar is set - Assert(t, is.Equal(os.Getenv(ExperimentalEnvVar), "")) - Check(t, !isExplicitSelectorInheritanceEnabled()) + assert.Assert(t, is.Equal(os.Getenv(ExperimentalEnvVar), "")) + assert.Check(t, !isExplicitSelectorInheritanceEnabled()) //check for env var ExperimentalEnvVar set to true defer env.Patch(t, ExperimentalEnvVar, "true")() - Check(t, isExplicitSelectorInheritanceEnabled()) + assert.Check(t, isExplicitSelectorInheritanceEnabled()) //check for env var ExperimentalEnvVar set to anything defer env.Patch(t, ExperimentalEnvVar, "foo")() - Check(t, !isExplicitSelectorInheritanceEnabled()) + assert.Check(t, !isExplicitSelectorInheritanceEnabled()) //check for env var ExperimentalEnvVar set to ExperimentalSelectorExplicit defer env.Patch(t, ExperimentalEnvVar, ExperimentalSelectorExplicit)() - Check(t, isExplicitSelectorInheritanceEnabled()) + assert.Check(t, isExplicitSelectorInheritanceEnabled()) } diff --git a/state/release_error.go b/state/release_error.go new file mode 100644 index 00000000..809d4fcb --- /dev/null +++ b/state/release_error.go @@ -0,0 +1,19 @@ +package state + +import "fmt" + +const ReleaseErrorCodeFailure = 1 + +type ReleaseError struct { + *ReleaseSpec + err error + Code int +} + +func (e *ReleaseError) Error() string { + return fmt.Sprintf("failed processing release %s: %v", e.Name, e.err.Error()) +} + +func newReleaseError(release *ReleaseSpec, err error) *ReleaseError { + return &ReleaseError{release, err, ReleaseErrorCodeFailure} +} diff --git a/state/state.go b/state/state.go index fab5263f..e7fc5cb2 100644 --- a/state/state.go +++ b/state/state.go @@ -15,9 +15,6 @@ import ( "regexp" - "os/exec" - "syscall" - "net/url" "github.com/roboll/helmfile/environment" @@ -202,15 +199,6 @@ func (st *HelmState) SyncRepos(helm RepoUpdater) []error { return nil } -type ReleaseError struct { - *ReleaseSpec - underlying error -} - -func (e *ReleaseError) Error() string { - return e.underlying.Error() -} - type syncResult struct { errors []*ReleaseError } @@ -250,7 +238,7 @@ func (st *HelmState) prepareSyncReleases(helm helmexec.Interface, additionalValu flags, flagsErr := st.flagsForUpgrade(helm, release, workerIndex) if flagsErr != nil { - results <- syncPrepareResult{errors: []*ReleaseError{&ReleaseError{release, flagsErr}}} + results <- syncPrepareResult{errors: []*ReleaseError{newReleaseError(release, flagsErr)}} continue } @@ -258,14 +246,14 @@ func (st *HelmState) prepareSyncReleases(helm helmexec.Interface, additionalValu for _, value := range additionalValues { valfile, err := filepath.Abs(value) if err != nil { - errs = append(errs, &ReleaseError{release, err}) + errs = append(errs, newReleaseError(release, err)) } ok, err := st.fileExists(valfile) if err != nil { - errs = append(errs, &ReleaseError{release, err}) + errs = append(errs, newReleaseError(release, err)) } else if !ok { - errs = append(errs, &ReleaseError{release, fmt.Errorf("file does not exist: %s", valfile)}) + errs = append(errs, newReleaseError(release, fmt.Errorf("file does not exist: %s", valfile))) } flags = append(flags, "--values", valfile) } @@ -351,22 +339,22 @@ func (st *HelmState) SyncReleases(affectedReleases *AffectedReleases, helm helme context := st.createHelmContext(release, workerIndex) if _, err := st.triggerPresyncEvent(release, "sync"); err != nil { - relErr = &ReleaseError{release, err} + relErr = newReleaseError(release, err) } else if !release.Desired() { installed, err := st.isReleaseInstalled(context, helm, *release) if err != nil { - relErr = &ReleaseError{release, err} + relErr = newReleaseError(release, err) } else if installed { if err := helm.DeleteRelease(context, release.Name, "--purge"); err != nil { affectedReleases.Failed = append(affectedReleases.Failed, release) - relErr = &ReleaseError{release, err} + relErr = newReleaseError(release, err) } else { affectedReleases.Deleted = append(affectedReleases.Deleted, release) } } } else if err := helm.SyncRelease(context, release.Name, chart, flags...); err != nil { affectedReleases.Failed = append(affectedReleases.Failed, release) - relErr = &ReleaseError{release, err} + relErr = newReleaseError(release, err) } else { affectedReleases.Upgraded = append(affectedReleases.Upgraded, release) installedVersion, err := st.getDeployedVersion(context, helm, release) @@ -612,18 +600,8 @@ func (st *HelmState) LintReleases(helm helmexec.Interface, additionalValues []st return nil } -type DiffError struct { - *ReleaseSpec - err error - Code int -} - -func (e *DiffError) Error() string { - return e.err.Error() -} - type diffResult struct { - err *DiffError + err *ReleaseError } type diffPrepareResult struct { @@ -691,7 +669,7 @@ func (st *HelmState) prepareDiffReleases(helm helmexec.Interface, additionalValu if len(errs) > 0 { rsErrs := make([]*ReleaseError, len(errs)) for i, e := range errs { - rsErrs[i] = &ReleaseError{release, e} + rsErrs[i] = newReleaseError(release, e) } results <- diffPrepareResult{errors: rsErrs} } else { @@ -762,12 +740,11 @@ func (st *HelmState) DiffReleases(helm helmexec.Interface, additionalValues []st release := prep.release if err := helm.DiffRelease(st.createHelmContext(release, workerIndex), release.Name, normalizeChart(st.basePath, release.Chart), flags...); err != nil { switch e := err.(type) { - case *exec.ExitError: + case helmexec.ExitError: // Propagate any non-zero exit status from the external command like `helm` that is failed under the hood - status := e.Sys().(syscall.WaitStatus) - results <- diffResult{&DiffError{release, err, status.ExitStatus()}} + results <- diffResult{&ReleaseError{release, err, e.ExitStatus()}} default: - results <- diffResult{&DiffError{release, err, 0}} + results <- diffResult{&ReleaseError{release, err, 0}} } } else { // diff succeeded, found no changes @@ -925,7 +902,7 @@ func (st *HelmState) PrepareRelease(helm helmexec.Interface, helmfileCommand str for _, release := range st.Releases { if _, err := st.triggerPrepareEvent(&release, helmfileCommand); err != nil { - errs = append(errs, &ReleaseError{&release, err}) + errs = append(errs, newReleaseError(&release, err)) continue } }