Merge pull request docker#19092 from anusha-ragunathan/builder-rm-merge

Remove runconfig.Merge

builder/dockerfile/dispatchers.go

@@ -24,7 +24,6 @@ import (
 	derr "github.com/docker/docker/errors"
 	"github.com/docker/docker/pkg/signal"
 	"github.com/docker/docker/pkg/system"
-	"github.com/docker/docker/runconfig"
 	runconfigopts "github.com/docker/docker/runconfig/opts"
 	"github.com/docker/go-connections/nat"
 )
@@ -317,7 +316,10 @@ func run(b *Builder, args []string, attributes map[string]bool, original string)
 
 	// stash the cmd
 	cmd := b.runConfig.Cmd
-	runconfig.Merge(b.runConfig, config)
+	if b.runConfig.Entrypoint.Len() == 0 && b.runConfig.Cmd.Len() == 0 {
+		b.runConfig.Cmd = config.Cmd
+	}
+
 	// stash the config environment
 	env := b.runConfig.Env
 

daemon/commit.go

@@ -8,16 +8,90 @@ import (
 	"time"
 
 	"github.com/docker/docker/api/types"
+	containertypes "github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/container"
 	"github.com/docker/docker/dockerversion"
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/layer"
 	"github.com/docker/docker/pkg/archive"
 	"github.com/docker/docker/pkg/ioutils"
 	"github.com/docker/docker/reference"
-	"github.com/docker/docker/runconfig"
+	"github.com/docker/go-connections/nat"
 )
 
+// merge merges two Config, the image container configuration (defaults values),
+// and the user container configuration, either passed by the API or generated
+// by the cli.
+// It will mutate the specified user configuration (userConf) with the image
+// configuration where the user configuration is incomplete.
+func merge(userConf, imageConf *containertypes.Config) error {
+	if userConf.User == "" {
+		userConf.User = imageConf.User
+	}
+	if len(userConf.ExposedPorts) == 0 {
+		userConf.ExposedPorts = imageConf.ExposedPorts
+	} else if imageConf.ExposedPorts != nil {
+		if userConf.ExposedPorts == nil {
+			userConf.ExposedPorts = make(nat.PortSet)
+		}
+		for port := range imageConf.ExposedPorts {
+			if _, exists := userConf.ExposedPorts[port]; !exists {
+				userConf.ExposedPorts[port] = struct{}{}
+			}
+		}
+	}
+
+	if 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
+					break
+				}
+			}
+			if !found {
+				userConf.Env = append(userConf.Env, imageEnv)
+			}
+		}
+	}
+
+	if userConf.Labels == nil {
+		userConf.Labels = map[string]string{}
+	}
+	if imageConf.Labels != nil {
+		for l := range userConf.Labels {
+			imageConf.Labels[l] = userConf.Labels[l]
+		}
+		userConf.Labels = imageConf.Labels
+	}
+
+	if userConf.Entrypoint.Len() == 0 {
+		if userConf.Cmd.Len() == 0 {
+			userConf.Cmd = imageConf.Cmd
+		}
+
+		if userConf.Entrypoint == nil {
+			userConf.Entrypoint = imageConf.Entrypoint
+		}
+	}
+	if userConf.WorkingDir == "" {
+		userConf.WorkingDir = imageConf.WorkingDir
+	}
+	if len(userConf.Volumes) == 0 {
+		userConf.Volumes = imageConf.Volumes
+	} else {
+		for k, v := range imageConf.Volumes {
+			userConf.Volumes[k] = v
+		}
+	}
+	return nil
+}
+
 // 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(name string, c *types.ContainerCommitConfig) (string, error) {
@@ -37,7 +111,7 @@ func (daemon *Daemon) Commit(name string, c *types.ContainerCommitConfig) (strin
 	}
 
 	if c.MergeConfigs {
-		if err := runconfig.Merge(c.Config, container.Config); err != nil {
+		if err := merge(c.Config, container.Config); err != nil {
 			return "", err
 		}
 	}

daemon/daemon.go

@@ -416,7 +416,7 @@ func (daemon *Daemon) restore() error {
 
 func (daemon *Daemon) mergeAndVerifyConfig(config *containertypes.Config, img *image.Image) error {
 	if img != nil && img.Config != nil {
-		if err := runconfig.Merge(config, img.Config); err != nil {
+		if err := merge(config, img.Config); err != nil {
 			return err
 		}
 	}

daemon/daemon_test.go

@@ -15,6 +15,7 @@ import (
 	volumedrivers "github.com/docker/docker/volume/drivers"
 	"github.com/docker/docker/volume/local"
 	"github.com/docker/docker/volume/store"
+	"github.com/docker/go-connections/nat"
 )
 
 //
@@ -310,3 +311,84 @@ func TestContainerInitDNS(t *testing.T) {
 		t.Fatal("Expected container DNSOptions to not be nil")
 	}
 }
+
+func newPortNoError(proto, port string) nat.Port {
+	p, _ := nat.NewPort(proto, port)
+	return p
+}
+
+func TestMerge(t *testing.T) {
+	volumesImage := make(map[string]struct{})
+	volumesImage["/test1"] = struct{}{}
+	volumesImage["/test2"] = struct{}{}
+	portsImage := make(nat.PortSet)
+	portsImage[newPortNoError("tcp", "1111")] = struct{}{}
+	portsImage[newPortNoError("tcp", "2222")] = struct{}{}
+	configImage := &containertypes.Config{
+		ExposedPorts: portsImage,
+		Env:          []string{"VAR1=1", "VAR2=2"},
+		Volumes:      volumesImage,
+	}
+
+	portsUser := make(nat.PortSet)
+	portsUser[newPortNoError("tcp", "2222")] = struct{}{}
+	portsUser[newPortNoError("tcp", "3333")] = struct{}{}
+	volumesUser := make(map[string]struct{})
+	volumesUser["/test3"] = struct{}{}
+	configUser := &containertypes.Config{
+		ExposedPorts: portsUser,
+		Env:          []string{"VAR2=3", "VAR3=3"},
+		Volumes:      volumesUser,
+	}
+
+	if err := merge(configUser, configImage); err != nil {
+		t.Error(err)
+	}
+
+	if len(configUser.ExposedPorts) != 3 {
+		t.Fatalf("Expected 3 ExposedPorts, 1111, 2222 and 3333, found %d", len(configUser.ExposedPorts))
+	}
+	for portSpecs := range configUser.ExposedPorts {
+		if portSpecs.Port() != "1111" && portSpecs.Port() != "2222" && portSpecs.Port() != "3333" {
+			t.Fatalf("Expected 1111 or 2222 or 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)
+		}
+	}
+
+	ports, _, err := nat.ParsePortSpecs([]string{"0000"})
+	if err != nil {
+		t.Error(err)
+	}
+	configImage2 := &containertypes.Config{
+		ExposedPorts: ports,
+	}
+
+	if err := merge(configUser, configImage2); err != nil {
+		t.Error(err)
+	}
+
+	if len(configUser.ExposedPorts) != 4 {
+		t.Fatalf("Expected 4 ExposedPorts, 0000, 1111, 2222 and 3333, found %d", len(configUser.ExposedPorts))
+	}
+	for portSpecs := range configUser.ExposedPorts {
+		if portSpecs.Port() != "0" && portSpecs.Port() != "1111" && portSpecs.Port() != "2222" && portSpecs.Port() != "3333" {
+			t.Fatalf("Expected %q or %q or %q or %q, found %s", 0, 1111, 2222, 3333, portSpecs)
+		}
+	}
+}