Merge pull request #6909 from LK4D4/prune_cmd_on_entrypoint_setting_#5147

Cleanup cmd on entrypoint setting

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
 	}

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 := "<no value>"; 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 := "<no value>"; 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")
+}

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")
+}

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 {