remove deprecated feature of passing HostConfig at API container start

mountkin · mountkin · commit 0a8386c8be3f · 2016-06-01T22:25:17.000+08:00
Signed-off-by: Shijiang Wei &lt;mountkin@gmail.com&gt;
diff --git a/api/server/router/container/container.go b/api/server/router/container/container.go
@@ -5,6 +5,14 @@ import (
 	"github.com/docker/docker/api/server/router"
 )
 
+type validationError struct {
+	error
+}
+
+func (validationError) IsValidationError() bool {
+	return true
+}
+
 // containerRouter is a router to talk with the container controller
 type containerRouter struct {
 	backend Backend
diff --git a/api/server/router/container/container_routes.go b/api/server/router/container/container_routes.go
@@ -131,8 +131,15 @@ func (s *containerRouter) postContainersStart(ctx context.Context, w http.Respon
 	// net/http otherwise seems to swallow any headers related to chunked encoding
 	// including r.TransferEncoding
 	// allow a nil body for backwards compatibility
+
 	var hostConfig *container.HostConfig
-	if r.Body != nil && (r.ContentLength > 0 || r.ContentLength == -1) {
+	// A non-nil json object is at least 7 characters.
+	if r.ContentLength > 7 || r.ContentLength == -1 {
+		version := httputils.VersionFromContext(ctx)
+		if versions.GreaterThanOrEqualTo(version, "1.24") {
+			return validationError{fmt.Errorf("starting container with HostConfig was deprecated since v1.10 and removed in v1.12")}
+		}
+
 		if err := httputils.CheckForJSON(r); err != nil {
 			return err
 		}
@@ -141,7 +148,6 @@ func (s *containerRouter) postContainersStart(ctx context.Context, w http.Respon
 		if err != nil {
 			return err
 		}
-
 		hostConfig = c
 	}
 
diff --git a/docs/reference/api/docker_remote_api.md b/docs/reference/api/docker_remote_api.md
@@ -122,6 +122,7 @@ This section lists each version from latest to oldest.  Each listing includes a
 * `GET /events` now supports a `reload` event that is emitted when the daemon configuration is reloaded.
 * `GET /events` now supports filtering by daemon name or ID.
 * `GET /images/json` now supports filters `since` and `before`.
+* `POST /containers/(id or name)/start` no longer accepts a `HostConfig`.
 
 ### v1.23 API changes
 
diff --git a/docs/reference/api/docker_remote_api_v1.24.md b/docs/reference/api/docker_remote_api_v1.24.md
@@ -1010,10 +1010,6 @@ Status Codes:
 
 Start the container `id`
 
-> **Note**:
-> For backwards compatibility, this endpoint accepts a `HostConfig` as JSON-encoded request body.
-> See [create a container](#create-a-container) for details.
-
 **Example request**:
 
     POST /containers/e90e34656806/start HTTP/1.1
diff --git a/integration-cli/docker_api_containers_test.go b/integration-cli/docker_api_containers_test.go
@@ -168,94 +168,6 @@ func (s *DockerSuite) TestContainerApiGetChanges(c *check.C) {
 	c.Assert(success, checker.True, check.Commentf("/etc/passwd has been removed but is not present in the diff"))
 }
 
-func (s *DockerSuite) TestContainerApiStartVolumeBinds(c *check.C) {
-	// TODO Windows CI: Investigate further why this fails on Windows to Windows CI.
-	testRequires(c, DaemonIsLinux)
-	path := "/foo"
-	if daemonPlatform == "windows" {
-		path = `c:\foo`
-	}
-	name := "testing"
-	config := map[string]interface{}{
-		"Image":   "busybox",
-		"Volumes": map[string]struct{}{path: {}},
-	}
-
-	status, _, err := sockRequest("POST", "/containers/create?name="+name, config)
-	c.Assert(err, checker.IsNil)
-	c.Assert(status, checker.Equals, http.StatusCreated)
-
-	bindPath := randomTmpDirPath("test", daemonPlatform)
-	config = map[string]interface{}{
-		"Binds": []string{bindPath + ":" + path},
-	}
-	status, _, err = sockRequest("POST", "/containers/"+name+"/start", config)
-	c.Assert(err, checker.IsNil)
-	c.Assert(status, checker.Equals, http.StatusNoContent)
-
-	pth, err := inspectMountSourceField(name, path)
-	c.Assert(err, checker.IsNil)
-	c.Assert(pth, checker.Equals, bindPath, check.Commentf("expected volume host path to be %s, got %s", bindPath, pth))
-}
-
-// Test for GH#10618
-func (s *DockerSuite) TestContainerApiStartDupVolumeBinds(c *check.C) {
-	// TODO Windows to Windows CI - Port this
-	testRequires(c, DaemonIsLinux)
-	name := "testdups"
-	config := map[string]interface{}{
-		"Image":   "busybox",
-		"Volumes": map[string]struct{}{"/tmp": {}},
-	}
-
-	status, _, err := sockRequest("POST", "/containers/create?name="+name, config)
-	c.Assert(err, checker.IsNil)
-	c.Assert(status, checker.Equals, http.StatusCreated)
-
-	bindPath1 := randomTmpDirPath("test1", daemonPlatform)
-	bindPath2 := randomTmpDirPath("test2", daemonPlatform)
-
-	config = map[string]interface{}{
-		"Binds": []string{bindPath1 + ":/tmp", bindPath2 + ":/tmp"},
-	}
-	status, body, err := sockRequest("POST", "/containers/"+name+"/start", config)
-	c.Assert(err, checker.IsNil)
-	c.Assert(status, checker.Equals, http.StatusInternalServerError)
-	c.Assert(string(body), checker.Contains, "Duplicate mount point", check.Commentf("Expected failure due to duplicate bind mounts to same path, instead got: %q with error: %v", string(body), err))
-}
-
-func (s *DockerSuite) TestContainerApiStartVolumesFrom(c *check.C) {
-	// TODO Windows to Windows CI - Port this
-	testRequires(c, DaemonIsLinux)
-	volName := "voltst"
-	volPath := "/tmp"
-
-	dockerCmd(c, "run", "--name", volName, "-v", volPath, "busybox")
-
-	name := "TestContainerApiStartVolumesFrom"
-	config := map[string]interface{}{
-		"Image":   "busybox",
-		"Volumes": map[string]struct{}{volPath: {}},
-	}
-
-	status, _, err := sockRequest("POST", "/containers/create?name="+name, config)
-	c.Assert(err, checker.IsNil)
-	c.Assert(status, checker.Equals, http.StatusCreated)
-
-	config = map[string]interface{}{
-		"VolumesFrom": []string{volName},
-	}
-	status, _, err = sockRequest("POST", "/containers/"+name+"/start", config)
-	c.Assert(err, checker.IsNil)
-	c.Assert(status, checker.Equals, http.StatusNoContent)
-
-	pth, err := inspectMountSourceField(name, volPath)
-	c.Assert(err, checker.IsNil)
-	pth2, err := inspectMountSourceField(volName, volPath)
-	c.Assert(err, checker.IsNil)
-	c.Assert(pth, checker.Equals, pth2, check.Commentf("expected volume host path to be %s, got %s", pth, pth2))
-}
-
 func (s *DockerSuite) TestGetContainerStats(c *check.C) {
 	// Problematic on Windows as Windows does not support stats
 	testRequires(c, DaemonIsLinux)
@@ -442,27 +354,6 @@ func (s *DockerSuite) TestGetStoppedContainerStats(c *check.C) {
 	}
 }
 
-// #9981 - Allow a docker created volume (ie, one in /var/lib/docker/volumes) to be used to overwrite (via passing in Binds on api start) an existing volume
-func (s *DockerSuite) TestPostContainerBindNormalVolume(c *check.C) {
-	// TODO Windows to Windows CI - Port this
-	testRequires(c, DaemonIsLinux)
-	dockerCmd(c, "create", "-v", "/foo", "--name=one", "busybox")
-
-	fooDir, err := inspectMountSourceField("one", "/foo")
-	c.Assert(err, checker.IsNil)
-
-	dockerCmd(c, "create", "-v", "/foo", "--name=two", "busybox")
-
-	bindSpec := map[string][]string{"Binds": {fooDir + ":/foo"}}
-	status, _, err := sockRequest("POST", "/containers/two/start", bindSpec)
-	c.Assert(err, checker.IsNil)
-	c.Assert(status, checker.Equals, http.StatusNoContent)
-
-	fooDir2, err := inspectMountSourceField("two", "/foo")
-	c.Assert(err, checker.IsNil)
-	c.Assert(fooDir2, checker.Equals, fooDir, check.Commentf("expected volume path to be %s, got: %s", fooDir, fooDir2))
-}
-
 func (s *DockerSuite) TestContainerApiPause(c *check.C) {
 	// Problematic on Windows as Windows does not support pause
 	testRequires(c, DaemonIsLinux)
@@ -887,26 +778,6 @@ func (s *DockerSuite) TestCreateWithTooLowMemoryLimit(c *check.C) {
 	c.Assert(string(b), checker.Contains, "Minimum memory limit allowed is 4MB")
 }
 
-func (s *DockerSuite) TestStartWithTooLowMemoryLimit(c *check.C) {
-	// TODO Windows: Port once memory is supported
-	testRequires(c, DaemonIsLinux)
-	out, _ := dockerCmd(c, "create", "busybox")
-
-	containerID := strings.TrimSpace(out)
-
-	config := `{
-                "CpuShares": 100,
-                "Memory":    524287
-        }`
-
-	res, body, err := sockRequestRaw("POST", "/containers/"+containerID+"/start", strings.NewReader(config), "application/json")
-	c.Assert(err, checker.IsNil)
-	b, err2 := readBody(body)
-	c.Assert(err2, checker.IsNil)
-	c.Assert(res.StatusCode, checker.Equals, http.StatusInternalServerError)
-	c.Assert(string(b), checker.Contains, "Minimum memory limit allowed is 4MB")
-}
-
 func (s *DockerSuite) TestContainerApiRename(c *check.C) {
 	// TODO Windows: Debug why this sometimes fails on TP5. For now, leave disabled
 	testRequires(c, DaemonIsLinux)
@@ -973,13 +844,12 @@ func (s *DockerSuite) TestContainerApiStart(c *check.C) {
 	c.Assert(err, checker.IsNil)
 	c.Assert(status, checker.Equals, http.StatusCreated)
 
-	conf := make(map[string]interface{})
-	status, _, err = sockRequest("POST", "/containers/"+name+"/start", conf)
+	status, _, err = sockRequest("POST", "/containers/"+name+"/start", nil)
 	c.Assert(err, checker.IsNil)
 	c.Assert(status, checker.Equals, http.StatusNoContent)
 
 	// second call to start should give 304
-	status, _, err = sockRequest("POST", "/containers/"+name+"/start", conf)
+	status, _, err = sockRequest("POST", "/containers/"+name+"/start", nil)
 	c.Assert(err, checker.IsNil)
 
 	// TODO(tibor): figure out why this doesn't work on windows
@@ -1189,16 +1059,21 @@ func (s *DockerSuite) TestContainerApiDeleteRemoveVolume(c *check.C) {
 func (s *DockerSuite) TestContainerApiChunkedEncoding(c *check.C) {
 	// TODO Windows CI: This can be ported
 	testRequires(c, DaemonIsLinux)
-	out, _ := dockerCmd(c, "create", "-v", "/foo", "busybox", "true")
-	id := strings.TrimSpace(out)
 
 	conn, err := sockConn(time.Duration(10 * time.Second))
 	c.Assert(err, checker.IsNil)
 	client := httputil.NewClientConn(conn, nil)
 	defer client.Close()
 
-	bindCfg := strings.NewReader(`{"Binds": ["/tmp:/foo"]}`)
-	req, err := http.NewRequest("POST", "/containers/"+id+"/start", bindCfg)
+	config := map[string]interface{}{
+		"Image":     "busybox",
+		"Cmd":       append([]string{"/bin/sh", "-c"}, defaultSleepCommand...),
+		"OpenStdin": true,
+	}
+	b, err := json.Marshal(config)
+	c.Assert(err, checker.IsNil)
+
+	req, err := http.NewRequest("POST", "/containers/create", bytes.NewBuffer(b))
 	c.Assert(err, checker.IsNil)
 	req.Header.Set("Content-Type", "application/json")
 	// This is a cheat to make the http request do chunked encoding
@@ -1207,18 +1082,9 @@ func (s *DockerSuite) TestContainerApiChunkedEncoding(c *check.C) {
 	req.ContentLength = -1
 
 	resp, err := client.Do(req)
-	c.Assert(err, checker.IsNil, check.Commentf("error starting container with chunked encoding"))
+	c.Assert(err, checker.IsNil, check.Commentf("error creating container with chunked encoding"))
 	resp.Body.Close()
-	c.Assert(resp.StatusCode, checker.Equals, 204)
-
-	out = inspectFieldJSON(c, id, "HostConfig.Binds")
-
-	var binds []string
-	c.Assert(json.NewDecoder(strings.NewReader(out)).Decode(&binds), checker.IsNil)
-	c.Assert(binds, checker.HasLen, 1, check.Commentf("Got unexpected binds: %v", binds))
-
-	expected := "/tmp:/foo"
-	c.Assert(binds[0], checker.Equals, expected, check.Commentf("got incorrect bind spec"))
+	c.Assert(resp.StatusCode, checker.Equals, http.StatusCreated)
 }
 
 func (s *DockerSuite) TestContainerApiPostContainerStop(c *check.C) {
@@ -1302,59 +1168,6 @@ func (s *DockerSuite) TestPostContainersCreateWithStringOrSliceCapAddDrop(c *che
 	c.Assert(status, checker.Equals, http.StatusCreated)
 }
 
-// #14640
-func (s *DockerSuite) TestPostContainersStartWithoutLinksInHostConfig(c *check.C) {
-	// TODO Windows: Windows doesn't support supplying a hostconfig on start.
-	// An alternate test could be written to validate the negative testing aspect of this
-	testRequires(c, DaemonIsLinux)
-	name := "test-host-config-links"
-	dockerCmd(c, append([]string{"create", "--name", name, "busybox"}, defaultSleepCommand...)...)
-
-	hc := inspectFieldJSON(c, name, "HostConfig")
-	config := `{"HostConfig":` + hc + `}`
-
-	res, b, err := sockRequestRaw("POST", "/containers/"+name+"/start", strings.NewReader(config), "application/json")
-	c.Assert(err, checker.IsNil)
-	c.Assert(res.StatusCode, checker.Equals, http.StatusNoContent)
-	b.Close()
-}
-
-// #14640
-func (s *DockerSuite) TestPostContainersStartWithLinksInHostConfig(c *check.C) {
-	// TODO Windows: Windows doesn't support supplying a hostconfig on start.
-	// An alternate test could be written to validate the negative testing aspect of this
-	testRequires(c, DaemonIsLinux)
-	name := "test-host-config-links"
-	dockerCmd(c, "run", "--name", "foo", "-d", "busybox", "top")
-	dockerCmd(c, "create", "--name", name, "--link", "foo:bar", "busybox", "top")
-
-	hc := inspectFieldJSON(c, name, "HostConfig")
-	config := `{"HostConfig":` + hc + `}`
-
-	res, b, err := sockRequestRaw("POST", "/containers/"+name+"/start", strings.NewReader(config), "application/json")
-	c.Assert(err, checker.IsNil)
-	c.Assert(res.StatusCode, checker.Equals, http.StatusNoContent)
-	b.Close()
-}
-
-// #14640
-func (s *DockerSuite) TestPostContainersStartWithLinksInHostConfigIdLinked(c *check.C) {
-	// Windows does not support links
-	testRequires(c, DaemonIsLinux)
-	name := "test-host-config-links"
-	out, _ := dockerCmd(c, "run", "--name", "link0", "-d", "busybox", "top")
-	id := strings.TrimSpace(out)
-	dockerCmd(c, "create", "--name", name, "--link", id, "busybox", "top")
-
-	hc := inspectFieldJSON(c, name, "HostConfig")
-	config := `{"HostConfig":` + hc + `}`
-
-	res, b, err := sockRequestRaw("POST", "/containers/"+name+"/start", strings.NewReader(config), "application/json")
-	c.Assert(err, checker.IsNil)
-	c.Assert(res.StatusCode, checker.Equals, http.StatusNoContent)
-	b.Close()
-}
-
 // #14915
 func (s *DockerSuite) TestContainerApiCreateNoHostConfig118(c *check.C) {
 	config := struct {
@@ -1434,23 +1247,6 @@ func (s *DockerSuite) TestPostContainersCreateWithWrongCpusetValues(c *check.C)
 	c.Assert(string(body), checker.Equals, expected)
 }
 
-func (s *DockerSuite) TestStartWithNilDNS(c *check.C) {
-	// TODO Windows: Add once DNS is supported
-	testRequires(c, DaemonIsLinux)
-	out, _ := dockerCmd(c, "create", "busybox")
-	containerID := strings.TrimSpace(out)
-
-	config := `{"HostConfig": {"Dns": null}}`
-
-	res, b, err := sockRequestRaw("POST", "/containers/"+containerID+"/start", strings.NewReader(config), "application/json")
-	c.Assert(err, checker.IsNil)
-	c.Assert(res.StatusCode, checker.Equals, http.StatusNoContent)
-	b.Close()
-
-	dns := inspectFieldJSON(c, containerID, "HostConfig.Dns")
-	c.Assert(dns, checker.Equals, "[]")
-}
-
 func (s *DockerSuite) TestPostContainersCreateShmSizeNegative(c *check.C) {
 	// ShmSize is not supported on Windows
 	testRequires(c, DaemonIsLinux)
diff --git a/integration-cli/docker_cli_network_unix_test.go b/integration-cli/docker_cli_network_unix_test.go
@@ -1366,26 +1366,6 @@ func (s *DockerSuite) TestUserDefinedNetworkConnectDisconnectLink(c *check.C) {
 	c.Assert(err, check.IsNil)
 }
 
-// #19100 This is a deprecated feature test, it should be removed in Docker 1.12
-func (s *DockerNetworkSuite) TestDockerNetworkStartAPIWithHostconfig(c *check.C) {
-	netName := "test"
-	conName := "foo"
-	dockerCmd(c, "network", "create", netName)
-	dockerCmd(c, "create", "--name", conName, "busybox", "top")
-
-	config := map[string]interface{}{
-		"HostConfig": map[string]interface{}{
-			"NetworkMode": netName,
-		},
-	}
-	_, _, err := sockRequest("POST", "/containers/"+conName+"/start", config)
-	c.Assert(err, checker.IsNil)
-	c.Assert(waitRun(conName), checker.IsNil)
-	networks := inspectField(c, conName, "NetworkSettings.Networks")
-	c.Assert(networks, checker.Contains, netName, check.Commentf(fmt.Sprintf("Should contain '%s' network", netName)))
-	c.Assert(networks, checker.Not(checker.Contains), "bridge", check.Commentf("Should not contain 'bridge' network"))
-}
-
 func (s *DockerNetworkSuite) TestDockerNetworkDisconnectDefault(c *check.C) {
 	netWorkName1 := "test1"
 	netWorkName2 := "test2"
diff --git a/integration-cli/docker_deprecated_api_v124_test.go b/integration-cli/docker_deprecated_api_v124_test.go
diff --git a/integration-cli/docker_deprecated_api_v124_unix_test.go b/integration-cli/docker_deprecated_api_v124_unix_test.go
