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.

Sign up for GitHub

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

Jump to bottom

rbd: implement CSI-Addons ControllerGetVolumeGroup operation #4733

Merged
merged 1 commit into from
Jul 26, 2024
Merged

rbd: implement CSI-Addons ControllerGetVolumeGroup operation #4733

merged 1 commit into from
Jul 26, 2024

Conversation

nixpanic
Copy link
Member

@nixpanic nixpanic commented Jul 25, 2024 •
edited
Loading

With the ControllerGetVolumeGroup operation the caller can verify that a
VolumeGroup exists, and validate the volumes that are part of it.

I0725 12:44:52.504216       1 utils.go:235] ID: 37 GRPC call: /volumegroup.Controller/ControllerGetVolumeGroup
I0725 12:44:52.504255       1 utils.go:236] ID: 37 GRPC request: {"secrets":"***stripped***","volume_group_id":"0001-0011-openshift-storage-0000000000000002-2aaad87d-2e93-4647-9ac0-3660a7912de6"}
I0725 12:44:52.505101       1 omap.go:221] ID: 37 got omap values: (pool="ocs-storagecluster-cephblockpool", namespace="", name="csi.volume.group.2aaad87d-2e93-4647-9ac0-3660a7912de6"): map[0001-0011-openshift-storage-0000000000000002-0b5e14d8-2237-4262-a1d4-725a2942bf3a: 0001-0011-openshift-storage-0000000000000002-a47a50fc-3bb1-40d9-bf58-9a522bfd112c: csi.groupname:csi-vol-group-2aaad87d-2e93-4647-9ac0-3660a7912de6 csi.volname:the-group]
I0725 12:44:52.505709       1 omap.go:89] ID: 37 got omap values: (pool="ocs-storagecluster-cephblockpool", namespace="", name="csi.volume.0b5e14d8-2237-4262-a1d4-725a2942bf3a"): map[csi.imageid:20e5a173b8bd csi.imagename:csi-vol-0b5e14d8-2237-4262-a1d4-725a2942bf3a csi.volname:pvc-479506a3-b6d7-446c-88a1-d9899789aaff csi.volume.owner:default]
I0725 12:44:52.526239       1 omap.go:89] ID: 37 got omap values: (pool="ocs-storagecluster-cephblockpool", namespace="", name="csi.volume.a47a50fc-3bb1-40d9-bf58-9a522bfd112c"): map[csi.imageid:20e5705d82c9 csi.imagename:csi-vol-a47a50fc-3bb1-40d9-bf58-9a522bfd112c csi.volname:pvc-b3331ae5-d1b8-453d-9d83-a5044f70c56e csi.volume.owner:default]
I0725 12:44:52.543456       1 volume_group.go:149] ID: 37 GetVolumeGroup(0001-0011-openshift-storage-0000000000000002-2aaad87d-2e93-4647-9ac0-3660a7912de6) returns {id:0001-0011-openshift-storage-0000000000000002-2aaad87d-2e93-4647-9ac0-3660a7912de6 name:csi-vol-group-2aaad87d-2e93-4647-9ac0-3660a7912de6 clusterID:openshift-storage credentials:0xc000837380 conn:<nil> ioctx:<nil> monitors:172.30.137.172:3300,172.30.174.236:3300,172.30.128.147:3300 pool:ocs-storagecluster-cephblockpool namespace: journal:0xc000adb510 volumes:[0xc0007dcfc8 0xc0007dd208] volumesToFree:[0xc0007dcfc8 0xc0007dd208]}
I0725 12:44:52.543519       1 volume_group.go:307] ID: 37 destroyed volume group instance with id "0001-0011-openshift-storage-0000000000000002-2aaad87d-2e93-4647-9ac0-3660a7912de6"
I0725 12:44:52.543604       1 utils.go:242] ID: 37 GRPC response: {"volume_group":{"volume_group_id":"0001-0011-openshift-storage-0000000000000002-2aaad87d-2e93-4647-9ac0-3660a7912de6","volumes":[{"capacity_bytes":8589934592,"volume_context":{"imageName":"csi-vol-0b5e14d8-2237-4262-a1d4-725a2942bf3a","journalPool":"ocs-storagecluster-cephblockpool","pool":"ocs-storagecluster-cephblockpool"},"volume_id":"0001-0011-openshift-storage-0000000000000002-0b5e14d8-2237-4262-a1d4-725a2942bf3a"},{"capacity_bytes":8589934592,"volume_context":{"imageName":"csi-vol-a47a50fc-3bb1-40d9-bf58-9a522bfd112c","journalPool":"ocs-storagecluster-cephblockpool","pool":"ocs-storagecluster-cephblockpool"},"volume_id":"0001-0011-openshift-storage-0000000000000002-a47a50fc-3bb1-40d9-bf58-9a522bfd112c"}]}}

@nixpanic nixpanic force-pushed the csi-addons/rbd/volumegroup/get branch from 9634980 to a09f93d Compare July 25, 2024 13:29
@nixpanic nixpanic changed the title csi addons/rbd/volumegroup/get rbd: implement CSI-Addons ControllerGetVolumeGroup operation Jul 25, 2024
@nixpanic nixpanic added the ci/skip/e2e skip running e2e CI jobs label Jul 25, 2024
@mergify mergify bot added the component/rbd Issues related to RBD label Jul 25, 2024
@nixpanic nixpanic requested a review from a team July 25, 2024 14:12
Madhu-1
Madhu-1 requested changes Jul 26, 2024
View reviewed changes
internal/csi-addons/rbd/volumegroup.go
vg, err := mgr.GetVolumeGroupByID(ctx, req.GetVolumeGroupId())
if err != nil {
return nil, status.Errorf(
codes.NotFound,
Copy link
Collaborator

Choose a reason for hiding this comment

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

@nixpanic what about the case where we have invalid secret or missing secret etc? do we need to consider it as not found?

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, the only error that is documented in the spec is NOT_FOUND. The error description will contain more details on why the volume group could not be found.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Lets revisit this later. 👍🏻

@Madhu-1
Copy link
Collaborator

Madhu-1 commented Jul 26, 2024

@Mergifyio queue

@mergify Mergify
Copy link
Contributor

mergify bot commented Jul 26, 2024 •
edited
Loading

queue

🛑 The pull request has been removed from the queue default

The queue conditions cannot be satisfied due to failing checks.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

@nixpanic
rbd: implement CSI-Addons ControllerGetVolumeGroup operation
94bef9c 
With the ControllerGetVolumeGroup operation the caller can verify that a
VolumeGroup exists, and validate the volumes that are part of it.

Signed-off-by: Niels de Vos <ndevos@ibm.com>
@nixpanic nixpanic force-pushed the csi-addons/rbd/volumegroup/get branch from a09f93d to 94bef9c Compare July 26, 2024 07:18
@mergify mergify bot added the ok-to-test Label to trigger E2E tests label Jul 26, 2024
@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.29

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.29

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.29

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-cephfs

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.28

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-rbd

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.28

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.30

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.28

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.30

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.30

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Jul 26, 2024
@nixpanic
Copy link
Member Author

/retest ci/centos/mini-e2e/k8s-1.27

@nixpanic
Copy link
Member Author

@Mergifyio requeue

ci/centos/mini-e2e/k8s-1.27 failed to clone the git repository

@mergify mergify bot merged commit aa88b4c into ceph:devel Jul 26, 2024
40 checks passed
@mergify Mergify
Copy link
Contributor

mergify bot commented Jul 26, 2024

requeue

☑️ This pull request is already queued

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Rakshith-R Rakshith-R Rakshith-R approved these changes

@iPraveenParihar iPraveenParihar iPraveenParihar approved these changes

@Madhu-1 Madhu-1 Madhu-1 approved these changes

Assignees
No one assigned
Labels
ci/skip/e2e skip running e2e CI jobs component/rbd Issues related to RBD
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@nixpanic @Madhu-1 @ceph-csi-bot @iPraveenParihar @Rakshith-R