Skip to content

Commit

Permalink
rbd: add volume locks for reclaimspace operations
Browse files Browse the repository at this point in the history
This commit adds locks on reclaimspace operations to
prevent multiple process executing rbd sparsify/fstrim
on same volume.

Signed-off-by: Praveen M <m.praveen@ibm.com>
  • Loading branch information
iPraveenParihar committed May 27, 2024
1 parent 822794c commit 0d019f8
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 8 deletions.
26 changes: 22 additions & 4 deletions internal/csi-addons/rbd/reclaimspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,14 @@ import (
// of CSI-addons reclaimspace controller service spec.
type ReclaimSpaceControllerServer struct {
*rs.UnimplementedReclaimSpaceControllerServer
// Embed ControllerServer as it implements helper functions
*rbdutil.ControllerServer
}

// NewReclaimSpaceControllerServer creates a new ReclaimSpaceControllerServer which handles
// the ReclaimSpace Service requests from the CSI-Addons specification.
func NewReclaimSpaceControllerServer() *ReclaimSpaceControllerServer {
return &ReclaimSpaceControllerServer{}
func NewReclaimSpaceControllerServer(c *rbdutil.ControllerServer) *ReclaimSpaceControllerServer {
return &ReclaimSpaceControllerServer{ControllerServer: c}
}

func (rscs *ReclaimSpaceControllerServer) RegisterService(server grpc.ServiceRegistrar) {
Expand All @@ -64,6 +66,13 @@ func (rscs *ReclaimSpaceControllerServer) ControllerReclaimSpace(
}
defer cr.DeleteCredentials()

if acquired := rscs.VolumeLocks.TryAcquire(volumeID); !acquired {
log.ErrorLog(ctx, util.VolumeOperationAlreadyExistsFmt, volumeID)

return nil, status.Errorf(codes.Aborted, util.VolumeOperationAlreadyExistsFmt, volumeID)
}
defer rscs.VolumeLocks.Release(volumeID)

rbdVol, err := rbdutil.GenVolFromVolID(ctx, volumeID, cr, req.GetSecrets())
if err != nil {
return nil, status.Errorf(codes.Aborted, "failed to find volume with ID %q: %s", volumeID, err.Error())
Expand All @@ -90,12 +99,14 @@ func (rscs *ReclaimSpaceControllerServer) ControllerReclaimSpace(
// of CSI-addons reclaimspace controller service spec.
type ReclaimSpaceNodeServer struct {
*rs.UnimplementedReclaimSpaceNodeServer
// Embed ControllerServer as it implements helper functions
*rbdutil.ControllerServer
}

// NewReclaimSpaceNodeServer creates a new IdentityServer which handles the
// Identity Service requests from the CSI-Addons specification.
func NewReclaimSpaceNodeServer() *ReclaimSpaceNodeServer {
return &ReclaimSpaceNodeServer{}
func NewReclaimSpaceNodeServer(c *rbdutil.ControllerServer) *ReclaimSpaceNodeServer {
return &ReclaimSpaceNodeServer{ControllerServer: c}
}

func (rsns *ReclaimSpaceNodeServer) RegisterService(server grpc.ServiceRegistrar) {
Expand All @@ -116,6 +127,13 @@ func (rsns *ReclaimSpaceNodeServer) NodeReclaimSpace(
return nil, status.Error(codes.InvalidArgument, "empty volume ID in request")
}

if acquired := rsns.VolumeLocks.TryAcquire(volumeID); !acquired {
log.ErrorLog(ctx, util.VolumeOperationAlreadyExistsFmt, volumeID)

return nil, status.Errorf(codes.Aborted, util.VolumeOperationAlreadyExistsFmt, volumeID)
}
defer rsns.VolumeLocks.Release(volumeID)

// path can either be the staging path on the node, or the volume path
// inside an application container
path := req.GetStagingTargetPath()
Expand Down
5 changes: 3 additions & 2 deletions internal/csi-addons/rbd/reclaimspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"testing"

rbdutil "github.com/ceph/ceph-csi/internal/rbd"
rs "github.com/csi-addons/spec/lib/go/reclaimspace"
"github.com/stretchr/testify/require"
)
Expand All @@ -30,7 +31,7 @@ import (
func TestControllerReclaimSpace(t *testing.T) {
t.Parallel()

controller := NewReclaimSpaceControllerServer()
controller := NewReclaimSpaceControllerServer(&rbdutil.ControllerServer{})

req := &rs.ControllerReclaimSpaceRequest{
VolumeId: "",
Expand All @@ -47,7 +48,7 @@ func TestControllerReclaimSpace(t *testing.T) {
func TestNodeReclaimSpace(t *testing.T) {
t.Parallel()

node := NewReclaimSpaceNodeServer()
node := NewReclaimSpaceNodeServer(&rbdutil.ControllerServer{})

req := &rs.NodeReclaimSpaceRequest{
VolumeId: "",
Expand Down
4 changes: 2 additions & 2 deletions internal/rbd/driver/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ func (r *Driver) setupCSIAddonsServer(conf *util.Config) error {
r.cas.RegisterService(is)

if conf.IsControllerServer {
rs := casrbd.NewReclaimSpaceControllerServer()
rs := casrbd.NewReclaimSpaceControllerServer(NewControllerServer(r.cd))
r.cas.RegisterService(rs)

fcs := casrbd.NewFenceControllerServer()
Expand All @@ -224,7 +224,7 @@ func (r *Driver) setupCSIAddonsServer(conf *util.Config) error {
}

if conf.IsNodeServer {
rs := casrbd.NewReclaimSpaceNodeServer()
rs := casrbd.NewReclaimSpaceNodeServer(NewControllerServer(r.cd))
r.cas.RegisterService(rs)
}

Expand Down

0 comments on commit 0d019f8

Please sign in to comment.