From 17d870bed5ef997c30da1e8b9843f4e84202f8d4 Mon Sep 17 00:00:00 2001 From: Eric Windisch Date: Sat, 7 Jun 2014 23:37:31 -0700 Subject: [PATCH 1/2] Pause/freeze containers during commit Initiates a pause before committing a container, adds a pause option to the commit command, defaulting to 'true'. Fixes bug: #6267 Fixes bug: #3675 Docker-DCO-1.1-Signed-off-by: Eric Windisch (github: ewindisch) --- api/client/commands.go | 6 +++++ api/common.go | 2 +- api/server/server.go | 6 +++++ daemon/daemon.go | 8 +++++-- docs/sources/reference/commandline/cli.md | 6 +++++ integration-cli/docker_cli_commit_test.go | 27 +++++++++++++++++++++++ integration/container_test.go | 6 ++--- integration/runtime_test.go | 2 +- server/buildfile.go | 2 +- server/server.go | 2 +- 10 files changed, 58 insertions(+), 9 deletions(-) diff --git a/api/client/commands.go b/api/client/commands.go index 2bf40a666..9580ddd91 100644 --- a/api/client/commands.go +++ b/api/client/commands.go @@ -1538,6 +1538,7 @@ func (cli *DockerCli) CmdPs(args ...string) error { func (cli *DockerCli) CmdCommit(args ...string) error { cmd := cli.Subcmd("commit", "[OPTIONS] CONTAINER [REPOSITORY[:TAG]]", "Create a new image from a container's changes") + flPause := cmd.Bool([]string{"p", "-pause"}, true, "Pause container during commit") flComment := cmd.String([]string{"m", "-message"}, "", "Commit message") flAuthor := cmd.String([]string{"a", "#author", "-author"}, "", "Author (eg. \"John Hannibal Smith \")") // FIXME: --run is deprecated, it will be replaced with inline Dockerfile commands. @@ -1569,6 +1570,11 @@ func (cli *DockerCli) CmdCommit(args ...string) error { v.Set("tag", tag) v.Set("comment", *flComment) v.Set("author", *flAuthor) + + if *flPause != true { + v.Set("pause", "0") + } + var ( config *runconfig.Config env engine.Env diff --git a/api/common.go b/api/common.go index a20c5d7d1..e73705000 100644 --- a/api/common.go +++ b/api/common.go @@ -11,7 +11,7 @@ import ( ) const ( - APIVERSION version.Version = "1.12" + APIVERSION version.Version = "1.13" DEFAULTHTTPHOST = "127.0.0.1" DEFAULTUNIXSOCKET = "/var/run/docker.sock" ) diff --git a/api/server/server.go b/api/server/server.go index 6cb9fc872..0cb7134c6 100644 --- a/api/server/server.go +++ b/api/server/server.go @@ -439,6 +439,12 @@ func postCommit(eng *engine.Engine, version version.Version, w http.ResponseWrit utils.Errorf("%s", err) } + if r.FormValue("pause") == "" && version.GreaterThanOrEqualTo("1.13") { + job.Setenv("pause", "1") + } else { + job.Setenv("pause", r.FormValue("pause")) + } + job.Setenv("repo", r.Form.Get("repo")) job.Setenv("tag", r.Form.Get("tag")) job.Setenv("author", r.Form.Get("author")) diff --git a/daemon/daemon.go b/daemon/daemon.go index aa8d2bad3..9500526f9 100644 --- a/daemon/daemon.go +++ b/daemon/daemon.go @@ -620,8 +620,12 @@ func (daemon *Daemon) createRootfs(container *Container, img *image.Image) error // Commit creates a new filesystem image from the current state of a container. // The image can optionally be tagged into a repository -func (daemon *Daemon) Commit(container *Container, repository, tag, comment, author string, config *runconfig.Config) (*image.Image, error) { - // FIXME: freeze the container before copying it to avoid data corruption? +func (daemon *Daemon) Commit(container *Container, repository, tag, comment, author string, pause bool, config *runconfig.Config) (*image.Image, error) { + if pause { + container.Pause() + defer container.Unpause() + } + if err := container.Mount(); err != nil { return nil, err } diff --git a/docs/sources/reference/commandline/cli.md b/docs/sources/reference/commandline/cli.md index 56e420204..ff41ec432 100644 --- a/docs/sources/reference/commandline/cli.md +++ b/docs/sources/reference/commandline/cli.md @@ -337,6 +337,7 @@ schema. -a, --author="" Author (e.g., "John Hannibal Smith ") -m, --message="" Commit message + -p, --pause=true Pause container during commit It can be useful to commit a container's file changes or settings into a new image. This allows you debug a container by running an interactive @@ -344,6 +345,11 @@ shell, or to export a working dataset to another server. Generally, it is better to use Dockerfiles to manage your images in a documented and maintainable way. +By default, the container being committed and its processes will be paused +during the process of committing the image. This reduces the likelihood of +encountering data corruption during the process of creating the commit. +If this behavior is undesired, set the 'p' option to false. + ### Commit an existing container $ sudo docker ps diff --git a/integration-cli/docker_cli_commit_test.go b/integration-cli/docker_cli_commit_test.go index c02c89cd3..3a76a0f38 100644 --- a/integration-cli/docker_cli_commit_test.go +++ b/integration-cli/docker_cli_commit_test.go @@ -34,6 +34,33 @@ func TestCommitAfterContainerIsDone(t *testing.T) { logDone("commit - echo foo and commit the image") } +func TestCommitWithoutPause(t *testing.T) { + runCmd := exec.Command(dockerBinary, "run", "-i", "-a", "stdin", "busybox", "echo", "foo") + out, _, _, err := runCommandWithStdoutStderr(runCmd) + errorOut(err, t, fmt.Sprintf("failed to run container: %v %v", out, err)) + + cleanedContainerID := stripTrailingCharacters(out) + + waitCmd := exec.Command(dockerBinary, "wait", cleanedContainerID) + _, _, err = runCommandWithOutput(waitCmd) + errorOut(err, t, fmt.Sprintf("error thrown while waiting for container: %s", out)) + + commitCmd := exec.Command(dockerBinary, "commit", "-p", "false", cleanedContainerID) + out, _, err = runCommandWithOutput(commitCmd) + errorOut(err, t, fmt.Sprintf("failed to commit container to image: %v %v", out, err)) + + cleanedImageID := stripTrailingCharacters(out) + + inspectCmd := exec.Command(dockerBinary, "inspect", cleanedImageID) + out, _, err = runCommandWithOutput(inspectCmd) + errorOut(err, t, fmt.Sprintf("failed to inspect image: %v %v", out, err)) + + deleteContainer(cleanedContainerID) + deleteImages(cleanedImageID) + + logDone("commit - echo foo and commit the image") +} + func TestCommitNewFile(t *testing.T) { cmd := exec.Command(dockerBinary, "run", "--name", "commiter", "busybox", "/bin/sh", "-c", "echo koye > /foo") if _, err := runCommand(cmd); err != nil { diff --git a/integration/container_test.go b/integration/container_test.go index 31a57df77..48b3321a5 100644 --- a/integration/container_test.go +++ b/integration/container_test.go @@ -421,7 +421,7 @@ func TestCopyVolumeUidGid(t *testing.T) { t.Errorf("Container shouldn't be running") } - img, err := r.Commit(container1, "", "", "unit test commited image", "", nil) + img, err := r.Commit(container1, "", "", "unit test commited image", "", true, nil) if err != nil { t.Error(err) } @@ -447,7 +447,7 @@ func TestCopyVolumeUidGid(t *testing.T) { t.Errorf("Container shouldn't be running") } - img2, err := r.Commit(container2, "", "", "unit test commited image", "", nil) + img2, err := r.Commit(container2, "", "", "unit test commited image", "", true, nil) if err != nil { t.Error(err) } @@ -481,7 +481,7 @@ func TestCopyVolumeContent(t *testing.T) { t.Errorf("Container shouldn't be running") } - img, err := r.Commit(container1, "", "", "unit test commited image", "", nil) + img, err := r.Commit(container1, "", "", "unit test commited image", "", true, nil) if err != nil { t.Error(err) } diff --git a/integration/runtime_test.go b/integration/runtime_test.go index 754146c5f..4c0f636d6 100644 --- a/integration/runtime_test.go +++ b/integration/runtime_test.go @@ -334,7 +334,7 @@ func TestDaemonCreate(t *testing.T) { } container, _, err = daemon.Create(config, "") - _, err = daemon.Commit(container, "testrepo", "testtag", "", "", config) + _, err = daemon.Commit(container, "testrepo", "testtag", "", "", true, config) if err != nil { t.Error(err) } diff --git a/server/buildfile.go b/server/buildfile.go index f5ef6e0d2..71fed660b 100644 --- a/server/buildfile.go +++ b/server/buildfile.go @@ -752,7 +752,7 @@ func (b *buildFile) commit(id string, autoCmd []string, comment string) error { autoConfig := *b.config autoConfig.Cmd = autoCmd // Commit the container - image, err := b.daemon.Commit(container, "", "", "", b.maintainer, &autoConfig) + image, err := b.daemon.Commit(container, "", "", "", b.maintainer, true, &autoConfig) if err != nil { return err } diff --git a/server/server.go b/server/server.go index 0bbb9f31c..a0ff5fe89 100644 --- a/server/server.go +++ b/server/server.go @@ -1038,7 +1038,7 @@ func (srv *Server) ContainerCommit(job *engine.Job) engine.Status { return job.Error(err) } - img, err := srv.daemon.Commit(container, job.Getenv("repo"), job.Getenv("tag"), job.Getenv("comment"), job.Getenv("author"), &newConfig) + img, err := srv.daemon.Commit(container, job.Getenv("repo"), job.Getenv("tag"), job.Getenv("comment"), job.Getenv("author"), job.GetenvBool("pause"), &newConfig) if err != nil { return job.Error(err) } From 957c510d729ac3dcbe892b145fb2d9ed766be2ca Mon Sep 17 00:00:00 2001 From: Victor Vieux Date: Tue, 1 Jul 2014 00:39:38 +0000 Subject: [PATCH 2/2] updated docs & tests Docker-DCO-1.1-Signed-off-by: Victor Vieux (github: vieux) --- docs/man/docker-commit.1.md | 3 +++ docs/sources/reference/api/docker_remote_api.md | 5 +++++ docs/sources/reference/commandline/cli.md | 2 +- integration-cli/docker_cli_commit_test.go | 4 ++-- 4 files changed, 11 insertions(+), 3 deletions(-) diff --git a/docs/man/docker-commit.1.md b/docs/man/docker-commit.1.md index e9f339338..c0f2c734c 100644 --- a/docs/man/docker-commit.1.md +++ b/docs/man/docker-commit.1.md @@ -19,6 +19,9 @@ Using an existing container's name or ID you can create a new image. **-m, --message**="" Commit message +**-p, --pause**=true + Pause container during commit + # EXAMPLES ## Creating a new image from an existing container diff --git a/docs/sources/reference/api/docker_remote_api.md b/docs/sources/reference/api/docker_remote_api.md index c3cce3ea3..36f35383e 100644 --- a/docs/sources/reference/api/docker_remote_api.md +++ b/docs/sources/reference/api/docker_remote_api.md @@ -49,6 +49,11 @@ daemon is configured to listen on. **New!** `start` and `stop` will now return 304 if the container's status is not modified +`POST /commit` + +**New!** +Added a `pause` parameter (default `true`) to pause the container during commit + ## v1.12 ### Full Documentation diff --git a/docs/sources/reference/commandline/cli.md b/docs/sources/reference/commandline/cli.md index ff41ec432..770ac1177 100644 --- a/docs/sources/reference/commandline/cli.md +++ b/docs/sources/reference/commandline/cli.md @@ -346,7 +346,7 @@ is better to use Dockerfiles to manage your images in a documented and maintainable way. By default, the container being committed and its processes will be paused -during the process of committing the image. This reduces the likelihood of +while the image is committed. This reduces the likelihood of encountering data corruption during the process of creating the commit. If this behavior is undesired, set the 'p' option to false. diff --git a/integration-cli/docker_cli_commit_test.go b/integration-cli/docker_cli_commit_test.go index 3a76a0f38..7caf3588c 100644 --- a/integration-cli/docker_cli_commit_test.go +++ b/integration-cli/docker_cli_commit_test.go @@ -45,7 +45,7 @@ func TestCommitWithoutPause(t *testing.T) { _, _, err = runCommandWithOutput(waitCmd) errorOut(err, t, fmt.Sprintf("error thrown while waiting for container: %s", out)) - commitCmd := exec.Command(dockerBinary, "commit", "-p", "false", cleanedContainerID) + commitCmd := exec.Command(dockerBinary, "commit", "-p=false", cleanedContainerID) out, _, err = runCommandWithOutput(commitCmd) errorOut(err, t, fmt.Sprintf("failed to commit container to image: %v %v", out, err)) @@ -58,7 +58,7 @@ func TestCommitWithoutPause(t *testing.T) { deleteContainer(cleanedContainerID) deleteImages(cleanedImageID) - logDone("commit - echo foo and commit the image") + logDone("commit - echo foo and commit the image with --pause=false") } func TestCommitNewFile(t *testing.T) {