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

Merge pull request #2457 from thaJeztah/19.03_backport_lazy_feature_detection 

[19.03 backport] cli: perform feature detection lazily
This commit is contained in:
Silvin Lubecki 2020-04-30 17:43:14 +02:00 committed by GitHub
parent 62baa3ce57 cf663b526a
commit 3929933368
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 149 additions and 96 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
cli/command/cli.go
View File

@ -8,6 +8,9 @@ import (
"path/filepath"
"runtime"
"strconv"
"strings"
"sync"
"time"
"github.com/docker/cli/cli/config"
cliconfig "github.com/docker/cli/cli/config"
@ -25,6 +28,7 @@ import (
"github.com/docker/cli/internal/containerizedengine"
dopts "github.com/docker/cli/opts"
clitypes "github.com/docker/cli/types"
"github.com/docker/docker/api"
"github.com/docker/docker/api/types"
registrytypes "github.com/docker/docker/api/types/registry"
"github.com/docker/docker/client"
@ -76,7 +80,7 @@ type DockerCli struct {
err io.Writer
client client.APIClient
serverInfo ServerInfo
clientInfo ClientInfo
clientInfo *ClientInfo
contentTrust bool
newContainerizeClient func(string) (clitypes.ContainerizedClient, error)
contextStore store.Store
@ -87,7 +91,7 @@ type DockerCli struct {
// DefaultVersion returns api.defaultVersion or DOCKER_API_VERSION if specified.
func (cli *DockerCli) DefaultVersion() string {
return cli.clientInfo.DefaultVersion
return cli.ClientInfo().DefaultVersion
}
// Client returns the APIClient
@ -126,18 +130,55 @@ func ShowHelp(err io.Writer) func(*cobra.Command, []string) error {
// ConfigFile returns the ConfigFile
func (cli *DockerCli) ConfigFile() *configfile.ConfigFile {
if cli.configFile == nil {
cli.loadConfigFile()
}
return cli.configFile
}
func (cli *DockerCli) loadConfigFile() {
cli.configFile = cliconfig.LoadDefaultConfigFile(cli.err)
}
var fetchServerInfo sync.Once
// ServerInfo returns the server version details for the host this client is
// connected to
func (cli *DockerCli) ServerInfo() ServerInfo {
fetchServerInfo.Do(cli.initializeFromClient)
return cli.serverInfo
}
// ClientInfo returns the client details for the cli
func (cli *DockerCli) ClientInfo() ClientInfo {
return cli.clientInfo
if cli.clientInfo == nil {
_ = cli.loadClientInfo()
}
return *cli.clientInfo
}
func (cli *DockerCli) loadClientInfo() error {
var experimentalValue string
// Environment variable always overrides configuration
if experimentalValue = os.Getenv("DOCKER_CLI_EXPERIMENTAL"); experimentalValue == "" {
experimentalValue = cli.ConfigFile().Experimental
}
hasExperimental, err := isEnabled(experimentalValue)
if err != nil {
return errors.Wrap(err, "Experimental field")
}
var v string
if cli.client != nil {
v = cli.client.ClientVersion()
} else {
v = api.DefaultVersion
}
cli.clientInfo = &ClientInfo{
DefaultVersion: v,
HasExperimental: hasExperimental,
}
return nil
}
// ContentTrustEnabled returns whether content trust has been enabled by an
@ -207,7 +248,7 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions, ops ...Initialize
debug.Enable()
}
cli.configFile = cliconfig.LoadDefaultConfigFile(cli.err)
cli.loadConfigFile()
baseContextStore := store.New(cliconfig.ContextStoreDir(), cli.contextStoreConfig)
cli.contextStore = &ContextStoreWithDefault{
@ -239,20 +280,6 @@ func (cli *DockerCli) Initialize(opts *cliflags.ClientOptions, ops ...Initialize
return err
}
}
var experimentalValue string
// Environment variable always overrides configuration
if experimentalValue = os.Getenv("DOCKER_CLI_EXPERIMENTAL"); experimentalValue == "" {
experimentalValue = cli.configFile.Experimental
}
hasExperimental, err := isEnabled(experimentalValue)
if err != nil {
return errors.Wrap(err, "Experimental field")
}
cli.clientInfo = ClientInfo{
DefaultVersion: cli.client.ClientVersion(),
HasExperimental: hasExperimental,
}
cli.initializeFromClient()
return nil
}
@ -343,7 +370,16 @@ func isEnabled(value string) (bool, error) {
}
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 {
// Default to true if we fail to connect to daemon
cli.serverInfo = ServerInfo{HasExperimental: true}

29
cli/command/image/build.go
View File

@ -115,6 +115,25 @@ func NewBuildCommand(dockerCli command.Cli) *cobra.Command {
},
}
// Wrap the global pre-run to handle non-BuildKit use of the --platform flag.
//
// We're doing it here so that we're only contacting the daemon when actually
// running the command, and not during initialization.
// TODO remove this hack once we no longer support the experimental use of --platform
rootFn := cmd.Root().PersistentPreRunE
cmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error {
if ok, _ := command.BuildKitEnabled(dockerCli.ServerInfo()); !ok {
f := cmd.Flag("platform")
delete(f.Annotations, "buildkit")
f.Annotations["version"] = []string{"1.32"}
f.Annotations["experimental"] = nil
}
if rootFn != nil {
return rootFn(cmd, args)
}
return nil
}
flags := cmd.Flags()
flags.VarP(&options.tags, "tag", "t", "Name and optionally a tag in the 'name:tag' format")
@ -163,14 +182,8 @@ func NewBuildCommand(dockerCli command.Cli) *cobra.Command {
command.AddTrustVerificationFlags(flags, &options.untrusted, dockerCli.ContentTrustEnabled())
flags.StringVar(&options.platform, "platform", os.Getenv("DOCKER_DEFAULT_PLATFORM"), "Set platform if server is multi-platform capable")
// Platform is not experimental when BuildKit is used
buildkitEnabled, err := command.BuildKitEnabled(dockerCli.ServerInfo())
if err == nil && buildkitEnabled {
flags.SetAnnotation("platform", "version", []string{"1.38"})
} else {
flags.SetAnnotation("platform", "version", []string{"1.32"})
flags.SetAnnotation("platform", "experimental", nil)
}
flags.SetAnnotation("platform", "version", []string{"1.38"})
flags.SetAnnotation("platform", "buildkit", nil)
flags.BoolVar(&options.squash, "squash", false, "Squash newly built layers into a single new layer")
flags.SetAnnotation("squash", "experimental", nil)

9
cli/command/image/build_test.go
View File

@ -20,11 +20,13 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/moby/buildkit/session/secrets/secretsprovider"
"gotest.tools/v3/assert"
"gotest.tools/v3/env"
"gotest.tools/v3/fs"
"gotest.tools/v3/skip"
)
func TestRunBuildDockerfileFromStdinWithCompress(t *testing.T) {
defer env.Patch(t, "DOCKER_BUILDKIT", "0")()
buffer := new(bytes.Buffer)
fakeBuild := newFakeBuild()
fakeImageBuild := func(ctx context.Context, context io.Reader, options types.ImageBuildOptions) (types.ImageBuildResponse, error) {
@ -60,6 +62,7 @@ func TestRunBuildDockerfileFromStdinWithCompress(t *testing.T) {
}
func TestRunBuildResetsUidAndGidInContext(t *testing.T) {
defer env.Patch(t, "DOCKER_BUILDKIT", "0")()
skip.If(t, os.Getuid() != 0, "root is required to chown files")
fakeBuild := newFakeBuild()
cli := test.NewFakeCli(&fakeClient{imageBuildFunc: fakeBuild.build})
@ -90,6 +93,7 @@ func TestRunBuildResetsUidAndGidInContext(t *testing.T) {
}
func TestRunBuildDockerfileOutsideContext(t *testing.T) {
defer env.Patch(t, "DOCKER_BUILDKIT", "0")()
dir := fs.NewDir(t, t.Name(),
fs.WithFile("data", "data file"))
defer dir.Remove()
@ -122,7 +126,8 @@ COPY data /data
// TODO: test "context selection" logic directly when runBuild is refactored
// to support testing (ex: docker/cli#294)
func TestRunBuildFromGitHubSpecialCase(t *testing.T) {
cmd := NewBuildCommand(test.NewFakeCli(nil))
defer env.Patch(t, "DOCKER_BUILDKIT", "0")()
cmd := NewBuildCommand(test.NewFakeCli(&fakeClient{}))
// Clone a small repo that exists so git doesn't prompt for credentials
cmd.SetArgs([]string{"github.com/docker/for-win"})
cmd.SetOutput(ioutil.Discard)
@ -135,6 +140,7 @@ func TestRunBuildFromGitHubSpecialCase(t *testing.T) {
// starting with `github.com` takes precedence over the `github.com` special
// case.
func TestRunBuildFromLocalGitHubDir(t *testing.T) {
defer env.Patch(t, "DOCKER_BUILDKIT", "0")()
tmpDir, err := ioutil.TempDir("", "docker-build-from-local-dir-")
assert.NilError(t, err)
defer os.RemoveAll(tmpDir)
@ -154,6 +160,7 @@ func TestRunBuildFromLocalGitHubDir(t *testing.T) {
}
func TestRunBuildWithSymlinkedContext(t *testing.T) {
defer env.Patch(t, "DOCKER_BUILDKIT", "0")()
dockerfile := `
FROM alpine:3.6
RUN echo hello world

133
cmd/docker/docker.go
View File

@ -312,63 +312,65 @@ type versionDetails interface {
ServerInfo() command.ServerInfo
}
func hideFeatureFlag(f *pflag.Flag, hasFeature bool, annotation string) {
if hasFeature {
func hideFlagIf(f *pflag.Flag, condition func(string) bool, annotation string) {
if f.Hidden {
return
}
if _, ok := f.Annotations[annotation]; ok {
f.Hidden = true
if values, ok := f.Annotations[annotation]; ok && len(values) > 0 {
if condition(values[0]) {
f.Hidden = true
}
}
}
func hideFeatureSubCommand(subcmd *cobra.Command, hasFeature bool, annotation string) {
if hasFeature {
func hideSubcommandIf(subcmd *cobra.Command, condition func(string) bool, annotation string) {
if subcmd.Hidden {
return
}
if _, ok := subcmd.Annotations[annotation]; ok {
subcmd.Hidden = true
if v, ok := subcmd.Annotations[annotation]; ok {
if condition(v) {
subcmd.Hidden = true
}
}
}
func hideUnsupportedFeatures(cmd *cobra.Command, details versionDetails) error {
clientVersion := details.Client().ClientVersion()
osType := details.ServerInfo().OSType
hasExperimental := details.ServerInfo().HasExperimental
hasExperimentalCLI := details.ClientInfo().HasExperimental
hasBuildKit, err := command.BuildKitEnabled(details.ServerInfo())
if err != nil {
return err
}
var (
buildKitDisabled = func(_ string) bool { v, _ := command.BuildKitEnabled(details.ServerInfo()); return !v }
buildKitEnabled = func(_ string) bool { v, _ := command.BuildKitEnabled(details.ServerInfo()); return v }
notExperimental = func(_ string) bool { return !details.ServerInfo().HasExperimental }
notExperimentalCLI = func(_ string) bool { return !details.ClientInfo().HasExperimental }
notOSType = func(v string) bool { return v != details.ServerInfo().OSType }
versionOlderThan = func(v string) bool { return versions.LessThan(details.Client().ClientVersion(), v) }
)
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
if !isOSTypeSupported(f, osType) || !isVersionSupported(f, clientVersion) {
f.Hidden = true
}
// root command shows all top-level flags
if cmd.Parent() != nil {
if commands, ok := f.Annotations["top-level"]; ok {
f.Hidden = !findCommand(cmd, commands)
if cmds, ok := f.Annotations["top-level"]; ok {
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() {
hideFeatureSubCommand(subcmd, hasExperimental, "experimental")
hideFeatureSubCommand(subcmd, hasExperimentalCLI, "experimentalCLI")
hideFeatureSubCommand(subcmd, hasBuildKit, "buildkit")
hideFeatureSubCommand(subcmd, !hasBuildKit, "no-buildkit")
// hide subcommands not supported by the server
if subcmdVersion, ok := subcmd.Annotations["version"]; ok && versions.LessThan(clientVersion, subcmdVersion) {
subcmd.Hidden = true
}
if v, ok := subcmd.Annotations["ostype"]; ok && v != osType {
subcmd.Hidden = true
}
hideSubcommandIf(subcmd, buildKitDisabled, "buildkit")
hideSubcommandIf(subcmd, buildKitEnabled, "no-buildkit")
hideSubcommandIf(subcmd, notExperimental, "experimental")
hideSubcommandIf(subcmd, notExperimentalCLI, "experimentalCLI")
hideSubcommandIf(subcmd, notOSType, "ostype")
hideSubcommandIf(subcmd, versionOlderThan, "version")
}
return nil
}
@ -394,31 +396,31 @@ func isSupported(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{}
cmd.Flags().VisitAll(func(f *pflag.Flag) {
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
}
if !isOSTypeSupported(f, osType) {
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))
return
}
if _, ok := f.Annotations["experimental"]; ok && !hasExperimental {
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 && !hasExperimentalCLI {
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
if !f.Changed {
return
}
if !isVersionSupported(f, details.Client().ClientVersion()) {
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
}
if !isOSTypeSupported(f, details.ServerInfo().OSType) {
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["experimental"]; ok && !details.ServerInfo().HasExperimental {
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
})
if len(errs) > 0 {
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`
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`
for curr := cmd; curr != nil; curr = curr.Parent() {
if cmdVersion, ok := curr.Annotations["version"]; ok && versions.LessThan(clientVersion, cmdVersion) {
return fmt.Errorf("%s requires API version %s, but the Docker daemon API version is %s", cmd.CommandPath(), cmdVersion, clientVersion)
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, details.Client().ClientVersion())
}
if os, ok := curr.Annotations["ostype"]; ok && 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, 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, 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())
}
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())
}
}