Merge pull request #6790 from mumoshu/fix-v3-regression-non-zero-plugin-exit-status

fix(v3): fix regression on non-zero plugin exist status

cmd/helm/helm.go

@@ -68,6 +68,11 @@ func main() {
 	cmd := newRootCmd(actionConfig, os.Stdout, os.Args[1:])
 
 	if err := actionConfig.Init(settings, false, os.Getenv("HELM_DRIVER"), debug); err != nil {
+		debug("%+v", err)
+		os.Exit(1)
+	}
+
+	if err := cmd.Execute(); err != nil {
 		debug("%+v", err)
 		switch e := err.(type) {
 		case pluginError:
@@ -76,11 +81,6 @@ func main() {
 			os.Exit(1)
 		}
 	}
-
-	if err := cmd.Execute(); err != nil {
-		debug("%+v", err)
-		os.Exit(1)
-	}
 }
 
 // wordSepNormalizeFunc changes all flags that contain "_" separators

cmd/helm/helm_test.go

@@ -20,6 +20,8 @@ import (
 	"bytes"
 	"io/ioutil"
 	"os"
+	"os/exec"
+	"runtime"
 	"strings"
 	"testing"
 
@@ -151,3 +153,59 @@ func testChdir(t *testing.T, dir string) func() {
 	}
 	return func() { os.Chdir(old) }
 }
+
+func TestPluginExitCode(t *testing.T) {
+	if os.Getenv("RUN_MAIN_FOR_TESTING") == "1" {
+		os.Args = []string{"helm", "exitwith", "2"}
+
+		// We DO call helm's main() here. So this looks like a normal `helm` process.
+		main()
+
+		// As main calls os.Exit, we never reach this line.
+		// But the test called this block of code catches and verifies the exit code.
+		return
+	}
+
+	// Currently, plugins assume a Linux subsystem. Skip the execution
+	// tests until this is fixed
+	if runtime.GOOS != "windows" {
+		// Do a second run of this specific test(TestPluginExitCode) with RUN_MAIN_FOR_TESTING=1 set,
+		// So that the second run is able to run main() and this first run can verify the exit status returned by that.
+		//
+		// This technique originates from https://talks.golang.org/2014/testing.slide#23.
+		cmd := exec.Command(os.Args[0], "-test.run=TestPluginExitCode")
+		cmd.Env = append(
+			os.Environ(),
+			"RUN_MAIN_FOR_TESTING=1",
+			// See pkg/cli/environment.go for which envvars can be used for configuring these passes
+			// and also see plugin_test.go for how a plugin env can be set up.
+			// We just does the same setup as plugin_test.go via envvars
+			"HELM_PLUGINS=testdata/helmhome/helm/plugins",
+			"HELM_REPOSITORY_CONFIG=testdata/helmhome/helm/repositories.yaml",
+			"HELM_REPOSITORY_CACHE=testdata/helmhome/helm/repository",
+		)
+		stdout := &bytes.Buffer{}
+		stderr := &bytes.Buffer{}
+		cmd.Stdout = stdout
+		cmd.Stderr = stderr
+		err := cmd.Run()
+		exiterr, ok := err.(*exec.ExitError)
+
+		if !ok {
+			t.Fatalf("Unexpected error returned by os.Exit: %T", err)
+		}
+
+		if stdout.String() != "" {
+			t.Errorf("Expected no write to stdout: Got %q", stdout.String())
+		}
+
+		expectedStderr := "Error: plugin \"exitwith\" exited with error\n"
+		if stderr.String() != expectedStderr {
+			t.Errorf("Expected %q written to stderr: Got %q", expectedStderr, stderr.String())
+		}
+
+		if exiterr.ExitCode() != 2 {
+			t.Errorf("Expected exit code 2: Got %d", exiterr.ExitCode())
+		}
+	}
+}

cmd/helm/plugin_test.go

@@ -86,11 +86,13 @@ func TestLoadPlugins(t *testing.T) {
 		long   string
 		expect string
 		args   []string
+		code   int
 	}{
-		{"args", "echo args", "This echos args", "-a -b -c\n", []string{"-a", "-b", "-c"}},
-		{"echo", "echo stuff", "This echos stuff", "hello\n", []string{}},
-		{"env", "env stuff", "show the env", "env\n", []string{}},
-		{"fullenv", "show env vars", "show all env vars", envs + "\n", []string{}},
+		{"args", "echo args", "This echos args", "-a -b -c\n", []string{"-a", "-b", "-c"}, 0},
+		{"echo", "echo stuff", "This echos stuff", "hello\n", []string{}, 0},
+		{"env", "env stuff", "show the env", "env\n", []string{}, 0},
+		{"exitwith", "exitwith code", "This exits with the specified exit code", "", []string{"2"}, 2},
+		{"fullenv", "show env vars", "show all env vars", envs + "\n", []string{}, 0},
 	}
 
 	plugins := cmd.Commands()
@@ -117,7 +119,17 @@ func TestLoadPlugins(t *testing.T) {
 		// tests until this is fixed
 		if runtime.GOOS != "windows" {
 			if err := pp.RunE(pp, tt.args); err != nil {
-				t.Errorf("Error running %s: %+v", tt.use, err)
+				if tt.code > 0 {
+					perr, ok := err.(pluginError)
+					if !ok {
+						t.Errorf("Expected %s to return pluginError: got %v(%T)", tt.use, err, err)
+					}
+					if perr.code != tt.code {
+						t.Errorf("Expected %s to return %d: got %d", tt.use, tt.code, perr.code)
+					}
+				} else {
+					t.Errorf("Error running %s: %+v", tt.use, err)
+				}
 			}
 			if out.String() != tt.expect {
 				t.Errorf("Expected %s to output:\n%s\ngot\n%s", tt.use, tt.expect, out.String())

cmd/helm/testdata/helmhome/helm/plugins/exitwith/exitwith.sh

@@ -0,0 +1,2 @@
+#!/bin/bash
+exit $*

cmd/helm/testdata/helmhome/helm/plugins/exitwith/plugin.yaml

@@ -0,0 +1,4 @@
+name: exitwith
+usage: "exitwith code"
+description: "This exits with the specified exit code"
+command: "$HELM_PLUGIN_DIR/exitwith.sh"