Guest24897
/
DockerCLI
mirror of https://github.com/docker/cli.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

cli: perform feature detection lazily 

- Perform feature detection when actually needed, instead of during
  initializing
- Version negotiation is performed either when making an API request,
  or when (e.g.) running `docker help` (to hide unsupported features)
- Use a 2 second timeout when 'pinging' the daemon; this should be
  sufficient for most cases, and when feature detection failed, the
  daemon will still perform validation (and produce an error if needed)
- context.WithTimeout doesn't currently work with ssh connections (connhelper),
  so we're only applying this timeout for tcp:// connections, otherwise
  keep the old behavior.

Before this change:

    time sh -c 'DOCKER_HOST=tcp://42.42.42.41:4242 docker help &> /dev/null'
    real   0m32.919s
    user   0m0.370s
    sys    0m0.227s

    time sh -c 'DOCKER_HOST=tcp://42.42.42.41:4242 docker context ls &> /dev/null'
    real   0m32.072s
    user   0m0.029s
    sys    0m0.023s

After this change:

    time sh -c 'DOCKER_HOST=tcp://42.42.42.41:4242 docker help &> /dev/null'
    real   0m 2.28s
    user   0m 0.03s
    sys    0m 0.03s

    time sh -c 'DOCKER_HOST=tcp://42.42.42.41:4242 docker context ls &> /dev/null'
    real   0m 0.13s
    user   0m 0.02s
    sys    0m 0.02s

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This commit is contained in:
Sebastiaan van Stijn 2020-04-08 00:57:41 +02:00
parent a88a1bea23
commit b39739123b
No known key found for this signature in database
GPG Key ID: 76698F39D527CE8C
2 changed files with 81 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
cli/command/cli.go
View File

