Merge pull request #6649 from vieux/fix_api_return_codes_start_stop

return 304 is status isn't modified in start and stop

api/server/server.go

@@ -693,8 +693,11 @@ func postContainersStart(eng *engine.Engine, version version.Version, w http.Res
 	if vars == nil {
 		return fmt.Errorf("Missing parameter")
 	}
-	name := vars["name"]
-	job := eng.Job("start", name)
+	var (
+		name = vars["name"]
+		job  = eng.Job("start", name)
+	)
+
 	// allow a nil body for backwards compatibility
 	if r.Body != nil {
 		if api.MatchesContentType(r.Header.Get("Content-Type"), "application/json") {
@@ -704,6 +707,10 @@ func postContainersStart(eng *engine.Engine, version version.Version, w http.Res
 		}
 	}
 	if err := job.Run(); err != nil {
+		if err.Error() == "Container already started" {
+			w.WriteHeader(http.StatusNotModified)
+			return nil
+		}
 		return err
 	}
 	w.WriteHeader(http.StatusNoContent)
@@ -720,6 +727,10 @@ func postContainersStop(eng *engine.Engine, version version.Version, w http.Resp
 	job := eng.Job("stop", vars["name"])
 	job.Setenv("t", r.Form.Get("t"))
 	if err := job.Run(); err != nil {
+		if err.Error() == "Container already stopped" {
+			w.WriteHeader(http.StatusNotModified)
+			return nil
+		}
 		return err
 	}
 	w.WriteHeader(http.StatusNoContent)

docs/sources/reference/api/docker_remote_api.md

@@ -21,7 +21,7 @@ page_keywords: API, Docker, rcli, REST, documentation
 The current version of the API is v1.13
 
 Calling `/images/<name>/insert` is the same as calling
-`/v1.12/images/<name>/insert`.
+`/v1.13/images/<name>/insert`.
 
 You can still call an old version of the API using
 `/v1.12/images/<name>/insert`.
@@ -38,6 +38,12 @@ You can still call an old version of the API using
 `Sockets` parameter added to the `/info` endpoint listing all the sockets the 
 daemon is configured to listen on.
 
+`POST /containers/(name)/start`
+`POST /containers/(name)/stop`
+
+**New!**
+`start` and `stop` will now return 304 if the container's status is not modified
+
 ## v1.12
 
 ### Full Documentation

docs/sources/reference/api/docker_remote_api_v1.13.md

@@ -429,6 +429,7 @@ Start the container `id`
     Status Codes:
 
     -   **204** – no error
+    -   **304** – container already started
     -   **404** – no such container
     -   **500** – server error
 
@@ -455,6 +456,7 @@ Stop the container `id`
     Status Codes:
 
     -   **204** – no error
+    -   **304** – container already stopped
     -   **404** – no such container
     -   **500** – server error
 

integration/api_test.go

@@ -650,21 +650,24 @@ func TestPostContainersStart(t *testing.T) {
 	}
 
 	containerAssertExists(eng, containerID, t)
-	// Give some time to the process to start
-	// FIXME: use Wait once it's available as a job
-	containerWaitTimeout(eng, containerID, t)
-	if !containerRunning(eng, containerID, t) {
-		t.Errorf("Container should be running")
+
+	req, err = http.NewRequest("POST", "/containers/"+containerID+"/start", bytes.NewReader(hostConfigJSON))
+	if err != nil {
+		t.Fatal(err)
 	}
 
+	req.Header.Set("Content-Type", "application/json")
+
 	r = httptest.NewRecorder()
 	if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
 		t.Fatal(err)
 	}
-	// Starting an already started container should return an error
-	// FIXME: verify a precise error code. There is a possible bug here
-	// which causes this to return 404 even though the container exists.
-	assertHttpError(r, t)
+
+	// Starting an already started container should return a 304
+	assertHttpNotError(r, t)
+	if r.Code != http.StatusNotModified {
+		t.Fatalf("%d NOT MODIFIER expected, received %d\n", http.StatusNotModified, r.Code)
+	}
 	containerAssertExists(eng, containerID, t)
 	containerKill(eng, containerID, t)
 }
@@ -743,6 +746,22 @@ func TestPostContainersStop(t *testing.T) {
 	if containerRunning(eng, containerID, t) {
 		t.Fatalf("The container hasn't been stopped")
 	}
+
+	req, err = http.NewRequest("POST", "/containers/"+containerID+"/stop?t=1", bytes.NewReader([]byte{}))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	r = httptest.NewRecorder()
+	if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
+		t.Fatal(err)
+	}
+
+	// Stopping an already stopper container should return a 304
+	assertHttpNotError(r, t)
+	if r.Code != http.StatusNotModified {
+		t.Fatalf("%d NOT MODIFIER expected, received %d\n", http.StatusNotModified, r.Code)
+	}
 }
 
 func TestPostContainersWait(t *testing.T) {

server/server.go

@@ -2046,6 +2046,11 @@ func (srv *Server) ContainerStart(job *engine.Job) engine.Status {
 	if container == nil {
 		return job.Errorf("No such container: %s", name)
 	}
+
+	if container.State.IsRunning() {
+		return job.Errorf("Container already started")
+	}
+
 	// If no environment was set, then no hostconfig was passed.
 	if len(job.Environ()) > 0 {
 		hostConfig := runconfig.ContainerHostConfigFromJob(job)
@@ -2099,6 +2104,9 @@ func (srv *Server) ContainerStop(job *engine.Job) engine.Status {
 		t = job.GetenvInt("t")
 	}
 	if container := srv.daemon.Get(name); container != nil {
+		if !container.State.IsRunning() {
+			return job.Errorf("Container already stopped")
+		}
 		if err := container.Stop(int(t)); err != nil {
 			return job.Errorf("Cannot stop container %s: %s\n", name, err)
 		}