server: Apply review comments and split basic handler

etcd-io · Mar 14, 2022 · ed13c11 · ed13c11
1 parent c38cfa9
commit ed13c11
Show file tree

Hide file tree

Showing 4 changed files with 11 additions and 12 deletions.
diff --git a/server/embed/etcd.go b/server/embed/etcd.go
@@ -701,7 +701,8 @@ func (e *Etcd) serveClients() (err error) {
 
 	// Start a client server goroutine for each listen address
 	mux := http.NewServeMux()
-	etcdhttp.HandleBasic(e.cfg.logger, mux, e.Server)
+	etcdhttp.HandleDebug(mux)
+	etcdhttp.HandleVersion(mux, e.Server)
 	etcdhttp.HandleMetrics(mux)
 	etcdhttp.HandleHealth(e.cfg.logger, mux, e.Server)
 

diff --git a/server/etcdserver/api/etcdhttp/health_test.go b/server/etcdserver/api/etcdhttp/health_test.go
@@ -115,26 +115,28 @@ func TestHealthHandler(t *testing.T) {
 			expectHealth:     "true",
 		},
 		{
+			name:             "Healthy even if authentication failed",
 			healthCheckURL:   "/health",
 			apiError:         auth.ErrUserEmpty,
 			expectStatusCode: http.StatusOK,
 			expectHealth:     "true",
 		},
 		{
-			healthCheckURL:   "/health",
+			name:             "Healthy even if authorization failed",
 			apiError:         auth.ErrPermissionDenied,
 			expectStatusCode: http.StatusOK,
 			expectHealth:     "true",
 		},
 		{
+			name:             "Unhealthy if api is not available",
 			healthCheckURL:   "/health",
 			apiError:         fmt.Errorf("Unexpected error"),
 			expectStatusCode: http.StatusServiceUnavailable,
 			expectHealth:     "false",
 		},
 	}
 
-	for i, tt := range tests {
+	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			mux := http.NewServeMux()
 			HandleHealth(zap.NewExample(), mux, &fakeHealthServer{
@@ -147,14 +149,14 @@ func TestHealthHandler(t *testing.T) {
 
 			res, err := ts.Client().Do(&http.Request{Method: http.MethodGet, URL: testutil.MustNewURL(t, ts.URL+tt.healthCheckURL)})
 			if err != nil {
-				t.Errorf("fail serve http request %s %v in test case #%d", tt.healthCheckURL, err, i+1)
+				t.Errorf("fail serve http request %s %v", tt.healthCheckURL, err)
 			}
 			if res == nil {
-				t.Errorf("got nil http response with http request %s in test case #%d", tt.healthCheckURL, i+1)
+				t.Errorf("got nil http response with http request %s", tt.healthCheckURL)
 				return
 			}
 			if res.StatusCode != tt.expectStatusCode {
-				t.Errorf("want statusCode %d but got %d in test case #%d", tt.expectStatusCode, res.StatusCode, i+1)
+				t.Errorf("want statusCode %d but got %d", tt.expectStatusCode, res.StatusCode)
 			}
 			health, err := parseHealthOutput(res.Body)
 			if err != nil {

diff --git a/server/etcdserver/api/etcdhttp/basic.go → server/etcdserver/api/etcdhttp/utils.go b/server/etcdserver/api/etcdhttp/basic.go → server/etcdserver/api/etcdhttp/utils.go
@@ -23,11 +23,6 @@ import (
 	"go.uber.org/zap"
 )
 
-func HandleBasic(lg *zap.Logger, mux *http.ServeMux, peer etcdserver.ServerPeer) {
-	HandleDebug(mux)
-	HandleVersion(mux, peer)
-}
-
 func allowMethod(w http.ResponseWriter, r *http.Request, m string) bool {
 	if m == r.Method {
 		return true

diff --git a/tests/framework/integration/cluster.go b/tests/framework/integration/cluster.go
@@ -955,7 +955,8 @@ func (m *Member) Launch() error {
 	}
 	for _, ln := range m.ClientListeners {
 		handler := http.NewServeMux()
-		etcdhttp.HandleBasic(m.Logger, handler, m.Server)
+		etcdhttp.HandleDebug(handler)
+		etcdhttp.HandleVersion(handler, m.Server)
 		etcdhttp.HandleMetrics(handler)
 		etcdhttp.HandleHealth(m.Logger, handler, m.Server)
 		hs := &httptest.Server{