rbd: Add exclusive-lock and journaling image features for RBD image #684

woohhan · 2019-10-15T04:25:14Z

Describe what this PR does

Add exclusive-lock and journaling image features for rbd image

Current rbd plugin only supports the layering feature
for rbd image. Add exclusive-lock and journaling image
features for the rbd plugin.

Is there anything that requires special attention

If we enable the journaling feature, krbd doesn't work for the mount. We need to force mapping by rbd-nbd.

Related issues

closes: #676

Future concerns

NONE

woohhan · 2019-10-15T04:33:17Z

Hi, I'm glad to join ceph-csi. If I have wrong code or concept, please let me know. I will fix up immediately. Thanks!

Madhu-1

@techhanx Thanks for the PR 👍

I would like to see a few things in this PR

passing exlusive-lock and journaling from storageclass
E2E to cover this scenario
Testing with krbd and rbd-nbd

pkg/rbd/rbd_util.go

Madhu-1 · 2019-10-15T05:03:43Z

@dillaman PTAL

woohhan · 2019-10-15T09:00:46Z

@Madhu-1
Thanks for your reply. I will add E2E test to cover this.
but i didn't understand other two suggestion. could you please explain more?

passing exlusive-lock and journaling from storageclass
I'm testing by creating storage class parameter with 'imageFeatures: layering,journaling,exclusive-lock' and check the image has these features. is there any other to do? (except e2e tests)
Testing with krbd and rbd-nbd
If we enable image journaling option, we can't mount with krbd. because It is not supported that mounting the journaling volume in krbd. we need to use rbd-nbd.

Madhu-1 · 2019-10-15T09:09:51Z

@Madhu-1
Thanks for your reply. I will add E2E test to cover this.
but i didn't understand other two suggestion. could you please explain more?

passing exlusive-lock and journaling from storageclass

okay I missed adding more information we need to update documentation and storageclass examples for these features https://github.com/ceph/ceph-csi/blob/master/examples/rbd/storageclass.yaml#L24-L26 and https://github.com/ceph/ceph-csi/blob/master/docs/deploy-rbd.md

I'm testing by creating storage class parameter with 'imageFeatures: layering,journaling,exclusive-lock' and check the image has these features. is there any other to do? (except e2e tests)

Testing with krbd and rbd-nbd
If we enable image journaling option, we can't mount with krbd. because It is not supported that mounting the journaling volume in krbd. we need to use rbd-nbd.

i that case we need to error out in the node stage request if the mounter is krbd

woohhan · 2019-11-30T13:53:24Z

@Madhu-1 I applied your suggestion:) Could you please check the commit? Thanks.

dimm0 · 2020-01-04T01:36:30Z

How about object-map feature? Any reason to not enable it?

woohhan · 2020-01-04T11:37:32Z

@dimm0 Yes, there's no reason to not enable the object-map feature. But the pr stale for now, If pr is processed, I will add the feature. Thanks :)

dimm0 · 2020-03-03T01:43:42Z

Is this still pending in the new version?

cr1cr1 · 2020-03-23T12:37:10Z

@Madhu-1 did you get a chance to review this PR? Thank you.

Madhu-1

changes look good, can you please resolve merge conflicts

pkg/rbd/rbd_util.go

woohhan · 2020-04-14T06:37:37Z

@Madhu-1 I made some code changes to make it easy to add image features in the future. For example, if we want to add a fast-diff image feature, we need to specify it requires an object map feature. Could you please review the code again?

Madhu-1

changes looks good, Thanks for adding Unit testing. i will review it once again

pkg/rbd/rbd_util.go

woohhan · 2020-04-15T04:25:24Z

@Madhu-1 I am looking at how to change this PR and trying to change the ImageFeature field from string to librbd.FeatureSet in rbdVolume struct. Can this makes another impact? Why is there a JSON tag in the rbdVolume type? The implementation of FeatureSet is here: https://github.com/ceph/go-ceph/blob/master/rbd/features.go#L106

current rbdVolume

type rbdVolume struct {
	RbdImageName       string
// ...
	ImageFeatures      string `json:"imageFeatures"`
// ...
}

changed rbdVolume

type rbdVolume struct {
	RbdImageName       string
// ...
	ImageFeatures      librbd.FeatureSet
// ...
}

Madhu-1 · 2020-04-15T04:33:05Z

@Madhu-1 I am looking at how to change this PR and trying to change the ImageFeature field from string to librbd.FeatureSet in rbdVolume struct. Can this makes another impact? Why is there a JSON tag in the rbdVolume type? The implementation of FeatureSet is here: https://github.com/ceph/go-ceph/blob/master/rbd/features.go#L106

current rbdVolume
type rbdVolume struct {
	RbdImageName       string
// ...
	ImageFeatures      string `json:"imageFeatures"`
// ...
}
changed rbdVolume
type rbdVolume struct {
	RbdImageName       string
// ...
	ImageFeatures      librbd.FeatureSet
// ...
}

we have json tag to support backward compatibility for v1.0.0 volumes, removing/changing the fields may result in breakage

woohhan · 2020-04-15T04:35:26Z

we have json tag to support backward compatibility for v1.0.0 volumes, removing/changing the fields may result in breakage

Ok, I will proceed without changing the field. Thanks!

woohhan · 2020-04-15T11:48:41Z

@Madhu-1 I added a new commit, and it seems to work fine. Can you please review again?

nixpanic

Looks pretty good to me. Just a few minor comments that can be addressed.

go.mod

pkg/rbd/rbd_util_test.go

This PR upgrade go-ceph version to v0.3.0 Signed-off-by: woohhan <woohyung_han@tmax.co.kr>

Current rbd plugin only supports the layering feature for rbd image. Add exclusive-lock and journaling image features for the rbd plugin. Signed-off-by: woohhan <woohyung_han@tmax.co.kr>

nixpanic · 2020-05-12T08:01:32Z

I think the CI failed because of this problem:

May 11 12:21:12.848: INFO: At 2020-05-11 12:11:12 +0000 UTC - event for csi-rbd-demo-pod: {attachdetach-controller } SuccessfulAttachVolume: AttachVolume.Attach succeeded for volume "pvc-806abbbd-9380-11ea-9312-42010a140134" 
May 11 12:21:12.848: INFO: At 2020-05-11 12:11:29 +0000 UTC - event for csi-rbd-demo-pod: {kubelet minikube} FailedMount: MountVolume.MountDevice failed for volume "pvc-806abbbd-9380-11ea-9312-42010a140134" : rpc error: code = Internal desc = rbd: map failed exit status 6, rbd output: rbd: sysfs write failed
RBD image feature set mismatch. You can disable features unsupported by the kernel with "rbd feature disable replicapool/csi-vol-80a56ad0-9380-11ea-9c69-0242ac110010 journaling".

In case the attacher/rbd-plugin needs to write to /sys/..., it may need additional configuration in the deployment spec.