@ -8,6 +8,9 @@ import (
"path/filepath" "path/filepath"
"runtime" "runtime"
"strconv" "strconv"
"strings"
"sync"
"time"
"github.com/docker/cli/cli/config" "github.com/docker/cli/cli/config"
cliconfig "github.com/docker/cli/cli/config" cliconfig "github.com/docker/cli/cli/config"
@ -133,9 +136,12 @@ func (cli *DockerCli) loadConfigFile() {
cli.configFile = cliconfig.LoadDefaultConfigFile(cli.err) cli.configFile = cliconfig.LoadDefaultConfigFile(cli.err)
} }
var fetchServerInfo sync.Once
// ServerInfo returns the server version details for the host this client is // ServerInfo returns the server version details for the host this client is
// connected to // connected to
func (cli *DockerCli) ServerInfo() ServerInfo { func (cli *DockerCli) ServerInfo() ServerInfo {
fetchServerInfo.Do(cli.initializeFromClient)
return cli.serverInfo return cli.serverInfo
} }
@ -270,11 +276,6 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions, ops ...Initialize
return err return err
} }
} }
err = cli.loadClientInfo()
if err != nil {
return err
}
cli.initializeFromClient()
return nil return nil
} }
@ -365,7 +366,16 @@ func isEnabled(value string) (bool, error) {
} }
func (cli *DockerCli) initializeFromClient() { func (cli *DockerCli) initializeFromClient() {
ping, err := cli.client.Ping(context.Background()) ctx := context.Background()
if strings.HasPrefix(cli.DockerEndpoint().Host, "tcp://") {
// @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()
ctx, cancel = context.WithTimeout(ctx, 2*time.Second)
defer cancel()
}
ping, err := cli.client.Ping(ctx)
if err != nil { if err != nil {
// Default to true if we fail to connect to daemon // Default to true if we fail to connect to daemon
cli.serverInfo = ServerInfo{HasExperimental: true} cli.serverInfo = ServerInfo{HasExperimental: true}

117
cmd/docker/docker.go
View File

@ -312,63 +312,65 @@ type versionDetails interface {
ServerInfo() command.ServerInfo ServerInfo() command.ServerInfo
} }
func hideFeatureFlag(f *pflag.Flag, hasFeature bool, annotation string) { func hideFlagIf(f *pflag.Flag, condition func(string) bool, annotation string) {
if hasFeature { if f.Hidden {
return return
} }
if _, ok := f.Annotations[annotation]; ok { if values, ok := f.Annotations[annotation]; ok && len(values) > 0 {
if condition(values[0]) {
f.Hidden = true f.Hidden = true
} }
} }
}
func hideFeatureSubCommand(subcmd *cobra.Command, hasFeature bool, annotation string) { func hideSubcommandIf(subcmd *cobra.Command, condition func(string) bool, annotation string) {
if hasFeature { if subcmd.Hidden {
return return
} }
if _, ok := subcmd.Annotations[annotation]; ok { if v, ok := subcmd.Annotations[annotation]; ok {
if condition(v) {
subcmd.Hidden = true subcmd.Hidden = true
} }
} }
}
func hideUnsupportedFeatures(cmd *cobra.Command, details versionDetails) error { func hideUnsupportedFeatures(cmd *cobra.Command, details versionDetails) error {
clientVersion := details.Client().ClientVersion() var (
osType := details.ServerInfo().OSType buildKitDisabled = func(_ string) bool { v, _ := command.BuildKitEnabled(details.ServerInfo()); return !v }
hasExperimental := details.ServerInfo().HasExperimental buildKitEnabled = func(_ string) bool { v, _ := command.BuildKitEnabled(details.ServerInfo()); return v }
hasExperimentalCLI := details.ClientInfo().HasExperimental notExperimental = func(_ string) bool { return !details.ServerInfo().HasExperimental }
hasBuildKit, err := command.BuildKitEnabled(details.ServerInfo()) notExperimentalCLI = func(_ string) bool { return !details.ClientInfo().HasExperimental }
if err != nil { notOSType = func(v string) bool { return v != details.ServerInfo().OSType }
return err versionOlderThan = func(v string) bool { return versions.LessThan(details.Client().ClientVersion(), v) }
} )
cmd.Flags().VisitAll(func(f *pflag.Flag) { cmd.Flags().VisitAll(func(f *pflag.Flag) {
hideFeatureFlag(f, hasExperimental, "experimental")
hideFeatureFlag(f, hasExperimentalCLI, "experimentalCLI")
hideFeatureFlag(f, hasBuildKit, "buildkit")
hideFeatureFlag(f, !hasBuildKit, "no-buildkit")
// hide flags not supported by the server // hide flags not supported by the server
if !isOSTypeSupported(f, osType) || !isVersionSupported(f, clientVersion) {
f.Hidden = true
}
// root command shows all top-level flags // root command shows all top-level flags
if cmd.Parent() != nil { if cmd.Parent() != nil {
if commands, ok := f.Annotations["top-level"]; ok { if cmds, ok := f.Annotations["top-level"]; ok {
f.Hidden = !findCommand(cmd, commands) f.Hidden = !findCommand(cmd, cmds)
}
if f.Hidden {
return
} }
} }
hideFlagIf(f, buildKitDisabled, "buildkit")
hideFlagIf(f, buildKitEnabled, "no-buildkit")
hideFlagIf(f, notExperimental, "experimental")
hideFlagIf(f, notExperimentalCLI, "experimentalCLI")
hideFlagIf(f, notOSType, "ostype")
hideFlagIf(f, versionOlderThan, "version")
}) })
for _, subcmd := range cmd.Commands() { for _, subcmd := range cmd.Commands() {
hideFeatureSubCommand(subcmd, hasExperimental, "experimental") hideSubcommandIf(subcmd, buildKitDisabled, "buildkit")
hideFeatureSubCommand(subcmd, hasExperimentalCLI, "experimentalCLI") hideSubcommandIf(subcmd, buildKitEnabled, "no-buildkit")
hideFeatureSubCommand(subcmd, hasBuildKit, "buildkit") hideSubcommandIf(subcmd, notExperimental, "experimental")
hideFeatureSubCommand(subcmd, !hasBuildKit, "no-buildkit") hideSubcommandIf(subcmd, notExperimentalCLI, "experimentalCLI")
// hide subcommands not supported by the server hideSubcommandIf(subcmd, notOSType, "ostype")
if subcmdVersion, ok := subcmd.Annotations["version"]; ok && versions.LessThan(clientVersion, subcmdVersion) { hideSubcommandIf(subcmd, versionOlderThan, "version")
subcmd.Hidden = true
}
if v, ok := subcmd.Annotations["ostype"]; ok && v != osType {
subcmd.Hidden = true
}
} }
return nil return nil
} }
@ -394,31 +396,31 @@ func isSupported(cmd *cobra.Command, details versionDetails) error {
} }
func areFlagsSupported(cmd *cobra.Command, details versionDetails) error { func areFlagsSupported(cmd *cobra.Command, details versionDetails) error {
clientVersion := details.Client().ClientVersion()
osType := details.ServerInfo().OSType
hasExperimental := details.ServerInfo().HasExperimental
hasExperimentalCLI := details.ClientInfo().HasExperimental
errs := []string{} errs := []string{}
cmd.Flags().VisitAll(func(f *pflag.Flag) { cmd.Flags().VisitAll(func(f *pflag.Flag) {
if f.Changed { if !f.Changed {
if !isVersionSupported(f, clientVersion) {
errs = append(errs, fmt.Sprintf("\"--%s\" requires API version %s, but the Docker daemon API version is %s", f.Name, getFlagAnnotation(f, "version"), clientVersion))
return return
} }
if !isOSTypeSupported(f, osType) { if !isVersionSupported(f, details.Client().ClientVersion()) {
errs = append(errs, fmt.Sprintf("\"--%s\" is only supported on a Docker daemon running on %s, but the Docker daemon is running on %s", f.Name, getFlagAnnotation(f, "ostype"), osType)) errs = append(errs, fmt.Sprintf(`"--%s" requires API version %s, but the Docker daemon API version is %s`, f.Name, getFlagAnnotation(f, "version"), details.Client().ClientVersion()))
return return
} }
if _, ok := f.Annotations["experimental"]; ok && !hasExperimental { if !isOSTypeSupported(f, details.ServerInfo().OSType) {
errs = append(errs, fmt.Sprintf("\"--%s\" is only supported on a Docker daemon with experimental features enabled", f.Name)) errs = append(errs, fmt.Sprintf(
`"--%s" is only supported on a Docker daemon running on %s, but the Docker daemon is running on %s`,
f.Name,
getFlagAnnotation(f, "ostype"), details.ServerInfo().OSType),
)
return
} }
if _, ok := f.Annotations["experimentalCLI"]; ok && !hasExperimentalCLI { if _, ok := f.Annotations["experimental"]; ok && !details.ServerInfo().HasExperimental {
errs = append(errs, fmt.Sprintf("\"--%s\" is only supported on a Docker cli with experimental cli features enabled", f.Name)) errs = append(errs, fmt.Sprintf(`"--%s" is only supported on a Docker daemon with experimental features enabled`, f.Name))
}
if _, ok := f.Annotations["experimentalCLI"]; ok && !details.ClientInfo().HasExperimental {
errs = append(errs, fmt.Sprintf(`"--%s" is only supported on a Docker cli with experimental cli features enabled`, f.Name))
} }
// buildkit-specific flags are noop when buildkit is not enabled, so we do not add an error in that case // buildkit-specific flags are noop when buildkit is not enabled, so we do not add an error in that case
}
}) })
if len(errs) > 0 { if len(errs) > 0 {
return errors.New(strings.Join(errs, "\n")) return errors.New(strings.Join(errs, "\n"))
@ -428,23 +430,18 @@ func areFlagsSupported(cmd *cobra.Command, details versionDetails) error {
// Check recursively so that, e.g., `docker stack ls` returns the same output as `docker stack` // Check recursively so that, e.g., `docker stack ls` returns the same output as `docker stack`
func areSubcommandsSupported(cmd *cobra.Command, details versionDetails) error { func areSubcommandsSupported(cmd *cobra.Command, details versionDetails) error {
clientVersion := details.Client().ClientVersion()
osType := details.ServerInfo().OSType
hasExperimental := details.ServerInfo().HasExperimental
hasExperimentalCLI := details.ClientInfo().HasExperimental
// Check recursively so that, e.g., `docker stack ls` returns the same output as `docker stack` // Check recursively so that, e.g., `docker stack ls` returns the same output as `docker stack`
for curr := cmd; curr != nil; curr = curr.Parent() { for curr := cmd; curr != nil; curr = curr.Parent() {
if cmdVersion, ok := curr.Annotations["version"]; ok && versions.LessThan(clientVersion, cmdVersion) { if cmdVersion, ok := curr.Annotations["version"]; ok && versions.LessThan(details.Client().ClientVersion(), cmdVersion) {
return fmt.Errorf("%s requires API version %s, but the Docker daemon API version is %s", cmd.CommandPath(), cmdVersion, clientVersion) return fmt.Errorf("%s requires API version %s, but the Docker daemon API version is %s", cmd.CommandPath(), cmdVersion, details.Client().ClientVersion())
} }
if os, ok := curr.Annotations["ostype"]; ok && os != osType { if os, ok := curr.Annotations["ostype"]; ok && os != details.ServerInfo().OSType {
return fmt.Errorf("%s is only supported on a Docker daemon running on %s, but the Docker daemon is running on %s", cmd.CommandPath(), os, osType) return fmt.Errorf("%s is only supported on a Docker daemon running on %s, but the Docker daemon is running on %s", cmd.CommandPath(), os, details.ServerInfo().OSType)
} }
if _, ok := curr.Annotations["experimental"]; ok && !hasExperimental { if _, ok := curr.Annotations["experimental"]; ok && !details.ServerInfo().HasExperimental {
return fmt.Errorf("%s is only supported on a Docker daemon with experimental features enabled", cmd.CommandPath()) return fmt.Errorf("%s is only supported on a Docker daemon with experimental features enabled", cmd.CommandPath())
} }
if _, ok := curr.Annotations["experimentalCLI"]; ok && !hasExperimentalCLI { if _, ok := curr.Annotations["experimentalCLI"]; ok && !details.ClientInfo().HasExperimental {
return fmt.Errorf("%s is only supported on a Docker cli with experimental cli features enabled", cmd.CommandPath()) return fmt.Errorf("%s is only supported on a Docker cli with experimental cli features enabled", cmd.CommandPath())
} }
} }