Convert 'docker create' to use cobra and pflag

Return the correct status code on flag parsins errors.

Signed-off-by: Daniel Nephin <dnephin@docker.com>

api/client/build.go

@@ -172,10 +172,10 @@ func (cli *DockerCli) CmdBuild(args ...string) error {
 	ctx := context.Background()
 
 	var resolvedTags []*resolvedTag
-	if isTrusted() {
+	if IsTrusted() {
 		// Wrap the tar archive to replace the Dockerfile entry with the rewritten
 		// Dockerfile which uses trusted pulls.
-		buildCtx = replaceDockerfileTarWrapper(ctx, buildCtx, relDockerfile, cli.trustedReference, &resolvedTags)
+		buildCtx = replaceDockerfileTarWrapper(ctx, buildCtx, relDockerfile, cli.TrustedReference, &resolvedTags)
 	}
 
 	// Setup an upload progress bar
@@ -269,11 +269,11 @@ func (cli *DockerCli) CmdBuild(args ...string) error {
 		fmt.Fprintf(cli.out, "%s", buildBuff)
 	}
 
-	if isTrusted() {
+	if IsTrusted() {
 		// Since the build was successful, now we must tag any of the resolved
 		// images from the above Dockerfile rewrite.
 		for _, resolved := range resolvedTags {
-			if err := cli.tagTrusted(ctx, resolved.digestRef, resolved.tagRef); err != nil {
+			if err := cli.TagTrusted(ctx, resolved.digestRef, resolved.tagRef); err != nil {
 				return err
 			}
 		}
@@ -321,7 +321,7 @@ func rewriteDockerfileFrom(ctx context.Context, dockerfile io.Reader, translator
 				return nil, nil, err
 			}
 			ref = reference.WithDefaultTag(ref)
-			if ref, ok := ref.(reference.NamedTagged); ok && isTrusted() {
+			if ref, ok := ref.(reference.NamedTagged); ok && IsTrusted() {
 				trustedRef, err := translator(ctx, ref)
 				if err != nil {
 					return nil, nil, err

api/client/cli.go

@@ -90,6 +90,11 @@ func (cli *DockerCli) IsTerminalOut() bool {
 	return cli.isTerminalOut
 }
 
+// OutFd returns the fd for the stdout stream
+func (cli *DockerCli) OutFd() uintptr {
+	return cli.outFd
+}
+
 // CheckTtyInput checks if we are trying to attach to a container tty
 // from a non-tty client input stream, and if so, returns an error.
 func (cli *DockerCli) CheckTtyInput(attachStdin, ttyMode bool) error {

api/client/commands.go

@@ -7,7 +7,6 @@ func (cli *DockerCli) Command(name string) func(...string) error {
 		"build":              cli.CmdBuild,
 		"commit":             cli.CmdCommit,
 		"cp":                 cli.CmdCp,
-		"create":             cli.CmdCreate,
 		"diff":               cli.CmdDiff,
 		"events":             cli.CmdEvents,
 		"exec":               cli.CmdExec,

api/client/container/create.go

@@ -0,0 +1,216 @@
+package container
+
+import (
+	"fmt"
+	"io"
+	"os"
+
+	"golang.org/x/net/context"
+
+	"github.com/docker/docker/api/client"
+	"github.com/docker/docker/cli"
+	"github.com/docker/docker/pkg/jsonmessage"
+	// FIXME migrate to docker/distribution/reference
+	"github.com/docker/docker/reference"
+	"github.com/docker/docker/registry"
+	runconfigopts "github.com/docker/docker/runconfig/opts"
+	apiclient "github.com/docker/engine-api/client"
+	"github.com/docker/engine-api/types"
+	"github.com/docker/engine-api/types/container"
+	networktypes "github.com/docker/engine-api/types/network"
+	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
+)
+
+type createOptions struct {
+	name string
+}
+
+// NewCreateCommand creats a new cobra.Command for `docker create`
+func NewCreateCommand(dockerCli *client.DockerCli) *cobra.Command {
+	var opts createOptions
+	var copts *runconfigopts.ContainerOptions
+
+	cmd := &cobra.Command{
+		Use:   "create [OPTIONS] IMAGE [COMMAND] [ARG...]",
+		Short: "Create a new container",
+		Args:  cli.RequiresMinArgs(1),
+		RunE: func(cmd *cobra.Command, args []string) error {
+			copts.Image = args[0]
+			if len(args) > 1 {
+				copts.Args = args[1:]
+			}
+			return runCreate(dockerCli, cmd.Flags(), &opts, copts)
+		},
+	}
+	cmd.SetFlagErrorFunc(flagErrorFunc)
+
+	flags := cmd.Flags()
+	flags.StringVar(&opts.name, "name", "", "Assign a name to the container")
+
+	// Add an explicit help that doesn't have a `-h` to prevent the conflict
+	// with hostname
+	flags.Bool("help", false, "Print usage")
+
+	client.AddTrustedFlags(flags, true)
+	copts = runconfigopts.AddFlags(flags)
+	return cmd
+}
+
+func runCreate(dockerCli *client.DockerCli, flags *pflag.FlagSet, opts *createOptions, copts *runconfigopts.ContainerOptions) error {
+	config, hostConfig, networkingConfig, err := runconfigopts.Parse(flags, copts)
+	if err != nil {
+		reportError(dockerCli.Err(), "create", err.Error(), true)
+		return cli.StatusError{StatusCode: 125}
+	}
+	response, err := createContainer(context.Background(), dockerCli, config, hostConfig, networkingConfig, hostConfig.ContainerIDFile, opts.name)
+	if err != nil {
+		return err
+	}
+	fmt.Fprintf(dockerCli.Out(), "%s\n", response.ID)
+	return nil
+}
+
+func pullImage(ctx context.Context, dockerCli *client.DockerCli, image string, out io.Writer) error {
+	ref, err := reference.ParseNamed(image)
+	if err != nil {
+		return err
+	}
+
+	// Resolve the Repository name from fqn to RepositoryInfo
+	repoInfo, err := registry.ParseRepositoryInfo(ref)
+	if err != nil {
+		return err
+	}
+
+	authConfig := dockerCli.ResolveAuthConfig(ctx, repoInfo.Index)
+	encodedAuth, err := client.EncodeAuthToBase64(authConfig)
+	if err != nil {
+		return err
+	}
+
+	options := types.ImageCreateOptions{
+		RegistryAuth: encodedAuth,
+	}
+
+	responseBody, err := dockerCli.Client().ImageCreate(ctx, image, options)
+	if err != nil {
+		return err
+	}
+	defer responseBody.Close()
+
+	return jsonmessage.DisplayJSONMessagesStream(
+		responseBody,
+		out,
+		dockerCli.OutFd(),
+		dockerCli.IsTerminalOut(),
+		nil)
+}
+
+type cidFile struct {
+	path    string
+	file    *os.File
+	written bool
+}
+
+func (cid *cidFile) Close() error {
+	cid.file.Close()
+
+	if !cid.written {
+		if err := os.Remove(cid.path); err != nil {
+			return fmt.Errorf("failed to remove the CID file '%s': %s \n", cid.path, err)
+		}
+	}
+
+	return nil
+}
+
+func (cid *cidFile) Write(id string) error {
+	if _, err := cid.file.Write([]byte(id)); err != nil {
+		return fmt.Errorf("Failed to write the container ID to the file: %s", err)
+	}
+	cid.written = true
+	return nil
+}
+
+func newCIDFile(path string) (*cidFile, error) {
+	if _, err := os.Stat(path); err == nil {
+		return nil, fmt.Errorf("Container ID file found, make sure the other container isn't running or delete %s", path)
+	}
+
+	f, err := os.Create(path)
+	if err != nil {
+		return nil, fmt.Errorf("Failed to create the container ID file: %s", err)
+	}
+
+	return &cidFile{path: path, file: f}, nil
+}
+
+func createContainer(ctx context.Context, dockerCli *client.DockerCli, config *container.Config, hostConfig *container.HostConfig, networkingConfig *networktypes.NetworkingConfig, cidfile, name string) (*types.ContainerCreateResponse, error) {
+	stderr := dockerCli.Err()
+
+	var containerIDFile *cidFile
+	if cidfile != "" {
+		var err error
+		if containerIDFile, err = newCIDFile(cidfile); err != nil {
+			return nil, err
+		}
+		defer containerIDFile.Close()
+	}
+
+	var trustedRef reference.Canonical
+	_, ref, err := reference.ParseIDOrReference(config.Image)
+	if err != nil {
+		return nil, err
+	}
+	if ref != nil {
+		ref = reference.WithDefaultTag(ref)
+
+		if ref, ok := ref.(reference.NamedTagged); ok && client.IsTrusted() {
+			var err error
+			trustedRef, err = dockerCli.TrustedReference(ctx, ref)
+			if err != nil {
+				return nil, err
+			}
+			config.Image = trustedRef.String()
+		}
+	}
+
+	//create the container
+	response, err := dockerCli.Client().ContainerCreate(ctx, config, hostConfig, networkingConfig, name)
+
+	//if image not found try to pull it
+	if err != nil {
+		if apiclient.IsErrImageNotFound(err) && ref != nil {
+			fmt.Fprintf(stderr, "Unable to find image '%s' locally\n", ref.String())
+
+			// we don't want to write to stdout anything apart from container.ID
+			if err = pullImage(ctx, dockerCli, config.Image, stderr); err != nil {
+				return nil, err
+			}
+			if ref, ok := ref.(reference.NamedTagged); ok && trustedRef != nil {
+				if err := dockerCli.TagTrusted(ctx, trustedRef, ref); err != nil {
+					return nil, err
+				}
+			}
+			// Retry
+			var retryErr error
+			response, retryErr = dockerCli.Client().ContainerCreate(ctx, config, hostConfig, networkingConfig, name)
+			if retryErr != nil {
+				return nil, retryErr
+			}
+		} else {
+			return nil, err
+		}
+	}
+
+	for _, warning := range response.Warnings {
+		fmt.Fprintf(stderr, "WARNING: %s\n", warning)
+	}
+	if containerIDFile != nil {
+		if err = containerIDFile.Write(response.ID); err != nil {
+			return nil, err
+		}
+	}
+	return &response, nil
+}

api/client/container/run.go

@@ -53,6 +53,7 @@ func NewRunCommand(dockerCli *client.DockerCli) *cobra.Command {
 			return runRun(dockerCli, cmd.Flags(), &opts, copts)
 		},
 	}
+	cmd.SetFlagErrorFunc(flagErrorFunc)
 
 	flags := cmd.Flags()
 	flags.SetInterspersed(false)
@@ -73,6 +74,13 @@ func NewRunCommand(dockerCli *client.DockerCli) *cobra.Command {
 	return cmd
 }
 
+func flagErrorFunc(cmd *cobra.Command, err error) error {
+	return cli.StatusError{
+		Status:     cli.FlagErrorFunc(cmd, err).Error(),
+		StatusCode: 125,
+	}
+}
+
 func runRun(dockerCli *client.DockerCli, flags *pflag.FlagSet, opts *runOptions, copts *runconfigopts.ContainerOptions) error {
 	stdout, stderr, stdin := dockerCli.Out(), dockerCli.Err(), dockerCli.In()
 	client := dockerCli.Client()
@@ -91,7 +99,7 @@ func runRun(dockerCli *client.DockerCli, flags *pflag.FlagSet, opts *runOptions,
 	// just in case the Parse does not exit
 	if err != nil {
 		reportError(stderr, cmdPath, err.Error(), true)
-		os.Exit(125)
+		return cli.StatusError{StatusCode: 125}
 	}
 
 	if hostConfig.OomKillDisable != nil && *hostConfig.OomKillDisable && hostConfig.Memory == 0 {
@@ -147,7 +155,7 @@ func runRun(dockerCli *client.DockerCli, flags *pflag.FlagSet, opts *runOptions,
 
 	ctx, cancelFun := context.WithCancel(context.Background())
 
-	createResponse, err := dockerCli.CreateContainer(ctx, config, hostConfig, networkingConfig, hostConfig.ContainerIDFile, opts.name)
+	createResponse, err := createContainer(ctx, dockerCli, config, hostConfig, networkingConfig, hostConfig.ContainerIDFile, opts.name)
 	if err != nil {
 		reportError(stderr, cmdPath, err.Error(), true)
 		return runStartContainerErr(err)

api/client/create.go

@@ -1,191 +0,0 @@
-package client
-
-import (
-	"fmt"
-	"io"
-	"os"
-
-	"golang.org/x/net/context"
-
-	Cli "github.com/docker/docker/cli"
-	"github.com/docker/docker/pkg/jsonmessage"
-	// FIXME migrate to docker/distribution/reference
-	"github.com/docker/docker/reference"
-	"github.com/docker/docker/registry"
-	//runconfigopts "github.com/docker/docker/runconfig/opts"
-	"github.com/docker/engine-api/client"
-	"github.com/docker/engine-api/types"
-	"github.com/docker/engine-api/types/container"
-	networktypes "github.com/docker/engine-api/types/network"
-)
-
-func (cli *DockerCli) pullImage(ctx context.Context, image string, out io.Writer) error {
-	ref, err := reference.ParseNamed(image)
-	if err != nil {
-		return err
-	}
-
-	// Resolve the Repository name from fqn to RepositoryInfo
-	repoInfo, err := registry.ParseRepositoryInfo(ref)
-	if err != nil {
-		return err
-	}
-
-	authConfig := cli.ResolveAuthConfig(ctx, repoInfo.Index)
-	encodedAuth, err := EncodeAuthToBase64(authConfig)
-	if err != nil {
-		return err
-	}
-
-	options := types.ImageCreateOptions{
-		RegistryAuth: encodedAuth,
-	}
-
-	responseBody, err := cli.client.ImageCreate(ctx, image, options)
-	if err != nil {
-		return err
-	}
-	defer responseBody.Close()
-
-	return jsonmessage.DisplayJSONMessagesStream(responseBody, out, cli.outFd, cli.isTerminalOut, nil)
-}
-
-type cidFile struct {
-	path    string
-	file    *os.File
-	written bool
-}
-
-func (cid *cidFile) Close() error {
-	cid.file.Close()
-
-	if !cid.written {
-		if err := os.Remove(cid.path); err != nil {
-			return fmt.Errorf("failed to remove the CID file '%s': %s \n", cid.path, err)
-		}
-	}
-
-	return nil
-}
-
-func (cid *cidFile) Write(id string) error {
-	if _, err := cid.file.Write([]byte(id)); err != nil {
-		return fmt.Errorf("Failed to write the container ID to the file: %s", err)
-	}
-	cid.written = true
-	return nil
-}
-
-func newCIDFile(path string) (*cidFile, error) {
-	if _, err := os.Stat(path); err == nil {
-		return nil, fmt.Errorf("Container ID file found, make sure the other container isn't running or delete %s", path)
-	}
-
-	f, err := os.Create(path)
-	if err != nil {
-		return nil, fmt.Errorf("Failed to create the container ID file: %s", err)
-	}
-
-	return &cidFile{path: path, file: f}, nil
-}
-
-// CreateContainer creates a container from a config
-// TODO: this can be unexported again once all container commands are under
-// api/client/container
-func (cli *DockerCli) CreateContainer(ctx context.Context, config *container.Config, hostConfig *container.HostConfig, networkingConfig *networktypes.NetworkingConfig, cidfile, name string) (*types.ContainerCreateResponse, error) {
-	var containerIDFile *cidFile
-	if cidfile != "" {
-		var err error
-		if containerIDFile, err = newCIDFile(cidfile); err != nil {
-			return nil, err
-		}
-		defer containerIDFile.Close()
-	}
-
-	var trustedRef reference.Canonical
-	_, ref, err := reference.ParseIDOrReference(config.Image)
-	if err != nil {
-		return nil, err
-	}
-	if ref != nil {
-		ref = reference.WithDefaultTag(ref)
-
-		if ref, ok := ref.(reference.NamedTagged); ok && isTrusted() {
-			var err error
-			trustedRef, err = cli.trustedReference(ctx, ref)
-			if err != nil {
-				return nil, err
-			}
-			config.Image = trustedRef.String()
-		}
-	}
-
-	//create the container
-	response, err := cli.client.ContainerCreate(ctx, config, hostConfig, networkingConfig, name)
-
-	//if image not found try to pull it
-	if err != nil {
-		if client.IsErrImageNotFound(err) && ref != nil {
-			fmt.Fprintf(cli.err, "Unable to find image '%s' locally\n", ref.String())
-
-			// we don't want to write to stdout anything apart from container.ID
-			if err = cli.pullImage(ctx, config.Image, cli.err); err != nil {
-				return nil, err
-			}
-			if ref, ok := ref.(reference.NamedTagged); ok && trustedRef != nil {
-				if err := cli.tagTrusted(ctx, trustedRef, ref); err != nil {
-					return nil, err
-				}
-			}
-			// Retry
-			var retryErr error
-			response, retryErr = cli.client.ContainerCreate(ctx, config, hostConfig, networkingConfig, name)
-			if retryErr != nil {
-				return nil, retryErr
-			}
-		} else {
-			return nil, err
-		}
-	}
-
-	for _, warning := range response.Warnings {
-		fmt.Fprintf(cli.err, "WARNING: %s\n", warning)
-	}
-	if containerIDFile != nil {
-		if err = containerIDFile.Write(response.ID); err != nil {
-			return nil, err
-		}
-	}
-	return &response, nil
-}
-
-// CmdCreate creates a new container from a given image.
-//
-// Usage: docker create [OPTIONS] IMAGE [COMMAND] [ARG...]
-func (cli *DockerCli) CmdCreate(args ...string) error {
-	cmd := Cli.Subcmd("create", []string{"IMAGE [COMMAND] [ARG...]"}, Cli.DockerCommands["create"].Description, true)
-	addTrustedFlags(cmd, true)
-
-	// TODO: tmp disable for PoC, convert to cobra and pflag later
-	// These are flags not stored in Config/HostConfig
-	//	var (
-	//		flName = cmd.String([]string{"-name"}, "", "Assign a name to the container")
-	//	)
-
-	//	config, hostConfig, networkingConfig, cmd, err := runconfigopts.Parse(cmd, args)
-	//
-	//	if err != nil {
-	//		cmd.ReportError(err.Error(), true)
-	//		os.Exit(1)
-	//	}
-	//	if config.Image == "" {
-	//		cmd.Usage()
-	//		return nil
-	//	}
-	//	response, err := cli.CreateContainer(context.Background(), config, hostConfig, networkingConfig, hostConfig.ContainerIDFile, *flName)
-	//	if err != nil {
-	//		return err
-	//	}
-	//	fmt.Fprintf(cli.out, "%s\n", response.ID)
-	return nil
-}

api/client/hijack.go

@@ -11,7 +11,8 @@ import (
 	"github.com/docker/engine-api/types"
 )
 
-// HoldHijackedConnection ... TODO docstring
+// HoldHijackedConnection handles copying input to and output from streams to the
+// connection
 func (cli *DockerCli) HoldHijackedConnection(ctx context.Context, tty bool, inputStream io.ReadCloser, outputStream, errorStream io.Writer, resp types.HijackedResponse) error {
 	var (
 		err         error

api/client/pull.go

@@ -60,7 +60,7 @@ func (cli *DockerCli) CmdPull(args ...string) error {
 	authConfig := cli.ResolveAuthConfig(ctx, repoInfo.Index)
 	requestPrivilege := cli.RegistryAuthenticationPrivilegedFunc(repoInfo.Index, "pull")
 
-	if isTrusted() && !registryRef.HasDigest() {
+	if IsTrusted() && !registryRef.HasDigest() {
 		// Check if tag is digest
 		return cli.trustedPull(ctx, repoInfo, registryRef, authConfig, requestPrivilege)
 	}

api/client/push.go

@@ -40,7 +40,7 @@ func (cli *DockerCli) CmdPush(args ...string) error {
 	authConfig := cli.ResolveAuthConfig(ctx, repoInfo.Index)
 	requestPrivilege := cli.RegistryAuthenticationPrivilegedFunc(repoInfo.Index, "push")
 
-	if isTrusted() {
+	if IsTrusted() {
 		return cli.trustedPush(ctx, repoInfo, ref, authConfig, requestPrivilege)
 	}
 

api/client/trust.go

@@ -45,8 +45,7 @@ var (
 	untrusted    bool
 )
 
-// TODO: tmp workaround to get this PoC working, change everything to use
+// addTrustedFlags is the mflag version of AddTrustedFlags
-// exported version
 func addTrustedFlags(fs *flag.FlagSet, verify bool) {
 	trusted, message := setupTrustedFlag(verify)
 	fs.BoolVar(&untrusted, []string{"-disable-content-trust"}, !trusted, message)
@@ -73,7 +72,8 @@ func setupTrustedFlag(verify bool) (bool, string) {
 	return trusted, message
 }
 
-func isTrusted() bool {
+// IsTrusted returns true if content trust is enabled
+func IsTrusted() bool {
 	return !untrusted
 }
 
@@ -243,7 +243,8 @@ func (cli *DockerCli) getPassphraseRetriever() passphrase.Retriever {
 	}
 }
 
-func (cli *DockerCli) trustedReference(ctx context.Context, ref reference.NamedTagged) (reference.Canonical, error) {
+// TrustedReference returns the canonical trusted reference for an image reference
+func (cli *DockerCli) TrustedReference(ctx context.Context, ref reference.NamedTagged) (reference.Canonical, error) {
 	repoInfo, err := registry.ParseRepositoryInfo(ref)
 	if err != nil {
 		return nil, err
@@ -276,7 +277,8 @@ func (cli *DockerCli) trustedReference(ctx context.Context, ref reference.NamedT
 	return reference.WithDigest(ref, r.digest)
 }
 
-func (cli *DockerCli) tagTrusted(ctx context.Context, trustedRef reference.Canonical, ref reference.NamedTagged) error {
+// TagTrusted tags a trusted ref
+func (cli *DockerCli) TagTrusted(ctx context.Context, trustedRef reference.Canonical, ref reference.NamedTagged) error {
 	fmt.Fprintf(cli.out, "Tagging %s as %s\n", trustedRef.String(), ref.String())
 
 	return cli.client.ImageTag(ctx, trustedRef.String(), ref.String())
@@ -388,7 +390,7 @@ func (cli *DockerCli) trustedPull(ctx context.Context, repoInfo *registry.Reposi
 			if err != nil {
 				return err
 			}
-			if err := cli.tagTrusted(ctx, trustedRef, tagged); err != nil {
+			if err := cli.TagTrusted(ctx, trustedRef, tagged); err != nil {
 				return err
 			}
 		}

cli/cobraadaptor/adaptor.go

@@ -1,8 +1,6 @@
 package cobraadaptor
 
 import (
-	"fmt"
-
 	"github.com/docker/docker/api/client"
 	"github.com/docker/docker/api/client/container"
 	"github.com/docker/docker/api/client/image"
@@ -32,9 +30,10 @@ func NewCobraAdaptor(clientFlags *cliflags.ClientFlags) CobraAdaptor {
 	}
 	rootCmd.SetUsageTemplate(usageTemplate)
 	rootCmd.SetHelpTemplate(helpTemplate)
-	rootCmd.SetFlagErrorFunc(flagErrorFunc)
+	rootCmd.SetFlagErrorFunc(cli.FlagErrorFunc)
 	rootCmd.SetOutput(stdout)
 	rootCmd.AddCommand(
+		container.NewCreateCommand(dockerCli),
 		container.NewRunCommand(dockerCli),
 		image.NewSearchCommand(dockerCli),
 		volume.NewVolumeCommand(dockerCli),
@@ -78,20 +77,6 @@ func (c CobraAdaptor) Command(name string) func(...string) error {
 	return nil
 }
 
-// flagErrorFunc prints an error messages which matches the format of the
-// docker/docker/cli error messages
-func flagErrorFunc(cmd *cobra.Command, err error) error {
-	if err == nil {
-		return err
-	}
-
-	usage := ""
-	if cmd.HasSubCommands() {
-		usage = "\n\n" + cmd.UsageString()
-	}
-	return fmt.Errorf("%s\nSee '%s --help'.%s", err, cmd.CommandPath(), usage)
-}
-
 var usageTemplate = `Usage:	{{if not .HasSubCommands}}{{if .HasLocalFlags}}{{appendIfNotPresent .UseLine "[OPTIONS]"}}{{else}}{{.UseLine}}{{end}}{{end}}{{if .HasSubCommands}}{{ .CommandPath}} COMMAND{{end}}
 
 {{with or .Long .Short }}{{. | trim}}{{end}}{{if gt .Aliases 0}}

cli/flagerrors.go

@@ -0,0 +1,21 @@
+package cli
+
+import (
+	"fmt"
+
+	"github.com/spf13/cobra"
+)
+
+// FlagErrorFunc prints an error messages which matches the format of the
+// docker/docker/cli error messages
+func FlagErrorFunc(cmd *cobra.Command, err error) error {
+	if err == nil {
+		return err
+	}
+
+	usage := ""
+	if cmd.HasSubCommands() {
+		usage = "\n\n" + cmd.UsageString()
+	}
+	return fmt.Errorf("%s\nSee '%s --help'.%s", err, cmd.CommandPath(), usage)
+}

cli/usage.go

@@ -12,7 +12,6 @@ var DockerCommandUsage = []Command{
 	{"build", "Build an image from a Dockerfile"},
 	{"commit", "Create a new image from a container's changes"},
 	{"cp", "Copy files/folders between a container and the local filesystem"},
-	{"create", "Create a new container"},
 	{"diff", "Inspect changes on a container's filesystem"},
 	{"events", "Get real time events from the server"},
 	{"exec", "Run a command in a running container"},

cmd/docker/docker.go

@@ -73,6 +73,10 @@ func main() {
 		if sterr, ok := err.(cli.StatusError); ok {
 			if sterr.Status != "" {
 				fmt.Fprintln(stderr, sterr.Status)
+			}
+			// StatusError should only be used for errors, and all errors should
+			// have a non-zero exit status, so never exit with 0
+			if sterr.StatusCode == 0 {
 				os.Exit(1)
 			}
 			os.Exit(sterr.StatusCode)

integration-cli/docker_cli_run_test.go

@@ -1455,7 +1455,7 @@ func (s *DockerSuite) TestRunResolvconfUpdate(c *check.C) {
 	}()
 
 	//1. test that a restarting container gets an updated resolv.conf
-	dockerCmd(c, "run", "--name='first'", "busybox", "true")
+	dockerCmd(c, "run", "--name=first", "busybox", "true")
 	containerID1, err := getIDByName("first")
 	if err != nil {
 		c.Fatal(err)
@@ -1485,7 +1485,7 @@ func (s *DockerSuite) TestRunResolvconfUpdate(c *check.C) {
 								} */
 	//2. test that a restarting container does not receive resolv.conf updates
 	//   if it modified the container copy of the starting point resolv.conf
-	dockerCmd(c, "run", "--name='second'", "busybox", "sh", "-c", "echo 'search mylittlepony.com' >>/etc/resolv.conf")
+	dockerCmd(c, "run", "--name=second", "busybox", "sh", "-c", "echo 'search mylittlepony.com' >>/etc/resolv.conf")
 	containerID2, err := getIDByName("second")
 	if err != nil {
 		c.Fatal(err)
@@ -1574,7 +1574,7 @@ func (s *DockerSuite) TestRunResolvconfUpdate(c *check.C) {
 	}
 
 	// Run the container so it picks up the old settings
-	dockerCmd(c, "run", "--name='third'", "busybox", "true")
+	dockerCmd(c, "run", "--name=third", "busybox", "true")
 	containerID3, err := getIDByName("third")
 	if err != nil {
 		c.Fatal(err)

integration-cli/docker_cli_run_unix_test.go

@@ -894,7 +894,7 @@ func (s *DockerSuite) TestRunSysctls(c *check.C) {
 
 	runCmd := exec.Command(dockerBinary, "run", "--sysctl", "kernel.foobar=1", "--name", "test2", "busybox", "cat", "/proc/sys/kernel/foobar")
 	out, _, _ = runCommandWithOutput(runCmd)
-	if !strings.Contains(out, "invalid value") {
+	if !strings.Contains(out, "invalid argument") {
 		c.Fatalf("expected --sysctl to fail, got %s", out)
 	}
 }

runconfig/opts/parse.go

@@ -184,11 +184,11 @@ func AddFlags(flags *pflag.FlagSet) *ContainerOptions {
 		flStopSignal:        flags.String("stop-signal", signal.DefaultStopSignal, fmt.Sprintf("Signal to stop a container, %v by default", signal.DefaultStopSignal)),
 		flIsolation:         flags.String("isolation", "", "Container isolation technology"),
 		flShmSize:           flags.String("shm-size", "", "Size of /dev/shm, default value is 64MB"),
-		flNoHealthcheck:     cmd.Bool([]string{"-no-healthcheck"}, false, "Disable any container-specified HEALTHCHECK"),
+		flNoHealthcheck:     flags.Bool("no-healthcheck", false, "Disable any container-specified HEALTHCHECK"),
-		flHealthCmd:         cmd.String([]string{"-health-cmd"}, "", "Command to run to check health"),
+		flHealthCmd:         flags.String("health-cmd", "", "Command to run to check health"),
-		flHealthInterval:    cmd.Duration([]string{"-health-interval"}, 0, "Time between running the check"),
+		flHealthInterval:    flags.Duration("health-interval", 0, "Time between running the check"),
-		flHealthTimeout:     cmd.Duration([]string{"-health-timeout"}, 0, "Maximum time to allow one check to run"),
+		flHealthTimeout:     flags.Duration("health-timeout", 0, "Maximum time to allow one check to run"),
-		flHealthRetries:     cmd.Int([]string{"-health-retries"}, 0, "Consecutive failures needed to report unhealthy"),
+		flHealthRetries:     flags.Int("health-retries", 0, "Consecutive failures needed to report unhealthy"),
 	}
 
 	flags.VarP(&copts.flAttach, "attach", "a", "Attach to STDIN, STDOUT or STDERR")
@@ -442,34 +442,34 @@ func Parse(flags *pflag.FlagSet, copts *ContainerOptions) (*container.Config, *c
 
 	// Healthcheck
 	var healthConfig *container.HealthConfig
-	haveHealthSettings := *flHealthCmd != "" ||
+	haveHealthSettings := *copts.flHealthCmd != "" ||
-		*flHealthInterval != 0 ||
+		*copts.flHealthInterval != 0 ||
-		*flHealthTimeout != 0 ||
+		*copts.flHealthTimeout != 0 ||
-		*flHealthRetries != 0
+		*copts.flHealthRetries != 0
-	if *flNoHealthcheck {
+	if *copts.flNoHealthcheck {
 		if haveHealthSettings {
-			return nil, nil, nil, cmd, fmt.Errorf("--no-healthcheck conflicts with --health-* options")
+			return nil, nil, nil, fmt.Errorf("--no-healthcheck conflicts with --health-* options")
 		}
 		test := strslice.StrSlice{"NONE"}
 		healthConfig = &container.HealthConfig{Test: test}
 	} else if haveHealthSettings {
 		var probe strslice.StrSlice
-		if *flHealthCmd != "" {
+		if *copts.flHealthCmd != "" {
-			args := []string{"CMD-SHELL", *flHealthCmd}
+			args := []string{"CMD-SHELL", *copts.flHealthCmd}
 			probe = strslice.StrSlice(args)
 		}
-		if *flHealthInterval < 0 {
+		if *copts.flHealthInterval < 0 {
-			return nil, nil, nil, cmd, fmt.Errorf("--health-interval cannot be negative")
+			return nil, nil, nil, fmt.Errorf("--health-interval cannot be negative")
 		}
-		if *flHealthTimeout < 0 {
+		if *copts.flHealthTimeout < 0 {
-			return nil, nil, nil, cmd, fmt.Errorf("--health-timeout cannot be negative")
+			return nil, nil, nil, fmt.Errorf("--health-timeout cannot be negative")
 		}
 
 		healthConfig = &container.HealthConfig{
 			Test:     probe,
-			Interval: *flHealthInterval,
+			Interval: *copts.flHealthInterval,
-			Timeout:  *flHealthTimeout,
+			Timeout:  *copts.flHealthTimeout,
-			Retries:  *flHealthRetries,
+			Retries:  *copts.flHealthRetries,
 		}
 	}
 

runconfig/opts/parse_test.go

@@ -18,7 +18,6 @@ import (
 	"github.com/spf13/pflag"
 )
 
-// TODO: drop FlagSet from return value
 func parseRun(args []string) (*container.Config, *container.HostConfig, *networktypes.NetworkingConfig, error) {
 	flags := pflag.NewFlagSet("run", pflag.ContinueOnError)
 	flags.SetOutput(ioutil.Discard)
@@ -592,14 +591,14 @@ func TestParseRestartPolicy(t *testing.T) {
 
 func TestParseHealth(t *testing.T) {
 	checkOk := func(args ...string) *container.HealthConfig {
-		config, _, _, _, err := parseRun(args)
+		config, _, _, err := parseRun(args)
 		if err != nil {
 			t.Fatalf("%#v: %v", args, err)
 		}
 		return config.Healthcheck
 	}
 	checkError := func(expected string, args ...string) {
-		config, _, _, _, err := parseRun(args)
+		config, _, _, err := parseRun(args)
 		if err == nil {
 			t.Fatalf("Expected error, but got %#v", config)
 		}