(note that the Semantic Pull Request bot is not enforced, failing is accepted - and we don't understand yet why it fails)

woohhan · 2020-05-13T10:24:34Z

I think the CI failed because of this problem:

May 11 12:21:12.848: INFO: At 2020-05-11 12:11:12 +0000 UTC - event for csi-rbd-demo-pod: {attachdetach-controller } SuccessfulAttachVolume: AttachVolume.Attach succeeded for volume "pvc-806abbbd-9380-11ea-9312-42010a140134" 
May 11 12:21:12.848: INFO: At 2020-05-11 12:11:29 +0000 UTC - event for csi-rbd-demo-pod: {kubelet minikube} FailedMount: MountVolume.MountDevice failed for volume "pvc-806abbbd-9380-11ea-9312-42010a140134" : rpc error: code = Internal desc = rbd: map failed exit status 6, rbd output: rbd: sysfs write failed
RBD image feature set mismatch. You can disable features unsupported by the kernel with "rbd feature disable replicapool/csi-vol-80a56ad0-9380-11ea-9c69-0242ac110010 journaling".

In case the attacher/rbd-plugin needs to write to /sys/..., it may need additional configuration in the deployment spec.

(note that the Semantic Pull Request bot is not enforced, failing is accepted - and we don't understand yet why it fails)

@nixpanic looks deploy mounts /sys here? But it's strange, I haven't changed the code. I think I need to build a test environment again and fix it...

Madhu-1 · 2020-05-13T10:48:01Z

internal/rbd/rbd_util.go

 	}
+	if err = validateImageFeatures(rbdVol.ImageFeatures, rbdVol.Mounter); err != nil {
+		// for backward compatibility, just log and pass
+		klog.Errorf(err.Error())


logging and passing is not a good idea.

Madhu-1 · 2020-05-13T10:49:21Z

internal/rbd/rbd_util.go

+	for _, f := range arr {
+		sf, found := supportedFeatures[f]
+		if !found {
+			return errors.Errorf("invalid feature %s for volume csi-rbdplugin", f)


Suggested change

return errors.Errorf("invalid feature %s for volume csi-rbdplugin", f)

return errors.Errorf("invalid feature %s for %s", f,reqName)

please pass request name for more clarity

Madhu-1 · 2020-05-13T10:51:21Z

E0511 12:16:37.634418 2911 rbd_util.go:604] feature journaling requires rbd-nbd for mounter
I0511 12:16:37.634510 2911 rbd_util.go:606] ID: 72 Req-ID: 0001-0024-b7b15817-390a-4571-b21f-ca6e7f7a502b-0000000000000001-1758ca5c-9380-11ea-90d0-0242ac110011 setting rbd image features: layering,journaling,exclusive-lock, mounter: rbd
I0511 12:16:37.634532 2911 rbd_util.go:608] ID: 72 Req-ID: 0001-0024-b7b15817-390a-4571-b21f-ca6e7f7a502b-0000000000000001-1758ca5c-9380-11ea-90d0-0242ac110011 setting disableInUseChecks on rbd volume to: false
I0511 12:16:37.841479 2911 rbd_util.go:231] ID: 72 Req-ID: 0001-0024-b7b15817-390a-4571-b21f-ca6e7f7a502b-0000000000000001-1758ca5c-9380-11ea-90d0-0242ac110011 rbd: status csi-vol-1758ca5c-9380-11ea-90d0-0242ac110011 using mon rook-ceph-mon-a.rook-ceph.svc.cluster.local:6789, pool replicapool
W0511 12:16:37.881034 2911 rbd_util.go:253] ID: 72 Req-ID: 0001-0024-b7b15817-390a-4571-b21f-ca6e7f7a502b-0000000000000001-1758ca5c-9380-11ea-90d0-0242ac110011 rbd: no watchers on csi-vol-1758ca5c-9380-11ea-90d0-0242ac110011
I0511 12:16:37.881080 2911 rbd_attach.go:208] ID: 72 Req-ID: 0001-0024-b7b15817-390a-4571-b21f-ca6e7f7a502b-0000000000000001-1758ca5c-9380-11ea-90d0-0242ac110011 rbd: map mon rook-ceph-mon-a.rook-ceph.svc.cluster.local:6789
W0511 12:16:37.943905 2911 rbd_attach.go:231] ID: 72 Req-ID: 0001-0024-b7b15817-390a-4571-b21f-ca6e7f7a502b-0000000000000001-1758ca5c-9380-11ea-90d0-0242ac110011 rbd: map error exit status 6, rbd output: rbd: sysfs write failed
RBD image feature set mismatch. You can disable features unsupported by the kernel with "rbd feature disable replicapool/csi-vol-1758ca5c-9380-11ea-90d0-0242ac110011 journaling".
In some cases useful info is found in syslog - try "dmesg | tail".
rbd: map failed: (6) No such device or address
E0511 12:16:37.944081 2911 utils.go:163] ID: 72 Req-ID: 0001-0024-b7b15817-390a-4571-b21f-ca6e7f7a502b-0000000000000001-1758ca5c-9380-11ea-90d0-0242ac110011 GRPC error: rpc error: code = Internal desc = rbd: map failed exit status 6, rbd output: rbd: sysfs write failed
RBD image feature set mismatch. You can disable features unsupported by the kernel with "rbd feature disable replicapool/csi-vol-1758ca5c-9380-11ea-90d0-0242ac110011 journaling".
In some cases useful info is found in syslog - try "dmesg | tail".
rbd: map failed: (6) No such device or address

the issue here is its using rbd mounted for image features layering,journaling,exclusive-lock is it something supported?

mergify · 2020-05-21T08:04:46Z

This pull request now has conflicts with the target branch. Could you please resolve conflicts and force push the corrected changes? 🙏

