Skip to content

Commit

Permalink
Rebase-fix: add a lazy-check annotation
Browse files Browse the repository at this point in the history
Signed-off-by: Simon Ferquel <[email protected]>
  • Loading branch information
simonferquel committed Mar 21, 2019
1 parent 792b6ec commit 19c0830
Show file tree
Hide file tree
Showing 4 changed files with 129 additions and 13 deletions.
51 changes: 51 additions & 0 deletions cli/command/commands/lazychecks/lazychecks.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package lazychecks

import (
"errors"
"fmt"
"strconv"
"strings"

"github.com/docker/cli/cli/command"
"github.com/spf13/pflag"
)

var lazyChecks []LazyCheck

const lazyCheckAnnotation = "lazy-checks"

// LazyCheck is a callback that is called lazily to know if a command / flag should be enabled
type LazyCheck func(clientInfo command.ClientInfo, serverInfo command.ServerInfo, clientVersion string) error

// AddLazyFlagCheck adds a LazyCheck on a flag
func AddLazyFlagCheck(flagset *pflag.FlagSet, name string, check LazyCheck) {
index := len(lazyChecks)
lazyChecks = append(lazyChecks, check)
f := flagset.Lookup(name)
if f == nil {
return
}
if f.Annotations == nil {
f.Annotations = map[string][]string{}
}
f.Annotations[lazyCheckAnnotation] = append(f.Annotations[lazyCheckAnnotation], fmt.Sprintf("%d", index))
}

// EvaluateFlagLazyChacks evaluates the lazy checks associated with a flag depending on client/server info
func EvaluateFlagLazyChacks(flag *pflag.Flag, clientInfo command.ClientInfo, serverInfo command.ServerInfo, clientVersion string) error {
var errs []string
for _, indexStr := range flag.Annotations[lazyCheckAnnotation] {
index, err := strconv.ParseInt(indexStr, 10, 32)
if err != nil {
errs = append(errs, err.Error())
continue
}
if err := lazyChecks[int(index)](clientInfo, serverInfo, clientVersion); err != nil {
errs = append(errs, err.Error())
}
}
if len(errs) == 0 {
return nil
}
return errors.New(strings.Join(errs, "\n"))
}
23 changes: 16 additions & 7 deletions cli/command/image/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import (
"runtime"
"strings"

"github.com/docker/cli/cli/command/commands/lazychecks"

"github.com/docker/cli/cli"
"github.com/docker/cli/cli/command"
"github.com/docker/cli/cli/command/image/build"
Expand All @@ -23,6 +25,7 @@ import (
"github.com/docker/docker/api"
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/container"
"github.com/docker/docker/api/types/versions"
"github.com/docker/docker/pkg/archive"
"github.com/docker/docker/pkg/idtools"
"github.com/docker/docker/pkg/jsonmessage"
Expand Down Expand Up @@ -149,13 +152,19 @@ func NewBuildCommand(dockerCli command.Cli) *cobra.Command {

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)
}
lazychecks.AddLazyFlagCheck(flags, "platform", func(ci command.ClientInfo, si command.ServerInfo, clientVersion string) error {
buildkitEnabled, err := command.BuildKitEnabled(si)
if err == nil && buildkitEnabled {
if versions.LessThan(clientVersion, "1.38") {
return fmt.Errorf("with buildkit enabled, --platform flag requires API version 1.38 or greater (current is %s)", clientVersion)
}
} else {
if versions.LessThan(clientVersion, "1.32") || !si.HasExperimental {
return fmt.Errorf("with buildkit disabled, --platform requires a daemon with API version 1.32 or greater and experimental enabled")
}
}
return nil
})

flags.BoolVar(&options.squash, "squash", false, "Squash newly built layers into a single new layer")
flags.SetAnnotation("squash", "experimental", nil)
Expand Down
28 changes: 22 additions & 6 deletions cmd/docker/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
"strings"
"syscall"

"github.com/docker/cli/cli/command/commands/lazychecks"

"github.com/docker/cli/cli"
pluginmanager "github.com/docker/cli/cli-plugins/manager"
"github.com/docker/cli/cli/command"
Expand Down Expand Up @@ -271,6 +273,12 @@ func hideFeatureFlag(f *pflag.Flag, hasFeature bool, annotation string) {
}
}

