Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

context: implement lazy loading, and other improvements #3847

Merged
merged 7 commits into from
Nov 28, 2022
60 changes: 44 additions & 16 deletions cli/command/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ package command

import (
"context"
"fmt"
"io"
"os"
"path/filepath"
"runtime"
"strconv"
"strings"
"sync"
"time"

"github.com/docker/cli/cli/config"
Expand Down Expand Up @@ -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
Expand All @@ -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
}
Expand All @@ -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
}

Expand Down Expand Up @@ -143,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
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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 {
Expand Down
6 changes: 4 additions & 2 deletions cli/command/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
})
Expand Down Expand Up @@ -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)
}()

Expand Down
32 changes: 30 additions & 2 deletions cli/command/context/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,26 +50,54 @@ 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 {
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)
}
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)
})
Expand Down
8 changes: 8 additions & 0 deletions cli/command/context/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
3 changes: 3 additions & 0 deletions cli/command/context/testdata/list-with-error.golden
Original file line number Diff line number Diff line change
@@ -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: …
10 changes: 5 additions & 5 deletions cli/command/context/testdata/list.golden
Original file line number Diff line number Diff line change
@@ -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
18 changes: 14 additions & 4 deletions cli/command/formatter/context.go
Original file line number Diff line number Diff line change
@@ -1,20 +1,22 @@
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}}"

maxErrLength = 45
)

// 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)
}
Expand All @@ -25,6 +27,7 @@ type ClientContext struct {
Description string
DockerEndpoint string
Current bool
Error string
}

// ClientContextWrite writes formatted contexts using the Context
Expand All @@ -51,6 +54,7 @@ func newClientContextContext() *clientContextContext {
"Name": NameHeader,
"Description": DescriptionHeader,
"DockerEndpoint": dockerEndpointHeader,
"Error": ErrorHeader,
}
return &ctx
}
Expand All @@ -75,6 +79,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, maxErrLength)
}

// KubernetesEndpoint returns the kubernetes endpoint.
//
// Deprecated: support for kubernetes endpoints in contexts has been removed, and this formatting option will always be empty.
Expand Down
1 change: 1 addition & 0 deletions cli/command/formatter/custom.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const (
StatusHeader = "STATUS"
PortsHeader = "PORTS"
ImageHeader = "IMAGE"
ErrorHeader = "ERROR"
ContainerIDHeader = "CONTAINER ID"
)

Expand Down
9 changes: 4 additions & 5 deletions cli/command/task/formatter.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ const (
taskIDHeader = "ID"
desiredStateHeader = "DESIRED STATE"
currentStateHeader = "CURRENT STATE"
errorHeader = "ERROR"

maxErrLength = 30
)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -124,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
}
Expand Down
4 changes: 2 additions & 2 deletions cli/context/store/metadatastore.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down
6 changes: 3 additions & 3 deletions e2e/context/testdata/context-ls-notls.golden
Original file line number Diff line number Diff line change
@@ -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
6 changes: 3 additions & 3 deletions e2e/context/testdata/context-ls-tls.golden
Original file line number Diff line number Diff line change
@@ -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
6 changes: 3 additions & 3 deletions e2e/context/testdata/context-ls.golden
Original file line number Diff line number Diff line change
@@ -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