From 3b26cfce8b7af1073b28bb78eb185a0689dbfcbf Mon Sep 17 00:00:00 2001 From: Jean-Christophe Sirot Date: Mon, 25 Feb 2019 17:35:53 +0100 Subject: [PATCH 1/3] Always initialize context store Signed-off-by: Jean-Christophe Sirot --- cli/command/cli.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/cli/command/cli.go b/cli/command/cli.go index 47e9bdfffab0..55858aedbc05 100644 --- a/cli/command/cli.go +++ b/cli/command/cli.go @@ -209,12 +209,13 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions, ops ...Initialize cli.configFile = cliconfig.LoadDefaultConfigFile(cli.err) + cli.contextStore = store.New(cliconfig.ContextStoreDir(), cli.contextStoreConfig) + cli.currentContext, err = resolveContextName(opts.Common, cli.configFile, cli.contextStore) + if err != nil { + return err + } + if cli.client == nil { - cli.contextStore = store.New(cliconfig.ContextStoreDir(), cli.contextStoreConfig) - cli.currentContext, err = resolveContextName(opts.Common, cli.configFile, cli.contextStore) - if err != nil { - return err - } endpoint, err := resolveDockerEndpoint(cli.contextStore, cli.currentContext, opts.Common) if err != nil { return errors.Wrap(err, "unable to resolve docker endpoint") From 37fcaf7a29e862fb9e2a1591c2fa1d66c5775a78 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Sirot Date: Tue, 26 Feb 2019 16:08:26 +0100 Subject: [PATCH 2/3] Resolve the docker Endpoint even if the client already exists. In that case the `TestDialStdio` e2e test had to be modified: the `--tls` option triggers an error since the endpoint resolution tries to read the `${DOCKER_CERT_PATH}/ca.pem` file which does not exist. Signed-off-by: Jean-Christophe Sirot --- cli/command/cli.go | 16 +++++++--------- e2e/cli-plugins/dial_test.go | 4 ++-- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/cli/command/cli.go b/cli/command/cli.go index 55858aedbc05..3208a790417e 100644 --- a/cli/command/cli.go +++ b/cli/command/cli.go @@ -214,20 +214,18 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions, ops ...Initialize if err != nil { return err } + cli.dockerEndpoint, err = resolveDockerEndpoint(cli.contextStore, cli.currentContext, opts.Common) + if err != nil { + return errors.Wrap(err, "unable to resolve docker endpoint") + } if cli.client == nil { - endpoint, err := resolveDockerEndpoint(cli.contextStore, cli.currentContext, opts.Common) - if err != nil { - return errors.Wrap(err, "unable to resolve docker endpoint") - } - cli.dockerEndpoint = endpoint - - cli.client, err = newAPIClientFromEndpoint(endpoint, cli.configFile) + cli.client, err = newAPIClientFromEndpoint(cli.dockerEndpoint, cli.configFile) if tlsconfig.IsErrEncryptedKey(err) { passRetriever := passphrase.PromptRetrieverWithInOut(cli.In(), cli.Out(), nil) newClient := func(password string) (client.APIClient, error) { - endpoint.TLSPassword = password - return newAPIClientFromEndpoint(endpoint, cli.configFile) + cli.dockerEndpoint.TLSPassword = password + return newAPIClientFromEndpoint(cli.dockerEndpoint, cli.configFile) } cli.client, err = getClientWithPassword(passRetriever, newClient) } diff --git a/e2e/cli-plugins/dial_test.go b/e2e/cli-plugins/dial_test.go index a01feb034539..9953f3a7a8a7 100644 --- a/e2e/cli-plugins/dial_test.go +++ b/e2e/cli-plugins/dial_test.go @@ -18,9 +18,9 @@ func TestDialStdio(t *testing.T) { // follows. We observe this from the debug level logging from // the connhelper stuff. helloworld := filepath.Join(os.Getenv("DOCKER_CLI_E2E_PLUGINS_EXTRA_DIRS"), "docker-helloworld") - cmd := icmd.Command(helloworld, "--config=blah", "--tls", "--log-level", "debug", "helloworld", "--who=foo") + cmd := icmd.Command(helloworld, "--config=blah", "--log-level", "debug", "helloworld", "--who=foo") res := icmd.RunCmd(cmd, icmd.WithEnv(manager.ReexecEnvvar+"=/bin/true")) res.Assert(t, icmd.Success) - assert.Assert(t, is.Contains(res.Stderr(), `msg="commandconn: starting /bin/true with [--config=blah --tls --log-level debug system dial-stdio]"`)) + assert.Assert(t, is.Contains(res.Stderr(), `msg="commandconn: starting /bin/true with [--config=blah --log-level debug system dial-stdio]"`)) assert.Assert(t, is.Equal(res.Stdout(), "Hello foo!\n")) } From a1af6e261f61668fb213f04994c6e32934ce2ae3 Mon Sep 17 00:00:00 2001 From: Jean-Christophe Sirot Date: Thu, 7 Mar 2019 15:35:11 +0100 Subject: [PATCH 3/3] Cover the changes with unit test Signed-off-by: Jean-Christophe Sirot --- cli/command/cli_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/cli/command/cli_test.go b/cli/command/cli_test.go index 0bbe6df05528..926d43f2ddf7 100644 --- a/cli/command/cli_test.go +++ b/cli/command/cli_test.go @@ -295,3 +295,12 @@ func TestNewDockerCliAndOperators(t *testing.T) { assert.NilError(t, err) assert.Equal(t, string(errStream), "error") } + +func TestInitializeShouldAlwaysCreateTheContextStore(t *testing.T) { + cli, err := NewDockerCli() + assert.NilError(t, err) + assert.NilError(t, cli.Initialize(flags.NewClientOptions(), WithInitializeClient(func(cli *DockerCli) (client.APIClient, error) { + return client.NewClientWithOpts() + }))) + assert.Check(t, cli.ContextStore() != nil) +}