Skip to content

Commit ad40976

Browse files
silvin-lubeckisilvin-lubecki
committed
Activate kubernetes only when experimental cli is enabled
* Refactor tests on version and kubernetes switch * Fix rebase errors * Refactor for gocyclo linter Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
1 parent 5d375b3 commit ad40976

11 files changed

Lines changed: 163 additions & 65 deletions

File tree

‎cli/command/cli.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,11 +136,11 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions) error {
136136
if err != nil {
137137
return errors.Wrap(err, "Experimental field")
138138
}
139-
orchestrator := GetOrchestrator(opts.Common.Orchestrator, cli.configFile.Orchestrator)
139+
orchestrator := GetOrchestrator(hasExperimental, opts.Common.Orchestrator, cli.configFile.Orchestrator)
140140
cli.clientInfo = ClientInfo{
141141
DefaultVersion: cli.client.ClientVersion(),
142142
HasExperimental: hasExperimental,
143-
HasKubernetes: orchestrator == OrchestratorKubernetes,
143+
HasKubernetes: hasExperimental && orchestrator == OrchestratorKubernetes,
144144
Orchestrator: orchestrator,
145145
}
146146
cli.initializeFromClient()

‎cli/command/cli_test.go‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,110 @@ func TestExperimentalCLI(t *testing.T) {
170170
}
171171
}
172172