Madhu-1 · 2020-07-20T12:10:58Z

moved this out of release-v3.0.0 milestone

dimm0 · 2020-07-21T01:10:44Z

moved this out of release-v3.0.0 milestone

Why!?

Madhu-1 · 2020-07-21T02:39:09Z

moved this out of release-v3.0.0 milestone

Why!?

As this PR is stale for at least last 2 months moved it to next release -v3.1.0

Madhu-1 · 2020-08-06T05:22:24Z

@woohhan do you have free time to work on this one? or shall I take it over and mention your name as a co-author?

woohhan · 2020-08-06T06:15:37Z

@Madhu-1 Could you please take it over? unfortunately i have no time to proceed... thanks

Madhu-1 · 2020-08-06T06:16:38Z

@Madhu-1 Could you please take it over? unfortunately i have no time to proceed... thanks

@woohhan Thanks for the confirmation 👍

Current rbd plugin only supports the layering feature for rbd image. Add exclusive-lock and journaling image features for the rbd. Signed-off-by: woohhan <woohyung_han@tmax.co.kr> Co-authored-by: Madhu Rajanna <madhupr007@gmail.com> This will abandon the PR ceph#684

Madhu-1 · 2020-08-06T08:01:29Z

closing this one in favor of #1325

Current rbd plugin only supports the layering feature for rbd image. Add exclusive-lock and journaling image features for the rbd. Signed-off-by: woohhan <woohyung_han@tmax.co.kr> Co-authored-by: Madhu Rajanna <madhupr007@gmail.com> This will abandon the PR ceph#684

Madhu-1 requested changes Oct 15, 2019

View reviewed changes

pkg/rbd/rbd_util.go Outdated Show resolved Hide resolved

woohhan changed the title ~~Add rbd image features that exclusive-lock, journaling~~ WIP: Add rbd image features that exclusive-lock, journaling Nov 30, 2019

woohhan requested a review from Madhu-1 November 30, 2019 13:54

woohhan changed the title ~~WIP: Add rbd image features that exclusive-lock, journaling~~ Add rbd image features that exclusive-lock, journaling Dec 1, 2019

Madhu-1 requested changes Apr 13, 2020

View reviewed changes

pkg/rbd/rbd_util.go Outdated Show resolved Hide resolved

Madhu-1 added this to the release-3.0.0 milestone Apr 13, 2020

woohhan requested a review from Madhu-1 April 14, 2020 06:37

woohhan changed the title ~~Add rbd image features that exclusive-lock, journaling~~ Add exclusive-lock and journlaing image features for RBD image Apr 14, 2020

Madhu-1 reviewed Apr 14, 2020

View reviewed changes

nixpanic requested changes Apr 14, 2020

View reviewed changes

pkg/rbd/rbd_util.go Outdated Show resolved Hide resolved

nixpanic added this to In progress in Use go-ceph for volume management operations via automation Apr 14, 2020

woohhan changed the title ~~Add exclusive-lock and journlaing image features for RBD image~~ WIP: Add exclusive-lock and journlaing image features for RBD image Apr 15, 2020

woohhan requested review from Madhu-1 and nixpanic April 15, 2020 11:46

woohhan changed the title ~~WIP: Add exclusive-lock and journlaing image features for RBD image~~ Add exclusive-lock and journlaing image features for RBD image Apr 15, 2020

nixpanic requested changes Apr 15, 2020

View reviewed changes

go.mod Outdated Show resolved Hide resolved

pkg/rbd/rbd_util_test.go Outdated Show resolved Hide resolved

woohhan added 2 commits May 11, 2020 19:27

Upgrade: Changed the version of go-ceph from 0.2.0 to 0.3.0

42f50b5

This PR upgrade go-ceph version to v0.3.0 Signed-off-by: woohhan <woohyung_han@tmax.co.kr>

rbd: Add exclusive-lock and journaling image features for rbd image

5d6f73c

Current rbd plugin only supports the layering feature for rbd image. Add exclusive-lock and journaling image features for the rbd plugin. Signed-off-by: woohhan <woohyung_han@tmax.co.kr>

Madhu-1 reviewed May 13, 2020

View reviewed changes

Madhu-1 removed this from the release-3.0.0 milestone Jul 20, 2020

