Merge pull request #1137 from silvin-lubecki/stack-orchestrator

Fix broken swarm commands with Kubernetes defined as orchestrator
docker · Jun 22, 2018 · 151990d · 151990d
2 parents 8de0753 + f0a8598
commit 151990d
Show file tree

Hide file tree

Showing 33 changed files with 368 additions and 317 deletions.
diff --git a/cli/cobra.go b/cli/cobra.go
@@ -102,7 +102,7 @@ func managementSubCommands(cmd *cobra.Command) []*cobra.Command {
 var usageTemplate = `Usage:
 
 {{- if not .HasSubCommands}}	{{.UseLine}}{{end}}
-{{- if .HasSubCommands}}	{{ .CommandPath}} COMMAND{{end}}
+{{- if .HasSubCommands}}	{{ .CommandPath}}{{- if .HasAvailableFlags}} [OPTIONS]{{end}} COMMAND{{end}}
 
 {{ .Short | trim }}
 

diff --git a/cli/command/cli.go b/cli/command/cli.go
@@ -166,14 +166,9 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions) error {
 	if err != nil {
 		return errors.Wrap(err, "Experimental field")
 	}
-	orchestrator, err := GetOrchestrator(opts.Common.Orchestrator, cli.configFile.Orchestrator)
-	if err != nil {
-		return err
-	}
 	cli.clientInfo = ClientInfo{
 		DefaultVersion:  cli.client.ClientVersion(),
 		HasExperimental: hasExperimental,
-		Orchestrator:    orchestrator,
 	}
 	cli.initializeFromClient()
 	return nil
@@ -239,22 +234,6 @@ type ServerInfo struct {
 type ClientInfo struct {
 	HasExperimental bool
 	DefaultVersion  string
-	Orchestrator    Orchestrator
-}
-
-// HasKubernetes checks if kubernetes orchestrator is enabled
-func (c ClientInfo) HasKubernetes() bool {
-	return c.Orchestrator == OrchestratorKubernetes || c.Orchestrator == OrchestratorAll
-}
-
-// HasSwarm checks if swarm orchestrator is enabled
-func (c ClientInfo) HasSwarm() bool {
-	return c.Orchestrator == OrchestratorSwarm || c.Orchestrator == OrchestratorAll
-}
-
-// HasAll checks if all orchestrator is enabled
-func (c ClientInfo) HasAll() bool {
-	return c.Orchestrator == OrchestratorAll
 }
 
 // NewDockerCli returns a DockerCli instance with IO output and error streams set by in, out and err.

diff --git a/cli/command/cli_test.go b/cli/command/cli_test.go
@@ -161,110 +161,6 @@ func TestExperimentalCLI(t *testing.T) {
 	}
 }
 
-func TestOrchestratorSwitch(t *testing.T) {
-	defaultVersion := "v0.00"
-
-	var testcases = []struct {
-		doc                  string
-		configfile           string
-		envOrchestrator      string
-		flagOrchestrator     string
-		expectedOrchestrator string
-		expectedKubernetes   bool
-		expectedSwarm        bool
-	}{
-		{
-			doc: "default",
-			configfile: `{
-			}`,
-			expectedOrchestrator: "swarm",
-			expectedKubernetes:   false,
-			expectedSwarm:        true,
-		},
-		{
-			doc: "kubernetesConfigFile",
-			configfile: `{
-				"orchestrator": "kubernetes"
-			}`,
-			expectedOrchestrator: "kubernetes",
-			expectedKubernetes:   true,
-			expectedSwarm:        false,
-		},
-		{
-			doc: "kubernetesEnv",
-			configfile: `{
-			}`,
-			envOrchestrator:      "kubernetes",
-			expectedOrchestrator: "kubernetes",
-			expectedKubernetes:   true,
-			expectedSwarm:        false,
-		},
-		{
-			doc: "kubernetesFlag",
-			configfile: `{
-			}`,
-			flagOrchestrator:     "kubernetes",
-			expectedOrchestrator: "kubernetes",
-			expectedKubernetes:   true,
-			expectedSwarm:        false,
-		},
-		{
-			doc: "allOrchestratorFlag",
-			configfile: `{
-			}`,
-			flagOrchestrator:     "all",
-			expectedOrchestrator: "all",
-			expectedKubernetes:   true,
-			expectedSwarm:        true,
-		},
-		{
-			doc: "envOverridesConfigFile",
-			configfile: `{
-				"orchestrator": "kubernetes"
-			}`,
-			envOrchestrator:      "swarm",
-			expectedOrchestrator: "swarm",
-			expectedKubernetes:   false,
-			expectedSwarm:        true,
-		},
-		{
-			doc: "flagOverridesEnv",
-			configfile: `{
-			}`,
-			envOrchestrator:      "kubernetes",
-			flagOrchestrator:     "swarm",
-			expectedOrchestrator: "swarm",
-			expectedKubernetes:   false,
-			expectedSwarm:        true,
-		},
-	}
-
-	for _, testcase := range testcases {
-		t.Run(testcase.doc, func(t *testing.T) {
-			dir := fs.NewDir(t, testcase.doc, fs.WithFile("config.json", testcase.configfile))
-			defer dir.Remove()
-			apiclient := &fakeClient{
-				version: defaultVersion,
-			}
-			if testcase.envOrchestrator != "" {
-				defer env.Patch(t, "DOCKER_ORCHESTRATOR", testcase.envOrchestrator)()
-			}
-
-			cli := &DockerCli{client: apiclient, err: os.Stderr}
-			cliconfig.SetDir(dir.Path())
-			options := flags.NewClientOptions()
-			if testcase.flagOrchestrator != "" {
-				options.Common.Orchestrator = testcase.flagOrchestrator
-			}
-			err := cli.Initialize(options)
-			assert.NilError(t, err)
-			assert.Check(t, is.Equal(testcase.expectedKubernetes, cli.ClientInfo().HasKubernetes()))
-			assert.Check(t, is.Equal(testcase.expectedSwarm, cli.ClientInfo().HasSwarm()))
-			assert.Check(t, is.Equal(testcase.expectedOrchestrator, string(cli.ClientInfo().Orchestrator)))
-		})
-	}
-}
-
 func TestGetClientWithPassword(t *testing.T) {
 	expected := "password"
 

diff --git a/cli/command/orchestrator.go b/cli/command/orchestrator.go
@@ -17,10 +17,25 @@ const (
 	OrchestratorAll   = Orchestrator("all")
 	orchestratorUnset = Orchestrator("unset")
 
-	defaultOrchestrator      = OrchestratorSwarm
-	envVarDockerOrchestrator = "DOCKER_ORCHESTRATOR"
+	defaultOrchestrator           = OrchestratorSwarm
+	envVarDockerStackOrchestrator = "DOCKER_STACK_ORCHESTRATOR"
 )
 
+// HasKubernetes returns true if defined orchestrator has Kubernetes capabilities.
+func (o Orchestrator) HasKubernetes() bool {
+	return o == OrchestratorKubernetes || o == OrchestratorAll
+}
+
+// HasSwarm returns true if defined orchestrator has Swarm capabilities.
+func (o Orchestrator) HasSwarm() bool {
+	return o == OrchestratorSwarm || o == OrchestratorAll
+}
+
+// HasAll returns true if defined orchestrator has both Swarm and Kubernetes capabilities.
+func (o Orchestrator) HasAll() bool {
+	return o == OrchestratorAll
+}
+
 func normalize(value string) (Orchestrator, error) {
 	switch value {
 	case "kubernetes":
@@ -36,15 +51,15 @@ func normalize(value string) (Orchestrator, error) {
 	}
 }
 
-// GetOrchestrator checks DOCKER_ORCHESTRATOR environment variable and configuration file
+// GetStackOrchestrator checks DOCKER_STACK_ORCHESTRATOR environment variable and configuration file
 // orchestrator value and returns user defined Orchestrator.
-func GetOrchestrator(flagValue, value string) (Orchestrator, error) {
+func GetStackOrchestrator(flagValue, value string) (Orchestrator, error) {
 	// Check flag
 	if o, err := normalize(flagValue); o != orchestratorUnset {
 		return o, err
 	}
 	// Check environment variable
-	env := os.Getenv(envVarDockerOrchestrator)
+	env := os.Getenv(envVarDockerStackOrchestrator)
 	if o, err := normalize(env); o != orchestratorUnset {
 		return o, err
 	}

diff --git a/cli/command/orchestrator_test.go b/cli/command/orchestrator_test.go
@@ -0,0 +1,117 @@
+package command
+
+import (
+	"os"
+	"testing"
+
+	cliconfig "github.com/docker/cli/cli/config"
+	"github.com/docker/cli/cli/flags"
+	"gotest.tools/assert"
+	is "gotest.tools/assert/cmp"
+	"gotest.tools/env"
+	"gotest.tools/fs"
+)
+
+func TestOrchestratorSwitch(t *testing.T) {
+	defaultVersion := "v0.00"
+
+	var testcases = []struct {
+		doc                  string
+		configfile           string
+		envOrchestrator      string
+		flagOrchestrator     string
+		expectedOrchestrator string
+		expectedKubernetes   bool
+		expectedSwarm        bool
+	}{
+		{
+			doc: "default",
+			configfile: `{
+			}`,
+			expectedOrchestrator: "swarm",
+			expectedKubernetes:   false,
+			expectedSwarm:        true,
+		},
+		{
+			doc: "kubernetesConfigFile",
+			configfile: `{
+				"stackOrchestrator": "kubernetes"
+			}`,
+			expectedOrchestrator: "kubernetes",
+			expectedKubernetes:   true,
+			expectedSwarm:        false,
+		},
+		{
+			doc: "kubernetesEnv",
+			configfile: `{
+			}`,
+			envOrchestrator:      "kubernetes",
+			expectedOrchestrator: "kubernetes",
+			expectedKubernetes:   true,
+			expectedSwarm:        false,
+		},
+		{
+			doc: "kubernetesFlag",
+			configfile: `{
+			}`,
+			flagOrchestrator:     "kubernetes",
+			expectedOrchestrator: "kubernetes",
+			expectedKubernetes:   true,
+			expectedSwarm:        false,
+		},
+		{
+			doc: "allOrchestratorFlag",
+			configfile: `{
+			}`,
+			flagOrchestrator:     "all",
+			expectedOrchestrator: "all",
+			expectedKubernetes:   true,
+			expectedSwarm:        true,
+		},
+		{
+			doc: "envOverridesConfigFile",
+			configfile: `{
+				"stackOrchestrator": "kubernetes"
+			}`,
+			envOrchestrator:      "swarm",
+			expectedOrchestrator: "swarm",
+			expectedKubernetes:   false,
+			expectedSwarm:        true,
+		},
+		{
+			doc: "flagOverridesEnv",
+			configfile: `{
+			}`,
+			envOrchestrator:      "kubernetes",
+			flagOrchestrator:     "swarm",
+			expectedOrchestrator: "swarm",
+			expectedKubernetes:   false,
+			expectedSwarm:        true,
+		},
+	}
+
+	for _, testcase := range testcases {
+		t.Run(testcase.doc, func(t *testing.T) {
+			dir := fs.NewDir(t, testcase.doc, fs.WithFile("config.json", testcase.configfile))
+			defer dir.Remove()
+			apiclient := &fakeClient{
+				version: defaultVersion,
+			}
+			if testcase.envOrchestrator != "" {
+				defer env.Patch(t, "DOCKER_STACK_ORCHESTRATOR", testcase.envOrchestrator)()
+			}
+
+			cli := &DockerCli{client: apiclient, err: os.Stderr}
+			cliconfig.SetDir(dir.Path())
+			options := flags.NewClientOptions()
+			err := cli.Initialize(options)
+			assert.NilError(t, err)
+
+			orchestrator, err := GetStackOrchestrator(testcase.flagOrchestrator, cli.ConfigFile().StackOrchestrator)
+			assert.NilError(t, err)
+			assert.Check(t, is.Equal(testcase.expectedKubernetes, orchestrator.HasKubernetes()))
+			assert.Check(t, is.Equal(testcase.expectedSwarm, orchestrator.HasSwarm()))
+			assert.Check(t, is.Equal(testcase.expectedOrchestrator, string(orchestrator)))
+		})
+	}
+}