From 7a35c21bad3ac3f8a40adc4c5542eb18c3c19c00 Mon Sep 17 00:00:00 2001 From: rishubhjain Date: Tue, 10 Jul 2018 12:39:04 -0400 Subject: [PATCH 1/2] Rebalance code refactor and validation --- plugins/rebalance/errors.go | 2 + plugins/rebalance/init.go | 40 ++++++++-------- plugins/rebalance/rest.go | 92 ++++++++++++++++++++----------------- 3 files changed, 73 insertions(+), 61 deletions(-) diff --git a/plugins/rebalance/errors.go b/plugins/rebalance/errors.go index 9f42a6056..68a6f7b1d 100644 --- a/plugins/rebalance/errors.go +++ b/plugins/rebalance/errors.go @@ -7,6 +7,8 @@ import ( var ( // ErrVolNotDistribute : Cannot run rebalance on a non distribute volume ErrVolNotDistribute = errors.New("not a distribute volume") + // ErrRebalanceAlreadyStarted : Rebalance already started on the volume + ErrRebalanceAlreadyStarted = errors.New("rebalance already started") // ErrRebalanceNotStarted : Rebalance not started on the volume ErrRebalanceNotStarted = errors.New("rebalance not started") // ErrRebalanceInvalidOption : Invalid option provided to the rebalance start command diff --git a/plugins/rebalance/init.go b/plugins/rebalance/init.go index 24339324f..add53fd69 100644 --- a/plugins/rebalance/init.go +++ b/plugins/rebalance/init.go @@ -5,6 +5,8 @@ import ( "github.com/gluster/glusterd2/glusterd2/transaction" "github.com/gluster/glusterd2/pkg/utils" rebalanceapi "github.com/gluster/glusterd2/plugins/rebalance/api" + + "github.com/pborman/uuid" ) // Plugin is a structure which implements GlusterdPlugin interface @@ -20,27 +22,27 @@ func (p *Plugin) Name() string { func (p *Plugin) RestRoutes() route.Routes { return route.Routes{ route.Route{ - Name: "RebalanceStart", - Method: "POST", - Pattern: "/volumes/{volname}/rebalance/start", - Version: 1, - RequestType: utils.GetTypeString((*rebalanceapi.StartReq)(nil)), - // ResponseType: utils.GetTypeString((*rebalanceapi.RebalInfo)(nil)), - HandlerFunc: rebalanceStartHandler}, + Name: "RebalanceStart", + Method: "POST", + Pattern: "/volumes/{volname}/rebalance/start", + Version: 1, + RequestType: utils.GetTypeString((*rebalanceapi.StartReq)(nil)), + ResponseType: utils.GetTypeString((*uuid.UUID)(nil)), + HandlerFunc: rebalanceStartHandler}, route.Route{ - Name: "RebalanceStop", - Method: "POST", - Pattern: "/volumes/{volname}/rebalance/stop", - Version: 1, - // ResponseType: utils.GetTypeString((*rebalanceapi.RebalInfo)(nil)), - HandlerFunc: rebalanceStopHandler}, + Name: "RebalanceStop", + Method: "POST", + Pattern: "/volumes/{volname}/rebalance/stop", + Version: 1, + ResponseType: utils.GetTypeString((*rebalanceapi.RebalInfo)(nil)), + HandlerFunc: rebalanceStopHandler}, route.Route{ - Name: "RebalanceStatus", - Method: "GET", - Pattern: "/volumes/{volname}/rebalance", - Version: 1, - // ResponseType: utils.GetTypeString((*rebalanceapi.RebalInfo)(nil)), - HandlerFunc: rebalanceStatusHandler}, + Name: "RebalanceStatus", + Method: "GET", + Pattern: "/volumes/{volname}/rebalance", + Version: 1, + ResponseType: utils.GetTypeString((*rebalanceapi.RebalStatus)(nil)), + HandlerFunc: rebalanceStatusHandler}, } } diff --git a/plugins/rebalance/rest.go b/plugins/rebalance/rest.go index 9f35d625d..e62782eca 100644 --- a/plugins/rebalance/rest.go +++ b/plugins/rebalance/rest.go @@ -44,14 +44,23 @@ func rebalanceStartHandler(w http.ResponseWriter, r *http.Request) { return } - rebalinfo := createRebalanceInfo(volname, &req) - if rebalinfo == nil { - logger.WithError(err).Error("failed to create Rebalance info") + rebalInfo, err := GetRebalanceInfo(volname) + if err == nil { + if rebalInfo.State == rebalanceapi.Started { + log.WithError(err).WithField("volume", volname).Error("Rebalance process has already been started.") + restutils.SendHTTPError(ctx, w, http.StatusBadRequest, ErrRebalanceAlreadyStarted) + return + } + } + + rebalInfo = createRebalanceInfo(volname, &req) + if rebalInfo == nil { + logger.WithError(err).WithField("volume", volname).Error("failed to create Rebalance info") restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err) return } - if rebalinfo.Cmd == rebalanceapi.CmdNone { + if rebalInfo.Cmd == rebalanceapi.CmdNone { restutils.SendHTTPError(ctx, w, http.StatusBadRequest, ErrRebalanceInvalidOption) return } @@ -100,14 +109,14 @@ func rebalanceStartHandler(w http.ResponseWriter, r *http.Request) { err = txn.Ctx.Set("volname", volname) if err != nil { - logger.WithError(err).Error("failed to set volname in transaction context") + logger.WithError(err).WithField("key", "volname").Error("failed to set volname in transaction context") restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err) return } - err = txn.Ctx.Set("rinfo", rebalinfo) + err = txn.Ctx.Set("rinfo", rebalInfo) if err != nil { - logger.WithError(err).Error("failed to set rebalance info in transaction context") + logger.WithError(err).WithField("key", "rinfo").Error("failed to set rebalance info in transaction context") restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err) return } @@ -118,25 +127,25 @@ func rebalanceStartHandler(w http.ResponseWriter, r *http.Request) { * rebalance process is one per node per volume. * Need to handle scenarios where process is started in * few nodes and failed in few others */ - logger.WithFields(log.Fields{ - "error": err.Error(), + logger.WithError(err).WithFields(log.Fields{ "volname": volname, }).Error("failed to start rebalance on volume") - restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err) + status, err := restutils.ErrToStatusCode(err) + restutils.SendHTTPError(ctx, w, status, err) return } - rebalinfo, err = GetRebalanceInfo(volname) + rebalInfo, err = GetRebalanceInfo(volname) if err != nil { - logger.WithFields(log.Fields{ - "error": err.Error(), + logger.WithError(err).WithFields(log.Fields{ "volname": volname, }).Error("failed to get the rebalance info for volume") + restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err) + return } - logger.WithField("volname", rebalinfo.Volname).Info("rebalance started") - - restutils.SendHTTPResponse(ctx, w, http.StatusOK, rebalinfo.RebalanceID) + logger.WithField("volname", rebalInfo.Volname).Info("rebalance started") + restutils.SendHTTPResponse(ctx, w, http.StatusOK, rebalInfo.RebalanceID) } func rebalanceStopHandler(w http.ResponseWriter, r *http.Request) { @@ -162,14 +171,14 @@ func rebalanceStopHandler(w http.ResponseWriter, r *http.Request) { return } - rebalinfo, err := GetRebalanceInfo(volname) + rebalInfo, err := GetRebalanceInfo(volname) if err != nil { restutils.SendHTTPError(r.Context(), w, http.StatusBadRequest, ErrRebalanceNotStarted) return } // Check whether the rebalance state is started - if rebalinfo.State != rebalanceapi.Started { + if rebalInfo.State != rebalanceapi.Started { restutils.SendHTTPError(r.Context(), w, http.StatusBadRequest, ErrRebalanceNotStarted) return } @@ -188,34 +197,33 @@ func rebalanceStopHandler(w http.ResponseWriter, r *http.Request) { err = txn.Ctx.Set("volname", volname) if err != nil { - logger.WithError(err).Error("failed to set volname in transaction context") + logger.WithError(err).WithField("key", "volname").Error("failed to set volname in transaction context") restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err.Error()) return } - rebalinfo.Volname = volname - rebalinfo.State = rebalanceapi.Stopped - rebalinfo.Cmd = rebalanceapi.CmdStop + rebalInfo.Volname = volname + rebalInfo.State = rebalanceapi.Stopped + rebalInfo.Cmd = rebalanceapi.CmdStop - err = txn.Ctx.Set("rinfo", rebalinfo) + err = txn.Ctx.Set("rinfo", rebalInfo) if err != nil { - logger.WithError(err).Error("failed to set rebalance info in transaction context") + logger.WithError(err).WithField("key", "rebalInfo").Error("failed to set rebalance info in transaction context") restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err.Error()) return } err = txn.Do() if err != nil { - logger.WithFields(log.Fields{ - "error": err.Error(), + logger.WithError(err).WithFields(log.Fields{ "volname": volname, }).Error("failed to stop rebalance on volume") - restutils.SendHTTPError(r.Context(), w, http.StatusInternalServerError, err.Error()) + restutils.SendHTTPError(r.Context(), w, http.StatusInternalServerError, err) return } - logger.WithField("volname", rebalinfo.Volname).Info("rebalance stopped") - restutils.SendHTTPResponse(r.Context(), w, http.StatusOK, rebalinfo) + logger.WithField("volname", rebalInfo.Volname).Info("rebalance stopped") + restutils.SendHTTPResponse(r.Context(), w, http.StatusOK, rebalInfo) } func rebalanceStatusHandler(w http.ResponseWriter, r *http.Request) { @@ -241,16 +249,16 @@ func rebalanceStatusHandler(w http.ResponseWriter, r *http.Request) { return } - rebalinfo, err := GetRebalanceInfo(volname) + rebalInfo, err := GetRebalanceInfo(volname) if err != nil { - restutils.SendHTTPError(ctx, w, http.StatusNotFound, err.Error()) + restutils.SendHTTPError(ctx, w, http.StatusNotFound, err) return } err = txn.Ctx.Set("volname", volname) if err != nil { - logger.WithError(err).Error("failed to set volname in transaction context") - restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err.Error()) + logger.WithError(err).WithField("key", "volname").Error("failed to set volname in transaction context") + restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err) return } @@ -266,27 +274,26 @@ func rebalanceStatusHandler(w http.ResponseWriter, r *http.Request) { }, } - err = txn.Ctx.Set("rinfo", rebalinfo) + err = txn.Ctx.Set("rinfo", rebalInfo) if err != nil { - logger.WithError(err).Error("failed to set rebalance info in transaction context") - restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err.Error()) + logger.WithError(err).WithField("key", "rinfo").Error("failed to set rebalance info in transaction context") + restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err) return } err = txn.Do() if err != nil { - logger.WithFields(log.Fields{ - "error": err.Error(), + logger.WithError(err).WithFields(log.Fields{ "volname": volname, }).Error("failed to query rebalance status for volume") + return } response, err := createRebalanceStatusResp(txn.Ctx, vol) if err != nil { - errMsg := "Failed to create rebalance status response" - logger.WithField("error", err.Error()).Error("rebalanceStatusHandler:" + errMsg) - restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, - errMsg) + errMsg := "failed to create rebalance status response" + logger.WithError(err).Error("rebalanceStatusHandler:" + errMsg) + restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, errMsg) return } @@ -325,5 +332,6 @@ func createRebalanceStatusResp(ctx transaction.TxnCtx, volinfo *volume.Volinfo) resp.Nodes = append(resp.Nodes, tmp) } + return &resp, nil } From ef84e98c80c3dce44df6ac264d078fea2e434de3 Mon Sep 17 00:00:00 2001 From: rishubhjain Date: Thu, 12 Jul 2018 04:59:46 -0400 Subject: [PATCH 2/2] Adding CLI --- glustercli/cmd/rebalance.go | 56 ++++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 23 deletions(-) diff --git a/glustercli/cmd/rebalance.go b/glustercli/cmd/rebalance.go index 946fd0c85..1e20d21c7 100644 --- a/glustercli/cmd/rebalance.go +++ b/glustercli/cmd/rebalance.go @@ -1,30 +1,29 @@ package cmd import ( - fmt + "fmt" "github.com/gluster/glusterd2/pkg/restclient" rebalance "github.com/gluster/glusterd2/plugins/rebalance/api" ) const ( - helpRebalanceCmd = "Gluster Rebalance" - helpRebalanceStartCmd = "Start rebalance session for gluster volume" - helpRebalanceStatusCmd = "Status of rebalance seesion" - helpRebalanceStopCmd = "Stop rebalance session" + helpRebalanceCmd = "Gluster Rebalance" + helpRebalanceStartCmd = "Start rebalance session for gluster volume" + helpRebalanceStatusCmd = "Status of rebalance seesion" + helpRebalanceStopCmd = "Stop rebalance session" ) -( - flagRebalanceStartCmdForce bool - flagRebalanceStartCmdFixLayout bool +var ( + flagRebalanceStartCmdForce bool + flagRebalanceStartCmdFixLayout bool ) var rebalanceCmd = &cobra.Command{ - Use: "volume rebalance", - Short: helpRebalanceCmd + Use: "volume rebalance", + Short: helpRebalanceCmd, } - func init() { // Rebalance Start @@ -36,20 +35,31 @@ func init() { } var rebalaceStartCmd = &cobra.Command{ - Use: " start [flags]", - Short: helpRebalanceStartCmd, - Args: cobra.ExactArgs(1), - Run: func(cmd *cobra.Command, args []string) { - volname := cmd.Flags().Args()[0] - err := client.VolumeStart(volname, "") - if err != nil { - if verbose { - log.WithError(err.Error()).WithFields(log.Fields{ - "volume": volname, - }).Error("rebalance start failed") + Use: " start [flags]", + Short: helpRebalanceStartCmd, + Args: cobra.ExactArgs(1), + Run: func(cmd *cobra.Command, args []string) { + volname := cmd.Flags().Args()[0] + var err error + if flagRebalanceStartCmdForce && flagRebalanceStartCmdFixLayout { + err := errors.New("Conflicting options found") + failure("Please provide only 1 option", err, 1) + } + if flagRebalanceStartCmdForce { + err = client.VolumeStart(volname, "force") + } else if flagRebalanceStartCmdFixLayout { + err = client.VolumeStart(volname, "fix-layout") + } else { + err = client.VolumeStart(volname, "") + } + if err != nil { + if verbose { + log.WithError(err.Error()).WithFields(log.Fields{ + "volume": volname, + }).Error("rebalance start failed") } failure("rebalance start failed", err, 1) } fmt.Printf("Rebalance for %s started successfully\n", volname) - }, + }, }