Merge pull request #10071 from mota/cleanup-router

Remove error return type from createRouter and ServeRequest

api/server/server.go

@@ -1279,7 +1279,7 @@ func AttachProfiler(router *mux.Router) {
 	router.HandleFunc("/debug/pprof/threadcreate", pprof.Handler("threadcreate").ServeHTTP)
 }
 
-func createRouter(eng *engine.Engine, logging, enableCors bool, dockerVersion string) (*mux.Router, error) {
+func createRouter(eng *engine.Engine, logging, enableCors bool, dockerVersion string) *mux.Router {
 	r := mux.NewRouter()
 	if os.Getenv("DEBUG") != "" {
 		AttachProfiler(r)
@@ -1361,30 +1361,23 @@ func createRouter(eng *engine.Engine, logging, enableCors bool, dockerVersion st
 		}
 	}
 
-	return r, nil
+	return r
 }
 
 // ServeRequest processes a single http request to the docker remote api.
 // FIXME: refactor this to be part of Server and not require re-creating a new
 // router each time. This requires first moving ListenAndServe into Server.
-func ServeRequest(eng *engine.Engine, apiversion version.Version, w http.ResponseWriter, req *http.Request) error {
-	router, err := createRouter(eng, false, true, "")
-	if err != nil {
-		return err
-	}
+func ServeRequest(eng *engine.Engine, apiversion version.Version, w http.ResponseWriter, req *http.Request) {
+	router := createRouter(eng, false, true, "")
 	// Insert APIVERSION into the request as a convenience
 	req.URL.Path = fmt.Sprintf("/v%s%s", apiversion, req.URL.Path)
 	router.ServeHTTP(w, req)
-	return nil
 }
 
 // serveFd creates an http.Server and sets it up to serve given a socket activated
 // argument.
 func serveFd(addr string, job *engine.Job) error {
-	r, err := createRouter(job.Eng, job.GetenvBool("Logging"), job.GetenvBool("EnableCors"), job.Getenv("Version"))
-	if err != nil {
-		return err
-	}
+	r := createRouter(job.Eng, job.GetenvBool("Logging"), job.GetenvBool("EnableCors"), job.Getenv("Version"))
 
 	ls, e := systemd.ListenFD(addr)
 	if e != nil {
@@ -1496,10 +1489,7 @@ func setSocketGroup(addr, group string) error {
 }
 
 func setupUnixHttp(addr string, job *engine.Job) (*HttpServer, error) {
-	r, err := createRouter(job.Eng, job.GetenvBool("Logging"), job.GetenvBool("EnableCors"), job.Getenv("Version"))
-	if err != nil {
-		return nil, err
-	}
+	r := createRouter(job.Eng, job.GetenvBool("Logging"), job.GetenvBool("EnableCors"), job.Getenv("Version"))
 
 	if err := syscall.Unlink(addr); err != nil && !os.IsNotExist(err) {
 		return nil, err
@@ -1554,10 +1544,7 @@ func setupTcpHttp(addr string, job *engine.Job) (*HttpServer, error) {
 		log.Infof("/!\\ DON'T BIND ON ANOTHER IP ADDRESS THAN 127.0.0.1 IF YOU DON'T KNOW WHAT YOU'RE DOING /!\\")
 	}
 
-	r, err := createRouter(job.Eng, job.GetenvBool("Logging"), job.GetenvBool("EnableCors"), job.Getenv("Version"))
-	if err != nil {
-		return nil, err
-	}
+	r := createRouter(job.Eng, job.GetenvBool("Logging"), job.GetenvBool("EnableCors"), job.Getenv("Version"))
 
 	l, err := newListener("tcp", addr, job.GetenvBool("BufferRequests"))
 	if err != nil {

api/server/server_unit_test.go

@@ -484,9 +484,7 @@ func serveRequestUsingVersion(method, target string, version version.Version, bo
 	if err != nil {
 		t.Fatal(err)
 	}
-	if err := ServeRequest(eng, version, r, req); err != nil {
-		t.Fatal(err)
-	}
+	ServeRequest(eng, version, r, req)
 	return r
 }
 

integration/api_test.go

@@ -31,9 +31,7 @@ func TestSaveImageAndThenLoad(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
-		t.Fatal(err)
-	}
+	server.ServeRequest(eng, api.APIVERSION, r, req)
 	if r.Code != http.StatusOK {
 		t.Fatalf("%d OK expected, received %d\n", http.StatusOK, r.Code)
 	}
@@ -45,9 +43,7 @@ func TestSaveImageAndThenLoad(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
-		t.Fatal(err)
-	}
+	server.ServeRequest(eng, api.APIVERSION, r, req)
 	if r.Code != http.StatusOK {
 		t.Fatalf("%d OK expected, received %d\n", http.StatusOK, r.Code)
 	}
@@ -58,9 +54,7 @@ func TestSaveImageAndThenLoad(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
-		t.Fatal(err)
-	}
+	server.ServeRequest(eng, api.APIVERSION, r, req)
 	if r.Code != http.StatusNotFound {
 		t.Fatalf("%d NotFound expected, received %d\n", http.StatusNotFound, r.Code)
 	}
@@ -71,9 +65,7 @@ func TestSaveImageAndThenLoad(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
-		t.Fatal(err)
-	}
+	server.ServeRequest(eng, api.APIVERSION, r, req)
 	if r.Code != http.StatusOK {
 		t.Fatalf("%d OK expected, received %d\n", http.StatusOK, r.Code)
 	}
@@ -84,9 +76,7 @@ func TestSaveImageAndThenLoad(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
-		t.Fatal(err)
-	}
+	server.ServeRequest(eng, api.APIVERSION, r, req)
 	if r.Code != http.StatusOK {
 		t.Fatalf("%d OK expected, received %d\n", http.StatusOK, r.Code)
 	}
@@ -138,9 +128,7 @@ func TestGetContainersTop(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
-		t.Fatal(err)
-	}
+	server.ServeRequest(eng, api.APIVERSION, r, req)
 	assertHttpNotError(r, t)
 	var procs engine.Env
 	if err := procs.Decode(r.Body); err != nil {
@@ -189,9 +177,7 @@ func TestPostCommit(t *testing.T) {
 	}
 
 	r := httptest.NewRecorder()
-	if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
-		t.Fatal(err)
-	}
+	server.ServeRequest(eng, api.APIVERSION, r, req)
 	assertHttpNotError(r, t)
 	if r.Code != http.StatusCreated {
 		t.Fatalf("%d Created expected, received %d\n", http.StatusCreated, r.Code)
@@ -227,9 +213,7 @@ func TestPostContainersCreate(t *testing.T) {
 	req.Header.Set("Content-Type", "application/json")
 
 	r := httptest.NewRecorder()
-	if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
-		t.Fatal(err)
-	}
+	server.ServeRequest(eng, api.APIVERSION, r, req)
 	assertHttpNotError(r, t)
 	if r.Code != http.StatusCreated {
 		t.Fatalf("%d Created expected, received %d\n", http.StatusCreated, r.Code)
@@ -269,14 +253,12 @@ func TestPostJsonVerify(t *testing.T) {
 
 	r := httptest.NewRecorder()
 
-	if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
-		t.Fatal(err)
-	}
+	server.ServeRequest(eng, api.APIVERSION, r, req)
 
 	// Don't add Content-Type header
 	// req.Header.Set("Content-Type", "application/json")
 
-	err = server.ServeRequest(eng, api.APIVERSION, r, req)
+	server.ServeRequest(eng, api.APIVERSION, r, req)
 	if r.Code != http.StatusInternalServerError || !strings.Contains(((*r.Body).String()), "application/json") {
 		t.Fatal("Create should have failed due to no Content-Type header - got:", r)
 	}
@@ -284,9 +266,7 @@ func TestPostJsonVerify(t *testing.T) {
 	// Now add header but with wrong type and retest
 	req.Header.Set("Content-Type", "application/xml")
 
-	if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
-		t.Fatal(err)
-	}
+	server.ServeRequest(eng, api.APIVERSION, r, req)
 	if r.Code != http.StatusInternalServerError || !strings.Contains(((*r.Body).String()), "application/json") {
 		t.Fatal("Create should have failed due to wrong Content-Type header - got:", r)
 	}
@@ -331,9 +311,7 @@ func TestPostCreateNull(t *testing.T) {
 	req.Header.Set("Content-Type", "application/json")
 
 	r := httptest.NewRecorder()
-	if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
-		t.Fatal(err)
-	}
+	server.ServeRequest(eng, api.APIVERSION, r, req)
 	assertHttpNotError(r, t)
 	if r.Code != http.StatusCreated {
 		t.Fatalf("%d Created expected, received %d\n", http.StatusCreated, r.Code)
@@ -380,9 +358,7 @@ func TestPostContainersKill(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
-		t.Fatal(err)
-	}
+	server.ServeRequest(eng, api.APIVERSION, r, req)
 	assertHttpNotError(r, t)
 	if r.Code != http.StatusNoContent {
 		t.Fatalf("%d NO CONTENT expected, received %d\n", http.StatusNoContent, r.Code)
@@ -419,9 +395,7 @@ func TestPostContainersRestart(t *testing.T) {
 		t.Fatal(err)
 	}
 	r := httptest.NewRecorder()
-	if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
-		t.Fatal(err)
-	}
+	server.ServeRequest(eng, api.APIVERSION, r, req)
 	assertHttpNotError(r, t)
 	if r.Code != http.StatusNoContent {
 		t.Fatalf("%d NO CONTENT expected, received %d\n", http.StatusNoContent, r.Code)
@@ -461,9 +435,7 @@ func TestPostContainersStart(t *testing.T) {
 	req.Header.Set("Content-Type", "application/json")
 
 	r := httptest.NewRecorder()
-	if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
-		t.Fatal(err)
-	}
+	server.ServeRequest(eng, api.APIVERSION, r, req)
 	assertHttpNotError(r, t)
 	if r.Code != http.StatusNoContent {
 		t.Fatalf("%d NO CONTENT expected, received %d\n", http.StatusNoContent, r.Code)
@@ -479,9 +451,7 @@ func TestPostContainersStart(t *testing.T) {
 	req.Header.Set("Content-Type", "application/json")
 
 	r = httptest.NewRecorder()
-	if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
-		t.Fatal(err)
-	}
+	server.ServeRequest(eng, api.APIVERSION, r, req)
 
 	// Starting an already started container should return a 304
 	assertHttpNotError(r, t)
@@ -520,9 +490,7 @@ func TestPostContainersStop(t *testing.T) {
 		t.Fatal(err)
 	}
 	r := httptest.NewRecorder()
-	if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
-		t.Fatal(err)
-	}
+	server.ServeRequest(eng, api.APIVERSION, r, req)
 	assertHttpNotError(r, t)
 	if r.Code != http.StatusNoContent {
 		t.Fatalf("%d NO CONTENT expected, received %d\n", http.StatusNoContent, r.Code)
@@ -537,9 +505,7 @@ func TestPostContainersStop(t *testing.T) {
 	}
 
 	r = httptest.NewRecorder()
-	if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
-		t.Fatal(err)
-	}
+	server.ServeRequest(eng, api.APIVERSION, r, req)
 
 	// Stopping an already stopper container should return a 304
 	assertHttpNotError(r, t)
@@ -568,9 +534,7 @@ func TestPostContainersWait(t *testing.T) {
 		if err != nil {
 			t.Fatal(err)
 		}
-		if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
-			t.Fatal(err)
-		}
+		server.ServeRequest(eng, api.APIVERSION, r, req)
 		assertHttpNotError(r, t)
 		var apiWait engine.Env
 		if err := apiWait.Decode(r.Body); err != nil {
@@ -626,9 +590,7 @@ func TestPostContainersAttach(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
-			t.Fatal(err)
-		}
+		server.ServeRequest(eng, api.APIVERSION, r, req)
 		assertHttpNotError(r.ResponseRecorder, t)
 	}()
 
@@ -704,9 +666,7 @@ func TestPostContainersAttachStderr(t *testing.T) {
 			t.Fatal(err)
 		}
 
-		if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
-			t.Fatal(err)
-		}
+		server.ServeRequest(eng, api.APIVERSION, r, req)
 		assertHttpNotError(r.ResponseRecorder, t)
 	}()
 
@@ -751,9 +711,7 @@ func TestOptionsRoute(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
-		t.Fatal(err)
-	}
+	server.ServeRequest(eng, api.APIVERSION, r, req)
 	assertHttpNotError(r, t)
 	if r.Code != http.StatusOK {
 		t.Errorf("Expected response for OPTIONS request to be \"200\", %v found.", r.Code)
@@ -770,9 +728,7 @@ func TestGetEnabledCors(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
-		t.Fatal(err)
-	}
+	server.ServeRequest(eng, api.APIVERSION, r, req)
 	assertHttpNotError(r, t)
 	if r.Code != http.StatusOK {
 		t.Errorf("Expected response for OPTIONS request to be \"200\", %v found.", r.Code)
@@ -817,9 +773,7 @@ func TestDeleteImages(t *testing.T) {
 	}
 
 	r := httptest.NewRecorder()
-	if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
-		t.Fatal(err)
-	}
+	server.ServeRequest(eng, api.APIVERSION, r, req)
 	if r.Code != http.StatusConflict {
 		t.Fatalf("Expected http status 409-conflict, got %v", r.Code)
 	}
@@ -830,9 +784,7 @@ func TestDeleteImages(t *testing.T) {
 	}
 
 	r2 := httptest.NewRecorder()
-	if err := server.ServeRequest(eng, api.APIVERSION, r2, req2); err != nil {
-		t.Fatal(err)
-	}
+	server.ServeRequest(eng, api.APIVERSION, r2, req2)
 	assertHttpNotError(r2, t)
 	if r2.Code != http.StatusOK {
 		t.Fatalf("%d OK expected, received %d\n", http.StatusOK, r.Code)
@@ -882,9 +834,7 @@ func TestPostContainersCopy(t *testing.T) {
 		t.Fatal(err)
 	}
 	req.Header.Add("Content-Type", "application/json")
-	if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
-		t.Fatal(err)
-	}
+	server.ServeRequest(eng, api.APIVERSION, r, req)
 	assertHttpNotError(r, t)
 
 	if r.Code != http.StatusOK {
@@ -930,9 +880,7 @@ func TestPostContainersCopyWhenContainerNotFound(t *testing.T) {
 		t.Fatal(err)
 	}
 	req.Header.Add("Content-Type", "application/json")
-	if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
-		t.Fatal(err)
-	}
+	server.ServeRequest(eng, api.APIVERSION, r, req)
 	if r.Code != http.StatusNotFound {
 		t.Fatalf("404 expected for id_not_found Container, received %v", r.Code)
 	}
@@ -960,9 +908,7 @@ func TestConstainersStartChunkedEncodingHostConfig(t *testing.T) {
 	}
 
 	req.Header.Add("Content-Type", "application/json")
-	if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
-		t.Fatal(err)
-	}
+	server.ServeRequest(eng, api.APIVERSION, r, req)
 	assertHttpNotError(r, t)
 
 	var testData2 engine.Env
@@ -982,9 +928,7 @@ func TestConstainersStartChunkedEncodingHostConfig(t *testing.T) {
 	// Otherwise (just setting the Content-Encoding to chunked) net/http will overwrite
 	// http://golang.org/src/pkg/net/http/request.go?s=11980:12172
 	req.ContentLength = -1
-	if err := server.ServeRequest(eng, api.APIVERSION, r, req); err != nil {
-		t.Fatal(err)
-	}
+	server.ServeRequest(eng, api.APIVERSION, r, req)
 	assertHttpNotError(r, t)
 
 	type config struct {
@@ -1000,9 +944,7 @@ func TestConstainersStartChunkedEncodingHostConfig(t *testing.T) {
 
 	r2 := httptest.NewRecorder()
 	req.Header.Add("Content-Type", "application/json")
-	if err := server.ServeRequest(eng, api.APIVERSION, r2, req); err != nil {
-		t.Fatal(err)
-	}
+	server.ServeRequest(eng, api.APIVERSION, r2, req)
 	assertHttpNotError(r, t)
 
 	c := config{}