Merge pull request #4000 from cap10morgan/1141-merge-configs-on-commit

merge existing config when committing

docs/sources/reference/commandline/cli.rst

@@ -305,7 +305,7 @@ by using the ``git://`` schema.
 
       -m, --message="": Commit message
       -a, --author="": Author (eg. "John Hannibal Smith <hannibal@a-team.com>"
-      --run="": Configuration to be applied when the image is launched with `docker run`.
+      --run="": Configuration changes to be applied when the image is launched with `docker run`.
                (ex: -run='{"Cmd": ["cat", "/world"], "PortSpecs": ["22"]}')
 
 .. _cli_commit_examples:
@@ -346,11 +346,40 @@ run ``ls /etc``.
         apt                     host.conf        lsb-base             rc2.d
         ...
 
+Merged configs example
+......................
+
+Say you have a Dockerfile like so:
+
+.. code-block:: bash
+
+        ENV MYVAR foobar
+        RUN apt-get install openssh
+        EXPOSE 22
+        CMD ["/usr/sbin/sshd -D"]
+        ...
+
+If you run that, make some changes, and then commit, Docker will merge the environment variable and exposed port configuration settings with any that you specify in the -run= option. This is a change from Docker 0.8.0 and prior where no attempt was made to preserve any existing configuration on commit.
+
+.. code-block:: bash
+
+        $ docker build -t me/foo .
+        $ docker run -t -i me/foo /bin/bash
+        foo-container$ [make changes in the container]
+        foo-container$ exit
+        $ docker commit -run='{"Cmd": ["ls"]}' [container-id] me/bar
+        ...
+
+The me/bar image will now have port 22 exposed, MYVAR env var set to 'foobar', and its default command will be ["ls"].
+
+Note that this is currently a shallow merge. So, for example, if you had specified a new port spec in the -run= config above, that would have clobbered the 'EXPOSE 22' setting from the parent container.
+
 Full -run example
 .................
 
 The ``--run`` JSON hash changes the ``Config`` section when running ``docker inspect CONTAINERID``
-or ``config`` when running ``docker inspect IMAGEID``.
+or ``config`` when running ``docker inspect IMAGEID``. Existing configuration key-values that are
+not overridden in the JSON hash will be merged in.
 
 (Multiline is okay within a single quote ``'``)
 

integration/server_test.go

@@ -281,6 +281,63 @@ func TestCommit(t *testing.T) {
 	}
 }
 
+func TestMergeConfigOnCommit(t *testing.T) {
+	eng := NewTestEngine(t)
+	runtime := mkRuntimeFromEngine(eng, t)
+	defer runtime.Nuke()
+
+	container1, _, _ := mkContainer(runtime, []string{"-e", "FOO=bar", unitTestImageID, "echo test > /tmp/foo"}, t)
+	defer runtime.Destroy(container1)
+
+	config, _, _, err := runconfig.Parse([]string{container1.ID, "cat /tmp/foo"}, nil)
+	if err != nil {
+		t.Error(err)
+	}
+
+	job := eng.Job("commit", container1.ID)
+	job.Setenv("repo", "testrepo")
+	job.Setenv("tag", "testtag")
+	job.SetenvJson("config", config)
+	var newId string
+	job.Stdout.AddString(&newId)
+	if err := job.Run(); err != nil {
+		t.Error(err)
+	}
+
+	container2, _, _ := mkContainer(runtime, []string{newId}, t)
+	defer runtime.Destroy(container2)
+
+	job = eng.Job("inspect", container1.Name, "container")
+	baseContainer, _ := job.Stdout.AddEnv()
+	if err := job.Run(); err != nil {
+		t.Error(err)
+	}
+
+	job = eng.Job("inspect", container2.Name, "container")
+	commitContainer, _ := job.Stdout.AddEnv()
+	if err := job.Run(); err != nil {
+		t.Error(err)
+	}
+
+	baseConfig := baseContainer.GetSubEnv("Config")
+	commitConfig := commitContainer.GetSubEnv("Config")
+
+	if commitConfig.Get("Env") != baseConfig.Get("Env") {
+		t.Fatalf("Env config in committed container should be %v, was %v",
+			baseConfig.Get("Env"), commitConfig.Get("Env"))
+	}
+
+	if baseConfig.Get("Cmd") != "[\"echo test \\u003e /tmp/foo\"]" {
+		t.Fatalf("Cmd in base container should be [\"echo test \\u003e /tmp/foo\"], was %s",
+			baseConfig.Get("Cmd"))
+	}
+
+	if commitConfig.Get("Cmd") != "[\"cat /tmp/foo\"]" {
+		t.Fatalf("Cmd in committed container should be [\"cat /tmp/foo\"], was %s",
+			commitConfig.Get("Cmd"))
+	}
+}
+
 func TestRestartKillWait(t *testing.T) {
 	eng := NewTestEngine(t)
 	srv := mkServerFromEngine(eng, t)

server.go

@@ -1038,12 +1038,17 @@ func (srv *Server) ContainerCommit(job *engine.Job) engine.Status {
 	if container == nil {
 		return job.Errorf("No such container: %s", name)
 	}
-	var config runconfig.Config
+	var config = container.Config
-	if err := job.GetenvJson("config", &config); err != nil {
+	var newConfig runconfig.Config
+	if err := job.GetenvJson("config", &newConfig); err != nil {
 		return job.Error(err)
 	}
 
-	img, err := srv.runtime.Commit(container, job.Getenv("repo"), job.Getenv("tag"), job.Getenv("comment"), job.Getenv("author"), &config)
+	if err := runconfig.Merge(&newConfig, config); err != nil {
+		return job.Error(err)
+	}
+
+	img, err := srv.runtime.Commit(container, job.Getenv("repo"), job.Getenv("tag"), job.Getenv("comment"), job.Getenv("author"), &newConfig)
 	if err != nil {
 		return job.Error(err)
 	}