api: return [] rather than null for empty scheduler results (tikv…

…#7454) close tikv#7452 Signed-off-by: lhy1024 <[email protected]>
lhy1024 · Nov 29, 2023 · 871be59 · 871be59
1 parent 180ff57
commit 871be59
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 5 deletions.
diff --git a/pkg/schedule/handler/handler.go b/pkg/schedule/handler/handler.go
@@ -807,7 +807,7 @@ func (h *Handler) GetSchedulerByStatus(status string, needTS bool) (interface{},
 	schedulers := sc.GetSchedulerNames()
 	switch status {
 	case "paused":
-		var pausedSchedulers []string
+		pausedSchedulers := make([]string, 0, len(schedulers))
 		pausedPeriods := []schedulerPausedPeriod{}
 		for _, scheduler := range schedulers {
 			paused, err := sc.IsSchedulerPaused(scheduler)
@@ -842,7 +842,7 @@ func (h *Handler) GetSchedulerByStatus(status string, needTS bool) (interface{},
 		}
 		return pausedSchedulers, nil
 	case "disabled":
-		var disabledSchedulers []string
+		disabledSchedulers := make([]string, 0, len(schedulers))
 		for _, scheduler := range schedulers {
 			disabled, err := sc.IsSchedulerDisabled(scheduler)
 			if err != nil {
@@ -857,7 +857,7 @@ func (h *Handler) GetSchedulerByStatus(status string, needTS bool) (interface{},
 		// The default scheduler could not be deleted in scheduling server,
 		// so schedulers could only be disabled.
 		// We should not return the disabled schedulers here.
-		var enabledSchedulers []string
+		enabledSchedulers := make([]string, 0, len(schedulers))
 		for _, scheduler := range schedulers {
 			disabled, err := sc.IsSchedulerDisabled(scheduler)
 			if err != nil {

diff --git a/tests/pdctl/scheduler/scheduler_test.go b/tests/pdctl/scheduler/scheduler_test.go
@@ -545,7 +545,7 @@ func (suite *schedulerTestSuite) checkScheduler(cluster *tests.TestCluster) {
 	mustUsage([]string{"-u", pdAddr, "scheduler", "resume", "balance-leader-scheduler", "60"})
 	echo = mustExec(re, cmd, []string{"-u", pdAddr, "scheduler", "resume", "balance-leader-scheduler"}, nil)
 	re.Contains(echo, "Success!")
-	checkSchedulerWithStatusCommand("paused", nil)
+	checkSchedulerWithStatusCommand("paused", []string{})
 
 	// set label scheduler to disabled manually.
 	echo = mustExec(re, cmd, []string{"-u", pdAddr, "scheduler", "add", "label-scheduler"}, nil)
@@ -560,7 +560,7 @@ func (suite *schedulerTestSuite) checkScheduler(cluster *tests.TestCluster) {
 	cfg.Schedulers = origin
 	err = leaderServer.GetServer().SetScheduleConfig(*cfg)
 	re.NoError(err)
-	checkSchedulerWithStatusCommand("disabled", nil)
+	checkSchedulerWithStatusCommand("disabled", []string{})
 }
 
 func (suite *schedulerTestSuite) TestSchedulerDiagnostic() {

diff --git a/tests/server/api/scheduler_test.go b/tests/server/api/scheduler_test.go
@@ -17,6 +17,7 @@ package api
 import (
 	"encoding/json"
 	"fmt"
+	"io"
 	"net/http"
 	"reflect"
 	"testing"
@@ -28,6 +29,7 @@ import (
 	"github.com/stretchr/testify/suite"
 	sc "github.com/tikv/pd/pkg/schedule/config"
 	"github.com/tikv/pd/pkg/slice"
+	"github.com/tikv/pd/pkg/utils/apiutil"
 	tu "github.com/tikv/pd/pkg/utils/testutil"
 	"github.com/tikv/pd/server"
 	"github.com/tikv/pd/tests"
@@ -673,6 +675,40 @@ func (suite *scheduleTestSuite) testPauseOrResume(urlPrefix string, name, create
 	suite.False(isPaused)
 }
 
+func (suite *scheduleTestSuite) TestEmptySchedulers() {
+	env := tests.NewSchedulingTestEnvironment(suite.T())
+	env.RunTestInTwoModes(suite.checkEmptySchedulers)
+}
+
+func (suite *scheduleTestSuite) checkEmptySchedulers(cluster *tests.TestCluster) {
+	re := suite.Require()
+	leaderAddr := cluster.GetLeaderServer().GetAddr()
+	urlPrefix := fmt.Sprintf("%s/pd/api/v1/schedulers", leaderAddr)
+	for i := 1; i <= 4; i++ {
+		store := &metapb.Store{
+			Id:            uint64(i),
+			State:         metapb.StoreState_Up,
+			NodeState:     metapb.NodeState_Serving,
+			LastHeartbeat: time.Now().UnixNano(),
+		}
+		tests.MustPutStore(suite.Require(), cluster, store)
+	}
+
+	// test disabled and paused schedulers
+	suite.checkEmptySchedulersResp(urlPrefix + "?status=disabled")
+	suite.checkEmptySchedulersResp(urlPrefix + "?status=paused")
+
+	// test enabled schedulers
+	schedulers := make([]string, 0)
+	suite.NoError(tu.ReadGetJSON(re, testDialClient, urlPrefix, &schedulers))
+	for _, scheduler := range schedulers {
+		suite.deleteScheduler(urlPrefix, scheduler)
+	}
+	suite.NoError(tu.ReadGetJSON(re, testDialClient, urlPrefix, &schedulers))
+	suite.Len(schedulers, 0)
+	suite.checkEmptySchedulersResp(urlPrefix)
+}
+
 func (suite *scheduleTestSuite) assertSchedulerExists(re *require.Assertions, urlPrefix string, scheduler string) {
 	var schedulers []string
 	tu.Eventually(re, func() bool {
@@ -700,3 +736,14 @@ func (suite *scheduleTestSuite) isSchedulerPaused(urlPrefix, name string) bool {
 	}
 	return false
 }
+
+func (suite *scheduleTestSuite) checkEmptySchedulersResp(url string) {
+	resp, err := apiutil.GetJSON(testDialClient, url, nil)
+	suite.NoError(err)
+	defer resp.Body.Close()
+	suite.Equal(http.StatusOK, resp.StatusCode)
+	b, err := io.ReadAll(resp.Body)
+	suite.NoError(err)
+	suite.Contains(string(b), "[]")
+	suite.NotContains(string(b), "null")
+}