diff --git a/components/engine/builder/builder.go b/components/engine/builder/builder.go index fb344e97e3..45f964daf6 100644 --- a/components/engine/builder/builder.go +++ b/components/engine/builder/builder.go @@ -70,6 +70,9 @@ type buildFile struct { // Deprecated, original writer used for ImagePull. To be removed. outOld io.Writer sf *utils.StreamFormatter + + // cmdSet indicates is CMD was set in current Dockerfile + cmdSet bool } func (b *buildFile) clearTmp(containers map[string]struct{}) { @@ -199,7 +202,8 @@ func (b *buildFile) CmdRun(args string) error { } cmd := b.config.Cmd - b.config.Cmd = nil + // set Cmd manually, this is special case only for Dockerfiles + b.config.Cmd = config.Cmd runconfig.Merge(b.config, config) defer func(cmd []string) { b.config.Cmd = cmd }(cmd) @@ -306,12 +310,17 @@ func (b *buildFile) CmdCmd(args string) error { if err := b.commit("", b.config.Cmd, fmt.Sprintf("CMD %v", cmd)); err != nil { return err } + b.cmdSet = true return nil } func (b *buildFile) CmdEntrypoint(args string) error { entrypoint := b.buildCmdFromJson(args) b.config.Entrypoint = entrypoint + // if there is no cmd in current Dockerfile - cleanup cmd + if !b.cmdSet { + b.config.Cmd = nil + } if err := b.commit("", b.config.Cmd, fmt.Sprintf("ENTRYPOINT %v", entrypoint)); err != nil { return err } diff --git a/components/engine/integration-cli/docker_cli_build_test.go b/components/engine/integration-cli/docker_cli_build_test.go index 57d004ea80..05ad5476f2 100644 --- a/components/engine/integration-cli/docker_cli_build_test.go +++ b/components/engine/integration-cli/docker_cli_build_test.go @@ -1310,8 +1310,8 @@ func TestBuildEntrypointRunCleanup(t *testing.T) { if err != nil { t.Fatal(err) } - // Cmd inherited from busybox, maybe will be fixed in #5147 - if expected := "[/bin/sh]"; res != expected { + // Cmd must be cleaned up + if expected := ""; res != expected { t.Fatalf("Cmd %s, expected %s", res, expected) } logDone("build - cleanup cmd after RUN") @@ -1875,3 +1875,36 @@ func TestBuildFromGIT(t *testing.T) { } logDone("build - build from GIT") } + +func TestBuildCleanupCmdOnEntrypoint(t *testing.T) { + name := "testbuildcmdcleanuponentrypoint" + defer deleteImages(name) + if _, err := buildImage(name, + `FROM scratch + CMD ["test"] + ENTRYPOINT ["echo"]`, + true); err != nil { + t.Fatal(err) + } + if _, err := buildImage(name, + fmt.Sprintf(`FROM %s + ENTRYPOINT ["cat"]`, name), + true); err != nil { + t.Fatal(err) + } + res, err := inspectField(name, "Config.Cmd") + if err != nil { + t.Fatal(err) + } + if expected := ""; res != expected { + t.Fatalf("Cmd %s, expected %s", res, expected) + } + res, err = inspectField(name, "Config.Entrypoint") + if err != nil { + t.Fatal(err) + } + if expected := "[cat]"; res != expected { + t.Fatalf("Entrypoint %s, expected %s", res, expected) + } + logDone("build - cleanup cmd on ENTRYPOINT") +} diff --git a/components/engine/integration-cli/docker_cli_run_test.go b/components/engine/integration-cli/docker_cli_run_test.go index d02ebb54e6..2cfef2acb9 100644 --- a/components/engine/integration-cli/docker_cli_run_test.go +++ b/components/engine/integration-cli/docker_cli_run_test.go @@ -1454,3 +1454,29 @@ func TestCopyVolumeContent(t *testing.T) { t.Fatal("Container failed to transfer content to volume") } } + +func TestRunCleanupCmdOnEntrypoint(t *testing.T) { + name := "testrunmdcleanuponentrypoint" + defer deleteImages(name) + defer deleteAllContainers() + if _, err := buildImage(name, + `FROM busybox + ENTRYPOINT ["echo"] + CMD ["testingpoint"]`, + true); err != nil { + t.Fatal(err) + } + runCmd := exec.Command(dockerBinary, "run", "--entrypoint", "whoami", name) + out, exit, err := runCommandWithOutput(runCmd) + if err != nil { + t.Fatalf("Error: %v, out: %q", err, out) + } + if exit != 0 { + t.Fatalf("expected exit code 0 received %d, out: %q", exit, out) + } + out = strings.TrimSpace(out) + if out != "root" { + t.Fatalf("Expected output root, got %q", out) + } + logDone("run - cleanup cmd on --entrypoint") +} diff --git a/components/engine/runconfig/merge.go b/components/engine/runconfig/merge.go index 290d5f3189..8b1428ed3b 100644 --- a/components/engine/runconfig/merge.go +++ b/components/engine/runconfig/merge.go @@ -20,7 +20,7 @@ func Merge(userConf, imageConf *Config) error { if userConf.CpuShares == 0 { userConf.CpuShares = imageConf.CpuShares } - if userConf.ExposedPorts == nil || len(userConf.ExposedPorts) == 0 { + if len(userConf.ExposedPorts) == 0 { userConf.ExposedPorts = imageConf.ExposedPorts } else if imageConf.ExposedPorts != nil { if userConf.ExposedPorts == nil { @@ -33,7 +33,7 @@ func Merge(userConf, imageConf *Config) error { } } - if userConf.PortSpecs != nil && len(userConf.PortSpecs) > 0 { + if len(userConf.PortSpecs) > 0 { if userConf.ExposedPorts == nil { userConf.ExposedPorts = make(nat.PortSet) } @@ -48,7 +48,7 @@ func Merge(userConf, imageConf *Config) error { } userConf.PortSpecs = nil } - if imageConf.PortSpecs != nil && len(imageConf.PortSpecs) > 0 { + if len(imageConf.PortSpecs) > 0 { // FIXME: I think we can safely remove this. Leaving it for now for the sake of reverse-compat paranoia. utils.Debugf("Migrating image port specs to containter: %s", strings.Join(imageConf.PortSpecs, ", ")) if userConf.ExposedPorts == nil { @@ -66,7 +66,7 @@ func Merge(userConf, imageConf *Config) error { } } - if userConf.Env == nil || len(userConf.Env) == 0 { + if len(userConf.Env) == 0 { userConf.Env = imageConf.Env } else { for _, imageEnv := range imageConf.Env { @@ -84,16 +84,16 @@ func Merge(userConf, imageConf *Config) error { } } - if userConf.Cmd == nil || len(userConf.Cmd) == 0 { - userConf.Cmd = imageConf.Cmd - } - if userConf.Entrypoint == nil || len(userConf.Entrypoint) == 0 { + if len(userConf.Entrypoint) == 0 { + if len(userConf.Cmd) == 0 { + userConf.Cmd = imageConf.Cmd + } userConf.Entrypoint = imageConf.Entrypoint } if userConf.WorkingDir == "" { userConf.WorkingDir = imageConf.WorkingDir } - if userConf.Volumes == nil || len(userConf.Volumes) == 0 { + if len(userConf.Volumes) == 0 { userConf.Volumes = imageConf.Volumes } else { for k, v := range imageConf.Volumes {