From edda2c5063db0cb08c6fe4402fee70d8105193bd Mon Sep 17 00:00:00 2001 From: Prashanth Pai Date: Tue, 9 Oct 2018 12:22:26 +0530 Subject: [PATCH] Refactor cluster options package * Merge 'glusterd2/cluster' package into `glusterd2/options` package. * Rename 'glusterd2/commands/global' to 'glusterd2/commands/options'. Signed-off-by: Prashanth Pai --- glustercli/cmd/volume-set.go | 2 +- glusterd2/cluster/cluster.go | 31 --------- glusterd2/cluster/store-utils.go | 40 ----------- glusterd2/commands/command.go | 4 +- glusterd2/commands/global/getglobaloptions.go | 52 -------------- .../commands/{global => options}/commands.go | 12 ++-- .../commands/options/getclusteroptions.go | 50 ++++++++++++++ .../setclusteroptions.go} | 25 +++---- glusterd2/options/cluster.go | 69 +++++++++++++++++++ pkg/api/cluster_options.go | 14 ++++ pkg/api/cluster_req.go | 6 -- pkg/api/cluster_resp.go | 9 --- pkg/errors/error.go | 2 +- pkg/restclient/volume.go | 4 +- 14 files changed, 158 insertions(+), 162 deletions(-) delete mode 100644 glusterd2/cluster/cluster.go delete mode 100644 glusterd2/cluster/store-utils.go delete mode 100644 glusterd2/commands/global/getglobaloptions.go rename glusterd2/commands/{global => options}/commands.go (70%) create mode 100644 glusterd2/commands/options/getclusteroptions.go rename glusterd2/commands/{global/setglobaloptions.go => options/setclusteroptions.go} (62%) create mode 100644 glusterd2/options/cluster.go create mode 100644 pkg/api/cluster_options.go delete mode 100644 pkg/api/cluster_req.go delete mode 100644 pkg/api/cluster_resp.go diff --git a/glustercli/cmd/volume-set.go b/glustercli/cmd/volume-set.go index fd63eae2..a8c906b4 100644 --- a/glustercli/cmd/volume-set.go +++ b/glustercli/cmd/volume-set.go @@ -71,7 +71,7 @@ func volumeOptionJSONHandler(cmd *cobra.Command, volname string, options []strin } if volname == "all" { - err := client.GlobalOptionSet(api.GlobalOptionReq{ + err := client.ClusterOptionSet(api.ClusterOptionReq{ Options: vopt, }) return err diff --git a/glusterd2/cluster/cluster.go b/glusterd2/cluster/cluster.go deleted file mode 100644 index 47824df6..00000000 --- a/glusterd2/cluster/cluster.go +++ /dev/null @@ -1,31 +0,0 @@ -package cluster - -import ( - "github.com/gluster/glusterd2/glusterd2/gdctx" - "github.com/gluster/glusterd2/glusterd2/options" - - "strconv" -) - -// GlobalOptMap contains list of supported cluster-wide options, default values and value types -var GlobalOptMap = map[string]GlobalOption{ - "cluster.server-quorum-ratio": {"cluster.server-quorum-ratio", "51", options.OptionTypePercent}, - "cluster.shared-storage": {"cluster.shared-storage", "disable", options.OptionTypeBool}, - "cluster.op-version": {"cluster.op-version", strconv.Itoa(gdctx.OpVersion), options.OptionTypeInt}, - "cluster.max-op-version": {"cluster.max-op-version", strconv.Itoa(gdctx.OpVersion), options.OptionTypeInt}, - "cluster.brick-multiplex": {"cluster.brick-multiplex", "disable", options.OptionTypeBool}, - "cluster.max-bricks-per-process": {"cluster.max-bricks-per-process", "0", options.OptionTypeInt}, - "cluster.localtime-logging": {"cluster.localtime-logging", "disable", options.OptionTypeBool}, -} - -// Cluster contains cluster-wide attributes -type Cluster struct { - Options map[string]string -} - -// GlobalOption reperesents cluster wide options -type GlobalOption struct { - Key string - DefaultValue string - Type options.OptionType -} diff --git a/glusterd2/cluster/store-utils.go b/glusterd2/cluster/store-utils.go deleted file mode 100644 index ed725bb8..00000000 --- a/glusterd2/cluster/store-utils.go +++ /dev/null @@ -1,40 +0,0 @@ -package cluster - -import ( - "context" - "encoding/json" - - "github.com/gluster/glusterd2/glusterd2/store" - "github.com/gluster/glusterd2/pkg/errors" -) - -const ( - // ClusterPrefix represents the etcd end-point for cluster wide attributes - ClusterPrefix string = "cluster/" -) - -// GetCluster gets cluster object instace from store -func GetCluster() (*Cluster, error) { - resp, err := store.Get(context.TODO(), ClusterPrefix) - if err != nil { - return nil, err - } - - if resp.Count != 1 { - return nil, errors.ErrClusterNotFound - } - - var c Cluster - if err = json.Unmarshal(resp.Kvs[0].Value, &c); err != nil { - return nil, err - } - - return &c, nil -} - -// UpdateCluster updates cluster instance in etcd store -func UpdateCluster(c *Cluster) error { - data, _ := json.Marshal(c) - _, err := store.Put(context.TODO(), ClusterPrefix, string(data)) - return err -} diff --git a/glusterd2/commands/command.go b/glusterd2/commands/command.go index 995d0b7b..14af4371 100644 --- a/glusterd2/commands/command.go +++ b/glusterd2/commands/command.go @@ -2,7 +2,7 @@ package commands import ( - "github.com/gluster/glusterd2/glusterd2/commands/global" + "github.com/gluster/glusterd2/glusterd2/commands/options" "github.com/gluster/glusterd2/glusterd2/commands/peers" "github.com/gluster/glusterd2/glusterd2/commands/snapshot" "github.com/gluster/glusterd2/glusterd2/commands/version" @@ -24,5 +24,5 @@ var Commands = []Command{ &volumecommands.Command{}, &snapshotcommands.Command{}, &peercommands.Command{}, - &globalcommands.Command{}, + &optionscommands.Command{}, } diff --git a/glusterd2/commands/global/getglobaloptions.go b/glusterd2/commands/global/getglobaloptions.go deleted file mode 100644 index ebad2d93..00000000 --- a/glusterd2/commands/global/getglobaloptions.go +++ /dev/null @@ -1,52 +0,0 @@ -package globalcommands - -import ( - "fmt" - "net/http" - - "github.com/gluster/glusterd2/glusterd2/cluster" - restutils "github.com/gluster/glusterd2/glusterd2/servers/rest/utils" - "github.com/gluster/glusterd2/pkg/api" - "github.com/gluster/glusterd2/pkg/errors" -) - -func getGlobalOptionsHandler(w http.ResponseWriter, r *http.Request) { - ctx := r.Context() - - c, err := cluster.GetCluster() - // ErrClusterNotFound here implies that no global option has yet been explicitly set. Ignoring it. - if err != nil && err != errors.ErrClusterNotFound { - restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, fmt.Sprintf("Problem retrieving cluster information from etcd store: %s", err.Error())) - return - } - - resp := createGlobalOptionsGetResp(c) - restutils.SendHTTPResponse(ctx, w, http.StatusOK, resp) -} - -func createGlobalOptionsGetResp(c *cluster.Cluster) []api.GlobalOptionsGetResp { - var resp []api.GlobalOptionsGetResp - - for k, v := range cluster.GlobalOptMap { - var val string - var found bool - if c == nil { - val = v.DefaultValue - found = false - } else { - val, found = c.Options[k] - if !found { - val = v.DefaultValue - } - } - - resp = append(resp, api.GlobalOptionsGetResp{ - Key: k, - Value: val, - DefaultValue: v.DefaultValue, - Modified: found, - }) - } - - return resp -} diff --git a/glusterd2/commands/global/commands.go b/glusterd2/commands/options/commands.go similarity index 70% rename from glusterd2/commands/global/commands.go rename to glusterd2/commands/options/commands.go index d6196ba1..145907d7 100644 --- a/glusterd2/commands/global/commands.go +++ b/glusterd2/commands/options/commands.go @@ -1,5 +1,5 @@ -// Package globalcommands implements the commands to get and set cluster level options -package globalcommands +// Package optionscommands implements the commands to get and set cluster level options +package optionscommands import ( "github.com/gluster/glusterd2/glusterd2/servers/rest/route" @@ -13,18 +13,18 @@ type Command struct { func (c *Command) Routes() route.Routes { return route.Routes{ route.Route{ - Name: "SetGlobalOptions", + Name: "SetClusterOptions", Method: "POST", Pattern: "/cluster/options", Version: 1, - HandlerFunc: setGlobalOptionsHandler, + HandlerFunc: setClusterOptionsHandler, }, route.Route{ - Name: "GetGlobalOptions", + Name: "GetClusterOptions", Method: "GET", Pattern: "/cluster/options", Version: 1, - HandlerFunc: getGlobalOptionsHandler, + HandlerFunc: getClusterOptionsHandler, }, } } diff --git a/glusterd2/commands/options/getclusteroptions.go b/glusterd2/commands/options/getclusteroptions.go new file mode 100644 index 00000000..bffdd2f7 --- /dev/null +++ b/glusterd2/commands/options/getclusteroptions.go @@ -0,0 +1,50 @@ +package optionscommands + +import ( + "net/http" + + "github.com/gluster/glusterd2/glusterd2/options" + restutils "github.com/gluster/glusterd2/glusterd2/servers/rest/utils" + "github.com/gluster/glusterd2/pkg/api" + "github.com/gluster/glusterd2/pkg/errors" +) + +func getClusterOptionsHandler(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + c, err := options.GetClusterOptions() + if err != nil && err != errors.ErrClusterOptionsNotFound { + restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err) + return + } + + resp := createClusterOptionsResp(c) + restutils.SendHTTPResponse(ctx, w, http.StatusOK, resp) +} + +func createClusterOptionsResp(c *options.ClusterOptions) []api.ClusterOptionsResp { + var resp []api.ClusterOptionsResp + + for k, v := range options.ClusterOptMap { + var val string + var found bool + if c == nil { + val = v.DefaultValue + found = false + } else { + val, found = c.Options[k] + if !found { + val = v.DefaultValue + } + } + + resp = append(resp, api.ClusterOptionsResp{ + Key: k, + Value: val, + DefaultValue: v.DefaultValue, + Modified: found, + }) + } + + return resp +} diff --git a/glusterd2/commands/global/setglobaloptions.go b/glusterd2/commands/options/setclusteroptions.go similarity index 62% rename from glusterd2/commands/global/setglobaloptions.go rename to glusterd2/commands/options/setclusteroptions.go index d76bce17..1089113e 100644 --- a/glusterd2/commands/global/setglobaloptions.go +++ b/glusterd2/commands/options/setclusteroptions.go @@ -1,47 +1,48 @@ -package globalcommands +package optionscommands import ( "fmt" "net/http" - "github.com/gluster/glusterd2/glusterd2/cluster" + "github.com/gluster/glusterd2/glusterd2/options" restutils "github.com/gluster/glusterd2/glusterd2/servers/rest/utils" "github.com/gluster/glusterd2/pkg/api" "github.com/gluster/glusterd2/pkg/errors" ) -func setGlobalOptionsHandler(w http.ResponseWriter, r *http.Request) { +func setClusterOptionsHandler(w http.ResponseWriter, r *http.Request) { ctx := r.Context() - var req api.GlobalOptionReq + var req api.ClusterOptionReq if err := restutils.UnmarshalRequest(r, &req); err != nil { restutils.SendHTTPError(ctx, w, http.StatusBadRequest, errors.ErrJSONParsingFailed) return } - c, err := cluster.GetCluster() - // ErrClusterNotFound here implies that no global option has yet been explicitly set. Ignoring it. - if err != nil && err != errors.ErrClusterNotFound { - restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, fmt.Sprintf("Problem retrieving cluster information from etcd store: %s", err.Error())) + // TODO: Take lock here + + c, err := options.GetClusterOptions() + if err != nil && err != errors.ErrClusterOptionsNotFound { + restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err) return } for k, v := range req.Options { - if _, found := cluster.GlobalOptMap[k]; found { + if _, found := options.ClusterOptMap[k]; found { // TODO validate the value type for global option if c == nil { - c = new(cluster.Cluster) + c = new(options.ClusterOptions) c.Options = make(map[string]string) } c.Options[k] = v } else { restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, fmt.Sprintf("Invalid global option: %s", k)) - continue + return } } - if err := cluster.UpdateCluster(c); err != nil { + if err := options.UpdateClusterOptions(c); err != nil { restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, fmt.Sprintf("Failed to update store with cluster attributes %s", err.Error())) return diff --git a/glusterd2/options/cluster.go b/glusterd2/options/cluster.go new file mode 100644 index 00000000..79c7314c --- /dev/null +++ b/glusterd2/options/cluster.go @@ -0,0 +1,69 @@ +package options + +import ( + "context" + "encoding/json" + + "github.com/gluster/glusterd2/glusterd2/gdctx" + "github.com/gluster/glusterd2/glusterd2/store" + "github.com/gluster/glusterd2/pkg/errors" + + "strconv" +) + +const ( + clusterOptionsKey string = "clusteroptions" +) + +// ClusterOption reperesents a single cluster wide option +type ClusterOption struct { + Key string + DefaultValue string + Type OptionType +} + +// ClusterOptMap contains list of supported cluster-wide options, default values and value types +var ClusterOptMap = map[string]ClusterOption{ + "cluster.server-quorum-ratio": {"cluster.server-quorum-ratio", "51", OptionTypePercent}, + "cluster.shared-storage": {"cluster.shared-storage", "disable", OptionTypeBool}, + "cluster.op-version": {"cluster.op-version", strconv.Itoa(gdctx.OpVersion), OptionTypeInt}, + "cluster.max-op-version": {"cluster.max-op-version", strconv.Itoa(gdctx.OpVersion), OptionTypeInt}, + "cluster.brick-multiplex": {"cluster.brick-multiplex", "disable", OptionTypeBool}, + "cluster.max-bricks-per-process": {"cluster.max-bricks-per-process", "0", OptionTypeInt}, + "cluster.localtime-logging": {"cluster.localtime-logging", "disable", OptionTypeBool}, +} + +// ClusterOptions contains cluster-wide attributes +type ClusterOptions struct { + Options map[string]string +} + +// GetClusterOptions gets cluster options from store. +func GetClusterOptions() (*ClusterOptions, error) { + resp, err := store.Get(context.TODO(), clusterOptionsKey) + if err != nil { + return nil, err + } + + if resp.Count != 1 { + return nil, errors.ErrClusterOptionsNotFound + } + + var c ClusterOptions + if err = json.Unmarshal(resp.Kvs[0].Value, &c); err != nil { + return nil, err + } + + return &c, nil +} + +// UpdateClusterOptions stores cluster options in store. +func UpdateClusterOptions(c *ClusterOptions) error { + b, err := json.Marshal(c) + if err != nil { + return err + } + + _, err = store.Put(context.TODO(), clusterOptionsKey, string(b)) + return err +} diff --git a/pkg/api/cluster_options.go b/pkg/api/cluster_options.go new file mode 100644 index 00000000..26b4db49 --- /dev/null +++ b/pkg/api/cluster_options.go @@ -0,0 +1,14 @@ +package api + +// ClusterOptionReq represents an incoming request to set cluster level options +type ClusterOptionReq struct { + Options map[string]string `json:"options"` +} + +// ClusterOptionsResp contains details for global options +type ClusterOptionsResp struct { + Key string `json:"key"` + Value string `json:"value"` + DefaultValue string `json:"default"` + Modified bool `json:"modified"` +} diff --git a/pkg/api/cluster_req.go b/pkg/api/cluster_req.go deleted file mode 100644 index 6f0deb32..00000000 --- a/pkg/api/cluster_req.go +++ /dev/null @@ -1,6 +0,0 @@ -package api - -// GlobalOptionReq represents an incoming request to set cluster level options -type GlobalOptionReq struct { - Options map[string]string `json:"options"` -} diff --git a/pkg/api/cluster_resp.go b/pkg/api/cluster_resp.go deleted file mode 100644 index 214f0370..00000000 --- a/pkg/api/cluster_resp.go +++ /dev/null @@ -1,9 +0,0 @@ -package api - -// GlobalOptionsGetResp contains details for global options -type GlobalOptionsGetResp struct { - Key string `json:"key"` - Value string `json:"value"` - DefaultValue string `json:"default"` - Modified bool `json:"modified"` -} diff --git a/pkg/errors/error.go b/pkg/errors/error.go index 013084f1..6ec14ff3 100644 --- a/pkg/errors/error.go +++ b/pkg/errors/error.go @@ -41,7 +41,7 @@ var ( ErrUnknownValue = errors.New("unknown value specified") ErrGetFailed = errors.New("failed to get value from the store") ErrUnmarshallFailed = errors.New("failed to unmarshall from json") - ErrClusterNotFound = errors.New("cluster instance not found in store") + ErrClusterOptionsNotFound = errors.New("cluster options not found in store") ErrDuplicateBrickPath = errors.New("duplicate brick entry") ErrRestrictedKeyFound = errors.New("key names starting with '_' are restricted in metadata field") ErrVolFileNotFound = errors.New("volume file not found") diff --git a/pkg/restclient/volume.go b/pkg/restclient/volume.go index f2bbb16b..6d6f5484 100644 --- a/pkg/restclient/volume.go +++ b/pkg/restclient/volume.go @@ -117,8 +117,8 @@ func (c *Client) VolumeSet(volname string, req api.VolOptionReq) error { return err } -// GlobalOptionSet sets cluster level options -func (c *Client) GlobalOptionSet(req api.GlobalOptionReq) error { +// 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) }