From 1df6b68111cff2ba69d9b132b99c69d0b6514772 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 30 May 2018 23:14:59 +0200 Subject: [PATCH 1/2] Mark checkpoint feature as Linux-only, and homogenize error messages This patch adds annotations to mark the checkpoint commands as Linux only, which hides them if the daemon is running a non-matching operating-system type; Before: docker Usage: docker COMMAND A self-sufficient runtime for containers ... Management Commands: config Manage Docker configs container Manage containers image Manage images After: docker Usage: docker COMMAND A self-sufficient runtime for containers ... Management Commands: checkpoint Manage checkpoints config Manage Docker configs container Manage containers image Manage images This change also prints errors when attempting to use checkpoint commands or flags if the feature is not supported by the Daemon's operating system; $ docker checkpoint --help docker checkpoint is only supported on a Docker daemon running on linux, but the Docker daemon is running on windows $ docker checkpoint create --help docker checkpoint create is only supported on a Docker daemon running on linux, but the Docker daemon is running on windows $ docker checkpoint ls --help docker checkpoint ls is only supported on a Docker daemon running on linux, but the Docker daemon is running on windows $ docker checkpoint rm --help docker checkpoint rm is only supported on a Docker daemon running on linux, but the Docker daemon is running on windows $ docker container start --checkpoint=foo mycontainer "--checkpoint" requires the Docker daemon to run on linux, but the Docker daemon is running on windows $ docker container start --checkpoint-dir=/foo/bar mycontainer "--checkpoint-dir" requires the Docker daemon to run on linux, but the Docker daemon is running on windows Signed-off-by: Sebastiaan van Stijn --- cli/command/checkpoint/cmd.go | 14 +++++++++----- cli/command/container/start.go | 2 ++ cmd/docker/docker.go | 13 ++++++++++--- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/cli/command/checkpoint/cmd.go b/cli/command/checkpoint/cmd.go index 097d4dbf90..2a698e74e5 100644 --- a/cli/command/checkpoint/cmd.go +++ b/cli/command/checkpoint/cmd.go @@ -9,11 +9,15 @@ import ( // NewCheckpointCommand returns the `checkpoint` subcommand (only in experimental) func NewCheckpointCommand(dockerCli command.Cli) *cobra.Command { cmd := &cobra.Command{ - Use: "checkpoint", - Short: "Manage checkpoints", - Args: cli.NoArgs, - RunE: command.ShowHelp(dockerCli.Err()), - Annotations: map[string]string{"experimental": "", "version": "1.25"}, + Use: "checkpoint", + Short: "Manage checkpoints", + Args: cli.NoArgs, + RunE: command.ShowHelp(dockerCli.Err()), + Annotations: map[string]string{ + "experimental": "", + "ostype": "linux", + "version": "1.25", + }, } cmd.AddCommand( newCreateCommand(dockerCli), diff --git a/cli/command/container/start.go b/cli/command/container/start.go index 243097c0cd..e388302841 100644 --- a/cli/command/container/start.go +++ b/cli/command/container/start.go @@ -47,8 +47,10 @@ func NewStartCommand(dockerCli command.Cli) *cobra.Command { flags.StringVar(&opts.checkpoint, "checkpoint", "", "Restore from this checkpoint") flags.SetAnnotation("checkpoint", "experimental", nil) + flags.SetAnnotation("checkpoint", "ostype", []string{"linux"}) flags.StringVar(&opts.checkpointDir, "checkpoint-dir", "", "Use a custom checkpoint storage directory") flags.SetAnnotation("checkpoint-dir", "experimental", nil) + flags.SetAnnotation("checkpoint-dir", "ostype", []string{"linux"}) return cmd } diff --git a/cmd/docker/docker.go b/cmd/docker/docker.go index f20530dac5..4fe8bf738e 100644 --- a/cmd/docker/docker.go +++ b/cmd/docker/docker.go @@ -256,6 +256,9 @@ func hideUnsupportedFeatures(cmd *cobra.Command, details versionDetails) { 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 + } } } @@ -282,14 +285,14 @@ func areFlagsSupported(cmd *cobra.Command, details versionDetails) error { return } if !isOSTypeSupported(f, osType) { - errs = append(errs, fmt.Sprintf("\"--%s\" requires the Docker daemon to run on %s, but the Docker daemon is running on %s", f.Name, getFlagAnnotation(f, "ostype"), 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 when experimental cli features are enabled", f.Name)) + errs = append(errs, fmt.Sprintf("\"--%s\" is on a Docker cli with experimental cli features enabled", f.Name)) } _, isKubernetesAnnotated := f.Annotations["kubernetes"] _, isSwarmAnnotated := f.Annotations["swarm"] @@ -310,6 +313,7 @@ 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 hasKubernetes := details.ClientInfo().HasKubernetes() @@ -319,11 +323,14 @@ func areSubcommandsSupported(cmd *cobra.Command, details versionDetails) error { 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 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 _, ok := curr.Annotations["experimental"]; ok && !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 { - return fmt.Errorf("%s is only supported when experimental cli features are enabled", cmd.CommandPath()) + return fmt.Errorf("%s is only supported on a Docker cli with experimental cli features enabled", cmd.CommandPath()) } _, isKubernetesAnnotated := curr.Annotations["kubernetes"] _, isSwarmAnnotated := curr.Annotations["swarm"] From be035a0272fac8416848934ca4be6d8869d91cae Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 30 May 2018 23:17:06 +0200 Subject: [PATCH 2/2] YAML docs: add os_type property on flags and (sub)commands This patch adds an `os_type` property in the generated YAML docs, both for commands, and for flags; Note that the ostype annotation on flags can have multiple values set, however, multiple values are currently not used (and unlikely will). To simplify usage of the os_type property in the YAML, and for consistency with the same property for commands, we're only using the first ostype that's set. ```yaml command: docker checkpoint create short: Create a checkpoint from a running container long: Create a checkpoint from a running container usage: docker checkpoint create [OPTIONS] CONTAINER CHECKPOINT [flags] pname: docker checkpoint plink: docker_checkpoint.yaml options: - option: checkpoint-dir value_type: string description: Use a custom checkpoint storage directory deprecated: false experimental: false experimentalcli: false kubernetes: false swarm: false - option: leave-running value_type: bool default_value: "false" description: Leave the container running after checkpoint deprecated: false experimental: false experimentalcli: false kubernetes: false swarm: false deprecated: false min_api_version: "1.25" experimental: true experimentalcli: false kubernetes: false swarm: false os_type: windows ``` ```yaml command: docker container start short: Start one or more stopped containers long: Start one or more stopped containers usage: docker container start [OPTIONS] CONTAINER [CONTAINER...] [flags] pname: docker container plink: docker_container.yaml options: - option: attach shorthand: a value_type: bool default_value: "false" description: Attach STDOUT/STDERR and forward signals deprecated: false experimental: false experimentalcli: false kubernetes: false swarm: false - option: checkpoint value_type: string description: Restore from this checkpoint deprecated: false experimental: true experimentalcli: false kubernetes: false swarm: false os_type: linux - option: checkpoint-dir value_type: string description: Use a custom checkpoint storage directory deprecated: false experimental: true experimentalcli: false kubernetes: false swarm: false os_type: linux - option: detach-keys value_type: string description: Override the key sequence for detaching a container deprecated: false experimental: false experimentalcli: false kubernetes: false swarm: false - option: interactive shorthand: i value_type: bool default_value: "false" description: Attach container's STDIN deprecated: false experimental: false experimentalcli: false kubernetes: false swarm: false deprecated: false experimental: false experimentalcli: false kubernetes: false swarm: false ``` Signed-off-by: Sebastiaan van Stijn --- docs/yaml/yaml.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/docs/yaml/yaml.go b/docs/yaml/yaml.go index 5dc7164192..82c90973c0 100644 --- a/docs/yaml/yaml.go +++ b/docs/yaml/yaml.go @@ -25,6 +25,7 @@ type cmdOption struct { ExperimentalCLI bool Kubernetes bool Swarm bool + OSType string `yaml:"os_type,omitempty"` } type cmdDoc struct { @@ -48,6 +49,7 @@ type cmdDoc struct { ExperimentalCLI bool Kubernetes bool Swarm bool + OSType string `yaml:"os_type,omitempty"` } // GenYamlTree creates yaml structured ref files @@ -121,6 +123,9 @@ func GenYamlCustom(cmd *cobra.Command, w io.Writer) error { if _, ok := curr.Annotations["swarm"]; ok && !cliDoc.Swarm { cliDoc.Swarm = true } + if os, ok := curr.Annotations["ostype"]; ok && cliDoc.OSType == "" { + cliDoc.OSType = os + } } flags := cmd.NonInheritedFlags() @@ -207,6 +212,15 @@ func genFlagResult(flags *pflag.FlagSet) []cmdOption { opt.Swarm = true } + // Note that the annotation can have multiple ostypes set, however, multiple + // values are currently not used (and unlikely will). + // + // To simplify usage of the os_type property in the YAML, and for consistency + // with the same property for commands, we're only using the first ostype that's set. + if ostypes, ok := flag.Annotations["ostype"]; ok && len(opt.OSType) == 0 && len(ostypes) > 0 { + opt.OSType = ostypes[0] + } + result = append(result, opt) })