util: add check delete json function (tikv#7113)

ref tikv#4399

Signed-off-by: lhy1024 <admin@liudos.us>

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

errors.toml

@@ -526,21 +526,6 @@ error = '''
 plugin is not found: %s
 '''
 
-["PD:operator:ErrRegionAbnormalPeer"]
-error = '''
-region %v has abnormal peer
-'''
-
-["PD:operator:ErrRegionNotAdjacent"]
-error = '''
-two regions are not adjacent
-'''
-
-["PD:operator:ErrRegionNotFound"]
-error = '''
-region %v not found
-'''
-
 ["PD:os:ErrOSOpen"]
 error = '''
 open error
@@ -611,6 +596,21 @@ error = '''
 failed to unmarshal proto
 '''
 
+["PD:region:ErrRegionAbnormalPeer"]
+error = '''
+region %v has abnormal peer
+'''
+
+["PD:region:ErrRegionNotAdjacent"]
+error = '''
+two regions are not adjacent
+'''
+
+["PD:region:ErrRegionNotFound"]
+error = '''
+region %v not found
+'''
+
 ["PD:region:ErrRegionRuleContent"]
 error = '''
 invalid region rule content, %s

pkg/autoscaling/prometheus_test.go

@@ -155,7 +155,7 @@ func makeJSONResponse(promResp *response) (*http.Response, []byte, error) {
 
  response := &http.Response{
  Status: "200 OK",
- StatusCode: 200,
+ StatusCode: http.StatusOK,
  Proto: "HTTP/1.1",
  ProtoMajor: 1,
  ProtoMinor: 1,
@@ -246,7 +246,7 @@ func (c *errorHTTPStatusClient) Do(_ context.Context, req *http.Request) (r *htt
 
  r, body, err = makeJSONResponse(promResp)
 
- r.StatusCode = 500
+ r.StatusCode = http.StatusInternalServerError
  r.Status = "500 Internal Server Error"
 
  return

pkg/errs/errno.go

@@ -102,11 +102,11 @@ var (
 // region errors
 var (
  // ErrRegionNotAdjacent is error info for region not adjacent.
- ErrRegionNotAdjacent = errors.Normalize("two regions are not adjacent", errors.RFCCodeText("PD:operator:ErrRegionNotAdjacent"))
+ ErrRegionNotAdjacent = errors.Normalize("two regions are not adjacent", errors.RFCCodeText("PD:region:ErrRegionNotAdjacent"))
  // ErrRegionNotFound is error info for region not found.
- ErrRegionNotFound = errors.Normalize("region %v not found", errors.RFCCodeText("PD:operator:ErrRegionNotFound"))
+ ErrRegionNotFound = errors.Normalize("region %v not found", errors.RFCCodeText("PD:region:ErrRegionNotFound"))
  // ErrRegionAbnormalPeer is error info for region has abnormal peer.
- ErrRegionAbnormalPeer = errors.Normalize("region %v has abnormal peer", errors.RFCCodeText("PD:operator:ErrRegionAbnormalPeer"))
+ ErrRegionAbnormalPeer = errors.Normalize("region %v has abnormal peer", errors.RFCCodeText("PD:region:ErrRegionAbnormalPeer"))
 )
 
 // plugin errors

pkg/tso/keyspace_group_manager.go

@@ -1226,16 +1226,17 @@ func (kgm *KeyspaceGroupManager) finishSplitKeyspaceGroup(id uint32) error {
  return nil
  }
  startRequest := time.Now()
- statusCode, err := apiutil.DoDelete(
+ resp, err := apiutil.DoDelete(
  kgm.httpClient,
  kgm.cfg.GeBackendEndpoints()+keyspaceGroupsAPIPrefix+fmt.Sprintf("/%d/split", id))
  if err != nil {
  return err
  }
- if statusCode != http.StatusOK {
+ defer resp.Body.Close()
+ if resp.StatusCode != http.StatusOK {
  log.Warn("failed to finish split keyspace group",
  zap.Uint32("keyspace-group-id", id),
- zap.Int("status-code", statusCode))
+ zap.Int("status-code", resp.StatusCode))
  return errs.ErrSendRequest.FastGenByArgs()
  }
  kgm.metrics.finishSplitSendDuration.Observe(time.Since(startRequest).Seconds())
@@ -1264,16 +1265,17 @@ func (kgm *KeyspaceGroupManager) finishMergeKeyspaceGroup(id uint32) error {
  return nil
  }
  startRequest := time.Now()
- statusCode, err := apiutil.DoDelete(
+ resp, err := apiutil.DoDelete(
  kgm.httpClient,
  kgm.cfg.GeBackendEndpoints()+keyspaceGroupsAPIPrefix+fmt.Sprintf("/%d/merge", id))
  if err != nil {
  return err
  }
- if statusCode != http.StatusOK {
+ defer resp.Body.Close()
+ if resp.StatusCode != http.StatusOK {
  log.Warn("failed to finish merging keyspace group",
  zap.Uint32("keyspace-group-id", id),
- zap.Int("status-code", statusCode))
+ zap.Int("status-code", resp.StatusCode))
  return errs.ErrSendRequest.FastGenByArgs()
  }
  kgm.metrics.finishMergeSendDuration.Observe(time.Since(startRequest).Seconds())

pkg/utils/apiutil/apiutil.go

@@ -226,17 +226,12 @@ func PostJSONIgnoreResp(client *http.Client, url string, data []byte) error {
 }
 
 // DoDelete is used to send delete request and return http response code.
-func DoDelete(client *http.Client, url string) (int, error) {
+func DoDelete(client *http.Client, url string) (*http.Response, error) {
  req, err := http.NewRequest(http.MethodDelete, url, nil)
  if err != nil {
- return http.StatusBadRequest, err
- }
- res, err := client.Do(req)
- if err != nil {
- return 0, err
+ return nil, err
  }
- defer res.Body.Close()
- return res.StatusCode, nil
+ return client.Do(req)
 }
 
 func checkResponse(resp *http.Response, err error) error {

pkg/utils/testutil/api_check.go

@@ -123,9 +123,18 @@ func CheckPatchJSON(client *http.Client, url string, data []byte, checkOpts ...f
  return checkResp(resp, checkOpts...)
 }
 
+// CheckDelete is used to do delete request and do check options.
+func CheckDelete(client *http.Client, url string, checkOpts ...func([]byte, int, http.Header)) error {
+ resp, err := apiutil.DoDelete(client, url)
+ if err != nil {
+ return err
+ }
+ return checkResp(resp, checkOpts...)
+}
+
 func checkResp(resp *http.Response, checkOpts ...func([]byte, int, http.Header)) error {
  res, err := io.ReadAll(resp.Body)
- resp.Body.Close()
+ defer resp.Body.Close()
  if err != nil {
  return err
  }

server/api/admin_test.go

@@ -26,7 +26,6 @@ import (
  "github.com/pingcap/kvproto/pkg/pdpb"
  "github.com/stretchr/testify/suite"
  "github.com/tikv/pd/pkg/core"
- "github.com/tikv/pd/pkg/utils/apiutil"
  tu "github.com/tikv/pd/pkg/utils/testutil"
  "github.com/tikv/pd/server"
 )
@@ -271,9 +270,8 @@ func (suite *adminTestSuite) TestMarkSnapshotRecovering() {
  suite.NoError(err2)
  suite.True(resp.Marked)
  // unmark
- code, err := apiutil.DoDelete(testDialClient, url)
+ err := tu.CheckDelete(testDialClient, url, tu.StatusOK(re))
  suite.NoError(err)
- suite.Equal(200, code)
  suite.NoError(tu.CheckGetJSON(testDialClient, url, nil,
  tu.StatusOK(re), tu.StringContain(re, "false")))
 }
@@ -310,9 +308,8 @@ func (suite *adminTestSuite) TestRecoverAllocID() {
  suite.NoError(err2)
  suite.Equal(id, uint64(99000001))
  // unmark
- code, err := apiutil.DoDelete(testDialClient, markRecoveringURL)
+ err := tu.CheckDelete(testDialClient, markRecoveringURL, tu.StatusOK(re))
  suite.NoError(err)
- suite.Equal(200, code)
  suite.NoError(tu.CheckGetJSON(testDialClient, markRecoveringURL, nil,
  tu.StatusOK(re), tu.StringContain(re, "false")))
  suite.NoError(tu.CheckPostJSON(testDialClient, url, []byte(`{"id": "100000"}`),

server/api/diagnostic_test.go

@@ -24,7 +24,6 @@ import (
  "github.com/stretchr/testify/suite"
  "github.com/tikv/pd/pkg/core"
  "github.com/tikv/pd/pkg/schedule/schedulers"
- "github.com/tikv/pd/pkg/utils/apiutil"
  tu "github.com/tikv/pd/pkg/utils/testutil"
  "github.com/tikv/pd/server"
  "github.com/tikv/pd/server/config"
@@ -129,7 +128,7 @@ func (suite *diagnosticTestSuite) TestSchedulerDiagnosticAPI() {
  suite.checkStatus("normal", balanceRegionURL)
 
  deleteURL := fmt.Sprintf("%s/%s", suite.schedulerPrifex, schedulers.BalanceRegionName)
- _, err = apiutil.DoDelete(testDialClient, deleteURL)
+ err = tu.CheckDelete(testDialClient, deleteURL, tu.StatusOK(re))
  suite.NoError(err)
  suite.checkStatus("disabled", balanceRegionURL)
 }

server/api/operator_test.go

@@ -33,7 +33,6 @@ import (
  "github.com/tikv/pd/pkg/mock/mockhbstream"
  pdoperator "github.com/tikv/pd/pkg/schedule/operator"
  "github.com/tikv/pd/pkg/schedule/placement"
- "github.com/tikv/pd/pkg/utils/apiutil"
  tu "github.com/tikv/pd/pkg/utils/testutil"
  "github.com/tikv/pd/pkg/versioninfo"
  "github.com/tikv/pd/server"
@@ -99,7 +98,7 @@ func (suite *operatorTestSuite) TestAddRemovePeer() {
  suite.Contains(operator, "add learner peer 1 on store 3")
  suite.Contains(operator, "RUNNING")
 
- _, err = apiutil.DoDelete(testDialClient, regionURL)
+ err = tu.CheckDelete(testDialClient, regionURL, tu.StatusOK(re))
  suite.NoError(err)
  records = mustReadURL(re, recordURL)
  suite.Contains(records, "admin-add-peer {add peer: store [3]}")
@@ -110,7 +109,7 @@ func (suite *operatorTestSuite) TestAddRemovePeer() {
  suite.Contains(operator, "RUNNING")
  suite.Contains(operator, "remove peer on store 2")
 
- _, err = apiutil.DoDelete(testDialClient, regionURL)
+ err = tu.CheckDelete(testDialClient, regionURL, tu.StatusOK(re))
  suite.NoError(err)
  records = mustReadURL(re, recordURL)
  suite.Contains(records, "admin-remove-peer {rm peer: store [2]}")
@@ -400,8 +399,10 @@ func (suite *transferRegionOperatorTestSuite) TestTransferRegionWithPlacementRul
  if len(testCase.expectSteps) > 0 {
  operator = mustReadURL(re, regionURL)
  suite.Contains(operator, testCase.expectSteps)
+ err = tu.CheckDelete(testDialClient, regionURL, tu.StatusOK(re))
+ } else {
+ err = tu.CheckDelete(testDialClient, regionURL, tu.StatusNotOK(re))
  }
- _, err = apiutil.DoDelete(testDialClient, regionURL)
  suite.NoError(err)
  }
 }

server/api/region_label_test.go

@@ -24,7 +24,6 @@ import (
  "github.com/pingcap/failpoint"
  "github.com/stretchr/testify/suite"
  "github.com/tikv/pd/pkg/schedule/labeler"
- "github.com/tikv/pd/pkg/utils/apiutil"
  tu "github.com/tikv/pd/pkg/utils/testutil"
  "github.com/tikv/pd/server"
 )
@@ -86,7 +85,7 @@ func (suite *regionLabelTestSuite) TestGetSet() {
  expects := []*labeler.LabelRule{rules[0], rules[2]}
  suite.Equal(expects, resp)
 
- _, err = apiutil.DoDelete(testDialClient, suite.urlPrefix+"rule/"+url.QueryEscape("rule2/a/b"))
+ err = tu.CheckDelete(testDialClient, suite.urlPrefix+"rule/"+url.QueryEscape("rule2/a/b"), tu.StatusOK(re))
  suite.NoError(err)
  err = tu.ReadGetJSON(re, testDialClient, suite.urlPrefix+"rules", &resp)
  suite.NoError(err)

server/api/rule_test.go

@@ -26,7 +26,6 @@ import (
  "github.com/stretchr/testify/suite"
  "github.com/tikv/pd/pkg/core"
  "github.com/tikv/pd/pkg/schedule/placement"
- "github.com/tikv/pd/pkg/utils/apiutil"
  tu "github.com/tikv/pd/pkg/utils/testutil"
  "github.com/tikv/pd/server"
  "github.com/tikv/pd/server/config"
@@ -202,13 +201,13 @@ func (suite *ruleTestSuite) TestGet() {
  name: "found",
  rule: rule,
  found: true,
- code: 200,
+ code: http.StatusOK,
  },
  {
  name: "not found",
  rule: placement.Rule{GroupID: "a", ID: "30", StartKeyHex: "1111", EndKeyHex: "3333", Role: "voter", Count: 1},
  found: false,
- code: 404,
+ code: http.StatusNotFound,
  },
  }
  for _, testCase := range testCases {
@@ -533,9 +532,8 @@ func (suite *ruleTestSuite) TestDelete() {
  url := fmt.Sprintf("%s/rule/%s/%s", suite.urlPrefix, testCase.groupID, testCase.id)
  // clear suspect keyRanges to prevent test case from others
  suite.svr.GetRaftCluster().ClearSuspectKeyRanges()
- statusCode, err := apiutil.DoDelete(testDialClient, url)
+ err = tu.CheckDelete(testDialClient, url, tu.StatusOK(suite.Require()))
  suite.NoError(err)
- suite.Equal(http.StatusOK, statusCode)
  if len(testCase.popKeyRange) > 0 {
  popKeyRangeMap := map[string]struct{}{}
  for i := 0; i < len(testCase.popKeyRange)/2; i++ {
@@ -726,7 +724,7 @@ func (suite *ruleTestSuite) TestBundle() {
  suite.compareBundle(bundles[1], b2)
 
  // Delete
- _, err = apiutil.DoDelete(testDialClient, suite.urlPrefix+"/placement-rule/pd")
+ err = tu.CheckDelete(testDialClient, suite.urlPrefix+"/placement-rule/pd", tu.StatusOK(suite.Require()))
  suite.NoError(err)
 
  // GetAll again
@@ -753,7 +751,7 @@ func (suite *ruleTestSuite) TestBundle() {
  suite.compareBundle(bundles[2], b3)
 
  // Delete using regexp
- _, err = apiutil.DoDelete(testDialClient, suite.urlPrefix+"/placement-rule/"+url.PathEscape("foo.*")+"?regexp")
+ err = tu.CheckDelete(testDialClient, suite.urlPrefix+"/placement-rule/"+url.PathEscape("foo.*")+"?regexp", tu.StatusOK(suite.Require()))
  suite.NoError(err)
 
  // GetAll again

server/api/scheduler.go

@@ -15,8 +15,8 @@
 package api
 
 import (
- "fmt"
  "net/http"
+ "net/url"
  "strings"
  "time"
 
@@ -321,13 +321,18 @@ func (h *schedulerHandler) handleErr(w http.ResponseWriter, err error) {
 func (h *schedulerHandler) redirectSchedulerDelete(w http.ResponseWriter, name, schedulerName string) {
  args := strings.Split(name, "-")
  args = args[len(args)-1:]
- url := fmt.Sprintf("%s/%s/%s/delete/%s", h.GetAddr(), schedulerConfigPrefix, schedulerName, args[0])
- statusCode, err := apiutil.DoDelete(h.svr.GetHTTPClient(), url)
+ deleteURL, err := url.JoinPath(h.GetAddr(), "pd", server.SchedulerConfigHandlerPath, schedulerName, "delete", args[0])
  if err != nil {
- h.r.JSON(w, statusCode, err.Error())
+ h.r.JSON(w, http.StatusInternalServerError, err.Error())
+ return
+ }
+ resp, err := apiutil.DoDelete(h.svr.GetHTTPClient(), deleteURL)
+ if err != nil {
+ h.r.JSON(w, resp.StatusCode, err.Error())
  return
  }
- h.r.JSON(w, statusCode, nil)
+ defer resp.Body.Close()
+ h.r.JSON(w, resp.StatusCode, nil)
 }
 
 // FIXME: details of input json body params