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 <ewindisch@docker.com> (github: ewindisch)

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 <hannibal@a-team.com>\")")
 	// 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

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"
 )

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"))

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
 	}

docs/sources/reference/commandline/cli.md

@@ -337,6 +337,7 @@ schema.
 
       -a, --author=""     Author (e.g., "John Hannibal Smith <hannibal@a-team.com>")
       -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

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 {

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)
 	}

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)
 	}

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
 	}

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)
 	}