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

Edit Zone API #603

Merged
merged 27 commits into from
May 23, 2018
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
3caf951
Edit Group API
rishubhjain Mar 15, 2018
76324c9
Updating code
rishubhjain Mar 15, 2018
fe15576
Merge branch 'master' of https://github.com/gluster/glusterd2 into group
rishubhjain Mar 19, 2018
9d1d4bf
Updating code
rishubhjain Mar 26, 2018
8ec64a7
Merge branch 'master' of https://github.com/gluster/glusterd2 into group
rishubhjain Mar 31, 2018
9ab82ca
Checking device if it already exists before adding in store
rishubhjain Apr 1, 2018
13c874d
Merge branch 'master' of https://github.com/gluster/glusterd2 into group
rishubhjain Apr 2, 2018
7ea6120
Handling device setup failures
rishubhjain Apr 2, 2018
3f6a1dd
Merge branch 'master' of https://github.com/gluster/glusterd2 into group
rishubhjain Apr 2, 2018
1f94dd3
Handling loosing metadata when glusterd restarts
rishubhjain Apr 3, 2018
c57768e
Merge branch 'master' of https://github.com/gluster/glusterd2 into group
rishubhjain Apr 3, 2018
47f90bd
Merge branch 'master' of https://github.com/gluster/glusterd2 into group
rishubhjain Apr 4, 2018
84e8866
Using Metadata instead of MetaData
rishubhjain Apr 5, 2018
f11f68e
Merge branch 'master' of https://github.com/gluster/glusterd2 into group
rishubhjain Apr 5, 2018
f2e2b8c
Merge branch 'master' of https://github.com/gluster/glusterd2 into group
rishubhjain Apr 5, 2018
cfd8136
Merge branch 'master' of https://github.com/gluster/glusterd2 into group
rishubhjain Apr 6, 2018
4e12707
Merge branch 'master' of https://github.com/gluster/glusterd2 into group
rishubhjain Apr 17, 2018
ec3b9c0
Merge branch 'master' of https://github.com/gluster/glusterd2 into group
rishubhjain Apr 18, 2018
1a6c001
Merge branch 'master' of https://github.com/gluster/glusterd2 into group
rishubhjain Apr 19, 2018
8282ce4
Merge branch 'master' of https://github.com/gluster/glusterd2 into group
rishubhjain Apr 26, 2018
d589c4d
Merge branch 'master' of https://github.com/gluster/glusterd2 into group
rishubhjain Apr 30, 2018
7d905c7
Merge branch 'master' of https://github.com/gluster/glusterd2 into group
rishubhjain May 2, 2018
552fbb4
Merge branch 'master' of https://github.com/gluster/glusterd2 into group
rishubhjain May 7, 2018
034fa48
Merge branch 'master' of https://github.com/gluster/glusterd2 into group
rishubhjain May 11, 2018
3ed7136
Renaming commands folder to utils
rishubhjain May 21, 2018
62273e4
Merge branch 'master' of https://github.com/gluster/glusterd2 into group
rishubhjain May 21, 2018
8ad368d
Merge branch 'master' of https://github.com/gluster/glusterd2 into group
rishubhjain May 23, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions glusterd2/commands/peers/addpeer.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package peercommands
import (
"fmt"
"net/http"
"strings"

"github.com/gluster/glusterd2/glusterd2/events"
"github.com/gluster/glusterd2/glusterd2/gdctx"
Expand All @@ -25,6 +26,14 @@ func addPeerHandler(w http.ResponseWriter, r *http.Request) {
return
}

for key := range req.Metadata {
if strings.HasPrefix(key, "_") {
logger.WithField("metadata-key", key).Error("Key names starting with '_' are restricted in metadata field")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error can be added to generic errors list. Used this in multiple places and also Volume metadata will also requires this error msg.

restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, "Key names starting with '_' are restricted in metadata field")
return
}
}

if len(req.Addresses) < 1 {
restutils.SendHTTPError(ctx, w, http.StatusBadRequest, errors.ErrNoHostnamesPresent)
return
Expand Down Expand Up @@ -84,7 +93,13 @@ func addPeerHandler(w http.ResponseWriter, r *http.Request) {
return
}

newpeer.MetaData = req.MetaData
if req.Zone != "" {
newpeer.Metadata["_zone"] = req.Zone
}
for key, value := range req.Metadata {
newpeer.Metadata[key] = value
}

err = peer.AddOrUpdatePeer(newpeer)
if err != nil {
restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, "Fail to add metadata to peer")
Expand All @@ -104,6 +119,6 @@ func createPeerAddResp(p *peer.Peer) *api.PeerAddResp {
Name: p.Name,
PeerAddresses: p.PeerAddresses,
ClientAddresses: p.ClientAddresses,
MetaData: p.MetaData,
Metadata: p.Metadata,
}
}
20 changes: 10 additions & 10 deletions glusterd2/commands/peers/editpeer.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func editPeer(w http.ResponseWriter, r *http.Request) {
return
}

for key := range req.MetaData {
for key := range req.Metadata {
if strings.HasPrefix(key, "_") {
logger.WithField("metadata-key", key).Error("Key names starting with '_' are restricted in metadata field")
restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, "Key names starting with '_' are restricted in metadata field")
Expand All @@ -58,7 +58,7 @@ func editPeer(w http.ResponseWriter, r *http.Request) {
}
err = txn.Ctx.Set("peerid", peerID)
if err != nil {
logger.WithError(err).WithField("key", peerID).Error("Failed to set key in transaction context")
logger.WithError(err).WithField("key", "peerid").WithField("value", peerID).Error("Failed to set key in transaction context")
restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
return
}
Expand Down Expand Up @@ -102,13 +102,13 @@ func txnPeerEdit(c transaction.TxnCtx) error {
c.Logger().WithError(err).WithField("peerid", peerID).Error("Peer ID not found in store")
return err
}
for k, v := range req.MetaData {
if peerInfo.MetaData != nil {
peerInfo.MetaData[k] = v
} else {
peerInfo.MetaData = make(map[string]string)
peerInfo.MetaData[k] = v
}

for k, v := range req.Metadata {
peerInfo.Metadata[k] = v
}

if req.Zone != "" {
peerInfo.Metadata["_zone"] = req.Zone
}
err = peer.AddOrUpdatePeer(peerInfo)
if err != nil {
Expand Down Expand Up @@ -141,6 +141,6 @@ func createPeerEditResp(p *peer.Peer) *api.PeerEditResp {
Name: p.Name,
PeerAddresses: p.PeerAddresses,
ClientAddresses: p.ClientAddresses,
MetaData: p.MetaData,
Metadata: p.Metadata,
}
}
2 changes: 1 addition & 1 deletion glusterd2/commands/peers/getpeer.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,6 @@ func createPeerGetResp(p *peer.Peer) *api.PeerGetResp {
PeerAddresses: p.PeerAddresses,
ClientAddresses: p.ClientAddresses,
Online: store.Store.IsNodeAlive(p.ID),
MetaData: p.MetaData,
Metadata: p.Metadata,
}
}
2 changes: 1 addition & 1 deletion glusterd2/commands/peers/getpeers.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func createPeerListResp(peers []*peer.Peer) *api.PeerListResp {
PeerAddresses: p.PeerAddresses,
ClientAddresses: p.ClientAddresses,
Online: store.Store.IsNodeAlive(p.ID),
MetaData: p.MetaData,
Metadata: p.Metadata,
})
}

Expand Down
2 changes: 1 addition & 1 deletion glusterd2/peer/peer.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ type Peer struct {
Name string
PeerAddresses []string
ClientAddresses []string
MetaData map[string]string
Metadata map[string]string
}

// ETCDConfig represents the structure which holds the ETCD env variables &
Expand Down
14 changes: 12 additions & 2 deletions glusterd2/peer/self.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"net"

"github.com/gluster/glusterd2/glusterd2/gdctx"
"github.com/gluster/glusterd2/pkg/errors"

config "github.com/spf13/viper"
)
Expand Down Expand Up @@ -39,18 +40,27 @@ func normalizeAddrs() ([]string, error) {

// AddSelfDetails results in the peer adding its own details into etcd
func AddSelfDetails() error {
var err error

var err error
p := &Peer{
ID: gdctx.MyUUID,
Name: gdctx.HostName,
PeerAddresses: []string{config.GetString("peeraddress")},
}

p.ClientAddresses, err = normalizeAddrs()
if err != nil {
return err
}

peerInfo, err := GetPeer(gdctx.MyUUID.String())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why these changes required?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case the glusterd2 service was restarted and the data (metadata ) already existed then it is important to go and get back the data .

if err == errors.ErrPeerNotFound {
p.Metadata = make(map[string]string)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have check err and handle errors.ErrPeerNotFound separate from rest of the errors. Peer not found is not an error. Can't access the store is an error.

p.Metadata["_zone"] = p.ID.String()
} else if err == nil && peerInfo != nil {
p.Metadata = peerInfo.Metadata
} else {
return err
}

return AddOrUpdatePeer(p)
}
10 changes: 6 additions & 4 deletions pkg/api/peer_req_resp.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,26 @@ type Peer struct {
PeerAddresses []string `json:"peer-addresses"`
ClientAddresses []string `json:"client-addresses"`
Online bool `json:"online"`
MetaData map[string]string `json:"metadata"`
Metadata map[string]string `json:"metadata"`
}

// PeerAddReq represents an incoming request to add a peer to the cluster
type PeerAddReq struct {
Addresses []string `json:"addresses"`
MetaData map[string]string `json:"metadata"`
Zone string `json:"zone,omitempty"`
Metadata map[string]string `json:"metadata,omitempty"`
}

// PeerEditReq represents an incoming request to edit metadata of peer
type PeerEditReq struct {
MetaData map[string]string `json:"metadata"`
Zone string `json:"zone"`
Metadata map[string]string `json:"metadata"`
}

// PeerAddResp is the success response sent to a PeerAddReq request
type PeerAddResp Peer

// PeerEditResp is the success response sent to a MetaDataEditReq request
// PeerEditResp is the success response sent to a PeerEditReq request
type PeerEditResp Peer

// PeerGetResp is the response sent for a peer get request
Expand Down
2 changes: 1 addition & 1 deletion plugins/device/api/resp.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"github.com/gluster/glusterd2/pkg/api"
)

// Info represents structure in which devices are to be store in Peer MetaData
// Info represents structure in which devices are to be store in Peer Metadata
type Info struct {
Name string `json:"name"`
State string `json:"state"`
Expand Down
51 changes: 51 additions & 0 deletions plugins/device/cmdexec/device.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package cmdexec
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move only device commands to plugins/device/deviceutils/utils.go

package deviceutils
func CreatePV
func CreateVG
func RemovePV
func RemoveVG

Move other parts of code back to transactions.go.

plugins/device/deviceutils/utils.go can be moved to pkg/deviceutils later if required.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is there a need to move commands to device utils?, won't it be useful if a separate library is made for commands

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cmdexec is not a good name for library, doesn't give any clue what that cmdexec is. Another problem with current approach is that this library can't be used without having Transaction context. Library should be independent of Transaction/etcd so that it can be reused in other places like Tests.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the discussion with you, I grouped the device related functions in separate file here 2adafaa#diff-388b0ce2a0b41316c76c470557c4cbd1


import (
"os/exec"
"strings"

"github.com/gluster/glusterd2/glusterd2/transaction"
)

func createVgName(device string) string {
vgName := strings.Replace("vg"+device, "/", "-", -1)
return vgName
}

// DeviceSetup is used to prepare device before using devices.
func DeviceSetup(c transaction.TxnCtx, device string) error {

var err error
defer func() {
if err != nil {
DeviceCleanup(c, device)
}
}()
pvcreateCmd := exec.Command("pvcreate", "--metadatasize=128M", "--dataalignment=256K", device)
if err := pvcreateCmd.Run(); err != nil {
c.Logger().WithError(err).WithField("device", device).Error("pvcreate failed for device")
return err
}
vgName := createVgName(device)
vgcreateCmd := exec.Command("vgcreate", vgName, device)
if err = vgcreateCmd.Run(); err != nil {
c.Logger().WithError(err).WithField("device", device).Error("vgcreate failed for device")
return err
}

return nil

}

// DeviceCleanup is used to clean up devices.
func DeviceCleanup(c transaction.TxnCtx, device string) {
vgName := createVgName(device)
vgremoveCmd := exec.Command("vgremove", vgName)
if err := vgremoveCmd.Run(); err != nil {
c.Logger().WithError(err).WithField("device", device).Error("vgremove failed for device")
}
pvremoveCmd := exec.Command("pvremove", device)
if err := pvremoveCmd.Run(); err != nil {
c.Logger().WithError(err).WithField("device", device).Error("pvremove failed for device")
}
}
3 changes: 2 additions & 1 deletion plugins/device/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ func (p *Plugin) RestRoutes() route.Routes {
Version: 1,
RequestType: utils.GetTypeString((*deviceapi.AddDeviceReq)(nil)),
ResponseType: utils.GetTypeString((*deviceapi.AddDeviceResp)(nil)),
HandlerFunc: deviceAddHandler},
HandlerFunc: deviceAddHandler,
},
}
}

Expand Down
47 changes: 36 additions & 11 deletions plugins/device/rest.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package device

import (
"encoding/json"
"net/http"

"github.com/gluster/glusterd2/glusterd2/gdctx"
Expand All @@ -18,18 +19,30 @@ func deviceAddHandler(w http.ResponseWriter, r *http.Request) {

ctx := r.Context()
logger := gdctx.GetReqLogger(ctx)
peerID := mux.Vars(r)["peerid"]
if uuid.Parse(peerID) == nil {
restutils.SendHTTPError(ctx, w, http.StatusBadRequest, "Invalid peer-id passed in url")
return
}

req := new(deviceapi.AddDeviceReq)
if err := restutils.UnmarshalRequest(r, req); err != nil {
logger.WithError(err).Error("Failed to Unmarshal request")
restutils.SendHTTPError(ctx, w, http.StatusBadRequest, errors.ErrJSONParsingFailed)
return
}
peerID := mux.Vars(r)["peerid"]
if peerID == "" {
restutils.SendHTTPError(ctx, w, http.StatusBadRequest, "peerid not present in request")

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lock here.

lock, unlock := transaction.CreateLockFuncs(peerID)
if err := lock(ctx); err != nil {
if err == transaction.ErrLockTimeout {
restutils.SendHTTPError(ctx, w, http.StatusConflict, err)
} else {
restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
}
return
}
defer unlock(ctx)

peerInfo, err := peer.GetPeer(peerID)
if err != nil {
logger.WithError(err).WithField("peerid", peerID).Error("Peer ID not found in store")
Expand All @@ -40,27 +53,39 @@ func deviceAddHandler(w http.ResponseWriter, r *http.Request) {
}
return
}

var devices []deviceapi.Info
err = json.Unmarshal([]byte(peerInfo.Metadata["_"]), &devices)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

peerInfo.Metadata["_devices"]?

if err != nil {
logger.WithError(err).WithField("peerid", peerID).Error(err)
restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
return
}
if !CheckIfDeviceExist(req.Devices, devices) {
logger.WithError(err).WithField("device", req.Devices).Error(" One or more devices already exists")
restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, " One or more devices already exists")
return
}

txn := transaction.NewTxn(ctx)
defer txn.Cleanup()
lock, unlock, err := transaction.CreateLockSteps(peerInfo.ID.String())

txn.Nodes = []uuid.UUID{peerInfo.ID}
txn.Steps = []*transaction.Step{
lock,
{
DoFunc: "prepare-device",
Nodes: txn.Nodes,
},
unlock,
}
err = txn.Ctx.Set("peerid", peerID)
err = txn.Ctx.Set("peerid", &peerID)
if err != nil {
logger.WithError(err).Error("Failed to set data for transaction")
logger.WithError(err).WithField("key", "peerid").WithField("value", peerID).Error("Failed to set key in transaction context")
restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
return
}
err = txn.Ctx.Set("req", req)
err = txn.Ctx.Set("", &req.Devices)
if err != nil {
logger.WithError(err).Error("Failed to set data for transaction")
logger.WithError(err).WithField("key", "").Error("Failed to set key in transaction context")
restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, err)
return
}
Expand All @@ -72,7 +97,7 @@ func deviceAddHandler(w http.ResponseWriter, r *http.Request) {
}
peerInfo, err = peer.GetPeer(peerID)
if err != nil {
logger.WithError(err).Error("Failed to get peer from store")
logger.WithError(err).WithField("peerid", peerID).Error("Failed to get peer from store")
restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, "Failed to get peer from store")
return
}
Expand Down
19 changes: 16 additions & 3 deletions plugins/device/store-utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,29 @@ func GetDevices(peerID string) ([]deviceapi.Info, error) {
if err != nil {
return nil, err
}
if len(peerInfo.MetaData["devices"]) > 0 {
if len(peerInfo.Metadata["_devices"]) > 0 {
var deviceInfo []deviceapi.Info
if err := json.Unmarshal([]byte(peerInfo.MetaData["devices"]), &deviceInfo); err != nil {
if err := json.Unmarshal([]byte(peerInfo.Metadata["_devices"]), &deviceInfo); err != nil {
return nil, err
}
return deviceInfo, nil
}
return nil, nil
}

//CheckIfDeviceExist returns error if all devices already exist or returns list of devices to be added
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CheckIfDeviceExist() can be simplified to just return a bool.
The Unmarshalling can be moved out to the caller.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect comment: This function returns true if any of the devices exist.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, do not export this function until you have an external consumer out of this package.

func CheckIfDeviceExist(reqDevices []string, devices []deviceapi.Info) bool {

for _, key := range reqDevices {
for _, reqKey := range devices {
if key == reqKey.Name {
return false
}
}
}
return true
}

// AddDevices adds device to specific peer
func AddDevices(devices []deviceapi.Info, peerID string) error {
deviceDetails, err := GetDevices(peerID)
Expand All @@ -40,7 +53,7 @@ func AddDevices(devices []deviceapi.Info, peerID string) error {
if err != nil {
return err
}
peerInfo.MetaData["devices"] = string(deviceJSON)
peerInfo.Metadata["_devices"] = string(deviceJSON)
err = peer.AddOrUpdatePeer(peerInfo)
if err != nil {
return err
Expand Down
Loading