improve mergeconfig, if dns, portspec, env or volumes specify in docker run, apend and not replace

utils.go

@@ -1,5 +1,9 @@
 package docker
 
+import (
+	"strings"
+)
+
 // Compare two Config struct. Do not compare the "Image" nor "Hostname" fields
 // If OpenStdin is set, then it differs
 func CompareConfig(a, b *Config) bool {
@@ -68,6 +72,20 @@ func MergeConfig(userConf, imageConf *Config) {
 	}
 	if userConf.PortSpecs == nil || len(userConf.PortSpecs) == 0 {
 		userConf.PortSpecs = imageConf.PortSpecs
+	} else {
+		for _, imagePortSpec := range imageConf.PortSpecs {
+			found := false
+			imageNat, _ := parseNat(imagePortSpec)
+			for _, userPortSpec := range userConf.PortSpecs {
+				userNat, _ := parseNat(userPortSpec)
+				if imageNat.Proto == userNat.Proto && imageNat.Frontend == userNat.Frontend {
+					found = true
+				}
+			}
+			if !found {
+				userConf.PortSpecs = append(userConf.PortSpecs, imagePortSpec)
+			}
+		}
 	}
 	if !userConf.Tty {
 		userConf.Tty = imageConf.Tty
@@ -80,17 +98,38 @@ func MergeConfig(userConf, imageConf *Config) {
 	}
 	if userConf.Env == nil || len(userConf.Env) == 0 {
 		userConf.Env = imageConf.Env
+	} else {
+		for _, imageEnv := range imageConf.Env {
+			found := false
+			imageEnvKey := strings.Split(imageEnv, "=")[0]
+			for _, userEnv := range userConf.Env {
+				userEnvKey := strings.Split(userEnv, "=")[0]
+				if imageEnvKey == userEnvKey {
+					found = true
+				}
+			}
+			if !found {
+				userConf.Env = append(userConf.Env, imageEnv)
+			}
+		}
 	}
 	if userConf.Cmd == nil || len(userConf.Cmd) == 0 {
 		userConf.Cmd = imageConf.Cmd
 	}
 	if userConf.Dns == nil || len(userConf.Dns) == 0 {
 		userConf.Dns = imageConf.Dns
+	} else {
+		//duplicates aren't an issue here
+		userConf.Dns = append(userConf.Dns, imageConf.Dns...)
 	}
 	if userConf.Entrypoint == nil || len(userConf.Entrypoint) == 0 {
 		userConf.Entrypoint = imageConf.Entrypoint
 	}
 	if userConf.Volumes == nil || len(userConf.Volumes) == 0 {
 		userConf.Volumes = imageConf.Volumes
+	} else {
+		for k, v := range imageConf.Volumes {
+			userConf.Volumes[k] = v
+		}
 	}
 }

utils_test.go

@@ -1,13 +1,13 @@
 package docker
 
 import (
+	"github.com/dotcloud/docker/utils"
 	"io"
 	"io/ioutil"
 	"os"
 	"path"
 	"strings"
 	"testing"
-	"github.com/dotcloud/docker/utils"
 )
 
 // This file contains utility functions for docker's unit test suite.
@@ -128,3 +128,61 @@ func runContainer(r *Runtime, args []string, t *testing.T) (output string, err e
 	output = string(data)
 	return
 }
+
+func TestMergeConfig(t *testing.T) {
+	volumesImage := make(map[string]struct{})
+	volumesImage["/test1"] = struct{}{}
+	volumesImage["/test2"] = struct{}{}
+	configImage := &Config{
+		Dns:       []string{"1.1.1.1", "2.2.2.2"},
+		PortSpecs: []string{"1111:1111", "2222:2222"},
+		Env:       []string{"VAR1=1", "VAR2=2"},
+		Volumes:   volumesImage,
+	}
+
+	volumesUser := make(map[string]struct{})
+	volumesUser["/test3"] = struct{}{}
+	configUser := &Config{
+		Dns:       []string{"3.3.3.3"},
+		PortSpecs: []string{"2222:3333", "3333:3333"},
+		Env:       []string{"VAR2=3", "VAR3=3"},
+		Volumes:   volumesUser,
+	}
+
+	MergeConfig(configUser, configImage)
+
+	if len(configUser.Dns) != 3 {
+		t.Fatalf("Expected 3 dns, 1.1.1.1, 2.2.2.2 and 3.3.3.3, found %d", len(configUser.Dns))
+	}
+	for _, dns := range configUser.Dns {
+		if dns != "1.1.1.1" && dns != "2.2.2.2" && dns != "3.3.3.3" {
+			t.Fatalf("Expected 1.1.1.1 or 2.2.2.2 or 3.3.3.3, found %s", dns)
+		}
+	}
+
+	if len(configUser.PortSpecs) != 3 {
+		t.Fatalf("Expected 3 portSpecs, 1111:1111, 2222:3333 and 3333:3333, found %d", len(configUser.PortSpecs))
+	}
+	for _, portSpecs := range configUser.PortSpecs {
+		if portSpecs != "1111:1111" && portSpecs != "2222:3333" && portSpecs != "3333:3333" {
+			t.Fatalf("Expected 1111:1111 or 2222:3333 or 3333:3333, found %s", portSpecs)
+		}
+	}
+	if len(configUser.Env) != 3 {
+		t.Fatalf("Expected 3 env var, VAR1=1, VAR2=3 and VAR3=3, found %d", len(configUser.Env))
+	}
+	for _, env := range configUser.Env {
+		if env != "VAR1=1" && env != "VAR2=3" && env != "VAR3=3" {
+			t.Fatalf("Expected VAR1=1 or VAR2=3 or VAR3=3, found %s", env)
+		}
+	}
+
+	if len(configUser.Volumes) != 3 {
+		t.Fatalf("Expected 3 volumes, /test1, /test2 and /test3, found %d", len(configUser.Volumes))
+	}
+	for v, _ := range configUser.Volumes {
+		if v != "/test1" && v != "/test2" && v != "/test3" {
+			t.Fatalf("Expected /test1 or /test2 or /test3, found %s", v)
+		}
+	}
+}