From 30a0d0c6d6df4d98d48477e96326329655a9fd7a Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 28 Nov 2022 16:27:45 +0100 Subject: [PATCH 1/7] cli/command/formatter: define const for error column-headers Signed-off-by: Sebastiaan van Stijn --- cli/command/formatter/custom.go | 1 + cli/command/task/formatter.go | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/command/formatter/custom.go b/cli/command/formatter/custom.go index 1d78ce62e2..2d6c979c32 100644 --- a/cli/command/formatter/custom.go +++ b/cli/command/formatter/custom.go @@ -16,6 +16,7 @@ const ( StatusHeader = "STATUS" PortsHeader = "PORTS" ImageHeader = "IMAGE" + ErrorHeader = "ERROR" ContainerIDHeader = "CONTAINER ID" ) diff --git a/cli/command/task/formatter.go b/cli/command/task/formatter.go index 0958a1e89e..64509abbea 100644 --- a/cli/command/task/formatter.go +++ b/cli/command/task/formatter.go @@ -20,7 +20,6 @@ const ( taskIDHeader = "ID" desiredStateHeader = "DESIRED STATE" currentStateHeader = "CURRENT STATE" - errorHeader = "ERROR" maxErrLength = 30 ) @@ -61,7 +60,7 @@ func FormatWrite(ctx formatter.Context, tasks []swarm.Task, names map[string]str "Node": nodeHeader, "DesiredState": desiredStateHeader, "CurrentState": currentStateHeader, - "Error": errorHeader, + "Error": formatter.ErrorHeader, "Ports": formatter.PortsHeader, } return ctx.Write(&taskCtx, render) From 0ed80a3a587b149d635def3adb2dd833a9a1bace Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 28 Nov 2022 16:28:47 +0100 Subject: [PATCH 2/7] cli/command/formatter: NewClientContextFormat(): unconvert Signed-off-by: Sebastiaan van Stijn --- cli/command/formatter/context.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/command/formatter/context.go b/cli/command/formatter/context.go index fd0e67d6f4..19780e7588 100644 --- a/cli/command/formatter/context.go +++ b/cli/command/formatter/context.go @@ -11,10 +11,10 @@ const ( // NewClientContextFormat returns a Format for rendering using a Context func NewClientContextFormat(source string, quiet bool) Format { if quiet { - return Format(quietContextFormat) + return quietContextFormat } if source == TableFormatKey { - return Format(ClientContextTableFormat) + return ClientContextTableFormat } return Format(source) } From 2c41bbc49fae55dcbec0f77d6efb438f3f064aeb Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 28 Nov 2022 16:22:54 +0100 Subject: [PATCH 3/7] cli/command/task: taskContext.Error(): use ellipsis utility Signed-off-by: Sebastiaan van Stijn --- cli/command/task/formatter.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cli/command/task/formatter.go b/cli/command/task/formatter.go index 64509abbea..a7d71115ac 100644 --- a/cli/command/task/formatter.go +++ b/cli/command/task/formatter.go @@ -123,11 +123,11 @@ func (c *taskContext) CurrentState() string { func (c *taskContext) Error() string { // Trim and quote the error message. taskErr := c.task.Status.Err - if c.trunc && len(taskErr) > maxErrLength { - taskErr = fmt.Sprintf("%s…", taskErr[:maxErrLength-1]) + if c.trunc { + taskErr = formatter.Ellipsis(taskErr, maxErrLength) } if len(taskErr) > 0 { - taskErr = fmt.Sprintf("\"%s\"", taskErr) + taskErr = fmt.Sprintf(`"%s"`, taskErr) } return taskErr } From 3b7235edca58203aa9a857bdf031ca39e8a686ed Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 4 Nov 2022 15:47:14 +0100 Subject: [PATCH 4/7] cli/command: initialize client and load content lazily This allows commands that don't require a client connection (such as `context use`) to be functional, but still produces an error when trying to run a command that needs to connect with the API; mkdir -p ~/.docker/ && echo '{"currentContext":"nosuchcontext"}' > ~/.docker/config.json docker version Failed to initialize: unable to resolve docker endpoint: load context "nosuchcontext": context does not exist: open /root/.docker/contexts/meta/8bfef2a74c7d06add4bf4c73b0af97d9f79c76fe151ae0e18b9d7e57104c149b/meta.json: no such file or directory docker context use default default Current context is now "default" docker version Client: Version: 22.06.0-dev API version: 1.42 ... Signed-off-by: Sebastiaan van Stijn --- cli/command/cli.go | 58 ++++++++++++++++++++++++++++++----------- cli/command/cli_test.go | 6 +++-- 2 files changed, 47 insertions(+), 17 deletions(-) diff --git a/cli/command/cli.go b/cli/command/cli.go index 0cce3fa505..2ac7a6698b 100644 --- a/cli/command/cli.go +++ b/cli/command/cli.go @@ -2,12 +2,14 @@ package command import ( "context" + "fmt" "io" "os" "path/filepath" "runtime" "strconv" "strings" + "sync" "time" "github.com/docker/cli/cli/config" @@ -78,6 +80,8 @@ type DockerCli struct { contentTrust bool contextStore store.Store currentContext string + init sync.Once + initErr error dockerEndpoint docker.Endpoint contextStoreConfig store.Config initTimeout time.Duration @@ -91,6 +95,7 @@ func (cli *DockerCli) DefaultVersion() string { // CurrentVersion returns the API version currently negotiated, or the default // version otherwise. func (cli *DockerCli) CurrentVersion() string { + _ = cli.initialize() if cli.client == nil { return api.DefaultVersion } @@ -99,6 +104,10 @@ func (cli *DockerCli) CurrentVersion() string { // Client returns the APIClient func (cli *DockerCli) Client() client.APIClient { + if err := cli.initialize(); err != nil { + _, _ = fmt.Fprintf(cli.Err(), "Failed to initialize: %s\n", err) + os.Exit(1) + } return cli.client } @@ -203,8 +212,6 @@ func WithInitializeClient(makeClient func(dockerCli *DockerCli) (client.APIClien // Initialize the dockerCli runs initialization that must happen after command // line flags are parsed. func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions, ops ...InitializeOpt) error { - var err error - for _, o := range ops { if err := o(cli); err != nil { return err @@ -232,18 +239,6 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions, ops ...Initialize return ResolveDefaultContext(cli.options, cli.contextStoreConfig) }, } - cli.dockerEndpoint, err = resolveDockerEndpoint(cli.contextStore, resolveContextName(opts, cli.configFile)) - if err != nil { - return errors.Wrap(err, "unable to resolve docker endpoint") - } - - if cli.client == nil { - cli.client, err = newAPIClientFromEndpoint(cli.dockerEndpoint, cli.configFile) - if err != nil { - return err - } - } - cli.initializeFromClient() return nil } @@ -282,6 +277,9 @@ func newAPIClientFromEndpoint(ep docker.Endpoint, configFile *configfile.ConfigF } func resolveDockerEndpoint(s store.Reader, contextName string) (docker.Endpoint, error) { + if s == nil { + return docker.Endpoint{}, fmt.Errorf("no context store initialized") + } ctxMeta, err := s.GetMetadata(contextName) if err != nil { return docker.Endpoint{}, err @@ -331,7 +329,7 @@ func (cli *DockerCli) getInitTimeout() time.Duration { func (cli *DockerCli) initializeFromClient() { ctx := context.Background() - if !strings.HasPrefix(cli.DockerEndpoint().Host, "ssh://") { + if !strings.HasPrefix(cli.dockerEndpoint.Host, "ssh://") { // @FIXME context.WithTimeout doesn't work with connhelper / ssh connections // time="2020-04-10T10:16:26Z" level=warning msg="commandConn.CloseWrite: commandconn: failed to wait: signal: killed" var cancel func() @@ -428,9 +426,39 @@ func resolveContextName(opts *cliflags.ClientOptions, config *configfile.ConfigF // DockerEndpoint returns the current docker endpoint func (cli *DockerCli) DockerEndpoint() docker.Endpoint { + if err := cli.initialize(); err != nil { + // Note that we're not terminating here, as this function may be used + // in cases where we're able to continue. + _, _ = fmt.Fprintf(cli.Err(), "%v\n", cli.initErr) + } return cli.dockerEndpoint } +func (cli *DockerCli) getDockerEndPoint() (ep docker.Endpoint, err error) { + cn := cli.CurrentContext() + if cn == DefaultContextName { + return resolveDefaultDockerEndpoint(cli.options) + } + return resolveDockerEndpoint(cli.contextStore, cn) +} + +func (cli *DockerCli) initialize() error { + cli.init.Do(func() { + cli.dockerEndpoint, cli.initErr = cli.getDockerEndPoint() + if cli.initErr != nil { + cli.initErr = errors.Wrap(cli.initErr, "unable to resolve docker endpoint") + return + } + if cli.client == nil { + if cli.client, cli.initErr = newAPIClientFromEndpoint(cli.dockerEndpoint, cli.configFile); cli.initErr != nil { + return + } + } + cli.initializeFromClient() + }) + return cli.initErr +} + // Apply all the operation on the cli func (cli *DockerCli) Apply(ops ...DockerCliOption) error { for _, op := range ops { diff --git a/cli/command/cli_test.go b/cli/command/cli_test.go index dcd9103fb6..7a0b4e727e 100644 --- a/cli/command/cli_test.go +++ b/cli/command/cli_test.go @@ -118,7 +118,7 @@ func (c *fakeClient) NegotiateAPIVersionPing(types.Ping) { } func TestInitializeFromClient(t *testing.T) { - defaultVersion := "v1.55" + const defaultVersion = "v1.55" testcases := []struct { doc string @@ -160,7 +160,8 @@ func TestInitializeFromClient(t *testing.T) { } cli := &DockerCli{client: apiclient} - cli.initializeFromClient() + err := cli.Initialize(flags.NewClientOptions()) + assert.NilError(t, err) assert.DeepEqual(t, cli.ServerInfo(), testcase.expectedServer) assert.Equal(t, apiclient.negotiated, testcase.negotiated) }) @@ -202,6 +203,7 @@ func TestInitializeFromClientHangs(t *testing.T) { cli := &DockerCli{client: apiClient, initTimeout: time.Millisecond} err := cli.Initialize(flags.NewClientOptions()) assert.Check(t, err) + cli.CurrentVersion() close(initializedCh) }() From 14f97cc10af0eea6c376bd745ec944c2b6bea5dc Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 28 Nov 2022 16:16:45 +0100 Subject: [PATCH 5/7] cli/command: DockerCli.ServerInfo() load info lazily Signed-off-by: Sebastiaan van Stijn --- cli/command/cli.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/command/cli.go b/cli/command/cli.go index 2ac7a6698b..b4fc151474 100644 --- a/cli/command/cli.go +++ b/cli/command/cli.go @@ -152,7 +152,7 @@ func (cli *DockerCli) ConfigFile() *configfile.ConfigFile { // ServerInfo returns the server version details for the host this client is // connected to func (cli *DockerCli) ServerInfo() ServerInfo { - // TODO(thaJeztah) make ServerInfo() lazily load the info (ping only when needed) + _ = cli.initialize() return cli.serverInfo } From ed4b0a67befc7d70c2f6cb2b8c65111ad6c8e57b Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 4 Nov 2022 14:39:07 +0100 Subject: [PATCH 6/7] cli/command/context: context ls: add ERROR column, and don't fail early This updates `docker context ls` to: - not abort listing contexts when failing one (or more) contexts - instead, adding an ERROR column to inform the user there was an issue loading the context. Signed-off-by: Sebastiaan van Stijn --- cli/command/context/list.go | 13 +++++++++++-- cli/command/formatter/context.go | 12 ++++++++++-- e2e/context/testdata/context-ls-notls.golden | 6 +++--- e2e/context/testdata/context-ls-tls.golden | 6 +++--- e2e/context/testdata/context-ls.golden | 6 +++--- 5 files changed, 30 insertions(+), 13 deletions(-) diff --git a/cli/command/context/list.go b/cli/command/context/list.go index 5077157352..6943a99ea2 100644 --- a/cli/command/context/list.go +++ b/cli/command/context/list.go @@ -56,17 +56,26 @@ func runList(dockerCli command.Cli, opts *listOptions) error { isCurrent := rawMeta.Name == curContext meta, err := command.GetDockerContext(rawMeta) if err != nil { - return err + // Add a stub-entry to the list, including the error-message + // indicating that the context couldn't be loaded. + contexts = append(contexts, &formatter.ClientContext{ + Name: rawMeta.Name, + Current: isCurrent, + Error: err.Error(), + }) + continue } + var errMsg string dockerEndpoint, err := docker.EndpointFromContext(rawMeta) if err != nil { - return err + errMsg = err.Error() } desc := formatter.ClientContext{ Name: rawMeta.Name, Current: isCurrent, Description: meta.Description, DockerEndpoint: dockerEndpoint.Host, + Error: errMsg, } contexts = append(contexts, &desc) } diff --git a/cli/command/formatter/context.go b/cli/command/formatter/context.go index 19780e7588..c0a36d1ac9 100644 --- a/cli/command/formatter/context.go +++ b/cli/command/formatter/context.go @@ -1,8 +1,8 @@ package formatter const ( - // ClientContextTableFormat is the default client context format - ClientContextTableFormat = "table {{.Name}}{{if .Current}} *{{end}}\t{{.Description}}\t{{.DockerEndpoint}}" + // ClientContextTableFormat is the default client context format. + ClientContextTableFormat = "table {{.Name}}{{if .Current}} *{{end}}\t{{.Description}}\t{{.DockerEndpoint}}\t{{.Error}}" dockerEndpointHeader = "DOCKER ENDPOINT" quietContextFormat = "{{.Name}}" @@ -25,6 +25,7 @@ type ClientContext struct { Description string DockerEndpoint string Current bool + Error string } // ClientContextWrite writes formatted contexts using the Context @@ -51,6 +52,7 @@ func newClientContextContext() *clientContextContext { "Name": NameHeader, "Description": DescriptionHeader, "DockerEndpoint": dockerEndpointHeader, + "Error": ErrorHeader, } return &ctx } @@ -75,6 +77,12 @@ func (c *clientContextContext) DockerEndpoint() string { return c.c.DockerEndpoint } +// Error returns the truncated error (if any) that occurred when loading the context. +func (c *clientContextContext) Error() string { + // TODO(thaJeztah) add "--no-trunc" option to context ls and set default to 30 cols to match "docker service ps" + return Ellipsis(c.c.Error, 45) +} + // KubernetesEndpoint returns the kubernetes endpoint. // // Deprecated: support for kubernetes endpoints in contexts has been removed, and this formatting option will always be empty. diff --git a/e2e/context/testdata/context-ls-notls.golden b/e2e/context/testdata/context-ls-notls.golden index 2d33df13a4..c36c8e7bb6 100644 --- a/e2e/context/testdata/context-ls-notls.golden +++ b/e2e/context/testdata/context-ls-notls.golden @@ -1,3 +1,3 @@ -NAME DESCRIPTION DOCKER ENDPOINT -default * Current DOCKER_HOST based configuration unix:///var/run/docker.sock -remote my remote cluster ssh://someserver +NAME DESCRIPTION DOCKER ENDPOINT ERROR +default * Current DOCKER_HOST based configuration unix:///var/run/docker.sock +remote my remote cluster ssh://someserver diff --git a/e2e/context/testdata/context-ls-tls.golden b/e2e/context/testdata/context-ls-tls.golden index 13aed65a88..6d9756e7e5 100644 --- a/e2e/context/testdata/context-ls-tls.golden +++ b/e2e/context/testdata/context-ls-tls.golden @@ -1,3 +1,3 @@ -NAME DESCRIPTION DOCKER ENDPOINT -default * Current DOCKER_HOST based configuration unix:///var/run/docker.sock -test unix:///var/run/docker.sock +NAME DESCRIPTION DOCKER ENDPOINT ERROR +default * Current DOCKER_HOST based configuration unix:///var/run/docker.sock +test unix:///var/run/docker.sock diff --git a/e2e/context/testdata/context-ls.golden b/e2e/context/testdata/context-ls.golden index 2d33df13a4..c36c8e7bb6 100644 --- a/e2e/context/testdata/context-ls.golden +++ b/e2e/context/testdata/context-ls.golden @@ -1,3 +1,3 @@ -NAME DESCRIPTION DOCKER ENDPOINT -default * Current DOCKER_HOST based configuration unix:///var/run/docker.sock -remote my remote cluster ssh://someserver +NAME DESCRIPTION DOCKER ENDPOINT ERROR +default * Current DOCKER_HOST based configuration unix:///var/run/docker.sock +remote my remote cluster ssh://someserver From 2c9dff1436fd2ba2a73748707a9685aa13d87f02 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 4 Nov 2022 15:48:30 +0100 Subject: [PATCH 7/7] cli/command/context: context ls: always show current context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit if a context is set (e.g. through DOCKER_CONTEXT or the CLI config file), but wasn't found, then a "stub" context is added, including an error message that the context doesn't exist. DOCKER_CONTEXT=nosuchcontext docker context ls NAME DESCRIPTION DOCKER ENDPOINT ERROR default Current DOCKER_HOST based configuration unix:///var/run/docker.sock nosuchcontext * context "nosuchcontext": context not found: … Signed-off-by: Sebastiaan van Stijn --- cli/command/context/list.go | 19 +++++++++++++++++++ cli/command/context/list_test.go | 8 ++++++++ .../context/testdata/list-with-error.golden | 3 +++ cli/command/context/testdata/list.golden | 10 +++++----- cli/command/formatter/context.go | 4 +++- cli/context/store/metadatastore.go | 4 ++-- 6 files changed, 40 insertions(+), 8 deletions(-) create mode 100644 cli/command/context/testdata/list-with-error.golden diff --git a/cli/command/context/list.go b/cli/command/context/list.go index 6943a99ea2..7798f131ae 100644 --- a/cli/command/context/list.go +++ b/cli/command/context/list.go @@ -50,10 +50,14 @@ func runList(dockerCli command.Cli, opts *listOptions) error { } var ( curContext = dockerCli.CurrentContext() + curFound bool contexts []*formatter.ClientContext ) for _, rawMeta := range contextMap { isCurrent := rawMeta.Name == curContext + if isCurrent { + curFound = true + } meta, err := command.GetDockerContext(rawMeta) if err != nil { // Add a stub-entry to the list, including the error-message @@ -79,6 +83,21 @@ func runList(dockerCli command.Cli, opts *listOptions) error { } contexts = append(contexts, &desc) } + if !curFound { + // The currently specified context wasn't found. We add a stub-entry + // to the list, including the error-message indicating that the context + // wasn't found. + var errMsg string + _, err := dockerCli.ContextStore().GetMetadata(curContext) + if err != nil { + errMsg = err.Error() + } + contexts = append(contexts, &formatter.ClientContext{ + Name: curContext, + Current: true, + Error: errMsg, + }) + } sort.Slice(contexts, func(i, j int) bool { return sortorder.NaturalLess(contexts[i].Name, contexts[j].Name) }) diff --git a/cli/command/context/list_test.go b/cli/command/context/list_test.go index ef6e6dfe0c..98ac2e480c 100644 --- a/cli/command/context/list_test.go +++ b/cli/command/context/list_test.go @@ -39,3 +39,11 @@ func TestListQuiet(t *testing.T) { assert.NilError(t, runList(cli, &listOptions{quiet: true})) golden.Assert(t, cli.OutBuffer().String(), "quiet-list.golden") } + +func TestListError(t *testing.T) { + cli := makeFakeCli(t) + cli.SetCurrentContext("nosuchcontext") + cli.OutBuffer().Reset() + assert.NilError(t, runList(cli, &listOptions{})) + golden.Assert(t, cli.OutBuffer().String(), "list-with-error.golden") +} diff --git a/cli/command/context/testdata/list-with-error.golden b/cli/command/context/testdata/list-with-error.golden new file mode 100644 index 0000000000..6d91b1b3ec --- /dev/null +++ b/cli/command/context/testdata/list-with-error.golden @@ -0,0 +1,3 @@ +NAME DESCRIPTION DOCKER ENDPOINT ERROR +default Current DOCKER_HOST based configuration unix:///var/run/docker.sock +nosuchcontext * context "nosuchcontext": context not found: … diff --git a/cli/command/context/testdata/list.golden b/cli/command/context/testdata/list.golden index aceb43ec60..d9b8121fcf 100644 --- a/cli/command/context/testdata/list.golden +++ b/cli/command/context/testdata/list.golden @@ -1,5 +1,5 @@ -NAME DESCRIPTION DOCKER ENDPOINT -current * description of current https://someswarmserver.example.com -default Current DOCKER_HOST based configuration unix:///var/run/docker.sock -other description of other https://someswarmserver.example.com -unset description of unset https://someswarmserver.example.com +NAME DESCRIPTION DOCKER ENDPOINT ERROR +current * description of current https://someswarmserver.example.com +default Current DOCKER_HOST based configuration unix:///var/run/docker.sock +other description of other https://someswarmserver.example.com +unset description of unset https://someswarmserver.example.com diff --git a/cli/command/formatter/context.go b/cli/command/formatter/context.go index c0a36d1ac9..71a53fd8ff 100644 --- a/cli/command/formatter/context.go +++ b/cli/command/formatter/context.go @@ -6,6 +6,8 @@ const ( dockerEndpointHeader = "DOCKER ENDPOINT" quietContextFormat = "{{.Name}}" + + maxErrLength = 45 ) // NewClientContextFormat returns a Format for rendering using a Context @@ -80,7 +82,7 @@ func (c *clientContextContext) DockerEndpoint() string { // Error returns the truncated error (if any) that occurred when loading the context. func (c *clientContextContext) Error() string { // TODO(thaJeztah) add "--no-trunc" option to context ls and set default to 30 cols to match "docker service ps" - return Ellipsis(c.c.Error, 45) + return Ellipsis(c.c.Error, maxErrLength) } // KubernetesEndpoint returns the kubernetes endpoint. diff --git a/cli/context/store/metadatastore.go b/cli/context/store/metadatastore.go index fa61c7aa48..ba3ea6c05a 100644 --- a/cli/context/store/metadatastore.go +++ b/cli/context/store/metadatastore.go @@ -59,7 +59,7 @@ func parseTypedOrMap(payload []byte, getter TypeGetter) (interface{}, error) { func (s *metadataStore) get(name string) (Metadata, error) { m, err := s.getByID(contextdirOf(name)) if err != nil { - return m, errors.Wrapf(err, "load context %q", name) + return m, errors.Wrapf(err, "context %q", name) } return m, nil } @@ -68,7 +68,7 @@ func (s *metadataStore) getByID(id contextdir) (Metadata, error) { bytes, err := os.ReadFile(filepath.Join(s.contextDir(id), metaFile)) if err != nil { if errors.Is(err, os.ErrNotExist) { - return Metadata{}, errdefs.NotFound(errors.Wrap(err, "context does not exist")) + return Metadata{}, errdefs.NotFound(errors.Wrap(err, "context not found")) } return Metadata{}, err }