-
Notifications
You must be signed in to change notification settings - Fork 82
Adding glustercli command to get options of all volumes #1406
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| package e2e | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/gluster/glusterd2/pkg/api" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func testGetClusterOptions(t *testing.T) { | ||
| r := require.New(t) | ||
| clusterOps, err := client.GetClusterOption() | ||
| r.Nil(err) | ||
| r.NotNil(clusterOps) | ||
| } | ||
|
|
||
| func ClusterOptionsSet(t *testing.T) { | ||
| r := require.New(t) | ||
| optReq := api.ClusterOptionReq{ | ||
| Options: map[string]string{"cluster.brick-multiplex": "on"}, | ||
| } | ||
| err := client.ClusterOptionSet(optReq) | ||
| r.Nil(err) | ||
| clusterOps, err := client.GetClusterOption() | ||
| for _, ops := range clusterOps { | ||
| if ops.Key == "cluster.brick-multiplex" { | ||
| r.Equal(ops.Value, "on") | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // TestClusterOption creates a cluster | ||
| func TestClusterOption(t *testing.T) { | ||
| var err error | ||
|
|
||
| r := require.New(t) | ||
|
|
||
| tc, err := setupCluster(t, "./config/1.toml", "./config/2.toml", "./config/3.toml") | ||
| r.Nil(err) | ||
| defer teardownCluster(tc) | ||
|
|
||
| client, err = initRestclient(tc.gds[0]) | ||
| r.Nil(err) | ||
| r.NotNil(client) | ||
|
|
||
| t.Run("Get Cluster Options", testGetClusterOptions) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| package cmd | ||
|
|
||
| import ( | ||
| "errors" | ||
| "fmt" | ||
| "os" | ||
|
|
||
| "github.com/gluster/glusterd2/pkg/api" | ||
|
|
||
| "github.com/olekukonko/tablewriter" | ||
| log "github.com/sirupsen/logrus" | ||
| "github.com/spf13/cobra" | ||
| ) | ||
|
|
||
| const ( | ||
| helpClusterOptionCmd = "Gluster Cluster Option Management" | ||
| helpClusterOptionSetCmd = "Set Cluster Option" | ||
| helpClusterOptionGetCmd = "Get Cluster Option" | ||
| ) | ||
|
|
||
| func init() { | ||
| clusterCmd.AddCommand(clusterOptionGetCmd) | ||
| clusterCmd.AddCommand(clusterOptionSetCmd) | ||
| } | ||
|
|
||
| var clusterCmd = &cobra.Command{ | ||
| Use: "cluster", | ||
| Short: helpClusterOptionCmd, | ||
| } | ||
|
|
||
| var clusterOptionSetCmd = &cobra.Command{ | ||
| Use: "set <option> <value> [<option> <value>]...", | ||
| Short: helpClusterOptionSetCmd, | ||
| Args: clusterSetCmdArgs, | ||
| Run: clusterSetCmdRun, | ||
| } | ||
|
|
||
| func clusterSetCmdArgs(cmd *cobra.Command, args []string) error { | ||
| // Ensure we have enough arguments for the command | ||
| if len(args) < 2 { | ||
| return errors.New("need at least 2 arguments") | ||
| } | ||
|
|
||
| // Ensure we have a proper option-value pairs | ||
| if (len(args) % 2) != 0 { | ||
| return errors.New("needs '<option> <value>' to be in pairs") | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func clusterSetCmdRun(cmd *cobra.Command, args []string) { | ||
| options := args[:] | ||
| if err := clusterOptionJSONHandler(cmd, options); err != nil { | ||
| if GlobalFlag.Verbose { | ||
| log.WithError(err).Error("cluster option set failed") | ||
| } | ||
| failure("Cluster option set failed", err, 1) | ||
| } else { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we can remove this else block, as failure does exit 1. |
||
| fmt.Println("Options set successfully") | ||
| } | ||
| } | ||
|
|
||
| func clusterOptionJSONHandler(cmd *cobra.Command, options []string) error { | ||
| copt := make(map[string]string) | ||
| for op, val := range options { | ||
| if op%2 == 0 { | ||
| copt[val] = options[op+1] | ||
| } | ||
| } | ||
|
|
||
| err := client.ClusterOptionSet(api.ClusterOptionReq{ | ||
| Options: copt}) | ||
| return err | ||
| } | ||
|
|
||
| var clusterOptionGetCmd = &cobra.Command{ | ||
| Use: "get", | ||
| Short: helpClusterOptionGetCmd, | ||
| Run: func(cmd *cobra.Command, args []string) { | ||
| table := tablewriter.NewWriter(os.Stdout) | ||
| opts, err := client.GetClusterOption() | ||
| if err != nil { | ||
| if GlobalFlag.Verbose { | ||
| log.WithError(err).Error("error getting cluster options") | ||
| } | ||
| failure("Error getting cluster options", err, 1) | ||
| } | ||
| table.SetHeader([]string{"Name", "Modified", "Value", "Default Value"}) | ||
| table.SetAlignment(tablewriter.ALIGN_LEFT) | ||
| for _, opt := range opts { | ||
| table.Append([]string{opt.Key, formatBoolYesNo(opt.Modified), opt.Value, opt.DefaultValue}) | ||
| } | ||
| table.Render() | ||
| }, | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -233,39 +233,55 @@ var volumeDeleteCmd = &cobra.Command{ | |
| } | ||
|
|
||
| var volumeGetCmd = &cobra.Command{ | ||
| Use: "get", | ||
| Use: "get <VOLNAME|all> <key|all>", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we rename
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what ever we chose here (In GD1's CLI it's referred as key though), request it to be used consistently across. |
||
| Short: helpVolumeGetCmd, | ||
| Args: cobra.ExactArgs(2), | ||
| Run: func(cmd *cobra.Command, args []string) { | ||
|
|
||
| volname := args[0] | ||
| optname := args[1] | ||
|
|
||
| opts, err := client.VolumeGet(volname, optname) | ||
| if err != nil { | ||
| log.WithError(err).Error("error getting volume options") | ||
| failure("Error getting volume options", err, 1) | ||
| } | ||
|
|
||
| table := tablewriter.NewWriter(os.Stdout) | ||
| table.SetHeader([]string{"Name", "Modified", "Value", "Default Value", "Option Level"}) | ||
| table.SetAlignment(tablewriter.ALIGN_LEFT) | ||
|
|
||
| for _, opt := range opts { | ||
| //if modified flag is set, discard unmodified options | ||
| if flagGetMdf && !opt.Modified { | ||
| continue | ||
| var volList []string | ||
| if volname == "all" { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what happens if the user creates a volume with the name
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good point, we need to reserve these keywords(
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice Catch, should I add this to volume-create? |
||
| volInfoList, err := client.Volumes("") | ||
| if len(volInfoList) <= 0 { | ||
| failure("No volumes found", err, 1) | ||
| } | ||
| for _, volInfo := range volInfoList { | ||
| volList = append(volList, volInfo.Name) | ||
| } | ||
| if flagGetBsc && opt.OptionLevel == "Basic" { | ||
| table.Append([]string{opt.OptName, formatBoolYesNo(opt.Modified), opt.Value, opt.DefaultValue, opt.OptionLevel}) | ||
| } else { | ||
| volList = append(volList, volname) | ||
| } | ||
| for _, volume := range volList { | ||
|
|
||
| fmt.Println("Volume :", volume) | ||
| opts, err := client.VolumeGet(volume, optname) | ||
| if err != nil { | ||
| log.WithError(err).Error("error getting volume options") | ||
| failure("Error getting volume options", err, 1) | ||
| } | ||
| if flagGetAdv && opt.OptionLevel == "Advanced" { | ||
| table.Append([]string{opt.OptName, formatBoolYesNo(opt.Modified), opt.Value, opt.DefaultValue, opt.OptionLevel}) | ||
| table = tablewriter.NewWriter(os.Stdout) | ||
| table.SetHeader([]string{"Name", "Modified", "Value", "Default Value", "Option Level"}) | ||
| table.SetAlignment(tablewriter.ALIGN_LEFT) | ||
|
|
||
| for _, opt := range opts { | ||
| //if modified flag is set, discard unmodified options | ||
| if flagGetMdf && !opt.Modified { | ||
| continue | ||
| } | ||
| if flagGetBsc && opt.OptionLevel == "Basic" { | ||
| table.Append([]string{opt.OptName, formatBoolYesNo(opt.Modified), opt.Value, opt.DefaultValue, opt.OptionLevel}) | ||
| } | ||
| if flagGetAdv && opt.OptionLevel == "Advanced" { | ||
| table.Append([]string{opt.OptName, formatBoolYesNo(opt.Modified), opt.Value, opt.DefaultValue, opt.OptionLevel}) | ||
|
|
||
| } | ||
| } | ||
| } | ||
| table.Render() | ||
| table.Render() | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. print one extra line after the table |
||
| fmt.Println("") | ||
|
|
||
| } | ||
| }, | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ package volumecommands | |
|
|
||
| import ( | ||
| "errors" | ||
| "fmt" | ||
| "net/http" | ||
| "path/filepath" | ||
| "strconv" | ||
|
|
@@ -42,6 +43,10 @@ func validateVolCreateReq(req *api.VolCreateReq) error { | |
| return gderrors.ErrInvalidVolName | ||
| } | ||
|
|
||
| if gutils.IsReservedKeyword(req.Name) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please add a e2e test case of this
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as mentioned for other case, please add this as well in the same issue
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. these code changes are in this PR so that I would like to see an e2e in this PR itself. |
||
| return fmt.Errorf("invalid name, volume name cannot be among reserved keywords:%v", gutils.ReservedKeywords) | ||
| } | ||
|
|
||
| if req.Transport != "" && req.Transport != "tcp" && req.Transport != "rdma" { | ||
| return errors.New("invalid transport. Supported values: tcp or rdma") | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,12 +27,10 @@ type validateFunc func(string, string) error | |
|
|
||
| // ClusterOptMap contains list of supported cluster-wide options, default values and value types | ||
| var ClusterOptMap = map[string]*ClusterOption{ | ||
| "cluster.shared-storage": {"cluster.shared-storage", "off", OptionTypeBool, nil}, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. any specific reason for removing this option?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shared-storage feature isn't a need in GD2 as of now and such feature doesn't exist yet. |
||
| "cluster.op-version": {"cluster.op-version", strconv.Itoa(gdctx.OpVersion), OptionTypeInt, nil}, | ||
| "cluster.max-op-version": {"cluster.max-op-version", strconv.Itoa(gdctx.OpVersion), OptionTypeInt, nil}, | ||
| "cluster.brick-multiplex": {"cluster.brick-multiplex", "off", OptionTypeBool, nil}, | ||
| "cluster.max-bricks-per-process": {"cluster.max-bricks-per-process", "250", OptionTypeInt, nil}, | ||
| "cluster.localtime-logging": {"cluster.localtime-logging", "off", OptionTypeBool, nil}, | ||
| } | ||
|
|
||
| // RegisterClusterOpValidationFunc registers a validation function for provided | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| package restclient | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "net/http" | ||
|
|
||
| "github.com/gluster/glusterd2/pkg/api" | ||
| ) | ||
|
|
||
| // GetClusterOption gets cluster level options | ||
| func (c *Client) GetClusterOption() ([]api.ClusterOptionsResp, error) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add e2e test case of this.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not a part of this PR, this PR only addresses glustercli commands, I will add e2e test cases for cluster option in separate PR, please raise an issue for it will fix that issue.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why you would like to add Client library in one PR and add e2e in other PR. what the concern here? |
||
| url := fmt.Sprintf("/v1/cluster/options") | ||
| var resp []api.ClusterOptionsResp | ||
| err := c.get(url, nil, http.StatusOK, &resp) | ||
| return resp, err | ||
| } | ||
|
|
||
| // ClusterOptionSet sets cluster level options | ||
| func (c *Client) ClusterOptionSet(req api.ClusterOptionReq) error { | ||
| url := fmt.Sprintf("/v1/cluster/options") | ||
| return c.post(url, req, http.StatusOK, nil) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,6 +31,18 @@ const ( | |
| TiB = 1024 * GiB | ||
| ) | ||
|
|
||
| var ( | ||
| // ReservedKeywords are Glusterd2 reserved keywords and must not be used as | ||
| // resource(volume, peer etc) name to avoid conflict in glustercli. | ||
| ReservedKeywords = []string{"all", "volume", "status", "list", "cluster", "gluster", "brick"} | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add comment for why we are reserving these keywords
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reserved key words is self explanatory
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wanted to see, why we are reserving these keywords, this will help the developers who will join later. |
||
| ) | ||
|
|
||
| // IsReservedKeyword returns true if the resource name is one of the | ||
| // GD2 Reserved Keyword | ||
| func IsReservedKeyword(resourceName string) bool { | ||
| return StringInSlice(resourceName, ReservedKeywords) | ||
| } | ||
|
|
||
| // IsLocalAddress checks whether a given host/IP is local | ||
| // Does lookup only after string matching IP addresses | ||
| func IsLocalAddress(address string) (bool, error) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
am not able to find out where this is getting called?