Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pd-ctl: add config replicate and show all config #573

Merged
merged 13 commits into from
Mar 27, 2017
142 changes: 114 additions & 28 deletions pdctl/command/config_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,58 @@ import (
"encoding/json"
"fmt"
"net/http"
"reflect"
"strconv"
"strings"

"github.com/pingcap/pd/server"
"github.com/spf13/cobra"
)

const (
scheduleOpt = iota
replicateOpt
)

type field struct {
tag string
typ reflect.Kind
kind int
}

var (
configPrefix = "pd/api/v1/config"
schedulePrefix = "pd/api/v1/config/schedule"
configPrefix = "pd/api/v1/config"
schedulePrefix = "pd/api/v1/config/schedule"
replicatePrefix = "pd/api/v1/config/replicate"
optionRel = make(map[string]field)
)

func init() {
s := server.ScheduleConfig{}
r := server.ReplicationConfig{}
fs := dumpConfig(s)
for _, f := range fs {
f.kind = scheduleOpt
optionRel[f.tag] = f
}
fs = dumpConfig(r)
for _, f := range fs {
f.kind = replicateOpt
optionRel[f.tag] = f
}
}

func dumpConfig(v interface{}) (ret []field) {
var f field
val := reflect.ValueOf(v)
for i := 0; i < val.Type().NumField(); i++ {
f.tag = val.Type().Field(i).Tag.Get("json")
f.typ = val.Type().Field(i).Type.Kind()
ret = append(ret, f)
}
return ret
}

// NewConfigCommand return a config subcommand of rootCmd
func NewConfigCommand() *cobra.Command {
conf := &cobra.Command{
Expand All @@ -46,6 +88,17 @@ func NewShowConfigCommand() *cobra.Command {
Short: "show config of PD",
Run: showConfigCommandFunc,
}
sc.AddCommand(NewShowAllConfigCommand())
return sc
}

// NewShowAllConfigCommand return a show all subcommand of show subcommand
func NewShowAllConfigCommand() *cobra.Command {
sc := &cobra.Command{
Use: "all",
Short: "show all config of PD",
Run: showAllConfigCommandFunc,
}
return sc
}

Expand All @@ -68,50 +121,83 @@ func showConfigCommandFunc(cmd *cobra.Command, args []string) {
fmt.Println(r)
}

func showAllConfigCommandFunc(cmd *cobra.Command, args []string) {
r, err := doRequest(cmd, configPrefix, http.MethodGet)
if err != nil {
fmt.Printf("Failed to get config: %s", err)
return
}
fmt.Println(r)
}

func setConfigCommandFunc(cmd *cobra.Command, args []string) {
if len(args) != 2 {
fmt.Println(cmd.UsageString())
return
}
var (
value interface{}
path string
)
opt, val := args[0], args[1]

url := getAddressFromCmd(cmd, schedulePrefix)
var value interface{}
data := make(map[string]interface{})
f, ok := optionRel[opt]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should parse args for different sub commands separately, it is more clear than using reflect here.

IMO, I don't like reflect here.

Copy link
Contributor Author

@nolouch nolouch Mar 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

like config set schedule <option> <value> and config set replicas <option> <value>? but we also need to distinguish the field type like string and []string in ReplicationConfig. I think this way is ok, it is no need to adjust when I add a field in config.

if !ok {
fmt.Println("Failed to set config: unknow option")
return
}
switch f.kind {
case scheduleOpt:
path = schedulePrefix
case replicateOpt:
path = replicatePrefix
}

r, err := http.Get(url)
r, err := doRequest(cmd, path, http.MethodGet)
if err != nil {
fmt.Printf("Failed to set config:[%s]\n", err)
fmt.Printf("Failed to set config: %s", err)
return
}
if r.StatusCode != http.StatusOK {
printResponseError(r)
r.Body.Close()
data := make(map[string]interface{})
err = json.Unmarshal([]byte(r), &data)
if err != nil {
fmt.Printf("Failed to set config: %s", err)
return
}

json.NewDecoder(r.Body).Decode(&data)
r.Body.Close()
value, err = strconv.ParseFloat(args[1], 64)
if err != nil {
value = args[1]
switch k := f.typ; true {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the purpose of this? I

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to convert the value to correct type, like LocationLabels are []string. the marshal need distinguish these.

Copy link
Contributor

@disksing disksing Mar 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer switch f.type here and list all numeric types instead of k <= reflect.Float64.

case k <= reflect.Float64:
value, err = strconv.ParseFloat(val, 64)
if err != nil {
fmt.Printf("Failed to set config: unsupport the option type %s", k)
return
}
case k == reflect.Slice:
value = []string{val}
if strings.Contains(val, ",") {
value = strings.Split(val, ",")
}
default:
value = val
}
data[args[0]] = value
data[opt] = value

req, err := json.Marshal(data)
reqData, err := json.Marshal(data)
if err != nil {
fmt.Printf("Failed to set config:[%s]\n", err)
fmt.Printf("Failed to set config: %s", err)
return
}

url = getAddressFromCmd(cmd, configPrefix)
r, err = http.Post(url, "application/json", bytes.NewBuffer(req))
if path == schedulePrefix {
path = configPrefix
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use schedulePrefix?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the API setSchedule and getSchedule are not consistent. considering compatible I just adjust it in here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's ok to give up supporting old pd-server.

req, err := getRequest(cmd, path, http.MethodPost, "application/json", bytes.NewBuffer(reqData))
if err != nil {
fmt.Printf("Failed to set config:[%s]\n", err)
fmt.Printf("Failed to set config: %s", err)
return
}
defer r.Body.Close()
if r.StatusCode == http.StatusOK {
fmt.Println("Success!")
} else {
printResponseError(r)
_, err = dail(req)
if err != nil {
fmt.Printf("Failed to set config: %s", err)
return
}
fmt.Println("Success!")
}
21 changes: 17 additions & 4 deletions pdctl/command/global.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,21 @@ var (
errInvalidAddr = errors.New("Invalid pd address, Cannot get connect to it")
)

func doRequest(cmd *cobra.Command, prefix string, method string) (string, error) {
var res string
func getRequest(cmd *cobra.Command, prefix string, method string, bodyType string, body io.Reader) (*http.Request, error) {
if method == "" {
method = http.MethodGet
}
url := getAddressFromCmd(cmd, prefix)
req, err := http.NewRequest(method, url, nil)
req, err := http.NewRequest(method, url, body)
if err != nil {
return res, err
return nil, err
}
req.Header.Set("Content-Type", bodyType)
return req, err
}

func dail(req *http.Request) (string, error) {
var res string
reps, err := dailClient.Do(req)
if err != nil {
return res, err
Expand All @@ -64,6 +69,14 @@ func doRequest(cmd *cobra.Command, prefix string, method string) (string, error)
return res, nil
}

func doRequest(cmd *cobra.Command, prefix string, method string) (string, error) {
req, err := getRequest(cmd, prefix, method, "", nil)
if err != nil {
return "", err
}
return dail(req)
}

func genResponseError(r *http.Response) error {
res, _ := ioutil.ReadAll(r.Body)
return errors.Errorf("[%d] %s", r.StatusCode, res)
Expand Down