From 7ccee6d6030bca53c48ec9aa33c3e627d023daf0 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 24 Feb 2022 11:32:19 +0100 Subject: [PATCH 1/3] remove unused KubernetesEnabled() test-utility Signed-off-by: Sebastiaan van Stijn --- internal/test/environment/testenv.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/internal/test/environment/testenv.go b/internal/test/environment/testenv.go index 2838fb0302..4340df0ed6 100644 --- a/internal/test/environment/testenv.go +++ b/internal/test/environment/testenv.go @@ -53,11 +53,6 @@ func Setup() error { return nil } -// KubernetesEnabled returns if Kubernetes testing is enabled -func KubernetesEnabled() bool { - return os.Getenv("KUBECONFIG") != "" -} - // RemoteDaemon returns true if running against a remote daemon func RemoteDaemon() bool { return os.Getenv("REMOTE_DAEMON") != "" From 242857dd81b3d091c11c27225dab323c897989f7 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 24 Feb 2022 12:59:15 +0100 Subject: [PATCH 2/3] update/remove various tests and options related to kubernetes support Remove various tests and utilities related to testing kubernetes support Also removing the Kubernetes and DefaultStackOrchestrator from CreateOptions and UpdateOptions, instead updating the flags to not be bound to a variable. This might break some consumers of those options, but given that they've become non-functional, that's probably ok (otherwise they may ignore the deprecation warning and end up with non-functional code). Signed-off-by: Sebastiaan van Stijn --- cli/command/context/create.go | 20 ++++++-------- cli/command/context/create_test.go | 29 -------------------- cli/command/context/export-import_test.go | 15 ++-------- cli/command/context/inspect_test.go | 2 +- cli/command/context/list_test.go | 24 +++++++--------- cli/command/context/remove_test.go | 18 ++++++------ cli/command/context/testdata/test-kubeconfig | 19 ------------- cli/command/context/update.go | 13 +++------ cli/command/context/update_test.go | 7 ++--- e2e/context/context_test.go | 10 ++----- e2e/context/testdata/test-kubeconfig | 20 -------------- internal/test/environment/testenv.go | 6 ---- scripts/test/e2e/run | 1 - 13 files changed, 39 insertions(+), 145 deletions(-) delete mode 100644 cli/command/context/testdata/test-kubeconfig delete mode 100644 e2e/context/testdata/test-kubeconfig diff --git a/cli/command/context/create.go b/cli/command/context/create.go index a1a7dd2733..105640a802 100644 --- a/cli/command/context/create.go +++ b/cli/command/context/create.go @@ -19,11 +19,6 @@ type CreateOptions struct { Description string Docker map[string]string From string - - // Deprecated - DefaultStackOrchestrator string - // Deprecated - Kubernetes map[string]string } func longCreateDescription() string { @@ -53,14 +48,15 @@ func newCreateCommand(dockerCli command.Cli) *cobra.Command { } flags := cmd.Flags() flags.StringVar(&opts.Description, "description", "", "Description of the context") - flags.StringVar( - &opts.DefaultStackOrchestrator, + flags.String( "default-stack-orchestrator", "", - "Default orchestrator for stack operations to use with this context (swarm|kubernetes|all)") + "Default orchestrator for stack operations to use with this context (swarm|kubernetes|all)", + ) + flags.SetAnnotation("default-stack-orchestrator", "deprecated", nil) flags.SetAnnotation("default-stack-orchestrator", "deprecated", nil) flags.MarkDeprecated("default-stack-orchestrator", "option will be ignored") flags.StringToStringVar(&opts.Docker, "docker", nil, "set the docker endpoint") - flags.StringToStringVar(&opts.Kubernetes, "kubernetes", nil, "set the kubernetes endpoint") + flags.StringToString("kubernetes", nil, "set the kubernetes endpoint") flags.SetAnnotation("kubernetes", "kubernetes", nil) flags.SetAnnotation("kubernetes", "deprecated", nil) flags.MarkDeprecated("kubernetes", "option will be ignored") @@ -76,7 +72,7 @@ func RunCreate(cli command.Cli, o *CreateOptions) error { return err } switch { - case o.From == "" && o.Docker == nil && o.Kubernetes == nil: + case o.From == "" && o.Docker == nil: err = createFromExistingContext(s, cli.CurrentContext(), o) case o.From != "": err = createFromExistingContext(s, o.From, o) @@ -132,8 +128,8 @@ func checkContextNameForCreation(s store.Reader, name string) error { } func createFromExistingContext(s store.ReaderWriter, fromContextName string, o *CreateOptions) error { - if len(o.Docker) != 0 || len(o.Kubernetes) != 0 { - return errors.New("cannot use --docker or --kubernetes flags when --from is set") + if len(o.Docker) != 0 { + return errors.New("cannot use --docker flag when --from is set") } reader := store.Export(fromContextName, &descriptionDecorator{ Reader: s, diff --git a/cli/command/context/create_test.go b/cli/command/context/create_test.go index 945ae23684..ea5aad3bb2 100644 --- a/cli/command/context/create_test.go +++ b/cli/command/context/create_test.go @@ -95,21 +95,6 @@ func TestCreate(t *testing.T) { }, expecterErr: `unable to parse docker host`, }, - { - options: CreateOptions{ - Name: "invalid-orchestrator", - DefaultStackOrchestrator: "invalid", - }, - expecterErr: "", - }, - { - options: CreateOptions{ - Name: "orchestrator-all-no-endpoint", - DefaultStackOrchestrator: "all", - Docker: map[string]string{}, - }, - expecterErr: "", - }, } for _, tc := range tests { tc := tc @@ -129,19 +114,6 @@ func assertContextCreateLogging(t *testing.T, cli *test.FakeCli, n string) { assert.Equal(t, fmt.Sprintf("Successfully created context %q\n", n), cli.ErrBuffer().String()) } -func TestCreateOrchestratorSwarm(t *testing.T) { - cli, cleanup := makeFakeCli(t) - defer cleanup() - - err := RunCreate(cli, &CreateOptions{ - Name: "test", - DefaultStackOrchestrator: "swarm", - Docker: map[string]string{}, - }) - assert.NilError(t, err) - assertContextCreateLogging(t, cli, "test") -} - func TestCreateOrchestratorEmpty(t *testing.T) { cli, cleanup := makeFakeCli(t) defer cleanup() @@ -160,7 +132,6 @@ func TestCreateFromContext(t *testing.T) { description string expectedDescription string docker map[string]string - kubernetes map[string]string }{ { name: "no-override", diff --git a/cli/command/context/export-import_test.go b/cli/command/context/export-import_test.go index 747270d380..01375e19f2 100644 --- a/cli/command/context/export-import_test.go +++ b/cli/command/context/export-import_test.go @@ -8,7 +8,6 @@ import ( "path/filepath" "testing" - "github.com/docker/cli/cli/command" "github.com/docker/cli/cli/streams" "gotest.tools/v3/assert" ) @@ -20,7 +19,7 @@ func TestExportImportWithFile(t *testing.T) { contextFile := filepath.Join(contextDir, "exported") cli, cleanup := makeFakeCli(t) defer cleanup() - createTestContext(t, cli) + createTestContext(t, cli, "test") cli.ErrBuffer().Reset() assert.NilError(t, RunExport(cli, &ExportOptions{ ContextName: "test", @@ -46,7 +45,7 @@ func TestExportImportWithFile(t *testing.T) { func TestExportImportPipe(t *testing.T) { cli, cleanup := makeFakeCli(t) defer cleanup() - createTestContext(t, cli) + createTestContext(t, cli, "test") cli.ErrBuffer().Reset() cli.OutBuffer().Reset() assert.NilError(t, RunExport(cli, &ExportOptions{ @@ -83,13 +82,3 @@ func TestExportExistingFile(t *testing.T) { err = RunExport(cli, &ExportOptions{ContextName: "test", Dest: contextFile}) assert.Assert(t, os.IsExist(err)) } - -func createTestContext(t *testing.T, cli command.Cli) { - t.Helper() - - err := RunCreate(cli, &CreateOptions{ - Name: "test", - Docker: map[string]string{}, - }) - assert.NilError(t, err) -} diff --git a/cli/command/context/inspect_test.go b/cli/command/context/inspect_test.go index 5e26c47fcb..e26afd31b4 100644 --- a/cli/command/context/inspect_test.go +++ b/cli/command/context/inspect_test.go @@ -11,7 +11,7 @@ import ( func TestInspect(t *testing.T) { cli, cleanup := makeFakeCli(t) defer cleanup() - createTestContextWithKubeAndSwarm(t, cli, "current", "all") + createTestContext(t, cli, "current") cli.OutBuffer().Reset() assert.NilError(t, runInspect(cli, inspectOptions{ refs: []string{"current"}, diff --git a/cli/command/context/list_test.go b/cli/command/context/list_test.go index b71a78b5a9..a8c5b3b5af 100644 --- a/cli/command/context/list_test.go +++ b/cli/command/context/list_test.go @@ -5,20 +5,16 @@ import ( "github.com/docker/cli/cli/command" "gotest.tools/v3/assert" - "gotest.tools/v3/env" "gotest.tools/v3/golden" ) -func createTestContextWithKubeAndSwarm(t *testing.T, cli command.Cli, name string, orchestrator string) { - revert := env.Patch(t, "KUBECONFIG", "./testdata/test-kubeconfig") - defer revert() +func createTestContext(t *testing.T, cli command.Cli, name string) { + t.Helper() err := RunCreate(cli, &CreateOptions{ - Name: name, - DefaultStackOrchestrator: orchestrator, - Description: "description of " + name, - Kubernetes: map[string]string{keyFrom: "default"}, - Docker: map[string]string{keyHost: "https://someswarmserver.example.com"}, + Name: name, + Description: "description of " + name, + Docker: map[string]string{keyHost: "https://someswarmserver.example.com"}, }) assert.NilError(t, err) } @@ -26,9 +22,9 @@ func createTestContextWithKubeAndSwarm(t *testing.T, cli command.Cli, name strin func TestList(t *testing.T) { cli, cleanup := makeFakeCli(t) defer cleanup() - createTestContextWithKubeAndSwarm(t, cli, "current", "all") - createTestContextWithKubeAndSwarm(t, cli, "other", "all") - createTestContextWithKubeAndSwarm(t, cli, "unset", "unset") + createTestContext(t, cli, "current") + createTestContext(t, cli, "other") + createTestContext(t, cli, "unset") cli.SetCurrentContext("current") cli.OutBuffer().Reset() assert.NilError(t, runList(cli, &listOptions{})) @@ -38,8 +34,8 @@ func TestList(t *testing.T) { func TestListQuiet(t *testing.T) { cli, cleanup := makeFakeCli(t) defer cleanup() - createTestContextWithKubeAndSwarm(t, cli, "current", "all") - createTestContextWithKubeAndSwarm(t, cli, "other", "all") + createTestContext(t, cli, "current") + createTestContext(t, cli, "other") cli.SetCurrentContext("current") cli.OutBuffer().Reset() assert.NilError(t, runList(cli, &listOptions{quiet: true})) diff --git a/cli/command/context/remove_test.go b/cli/command/context/remove_test.go index bf67c2f5cc..4950cf7cf6 100644 --- a/cli/command/context/remove_test.go +++ b/cli/command/context/remove_test.go @@ -15,8 +15,8 @@ import ( func TestRemove(t *testing.T) { cli, cleanup := makeFakeCli(t) defer cleanup() - createTestContextWithKubeAndSwarm(t, cli, "current", "all") - createTestContextWithKubeAndSwarm(t, cli, "other", "all") + createTestContext(t, cli, "current") + createTestContext(t, cli, "other") assert.NilError(t, RunRemove(cli, RemoveOptions{}, []string{"other"})) _, err := cli.ContextStore().GetMetadata("current") assert.NilError(t, err) @@ -27,8 +27,8 @@ func TestRemove(t *testing.T) { func TestRemoveNotAContext(t *testing.T) { cli, cleanup := makeFakeCli(t) defer cleanup() - createTestContextWithKubeAndSwarm(t, cli, "current", "all") - createTestContextWithKubeAndSwarm(t, cli, "other", "all") + createTestContext(t, cli, "current") + createTestContext(t, cli, "other") err := RunRemove(cli, RemoveOptions{}, []string{"not-a-context"}) assert.ErrorContains(t, err, `context "not-a-context" does not exist`) } @@ -36,8 +36,8 @@ func TestRemoveNotAContext(t *testing.T) { func TestRemoveCurrent(t *testing.T) { cli, cleanup := makeFakeCli(t) defer cleanup() - createTestContextWithKubeAndSwarm(t, cli, "current", "all") - createTestContextWithKubeAndSwarm(t, cli, "other", "all") + createTestContext(t, cli, "current") + createTestContext(t, cli, "other") cli.SetCurrentContext("current") err := RunRemove(cli, RemoveOptions{}, []string{"current"}) assert.ErrorContains(t, err, "current: context is in use, set -f flag to force remove") @@ -54,8 +54,8 @@ func TestRemoveCurrentForce(t *testing.T) { cli, cleanup := makeFakeCli(t, withCliConfig(testCfg)) defer cleanup() - createTestContextWithKubeAndSwarm(t, cli, "current", "all") - createTestContextWithKubeAndSwarm(t, cli, "other", "all") + createTestContext(t, cli, "current") + createTestContext(t, cli, "other") cli.SetCurrentContext("current") assert.NilError(t, RunRemove(cli, RemoveOptions{Force: true}, []string{"current"})) reloadedConfig, err := config.Load(configDir) @@ -66,7 +66,7 @@ func TestRemoveCurrentForce(t *testing.T) { func TestRemoveDefault(t *testing.T) { cli, cleanup := makeFakeCli(t) defer cleanup() - createTestContextWithKubeAndSwarm(t, cli, "other", "all") + createTestContext(t, cli, "other") cli.SetCurrentContext("current") err := RunRemove(cli, RemoveOptions{}, []string{"default"}) assert.ErrorContains(t, err, `default: context "default" cannot be removed`) diff --git a/cli/command/context/testdata/test-kubeconfig b/cli/command/context/testdata/test-kubeconfig deleted file mode 100644 index 5d5c858ed8..0000000000 --- a/cli/command/context/testdata/test-kubeconfig +++ /dev/null @@ -1,19 +0,0 @@ -apiVersion: v1 -clusters: -- cluster: - certificate-authority-data: dGhlLWNh - server: https://someserver.example.com - name: test-cluster -contexts: -- context: - cluster: test-cluster - user: test-user - name: test -current-context: test -kind: Config -preferences: {} -users: -- name: test-user - user: - client-certificate-data: dGhlLWNlcnQ= - client-key-data: dGhlLWtleQ== diff --git a/cli/command/context/update.go b/cli/command/context/update.go index 96bc4486ac..7ea9f74923 100644 --- a/cli/command/context/update.go +++ b/cli/command/context/update.go @@ -18,11 +18,6 @@ type UpdateOptions struct { Name string Description string Docker map[string]string - - // Deprecated - DefaultStackOrchestrator string - // Deprecated - Kubernetes map[string]string } func longUpdateDescription() string { @@ -52,14 +47,14 @@ func newUpdateCommand(dockerCli command.Cli) *cobra.Command { } flags := cmd.Flags() flags.StringVar(&opts.Description, "description", "", "Description of the context") - flags.StringVar( - &opts.DefaultStackOrchestrator, + flags.String( "default-stack-orchestrator", "", - "Default orchestrator for stack operations to use with this context (swarm|kubernetes|all)") + "Default orchestrator for stack operations to use with this context (swarm|kubernetes|all)", + ) flags.SetAnnotation("default-stack-orchestrator", "deprecated", nil) flags.MarkDeprecated("default-stack-orchestrator", "option will be ignored") flags.StringToStringVar(&opts.Docker, "docker", nil, "set the docker endpoint") - flags.StringToStringVar(&opts.Kubernetes, "kubernetes", nil, "set the kubernetes endpoint") + flags.StringToString("kubernetes", nil, "set the kubernetes endpoint") flags.SetAnnotation("kubernetes", "kubernetes", nil) flags.SetAnnotation("kubernetes", "deprecated", nil) flags.MarkDeprecated("kubernetes", "option will be ignored") diff --git a/cli/command/context/update_test.go b/cli/command/context/update_test.go index 2593fae902..eea48d6dea 100644 --- a/cli/command/context/update_test.go +++ b/cli/command/context/update_test.go @@ -13,9 +13,8 @@ func TestUpdateDescriptionOnly(t *testing.T) { cli, cleanup := makeFakeCli(t) defer cleanup() err := RunCreate(cli, &CreateOptions{ - Name: "test", - DefaultStackOrchestrator: "swarm", - Docker: map[string]string{}, + Name: "test", + Docker: map[string]string{}, }) assert.NilError(t, err) cli.OutBuffer().Reset() @@ -37,7 +36,7 @@ func TestUpdateDescriptionOnly(t *testing.T) { func TestUpdateDockerOnly(t *testing.T) { cli, cleanup := makeFakeCli(t) defer cleanup() - createTestContextWithKubeAndSwarm(t, cli, "test", "swarm") + createTestContext(t, cli, "test") assert.NilError(t, RunUpdate(cli, &UpdateOptions{ Name: "test", Docker: map[string]string{ diff --git a/e2e/context/context_test.go b/e2e/context/context_test.go index 6212b98dea..05cbca3d83 100644 --- a/e2e/context/context_test.go +++ b/e2e/context/context_test.go @@ -12,10 +12,7 @@ import ( func TestContextList(t *testing.T) { cmd := icmd.Command("docker", "context", "ls") - cmd.Env = append(cmd.Env, - "DOCKER_CONFIG=./testdata/test-dockerconfig", - "KUBECONFIG=./testdata/test-kubeconfig", - ) + cmd.Env = append(cmd.Env, "DOCKER_CONFIG=./testdata/test-dockerconfig") result := icmd.RunCmd(cmd).Assert(t, icmd.Expected{ Err: icmd.None, ExitCode: 0, @@ -35,10 +32,7 @@ func TestContextImportNoTLS(t *testing.T) { icmd.RunCmd(cmd).Assert(t, icmd.Success) cmd = icmd.Command("docker", "context", "ls") - cmd.Env = append(cmd.Env, - "DOCKER_CONFIG="+d, - "KUBECONFIG=./testdata/test-kubeconfig", // Allows reuse of context-ls.golden - ) + cmd.Env = append(cmd.Env, "DOCKER_CONFIG="+d) result := icmd.RunCmd(cmd).Assert(t, icmd.Success) golden.Assert(t, result.Stdout(), "context-ls.golden") } diff --git a/e2e/context/testdata/test-kubeconfig b/e2e/context/testdata/test-kubeconfig deleted file mode 100644 index e96df74a50..0000000000 --- a/e2e/context/testdata/test-kubeconfig +++ /dev/null @@ -1,20 +0,0 @@ -apiVersion: v1 -clusters: -- cluster: - certificate-authority-data: dGhlLWNh - server: https://someserver - name: test-cluster -contexts: -- context: - cluster: test-cluster - user: test-user - namespace: zoinx - name: test -current-context: test -kind: Config -preferences: {} -users: -- name: test-user - user: - client-certificate-data: dGhlLWNlcnQ= - client-key-data: dGhlLWtleQ== diff --git a/internal/test/environment/testenv.go b/internal/test/environment/testenv.go index 4340df0ed6..bb80a9b2cc 100644 --- a/internal/test/environment/testenv.go +++ b/internal/test/environment/testenv.go @@ -32,12 +32,6 @@ func Setup() error { } } - if kubeConfig := os.Getenv("TEST_KUBECONFIG"); kubeConfig != "" { - if err := os.Setenv("KUBECONFIG", kubeConfig); err != nil { - return err - } - } - if val := boolFromString(os.Getenv("TEST_REMOTE_DAEMON")); val { if err := os.Setenv("REMOTE_DAEMON", "1"); err != nil { return err diff --git a/scripts/test/e2e/run b/scripts/test/e2e/run index 5befee97aa..9065884a02 100755 --- a/scripts/test/e2e/run +++ b/scripts/test/e2e/run @@ -64,7 +64,6 @@ runtests() { env -i \ TEST_DOCKER_HOST="$engine_host" \ TEST_DOCKER_CERT_PATH="${DOCKER_CERT_PATH-}" \ - TEST_KUBECONFIG="${KUBECONFIG-}" \ TEST_REMOTE_DAEMON="${REMOTE_DAEMON-}" \ TEST_SKIP_PLUGIN_TESTS="${SKIP_PLUGIN_TESTS-}" \ GOPATH="$GOPATH" \ From ee9d17caeca9605908ee042e11fc401de8fb0dd1 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 24 Feb 2022 13:19:49 +0100 Subject: [PATCH 3/3] cli/context: update package documentation Signed-off-by: Sebastiaan van Stijn --- cli/context/store/doc.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cli/context/store/doc.go b/cli/context/store/doc.go index d5304b1d98..f84c023839 100644 --- a/cli/context/store/doc.go +++ b/cli/context/store/doc.go @@ -14,9 +14,7 @@ // // The context store itself has absolutely no knowledge about what a docker endpoint should contain in term of metadata or TLS config. // Client code is responsible for generating and parsing endpoint metadata and TLS files. -// The multi-endpoints approach of this package allows to combine many different endpoints in the same "context" (e.g., the Docker CLI -// is able for a single context to define both a docker endpoint and a Kubernetes endpoint for the same cluster, and also specify which -// orchestrator to use by default when deploying a compose stack on this cluster). +// The multi-endpoints approach of this package allows to combine many different endpoints in the same "context". // // Context IDs are actually SHA256 hashes of the context name, and are there only to avoid dealing with special characters in context names. package store