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: base implementation for CSI-Addons VolumeGroup #4707

Merged
merged 7 commits into from
Jul 11, 2024
Merged

rbd: base implementation for CSI-Addons VolumeGroup #4707

merged 7 commits into from
Jul 11, 2024

Conversation

nixpanic
Copy link
Member

@nixpanic nixpanic commented Jul 9, 2024 •
edited
Loading

Describe what this PR does

This is the basis of the implementation for the CSI-Addons VolumeGroup specification. The service and capabilities are added, but the actual implementation of the VolumeGroup creation/deletion/modifying is not included here, and will be sent as a follow-up PR.

Is there anything that requires special attention

By providing this base for the CSI-Addons VolumeGroup support, reviewing should be simpler than including everything a a huge PR.

Updates: #4502

Show available bot commands

These commands are normally not required, but in case of issues, leave any of
the following bot commands in an otherwise empty comment in this PR:

  • /retest ci/centos/<job-name>: retest the <job-name> after unrelated
    failure (please report the failure too!)

@nixpanic nixpanic added the dependency/csi-addons Related to a CSI-Addons issue or enhancement label Jul 9, 2024
@mergify mergify bot added the component/rbd Issues related to RBD label Jul 9, 2024
@nixpanic nixpanic force-pushed the csi-addons/rbd/volumegroup/base branch 6 times, most recently from 5302d54 to 6ff98ef Compare July 10, 2024 08:38
@nixpanic nixpanic marked this pull request as ready for review July 10, 2024 08:44
@nixpanic nixpanic requested review from Madhu-1 and a team July 10, 2024 08:44
Madhu-1
Madhu-1 reviewed Jul 10, 2024
View reviewed changes
Copy link
Collaborator

@Madhu-1 Madhu-1 left a comment

Choose a reason for hiding this comment

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

some nits, LGTM

internal/csi-addons/rbd/volumegroup.go Outdated Show resolved Hide resolved
internal/csi-addons/rbd/volumegroup.go Outdated Show resolved Hide resolved
internal/rbd/manager.go Outdated Show resolved Hide resolved
@mergify Mergify
Copy link
Contributor

mergify bot commented Jul 10, 2024

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

nixpanic and others added 5 commits July 11, 2024 09:35
@nixpanic
rbd: move internal/rbd_types -> internal/rbd/types
97cf3a5 
The rbd_types package was initially created with references to the rbd
package. And the rbd package references the rbd_types package. Having
rbd/types was not possible due to recursive imports. After cleaning up
the rbd_types package, it can be renamed to rbd/types.

Signed-off-by: Niels de Vos <ndevos@ibm.com>
@Madhu-1 @nixpanic
csiaddons: register volumegroup controller
ec59da3 
Register the volumegroup controller as part
of rbd controller server to serve the volume
group RPC spec.

Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
@nixpanic
rbd: add a ToCSI() function to the Volume interface
8786b60 
A VolumeGroup CSI-Addons object contains a list of CSI Volumes. A
ToCSI() function makes creating such a list much simpler.

Signed-off-by: Niels de Vos <ndevos@ibm.com>
@nixpanic
rbd: add the VolumeGroup type
17e0cd8 
Signed-off-by: Niels de Vos <ndevos@ibm.com>
@nixpanic
rbd: add Manager interface for using Volumes and VolumeGroups
81072a8 
Signed-off-by: Niels de Vos <ndevos@ibm.com>
@nixpanic nixpanic force-pushed the csi-addons/rbd/volumegroup/base branch from 6ff98ef to 3579d42 Compare July 11, 2024 07:36
@nixpanic nixpanic requested review from Madhu-1 and a team July 11, 2024 07:36
@nixpanic
csiaddons: initial implementation of CSI-Addons VolumeGroup
6299c1e 
Signed-off-by: Niels de Vos <ndevos@ibm.com>
@nixpanic
rebase: update github.com/csi-addons/spec to latest version
cf71bc0 
Update VolumeGroup API with CreateVolumeGroupRequest that contains an
optional list of VolumeIDs.

Signed-off-by: Niels de Vos <ndevos@ibm.com>
@nixpanic nixpanic force-pushed the csi-addons/rbd/volumegroup/base branch from 3579d42 to cf71bc0 Compare July 11, 2024 07:44
@nixpanic nixpanic requested a review from a team July 11, 2024 08:00
yati1998
yati1998 approved these changes Jul 11, 2024
View reviewed changes
Copy link
Contributor

@yati1998 yati1998 left a comment

Choose a reason for hiding this comment

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

LGTM

@Madhu-1
Copy link
Collaborator

Madhu-1 commented Jul 11, 2024

@Mergifyio queue

@mergify Mergify
Copy link
Contributor

mergify bot commented Jul 11, 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.

@mergify mergify bot added the ok-to-test Label to trigger E2E tests label Jul 11, 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/upgrade-tests-cephfs

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

@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.27

@ceph-csi-bot
Copy link
Collaborator

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

@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.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/k8s-e2e-external-storage/1.28

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

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

ci/centos/mini-e2e-helm/k8s-1.28 paniced in NFS testing logs:

  [PANICKED] in [AfterEach] - /go/pkg/mod/golang.org/toolchain@v0.0.1-go1.22.0.linux-amd64/src/runtime/panic.go:261 @ 07/11/24 09:23:33.79

@nixpanic
Copy link
Member Author

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

@nixpanic
Copy link
Member Author

@Mergifyio requeue

@mergify Mergify
Copy link
Contributor

mergify bot commented Jul 11, 2024

requeue

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

@mergify mergify bot merged commit e71a95f into ceph:devel Jul 11, 2024
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iPraveenParihar iPraveenParihar iPraveenParihar approved these changes

@yati1998 yati1998 yati1998 approved these changes

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

Assignees
No one assigned
Labels
component/rbd Issues related to RBD dependency/csi-addons Related to a CSI-Addons issue or enhancement
Projects
None yet
Milestone
No milestone
Development

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