-
Notifications
You must be signed in to change notification settings - Fork 82
gfproxy: Add plugin for daemon management #413
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 |
|---|---|---|
|
|
@@ -45,3 +45,12 @@ func IsQuotaEnabled(v *Volinfo) bool { | |
| } | ||
| return false | ||
| } | ||
|
|
||
| // IsGfproxydEnabled returns true if gfproxyd is enabled for a volume and false otherwise | ||
| func IsGfproxydEnabled(v *Volinfo) bool { | ||
|
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. This should be in the gfproxyd package. Again, plugins shouldn't be modifying core packages.
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. Same here, move this function to |
||
| val, exists := v.Options[VkeyFeaturesGfproxyd] | ||
| if exists && val == "on" { | ||
| return true | ||
| } | ||
| return false | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,5 +38,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") | ||
| ErrGfproxydAlreadyEnabled = errors.New("Gfproxyd is already enabled") | ||
|
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. This package is something I'd like to get rid of. Please keep the errors related to a package, within the package. This should be in the Also, (again) plugins shouldn't be modifying core packages.
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. Same here |
||
| ErrGfproxydAlreadyDisabled = errors.New("Gfproxyd is already disabled") | ||
| ErrClusterNotFound = errors.New("Cluster instance not found in store") | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,92 @@ | ||
| package gfproxyd | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "fmt" | ||
| "net" | ||
| "os/exec" | ||
| "path" | ||
| "strconv" | ||
|
|
||
| "github.com/gluster/glusterd2/glusterd2/pmap" | ||
| config "github.com/spf13/viper" | ||
| ) | ||
|
|
||
| const ( | ||
| gfproxydBin = "glusterfsd" | ||
| ) | ||
|
|
||
| // gfproxyd type represents information about gfproxyd process | ||
| type gfproxyd struct { | ||
| volname string | ||
| args string | ||
| pidfilepath string | ||
| binarypath string | ||
| volfileID string | ||
| logfile string | ||
| gfproxyID string | ||
| gfproxydPort string | ||
| } | ||
|
|
||
| // Name returns human-friendly name of the gfproxyd process. This is used for logging. | ||
| func (g *gfproxyd) Name() string { | ||
| return g.gfproxyID | ||
| } | ||
|
|
||
| // Path returns absolute path to the binary of gfproxyd process | ||
| func (g *gfproxyd) Path() string { | ||
| return g.binarypath | ||
| } | ||
|
|
||
| // Args returns arguments to be passed to gfproxyd process during spawn. | ||
| func (g *gfproxyd) Args() string { | ||
| return g.args | ||
| } | ||
|
|
||
| // SocketFile returns path to the socket file | ||
| func (g *gfproxyd) SocketFile() string { | ||
| return "" | ||
| } | ||
|
|
||
| // PidFile returns path to the pid file of the gfproxyd process | ||
| func (g *gfproxyd) PidFile() string { | ||
| return g.pidfilepath | ||
| } | ||
|
|
||
| // newgfproxyd returns a new instance of gfproxyd type which implements the Daemon interface | ||
| func newgfproxyd(volname string) (*gfproxyd, error) { | ||
| g := &gfproxyd{volname: volname} | ||
| binarypath, e := exec.LookPath(gfproxydBin) | ||
| if e != nil { | ||
| return nil, e | ||
| } | ||
| g.binarypath = binarypath | ||
| g.gfproxyID = fmt.Sprintf("gfproxyd-%s", volname) | ||
| g.gfproxydPort = strconv.Itoa(pmap.AssignPort(0, g.gfproxyID)) | ||
| g.volfileID = fmt.Sprintf("gfproxyd/%s", volname) | ||
|
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'm assuming the gfproxy volfile is already generated. Right?
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. Yes |
||
| g.logfile = path.Join(config.GetString("logdir"), "glusterfs", fmt.Sprintf("gfproxyd-%s.log", volname)) | ||
| g.pidfilepath = fmt.Sprintf("%s/gfproxyd-%s.pid", config.GetString("rundir"), volname) | ||
|
|
||
| shost, _, _ := net.SplitHostPort(config.GetString("clientaddress")) | ||
|
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. Glusterd2 can be configured to listen on any port and this code needs to take that into consideration. shost, sport, _ := net.SplitHostPort(config.GetString("clientaddress"))
...
buffer.WriteString(fmt.Sprintf(" --volfile-server-port %s", sport))You can refer to usage of the above code in
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. This is required, since tests will be run with different ports for glusterd2 |
||
| if shost == "" { | ||
| shost = "localhost" | ||
| } | ||
|
|
||
| var buffer bytes.Buffer | ||
|
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. This needs change now, convert this to string list |
||
| buffer.WriteString(fmt.Sprintf(" -s %s", shost)) | ||
| buffer.WriteString(fmt.Sprintf(" --volfile-id %s", g.volfileID)) | ||
| buffer.WriteString(fmt.Sprintf(" -p %s", g.pidfilepath)) | ||
| buffer.WriteString(fmt.Sprintf(" -l %s", g.logfile)) | ||
| buffer.WriteString(fmt.Sprintf(" --brick-name %s", g.gfproxyID)) | ||
| buffer.WriteString(fmt.Sprintf(" --brick-port %s", g.gfproxydPort)) | ||
| buffer.WriteString(fmt.Sprintf(" --xlator-option %s-server.listen-port=%s", g.volname, g.gfproxydPort)) | ||
|
|
||
| g.args = buffer.String() | ||
|
|
||
| return g, nil | ||
| } | ||
|
|
||
| // ID returns the unique identifier of the gfproxyd. | ||
| func (g *gfproxyd) ID() string { | ||
| return g.gfproxyID | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| package gfproxyd | ||
|
|
||
| import ( | ||
| "github.com/gluster/glusterd2/glusterd2/servers/rest/route" | ||
| "github.com/gluster/glusterd2/glusterd2/transaction" | ||
| "github.com/gluster/glusterd2/pkg/sunrpc" | ||
| ) | ||
|
|
||
| // Plugin is a structure which implements GlusterdPlugin interface | ||
| type Plugin struct { | ||
| } | ||
|
|
||
| // Name returns name of plugin | ||
| func (p *Plugin) Name() string { | ||
| return "gfproxyd" | ||
| } | ||
|
|
||
| // SunRPCProgram returns sunrpc program to register with Glusterd | ||
| func (p *Plugin) SunRPCProgram() sunrpc.Program { | ||
|
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. This function can be removed now, made changes to plugin interface |
||
| return nil | ||
| } | ||
|
|
||
| // RestRoutes returns list of REST API routes to register with Glusterd | ||
| func (p *Plugin) RestRoutes() route.Routes { | ||
| return route.Routes{ | ||
| route.Route{ | ||
| Name: "GfproxydEnable", | ||
| Method: "POST", | ||
| Pattern: "/volumes/{name}/gfproxy/enable", | ||
| Version: 1, | ||
| HandlerFunc: gfproxydEnableHandler}, | ||
| route.Route{ | ||
| Name: "GfproxydDisable", | ||
| Method: "POST", | ||
| Pattern: "/volumes/{name}/gfproxy/disable", | ||
| Version: 1, | ||
| HandlerFunc: gfproxydDisableHandler}, | ||
| } | ||
| } | ||
|
|
||
| // RegisterStepFuncs registers transaction step functions with | ||
| // Glusterd Transaction framework | ||
| func (p *Plugin) RegisterStepFuncs() { | ||
| transaction.RegisterStepFunc(txnGfproxydStart, "gfproxyd-start.Commit") | ||
|
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. Lose the |
||
| transaction.RegisterStepFunc(txnGfproxydStop, "gfproxyd-stop.Commit") | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,164 @@ | ||
| package gfproxyd | ||
|
|
||
| import ( | ||
| "net/http" | ||
|
|
||
| "github.com/gluster/glusterd2/glusterd2/gdctx" | ||
| restutils "github.com/gluster/glusterd2/glusterd2/servers/rest/utils" | ||
| "github.com/gluster/glusterd2/glusterd2/transaction" | ||
| "github.com/gluster/glusterd2/glusterd2/volume" | ||
| "github.com/gluster/glusterd2/pkg/api" | ||
| "github.com/gluster/glusterd2/pkg/errors" | ||
|
|
||
| "github.com/gorilla/mux" | ||
| log "github.com/sirupsen/logrus" | ||
|
|
||
| "github.com/pborman/uuid" | ||
| ) | ||
|
|
||
| func gfproxydEnableHandler(w http.ResponseWriter, r *http.Request) { | ||
| // Collect inputs from URL | ||
| p := mux.Vars(r) | ||
| volname := p["name"] | ||
|
|
||
| ctx := r.Context() | ||
| logger := gdctx.GetReqLogger(ctx) | ||
|
|
||
| //validate volume name | ||
| volinfo, err := volume.GetVolume(volname) | ||
| if err != nil { | ||
| restutils.SendHTTPError(ctx, w, http.StatusNotFound, errors.ErrVolNotFound.Error(), api.ErrCodeDefault) | ||
| return | ||
| } | ||
|
|
||
| // Check if volume is started | ||
| if volinfo.State != volume.VolStarted { | ||
| restutils.SendHTTPError(ctx, w, http.StatusBadRequest, errors.ErrVolNotStarted.Error(), api.ErrCodeDefault) | ||
| return | ||
| } | ||
|
|
||
| // Check if gfproxyd is already enabled | ||
| if volume.IsGfproxydEnabled(volinfo) { | ||
| restutils.SendHTTPError(ctx, w, http.StatusBadRequest, errors.ErrGfproxydAlreadyEnabled.Error(), api.ErrCodeDefault) | ||
| return | ||
| } | ||
|
|
||
| // Transaction which starts gfproxyd on all nodes. | ||
| txn := transaction.NewTxn(ctx) | ||
| defer txn.Cleanup() | ||
|
|
||
| txn.Ctx.Set("volname", volname) | ||
|
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. You can use |
||
|
|
||
| // Enable gfproxyd | ||
| volinfo.Options[volume.VkeyFeaturesGfproxyd] = "on" | ||
|
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. After this step,
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. You shouldn't be directly setting options in a volinfo, without trying to verify that it is supported on all peers. The volume set step functions could be used to do the same. I don't know if its possible right now, but a TODO here will help remind us later.
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. For now this behavior is fine for internal options. |
||
|
|
||
| if err := txn.Ctx.Set("volinfo", volinfo); err != nil { | ||
| logger.WithError(err).Error("failed to set volinfo in transaction context") | ||
| restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err.Error(), api.ErrCodeDefault) | ||
| return | ||
| } | ||
|
|
||
| //Lock on Volume Name | ||
| lock, unlock, err := transaction.CreateLockSteps(volname) | ||
| if err != nil { | ||
| restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err.Error(), api.ErrCodeDefault) | ||
| return | ||
| } | ||
|
|
||
| txn.Nodes = volinfo.Nodes() | ||
| txn.Steps = []*transaction.Step{ | ||
| lock, | ||
| { | ||
| DoFunc: "vol-option.UpdateVolinfo", | ||
| Nodes: []uuid.UUID{gdctx.MyUUID}, | ||
| }, | ||
| { | ||
| DoFunc: "gfproxyd-start.Commit", | ||
| Nodes: txn.Nodes, | ||
| }, | ||
| unlock, | ||
| } | ||
|
|
||
| err = txn.Do() | ||
| if err != nil { | ||
| logger.WithFields(log.Fields{ | ||
| "error": err.Error(), | ||
| "volname": volname, | ||
| }).Error("failed to start gfproxy daemon") | ||
| restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err.Error(), api.ErrCodeDefault) | ||
| return | ||
| } | ||
|
|
||
| restutils.SendHTTPResponse(ctx, w, http.StatusOK, nil) | ||
| } | ||
|
|
||
| func gfproxydDisableHandler(w http.ResponseWriter, r *http.Request) { | ||
| // Collect inputs from URL | ||
| p := mux.Vars(r) | ||
| volname := p["name"] | ||
|
|
||
| ctx := r.Context() | ||
| logger := gdctx.GetReqLogger(ctx) | ||
|
|
||
| //validate volume name | ||
| volinfo, err := volume.GetVolume(volname) | ||
| if err != nil { | ||
| restutils.SendHTTPError(ctx, w, http.StatusNotFound, errors.ErrVolNotFound.Error(), api.ErrCodeDefault) | ||
| return | ||
| } | ||
|
|
||
| // Check if gfproxyd is already disabled | ||
| if !volume.IsGfproxydEnabled(volinfo) { | ||
| restutils.SendHTTPError(ctx, w, http.StatusBadRequest, errors.ErrGfproxydAlreadyDisabled.Error(), api.ErrCodeDefault) | ||
| return | ||
| } | ||
|
|
||
| // Transaction which stop gfproxyd on all nodes. | ||
| txn := transaction.NewTxn(ctx) | ||
| defer txn.Cleanup() | ||
|
|
||
| txn.Ctx.Set("volname", volname) | ||
|
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. volname is part of volinfo which you set below in the txn context |
||
|
|
||
| // Disable gfproxyd | ||
| volinfo.Options[volume.VkeyFeaturesGfproxyd] = "off" | ||
|
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. Same as above. Ctx.Set "volinfo" is required |
||
|
|
||
| if err := txn.Ctx.Set("volinfo", volinfo); err != nil { | ||
| logger.WithError(err).Error("failed to set volinfo in transaction context") | ||
| restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err.Error(), api.ErrCodeDefault) | ||
| return | ||
| } | ||
|
|
||
| //Lock on Volume Name | ||
| lock, unlock, err := transaction.CreateLockSteps(volname) | ||
| if err != nil { | ||
| restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err.Error(), api.ErrCodeDefault) | ||
| return | ||
| } | ||
|
|
||
| txn.Nodes = volinfo.Nodes() | ||
| txn.Steps = []*transaction.Step{ | ||
| lock, | ||
| { | ||
| DoFunc: "vol-option.UpdateVolinfo", | ||
| Nodes: []uuid.UUID{gdctx.MyUUID}, | ||
| }, | ||
| { | ||
| DoFunc: "gfproxyd-stop.Commit", | ||
| Nodes: txn.Nodes, | ||
| }, | ||
| unlock, | ||
| } | ||
|
|
||
| err = txn.Do() | ||
| if err != nil { | ||
| logger.WithFields(log.Fields{ | ||
| "error": err.Error(), | ||
| "volname": volname, | ||
| }).Error("failed to start gfproxy daemon") | ||
|
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. "failed to |
||
| restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err.Error(), api.ErrCodeDefault) | ||
| return | ||
| } | ||
|
|
||
| restutils.SendHTTPResponse(ctx, w, http.StatusOK, nil) | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| package gfproxyd | ||
|
|
||
| import ( | ||
| "github.com/gluster/glusterd2/glusterd2/daemon" | ||
| "github.com/gluster/glusterd2/glusterd2/transaction" | ||
| ) | ||
|
|
||
|
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. If you prefer, you can separate the common code in these functions into separate function. Here is an example. |
||
| func txnGfproxydStart(c transaction.TxnCtx) error { | ||
| var volname string | ||
| if err := c.Get("volname", &volname); err != nil { | ||
| c.Logger().WithError(err).WithField( | ||
| "key", "volname").Error("failed to get value for key from context") | ||
| return err | ||
| } | ||
|
|
||
| gfproxyd, err := newgfproxyd(volname) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| err = daemon.Start(gfproxyd, true) | ||
| return err | ||
| } | ||
|
|
||
| func txnGfproxydStop(c transaction.TxnCtx) error { | ||
| var volname string | ||
| if err := c.Get("volname", &volname); err != nil { | ||
| c.Logger().WithError(err).WithField( | ||
| "key", "volname").Error("failed to get value for key from context") | ||
| return err | ||
| } | ||
|
|
||
| gfproxyd, err := newgfproxyd(volname) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| err = daemon.Stop(gfproxyd, true) | ||
| return err | ||
| } | ||
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.
I don't know when this started, but features shouldn't be modifying core packages to add their stuff. Ideally, to add these sorts of keys, the core package should provide some sort of interface that plugins use to add things they want.
As I see, currently this isn't used else where out of the gfproxyd plugin. So you can just move it to the plugin.
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.
Volfile generation may use it. This is common problem for many features. For example bitrot. Unless we provide a way to store enabled features this we can't avoid.
I asked the same question here #413 (comment)
I feel we should allow this till we implement infra to store this info.
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.
As @kshlm suggested, move this to
plugins/gfproxyd