Browse Source

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 <yong.tang.github@outlook.com>
Yong Tang 8 years ago
parent
commit
9f6fea8e7b
1 changed files with 22 additions and 2 deletions
  1. 22 2
      cmd/docker/docker.go

+ 22 - 2
cmd/docker/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")
+			}
 		}
 	}