Skip to content

Commit

Permalink
add more check
Browse files Browse the repository at this point in the history
Signed-off-by: lhy1024 <[email protected]>
  • Loading branch information
lhy1024 committed Jan 13, 2025
1 parent 0ba586c commit 1a815b9
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 8 deletions.
32 changes: 26 additions & 6 deletions tools/pd-ctl/pdctl/command/config_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import (

"github.com/spf13/cobra"

sc "github.com/tikv/pd/pkg/schedule/config"
"github.com/tikv/pd/pkg/schedule/placement"
"github.com/tikv/pd/pkg/utils/apiutil"
"github.com/tikv/pd/pkg/utils/reflectutil"
Expand Down Expand Up @@ -374,13 +373,10 @@ func postConfigDataWithPath(cmd *cobra.Command, key, value, path string) error {
if err != nil {
val = value
}
data[key] = val
if key == "max-replicas" {
replica, err := strconv.ParseInt(value, 10, 64)
if err == nil && replica < sc.DefaultMaxReplicas {
cmd.Println("Setting max-replica to less than 3 may be a mistake and carries high risk. Please confirm the setting.")
}
checkMaxReplicas(cmd, val)
}
data[key] = val
reqData, err := json.Marshal(data)
if err != nil {
return err
Expand All @@ -393,6 +389,30 @@ func postConfigDataWithPath(cmd *cobra.Command, key, value, path string) error {
return nil
}

func checkMaxReplicas(cmd *cobra.Command, value any) {
newReplica, ok := value.(float64)
if !ok {
// If the type is not float64, it will be handled elsewhere
return
}

Check warning on line 397 in tools/pd-ctl/pdctl/command/config_command.go

View check run for this annotation

Codecov / codecov/patch

tools/pd-ctl/pdctl/command/config_command.go#L395-L397

Added lines #L395 - L397 were not covered by tests
header := buildHeader(cmd)
r, err := doRequest(cmd, replicatePrefix, http.MethodGet, header)
if err != nil {
cmd.Printf("Failed to get config when checking config: %s\n", err)
return
}

Check warning on line 403 in tools/pd-ctl/pdctl/command/config_command.go

View check run for this annotation

Codecov / codecov/patch

tools/pd-ctl/pdctl/command/config_command.go#L401-L403

Added lines #L401 - L403 were not covered by tests
oldConfig := make(map[string]any)
err = json.Unmarshal([]byte(r), &oldConfig)
if err != nil {
cmd.Printf("Failed to unmarshal config when checking config: %s\n", err)
return
}

Check warning on line 409 in tools/pd-ctl/pdctl/command/config_command.go

View check run for this annotation

Codecov / codecov/patch

tools/pd-ctl/pdctl/command/config_command.go#L407-L409

Added lines #L407 - L409 were not covered by tests
oldReplica, ok := oldConfig["max-replicas"].(float64)
if ok && newReplica < oldReplica {
cmd.Printf("Setting max-replica to %v which is less than the current replicas (%v). This may pose a risk. Please confirm the setting.\n", newReplica, oldReplica)
}
}

func setConfigCommandFunc(cmd *cobra.Command, args []string) {
if len(args) != 2 {
cmd.Println(cmd.UsageString())
Expand Down
4 changes: 2 additions & 2 deletions tools/pd-ctl/tests/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1136,7 +1136,7 @@ func (suite *configTestSuite) checkUpdateDefaultReplicaConfig(cluster *pdTests.T
output, err := tests.ExecuteCommand(cmd, "-u", pdAddr, "config", "set", "max-replicas", "2")
re.NoError(err)
re.Contains(string(output), "Success!")
re.Contains(string(output), "Setting max-replica to less than 3 may be a mistake and carries high risk. Please confirm the setting.")
re.Contains(string(output), "which is less than the current replicas")
checkMaxReplicas(2)
output, err = tests.ExecuteCommand(cmd, "-u", pdAddr, "config", "set", "location-labels", "zone,host")
re.NoError(err)
Expand All @@ -1157,7 +1157,7 @@ func (suite *configTestSuite) checkUpdateDefaultReplicaConfig(cluster *pdTests.T
output, err = tests.ExecuteCommand(cmd, "-u", pdAddr, "config", "set", "max-replicas", "3")
re.NoError(err)
re.Contains(string(output), "Success!")
re.NotContains(string(output), "Setting max-replica to less than 3 may be a mistake and carries high risk. Please confirm the setting.")
re.NotContains(string(output), "which is less than the current replicas")
checkMaxReplicas(3)
checkRuleCount(3)

Expand Down

0 comments on commit 1a815b9

Please sign in to comment.