WIP: Added context to the error message for 'docker create network' and other similar incorrect patterns (fixes #1986)

[bartolone]

- What I did
Added additional error message context when user types docker create network instead of docker network create (or config, container, plugin, secret, service, or volume) to point user to the correct command. Fixes issue #1986

Also added additional context to the original error message as suggested by cpuguy83 in the discussion of issue #1986 and fixed a comment typo in create.go

- How I did it
Added an additional if statement to the error check in createContainer to note, if a user attempts to create a container named config, container, network, plugin, secret, service, or volume and no container by that name is found, that they may have been attempting to run (for example) 'docker network create' instead of 'docker create network' and points them to the correct command.

Also, as cpuguy83 suggested, I added "Error while creating container" to the beginning of the existing error message to make explicit that the user attempted to create a container (which may not have been their intent).

- How to verify it
Type 'docker create network' (or one of the other keywords listed above) to verify the additional context to the error message.

Note that I am having difficulty testing as per the directions for running a development container, so I am not 100% positive this works as intended. Wanted to make sure I was on the right track, however.

- Description for the changelog
Added context to the error message for 'docker create network' and other similar incorrect patterns.

- A picture of a cute animal (not mandatory but encouraged)

Signed-off-by: Jake Bartolone jakebartolone@gmail.com

[GordonTheTurtle]

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "1986-improved-docker-create-error-messages" git@github.com:bartolone/cli.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842361579784
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

[codecov-io]

Codecov Report

Merging #1987 into master will increase coverage by <.01%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master    #1987      +/-   ##
==========================================
+ Coverage   56.74%   56.74%   +<.01%     
==========================================
  Files         310      310              
  Lines       21802    21807       +5     
==========================================
+ Hits        12371    12374       +3     
- Misses       8517     8518       +1     
- Partials      914      915       +1

[thaJeztah]

Thanks! I left some thoughts inline

[thaJeztah]

I don't think this is the right location to add the Error .... This is inside the opts.pull == PullImageMissing, which means that this line is printed if the image was not found locally, and now the CLI will attempt to pull the image. The error should only be printed if that failed.

The "error" part should likely be added here;

cli/cli/command/container/create.go

Lines 94 to 97 in d88565d

	response, err := createContainer(context.Background(), dockerCli, containerConfig, options)
	if err != nil {
	return err
	}

Something like;

	response, err := createContainer(context.Background(), dockerCli, containerConfig, options)
if err != nil {
	return errors.Wrap(err, "Failed to create container")
}

(I used "Failed to", which is (I think) what we use in other locations)

[thaJeztah]

Instead of parsing namedRef multiple times, it could be stored in a variable, and a switch could be used here:

ref := reference.FamiliarString(namedRef)

// we don't want to write to stdout anything apart from container.ID
fmt.Fprintf(stderr, "Error while creating container: Unable to find image '%s' locally\n", ref)
switch ref {
case "config", "container", "context", "network", "plugin", "secret", "service", "volume":
	fmt.Fprintf(stderr, "If you were trying to create a %[1]s, see 'docker %[1]s create --help'\n", ref)
}

However as mentioned above; this should only be printed if we actually fail, so might need to be stored and then printed elsewhere.

[thaJeztah]

I'm also a bit on the fence on this one; this is a lot of guesswork, and we don't do this in other places, e.g.;

docker rm network foo 
Error: No such container: network
Error: No such container: foo

In addition, docker rm is an alias for docker container rm; I guess the same error message would also be printed when running docker container create container, which will be slightly confusing.

[bartolone]

Thank you @thaJeztah -- your comments make sense to me and I'll work on addressing them. I hadn't caught onto the (much cleaner) switch/case syntax yet as this is my first time using go.

Do you have any feedback on why the Jenkins build might be failing? I haven't been able to figure that out yet and would like to take care of that at the same time as these recodes if possible.