diff --git a/internal/app/cli.go b/internal/app/cli.go index 99a3f09c..f5779822 100644 --- a/internal/app/cli.go +++ b/internal/app/cli.go @@ -259,7 +259,9 @@ func (c *cli) readState(s *state) error { if len(c.spec) > 0 { sp := new(StateFiles) - sp.specFromYAML(c.spec) + if err := sp.specFromYAML(c.spec); err != nil { + return fmt.Errorf("error parsing spec file: %w", err) + } for _, val := range sp.StateFiles { fo := fileOption{} diff --git a/internal/app/command.go b/internal/app/command.go index 76a09d7a..e8b660eb 100644 --- a/internal/app/command.go +++ b/internal/app/command.go @@ -67,6 +67,10 @@ func (c *Command) String() string { // RetryExec runs exec command with retry func (c *Command) RetryExec(attempts int) (ExitStatus, error) { + return c.RetryExecWithThreshold(attempts, 0) +} + +func (c *Command) RetryExecWithThreshold(attempts, exitCodeThreshold int) (ExitStatus, error) { var ( result ExitStatus err error @@ -74,7 +78,7 @@ func (c *Command) RetryExec(attempts int) (ExitStatus, error) { for i := 0; i < attempts; i++ { result, err = c.Exec() - if err == nil { + if err == nil || (result.code >= 0 && result.code <= exitCodeThreshold) { return result, nil } if i < (attempts - 1) { @@ -123,7 +127,7 @@ func (c *Command) Exec() (ExitStatus, error) { errors: strings.TrimSpace(stderr.String()), } if err != nil { - res.code = 1 + res.code = 126 if exiterr, ok := err.(*exec.ExitError); ok { res.code = exiterr.ExitCode() } @@ -164,7 +168,7 @@ func (p CmdPipe) Exec() (ExitStatus, error) { errors: strings.TrimSpace(stderr.String()), } if err != nil { - res.code = 1 + res.code = 126 if exiterr, ok := err.(*exec.ExitError); ok { res.code = exiterr.ExitCode() } @@ -175,6 +179,10 @@ func (p CmdPipe) Exec() (ExitStatus, error) { // RetryExec runs piped commands with retry func (p CmdPipe) RetryExec(attempts int) (ExitStatus, error) { + return p.RetryExecWithThreshold(attempts, 0) +} + +func (p CmdPipe) RetryExecWithThreshold(attempts, exitCodeThreshold int) (ExitStatus, error) { var ( result ExitStatus err error @@ -183,7 +191,7 @@ func (p CmdPipe) RetryExec(attempts int) (ExitStatus, error) { l := len(p) - 1 for i := 0; i < attempts; i++ { result, err = p.Exec() - if err == nil { + if err == nil || (result.code >= 0 && result.code <= exitCodeThreshold) { return result, nil } if i < (attempts - 1) { @@ -209,7 +217,10 @@ func call(stack []*exec.Cmd) (err error) { if err == nil { err = call(stack[1:]) } else { - stack[1].Wait() + err = stack[1].Wait() + } + if err != nil { + log.Infof("call: %v", err) } }() } diff --git a/internal/app/decision_maker_test.go b/internal/app/decision_maker_test.go index 3c9ccb7d..6f148741 100644 --- a/internal/app/decision_maker_test.go +++ b/internal/app/decision_maker_test.go @@ -112,7 +112,9 @@ func Test_inspectUpgradeScenario(t *testing.T) { // Act c, _ := getChartInfo(tt.args.r.Chart, tt.args.r.Version) - cs.inspectUpgradeScenario(tt.args.r, &outcome, c) + if err := cs.inspectUpgradeScenario(tt.args.r, &outcome, c); err != nil { + t.Errorf("inspectUpgradeScenario() error = %v", err) + } got := outcome.Decisions[0].Type t.Log(outcome.Decisions[0].Description) diff --git a/internal/app/helm_helpers_test.go b/internal/app/helm_helpers_test.go index e08f03ee..f816f891 100644 --- a/internal/app/helm_helpers_test.go +++ b/internal/app/helm_helpers_test.go @@ -72,7 +72,7 @@ func Test_getChartInfo(t *testing.T) { t.Run(tt.name, func(t *testing.T) { got, err := getChartInfo(tt.args.r.Chart, tt.args.r.Version) if err != nil && tt.want != nil { - t.Errorf("getChartInfo() = Unexpected error: %w", err) + t.Errorf("getChartInfo() = Unexpected error: %v", err) } if !reflect.DeepEqual(got, tt.want) { t.Errorf("getChartInfo() = %v, want %v", got, tt.want) diff --git a/internal/app/kube_helpers.go b/internal/app/kube_helpers.go index f175ea92..e7771a63 100644 --- a/internal/app/kube_helpers.go +++ b/internal/app/kube_helpers.go @@ -78,10 +78,10 @@ func labelNamespace(ns string, labels map[string]string, authoritative bool) { // ignore default k8s namespace label from being removed delete(nsLabels, "kubernetes.io/metadata.name") // ignore every label defined in DSF for the namespace from being removed - for definedLabelKey, _ := range labels { + for definedLabelKey := range labels { delete(nsLabels, definedLabelKey) } - for label, _ := range nsLabels { + for label := range nsLabels { args = append(args, label+"-") } } diff --git a/internal/app/release.go b/internal/app/release.go index e9fd82b9..5f4613ab 100644 --- a/internal/app/release.go +++ b/internal/app/release.go @@ -166,7 +166,10 @@ func (r *release) uninstall(p *plan, optionalNamespace ...string) { // diffRelease diffs an existing release with the specified values.yaml func (r *release) diff() (string, error) { - var args []string + var ( + args []string + maxExitCode int + ) if !flags.kubectlDiff { args = []string{"diff", "--suppress-secrets"} @@ -186,9 +189,10 @@ func (r *release) diff() (string, error) { if flags.kubectlDiff { cmd = append(cmd, kubectl([]string{"diff", "--namespace", r.Namespace, "-f", "-"}, desc)) + maxExitCode = 1 } - res, err := cmd.RetryExec(3) + res, err := cmd.RetryExecWithThreshold(3, maxExitCode) if err != nil { if flags.kubectlDiff && res.code <= 1 { // kubectl diff exit status: diff --git a/internal/app/spec_state_test.go b/internal/app/spec_state_test.go index dc209414..80fa3686 100644 --- a/internal/app/spec_state_test.go +++ b/internal/app/spec_state_test.go @@ -31,7 +31,10 @@ func Test_specFromYAML(t *testing.T) { }, } - teardownTestCase := setupStateFileTestCase(t) + teardownTestCase, err := setupStateFileTestCase(t) + if err != nil { + t.Fatal(err) + } defer teardownTestCase(t) for _, tt := range tests { // os.Args = append(os.Args, "-f ../../examples/example.yaml") @@ -72,7 +75,10 @@ func Test_specFileSort(t *testing.T) { }, } - teardownTestCase := setupStateFileTestCase(t) + teardownTestCase, err := setupStateFileTestCase(t) + if err != nil { + t.Fatal(err) + } defer teardownTestCase(t) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/internal/app/state_files_test.go b/internal/app/state_files_test.go index 82c670c5..235cda6c 100644 --- a/internal/app/state_files_test.go +++ b/internal/app/state_files_test.go @@ -6,14 +6,17 @@ import ( "testing" ) -func setupStateFileTestCase(t *testing.T) func(t *testing.T) { +func setupStateFileTestCase(t *testing.T) (func(t *testing.T), error) { t.Log("setup test case") - os.MkdirAll(tempFilesDir, 0o755) + if err := os.MkdirAll(tempFilesDir, 0o755); err != nil { + t.Errorf("setupStateFileTestCase(), failed to create temp files dir: %v", err) + return nil, err + } return func(t *testing.T) { t.Log("teardown test case") os.RemoveAll(tempFilesDir) - } + }, nil } func Test_fromTOML(t *testing.T) { @@ -45,7 +48,10 @@ func Test_fromTOML(t *testing.T) { os.Setenv("ORG_PATH", "sample") os.Setenv("VALUE", "sample") - teardownTestCase := setupStateFileTestCase(t) + teardownTestCase, err := setupStateFileTestCase(t) + if err != nil { + t.Errorf("setupStateFileTestCase(), got: %v", err) + } defer teardownTestCase(t) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -97,7 +103,10 @@ func Test_fromTOML_Expand(t *testing.T) { os.Setenv("ORG_PATH", "sample") os.Setenv("VALUE", "sample") - teardownTestCase := setupStateFileTestCase(t) + teardownTestCase, err := setupStateFileTestCase(t) + if err != nil { + t.Errorf("setupStateFileTestCase(), got: %v", err) + } defer teardownTestCase(t) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -179,7 +188,10 @@ func Test_fromYAML(t *testing.T) { os.Setenv("VALUE", "sample") os.Setenv("ORG_PATH", "sample") - teardownTestCase := setupStateFileTestCase(t) + teardownTestCase, err := setupStateFileTestCase(t) + if err != nil { + t.Errorf("setupStateFileTestCase(), got: %v", err) + } defer teardownTestCase(t) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -225,13 +237,17 @@ func Test_fromYAML_UnsetVars(t *testing.T) { }, } - teardownTestCase := setupStateFileTestCase(t) + teardownTestCase, err := setupStateFileTestCase(t) + if err != nil { + t.Errorf("setupStateFileTestCase(), got: %v", err) + } defer teardownTestCase(t) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if tt.targetVar == "ORG_PATH" { + switch tt.targetVar { + case "ORG_PATH": os.Setenv("VALUE", "sample") - } else if tt.targetVar == "VALUE" { + case "VALUE": os.Setenv("ORG_PATH", "sample") } err := tt.args.s.fromYAML(tt.args.file) @@ -282,7 +298,10 @@ func Test_fromYAML_Expand(t *testing.T) { os.Setenv("ORG_PATH", "sample") os.Setenv("VALUE", "sample") - teardownTestCase := setupStateFileTestCase(t) + teardownTestCase, err := setupStateFileTestCase(t) + if err != nil { + t.Errorf("setupStateFileTestCase(), got: %v", err) + } defer teardownTestCase(t) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/internal/app/state_test.go b/internal/app/state_test.go index 5330a2be..26cbe03d 100644 --- a/internal/app/state_test.go +++ b/internal/app/state_test.go @@ -5,11 +5,17 @@ import ( "testing" ) -func setupTestCase(t *testing.T) func(t *testing.T) { +func setupTestCase(t *testing.T) (func(t *testing.T), error) { t.Log("setup test case") - os.MkdirAll(tempFilesDir, 0o755) - os.MkdirAll(os.TempDir()+"/helmsman-tests/myapp", os.ModePerm) - os.MkdirAll(os.TempDir()+"/helmsman-tests/dir-with space/myapp", os.ModePerm) + if err := os.MkdirAll(tempFilesDir, 0o755); err != nil { + return nil, err + } + if err := os.MkdirAll(os.TempDir()+"/helmsman-tests/myapp", os.ModePerm); err != nil { + return nil, err + } + if err := os.MkdirAll(os.TempDir()+"/helmsman-tests/dir-with space/myapp", os.ModePerm); err != nil { + return nil, err + } cmd := helmCmd([]string{"create", os.TempDir() + "/helmsman-tests/dir-with space/myapp"}, "creating an empty local chart directory") if _, err := cmd.Exec(); err != nil { log.Fatalf("Command failed: %v", err) @@ -19,7 +25,7 @@ func setupTestCase(t *testing.T) func(t *testing.T) { t.Log("teardown test case") os.RemoveAll(tempFilesDir) os.RemoveAll(os.TempDir() + "/helmsman-tests/") - } + }, nil } func Test_state_validate(t *testing.T) { @@ -494,7 +500,11 @@ func Test_state_getReleaseChartsInfo(t *testing.T) { }, } - teardownTestCase := setupTestCase(t) + teardownTestCase, err := setupTestCase(t) + if err != nil { + t.Errorf("setupTestCase() = %v", err) + return + } defer teardownTestCase(t) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {