rbd: use go-ceph rbd admin task api for 'trash remove' & 'flatten' #2633

Rakshith-R · 2021-11-11T06:15:49Z

rebase: use go-ceph v0.13.0
rbd: use go-ceph rbd admin task api instead of cli
rbd: remove redundant rbdVolume.connect() in flattneRbdImage()
rbd: remove redundant util.Credentials arg from flattenRbdImage()
rbd: refractor deleteImage() & trashRemoveImage()

Closes: #2186

New Mgr Not Supported errors thrown by go-ceph

API call not implemented server-side: No handler found for 'rbd task add trash remove' (rook 1.2.0, ceph v:14.2.2)
rados: ret=-13, Permission denied: "access denied: does your client key have mgr caps? See http://docs.ceph.com/en/latest/mgr/administrator/#client-authentication" (when user secret does not have mgr caps)

Rakshith-R · 2021-11-17T04:18:02Z

/retest ci/centos/k8s-e2e-external-storage/1.22

Rakshith-R · 2021-11-17T04:18:19Z

/retest ci/centos/k8s-e2e-external-storage/1.22

failed due to #2264
https://jenkins-ceph-csi.apps.ocp.ci.centos.org/blue/organizations/jenkins/k8s-e2e-external-storage-1.22/detail/k8s-e2e-external-storage-1.22/531/pipeline

humblec · 2021-11-17T05:55:14Z

build.env

@@ -13,7 +13,7 @@ CSI_IMAGE_VERSION=canary

 # Ceph version to use
 BASE_IMAGE=docker.io/ceph/ceph:v16
-CEPH_VERSION=octopus
+BUILD_TAGS=ceph_preview


can we document the ceph_preview thing ?

can we document the ceph_preview thing ?

Suggested change

BUILD_TAGS=ceph_preview

# ceph_preview tag is required to access go-ceph APIs which are under preview.

BUILD_TAGS=ceph_preview

@humblec is this sufficient ?

add a link to https://github.com/ceph/go-ceph/blob/master/docs/api-stability.md#preview here

I do not think we should set ceph_preview as default. Make sure that APIs from go-ceph are stable and part of the latest release.

ceph_preview is removed and go-ceph is upgraded to v0.13.0 in this pr instead

Madhu-1

LGTM . @Rakshith-R have you verified its working with older ceph releases where we don't have manager command support?

Madhu-1 · 2021-11-18T05:25:14Z

internal/util/connection.go

@@ -123,6 +123,16 @@ func (cc *ClusterConnection) GetRBDAdmin() (*ra.RBDAdmin, error) {
 	return ra.NewFromConn(cc.conn), nil
 }

+// GetTaskAdmin returns TaskAdmin to add tasks on rbd volumes.


rbd volumes to rbd images

Rakshith-R · 2021-11-18T05:27:27Z

LGTM . @Rakshith-R have you verified its working with older ceph releases where we don't have manager command support?

Yes, I ran it with ceph 14.2.2 where there's rbd task support, works as expected.

Madhu-1 · 2021-11-18T05:28:28Z

LGTM . @Rakshith-R have you verified its working with older ceph releases where we don't have manager command support?

Yes, I ran it with ceph 14.2.2 where there's rbd task support, works as expected.

you mean no rbd task support?

Rakshith-R · 2021-11-18T05:36:36Z

LGTM . @Rakshith-R have you verified its working with older ceph releases where we don't have manager command support?

Yes, I ran it with ceph 14.2.2 where there's rbd task support, works as expected.

you mean no rbd task support?

yes 😅
*ceph 14.2.2 where there's * no * rbd task support, works as expected.

nixpanic

Suggestion to squash the last two patches together (drop cr from flattenRbdImage()).

Having ceph_preview as default in the BUILD_TAGS is not very clean, we should probably wait until go-ceph provides a version where there is no need to set the build tag.

nixpanic · 2021-11-18T07:48:03Z

build.env

@@ -13,7 +13,7 @@ CSI_IMAGE_VERSION=canary

 # Ceph version to use
 BASE_IMAGE=docker.io/ceph/ceph:v16
-CEPH_VERSION=octopus
+BUILD_TAGS=ceph_preview


I do not think we should set ceph_preview as default. Make sure that APIs from go-ceph are stable and part of the latest release.

Madhu-1 · 2022-01-06T08:26:38Z

@humblec you have requested changes on this PR. PTAL

internal/rbd/migration.go

internal/rbd/rbd_util.go

github-actions · 2022-01-06T08:46:43Z

/retest ci/centos/k8s-e2e-external-storage/1.21

github-actions · 2022-01-06T08:46:44Z

@Rakshith-R "ci/centos/k8s-e2e-external-storage/1.21" test failed. Logs are available at location for debugging

This commit removes `rv.Connect(cr)` since the rbdVolume should have an active connection in this stage of the function call. `rv.getCloneDepth(ctx)` will work after a connect to the cluster. Signed-off-by: Rakshith R <rar@redhat.com>

Pull request has been modified.

With introduction of go-ceph rbd admin task api, credentials are no longer required to be passed as cli cmd is not invoked. Signed-off-by: Rakshith R <rar@redhat.com>

This commit removes rbdVol.getTrashPath() function since it is no longer being used due to introduction of go-ceph rbd admin task api for deletion. Signed-off-by: Rakshith R <rar@redhat.com>

This function returns new go-ceph TaskAdmin to add tasks on rbd volumes. Signed-off-by: Rakshith R <rar@redhat.com>

Signed-off-by: Rakshith R <rar@redhat.com>

Pull request has been modified.

Rakshith-R · 2022-01-06T10:48:09Z

few lines creeped in to migration.go during rebase, fixed that.
PTAL

humblec · 2022-01-06T11:33:56Z

few lines creeped in to migration.go during rebase, fixed that. PTAL

Thanks @Rakshith-R , I will let @nixpanic to confirm once, as I heard previous changes were tested too.

mergify bot added the component/rbd Issues related to RBD label Nov 11, 2021

Rakshith-R force-pushed the flatten-task-api branch 4 times, most recently from eae9bbf to 161fd06 Compare November 16, 2021 10:57

Rakshith-R mentioned this pull request Nov 16, 2021

build: use BUILD_TAGS instead of CEPH_VERSION #2626

Closed

Rakshith-R force-pushed the flatten-task-api branch from 161fd06 to aeffec9 Compare November 16, 2021 12:07

Rakshith-R marked this pull request as ready for review November 17, 2021 04:17

Rakshith-R requested review from a team November 17, 2021 04:18

humblec suggested changes Nov 17, 2021

View reviewed changes

Rakshith-R requested review from a team and humblec November 17, 2021 10:42

Madhu-1 reviewed Nov 18, 2021

View reviewed changes

Rakshith-R force-pushed the flatten-task-api branch from aeffec9 to 826f0be Compare November 18, 2021 06:37

Rakshith-R requested review from humblec and removed request for humblec November 18, 2021 06:38

Rakshith-R force-pushed the flatten-task-api branch 2 times, most recently from 5c01d0b to 2ead05f Compare November 18, 2021 06:41

Madhu-1 previously approved these changes Nov 18, 2021

View reviewed changes

nixpanic requested changes Nov 18, 2021

View reviewed changes

nixpanic added the dependency/go-ceph depends on go-ceph functionality label Nov 18, 2021

mergify bot added ci/skip/e2e skip running e2e CI jobs component/build Issues and PRs related to compiling Ceph-CSI component/testing Additional test cases or CI work labels Nov 23, 2021

Madhu-1 previously approved these changes Jan 6, 2022

View reviewed changes

humblec suggested changes Jan 6, 2022

View reviewed changes

internal/rbd/migration.go Outdated Show resolved Hide resolved

internal/rbd/migration.go Outdated Show resolved Hide resolved

humblec reviewed Jan 6, 2022

View reviewed changes

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

internal/rbd/rbd_util.go Show resolved Hide resolved

nixpanic force-pushed the flatten-task-api branch from 1f93b1e to c9f93de Compare January 6, 2022 09:07

nixpanic requested review from Madhu-1 and humblec January 6, 2022 09:07

Madhu-1 previously approved these changes Jan 6, 2022

View reviewed changes

humblec previously approved these changes Jan 6, 2022

View reviewed changes

