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

Check whether peer has devices before removing peer #1421

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 15 additions & 1 deletion e2e/peer_ops_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,21 @@ func TestAddRemovePeer(t *testing.T) {
r.Len(peers, 0)
}

// remove peer: ask g1 to remove g2 as peer
devicesDir := testTempDir(t, "devices")
r.Nil(prepareLoopDevice(devicesDir+"/gluster_dev1.img", "1", "250M"))
_, err = client.DeviceAdd(g2.PeerID(), "/dev/gluster_loop1")
r.Nil(err)

dev, err := client.DeviceList(g2.PeerID(), "/dev/gluster_loop1")
r.Nil(err)
r.Equal(dev[0].Device, "/dev/gluster_loop1")

err = client.PeerRemove(g2.PeerID())
Copy link
Member

Choose a reason for hiding this comment

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

once the peer remove failed, please remove the device and then try removing the peer again

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will remove the device once the delete device PR is in. Let's wait for the PR(#1120) to get merged.

r.NotNil(err)

err = client.PeerRemove(g3.PeerID())
r.Nil(err)

// Device Cleanup
r.Nil(loopDevicesCleanup(t))
}
22 changes: 22 additions & 0 deletions glusterd2/commands/peers/deletepeer.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/gluster/glusterd2/glusterd2/store"
"github.com/gluster/glusterd2/glusterd2/volume"
"github.com/gluster/glusterd2/pkg/utils"
"github.com/gluster/glusterd2/plugins/device/deviceutils"

"github.com/gorilla/mux"
"github.com/pborman/uuid"
Expand Down Expand Up @@ -70,6 +71,16 @@ func deletePeerHandler(w http.ResponseWriter, r *http.Request) {
return
}

if exists, err := deviceExist(id); err != nil {
logger.WithError(err).Error("failed to check if device exist on peer")
restutils.SendHTTPError(ctx, w, http.StatusInternalServerError, "could not validate delete peer request")
return
} else if exists {
logger.Debug("request denied, peer has devices")
restutils.SendHTTPError(ctx, w, http.StatusForbidden, "cannot delete peer, peer has device/devices")
return
}

remotePeerAddress, err := utils.FormRemotePeerAddress(p.PeerAddresses[0])
if err != nil {
logger.WithError(err).WithField("address", p.PeerAddresses[0]).Error("failed to parse peer address")
Expand Down Expand Up @@ -119,6 +130,17 @@ func deletePeerHandler(w http.ResponseWriter, r *http.Request) {
events.Broadcast(newPeerEvent(eventPeerRemoved, p))
}

func deviceExist(id string) (bool, error) {
deviceInfo, err := deviceutils.GetDevices(id)
if err != nil {
return false, err
}
if len(deviceInfo) > 0 {
return true, nil
}
return false, nil
}

// bricksExist checks if the given peer has any bricks on it
// TODO: Move this to a more appropriate place
func bricksExist(id string) (bool, error) {
Expand Down
8 changes: 8 additions & 0 deletions pkg/restclient/device.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,11 @@ func (c *Client) DeviceEdit(peerid, device, state string) error {
err := c.post(url, req, http.StatusOK, nil)
return err
}

// DevicesInPeer lists devices in peer
func (c *Client) DevicesInPeer(peerid string) (deviceapi.ListDeviceResp, error) {
Copy link
Member

Choose a reason for hiding this comment

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

why this is needed?
we already have an API to do this functionality. https://github.com/gluster/glusterd2/blob/master/pkg/restclient/device.go#L26

var deviceList deviceapi.ListDeviceResp
url := fmt.Sprintf("/v1/devices/%s", peerid)
err := c.get(url, nil, http.StatusOK, &deviceList)
return deviceList, err
}