Skip to content
This repository was archived by the owner on Mar 26, 2020. It is now read-only.
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions e2e/cluster_test.go
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) {
Copy link
Member

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?

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)
}
9 changes: 9 additions & 0 deletions e2e/smartvol_ops_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,15 @@ func testSmartVolumeDistribute(t *testing.T) {
r.NotNil(err)

r.Nil(client.VolumeDelete(smartvolname))

createReq = api.VolCreateReq{
Name: "volume",
Size: 60 * gutils.MiB,
DistributeCount: 3,
}
volinfo, err = client.VolumeCreate(createReq)
r.NotNil(err)

checkZeroLvs(r)
}

Expand Down
96 changes: 96 additions & 0 deletions glustercli/cmd/cluster-options.go
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 {
Copy link
Member

Choose a reason for hiding this comment

The 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()
},
}
1 change: 1 addition & 0 deletions glustercli/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ func addSubCommands(rootCmd *cobra.Command) {
rootCmd.AddCommand(georepCmd)
rootCmd.AddCommand(snapshotCmd)
rootCmd.AddCommand(volumeCmd)
rootCmd.AddCommand(clusterCmd)
}

// GlustercliOption will have all global flags set during run time
Expand Down
58 changes: 37 additions & 21 deletions glustercli/cmd/volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,39 +233,55 @@ var volumeDeleteCmd = &cobra.Command{
}

var volumeGetCmd = &cobra.Command{
Use: "get",
Use: "get <VOLNAME|all> <key|all>",
Copy link
Member

Choose a reason for hiding this comment

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

can we rename key to option as we are getting option details?

Copy link
Contributor

Choose a reason for hiding this comment

The 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" {
Copy link
Member

Choose a reason for hiding this comment

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

what happens if the user creates a volume with the name all and want to fetch the volume option for that particular volume :D.

Copy link
Member

Choose a reason for hiding this comment

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

good point, we need to reserve these keywords(all, status, volume etc) and not allow volume create.

Copy link
Contributor Author

@rishubhjain rishubhjain Dec 14, 2018

Choose a reason for hiding this comment

The 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()
Copy link
Member

Choose a reason for hiding this comment

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

print one extra line after the table

fmt.Println("")

}
},
}

Expand Down
5 changes: 5 additions & 0 deletions glusterd2/commands/volumes/volume-create.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package volumecommands

import (
"errors"
"fmt"
"net/http"
"path/filepath"
"strconv"
Expand Down Expand Up @@ -42,6 +43,10 @@ func validateVolCreateReq(req *api.VolCreateReq) error {
return gderrors.ErrInvalidVolName
}

if gutils.IsReservedKeyword(req.Name) {
Copy link
Member

Choose a reason for hiding this comment

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

please add a e2e test case of this

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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

Copy link
Member

Choose a reason for hiding this comment

The 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")
}
Expand Down
2 changes: 0 additions & 2 deletions glusterd2/options/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Copy link
Member

Choose a reason for hiding this comment

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

any specific reason for removing this option?

Copy link
Contributor

Choose a reason for hiding this comment

The 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
Expand Down
22 changes: 22 additions & 0 deletions pkg/restclient/cluster.go
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) {
Copy link
Member

Choose a reason for hiding this comment

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

add e2e test case of this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Member

Choose a reason for hiding this comment

The 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?
even if you are adding a functionality for CLI, you are adding a client library.

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)
}
8 changes: 1 addition & 7 deletions pkg/restclient/volume.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,14 +117,8 @@ func (c *Client) VolumeSet(volname string, req api.VolOptionReq) error {
return 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)
}

// VolumeGet gets volume options for a Gluster Volume
func (c *Client) VolumeGet(volname string, optname string) (api.VolumeOptionsGetResp, error) {
func (c *Client) VolumeGet(volname, optname string) (api.VolumeOptionsGetResp, error) {
if optname == "all" {
var opts api.VolumeOptionsGetResp
url := fmt.Sprintf("/v1/volumes/%s/options", volname)
Expand Down
12 changes: 12 additions & 0 deletions pkg/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"}
Copy link
Member

Choose a reason for hiding this comment

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

add comment for why we are reserving these keywords

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reserved key words is self explanatory

Copy link
Member

Choose a reason for hiding this comment

The 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.
by these comments, I don't think they will get more context why these keywords are reserved.

)

// 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) {
Expand Down