Merge pull request #637 from Praqma/exit_code

Diff exit code

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{}

internal/app/command.go

@@ -67,14 +67,18 @@ 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
 	)
 
 	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)
 			}
 		}()
 	}

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)
 

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)

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+"-")
 		}
 	}

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:

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) {

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) {

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) {