173+
func TestOrchestratorSwitch(t *testing.T) {
174+
defaultVersion := "v1.55"
175+
176+
var testcases = []struct {
177+
doc string
178+
configfile string
179+
envOrchestrator string
180+
flagOrchestrator string
181+
expectedOrchestrator string
182+
expectedKubernetes bool
183+
}{
184+
{
185+
doc: "default",
186+
configfile: `{
187+
"experimental": "enabled"
188+
}`,
189+
expectedOrchestrator: "swarm",
190+
expectedKubernetes: false,
191+
},
192+
{
193+
doc: "kubernetesIsExperimental",
194+
configfile: `{
195+
"experimental": "disabled",
196+
"orchestrator": "kubernetes"
197+
}`,
198+
envOrchestrator: "kubernetes",
199+
flagOrchestrator: "kubernetes",
200+
expectedOrchestrator: "swarm",
201+
expectedKubernetes: false,
202+
},
203+
{
204+
doc: "kubernetesConfigFile",
205+
configfile: `{
206+
"experimental": "enabled",
207+
"orchestrator": "kubernetes"
208+
}`,
209+
expectedOrchestrator: "kubernetes",
210+
expectedKubernetes: true,
211+
},
212+
{
213+
doc: "kubernetesEnv",
214+
configfile: `{
215+
"experimental": "enabled"
216+
}`,
217+
envOrchestrator: "kubernetes",
218+
expectedOrchestrator: "kubernetes",
219+
expectedKubernetes: true,
220+
},
221+
{
222+
doc: "kubernetesFlag",
223+
configfile: `{
224+
"experimental": "enabled"
225+
}`,
226+
flagOrchestrator: "kubernetes",
227+
expectedOrchestrator: "kubernetes",
228+
expectedKubernetes: true,
229+
},
230+
{
231+
doc: "envOverridesConfigFile",
232+
configfile: `{
233+
"experimental": "enabled",
234+
"orchestrator": "kubernetes"
235+
}`,
236+
envOrchestrator: "swarm",
237+
expectedOrchestrator: "swarm",
238+
expectedKubernetes: false,
239+
},
240+
{
241+
doc: "flagOverridesEnv",
242+
configfile: `{
243+
"experimental": "enabled"
244+
}`,
245+
envOrchestrator: "kubernetes",
246+
flagOrchestrator: "swarm",
247+
expectedOrchestrator: "swarm",
248+
expectedKubernetes: false,
249+
},
250+
}
251+
252+
for _, testcase := range testcases {
253+
t.Run(testcase.doc, func(t *testing.T) {
254+
dir := fs.NewDir(t, testcase.doc, fs.WithFile("config.json", testcase.configfile))
255+
defer dir.Remove()
256+
apiclient := &fakeClient{
257+
version: defaultVersion,
258+
}
259+
if testcase.envOrchestrator != "" {
260+
defer patchEnvVariable(t, "DOCKER_ORCHESTRATOR", testcase.envOrchestrator)()
261+
}
262+
263+
cli := &DockerCli{client: apiclient, err: os.Stderr}
264+
cliconfig.SetDir(dir.Path())
265+
options := flags.NewClientOptions()
266+
if testcase.flagOrchestrator != "" {
267+
options.Common.Orchestrator = testcase.flagOrchestrator
268+
}
269+
err := cli.Initialize(options)
270+
assert.NoError(t, err)
271+
assert.Equal(t, testcase.expectedKubernetes, cli.ClientInfo().HasKubernetes)
272+
assert.Equal(t, testcase.expectedOrchestrator, string(cli.ClientInfo().Orchestrator))
273+
})
274+
}
275+
}
276+
173277
func TestGetClientWithPassword(t *testing.T) {
174278
expected := "password"
175279

‎cli/command/orchestrator.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,11 @@ func normalize(flag string) Orchestrator {
3333

3434
// GetOrchestrator checks DOCKER_ORCHESTRATOR environment variable and configuration file
3535
// orchestrator value and returns user defined Orchestrator.
36-
func GetOrchestrator(flagValue, value string) Orchestrator {
36+
func GetOrchestrator(isExperimental bool, flagValue, value string) Orchestrator {
37+
// Non experimental CLI has kubernetes disabled
38+
if !isExperimental {
39+
return defaultOrchestrator
40+
}
3741
// Check flag
3842
if o := normalize(flagValue); o != orchestratorUnset {
3943
return o

‎cli/command/stack/deploy.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ func newDeployCommand(dockerCli command.Cli) *cobra.Command {
1919
Args: cli.ExactArgs(1),
2020
RunE: func(cmd *cobra.Command, args []string) error {
2121
opts.Namespace = args[0]
22-
if dockerCli.ClientInfo().HasKubernetes() {
22+
if dockerCli.ClientInfo().HasKubernetes {
2323
kli, err := kubernetes.WrapCli(dockerCli, cmd)
2424
if err != nil {
2525
return err

‎cli/command/stack/list.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ func newListCommand(dockerCli command.Cli) *cobra.Command {
1818
Short: "List stacks",
1919
Args: cli.NoArgs,
2020
RunE: func(cmd *cobra.Command, args []string) error {
21-
if dockerCli.ClientInfo().HasKubernetes() {
21+
if dockerCli.ClientInfo().HasKubernetes {
2222
kli, err := kubernetes.WrapCli(dockerCli, cmd)
2323
if err != nil {
2424
return err

‎cli/command/stack/ps.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ func newPsCommand(dockerCli command.Cli) *cobra.Command {
1919
Args: cli.ExactArgs(1),
2020
RunE: func(cmd *cobra.Command, args []string) error {
2121
opts.Namespace = args[0]
22-
if dockerCli.ClientInfo().HasKubernetes() {
22+
if dockerCli.ClientInfo().HasKubernetes {
2323
kli, err := kubernetes.WrapCli(dockerCli, cmd)
2424
if err != nil {
2525
return err

‎cli/command/stack/remove.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ func newRemoveCommand(dockerCli command.Cli) *cobra.Command {
1919
Args: cli.RequiresMinArgs(1),
2020
RunE: func(cmd *cobra.Command, args []string) error {
2121
opts.Namespaces = args
22-
if dockerCli.ClientInfo().HasKubernetes() {
22+
if dockerCli.ClientInfo().HasKubernetes {
2323
kli, err := kubernetes.WrapCli(dockerCli, cmd)
2424
if err != nil {
2525
return err

‎cli/command/stack/services.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ func newServicesCommand(dockerCli command.Cli) *cobra.Command {
1919
Args: cli.ExactArgs(1),
2020
RunE: func(cmd *cobra.Command, args []string) error {
2121
opts.Namespace = args[0]
22-
if dockerCli.ClientInfo().HasKubernetes() {
22+
if dockerCli.ClientInfo().HasKubernetes {
2323
kli, err := kubernetes.WrapCli(dockerCli, cmd)
2424
if err != nil {
2525
return err

‎cli/command/system/version_test.go‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ import (
55
"strings"
66
"testing"
77

8-
"github.com/docker/cli/cli/config/configfile"
8+
"github.com/docker/cli/cli/command"
9+
910
"github.com/docker/cli/internal/test"
1011
"github.com/docker/docker/api"
1112
"github.com/docker/docker/api/types"
@@ -33,23 +34,14 @@ func fakeServerVersion(ctx context.Context) (types.Version, error) {
3334
}, nil
3435
}
3536

36-
func TestVersionWithDefaultOrchestrator(t *testing.T) {
37+
func TestVersionWithOrchestrator(t *testing.T) {
3738
cli := test.NewFakeCli(&fakeClient{serverVersion: fakeServerVersion})
39+
cli.SetClientInfo(func() command.ClientInfo { return command.ClientInfo{Orchestrator: "swarm"} })
3840
cmd := NewVersionCommand(cli)
3941
assert.NoError(t, cmd.Execute())
4042
assert.Contains(t, cleanTabs(cli.OutBuffer().String()), "Orchestrator: swarm")
4143
}
4244

43-
func TestVersionWithOverridenOrchestrator(t *testing.T) {
44-
cli := test.NewFakeCli(&fakeClient{serverVersion: fakeServerVersion})
45-
config := configfile.New("configfile")
46-
config.Orchestrator = "Kubernetes"
47-
cli.SetConfigFile(config)
48-
cmd := NewVersionCommand(cli)
49-
assert.NoError(t, cmd.Execute())
50-
assert.Contains(t, cleanTabs(cli.OutBuffer().String()), "Orchestrator: kubernetes")
51-
}
52-
5345
func cleanTabs(line string) string {
5446
return strings.Join(strings.Fields(line), " ")
5547
}

‎cmd/docker/docker.go‎

Lines changed: 28 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,24 @@ type versionDetails interface {
195195
Client() client.APIClient
196196
ClientInfo() command.ClientInfo
197197
ServerInfo() command.ServerInfo
198-
ClientInfo() command.ClientInfo
198+
}
199+
200+
func hideFeatureFlag(f *pflag.Flag, hasFeature bool, annotation string) {
201+
if hasFeature {
202+
return
203+
}
204+
if _, ok := f.Annotations[annotation]; ok {
205+
f.Hidden = true
206+
}
207+
}
208+
209+
func hideFeatureSubCommand(subcmd *cobra.Command, hasFeature bool, annotation string) {
210+
if hasFeature {
211+
return
212+
}
213+
if _, ok := subcmd.Annotations[annotation]; ok {
214+
subcmd.Hidden = true
215+
}
199216
}
200217

201218
func hideUnsupportedFeatures(cmd *cobra.Command, details versionDetails) {
@@ -206,56 +223,21 @@ func hideUnsupportedFeatures(cmd *cobra.Command, details versionDetails) {
206223
hasKubernetes := details.ClientInfo().HasKubernetes
207224

208225
cmd.Flags().VisitAll(func(f *pflag.Flag) {
209-
// hide experimental flags
210-
if !hasExperimental {
211-
if _, ok := f.Annotations["experimental"]; ok {
212-
f.Hidden = true
213-
}
214-
}
215-
if !hasExperimentalCLI {
216-
if _, ok := f.Annotations["experimentalCLI"]; ok {
217-
f.Hidden = true
218-
}
219-
}
220-
if !hasKubernetes {
221-
if _, ok := f.Annotations["kubernetes"]; ok {
222-
f.Hidden = true
223-
}
224-
} else {
225-
if _, ok := f.Annotations["swarm"]; ok {
226-
f.Hidden = true
227-
}
228-
}
229-
226+
hideFeatureFlag(f, hasExperimental, "experimental")
227+
hideFeatureFlag(f, hasExperimentalCLI, "experimentalCLI")
228+
hideFeatureFlag(f, hasKubernetes, "kubernetes")
229+
hideFeatureFlag(f, !hasKubernetes, "swarm")
230230
// hide flags not supported by the server
231231
if !isOSTypeSupported(f, osType) || !isVersionSupported(f, clientVersion) {
232232
f.Hidden = true
233233
}
234234
})
235235

236236
for _, subcmd := range cmd.Commands() {
237-
// hide experimental subcommands
238-
if !hasExperimental {
239-
if _, ok := subcmd.Annotations["experimental"]; ok {
240-
subcmd.Hidden = true
241-
}
242-
}
243-
if !hasExperimentalCLI {
244-
if _, ok := subcmd.Annotations["experimentalCLI"]; ok {
245-
subcmd.Hidden = true
246-
}
247-
}
248-
249-
if !hasKubernetes {
250-
if _, ok := subcmd.Annotations["kubernetes"]; ok {
251-
subcmd.Hidden = true
252-
}
253-
} else {
254-
if _, ok := subcmd.Annotations["swarm"]; ok {
255-
subcmd.Hidden = true
256-
}
257-
}
258-
237+
hideFeatureSubCommand(subcmd, hasExperimental, "experimental")
238+
hideFeatureSubCommand(subcmd, hasExperimentalCLI, "experimentalCLI")
239+
hideFeatureSubCommand(subcmd, hasKubernetes, "kubernetes")
240+
hideFeatureSubCommand(subcmd, !hasKubernetes, "swarm")
259241
// hide subcommands not supported by the server
260242
if subcmdVersion, ok := subcmd.Annotations["version"]; ok && versions.LessThan(clientVersion, subcmdVersion) {
261243
subcmd.Hidden = true
@@ -279,7 +261,8 @@ func areFlagsSupported(cmd *cobra.Command, details versionDetails) error {
279261
clientVersion := details.Client().ClientVersion()
280262
osType := details.ServerInfo().OSType
281263
hasExperimental := details.ServerInfo().HasExperimental
282-
hasKubernetes := details.ClientInfo().HasKubernetes()
264+
hasKubernetes := details.ClientInfo().HasKubernetes
265+
hasExperimentalCLI := details.ClientInfo().HasExperimental
283266

284267
errs := []string{}
285268

0 commit comments

Comments
 (0)