Madhu-1 added this to the release-v3.1.0 milestone Jul 21, 2020

Madhu-1 mentioned this pull request Aug 6, 2020

rbd: add exclusive-lock and journaling image features for rbd image #1325

Merged

Madhu-1 closed this Aug 6, 2020

Use go-ceph for volume management operations automation moved this from In progress to Done Aug 6, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rbd: Add exclusive-lock and journaling image features for RBD image #684

rbd: Add exclusive-lock and journaling image features for RBD image #684

woohhan commented Oct 15, 2019 •

edited

woohhan commented Oct 15, 2019

Madhu-1 left a comment

Madhu-1 commented Oct 15, 2019

woohhan commented Oct 15, 2019

Madhu-1 commented Oct 15, 2019

woohhan commented Nov 30, 2019 •

edited

dimm0 commented Jan 4, 2020

woohhan commented Jan 4, 2020 •

edited

dimm0 commented Mar 3, 2020

cr1cr1 commented Mar 23, 2020

Madhu-1 left a comment

woohhan commented Apr 14, 2020 •

edited

Madhu-1 left a comment

woohhan commented Apr 15, 2020

Madhu-1 commented Apr 15, 2020

woohhan commented Apr 15, 2020 •

edited

woohhan commented Apr 15, 2020

nixpanic left a comment

nixpanic commented May 12, 2020

woohhan commented May 13, 2020 •

edited

Madhu-1 May 13, 2020

Madhu-1 May 13, 2020

Madhu-1 commented May 13, 2020 •

edited

mergify bot commented May 21, 2020

Madhu-1 commented Jul 20, 2020

dimm0 commented Jul 21, 2020

Madhu-1 commented Jul 21, 2020

Madhu-1 commented Aug 6, 2020

woohhan commented Aug 6, 2020

Madhu-1 commented Aug 6, 2020

Madhu-1 commented Aug 6, 2020

	return errors.Errorf("invalid feature %s for volume csi-rbdplugin", f)
	return errors.Errorf("invalid feature %s for %s", f,reqName)

rbd: Add exclusive-lock and journaling image features for RBD image #684

rbd: Add exclusive-lock and journaling image features for RBD image #684

Conversation

woohhan commented Oct 15, 2019 • edited

Describe what this PR does

Is there anything that requires special attention

Related issues

Future concerns

woohhan commented Oct 15, 2019

Madhu-1 left a comment

Choose a reason for hiding this comment

Madhu-1 commented Oct 15, 2019

woohhan commented Oct 15, 2019

Madhu-1 commented Oct 15, 2019

woohhan commented Nov 30, 2019 • edited

dimm0 commented Jan 4, 2020

woohhan commented Jan 4, 2020 • edited

dimm0 commented Mar 3, 2020

cr1cr1 commented Mar 23, 2020

Madhu-1 left a comment

Choose a reason for hiding this comment

woohhan commented Apr 14, 2020 • edited

Madhu-1 left a comment

Choose a reason for hiding this comment

woohhan commented Apr 15, 2020

Madhu-1 commented Apr 15, 2020

woohhan commented Apr 15, 2020 • edited

woohhan commented Apr 15, 2020

nixpanic left a comment

Choose a reason for hiding this comment

nixpanic commented May 12, 2020

woohhan commented May 13, 2020 • edited

Madhu-1 May 13, 2020

Choose a reason for hiding this comment

Madhu-1 May 13, 2020

Choose a reason for hiding this comment

Madhu-1 commented May 13, 2020 • edited

mergify bot commented May 21, 2020

Madhu-1 commented Jul 20, 2020

dimm0 commented Jul 21, 2020

Madhu-1 commented Jul 21, 2020

Madhu-1 commented Aug 6, 2020

woohhan commented Aug 6, 2020

Madhu-1 commented Aug 6, 2020

Madhu-1 commented Aug 6, 2020

woohhan commented Oct 15, 2019 •

edited

woohhan commented Nov 30, 2019 •

edited

woohhan commented Jan 4, 2020 •

edited

woohhan commented Apr 14, 2020 •

edited

woohhan commented Apr 15, 2020 •

edited

woohhan commented May 13, 2020 •

edited

Madhu-1 commented May 13, 2020 •

edited