Docker create (rebase of 6907) #7110

tiborvass · 2014-07-18T22:56:11Z

Closes #6907

Ping @vieux @unclejack @crosbymichael

mrunalp · 2014-07-18T23:28:23Z

@tiborvass Thanks!

tiborvass · 2014-07-18T23:29:38Z

@mrunalp Actually, thanks go to you! :)

jamtur01 · 2014-07-19T20:30:06Z

docs/sources/reference/commandline/cli.md

+    -w, --workdir=""           Working directory inside the container
+
+
+The `docker create` command `creates` a writeable container layer over


Why is creates in backticks?

mrunalp · 2014-07-24T17:05:36Z

ping :) Any updates?

tiborvass · 2014-07-26T19:06:32Z

Rebased. Ping @vieux @unclejack @crosbymichael

ndwns · 2014-07-28T22:01:27Z

integration-cli/docker_cli_create_test.go

+
+	c := containers[0]
+	if c.Path != "command" {
+		t.Fatalf("Unepexted container path. Expected command, recieved: %s", c.Path)


s/Unepexted/Unexpected/

crosbymichael · 2014-07-30T01:15:05Z

This needs rebased after we moved the repo and changed the imports.

crosbymichael · 2014-07-30T01:33:09Z

integration-cli/docker_cli_create_test.go

+	"os/exec"
+	"testing"
+
+	"github.com/dotcloud/docker/daemon"


these two imports should be docker/docker

Actually don't import these in the integration tests, you can make an adhoc type for unmarshaling the data you want.

SvenDowideit · 2014-07-30T04:10:57Z

Docs LGTM - @jamtur01 @fredlf @ostezer

jamtur01 · 2014-07-30T04:12:29Z

docs/sources/reference/commandline/cli.md

+    Usage: docker create [OPTIONS] IMAGE[:TAG] [COMMAND] [ARG...]
+
+
+    -a, --attach=[]            Attach to stdin, stdout or stderr.


tiborvass · 2014-07-30T15:10:09Z

Fixed docs.

ghost · 2014-07-30T15:40:52Z

api/client/commands.go

-			return fmt.Errorf("Failed to create the container ID file: %s", err)
+	if !cid.written {
+		if err := os.Remove(cid.path); err != nil {
+			return fmt.Errorf("failed to remove CID file '%s': %s \n", cid.path, err)


I think this should read: ".. the CID file ..", instead of ".. remove CID file .."

tiborvass · 2014-07-30T17:01:04Z

Fixed docs again. Ping @jamtur01 @ostezer

crosbymichael · 2014-07-30T17:51:24Z

runconfig/hostconfig.go

@@ -42,7 +42,27 @@ type HostConfig struct {
 	CapDrop         []string
 }

+// This is used by the create command when you want to both set the
+// Config and the HostConfig in the same call
+type ConfigAndHostConfig struct {


This request is backwards compatible right?

@crosbymichael I'm not sure what you mean by that, but basically, "create" will set the hostconfig in createContainer, but "run" will set it in srv.ContainerStart. In that regard, it (should) work just like before.
We basically create a HostConfig field in Config, that we check for to know if we have to set the hostconfig early on.

tiborvass · 2014-07-31T15:06:14Z

Rebased.

ghost · 2014-07-31T18:20:29Z

docs/sources/reference/commandline/cli.md

+The `docker create` command creates a writeable container layer over
+the specified image, and prepares it for running the specified
+command. The container ID is then printed to `STDOUT`. This is similar
+to what `docker run -d <cli_run>`, does except the container is never


The comma here seems redundant and I believe it should be removed.

ghost · 2014-07-31T18:25:45Z

@alexlarsson @tiborvass: docs mostly LGTM apart from the ID issue.

One final note:

<cli_start> and <cli_run> expressions do not really seem clear to me.
I'm not sure if docs here, as is, would make a lot of sense to everyone.

Would it be possible to amend this bit further more?

tiborvass · 2014-07-31T18:52:05Z

@ostezer Is it better?

ghost · 2014-07-31T18:55:09Z

@tiborvass wonderful! Thank you.

tiborvass · 2014-09-11T18:14:05Z

@crosbymichael thanks, will update. Sorry about that

tiborvass · 2014-09-11T18:25:37Z

@crosbymichael PTAL

tiborvass · 2014-09-15T16:20:39Z

Ping @crosbymichael

crosbymichael · 2014-09-15T17:48:44Z

still needs rebased ;)

tiborvass · 2014-09-15T22:04:00Z

@crosbymichael rebased. PTAL

jessfraz · 2014-09-16T22:10:04Z

sorry it needs to be rebased again because of this:

--- FAIL: TestTagInvalidPrefixedRepo (0.08 seconds)
    docker_cli_tag_test.go:64: tag busybox repo:f should have failed

tiborvass · 2014-09-16T22:24:18Z

Rebased

jessfraz · 2014-09-16T22:34:23Z

api/client/commands.go

+
+func (cli *DockerCli) CmdRun(args ...string) error {
+	// FIXME: just use runconfig.Parse already
+	cmd := cli.Subcmd("run", "[OPTIONS] IMAGE [COMMAND] [ARG...]", "Run a command in a new container")


same problem we had with exec [OPTIONS] gets printed twice in usage, but also why did this change..?

ohhh this is because of a PR that adds it automatically. thanks will remove!

This exposes the already existing "create container" operation. It is very similar to "docker run -d" except it doesn't actually start the container, but just prepares it. It can then be manually started using "docker start" at any point. Docker-DCO-1.1-Signed-off-by: Alexander Larsson <alexl@redhat.com> (github: alexlarsson) Conflicts: api/client/commands.go runconfig/parse.go server/container.go Docker-DCO-1.1-Signed-off-by: Tibor Vass <teabee89@gmail.com> (github: tiborvass)

Docker-DCO-1.1-Signed-off-by: Alexander Larsson <alexl@redhat.com> (github: alexlarsson) Docker-DCO-1.1-Signed-off-by: Tibor Vass <teabee89@gmail.com> (github: tiborvass)

Docker-DCO-1.1-Signed-off-by: Tibor Vass <teabee89@gmail.com> (github: tiborvass)

Signed-off-by: Tibor Vass <teabee89@gmail.com>

Docker-DCO-1.1-Signed-off-by: SvenDowideit <SvenDowideit@home.org.au> (github: SvenDowideit)

Signed-off-by: Tibor Vass <teabee89@gmail.com>

tiborvass · 2014-09-16T22:40:56Z

@jfrazelle updated

jessfraz · 2014-09-16T22:44:29Z

nice! LGTM

LK4D4 · 2014-09-16T23:21:44Z

LGTM

Docker create (rebase of 6907)

rhatdan · 2014-09-17T17:22:50Z

Really should start to require anyone adding a new command to provide man pages.

docker exec and docker create

both do not have man pages.

Also bash-completions should be added.

jessfraz · 2014-09-17T17:27:06Z

bash completions were added for both, and man pages were added for create looking into exec now

jessfraz · 2014-09-17T17:30:52Z

@rhatdan exec updated man pages as well

rhatdan · 2014-09-17T17:31:05Z

Looks like I need to rebuild, never mind.

tiborvass added the /project/doc label Jul 19, 2014

jamtur01 reviewed Jul 19, 2014
View reviewed changes

mrunalp mentioned this pull request Jul 23, 2014

Add "docker create" support #6907

Closed

ndwns reviewed Jul 28, 2014
View reviewed changes

crosbymichael reviewed Jul 30, 2014
View reviewed changes

jamtur01 reviewed Jul 30, 2014
View reviewed changes

ghost reviewed Jul 30, 2014
View reviewed changes

crosbymichael reviewed Jul 30, 2014
View reviewed changes

ghost reviewed Jul 31, 2014
View reviewed changes

tiborvass force-pushed the merge-6907 branch from 2b5e5a3 to 1218865 Compare September 11, 2014 18:25

tiborvass force-pushed the merge-6907 branch from 1218865 to 7c32546 Compare September 15, 2014 22:03

tiborvass force-pushed the merge-6907 branch from 7c32546 to 4eb7cc7 Compare September 16, 2014 22:24

jessfraz reviewed Sep 16, 2014
View reviewed changes

alexlarsson and others added 6 commits September 16, 2014 18:40

integration-cli: Add docker create tests

b0cb37f

Docker-DCO-1.1-Signed-off-by: Alexander Larsson <alexl@redhat.com> (github: alexlarsson) Docker-DCO-1.1-Signed-off-by: Tibor Vass <teabee89@gmail.com> (github: tiborvass)

docs fix

7c85cf5

Docker-DCO-1.1-Signed-off-by: Tibor Vass <teabee89@gmail.com> (github: tiborvass)

Add create to contrib/completion

1ec283c

Signed-off-by: Tibor Vass <teabee89@gmail.com>

add 'docker create' man page

22eb3a3

Docker-DCO-1.1-Signed-off-by: SvenDowideit <SvenDowideit@home.org.au> (github: SvenDowideit)

Resolve conflicts with restart policies

e49c701

Signed-off-by: Tibor Vass <teabee89@gmail.com>

tiborvass force-pushed the merge-6907 branch from 4eb7cc7 to e49c701 Compare September 16, 2014 22:40

LK4D4 added a commit that referenced this pull request Sep 16, 2014

Merge pull request #7110 from tiborvass/merge-6907

ca39a3e

Docker create (rebase of 6907)

LK4D4 merged commit ca39a3e into moby:master Sep 16, 2014

cpuguy83 mentioned this pull request Sep 18, 2014

Create a container without starting it in the CLI to allow external network configuration #8110

Closed

duglin mentioned this pull request Sep 26, 2014

suggestion: add a 'create' command #2177

Closed

tiborvass deleted the merge-6907 branch July 17, 2019 00:34

		-w, --workdir="" Working directory inside the container


		The `docker create` command `creates` a writeable container layer over

		Usage: docker create [OPTIONS] IMAGE[:TAG] [COMMAND] [ARG...]


		-a, --attach=[] Attach to stdin, stdout or stderr.

Docker create (rebase of 6907) #7110

Docker create (rebase of 6907) #7110

Conversation

tiborvass commented Jul 18, 2014

mrunalp commented Jul 18, 2014

tiborvass commented Jul 18, 2014

Choose a reason for hiding this comment

mrunalp commented Jul 24, 2014

tiborvass commented Jul 26, 2014

Choose a reason for hiding this comment

crosbymichael commented Jul 30, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SvenDowideit commented Jul 30, 2014

Choose a reason for hiding this comment

tiborvass commented Jul 30, 2014

Choose a reason for hiding this comment

tiborvass commented Jul 30, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tiborvass commented Jul 31, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Jul 31, 2014

tiborvass commented Jul 31, 2014

ghost commented Jul 31, 2014

tiborvass commented Sep 11, 2014

tiborvass commented Sep 11, 2014

tiborvass commented Sep 15, 2014

crosbymichael commented Sep 15, 2014

tiborvass commented Sep 15, 2014

jessfraz commented Sep 16, 2014

tiborvass commented Sep 16, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tiborvass commented Sep 16, 2014

jessfraz commented Sep 16, 2014

LK4D4 commented Sep 16, 2014

rhatdan commented Sep 17, 2014

jessfraz commented Sep 17, 2014

jessfraz commented Sep 17, 2014

rhatdan commented Sep 17, 2014