From a243cf52d47360c4c7c6d2b3553a366454cab21e Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Mon, 29 Jul 2024 12:51:32 +0200 Subject: [PATCH 01/11] rbd: return more descriptive error updated GetVolumeByID to return more descriptive error so that caller no need to add more details in the error message. Signed-off-by: Madhu Rajanna --- internal/rbd/manager.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/internal/rbd/manager.go b/internal/rbd/manager.go index 48b5130d9bb..3c41afdc69d 100644 --- a/internal/rbd/manager.go +++ b/internal/rbd/manager.go @@ -124,7 +124,18 @@ func (mgr *rbdManager) GetVolumeByID(ctx context.Context, id string) (types.Volu volume, err := GenVolFromVolID(ctx, id, creds, mgr.secrets) if err != nil { - return nil, fmt.Errorf("failed to get volume from id %q: %w", id, err) + switch { + case errors.Is(err, ErrImageNotFound): + err = fmt.Errorf("volume %s not found: %w", id, err) + + return nil, err + case errors.Is(err, util.ErrPoolNotFound): + err = fmt.Errorf("pool %s not found for %s: %w", volume.Pool, id, err) + + return nil, err + default: + return nil, fmt.Errorf("failed to get volume from id %q: %w", id, err) + } } return volume, nil From b222b773aacbe9af1e5bfd3dd7631a1480e032d3 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Mon, 29 Jul 2024 13:07:37 +0200 Subject: [PATCH 02/11] rbd: implement journalledObject for volumes implement journalledObject interface to return the journal objects of the volume. Signed-off-by: Madhu Rajanna --- internal/rbd/rbd_util.go | 41 +++++++++++++++++++++++++++++++----- internal/rbd/types/group.go | 8 +++---- internal/rbd/types/volume.go | 6 +----- 3 files changed, 41 insertions(+), 14 deletions(-) diff --git a/internal/rbd/rbd_util.go b/internal/rbd/rbd_util.go index 28437a92134..6f9120719e9 100644 --- a/internal/rbd/rbd_util.go +++ b/internal/rbd/rbd_util.go @@ -426,11 +426,6 @@ func (rs *rbdSnapshot) String() string { return fmt.Sprintf("%s/%s@%s", rs.Pool, rs.RbdImageName, rs.RbdSnapName) } -// GetID returns the CSI volume handle of the image. -func (ri *rbdImage) GetID(ctx context.Context) (string, error) { - return ri.VolID, nil -} - // createImage creates a new ceph image with provision and volume options. func createImage(ctx context.Context, pOpts *rbdVolume, cr *util.Credentials) error { volSzMiB := fmt.Sprintf("%dM", util.RoundOffVolSize(pOpts.VolSize)) @@ -2187,3 +2182,39 @@ func (rv *rbdVolume) unsetAllMetadata(keys []string) error { return nil } + +// GetID returns the ID of the volume. +func (ri *rbdImage) GetID(ctx context.Context) (string, error) { + if ri.VolID == "" { + return "", errors.New("BUG: VolID is not set") + } + + return ri.VolID, nil +} + +// GetName returns the name of the rbd image. +func (ri *rbdImage) GetName(ctx context.Context) (string, error) { + if ri.RbdImageName == "" { + return "", errors.New("BUG: name is not set") + } + + return ri.RbdImageName, nil +} + +// GetPool returns the name of the pool that holds the Volume. +func (ri *rbdImage) GetPool(ctx context.Context) (string, error) { + if ri.Pool == "" { + return "", errors.New("BUG: pool is not set") + } + + return ri.Pool, nil +} + +// GetClusterID returns the clusterID the volume belongs to. +func (ri *rbdImage) GetClusterID(ctx context.Context) (string, error) { + if ri.ClusterID == "" { + return "", errors.New("BUG: clusterID is not set") + } + + return ri.ClusterID, nil +} diff --git a/internal/rbd/types/group.go b/internal/rbd/types/group.go index 83d0adea376..36f89e807f5 100644 --- a/internal/rbd/types/group.go +++ b/internal/rbd/types/group.go @@ -24,16 +24,16 @@ import ( ) type journalledObject interface { - // GetID returns the CSI-Addons VolumeGroupId of the VolumeGroup. + // GetID returns the ID in the backend storage for the object. GetID(ctx context.Context) (string, error) - // GetName returns the name in the backend storage for the VolumeGroup. + // GetName returns the name of the object in the backend storage. GetName(ctx context.Context) (string, error) - // GetPool returns the name of the pool that holds the VolumeGroup. + // GetPool returns the name of the pool that holds the object. GetPool(ctx context.Context) (string, error) - // GetClusterID returns the ID of the cluster of the VolumeGroup. + // GetClusterID returns the ID of the cluster of the object. GetClusterID(ctx context.Context) (string, error) } diff --git a/internal/rbd/types/volume.go b/internal/rbd/types/volume.go index 7f534091f42..388676cf566 100644 --- a/internal/rbd/types/volume.go +++ b/internal/rbd/types/volume.go @@ -25,15 +25,13 @@ import ( //nolint:interfacebloat // more than 10 methods are needed for the interface type Volume interface { + journalledObject // Destroy frees the resources used by the Volume. Destroy(ctx context.Context) // Delete removes the volume from the storage backend. Delete(ctx context.Context) error - // GetID returns the CSI VolumeID for the volume. - GetID(ctx context.Context) (string, error) - // ToCSI creates a CSI protocol formatted struct of the volume. ToCSI(ctx context.Context) (*csi.Volume, error) @@ -43,8 +41,6 @@ type Volume interface { // RemoveFromGroup removes the Volume from the VolumeGroup. RemoveFromGroup(ctx context.Context, vg VolumeGroup) error - // GetPoolName returns the name of the pool where the volume is stored. - GetPoolName() string // GetCreationTime returns the creation time of the volume. GetCreationTime() (*timestamppb.Timestamp, error) // GetMetadata returns the value of the metadata key from the volume. From c773c984085ac381c32374feabec9f58bcf130f0 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Mon, 29 Jul 2024 13:47:59 +0200 Subject: [PATCH 03/11] rbd: flatten image in CreateVolumeGroup This commit adds support for flattenMode option for volumegroup. If the flattenMode is set to "force" in volumegroupreplicationclass parameters, cephcsi will add a task to flatten the image if it has parent before adding it to the group. This enable cephcsi to then mirror such images after flattening them. Signed-off-by: Madhu Rajanna --- internal/csi-addons/rbd/volumegroup.go | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/internal/csi-addons/rbd/volumegroup.go b/internal/csi-addons/rbd/volumegroup.go index 95d438f7b7f..7fc48db3c76 100644 --- a/internal/csi-addons/rbd/volumegroup.go +++ b/internal/csi-addons/rbd/volumegroup.go @@ -18,6 +18,7 @@ package rbd import ( "context" + "fmt" "slices" "github.com/ceph/ceph-csi/internal/rbd" @@ -87,6 +88,11 @@ func (vs *VolumeGroupServer) CreateVolumeGroup( // resolve all volumes volumes := make([]types.Volume, len(req.GetVolumeIds())) + defer func() { + for _, vol := range volumes { + vol.Destroy(ctx) + } + }() for i, id := range req.GetVolumeIds() { vol, err := mgr.GetVolumeByID(ctx, id) if err != nil { @@ -97,9 +103,6 @@ func (vs *VolumeGroupServer) CreateVolumeGroup( req.GetName(), err.Error()) } - - //nolint:gocritic // need to call .Destroy() for all volumes - defer vol.Destroy(ctx) volumes[i] = vol } @@ -117,6 +120,21 @@ func (vs *VolumeGroupServer) CreateVolumeGroup( log.DebugLog(ctx, "VolumeGroup %q has been created: %+v", req.GetName(), vg) + // extract the flatten mode + flattenMode, err := getFlattenMode(ctx, req.GetParameters()) + if err != nil { + return nil, err + } + // Flatten the image if the flatten mode is set to FlattenModeForce + // before adding it to the volume group. + for _, v := range volumes { + err = v.HandleParentImageExistence(ctx, flattenMode) + if err != nil { + err = fmt.Errorf("failed to handle parent image for volume group %q: %w", vg, err) + + return nil, getGRPCError(err) + } + } // add each rbd-image to the RBDVolumeGroup for _, vol := range volumes { err = vg.AddVolume(ctx, vol) From e682f2cc73a12b97595000ef4651d838d97f2a2f Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Tue, 30 Jul 2024 18:29:33 +0200 Subject: [PATCH 04/11] rbd: add struct to error updating HandleParentImageExistence function to return more details error which includes the pool/namespace/image name Signed-off-by: Madhu Rajanna --- internal/rbd/mirror.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/rbd/mirror.go b/internal/rbd/mirror.go index 905db1c44ab..08064dfa901 100644 --- a/internal/rbd/mirror.go +++ b/internal/rbd/mirror.go @@ -44,12 +44,12 @@ func (rv *rbdVolume) HandleParentImageExistence( // it is no longer required when the live image is flattened. err := rv.DeleteTempImage(ctx) if err != nil { - return fmt.Errorf("failed to delete temporary rbd image: %w", err) + return fmt.Errorf("failed to delete temporary rbd image %s: %w", rv, err) } err = rv.flattenRbdImage(ctx, true, 0, 0) if err != nil { - return err + return fmt.Errorf("failed to flatten image %s: %w", rv, err) } } @@ -61,7 +61,7 @@ func (rv *rbdVolume) HandleParentImageExistence( parent, err := rv.getParent() if err != nil { - return err + return fmt.Errorf("failed to get parent of image %s: %w", rv, err) } parentMirroringInfo, err := parent.GetMirroringInfo() if err != nil { From 88a5c8a0eb48594686f4e4261c69555999a03adc Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Tue, 30 Jul 2024 10:45:24 +0200 Subject: [PATCH 05/11] vendor: update csiaddons spec updating csiaddons spec to the latest main. Signed-off-by: Madhu Rajanna --- go.mod | 2 +- go.sum | 2 + .../spec/lib/go/volumegroup/volumegroup.pb.go | 89 ++++++++++++------- vendor/modules.txt | 2 +- 4 files changed, 60 insertions(+), 35 deletions(-) diff --git a/go.mod b/go.mod index 2d5aa94eb50..a3bc669397b 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/ceph/ceph-csi/api v0.0.0-00010101000000-000000000000 github.com/ceph/go-ceph v0.28.0 github.com/container-storage-interface/spec v1.10.0 - github.com/csi-addons/spec v0.2.1-0.20240718113938-dc98b454ba65 + github.com/csi-addons/spec v0.2.1-0.20240730084235-3958a5b17d24 github.com/gemalto/kmip-go v0.0.10 github.com/golang/protobuf v1.5.4 github.com/google/fscrypt v0.3.6-0.20240502174735-068b9f8f5dec diff --git a/go.sum b/go.sum index c2373b5355a..959f598eeac 100644 --- a/go.sum +++ b/go.sum @@ -913,6 +913,8 @@ github.com/creack/pty v1.1.11/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ github.com/creack/pty v1.1.18/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4= github.com/csi-addons/spec v0.2.1-0.20240718113938-dc98b454ba65 h1:i9JGGQTEmRQXSpQQPR96+DV4D4o+V1+gjAWf+bpxQxk= github.com/csi-addons/spec v0.2.1-0.20240718113938-dc98b454ba65/go.mod h1:Mwq4iLiUV4s+K1bszcWU6aMsR5KPsbIYzzszJ6+56vI= +github.com/csi-addons/spec v0.2.1-0.20240730084235-3958a5b17d24 h1:tJETaYbnnzlCSaqDXQzbszYyuAtG/sFzm6DargeVzJA= +github.com/csi-addons/spec v0.2.1-0.20240730084235-3958a5b17d24/go.mod h1:Mwq4iLiUV4s+K1bszcWU6aMsR5KPsbIYzzszJ6+56vI= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= diff --git a/vendor/github.com/csi-addons/spec/lib/go/volumegroup/volumegroup.pb.go b/vendor/github.com/csi-addons/spec/lib/go/volumegroup/volumegroup.pb.go index b5e422e7c9b..aa216a10742 100644 --- a/vendor/github.com/csi-addons/spec/lib/go/volumegroup/volumegroup.pb.go +++ b/vendor/github.com/csi-addons/spec/lib/go/volumegroup/volumegroup.pb.go @@ -360,6 +360,10 @@ type ModifyVolumeGroupMembershipRequest struct { // This field is OPTIONAL. Refer to the `Secrets Requirements` // section on how to use this field. Secrets map[string]string `protobuf:"bytes,3,rep,name=secrets,proto3" json:"secrets,omitempty" protobuf_key:"bytes,1,opt,name=key,proto3" protobuf_val:"bytes,2,opt,name=value,proto3"` + // parameters passed to the plugin to modify the volume group + // or to modify the volumes in the group. + // This field is OPTIONAL. + Parameters map[string]string `protobuf:"bytes,4,rep,name=parameters,proto3" json:"parameters,omitempty" protobuf_key:"bytes,1,opt,name=key,proto3" protobuf_val:"bytes,2,opt,name=value,proto3"` } func (x *ModifyVolumeGroupMembershipRequest) Reset() { @@ -415,6 +419,13 @@ func (x *ModifyVolumeGroupMembershipRequest) GetSecrets() map[string]string { return nil } +func (x *ModifyVolumeGroupMembershipRequest) GetParameters() map[string]string { + if x != nil { + return x.Parameters + } + return nil +} + // ModifyVolumeGroupMembershipResponse holds the information to // send when volumeGroup is successfully modified. type ModifyVolumeGroupMembershipResponse struct { @@ -737,7 +748,7 @@ type ListVolumeGroupsResponse_Entry struct { func (x *ListVolumeGroupsResponse_Entry) Reset() { *x = ListVolumeGroupsResponse_Entry{} if protoimpl.UnsafeEnabled { - mi := &file_volumegroup_volumegroup_proto_msgTypes[18] + mi := &file_volumegroup_volumegroup_proto_msgTypes[19] ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) ms.StoreMessageInfo(mi) } @@ -750,7 +761,7 @@ func (x *ListVolumeGroupsResponse_Entry) String() string { func (*ListVolumeGroupsResponse_Entry) ProtoMessage() {} func (x *ListVolumeGroupsResponse_Entry) ProtoReflect() protoreflect.Message { - mi := &file_volumegroup_volumegroup_proto_msgTypes[18] + mi := &file_volumegroup_volumegroup_proto_msgTypes[19] if protoimpl.UnsafeEnabled && x != nil { ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) if ms.LoadMessageInfo() == nil { @@ -843,7 +854,7 @@ var file_volumegroup_volumegroup_proto_rawDesc = []byte{ 0x03, 0x6b, 0x65, 0x79, 0x12, 0x14, 0x0a, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x3a, 0x02, 0x38, 0x01, 0x22, 0x1b, 0x0a, 0x19, 0x44, 0x65, 0x6c, 0x65, 0x74, 0x65, 0x56, 0x6f, 0x6c, 0x75, 0x6d, 0x65, 0x47, 0x72, - 0x6f, 0x75, 0x70, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x22, 0x84, 0x02, 0x0a, 0x22, + 0x6f, 0x75, 0x70, 0x52, 0x65, 0x73, 0x70, 0x6f, 0x6e, 0x73, 0x65, 0x22, 0xa4, 0x03, 0x0a, 0x22, 0x4d, 0x6f, 0x64, 0x69, 0x66, 0x79, 0x56, 0x6f, 0x6c, 0x75, 0x6d, 0x65, 0x47, 0x72, 0x6f, 0x75, 0x70, 0x4d, 0x65, 0x6d, 0x62, 0x65, 0x72, 0x73, 0x68, 0x69, 0x70, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x12, 0x26, 0x0a, 0x0f, 0x76, 0x6f, 0x6c, 0x75, 0x6d, 0x65, 0x5f, 0x67, 0x72, 0x6f, @@ -856,7 +867,17 @@ var file_volumegroup_volumegroup_proto_rawDesc = []byte{ 0x6f, 0x6c, 0x75, 0x6d, 0x65, 0x47, 0x72, 0x6f, 0x75, 0x70, 0x4d, 0x65, 0x6d, 0x62, 0x65, 0x72, 0x73, 0x68, 0x69, 0x70, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x2e, 0x53, 0x65, 0x63, 0x72, 0x65, 0x74, 0x73, 0x45, 0x6e, 0x74, 0x72, 0x79, 0x42, 0x03, 0x98, 0x42, 0x01, 0x52, 0x07, 0x73, - 0x65, 0x63, 0x72, 0x65, 0x74, 0x73, 0x1a, 0x3a, 0x0a, 0x0c, 0x53, 0x65, 0x63, 0x72, 0x65, 0x74, + 0x65, 0x63, 0x72, 0x65, 0x74, 0x73, 0x12, 0x5f, 0x0a, 0x0a, 0x70, 0x61, 0x72, 0x61, 0x6d, 0x65, + 0x74, 0x65, 0x72, 0x73, 0x18, 0x04, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x3f, 0x2e, 0x76, 0x6f, 0x6c, + 0x75, 0x6d, 0x65, 0x67, 0x72, 0x6f, 0x75, 0x70, 0x2e, 0x4d, 0x6f, 0x64, 0x69, 0x66, 0x79, 0x56, + 0x6f, 0x6c, 0x75, 0x6d, 0x65, 0x47, 0x72, 0x6f, 0x75, 0x70, 0x4d, 0x65, 0x6d, 0x62, 0x65, 0x72, + 0x73, 0x68, 0x69, 0x70, 0x52, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x2e, 0x50, 0x61, 0x72, 0x61, + 0x6d, 0x65, 0x74, 0x65, 0x72, 0x73, 0x45, 0x6e, 0x74, 0x72, 0x79, 0x52, 0x0a, 0x70, 0x61, 0x72, + 0x61, 0x6d, 0x65, 0x74, 0x65, 0x72, 0x73, 0x1a, 0x3a, 0x0a, 0x0c, 0x53, 0x65, 0x63, 0x72, 0x65, + 0x74, 0x73, 0x45, 0x6e, 0x74, 0x72, 0x79, 0x12, 0x10, 0x0a, 0x03, 0x6b, 0x65, 0x79, 0x18, 0x01, + 0x20, 0x01, 0x28, 0x09, 0x52, 0x03, 0x6b, 0x65, 0x79, 0x12, 0x14, 0x0a, 0x05, 0x76, 0x61, 0x6c, + 0x75, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x3a, + 0x02, 0x38, 0x01, 0x1a, 0x3d, 0x0a, 0x0f, 0x50, 0x61, 0x72, 0x61, 0x6d, 0x65, 0x74, 0x65, 0x72, 0x73, 0x45, 0x6e, 0x74, 0x72, 0x79, 0x12, 0x10, 0x0a, 0x03, 0x6b, 0x65, 0x79, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x03, 0x6b, 0x65, 0x79, 0x12, 0x14, 0x0a, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x3a, 0x02, @@ -966,7 +987,7 @@ func file_volumegroup_volumegroup_proto_rawDescGZIP() []byte { return file_volumegroup_volumegroup_proto_rawDescData } -var file_volumegroup_volumegroup_proto_msgTypes = make([]protoimpl.MessageInfo, 19) +var file_volumegroup_volumegroup_proto_msgTypes = make([]protoimpl.MessageInfo, 20) var file_volumegroup_volumegroup_proto_goTypes = []interface{}{ (*CreateVolumeGroupRequest)(nil), // 0: volumegroup.CreateVolumeGroupRequest (*CreateVolumeGroupResponse)(nil), // 1: volumegroup.CreateVolumeGroupResponse @@ -984,40 +1005,42 @@ var file_volumegroup_volumegroup_proto_goTypes = []interface{}{ nil, // 13: volumegroup.VolumeGroup.VolumeGroupContextEntry nil, // 14: volumegroup.DeleteVolumeGroupRequest.SecretsEntry nil, // 15: volumegroup.ModifyVolumeGroupMembershipRequest.SecretsEntry - nil, // 16: volumegroup.ControllerGetVolumeGroupRequest.SecretsEntry - nil, // 17: volumegroup.ListVolumeGroupsRequest.SecretsEntry - (*ListVolumeGroupsResponse_Entry)(nil), // 18: volumegroup.ListVolumeGroupsResponse.Entry - (*csi.Volume)(nil), // 19: csi.v1.Volume + nil, // 16: volumegroup.ModifyVolumeGroupMembershipRequest.ParametersEntry + nil, // 17: volumegroup.ControllerGetVolumeGroupRequest.SecretsEntry + nil, // 18: volumegroup.ListVolumeGroupsRequest.SecretsEntry + (*ListVolumeGroupsResponse_Entry)(nil), // 19: volumegroup.ListVolumeGroupsResponse.Entry + (*csi.Volume)(nil), // 20: csi.v1.Volume } var file_volumegroup_volumegroup_proto_depIdxs = []int32{ 11, // 0: volumegroup.CreateVolumeGroupRequest.parameters:type_name -> volumegroup.CreateVolumeGroupRequest.ParametersEntry 12, // 1: volumegroup.CreateVolumeGroupRequest.secrets:type_name -> volumegroup.CreateVolumeGroupRequest.SecretsEntry 2, // 2: volumegroup.CreateVolumeGroupResponse.volume_group:type_name -> volumegroup.VolumeGroup 13, // 3: volumegroup.VolumeGroup.volume_group_context:type_name -> volumegroup.VolumeGroup.VolumeGroupContextEntry - 19, // 4: volumegroup.VolumeGroup.volumes:type_name -> csi.v1.Volume + 20, // 4: volumegroup.VolumeGroup.volumes:type_name -> csi.v1.Volume 14, // 5: volumegroup.DeleteVolumeGroupRequest.secrets:type_name -> volumegroup.DeleteVolumeGroupRequest.SecretsEntry 15, // 6: volumegroup.ModifyVolumeGroupMembershipRequest.secrets:type_name -> volumegroup.ModifyVolumeGroupMembershipRequest.SecretsEntry - 2, // 7: volumegroup.ModifyVolumeGroupMembershipResponse.volume_group:type_name -> volumegroup.VolumeGroup - 16, // 8: volumegroup.ControllerGetVolumeGroupRequest.secrets:type_name -> volumegroup.ControllerGetVolumeGroupRequest.SecretsEntry - 2, // 9: volumegroup.ControllerGetVolumeGroupResponse.volume_group:type_name -> volumegroup.VolumeGroup - 17, // 10: volumegroup.ListVolumeGroupsRequest.secrets:type_name -> volumegroup.ListVolumeGroupsRequest.SecretsEntry - 18, // 11: volumegroup.ListVolumeGroupsResponse.entries:type_name -> volumegroup.ListVolumeGroupsResponse.Entry - 2, // 12: volumegroup.ListVolumeGroupsResponse.Entry.volume_group:type_name -> volumegroup.VolumeGroup - 0, // 13: volumegroup.Controller.CreateVolumeGroup:input_type -> volumegroup.CreateVolumeGroupRequest - 5, // 14: volumegroup.Controller.ModifyVolumeGroupMembership:input_type -> volumegroup.ModifyVolumeGroupMembershipRequest - 3, // 15: volumegroup.Controller.DeleteVolumeGroup:input_type -> volumegroup.DeleteVolumeGroupRequest - 9, // 16: volumegroup.Controller.ListVolumeGroups:input_type -> volumegroup.ListVolumeGroupsRequest - 7, // 17: volumegroup.Controller.ControllerGetVolumeGroup:input_type -> volumegroup.ControllerGetVolumeGroupRequest - 1, // 18: volumegroup.Controller.CreateVolumeGroup:output_type -> volumegroup.CreateVolumeGroupResponse - 6, // 19: volumegroup.Controller.ModifyVolumeGroupMembership:output_type -> volumegroup.ModifyVolumeGroupMembershipResponse - 4, // 20: volumegroup.Controller.DeleteVolumeGroup:output_type -> volumegroup.DeleteVolumeGroupResponse - 10, // 21: volumegroup.Controller.ListVolumeGroups:output_type -> volumegroup.ListVolumeGroupsResponse - 8, // 22: volumegroup.Controller.ControllerGetVolumeGroup:output_type -> volumegroup.ControllerGetVolumeGroupResponse - 18, // [18:23] is the sub-list for method output_type - 13, // [13:18] is the sub-list for method input_type - 13, // [13:13] is the sub-list for extension type_name - 13, // [13:13] is the sub-list for extension extendee - 0, // [0:13] is the sub-list for field type_name + 16, // 7: volumegroup.ModifyVolumeGroupMembershipRequest.parameters:type_name -> volumegroup.ModifyVolumeGroupMembershipRequest.ParametersEntry + 2, // 8: volumegroup.ModifyVolumeGroupMembershipResponse.volume_group:type_name -> volumegroup.VolumeGroup + 17, // 9: volumegroup.ControllerGetVolumeGroupRequest.secrets:type_name -> volumegroup.ControllerGetVolumeGroupRequest.SecretsEntry + 2, // 10: volumegroup.ControllerGetVolumeGroupResponse.volume_group:type_name -> volumegroup.VolumeGroup + 18, // 11: volumegroup.ListVolumeGroupsRequest.secrets:type_name -> volumegroup.ListVolumeGroupsRequest.SecretsEntry + 19, // 12: volumegroup.ListVolumeGroupsResponse.entries:type_name -> volumegroup.ListVolumeGroupsResponse.Entry + 2, // 13: volumegroup.ListVolumeGroupsResponse.Entry.volume_group:type_name -> volumegroup.VolumeGroup + 0, // 14: volumegroup.Controller.CreateVolumeGroup:input_type -> volumegroup.CreateVolumeGroupRequest + 5, // 15: volumegroup.Controller.ModifyVolumeGroupMembership:input_type -> volumegroup.ModifyVolumeGroupMembershipRequest + 3, // 16: volumegroup.Controller.DeleteVolumeGroup:input_type -> volumegroup.DeleteVolumeGroupRequest + 9, // 17: volumegroup.Controller.ListVolumeGroups:input_type -> volumegroup.ListVolumeGroupsRequest + 7, // 18: volumegroup.Controller.ControllerGetVolumeGroup:input_type -> volumegroup.ControllerGetVolumeGroupRequest + 1, // 19: volumegroup.Controller.CreateVolumeGroup:output_type -> volumegroup.CreateVolumeGroupResponse + 6, // 20: volumegroup.Controller.ModifyVolumeGroupMembership:output_type -> volumegroup.ModifyVolumeGroupMembershipResponse + 4, // 21: volumegroup.Controller.DeleteVolumeGroup:output_type -> volumegroup.DeleteVolumeGroupResponse + 10, // 22: volumegroup.Controller.ListVolumeGroups:output_type -> volumegroup.ListVolumeGroupsResponse + 8, // 23: volumegroup.Controller.ControllerGetVolumeGroup:output_type -> volumegroup.ControllerGetVolumeGroupResponse + 19, // [19:24] is the sub-list for method output_type + 14, // [14:19] is the sub-list for method input_type + 14, // [14:14] is the sub-list for extension type_name + 14, // [14:14] is the sub-list for extension extendee + 0, // [0:14] is the sub-list for field type_name } func init() { file_volumegroup_volumegroup_proto_init() } @@ -1158,7 +1181,7 @@ func file_volumegroup_volumegroup_proto_init() { return nil } } - file_volumegroup_volumegroup_proto_msgTypes[18].Exporter = func(v interface{}, i int) interface{} { + file_volumegroup_volumegroup_proto_msgTypes[19].Exporter = func(v interface{}, i int) interface{} { switch v := v.(*ListVolumeGroupsResponse_Entry); i { case 0: return &v.state @@ -1177,7 +1200,7 @@ func file_volumegroup_volumegroup_proto_init() { GoPackagePath: reflect.TypeOf(x{}).PkgPath(), RawDescriptor: file_volumegroup_volumegroup_proto_rawDesc, NumEnums: 0, - NumMessages: 19, + NumMessages: 20, NumExtensions: 0, NumServices: 1, }, diff --git a/vendor/modules.txt b/vendor/modules.txt index d2365e74fbd..fa4e5cb925d 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -236,7 +236,7 @@ github.com/coreos/go-semver/semver ## explicit; go 1.12 github.com/coreos/go-systemd/v22/daemon github.com/coreos/go-systemd/v22/journal -# github.com/csi-addons/spec v0.2.1-0.20240718113938-dc98b454ba65 +# github.com/csi-addons/spec v0.2.1-0.20240730084235-3958a5b17d24 ## explicit github.com/csi-addons/spec/lib/go/encryptionkeyrotation github.com/csi-addons/spec/lib/go/fence From 7e2e5ba2e5a3669858150e7a814374d476888dc9 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Mon, 29 Jul 2024 13:55:25 +0200 Subject: [PATCH 06/11] rbd: flatten image in ModifyVolumeGroupMembership in ModifyVolumeGroupMembership RPC call, flatten the required images before adding it to the group or else if the parent is not mirror enabled adding a child to the group will fail. Signed-off-by: Madhu Rajanna --- internal/csi-addons/rbd/volumegroup.go | 35 +++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/internal/csi-addons/rbd/volumegroup.go b/internal/csi-addons/rbd/volumegroup.go index 7fc48db3c76..1b418a0827f 100644 --- a/internal/csi-addons/rbd/volumegroup.go +++ b/internal/csi-addons/rbd/volumegroup.go @@ -346,17 +346,44 @@ func (vs *VolumeGroupServer) ModifyVolumeGroupMembership( } } - // add the new volumes to the group - for _, id := range toAdd { - vol, getErr := mgr.GetVolumeByID(ctx, id) - if getErr != nil { + // resolve all volumes + volumes := make([]types.Volume, len(toAdd)) + defer func() { + for _, vol := range volumes { + vol.Destroy(ctx) + } + }() + for i, id := range toAdd { + var vol types.Volume + vol, err = mgr.GetVolumeByID(ctx, id) + if err != nil { return nil, status.Errorf( codes.NotFound, "failed to find a volume with CSI ID %q: %v", id, err) } + volumes[i] = vol + } + // extract the flatten mode + flattenMode, err := getFlattenMode(ctx, req.GetParameters()) + if err != nil { + return nil, err + } + // Flatten the image if the flatten mode is set to FlattenModeForce + // before adding it to the volume group. + for _, vol := range volumes { + err = vol.HandleParentImageExistence(ctx, flattenMode) + if err != nil { + err = fmt.Errorf("failed to handle parent image for volume group %q: %w", vg, err) + + return nil, getGRPCError(err) + } + } + + // add the new volumes to the group + for _, vol := range volumes { err = vg.AddVolume(ctx, vol) if err != nil { return nil, status.Errorf( From 8788e5ec0869215dfeed662c5a025bb1c6afe372 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Tue, 30 Jul 2024 18:37:40 +0200 Subject: [PATCH 07/11] rbd: adapt to GetVolumeByID error message GetVolumeByID already returning detailed error message, the caller just need to return it. No need to add duplicate details to error message. Signed-off-by: Madhu Rajanna --- internal/csi-addons/rbd/replication.go | 61 +++------------------ internal/csi-addons/rbd/replication_test.go | 11 ++++ 2 files changed, 20 insertions(+), 52 deletions(-) diff --git a/internal/csi-addons/rbd/replication.go b/internal/csi-addons/rbd/replication.go index df5c3732bcd..09b0ca3fee0 100644 --- a/internal/csi-addons/rbd/replication.go +++ b/internal/csi-addons/rbd/replication.go @@ -282,16 +282,7 @@ func (rs *ReplicationServer) EnableVolumeReplication(ctx context.Context, rbdVol, err := mgr.GetVolumeByID(ctx, volumeID) if err != nil { - switch { - case errors.Is(err, corerbd.ErrImageNotFound): - err = status.Errorf(codes.NotFound, "volume %s not found", volumeID) - case errors.Is(err, util.ErrPoolNotFound): - err = status.Errorf(codes.NotFound, "pool %s not found for %s", rbdVol.GetPoolName(), volumeID) - default: - err = status.Errorf(codes.Internal, err.Error()) - } - - return nil, err + return nil, getGRPCError(err) } mirror, err := rbdVol.ToMirror() if err != nil { @@ -361,16 +352,7 @@ func (rs *ReplicationServer) DisableVolumeReplication(ctx context.Context, rbdVol, err := mgr.GetVolumeByID(ctx, volumeID) if err != nil { - switch { - case errors.Is(err, corerbd.ErrImageNotFound): - err = status.Errorf(codes.NotFound, "volume %s not found", volumeID) - case errors.Is(err, util.ErrPoolNotFound): - err = status.Errorf(codes.NotFound, "pool %s not found for %s", rbdVol.GetPoolName(), volumeID) - default: - err = status.Errorf(codes.Internal, err.Error()) - } - - return nil, err + return nil, getGRPCError(err) } mirror, err := rbdVol.ToMirror() if err != nil { @@ -438,16 +420,7 @@ func (rs *ReplicationServer) PromoteVolume(ctx context.Context, rbdVol, err := mgr.GetVolumeByID(ctx, volumeID) if err != nil { - switch { - case errors.Is(err, corerbd.ErrImageNotFound): - err = status.Errorf(codes.NotFound, "volume %s not found", volumeID) - case errors.Is(err, util.ErrPoolNotFound): - err = status.Errorf(codes.NotFound, "pool %s not found for %s", rbdVol.GetPoolName(), volumeID) - default: - err = status.Errorf(codes.Internal, err.Error()) - } - - return nil, err + return nil, getGRPCError(err) } mirror, err := rbdVol.ToMirror() if err != nil { @@ -540,16 +513,7 @@ func (rs *ReplicationServer) DemoteVolume(ctx context.Context, rbdVol, err := mgr.GetVolumeByID(ctx, volumeID) if err != nil { - switch { - case errors.Is(err, corerbd.ErrImageNotFound): - err = status.Errorf(codes.NotFound, "volume %s not found", volumeID) - case errors.Is(err, util.ErrPoolNotFound): - err = status.Errorf(codes.NotFound, "pool %s not found for %s", rbdVol.GetPoolName(), volumeID) - default: - err = status.Errorf(codes.Internal, err.Error()) - } - - return nil, err + return nil, getGRPCError(err) } mirror, err := rbdVol.ToMirror() if err != nil { @@ -659,16 +623,7 @@ func (rs *ReplicationServer) ResyncVolume(ctx context.Context, rbdVol, err := mgr.GetVolumeByID(ctx, volumeID) if err != nil { - switch { - case errors.Is(err, corerbd.ErrImageNotFound): - err = status.Errorf(codes.NotFound, "volume %s not found", volumeID) - case errors.Is(err, util.ErrPoolNotFound): - err = status.Errorf(codes.NotFound, "pool %s not found for %s", rbdVol.GetPoolName(), volumeID) - default: - err = status.Errorf(codes.Internal, err.Error()) - } - - return nil, err + return nil, getGRPCError(err) } mirror, err := rbdVol.ToMirror() if err != nil { @@ -830,6 +785,8 @@ func getGRPCError(err error) error { } errorStatusMap := map[error]codes.Code{ + corerbd.ErrImageNotFound: codes.NotFound, + util.ErrPoolNotFound: codes.NotFound, corerbd.ErrInvalidArgument: codes.InvalidArgument, corerbd.ErrFlattenInProgress: codes.Aborted, corerbd.ErrAborted: codes.Aborted, @@ -875,9 +832,9 @@ func (rs *ReplicationServer) GetVolumeReplicationInfo(ctx context.Context, if err != nil { switch { case errors.Is(err, corerbd.ErrImageNotFound): - err = status.Errorf(codes.NotFound, "volume %s not found", volumeID) + err = status.Errorf(codes.NotFound, err.Error()) case errors.Is(err, util.ErrPoolNotFound): - err = status.Errorf(codes.NotFound, "pool %s not found for %s", rbdVol.GetPoolName(), volumeID) + err = status.Errorf(codes.NotFound, err.Error()) default: err = status.Errorf(codes.Internal, err.Error()) } diff --git a/internal/csi-addons/rbd/replication_test.go b/internal/csi-addons/rbd/replication_test.go index 6da1b929eea..b4e81bf9519 100644 --- a/internal/csi-addons/rbd/replication_test.go +++ b/internal/csi-addons/rbd/replication_test.go @@ -27,6 +27,7 @@ import ( corerbd "github.com/ceph/ceph-csi/internal/rbd" "github.com/ceph/ceph-csi/internal/rbd/types" + "github.com/ceph/ceph-csi/internal/util" librbd "github.com/ceph/go-ceph/rbd" "github.com/ceph/go-ceph/rbd/admin" @@ -594,6 +595,16 @@ func TestGetGRPCError(t *testing.T) { err: nil, expectedErr: status.Error(codes.OK, "ok string"), }, + { + name: "ErrImageNotFound", + err: corerbd.ErrImageNotFound, + expectedErr: status.Error(codes.NotFound, corerbd.ErrImageNotFound.Error()), + }, + { + name: "ErrPoolNotFound", + err: util.ErrPoolNotFound, + expectedErr: status.Error(codes.NotFound, util.ErrPoolNotFound.Error()), + }, } for _, tt := range tests { From 37970ae21215531027f544d08ec5e4f6a37bbf9c Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Tue, 30 Jul 2024 18:42:35 +0200 Subject: [PATCH 08/11] rbd: add context to mirror interface adding required ctx to the mirror interface as ctx is required for the volumegroup operations. Signed-off-by: Madhu Rajanna --- internal/csi-addons/rbd/replication.go | 28 +++++++++++++------------- internal/rbd/controllerserver.go | 4 ++-- internal/rbd/mirror.go | 20 +++++++++--------- internal/rbd/replication.go | 7 ++++--- internal/rbd/types/mirror.go | 16 +++++++-------- 5 files changed, 38 insertions(+), 37 deletions(-) diff --git a/internal/csi-addons/rbd/replication.go b/internal/csi-addons/rbd/replication.go index 09b0ca3fee0..f58750892f1 100644 --- a/internal/csi-addons/rbd/replication.go +++ b/internal/csi-addons/rbd/replication.go @@ -300,7 +300,7 @@ func (rs *ReplicationServer) EnableVolumeReplication(ctx context.Context, return nil, err } - info, err := mirror.GetMirroringInfo() + info, err := mirror.GetMirroringInfo(ctx) if err != nil { log.ErrorLog(ctx, err.Error()) @@ -313,7 +313,7 @@ func (rs *ReplicationServer) EnableVolumeReplication(ctx context.Context, return nil, getGRPCError(err) } - err = mirror.EnableMirroring(mirroringMode) + err = mirror.EnableMirroring(ctx, mirroringMode) if err != nil { log.ErrorLog(ctx, err.Error()) @@ -365,7 +365,7 @@ func (rs *ReplicationServer) DisableVolumeReplication(ctx context.Context, return nil, err } - info, err := mirror.GetMirroringInfo() + info, err := mirror.GetMirroringInfo(ctx) if err != nil { log.ErrorLog(ctx, err.Error()) @@ -378,7 +378,7 @@ func (rs *ReplicationServer) DisableVolumeReplication(ctx context.Context, case librbd.MirrorImageDisabling.String(): return nil, status.Errorf(codes.Aborted, "%s is in disabling state", volumeID) case librbd.MirrorImageEnabled.String(): - err = corerbd.DisableVolumeReplication(mirror, info.IsPrimary(), force) + err = corerbd.DisableVolumeReplication(mirror, ctx, info.IsPrimary(), force) if err != nil { return nil, getGRPCError(err) } @@ -427,7 +427,7 @@ func (rs *ReplicationServer) PromoteVolume(ctx context.Context, return nil, status.Error(codes.Internal, err.Error()) } - info, err := mirror.GetMirroringInfo() + info, err := mirror.GetMirroringInfo(ctx) if err != nil { log.ErrorLog(ctx, err.Error()) @@ -447,9 +447,9 @@ func (rs *ReplicationServer) PromoteVolume(ctx context.Context, if req.GetForce() { // workaround for https://github.com/ceph/ceph-csi/issues/2736 // TODO: remove this workaround when the issue is fixed - err = mirror.ForcePromote(cr) + err = mirror.ForcePromote(ctx, cr) } else { - err = mirror.Promote(req.GetForce()) + err = mirror.Promote(ctx, req.GetForce()) } if err != nil { log.ErrorLog(ctx, err.Error()) @@ -527,7 +527,7 @@ func (rs *ReplicationServer) DemoteVolume(ctx context.Context, return nil, status.Error(codes.Internal, err.Error()) } - info, err := mirror.GetMirroringInfo() + info, err := mirror.GetMirroringInfo(ctx) if err != nil { log.ErrorLog(ctx, err.Error()) @@ -556,7 +556,7 @@ func (rs *ReplicationServer) DemoteVolume(ctx context.Context, return nil, status.Error(codes.Internal, err.Error()) } - err = mirror.Demote() + err = mirror.Demote(ctx) if err != nil { log.ErrorLog(ctx, err.Error()) @@ -630,7 +630,7 @@ func (rs *ReplicationServer) ResyncVolume(ctx context.Context, return nil, status.Error(codes.Internal, err.Error()) } - info, err := mirror.GetMirroringInfo() + info, err := mirror.GetMirroringInfo(ctx) if err != nil { // in case of Resync the image will get deleted and gets recreated and // it takes time for this operation. @@ -648,7 +648,7 @@ func (rs *ReplicationServer) ResyncVolume(ctx context.Context, return nil, status.Error(codes.InvalidArgument, "image is in primary state") } - sts, err := mirror.GetGlobalMirroringStatus() + sts, err := mirror.GetGlobalMirroringStatus(ctx) if err != nil { // the image gets recreated after issuing resync if errors.Is(err, corerbd.ErrImageNotFound) { @@ -719,7 +719,7 @@ func (rs *ReplicationServer) ResyncVolume(ctx context.Context, } log.DebugLog(ctx, "image %s, savedImageTime=%v, currentImageTime=%v", rbdVol, st, creationTime.AsTime()) if req.GetForce() && st.Equal(creationTime.AsTime()) { - err = mirror.Resync() + err = mirror.Resync(ctx) if err != nil { return nil, getGRPCError(err) } @@ -846,7 +846,7 @@ func (rs *ReplicationServer) GetVolumeReplicationInfo(ctx context.Context, return nil, status.Error(codes.Internal, err.Error()) } - info, err := mirror.GetMirroringInfo() + info, err := mirror.GetMirroringInfo(ctx) if err != nil { log.ErrorLog(ctx, err.Error()) @@ -862,7 +862,7 @@ func (rs *ReplicationServer) GetVolumeReplicationInfo(ctx context.Context, return nil, status.Error(codes.InvalidArgument, "image is not in primary state") } - mirrorStatus, err := mirror.GetGlobalMirroringStatus() + mirrorStatus, err := mirror.GetGlobalMirroringStatus(ctx) if err != nil { if errors.Is(err, corerbd.ErrImageNotFound) { return nil, status.Error(codes.Aborted, err.Error()) diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index cdf1443a5bb..56de60c58d0 100644 --- a/internal/rbd/controllerserver.go +++ b/internal/rbd/controllerserver.go @@ -988,7 +988,7 @@ func (cs *ControllerServer) DeleteVolume( func cleanupRBDImage(ctx context.Context, rbdVol *rbdVolume, cr *util.Credentials, ) (*csi.DeleteVolumeResponse, error) { - info, err := rbdVol.GetMirroringInfo() + info, err := rbdVol.GetMirroringInfo(ctx) if err != nil { log.ErrorLog(ctx, err.Error()) @@ -1007,7 +1007,7 @@ func cleanupRBDImage(ctx context.Context, // the image on all the remote (secondary) clusters will get // auto-deleted. This helps in garbage collecting the OMAP, PVC and PV // objects after failback operation. - sts, rErr := rbdVol.GetGlobalMirroringStatus() + sts, rErr := rbdVol.GetGlobalMirroringStatus(ctx) if rErr != nil { return nil, status.Error(codes.Internal, rErr.Error()) } diff --git a/internal/rbd/mirror.go b/internal/rbd/mirror.go index 08064dfa901..e20927d0144 100644 --- a/internal/rbd/mirror.go +++ b/internal/rbd/mirror.go @@ -63,7 +63,7 @@ func (rv *rbdVolume) HandleParentImageExistence( if err != nil { return fmt.Errorf("failed to get parent of image %s: %w", rv, err) } - parentMirroringInfo, err := parent.GetMirroringInfo() + parentMirroringInfo, err := parent.GetMirroringInfo(ctx) if err != nil { return fmt.Errorf( "failed to get mirroring info of parent %q of image %q: %w", @@ -82,7 +82,7 @@ func (rv *rbdVolume) HandleParentImageExistence( var _ types.Mirror = &rbdVolume{} // EnableMirroring enables mirroring on an image. -func (ri *rbdImage) EnableMirroring(mode librbd.ImageMirrorMode) error { +func (ri *rbdImage) EnableMirroring(_ context.Context, mode librbd.ImageMirrorMode) error { image, err := ri.open() if err != nil { return fmt.Errorf("failed to open image %q with error: %w", ri, err) @@ -98,7 +98,7 @@ func (ri *rbdImage) EnableMirroring(mode librbd.ImageMirrorMode) error { } // DisableMirroring disables mirroring on an image. -func (ri *rbdImage) DisableMirroring(force bool) error { +func (ri *rbdImage) DisableMirroring(_ context.Context, force bool) error { image, err := ri.open() if err != nil { return fmt.Errorf("failed to open image %q with error: %w", ri, err) @@ -114,7 +114,7 @@ func (ri *rbdImage) DisableMirroring(force bool) error { } // GetMirroringInfo gets mirroring information of an image. -func (ri *rbdImage) GetMirroringInfo() (types.MirrorInfo, error) { +func (ri *rbdImage) GetMirroringInfo(_ context.Context) (types.MirrorInfo, error) { image, err := ri.open() if err != nil { return nil, fmt.Errorf("failed to open image %q with error: %w", ri, err) @@ -130,7 +130,7 @@ func (ri *rbdImage) GetMirroringInfo() (types.MirrorInfo, error) { } // Promote promotes image to primary. -func (ri *rbdImage) Promote(force bool) error { +func (ri *rbdImage) Promote(_ context.Context, force bool) error { image, err := ri.open() if err != nil { return fmt.Errorf("failed to open image %q with error: %w", ri, err) @@ -147,7 +147,7 @@ func (ri *rbdImage) Promote(force bool) error { // ForcePromote promotes image to primary with force option with 2 minutes // timeout. If there is no response within 2 minutes,the rbd CLI process will be // killed and an error is returned. -func (rv *rbdVolume) ForcePromote(cr *util.Credentials) error { +func (rv *rbdVolume) ForcePromote(ctx context.Context, cr *util.Credentials) error { promoteArgs := []string{ "mirror", "image", "promote", rv.String(), @@ -157,7 +157,7 @@ func (rv *rbdVolume) ForcePromote(cr *util.Credentials) error { "--keyfile=" + cr.KeyFile, } _, stderr, err := util.ExecCommandWithTimeout( - context.TODO(), + ctx, // 2 minutes timeout as the Replication RPC timeout is 2.5 minutes. 2*time.Minute, "rbd", @@ -175,7 +175,7 @@ func (rv *rbdVolume) ForcePromote(cr *util.Credentials) error { } // Demote demotes image to secondary. -func (ri *rbdImage) Demote() error { +func (ri *rbdImage) Demote(_ context.Context) error { image, err := ri.open() if err != nil { return fmt.Errorf("failed to open image %q with error: %w", ri, err) @@ -190,7 +190,7 @@ func (ri *rbdImage) Demote() error { } // Resync resync image to correct the split-brain. -func (ri *rbdImage) Resync() error { +func (ri *rbdImage) Resync(_ context.Context) error { image, err := ri.open() if err != nil { return fmt.Errorf("failed to open image %q with error: %w", ri, err) @@ -208,7 +208,7 @@ func (ri *rbdImage) Resync() error { } // GetGlobalMirroringStatus get the mirroring status of an image. -func (ri *rbdImage) GetGlobalMirroringStatus() (types.GlobalStatus, error) { +func (ri *rbdImage) GetGlobalMirroringStatus(_ context.Context) (types.GlobalStatus, error) { image, err := ri.open() if err != nil { return nil, fmt.Errorf("failed to open image %q with error: %w", ri, err) diff --git a/internal/rbd/replication.go b/internal/rbd/replication.go index 86c31cd85fe..badcb12ce62 100644 --- a/internal/rbd/replication.go +++ b/internal/rbd/replication.go @@ -46,6 +46,7 @@ func (rv *rbdVolume) RepairResyncedImageID(ctx context.Context, ready bool) erro } func DisableVolumeReplication(mirror types.Mirror, + ctx context.Context, primary, force bool, ) error { @@ -62,7 +63,7 @@ func DisableVolumeReplication(mirror types.Mirror, // disabled the image on all the remote (secondary) clusters will get // auto-deleted. This helps in garbage collecting the volume // replication Kubernetes artifacts after failback operation. - sts, rErr := mirror.GetGlobalMirroringStatus() + sts, rErr := mirror.GetGlobalMirroringStatus(ctx) if rErr != nil { return fmt.Errorf("failed to get global state: %w", rErr) } @@ -78,13 +79,13 @@ func DisableVolumeReplication(mirror types.Mirror, return fmt.Errorf("%w: secondary image status is up=%t and state=%s", ErrInvalidArgument, localStatus.IsUP(), localStatus.GetState()) } - err := mirror.DisableMirroring(force) + err := mirror.DisableMirroring(ctx, force) if err != nil { return fmt.Errorf("failed to disable image mirroring: %w", err) } // the image state can be still disabling once we disable the mirroring // check the mirroring is disabled or not - info, err := mirror.GetMirroringInfo() + info, err := mirror.GetMirroringInfo(ctx) if err != nil { return fmt.Errorf("failed to get mirroring info of image: %w", err) } diff --git a/internal/rbd/types/mirror.go b/internal/rbd/types/mirror.go index 131fad8441a..12c0bffdfe0 100644 --- a/internal/rbd/types/mirror.go +++ b/internal/rbd/types/mirror.go @@ -39,21 +39,21 @@ const ( // Mirror is the interface for managing mirroring on an RBD image or a group. type Mirror interface { // EnableMirroring enables mirroring on the resource with the specified mode. - EnableMirroring(mode librbd.ImageMirrorMode) error + EnableMirroring(ctx context.Context, mode librbd.ImageMirrorMode) error // DisableMirroring disables mirroring on the resource with the option to force the operation - DisableMirroring(force bool) error + DisableMirroring(ctx context.Context, force bool) error // Promote promotes the resource to primary status with the option to force the operation - Promote(force bool) error + Promote(ctx context.Context, force bool) error // ForcePromote promotes the resource to primary status with a timeout - ForcePromote(cr *util.Credentials) error + ForcePromote(ctx context.Context, cr *util.Credentials) error // Demote demotes the resource to secondary status - Demote() error + Demote(ctx context.Context) error // Resync resynchronizes the resource - Resync() error + Resync(ctx context.Context) error // GetMirroringInfo returns the mirroring information of the resource - GetMirroringInfo() (MirrorInfo, error) + GetMirroringInfo(ctx context.Context) (MirrorInfo, error) // GetMirroringInfo returns the mirroring information of the resource - GetGlobalMirroringStatus() (GlobalStatus, error) + GetGlobalMirroringStatus(ctx context.Context) (GlobalStatus, error) // AddSnapshotScheduling adds a snapshot scheduling to the resource AddSnapshotScheduling(interval admin.Interval, startTime admin.StartTime) error } From f7c78ae4feedbdce8d98a0f47b72f7c7ca0d3fa7 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Mon, 29 Jul 2024 14:47:57 +0200 Subject: [PATCH 09/11] rbd: update group Stringer method updated the group stringer method to have pool and namespace for proper debugging/logging and to use it with CLI as agrument as well. Signed-off-by: Madhu Rajanna --- internal/rbd/group/volume_group.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/internal/rbd/group/volume_group.go b/internal/rbd/group/volume_group.go index e4f818b3ca4..6e6e5901942 100644 --- a/internal/rbd/group/volume_group.go +++ b/internal/rbd/group/volume_group.go @@ -151,18 +151,17 @@ func GetVolumeGroup( return vg, nil } -// String makes it easy to include the volumeGroup object in log and error -// messages. +// String returns the image-spec (pool/{namespace}/{name}) format of the group. func (vg *volumeGroup) String() string { - if vg.name != "" { - return vg.name + if vg.namespace != "" && vg.pool != "" && vg.name != "" { + return fmt.Sprintf("%s/%s/%s", vg.pool, vg.namespace, vg.name) } - if vg.id != "" { - return vg.id + if vg.name != "" && vg.pool != "" { + return fmt.Sprintf("%s/%s", vg.pool, vg.name) } - return fmt.Sprintf("", *vg) + return fmt.Sprintf("", *vg) } // GetID returns the CSI-Addons VolumeGroupId of the VolumeGroup. From 3a8981a7354a1c271c5a2a4f39e7a56e74293612 Mon Sep 17 00:00:00 2001 From: Madhu Rajanna Date: Tue, 30 Jul 2024 16:54:39 +0200 Subject: [PATCH 10/11] rbd: add support to get volumegroupID updated GetIDFromReplication to return volumeGroupID if its present. Signed-off-by: Madhu Rajanna --- internal/csi-common/utils.go | 5 +++ internal/csi-common/utils_test.go | 60 +++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/internal/csi-common/utils.go b/internal/csi-common/utils.go index 3f2dadd126a..91db95f9324 100644 --- a/internal/csi-common/utils.go +++ b/internal/csi-common/utils.go @@ -128,6 +128,11 @@ func GetIDFromReplication(req interface{}) string { if src != nil && src.GetVolume() != nil { reqID = src.GetVolume().GetVolumeId() } + if reqID == "" { + if src != nil && src.GetVolumegroup() != nil { + reqID = src.GetVolumegroup().GetVolumeGroupId() + } + } if reqID == "" { reqID = r.GetVolumeId() //nolint:nolintlint,staticcheck // req.VolumeId is deprecated } diff --git a/internal/csi-common/utils_test.go b/internal/csi-common/utils_test.go index a3c230d0d1a..9ca2a9105a8 100644 --- a/internal/csi-common/utils_test.go +++ b/internal/csi-common/utils_test.go @@ -150,6 +150,61 @@ func TestGetReqID(t *testing.T) { }, }, }, + // volumeGroupId is set in ReplicationSource + &replication.EnableVolumeReplicationRequest{ + ReplicationSource: &replication.ReplicationSource{ + Type: &replication.ReplicationSource_Volumegroup{ + Volumegroup: &replication.ReplicationSource_VolumeGroupSource{ + VolumeGroupId: fakeID, + }, + }, + }, + }, + &replication.DisableVolumeReplicationRequest{ + ReplicationSource: &replication.ReplicationSource{ + Type: &replication.ReplicationSource_Volumegroup{ + Volumegroup: &replication.ReplicationSource_VolumeGroupSource{ + VolumeGroupId: fakeID, + }, + }, + }, + }, + &replication.PromoteVolumeRequest{ + ReplicationSource: &replication.ReplicationSource{ + Type: &replication.ReplicationSource_Volumegroup{ + Volumegroup: &replication.ReplicationSource_VolumeGroupSource{ + VolumeGroupId: fakeID, + }, + }, + }, + }, + &replication.DemoteVolumeRequest{ + ReplicationSource: &replication.ReplicationSource{ + Type: &replication.ReplicationSource_Volumegroup{ + Volumegroup: &replication.ReplicationSource_VolumeGroupSource{ + VolumeGroupId: fakeID, + }, + }, + }, + }, + &replication.ResyncVolumeRequest{ + ReplicationSource: &replication.ReplicationSource{ + Type: &replication.ReplicationSource_Volumegroup{ + Volumegroup: &replication.ReplicationSource_VolumeGroupSource{ + VolumeGroupId: fakeID, + }, + }, + }, + }, + &replication.GetVolumeReplicationInfoRequest{ + ReplicationSource: &replication.ReplicationSource{ + Type: &replication.ReplicationSource_Volumegroup{ + Volumegroup: &replication.ReplicationSource_VolumeGroupSource{ + VolumeGroupId: fakeID, + }, + }, + }, + }, } for _, r := range req { if got := getReqID(r); got != fakeID { @@ -161,6 +216,11 @@ func TestGetReqID(t *testing.T) { if got := getReqID(nil); got != "" { t.Errorf("getReqID() = %v, want empty string", got) } + + // test when both volume and group id not set + if got := getReqID(&replication.EnableVolumeReplicationRequest{}); got != "" { + t.Errorf("getReqID() = %v, want empty string", got) + } } func TestFilesystemNodeGetVolumeStats(t *testing.T) { From e3697f4d3e4969850c1559921867f7a317977cb4 Mon Sep 17 00:00:00 2001 From: Niels de Vos Date: Wed, 31 Jul 2024 14:53:29 +0200 Subject: [PATCH 11/11] doc: inform users that the OS in the container-image is updated The Squid container-image that is used as base for the Ceph-CSI container-image uses CentOS Stream 9. Closes: #4659 Signed-off-by: Niels de Vos --- PendingReleaseNotes.md | 1 + 1 file changed, 1 insertion(+) diff --git a/PendingReleaseNotes.md b/PendingReleaseNotes.md index b9e8542143e..a77eb58d405 100644 --- a/PendingReleaseNotes.md +++ b/PendingReleaseNotes.md @@ -11,5 +11,6 @@ - cephfs: support omap data store in radosnamespace via cli argument in [PR](https://github.com/ceph/ceph-csi/pull/4652) - deploy: radosNamespaceCephFS can be configured for ceph-csi-cephfs chart in [PR](https://github.com/ceph/ceph-csi/pull/4652) - build: update ceph release to squid in [PR](https://github.com/ceph/ceph-csi/pull/4735) +- build: CentOS Stream 9 is used as OS in the container-images [PR](https://github.com/ceph/ceph-csi/pull/4735) ## NOTE