Skip to content
This repository has been archived by the owner on Mar 26, 2020. It is now read-only.

Commit

Permalink
code clean up in the glusterd2 plugins
Browse files Browse the repository at this point in the history 
Added logging with the log.WithError().
Allocate capacity to slice to reduce the allocation of memory during append.
Added UNDO function in txn steps.

Signed-off-by: Madhu Rajanna <[email protected]>
  • Loading branch information
@Madhu-1 @prashanthpai
Madhu-1 authored and prashanthpai committed Jul 12, 2018
1 parent a28efc6 commit febde46
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 80 deletions.
10 changes: 3 additions & 7 deletions plugins/dht/validate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,13 @@ import (

var names = [...]string{"distribute", "dht"}

func validateOptions(v *volume.Volinfo, key string, value string) error {
var err error
func validateOptions(v *volume.Volinfo, key, value string) error {
if strings.Contains(key, "readdirplus-for-dir") {
if value == "on" {
val, exists := v.Options["features.cache-invalidation"]
if exists && val == "on" {
if v, ok := v.Options["features.cache-invalidation"]; ok && v == "on" {
return nil
}
err = fmt.Errorf("Enable \"features.cache-invalidation\" before enabling %s",
key)
return err
return fmt.Errorf("Enable \"features.cache-invalidation\" before enabling %s", key)
}
}
return nil
Expand Down
79 changes: 36 additions & 43 deletions plugins/georeplication/rest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
)

// newGeorepSession creates new instance of GeorepSession
func newGeorepSession(mastervolid uuid.UUID, remotevolid uuid.UUID, req georepapi.GeorepCreateReq) *georepapi.GeorepSession {
func newGeorepSession(mastervolid, remotevolid uuid.UUID, req georepapi.GeorepCreateReq) *georepapi.GeorepSession {
remoteUser := req.RemoteUser
if req.RemoteUser == "" {
remoteUser = "root"
Expand All @@ -49,7 +49,7 @@ func newGeorepSession(mastervolid uuid.UUID, remotevolid uuid.UUID, req georepap
}
}

func validateMasterAndRemoteIDFormat(ctx context.Context, w http.ResponseWriter, masteridRaw string, remoteidRaw string) (uuid.UUID, uuid.UUID, error) {
func validateMasterAndRemoteIDFormat(ctx context.Context, w http.ResponseWriter, masteridRaw, remoteidRaw string) (uuid.UUID, uuid.UUID, error) {
// Validate UUID format of Master and Remote Volume ID
masterid := uuid.Parse(masteridRaw)
if masterid == nil {
Expand Down Expand Up @@ -163,6 +163,9 @@ func georepCreateHandler(w http.ResponseWriter, r *http.Request) {
geoSession.Options["gluster-logdir"] = path.Join(config.GetString("logdir"), "glusterfs")
}

//store volinfo to revert back changes in case of transaction failure
oldvolinfo := vol

// Required Volume Options
vol.Options["marker.xtime"] = "on"
vol.Options["marker.gsync-force-xtime"] = "on"
Expand All @@ -171,11 +174,19 @@ func georepCreateHandler(w http.ResponseWriter, r *http.Request) {
// Workaround till {{ volume.id }} added to the marker options table
vol.Options["marker.volume-uuid"] = vol.ID.String()

//save volume information for transaction failure scenario
if err := txn.Ctx.Set("oldvolinfo", oldvolinfo); err != nil {
logger.WithError(err).Error("failed to set oldvolinfo in transaction context")
restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
return
}

txn.Nodes = vol.Nodes()
txn.Steps = []*transaction.Step{
{
DoFunc: "vol-option.UpdateVolinfo",
Nodes: []uuid.UUID{gdctx.MyUUID},
DoFunc: "vol-option.UpdateVolinfo",
UndoFunc: "vol-option.UpdateVolinfo.Undo",
Nodes: []uuid.UUID{gdctx.MyUUID},
},
{
DoFunc: "vol-option.NotifyVolfileChange",
Expand All @@ -198,10 +209,8 @@ func georepCreateHandler(w http.ResponseWriter, r *http.Request) {
return
}

err = txn.Do()
if err != nil {
logger.WithFields(log.Fields{
"error": err.Error(),
if err = txn.Do(); err != nil {
logger.WithError(err).WithFields(log.Fields{
"mastervolid": masterid,
"remotevolid": remoteid,
}).Error("failed to create geo-replication session")
Expand Down Expand Up @@ -334,8 +343,7 @@ func georepActionHandler(w http.ResponseWriter, r *http.Request, action actionTy

err = txn.Do()
if err != nil {
logger.WithFields(log.Fields{
"error": err.Error(),
logger.WithError(err).WithFields(log.Fields{
"mastervolid": masterid,
"remotevolid": remoteid,
}).Error("failed to " + action.String() + " geo-replication session")
Expand Down Expand Up @@ -436,8 +444,7 @@ func georepDeleteHandler(w http.ResponseWriter, r *http.Request) {

err = txn.Do()
if err != nil {
logger.WithFields(log.Fields{
"error": err.Error(),
logger.WithError(err).WithFields(log.Fields{
"mastervolid": masterid,
"remotevolid": remoteid,
}).Error("failed to delete geo-replication session")
Expand Down Expand Up @@ -516,9 +523,7 @@ func georepStatusHandler(w http.ResponseWriter, r *http.Request) {
err = txn.Do()
if err != nil {
// TODO: Handle partial failure if a few glusterd's down

logger.WithFields(log.Fields{
"error": err.Error(),
logger.WithError(err).WithFields(log.Fields{
"mastervolid": masterid,
"remotevolid": remoteid,
}).Error("failed to get status of geo-replication session")
Expand All @@ -530,12 +535,16 @@ func georepStatusHandler(w http.ResponseWriter, r *http.Request) {
result, err := aggregateGsyncdStatus(txn.Ctx, txn.Nodes)
if err != nil {
errMsg := "Failed to aggregate gsyncd status results from multiple nodes."
logger.WithField("error", err.Error()).Error("gsyncdStatusHandler:" + errMsg)
logger.WithError(err).Error("gsyncdStatusHandler:" + errMsg)
restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, errMsg)
return
}

for _, b := range vol.GetBricks() {
bricks := vol.GetBricks()
geoSession.Workers = make([]georepapi.GeorepWorker, 0, len(bricks))

for _, b := range bricks {

// Set default values to all status fields, If a node or worker is down and
// status not available these default values will be sent back in response
geoSession.Workers = append(geoSession.Workers, georepapi.GeorepWorker{
Expand Down Expand Up @@ -639,8 +648,7 @@ func georepConfigGetHandler(w http.ResponseWriter, r *http.Request) {
}
out, err := utils.ExecuteCommandOutput(gsyncdCommand, args...)
if err != nil {
logger.WithFields(log.Fields{
"error": err.Error(),
logger.WithError(err).WithFields(log.Fields{
"mastervolid": masterid,
"remotevolid": remoteid,
}).Error("failed to get session configurations")
Expand All @@ -650,8 +658,7 @@ func georepConfigGetHandler(w http.ResponseWriter, r *http.Request) {

var opts []georepapi.GeorepOption
if err = json.Unmarshal(out, &opts); err != nil {
logger.WithFields(log.Fields{
"error": err.Error(),
logger.WithError(err).WithFields(log.Fields{
"mastervolid": masterid,
"remotevolid": remoteid,
}).Error("failed to parse configurations")
Expand Down Expand Up @@ -725,8 +732,7 @@ func georepConfigSetHandler(w http.ResponseWriter, r *http.Request) {
val, ok := geoSession.Options[k]
if (ok && v != val) || !ok {
configWillChange = true
err = checkConfig(k, v)
if err != nil {
if err = checkConfig(k, v); err != nil {
restutils.SendHTTPError(ctx, w, http.StatusBadRequest, "Invalid Config Name/Value")
return
}
Expand Down Expand Up @@ -803,16 +809,15 @@ func georepConfigSetHandler(w http.ResponseWriter, r *http.Request) {

err = txn.Do()
if err != nil {
logger.WithFields(log.Fields{
"error": err.Error(),
logger.WithError(err).WithFields(log.Fields{
"mastervolid": masterid,
"remotevolid": remoteid,
}).Error("failed to update geo-replication session config")
restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
return
}

var allopts []string
var allopts = make([]string, 0, len(req))
for k, v := range req {
allopts = append(allopts, k+"="+v)
}
Expand Down Expand Up @@ -863,10 +868,8 @@ func georepConfigResetHandler(w http.ResponseWriter, r *http.Request) {
restartRequired := false
// Check if config exists, reset can be done only if it is set before
for _, k := range req {
_, ok := geoSession.Options[k]
if ok {
if _, ok := geoSession.Options[k]; ok {
configWillChange = true

restartRequired = restartRequiredOnConfigChange(k)
}
}
Expand Down Expand Up @@ -939,8 +942,7 @@ func georepConfigResetHandler(w http.ResponseWriter, r *http.Request) {

err = txn.Do()
if err != nil {
logger.WithFields(log.Fields{
"error": err.Error(),
logger.WithError(err).WithFields(log.Fields{
"mastervolid": masterid,
"remotevolid": remoteid,
}).Error("failed to update geo-replication session config")
Expand Down Expand Up @@ -1007,10 +1009,7 @@ func georepSSHKeyGenerateHandler(w http.ResponseWriter, r *http.Request) {

err = txn.Do()
if err != nil {
logger.WithFields(log.Fields{
"error": err.Error(),
"volname": volname,
}).Error("failed to generate SSH Keys")
logger.WithError(err).WithField("volname", volname).Error("failed to generate SSH Keys")
restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
return
}
Expand All @@ -1034,10 +1033,7 @@ func georepSSHKeyGetHandler(w http.ResponseWriter, r *http.Request) {

sshkeys, err := getSSHPublicKeys(volname)
if err != nil {
logger.WithFields(log.Fields{
"error": err.Error(),
"volname": volname,
}).Error("failed to get SSH public Keys")
logger.WithError(err).WithField("volname", volname).Error("failed to get SSH public Keys")
restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
return
}
Expand Down Expand Up @@ -1101,10 +1097,7 @@ func georepSSHKeyPushHandler(w http.ResponseWriter, r *http.Request) {

err = txn.Do()
if err != nil {
logger.WithFields(log.Fields{
"error": err.Error(),
"volname": volname,
}).Error("failed to push SSH Keys")
logger.WithError(err).WithField("volname", volname).Error("failed to push SSH Keys")
restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
return
}
Expand Down
14 changes: 4 additions & 10 deletions plugins/georeplication/store-utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func getSession(masterid string, remoteid string) (*georepapi.GeorepSession, err
func addOrUpdateSession(v *georepapi.GeorepSession) error {
json, e := json.Marshal(v)
if e != nil {
log.WithField("error", e).Error("Failed to marshal the Info object")
log.WithError(e).Error("Failed to marshal the Info object")
return e
}

Expand Down Expand Up @@ -77,10 +77,7 @@ func getSessionList() ([]*georepapi.GeorepSession, error) {
var session georepapi.GeorepSession

if err := json.Unmarshal(kv.Value, &session); err != nil {
log.WithFields(log.Fields{
"session": string(kv.Key),
"error": err,
}).Error("Failed to unmarshal Geo-replication session")
log.WithError(err).WithField("session", string(kv.Key)).Error("Failed to unmarshal Geo-replication session")
continue
}

Expand All @@ -94,7 +91,7 @@ func getSessionList() ([]*georepapi.GeorepSession, error) {
func addOrUpdateSSHKey(volname string, sshkey georepapi.GeorepSSHPublicKey) error {
json, e := json.Marshal(sshkey)
if e != nil {
log.WithField("error", e).Error("Failed to marshal the sshkeys object")
log.WithError(e).Error("Failed to marshal the sshkeys object")
return e
}

Expand All @@ -110,10 +107,7 @@ func addOrUpdateSSHKey(volname string, sshkey georepapi.GeorepSSHPublicKey) erro
func getSSHPublicKeys(volname string) ([]georepapi.GeorepSSHPublicKey, error) {
resp, e := store.Get(context.TODO(), georepSSHKeysPrefix+volname, clientv3.WithPrefix())
if e != nil {
log.WithFields(log.Fields{
"volname": volname,
"error": e,
}).Error("Couldn't retrive SSH Key from the node")
log.WithError(e).WithField("volname", volname).Error("Couldn't retrive SSH Key from the node")
return nil, e
}

Expand Down
31 changes: 14 additions & 17 deletions plugins/georeplication/transactions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ func configFileGenerate(session *georepapi.GeorepSession) error {
}

// Remote host and UUID details
var remote []string
var remote = make([]string, 0, len(session.RemoteHosts))
for _, sh := range session.RemoteHosts {
remote = append(remote, sh.PeerID.String()+":"+sh.Hostname)
}
Expand All @@ -264,8 +264,9 @@ func configFileGenerate(session *georepapi.GeorepSession) error {
)

// Master Bricks details
var master []string
for _, b := range vol.GetBricks() {
bricks := vol.GetBricks()
var master = make([]string, 0, len(bricks))
for _, b := range bricks {
master = append(master, b.PeerID.String()+":"+b.Hostname+":"+b.Path)
}
confdata = append(confdata,
Expand Down Expand Up @@ -322,19 +323,18 @@ func txnGeorepConfigFilegen(c transaction.TxnCtx) error {
}

if restartRequired {
err = gsyncdAction(c, actionStop)
if err != nil {

if err = gsyncdAction(c, actionStop); err != nil {
return err
}
err = gsyncdAction(c, actionStart)
if err != nil {

if err = gsyncdAction(c, actionStart); err != nil {
return err
}
} else {
// Restart not required, Generate config file Gsynd will reload
// automatically if running
err = configFileGenerate(&session)
if err != nil {
if err = configFileGenerate(&session); err != nil {
return err
}
}
Expand Down Expand Up @@ -362,8 +362,8 @@ func txnSSHKeysGenerate(c transaction.TxnCtx) error {
)

// Create Directory if not exists
err = os.MkdirAll(path.Dir(secretPemFile), os.ModeDir|os.ModePerm)
if err != nil {

if err = os.MkdirAll(path.Dir(secretPemFile), os.ModeDir|os.ModePerm); err != nil {
return err
}

Expand Down Expand Up @@ -392,17 +392,14 @@ func txnSSHKeysGenerate(c transaction.TxnCtx) error {
return err
}
}
data, err = ioutil.ReadFile(tarSSHPemFile + ".pub")
if err != nil {
if data, err = ioutil.ReadFile(tarSSHPemFile + ".pub"); err != nil {
return err
}
sshkey.TarKey = string(data)

err = addOrUpdateSSHKey(volname, sshkey)
if err != nil {
return err
}
return nil

return err
}

func txnSSHKeysPush(c transaction.TxnCtx) error {
Expand Down
6 changes: 3 additions & 3 deletions plugins/glustershd/rest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ func runGlfshealBin(volname string, args []string) (string, error) {

cmd := exec.Command(path, args...)
cmd.Stdout = &out
err = cmd.Run()
if err != nil {

if err = cmd.Run(); err != nil {
return healInfoOutput, err
}

Expand Down Expand Up @@ -103,7 +103,7 @@ func selfhealInfoHandler(w http.ResponseWriter, r *http.Request) {
}
healInfoOutput, err := getHealInfo(volname, option)
if err != nil {
logger.WithField("volname", volname).Debug("heal info operation failed")
logger.WithError(err).WithField("volname", volname).Error("heal info operation failed")
restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, "heal info operation failed")
return
}
Expand Down

0 comments on commit febde46

Please sign in to comment.