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 <ppai@redhat.com>

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

glusterd2/cluster/cluster.go

@@ -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
-}

glusterd2/cluster/store-utils.go

@@ -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
-}

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{},
 }

glusterd2/commands/global/getglobaloptions.go

@@ -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
-}

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,
 		},
 	}
 }

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
+}

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

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
+}

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"`
+}

pkg/api/cluster_req.go

@@ -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"`
-}

pkg/api/cluster_resp.go

@@ -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"`
-}

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")

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)
 }