From eb4ae8e28aa0baf28d6cde1079a5f9c618d475b2 Mon Sep 17 00:00:00 2001 From: Anusha Ragunathan Date: Tue, 5 Jan 2016 08:48:09 -0800 Subject: [PATCH] Remove runconfig.Merge Merge was used by builder and daemon. With this commit, the builder call has been inlined and the function moved to the daemon package, which is the only other caller. Signed-off-by: Anusha Ragunathan --- builder/dockerfile/dispatchers.go | 6 ++- daemon/commit.go | 78 +++++++++++++++++++++++++++- daemon/daemon.go | 2 +- daemon/daemon_test.go | 82 ++++++++++++++++++++++++++++++ runconfig/merge.go | 81 ----------------------------- runconfig/merge_test.go | 84 ------------------------------- 6 files changed, 163 insertions(+), 170 deletions(-) delete mode 100644 runconfig/merge.go delete mode 100644 runconfig/merge_test.go diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go index 02d4413f1f..88e6750fd4 100644 --- a/builder/dockerfile/dispatchers.go +++ b/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 diff --git a/daemon/commit.go b/daemon/commit.go index ba083598b0..d8495ac65e 100644 --- a/daemon/commit.go +++ b/daemon/commit.go @@ -8,6 +8,7 @@ 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" @@ -15,9 +16,82 @@ import ( "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 } } diff --git a/daemon/daemon.go b/daemon/daemon.go index 15d6e3db71..63f2b2b168 100644 --- a/daemon/daemon.go +++ b/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 } } diff --git a/daemon/daemon_test.go b/daemon/daemon_test.go index c09191b9ce..a46712b47a 100644 --- a/daemon/daemon_test.go +++ b/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) + } + } +} diff --git a/runconfig/merge.go b/runconfig/merge.go deleted file mode 100644 index d2e953df27..0000000000 --- a/runconfig/merge.go +++ /dev/null @@ -1,81 +0,0 @@ -package runconfig - -import ( - "strings" - - "github.com/docker/docker/api/types/container" - "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 *container.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 -} diff --git a/runconfig/merge_test.go b/runconfig/merge_test.go deleted file mode 100644 index 2499dbb252..0000000000 --- a/runconfig/merge_test.go +++ /dev/null @@ -1,84 +0,0 @@ -package runconfig - -import ( - "testing" - - "github.com/docker/docker/api/types/container" - "github.com/docker/go-connections/nat" -) - -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 := &container.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 := &container.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 := &container.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) - } - } -}