From 6049691d3a8e06bfd3b66019091d14cab2ee905a Mon Sep 17 00:00:00 2001
From: Lynn <zimu_xia@126.com>
Date: Tue, 15 Aug 2023 10:48:50 +0800
Subject: [PATCH] *: address a comment

---
 server/handler/BUILD.bazel                |  3 ++-
 server/handler/tests/http_handler_test.go | 19 +++++++++++++------
 server/handler/upgradehandler.go          | 13 +++++++++----
 server/http_status.go                     |  2 +-
 4 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/server/handler/BUILD.bazel b/server/handler/BUILD.bazel
index 846c308ac6cb8..d443361eb22e6 100644
--- a/server/handler/BUILD.bazel
+++ b/server/handler/BUILD.bazel
@@ -23,11 +23,12 @@ go_library(
         "//tablecodec",
         "//types",
         "//util/codec",
-        "@com_github_gorilla_mux//:mux",
+        "//util/logutil",
         "@com_github_pingcap_errors//:errors",
         "@com_github_pingcap_failpoint//:failpoint",
         "@com_github_pingcap_kvproto//pkg/kvrpcpb",
         "@com_github_pingcap_kvproto//pkg/metapb",
         "@com_github_tikv_client_go_v2//tikv",
+        "@org_uber_go_zap//:zap",
     ],
 )
diff --git a/server/handler/tests/http_handler_test.go b/server/handler/tests/http_handler_test.go
index 8d1d691b028ec..fa6000261c093 100644
--- a/server/handler/tests/http_handler_test.go
+++ b/server/handler/tests/http_handler_test.go
@@ -1247,8 +1247,15 @@ func TestUpgrade(t *testing.T) {
 	ts.startServer(t)
 	defer ts.stopServer(t)
 
-	// test /upgrade/start
-	resp, err := ts.FetchStatus("/upgrade/start")
+	resp, err := ts.FetchStatus("/upgrade")
+	require.NoError(t, err)
+	require.Equal(t, http.StatusBadRequest, resp.StatusCode)
+	require.NoError(t, resp.Body.Close())
+
+	require.NoError(t, err)
+	require.NotNil(t, resp)
+	// test upgrade start
+	resp, err = ts.PostStatus("/upgrade", "application/x-www-form-urlencoded", bytes.NewBuffer([]byte(`op=start`)))
 	require.NoError(t, err)
 	require.Equal(t, http.StatusOK, resp.StatusCode)
 	b, err := httputil.DumpResponse(resp, true)
@@ -1266,7 +1273,7 @@ func TestUpgrade(t *testing.T) {
 	require.True(t, isUpgrading)
 
 	// Do start upgrade again.
-	resp, err = ts.FetchStatus("/upgrade/start")
+	resp, err = ts.PostStatus("/upgrade", "application/x-www-form-urlencoded", bytes.NewBuffer([]byte(`op=start`)))
 	require.NoError(t, err)
 	require.Equal(t, http.StatusOK, resp.StatusCode)
 	b, err = httputil.DumpResponse(resp, true)
@@ -1283,8 +1290,8 @@ func TestUpgrade(t *testing.T) {
 	require.NoError(t, err)
 	require.True(t, isUpgrading)
 
-	// test /upgrade/finish
-	resp, err = ts.FetchStatus("/upgrade/finish")
+	// test upgrade finish
+	resp, err = ts.PostStatus("/upgrade", "application/x-www-form-urlencoded", bytes.NewBuffer([]byte(`op=finish`)))
 	require.NoError(t, err)
 	require.Equal(t, http.StatusOK, resp.StatusCode)
 	b, err = httputil.DumpResponse(resp, true)
@@ -1302,7 +1309,7 @@ func TestUpgrade(t *testing.T) {
 	require.False(t, isUpgrading)
 
 	// Do finish upgrade again.
-	resp, err = ts.FetchStatus("/upgrade/finish")
+	resp, err = ts.PostStatus("/upgrade", "application/x-www-form-urlencoded", bytes.NewBuffer([]byte(`op=finish`)))
 	require.NoError(t, err)
 	require.Equal(t, http.StatusOK, resp.StatusCode)
 	b, err = httputil.DumpResponse(resp, true)
diff --git a/server/handler/upgradehandler.go b/server/handler/upgradehandler.go
index a86f990830e03..e3957b97568f2 100644
--- a/server/handler/upgradehandler.go
+++ b/server/handler/upgradehandler.go
@@ -17,10 +17,11 @@ package handler
 import (
 	"net/http"
 
-	"github.com/gorilla/mux"
 	"github.com/pingcap/errors"
 	"github.com/pingcap/tidb/kv"
 	"github.com/pingcap/tidb/session"
+	"github.com/pingcap/tidb/util/logutil"
+	"go.uber.org/zap"
 )
 
 // ClusterUpgradeHandler is the handler for upgrading cluster.
@@ -35,12 +36,14 @@ func NewClusterUpgradeHandler(store kv.Storage) *ClusterUpgradeHandler {
 
 // ServeHTTP handles request of ddl server info.
 func (h ClusterUpgradeHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
-	// parse params
-	params := mux.Vars(req)
+	if req.Method != http.MethodPost {
+		WriteError(w, errors.Errorf("This API only support POST method"))
+		return
+	}
 
 	var err error
 	var hasDone bool
-	op := params[Operation]
+	op := req.FormValue("op")
 	switch op {
 	case "start":
 		hasDone, err = h.startUpgrade()
@@ -64,6 +67,8 @@ func (h ClusterUpgradeHandler) ServeHTTP(w http.ResponseWriter, req *http.Reques
 		}
 	}
 	WriteData(w, "success!")
+	logutil.Logger(req.Context()).Info("[upgrading] upgrade op success",
+		zap.String("op", req.FormValue("op")), zap.Bool("hasDone", hasDone))
 }
 
 func (h ClusterUpgradeHandler) startUpgrade() (hasDone bool, err error) {
diff --git a/server/http_status.go b/server/http_status.go
index 7f6a2aa0126a7..4f9bd410322c6 100644
--- a/server/http_status.go
+++ b/server/http_status.go
@@ -248,7 +248,7 @@ func (s *Server) startHTTPServer() {
 	router.Handle("/tiflash/replica-deprecated", tikvhandler.NewFlashReplicaHandler(tikvHandlerTool))
 
 	// HTTP path for upgrade operations.
-	router.Handle("/upgrade/{op}", handler.NewClusterUpgradeHandler(tikvHandlerTool.Store.(kv.Storage))).Name("upgrade operations")
+	router.Handle("/upgrade", handler.NewClusterUpgradeHandler(tikvHandlerTool.Store.(kv.Storage))).Name("upgrade operations")
 
 	if s.cfg.Store == "tikv" {
 		// HTTP path for tikv.