From 8e688f17a35a01023b58b0da56b917a7d4345c38 Mon Sep 17 00:00:00 2001 From: Yong Tang Date: Sun, 20 Nov 2016 09:57:06 -0800 Subject: [PATCH] Improve error handling of experimental features in non-experimental mode This fix tries to address several issues raised in 28626 where run against a non-experimental daemon may not generate correct error message: 1. Incorrect flags were not checked against the supported features: ``` $ docker stack --nonsense unknown flag: --nonsense ``` 2. Subcommands were not checked against the supported features: ``` $ docker stack ls Error response from daemon: This node is not a swarm manager... ``` This fix address the above mentioned issues by: 1. Add a pre-check for FlagErrorFunc 2. Recursively check if a feature is supported for cmd and its parents. This fix fixes 28626. Signed-off-by: Yong Tang --- docker.go | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/docker.go b/docker.go index 654c7f41d6..44e6529b82 100644 --- a/docker.go +++ b/docker.go @@ -53,6 +53,23 @@ func newDockerCommand(dockerCli *command.DockerCli) *cobra.Command { }, } cli.SetupRootCommand(cmd) + // When invoking `docker stack --nonsense`, we need to make sure FlagErrorFunc return appropriate + // output if the feature is not supported. + // As above cli.SetupRootCommand(cmd) have already setup the FlagErrorFunc, we will add a pre-check before the FlagErrorFunc + // is called. + flagErrorFunc := cmd.FlagErrorFunc() + cmd.SetFlagErrorFunc(func(cmd *cobra.Command, err error) error { + if dockerCli.Client() == nil { // when using --help, PersistenPreRun is not called, so initialization is needed. + // flags must be the top-level command flags, not cmd.Flags() + opts.Common.SetDefaultOptions(flags) + dockerPreRun(opts) + dockerCli.Initialize(opts) + } + if err := isSupported(cmd, dockerCli.Client().ClientVersion(), dockerCli.HasExperimental()); err != nil { + return err + } + return flagErrorFunc(cmd, err) + }) cmd.SetHelpFunc(func(ccmd *cobra.Command, args []string) { if dockerCli.Client() == nil { // when using --help, PersistentPreRun is not called, so initialization is needed. @@ -167,9 +184,12 @@ func hideUnsupportedFeatures(cmd *cobra.Command, clientVersion string, hasExperi } func isSupported(cmd *cobra.Command, clientVersion string, hasExperimental bool) error { + // We check recursively so that, e.g., `docker stack ls` will return the same output as `docker stack` if !hasExperimental { - if _, ok := cmd.Tags["experimental"]; ok { - return errors.New("only supported on a Docker daemon with experimental features enabled") + for curr := cmd; curr != nil; curr = curr.Parent() { + if _, ok := curr.Tags["experimental"]; ok { + return errors.New("only supported on a Docker daemon with experimental features enabled") + } } }