Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rbd: add volume locks for reclaimspace operations #4641

Merged
merged 2 commits into from
Jul 31, 2024
Merged
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
24 changes: 20 additions & 4 deletions internal/csi-addons/rbd/reclaimspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,13 @@ import (
// of CSI-addons reclaimspace controller service spec.
type ReclaimSpaceControllerServer struct {
*rs.UnimplementedReclaimSpaceControllerServer
volumeLocks *util.VolumeLocks
}

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

func (rscs *ReclaimSpaceControllerServer) RegisterService(server grpc.ServiceRegistrar) {
Expand All @@ -64,6 +65,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 +98,13 @@ func (rscs *ReclaimSpaceControllerServer) ControllerReclaimSpace(
// of CSI-addons reclaimspace controller service spec.
type ReclaimSpaceNodeServer struct {
*rs.UnimplementedReclaimSpaceNodeServer
volumeLocks *util.VolumeLocks
}

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

func (rsns *ReclaimSpaceNodeServer) RegisterService(server grpc.ServiceRegistrar) {
Expand All @@ -116,6 +125,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
6 changes: 4 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,8 @@ import (
"context"
"testing"

"github.com/ceph/ceph-csi/internal/util"

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

controller := NewReclaimSpaceControllerServer()
controller := NewReclaimSpaceControllerServer(util.NewVolumeLocks())

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

node := NewReclaimSpaceNodeServer()
node := NewReclaimSpaceNodeServer(&util.VolumeLocks{})

req := &rs.NodeReclaimSpaceRequest{
VolumeId: "",
Expand Down
14 changes: 6 additions & 8 deletions internal/rbd/driver/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,14 @@ func NewNodeServer(
d *csicommon.CSIDriver,
t string,
nodeLabels, topology, crushLocationMap map[string]string,
) (*rbd.NodeServer, error) {
) *rbd.NodeServer {
cliReadAffinityMapOptions := util.ConstructReadAffinityMapOption(crushLocationMap)
ns := rbd.NodeServer{
DefaultNodeServer: csicommon.NewDefaultNodeServer(d, t, cliReadAffinityMapOptions, topology, nodeLabels),
VolumeLocks: util.NewVolumeLocks(),
}

return &ns, nil
return &ns
iPraveenParihar marked this conversation as resolved.
Show resolved Hide resolved
}

// Run start a non-blocking grpc controller,node and identityserver for
Expand Down Expand Up @@ -144,10 +144,8 @@ func (r *Driver) Run(conf *util.Config) {
if err != nil {
log.FatalLogMsg(err.Error())
}
r.ns, err = NewNodeServer(r.cd, conf.Vtype, nodeLabels, topology, crushLocationMap)
if err != nil {
log.FatalLogMsg("failed to start node server, err %v\n", err)
}
r.ns = NewNodeServer(r.cd, conf.Vtype, nodeLabels, topology, crushLocationMap)

var attr string
attr, err = rbd.GetKrbdSupportedFeatures()
if err != nil && !errors.Is(err, os.ErrNotExist) {
Expand Down Expand Up @@ -213,7 +211,7 @@ func (r *Driver) setupCSIAddonsServer(conf *util.Config) error {
r.cas.RegisterService(is)

if conf.IsControllerServer {
rs := casrbd.NewReclaimSpaceControllerServer()
rs := casrbd.NewReclaimSpaceControllerServer(r.cs.VolumeLocks)
r.cas.RegisterService(rs)

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

if conf.IsNodeServer {
rs := casrbd.NewReclaimSpaceNodeServer()
rs := casrbd.NewReclaimSpaceNodeServer(r.ns.VolumeLocks)
r.cas.RegisterService(rs)

ekr := casrbd.NewEncryptionKeyRotationServer(r.ns.VolumeLocks)
Expand Down