From 29cc59cf42eb5fd7e04b3415b4ef2bca18ccdf05 Mon Sep 17 00:00:00 2001
From: aiordache <anca.iordache@docker.com>
Date: Fri, 5 Mar 2021 17:45:04 +0100
Subject: [PATCH] Return exit code 1 if engine error on version query

Signed-off-by: aiordache <anca.iordache@docker.com>
---
 api/context/context.go         |  2 +-
 cli/cmd/version.go             | 28 ++++++++++++++++++----------
 local/e2e/cli-only/e2e_test.go | 14 +++++++-------
 3 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/api/context/context.go b/api/context/context.go
index b0c1f64e9..27ac5e36f 100644
--- a/api/context/context.go
+++ b/api/context/context.go
@@ -43,7 +43,7 @@ func WithCliOptions(ctx gocontext.Context, options cliflags.CommonOptions) conte
 	return context.WithValue(ctx, cliOptionsKey{}, options)
 }
 
-// CliOptions returns the current context name
+// CliOptions returns common cli options
 func CliOptions(ctx context.Context) cliflags.CommonOptions {
 	cc, _ := ctx.Value(cliOptionsKey{}).(cliflags.CommonOptions)
 	return cc
diff --git a/cli/cmd/version.go b/cli/cmd/version.go
index 89ea64770..878d0b1e5 100644
--- a/cli/cmd/version.go
+++ b/cli/cmd/version.go
@@ -36,8 +36,12 @@ func VersionCommand() *cobra.Command {
 		Use:   "version",
 		Short: "Show the Docker version information",
 		Args:  cobra.MaximumNArgs(0),
-		Run: func(cmd *cobra.Command, _ []string) {
-			runVersion(cmd)
+		RunE: func(cmd *cobra.Command, _ []string) error {
+			err := runVersion(cmd)
+			if err != nil {
+				return ExitCodeError{ExitCode: 1}
+			}
+			return nil
 		},
 	}
 	// define flags for backward compatibility with com.docker.cli
@@ -48,34 +52,38 @@ func VersionCommand() *cobra.Command {
 	return cmd
 }
 
-func runVersion(cmd *cobra.Command) {
+func runVersion(cmd *cobra.Command) error {
 	var versionString string
+	var err error
 	format := strings.ToLower(strings.ReplaceAll(cmd.Flag(formatOpt).Value.String(), " ", ""))
 	displayedVersion := strings.TrimPrefix(internal.Version, "v")
 	// Replace is preferred in this case to keep the order.
 	switch format {
 	case formatter.PRETTY, "":
-		versionString = strings.Replace(getOutFromMoby(cmd, fixedPrettyArgs(os.Args[1:])...),
+		versionString, err = getOutFromMoby(cmd, fixedPrettyArgs(os.Args[1:])...)
+		versionString = strings.Replace(versionString,
 			"\n Version:", "\n Cloud integration: "+displayedVersion+"\n Version:", 1)
 	case formatter.JSON, formatter.TemplateLegacyJSON: // Try to catch full JSON formats
-		versionString = strings.Replace(getOutFromMoby(cmd, fixedJSONArgs(os.Args[1:])...),
+		versionString, err = getOutFromMoby(cmd, fixedJSONArgs(os.Args[1:])...)
+		versionString = strings.Replace(versionString,
 			`"Version":`, fmt.Sprintf(`"CloudIntegration":%q,"Version":`, displayedVersion), 1)
 	default:
-		versionString = getOutFromMoby(cmd)
+		versionString, err = getOutFromMoby(cmd)
 	}
 
 	fmt.Print(versionString)
+	return err
 }
 
-func getOutFromMoby(cmd *cobra.Command, args ...string) string {
-	versionResult, _ := mobycli.ExecSilent(cmd.Context(), args...)
+func getOutFromMoby(cmd *cobra.Command, args ...string) (string, error) {
+	versionResult, err := mobycli.ExecSilent(cmd.Context(), args...)
 	// we don't want to fail on error, there is an error if the engine is not available but it displays client version info
 	// Still, technically the [] byte versionResult could be nil, just let the original command display what it has to display
 	if versionResult == nil {
 		mobycli.Exec(cmd.Root())
-		return ""
+		return "", nil
 	}
-	return string(versionResult)
+	return string(versionResult), err
 }
 
 func fixedPrettyArgs(oArgs []string) []string {
diff --git a/local/e2e/cli-only/e2e_test.go b/local/e2e/cli-only/e2e_test.go
index d02338267..689cc56ba 100644
--- a/local/e2e/cli-only/e2e_test.go
+++ b/local/e2e/cli-only/e2e_test.go
@@ -396,14 +396,14 @@ func TestLegacy(t *testing.T) {
 	})
 
 	t.Run("host flag", func(t *testing.T) {
-		stderr := []string{"dial tcp: lookup nonexistent", "no such host"}
-		res := c.RunDockerOrExitError("-H", "tcp://nonexistent:123", "version")
-		res.Assert(t, icmd.Expected{
-			ExitCode: 1,
-		})
-		for _, s := range stderr {
-			assert.Assert(t, strings.Contains(res.Stderr(), s), res.Stderr())
+		stderr := "dial tcp: lookup nonexistent"
+		if runtime.GOOS == "windows" {
+			stderr = "dial tcp: lookup nonexistent: no such host"
 		}
+		res := c.RunDockerOrExitError("-H", "tcp://nonexistent:123", "version")
+		assert.Assert(t, res.ExitCode == 1)
+		assert.Assert(t, strings.Contains(res.Stdout(), stderr), res.Stdout())
+
 	})
 
 	t.Run("existing contexts delegate", func(t *testing.T) {