func hideFailingFlagLazyChecks(f *pflag.Flag, clientInfo command.ClientInfo, serverInfo command.ServerInfo, clientVersion string) {
if lazychecks.EvaluateFlagLazyChacks(f, clientInfo, serverInfo, clientVersion) != nil {
f.Hidden = true
}
}

func hideFeatureSubCommand(subcmd *cobra.Command, hasFeature bool, annotation string) {
if hasFeature {
return
Expand All @@ -282,10 +290,12 @@ func hideFeatureSubCommand(subcmd *cobra.Command, hasFeature bool, annotation st

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())
serverInfo := details.ServerInfo()
clientInfo := details.ClientInfo()
osType := serverInfo.OSType
hasExperimental := serverInfo.HasExperimental
hasExperimentalCLI := clientInfo.HasExperimental
hasBuildKit, err := command.BuildKitEnabled(serverInfo)
if err != nil {
return err
}
Expand All @@ -295,6 +305,7 @@ func hideUnsupportedFeatures(cmd *cobra.Command, details versionDetails) error {
hideFeatureFlag(f, hasExperimentalCLI, "experimentalCLI")
hideFeatureFlag(f, hasBuildKit, "buildkit")
hideFeatureFlag(f, !hasBuildKit, "no-buildkit")
hideFailingFlagLazyChecks(f, clientInfo, serverInfo, clientVersion)
// hide flags not supported by the server
if !isOSTypeSupported(f, osType) || !isVersionSupported(f, clientVersion) {
f.Hidden = true
Expand Down Expand Up @@ -353,8 +364,10 @@ func areFlagsSupported(cmd *cobra.Command, details versionDetails) error {

if !isLocalOnly(cmd) {
clientVersion := details.Client().ClientVersion()
osType := details.ServerInfo().OSType
hasExperimental := details.ServerInfo().HasExperimental
serverInfo := details.ServerInfo()
clientInfo := details.ClientInfo()
osType := serverInfo.OSType
hasExperimental := serverInfo.HasExperimental
checks = append(checks,
flagAnnotationCheck("version", func(f *pflag.Flag) error {
if !isVersionSupported(f, clientVersion) {
Expand All @@ -368,6 +381,9 @@ func areFlagsSupported(cmd *cobra.Command, details versionDetails) error {
}
return nil
}),
func(f *pflag.Flag) error {
return lazychecks.EvaluateFlagLazyChacks(f, clientInfo, serverInfo, clientVersion)
},
)
if !hasExperimental {
checks = append(checks, flagAnnotationCheck("experimental", func(f *pflag.Flag) error {
Expand Down
40 changes: 40 additions & 0 deletions cmd/docker/docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"os"
"testing"

"github.com/docker/cli/cli/command/commands/lazychecks"

"github.com/docker/cli/cli/command"
"github.com/docker/cli/cli/command/commands"
"github.com/docker/cli/cli/debug"
Expand Down Expand Up @@ -123,6 +125,26 @@ func commandWithParent(parent *cobra.Command, annotations ...kv) *cobra.Command
return cmd
}

type lazyCheckFlag struct {
flag string
err error
isSet bool
}

func commandWithLazyCheckFlag(f lazyCheckFlag) *cobra.Command {
cmd := &cobra.Command{
Use: "test",
}
cmd.Flags().String(f.flag, "", "")
lazychecks.AddLazyFlagCheck(cmd.Flags(), f.flag, func(_ command.ClientInfo, _ command.ServerInfo, _ string) error {
return f.err
})
if f.isSet {
cmd.Flags().Set(f.flag, "value")
}
return cmd
}

func TestIsLocalOnly(t *testing.T) {
cases := []struct {
name string
Expand Down Expand Up @@ -461,6 +483,24 @@ func TestIsSupported(t *testing.T) {
},
expectedError: "",
},
{
name: "lazy-check-unset",
cmd: commandWithLazyCheckFlag(lazyCheckFlag{flag: "flag1", isSet: false, err: errors.New("boom")}),
details: &testVersionDetails{
clientVersion: "1.25",
serverExperimental: true,
},
expectedError: "",
},
{
name: "lazy-check-set",
cmd: commandWithLazyCheckFlag(lazyCheckFlag{flag: "flag1", isSet: true, err: errors.New("boom")}),
details: &testVersionDetails{
clientVersion: "1.25",
serverExperimental: true,
},
expectedError: "boom",
},
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
Expand Down

0 comments on commit 19c0830

Please sign in to comment.