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 VolumeGroup.ModifyVolumeGroupMembership CSI-Addons operation #4729

Merged
merged 1 commit into from
Jul 25, 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
6 changes: 6 additions & 0 deletions internal/csi-addons/rbd/identity.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,12 @@ func (is *IdentityServer) GetCapabilities(
Type: identity.Capability_VolumeGroup_LIMIT_VOLUME_TO_ONE_VOLUME_GROUP,
},
},
}, &identity.Capability{
Type: &identity.Capability_VolumeGroup_{
VolumeGroup: &identity.Capability_VolumeGroup{
Type: identity.Capability_VolumeGroup_MODIFY_VOLUME_GROUP,
},
},
})
}

Expand Down
148 changes: 148 additions & 0 deletions internal/csi-addons/rbd/volumegroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package rbd

import (
"context"
"slices"

"github.com/ceph/ceph-csi/internal/rbd"
"github.com/ceph/ceph-csi/internal/rbd/types"
Expand Down Expand Up @@ -215,3 +216,150 @@ func (vs *VolumeGroupServer) DeleteVolumeGroup(

return &volumegroup.DeleteVolumeGroupResponse{}, nil
}

// ModifyVolumeGroupMembership RPC call to modify a volume group.
//
// From the spec:
// This RPC will be called by the CO to modify an existing volume group on
// behalf of a user. volume_ids provided in the
// ModifyVolumeGroupMembershipRequest will be compared to the ones in the
// existing volume group. New volume_ids in the modified volume group will be
// added to the volume group. Existing volume_ids not in the modified volume
// group will be removed from the volume group. If volume_ids is empty, the
// volume group will be removed of all existing volumes. This operation MUST be
// idempotent.
//
// File-based storage systems usually do not support this PRC. Block-based
// storage systems usually support this PRC.
//
// By adding an existing volume to a group, however, there is no way to pass in
// parameters to influence placement when provisioning a volume.
//
// It is out of the scope of the CSI spec to determine whether a group is
// consistent or not. It is up to the storage provider to clarify that in the
// vendor specific documentation. This is true either when creating a new
// volume with a group id or adding an existing volume to a group.
//
// CSI drivers supporting MODIFY_VOLUME_GROUP_MEMBERSHIP MUST implement
// ModifyVolumeGroupMembership RPC.
//
// Note:
//
// The implementation works as the following:
// - resolve the existing volume group
// - get the CSI-IDs of all volumes
// - create a list of volumes that should be removed
// - create a list of volume IDs that should be added
// - remove the volumes from the group
// - add the volumes to the group
//
// Also, MODIFY_VOLUME_GROUP_MEMBERSHIP does not exist, it is called
// MODIFY_VOLUME_GROUP instead.
func (vs *VolumeGroupServer) ModifyVolumeGroupMembership(
ctx context.Context,
req *volumegroup.ModifyVolumeGroupMembershipRequest,
) (*volumegroup.ModifyVolumeGroupMembershipResponse, error) {
mgr := rbd.NewManager(vs.csiID, nil, req.GetSecrets())
defer mgr.Destroy(ctx)

// resolve the volume group
vg, err := mgr.GetVolumeGroupByID(ctx, req.GetVolumeGroupId())
if err != nil {
return nil, status.Errorf(
codes.NotFound,
"could not find volume group %q: %s",
req.GetVolumeGroupId(),
err.Error())
}
defer vg.Destroy(ctx)

beforeVolumes, err := vg.ListVolumes(ctx)
if err != nil {
return nil, status.Errorf(
codes.Internal,
"failed to list volumes of volume group %q: %v",
vg,
err)
}

// beforeIDs contains the csiID as key, volume as value
beforeIDs := make(map[string]types.Volume, len(beforeVolumes))
for _, vol := range beforeVolumes {
id, idErr := vol.GetID(ctx)
if idErr != nil {
return nil, status.Errorf(
codes.InvalidArgument,
"failed to get the CSI ID of volume %q: %v",
vol,
err)
}

beforeIDs[id] = vol
}

// check which volumes should not be part of the group
afterIDs := req.GetVolumeIds()
toRemove := make([]string, 0)
for id := range beforeIDs {
if !slices.Contains(afterIDs, id) {
toRemove = append(toRemove, id)
}
}

// check which volumes are new to the group
toAdd := make([]string, 0)
for _, id := range afterIDs {
if _, ok := beforeIDs[id]; !ok {
toAdd = append(toAdd, id)
}
}

// remove the volume that should not be part of the group
for _, id := range toRemove {
vol := beforeIDs[id]
err = vg.RemoveVolume(ctx, vol)
if err != nil {
return nil, status.Errorf(
codes.Internal,
"failed to remove volume %q from volume group %q: %v",
vol,
vg,
err)
}
}

// add the new volumes to the group
for _, id := range toAdd {
vol, getErr := mgr.GetVolumeByID(ctx, id)
if getErr != nil {
return nil, status.Errorf(
codes.NotFound,
"failed to find a volume with CSI ID %q: %v",
id,
err)
}

err = vg.AddVolume(ctx, vol)
if err != nil {
return nil, status.Errorf(
codes.Internal,
"failed to add volume %q to volume group %q: %v",
vol,
vg,
err)
}
}

csiVG, err := vg.ToCSI(ctx)
if err != nil {
return nil, status.Errorf(
codes.Internal,
"failed to convert volume group %q to CSI format: %v",
vg,
err)
}

return &volumegroup.ModifyVolumeGroupMembershipResponse{
VolumeGroup: csiVG,
Copy link
Contributor

@Nikhil-Ladha Nikhil-Ladha Jul 25, 2024

Choose a reason for hiding this comment

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

As discussed in the other (csi-addons PR), do we really need to send a response with the volumegroup or just send an empty response similar to Delete operation?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is like this in the specification of the CSI-Addons protocol for the operation, see https://github.com/csi-addons/spec/blob/main/volumegroup/README.md#modifyvolumegroupmembership.

The protocol that kubernetes-csi-addons uses to communicate between its Controller and sidecars can be different from the actual CSI-Addons Spec.

}, nil
}