Skip to content

Commit

Permalink
pd-ctl: escape the scheduler name for some commands (#7799)
Browse files Browse the repository at this point in the history 
close #7798

Escape the scheduler name for some commands.

Signed-off-by: JmPotato <ghzpotato@gmail.com>
  • Loading branch information
@JmPotato
JmPotato committed Feb 4, 2024
1 parent 54ffd34 commit 21ced9a
Show file tree
Hide file tree
Showing 3 changed files with 6,878 additions and 69 deletions.
13 changes: 10 additions & 3 deletions tools/pd-ctl/pdctl/command/scheduler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,18 @@ func pauseSchedulerCommandFunc(cmd *cobra.Command, args []string) {
cmd.Usage()
return
}
path := schedulersPrefix + "/" + args[0]
path := schedulersPrefix + "/" + getEscapedSchedulerName(args[0])
input := map[string]any{"delay": delay}
postJSON(cmd, path, input)
}

// Since certain scheduler's name is defined by caller such as scatter-range,
// it's possible the name contains special characters, like "#", "&" and so on.
// So we need to escape the scheduler name here before attaching it to the URL.
func getEscapedSchedulerName(schedulerName string) string {
return url.PathEscape(schedulerName)
}

// NewResumeSchedulerCommand returns a command to resume a scheduler.
func NewResumeSchedulerCommand() *cobra.Command {
c := &cobra.Command{
Expand All @@ -92,7 +99,7 @@ func resumeSchedulerCommandFunc(cmd *cobra.Command, args []string) {
cmd.Usage()
return
}
path := schedulersPrefix + "/" + args[0]
path := schedulersPrefix + "/" + getEscapedSchedulerName(args[0])
input := map[string]any{"delay": 0}
postJSON(cmd, path, input)
}
Expand Down Expand Up @@ -475,7 +482,7 @@ func removeSchedulerCommandFunc(cmd *cobra.Command, args []string) {
case strings.HasPrefix(args[0], grantLeaderSchedulerName) && args[0] != grantLeaderSchedulerName:
redirectRemoveSchedulerToDeleteConfig(cmd, grantLeaderSchedulerName, args)
default:
path := schedulersPrefix + "/" + args[0]
path := schedulersPrefix + "/" + getEscapedSchedulerName(args[0])
_, err := doRequest(cmd, path, http.MethodDelete, http.Header{})
if err != nil {
cmd.Println(err)
Expand Down
130 changes: 64 additions & 66 deletions tools/pd-ctl/tests/scheduler/scheduler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ func (suite *schedulerTestSuite) SetupSuite() {
"balance-leader-scheduler",
"balance-region-scheduler",
"balance-hot-region-scheduler",
"balance-witness-scheduler",
"transfer-witness-leader-scheduler",
"evict-slow-store-scheduler",
}
}
Expand Down Expand Up @@ -171,23 +169,19 @@ func (suite *schedulerTestSuite) checkScheduler(cluster *pdTests.TestCluster) {

// scheduler show command
expected := map[string]bool{
"balance-region-scheduler": true,
"balance-leader-scheduler": true,
"balance-hot-region-scheduler": true,
"transfer-witness-leader-scheduler": true,
"balance-witness-scheduler": true,
"evict-slow-store-scheduler": true,
"balance-region-scheduler": true,
"balance-leader-scheduler": true,
"balance-hot-region-scheduler": true,
"evict-slow-store-scheduler": true,
}
checkSchedulerCommand(nil, expected)

// scheduler delete command
args := []string{"-u", pdAddr, "scheduler", "remove", "balance-region-scheduler"}
expected = map[string]bool{
"balance-leader-scheduler": true,
"balance-hot-region-scheduler": true,
"transfer-witness-leader-scheduler": true,
"balance-witness-scheduler": true,
"evict-slow-store-scheduler": true,
"balance-leader-scheduler": true,
"balance-hot-region-scheduler": true,
"evict-slow-store-scheduler": true,
}
checkSchedulerCommand(args, expected)

Expand Down Expand Up @@ -228,12 +222,10 @@ func (suite *schedulerTestSuite) checkScheduler(cluster *pdTests.TestCluster) {
// scheduler add command
args = []string{"-u", pdAddr, "scheduler", "add", schedulers[idx], "2"}
expected = map[string]bool{
"balance-leader-scheduler": true,
"balance-hot-region-scheduler": true,
schedulers[idx]: true,
"transfer-witness-leader-scheduler": true,
"balance-witness-scheduler": true,
"evict-slow-store-scheduler": true,
"balance-leader-scheduler": true,
"balance-hot-region-scheduler": true,
schedulers[idx]: true,
"evict-slow-store-scheduler": true,
}
checkSchedulerCommand(args, expected)

Expand All @@ -246,12 +238,10 @@ func (suite *schedulerTestSuite) checkScheduler(cluster *pdTests.TestCluster) {
// scheduler config update command
args = []string{"-u", pdAddr, "scheduler", "config", schedulers[idx], "add-store", "3"}
expected = map[string]bool{
"balance-leader-scheduler": true,
"balance-hot-region-scheduler": true,
schedulers[idx]: true,
"transfer-witness-leader-scheduler": true,
"balance-witness-scheduler": true,
"evict-slow-store-scheduler": true,
"balance-leader-scheduler": true,
"balance-hot-region-scheduler": true,
schedulers[idx]: true,
"evict-slow-store-scheduler": true,
}

// check update success
Expand All @@ -263,37 +253,31 @@ func (suite *schedulerTestSuite) checkScheduler(cluster *pdTests.TestCluster) {
// scheduler delete command
args = []string{"-u", pdAddr, "scheduler", "remove", schedulers[idx]}
expected = map[string]bool{
"balance-leader-scheduler": true,
"balance-hot-region-scheduler": true,
"transfer-witness-leader-scheduler": true,
"balance-witness-scheduler": true,
"evict-slow-store-scheduler": true,
"balance-leader-scheduler": true,
"balance-hot-region-scheduler": true,
"evict-slow-store-scheduler": true,
}
checkSchedulerCommand(args, expected)
checkStorePause([]uint64{}, schedulers[idx])

// scheduler add command
args = []string{"-u", pdAddr, "scheduler", "add", schedulers[idx], "2"}
expected = map[string]bool{
"balance-leader-scheduler": true,
"balance-hot-region-scheduler": true,
schedulers[idx]: true,
"transfer-witness-leader-scheduler": true,
"balance-witness-scheduler": true,
"evict-slow-store-scheduler": true,
"balance-leader-scheduler": true,
"balance-hot-region-scheduler": true,
schedulers[idx]: true,
"evict-slow-store-scheduler": true,
}
checkSchedulerCommand(args, expected)
checkStorePause([]uint64{2}, schedulers[idx])

// scheduler add command twice
args = []string{"-u", pdAddr, "scheduler", "add", schedulers[idx], "4"}
expected = map[string]bool{
"balance-leader-scheduler": true,
"balance-hot-region-scheduler": true,
schedulers[idx]: true,
"transfer-witness-leader-scheduler": true,
"balance-witness-scheduler": true,
"evict-slow-store-scheduler": true,
"balance-leader-scheduler": true,
"balance-hot-region-scheduler": true,
schedulers[idx]: true,
"evict-slow-store-scheduler": true,
}
checkSchedulerCommand(args, expected)

Expand All @@ -305,12 +289,10 @@ func (suite *schedulerTestSuite) checkScheduler(cluster *pdTests.TestCluster) {
// scheduler remove command [old]
args = []string{"-u", pdAddr, "scheduler", "remove", schedulers[idx] + "-4"}
expected = map[string]bool{
"balance-leader-scheduler": true,
"balance-hot-region-scheduler": true,
schedulers[idx]: true,
"transfer-witness-leader-scheduler": true,
"balance-witness-scheduler": true,
"evict-slow-store-scheduler": true,
"balance-leader-scheduler": true,
"balance-hot-region-scheduler": true,
schedulers[idx]: true,
"evict-slow-store-scheduler": true,
}
checkSchedulerCommand(args, expected)

Expand All @@ -322,24 +304,20 @@ func (suite *schedulerTestSuite) checkScheduler(cluster *pdTests.TestCluster) {
// scheduler remove command, when remove the last store, it should remove whole scheduler
args = []string{"-u", pdAddr, "scheduler", "remove", schedulers[idx] + "-2"}
expected = map[string]bool{
"balance-leader-scheduler": true,
"balance-hot-region-scheduler": true,
"transfer-witness-leader-scheduler": true,
"balance-witness-scheduler": true,
"evict-slow-store-scheduler": true,
"balance-leader-scheduler": true,
"balance-hot-region-scheduler": true,
"evict-slow-store-scheduler": true,
}
checkSchedulerCommand(args, expected)
checkStorePause([]uint64{}, schedulers[idx])
}

// test shuffle region config
checkSchedulerCommand([]string{"-u", pdAddr, "scheduler", "add", "shuffle-region-scheduler"}, map[string]bool{
"balance-leader-scheduler": true,
"balance-hot-region-scheduler": true,
"shuffle-region-scheduler": true,
"transfer-witness-leader-scheduler": true,
"balance-witness-scheduler": true,
"evict-slow-store-scheduler": true,
"balance-leader-scheduler": true,
"balance-hot-region-scheduler": true,
"shuffle-region-scheduler": true,
"evict-slow-store-scheduler": true,
})
var roles []string
mustExec(re, cmd, []string{"-u", pdAddr, "scheduler", "config", "shuffle-region-scheduler", "show-roles"}, &roles)
Expand All @@ -355,13 +333,11 @@ func (suite *schedulerTestSuite) checkScheduler(cluster *pdTests.TestCluster) {

// test grant hot region scheduler config
checkSchedulerCommand([]string{"-u", pdAddr, "scheduler", "add", "grant-hot-region-scheduler", "1", "1,2,3"}, map[string]bool{
"balance-leader-scheduler": true,
"balance-hot-region-scheduler": true,
"shuffle-region-scheduler": true,
"grant-hot-region-scheduler": true,
"transfer-witness-leader-scheduler": true,
"balance-witness-scheduler": true,
"evict-slow-store-scheduler": true,
"balance-leader-scheduler": true,
"balance-hot-region-scheduler": true,
"shuffle-region-scheduler": true,
"grant-hot-region-scheduler": true,
"evict-slow-store-scheduler": true,
})
var conf3 map[string]any
expected3 := map[string]any{
Expand Down Expand Up @@ -601,6 +577,28 @@ func (suite *schedulerTestSuite) checkScheduler(cluster *pdTests.TestCluster) {
})
}

// test scatter range scheduler
for _, name := range []string{
"test", "test#", "?test",
/* TODO: to handle case like "tes&t", we need to modify the server's JSON render to unescape the HTML characters */
} {
echo = mustExec(re, cmd, []string{"-u", pdAddr, "scheduler", "add", "scatter-range", "--format=raw", "a", "b", name}, nil)
re.Contains(echo, "Success!")
schedulerName := fmt.Sprintf("scatter-range-%s", name)
// test show scheduler
testutil.Eventually(re, func() bool {
echo = mustExec(re, cmd, []string{"-u", pdAddr, "scheduler", "show"}, nil)
return strings.Contains(echo, schedulerName)
})
// test remove scheduler
echo = mustExec(re, cmd, []string{"-u", pdAddr, "scheduler", "remove", schedulerName}, nil)
re.Contains(echo, "Success!")
testutil.Eventually(re, func() bool {
echo = mustExec(re, cmd, []string{"-u", pdAddr, "scheduler", "show"}, nil)
return !strings.Contains(echo, schedulerName)
})
}

mustUsage([]string{"-u", pdAddr, "scheduler", "pause", "balance-leader-scheduler"})
echo = mustExec(re, cmd, []string{"-u", pdAddr, "scheduler", "pause", "balance-leader-scheduler", "60"}, nil)
re.Contains(echo, "Success!")
Expand Down
Loading

0 comments on commit 21ced9a

Please sign in to comment.