diff --git a/errors.toml b/errors.toml index b60b84076e8..6caee5de3d8 100644 --- a/errors.toml +++ b/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 diff --git a/pkg/autoscaling/prometheus_test.go b/pkg/autoscaling/prometheus_test.go index 6d4a27b0411..6c30e3ead4c 100644 --- a/pkg/autoscaling/prometheus_test.go +++ b/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 diff --git a/pkg/errs/errno.go b/pkg/errs/errno.go index fa13e579116..728e473ccfe 100644 --- a/pkg/errs/errno.go +++ b/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 diff --git a/pkg/tso/keyspace_group_manager.go b/pkg/tso/keyspace_group_manager.go index 30de38bf20c..2ef6a041686 100644 --- a/pkg/tso/keyspace_group_manager.go +++ b/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()) diff --git a/pkg/utils/apiutil/apiutil.go b/pkg/utils/apiutil/apiutil.go index 0467bac9b04..56e635f083f 100644 --- a/pkg/utils/apiutil/apiutil.go +++ b/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 { diff --git a/pkg/utils/testutil/api_check.go b/pkg/utils/testutil/api_check.go index d11d575967d..84af97f828d 100644 --- a/pkg/utils/testutil/api_check.go +++ b/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 } diff --git a/server/api/admin_test.go b/server/api/admin_test.go index 1f2b386eb98..6a972171e1f 100644 --- a/server/api/admin_test.go +++ b/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"}`), diff --git a/server/api/diagnostic_test.go b/server/api/diagnostic_test.go index 8a39b2e0007..1774c221539 100644 --- a/server/api/diagnostic_test.go +++ b/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) } diff --git a/server/api/operator_test.go b/server/api/operator_test.go index 54f816c264e..1b86072b2c8 100644 --- a/server/api/operator_test.go +++ b/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) } } diff --git a/server/api/region_label_test.go b/server/api/region_label_test.go index 021ec7f1359..fd7401b83e0 100644 --- a/server/api/region_label_test.go +++ b/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) diff --git a/server/api/rule_test.go b/server/api/rule_test.go index 4cea1523401..d2dc50f1119 100644 --- a/server/api/rule_test.go +++ b/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 diff --git a/server/api/scheduler.go b/server/api/scheduler.go index cf1c82c658b..b612dd335ee 100644 --- a/server/api/scheduler.go +++ b/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 diff --git a/server/api/scheduler_test.go b/server/api/scheduler_test.go index 026d7a3cd2f..63c643b2a20 100644 --- a/server/api/scheduler_test.go +++ b/server/api/scheduler_test.go @@ -25,7 +25,6 @@ import ( "github.com/pingcap/kvproto/pkg/metapb" "github.com/stretchr/testify/suite" sc "github.com/tikv/pd/pkg/schedule/config" - "github.com/tikv/pd/pkg/utils/apiutil" tu "github.com/tikv/pd/pkg/utils/testutil" "github.com/tikv/pd/server" ) @@ -91,7 +90,7 @@ func (suite *scheduleTestSuite) TestOriginAPI() { suite.NoError(tu.ReadGetJSON(re, testDialClient, listURL, &resp)) suite.Len(resp["store-id-ranges"], 2) deleteURL := fmt.Sprintf("%s/%s", suite.urlPrefix, "evict-leader-scheduler-1") - _, err = apiutil.DoDelete(testDialClient, deleteURL) + err = tu.CheckDelete(testDialClient, deleteURL, tu.StatusOK(re)) suite.NoError(err) suite.Len(rc.GetSchedulers(), 1) resp1 := make(map[string]interface{}) @@ -99,18 +98,16 @@ func (suite *scheduleTestSuite) TestOriginAPI() { suite.Len(resp1["store-id-ranges"], 1) deleteURL = fmt.Sprintf("%s/%s", suite.urlPrefix, "evict-leader-scheduler-2") suite.NoError(failpoint.Enable("github.com/tikv/pd/server/config/persistFail", "return(true)")) - statusCode, err := apiutil.DoDelete(testDialClient, deleteURL) + err = tu.CheckDelete(testDialClient, deleteURL, tu.Status(re, http.StatusInternalServerError)) suite.NoError(err) - suite.Equal(500, statusCode) suite.Len(rc.GetSchedulers(), 1) suite.NoError(failpoint.Disable("github.com/tikv/pd/server/config/persistFail")) - statusCode, err = apiutil.DoDelete(testDialClient, deleteURL) + err = tu.CheckDelete(testDialClient, deleteURL, tu.StatusOK(re)) suite.NoError(err) - suite.Equal(200, statusCode) suite.Empty(rc.GetSchedulers()) - suite.NoError(tu.CheckGetJSON(testDialClient, listURL, nil, tu.Status(re, 404))) - statusCode, _ = apiutil.DoDelete(testDialClient, deleteURL) - suite.Equal(404, statusCode) + suite.NoError(tu.CheckGetJSON(testDialClient, listURL, nil, tu.Status(re, http.StatusNotFound))) + err = tu.CheckDelete(testDialClient, deleteURL, tu.Status(re, http.StatusNotFound)) + suite.NoError(err) } func (suite *scheduleTestSuite) TestAPI() { @@ -294,15 +291,14 @@ func (suite *scheduleTestSuite) TestAPI() { // using /pd/v1/schedule-config/grant-leader-scheduler/config to delete exists store from grant-leader-scheduler deleteURL := fmt.Sprintf("%s%s%s/%s/delete/%s", suite.svr.GetAddr(), apiPrefix, server.SchedulerConfigHandlerPath, name, "2") - _, err = apiutil.DoDelete(testDialClient, deleteURL) + err = tu.CheckDelete(testDialClient, deleteURL, tu.StatusOK(re)) suite.NoError(err) resp = make(map[string]interface{}) suite.NoError(tu.ReadGetJSON(re, testDialClient, listURL, &resp)) delete(exceptMap, "2") suite.Equal(exceptMap, resp["store-id-ranges"]) - statusCode, err := apiutil.DoDelete(testDialClient, deleteURL) + err = tu.CheckDelete(testDialClient, deleteURL, tu.Status(re, http.StatusNotFound)) suite.NoError(err) - suite.Equal(404, statusCode) }, }, { @@ -357,16 +353,15 @@ func (suite *scheduleTestSuite) TestAPI() { suite.Equal(exceptMap, resp["store-id-ranges"]) // using /pd/v1/schedule-config/evict-leader-scheduler/config to delete exist store from evict-leader-scheduler - deleteURL := fmt.Sprintf("%s%s%s/%s/delete/%s", suite.svr.GetAddr(), apiPrefix, server.SchedulerConfigHandlerPath, name, "2") - _, err = apiutil.DoDelete(testDialClient, deleteURL) + deleteURL := fmt.Sprintf("%s%s%s/%s/delete/%s", suite.svr.GetAddr(), apiPrefix, server.SchedulerConfigHandlerPath, name, "4") + err = tu.CheckDelete(testDialClient, deleteURL, tu.StatusOK(re)) suite.NoError(err) resp = make(map[string]interface{}) suite.NoError(tu.ReadGetJSON(re, testDialClient, listURL, &resp)) - delete(exceptMap, "2") + delete(exceptMap, "4") suite.Equal(exceptMap, resp["store-id-ranges"]) - statusCode, err := apiutil.DoDelete(testDialClient, deleteURL) + err = tu.CheckDelete(testDialClient, deleteURL, tu.Status(re, http.StatusNotFound)) suite.NoError(err) - suite.Equal(404, statusCode) }, }, } @@ -509,7 +504,7 @@ func (suite *scheduleTestSuite) addScheduler(body []byte) { func (suite *scheduleTestSuite) deleteScheduler(createdName string) { deleteURL := fmt.Sprintf("%s/%s", suite.urlPrefix, createdName) - _, err := apiutil.DoDelete(testDialClient, deleteURL) + err := tu.CheckDelete(testDialClient, deleteURL, tu.StatusOK(suite.Require())) suite.NoError(err) } diff --git a/server/api/service_gc_safepoint_test.go b/server/api/service_gc_safepoint_test.go index fe52204dfb2..517a94c2e23 100644 --- a/server/api/service_gc_safepoint_test.go +++ b/server/api/service_gc_safepoint_test.go @@ -16,7 +16,6 @@ package api import ( "fmt" - "net/http" "testing" "time" @@ -93,9 +92,8 @@ func (suite *serviceGCSafepointTestSuite) TestServiceGCSafepoint() { suite.NoError(err) suite.Equal(list, listResp) - statusCode, err := apiutil.DoDelete(testDialClient, sspURL+"/a") + err = testutil.CheckDelete(testDialClient, sspURL+"/a", testutil.StatusOK(suite.Require())) suite.NoError(err) - suite.Equal(http.StatusOK, statusCode) left, err := storage.LoadAllServiceGCSafePoints() suite.NoError(err)