Rakshith-R added 4 commits January 6, 2022 15:30

rbd: remove redundant util.Credentials arg from flattenRbdImage()

0983650

With introduction of go-ceph rbd admin task api, credentials are no longer required to be passed as cli cmd is not invoked. Signed-off-by: Rakshith R <rar@redhat.com>

rbd: remove redundant rbdVol.getTrashPath() function

27bfa79

This commit removes rbdVol.getTrashPath() function since it is no longer being used due to introduction of go-ceph rbd admin task api for deletion. Signed-off-by: Rakshith R <rar@redhat.com>

rbd: add function (cc *ClusterConnection) GetTaskAdmin()

f471ccf

This function returns new go-ceph TaskAdmin to add tasks on rbd volumes. Signed-off-by: Rakshith R <rar@redhat.com>

cleanup: use %q instead of %s for logging

3da4afd

Signed-off-by: Rakshith R <rar@redhat.com>

Rakshith-R force-pushed the flatten-task-api branch from c9f93de to 3da4afd Compare January 6, 2022 10:01

Madhu-1 approved these changes Jan 6, 2022

View reviewed changes

Rakshith-R requested review from nixpanic and humblec January 6, 2022 11:31

nixpanic approved these changes Jan 6, 2022

View reviewed changes

humblec approved these changes Jan 6, 2022

View reviewed changes

mergify bot merged commit 384ab42 into ceph:devel Jan 6, 2022

Madhu-1 mentioned this pull request Feb 22, 2022

add atomic reference tracker #2893

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rbd: use go-ceph rbd admin task api for 'trash remove' & 'flatten' #2633

rbd: use go-ceph rbd admin task api for 'trash remove' & 'flatten' #2633

Rakshith-R commented Nov 11, 2021 •

edited

Loading

Rakshith-R commented Nov 17, 2021

Rakshith-R commented Nov 17, 2021

humblec Nov 17, 2021

Rakshith-R Nov 17, 2021

Madhu-1 Nov 18, 2021

nixpanic Nov 18, 2021

Rakshith-R Dec 15, 2021

Madhu-1 left a comment

Madhu-1 Nov 18, 2021

Rakshith-R commented Nov 18, 2021

Madhu-1 commented Nov 18, 2021

Rakshith-R commented Nov 18, 2021

nixpanic left a comment

nixpanic Nov 18, 2021

Madhu-1 commented Jan 6, 2022

github-actions bot commented Jan 6, 2022

github-actions bot commented Jan 6, 2022

Rakshith-R commented Jan 6, 2022

humblec commented Jan 6, 2022

	BUILD_TAGS=ceph_preview
	# ceph_preview tag is required to access go-ceph APIs which are under preview.
	BUILD_TAGS=ceph_preview

rbd: use go-ceph rbd admin task api for 'trash remove' & 'flatten' #2633

rbd: use go-ceph rbd admin task api for 'trash remove' & 'flatten' #2633

Conversation

Rakshith-R commented Nov 11, 2021 • edited Loading

Rakshith-R commented Nov 17, 2021

Rakshith-R commented Nov 17, 2021

humblec Nov 17, 2021

Choose a reason for hiding this comment

Rakshith-R Nov 17, 2021

Choose a reason for hiding this comment

Madhu-1 Nov 18, 2021

Choose a reason for hiding this comment

nixpanic Nov 18, 2021

Choose a reason for hiding this comment

Rakshith-R Dec 15, 2021

Choose a reason for hiding this comment

Madhu-1 left a comment

Choose a reason for hiding this comment

Madhu-1 Nov 18, 2021

Choose a reason for hiding this comment

Rakshith-R commented Nov 18, 2021

Madhu-1 commented Nov 18, 2021

Rakshith-R commented Nov 18, 2021

nixpanic left a comment

Choose a reason for hiding this comment

nixpanic Nov 18, 2021

Choose a reason for hiding this comment

Madhu-1 commented Jan 6, 2022

github-actions bot commented Jan 6, 2022

github-actions bot commented Jan 6, 2022

Rakshith-R commented Jan 6, 2022

humblec commented Jan 6, 2022

Rakshith-R commented Nov 11, 2021 •

edited

Loading