-
Notifications
You must be signed in to change notification settings - Fork 545
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.
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
rbd: add feature check to see if GroupSnapGetInfo is available #4898
Conversation
var opts C.rbd_image_options_t | ||
//nolint:gocritic // ignore result of rbd_image_options functions | ||
C.rbd_image_options_create(&opts) | ||
C.rbd_image_options_destroy(opts) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nixpanic do we need to create/destory the image? can you please add a comment about a need for it as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is just to call an RBD function that won't cause any harm.
RBD-image-options can be created and destroyed without accessing the Ceph cluster. You can see the options as an in-memory list of features. The options are used when an image is created, and features of the image can be set in the options object and passed to rbd.CreateImage()
and similar functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, the comment is just above it:
// make sure librbd.so.x is loaded, might not (yet) be the case
// if no rbd functions are called
@Mergifyio queue |
✅ The pull request has been merged automaticallyThe pull request has been merged automatically at 3802dd2 |
The go-ceph rbd package provides the GroupSnapGetInfo function, but it may return ErrUnsupported when called. Returning this error after advertising the support for VolumeGroupSnapshot seems ugly. In order to advertise support for VolumeGroupSnapshot, SupportsGroupSnapGetInfo() can be used, which detects the required C function of librbd. Signed-off-by: Niels de Vos <ndevos@ibm.com>
fe9d932
to
a3f717a
Compare
/test ci/centos/k8s-e2e-external-storage/1.29 |
/test ci/centos/mini-e2e-helm/k8s-1.29 |
/test ci/centos/mini-e2e/k8s-1.29 |
/test ci/centos/upgrade-tests-cephfs |
/test ci/centos/k8s-e2e-external-storage/1.31 |
/test ci/centos/upgrade-tests-rbd |
/test ci/centos/k8s-e2e-external-storage/1.30 |
/test ci/centos/mini-e2e-helm/k8s-1.31 |
/test ci/centos/mini-e2e-helm/k8s-1.30 |
/test ci/centos/mini-e2e/k8s-1.31 |
/test ci/centos/mini-e2e/k8s-1.30 |
The go-ceph rbd package provides the GroupSnapGetInfo function, but it may return ErrUnsupported when called. Returning this error after advertising the support for VolumeGroupSnapshot seems ugly.
In order to advertise support for VolumeGroupSnapshot, SupportsGroupSnapGetInfo() can be used, which detects the required C function of librbd.
Related issues
To be consumed in #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 unrelatedfailure (please report the failure too!)