-
Notifications
You must be signed in to change notification settings - Fork 536
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-nbd: add tests with different accessModes and volumeModes #2601
Conversation
c9b83f3
to
67ac3d4
Compare
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.
just checking, is it possible to have a common helper function to avoid code duplication?
67ac3d4
to
bf02d25
Compare
Yes! At some point we need to keep efforts on writing more generic code for our e2e, it would be a big refactor of e2e/xyz.go. [which would be a different topic IMO] Coming to this PR, it's adding tests just like any other test. You might have noticed I have already generalized the code into e2e/deployment.go as much as possible. If you feel like I missed something and we can move some more pieces into a common functions feel free to point it Thanks! |
bf02d25
to
165f94e
Compare
165f94e
to
b92ed1a
Compare
b92ed1a
to
e079f9a
Compare
@humblec Thanks for the review, the comments are cosmic. AFAIK there is no strict naming convention that we are binding to in e2e, I would love to see if anyone else has objections on the naming. |
|
/retest ci/centos/mini-e2e/k8s-1.21 |
e079f9a
to
51ba4ad
Compare
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.
LGTM. need one correction in e2e.
9104187
to
6daf4f0
Compare
@Madhu-1 fixed now, please take a look |
6daf4f0
to
0fc7d08
Compare
Pull request has been modified.
/retest ci/centos/mini-e2e-helm/k8s-1.20
|
@Mergifyio rebase |
0fc7d08
to
546c9da
Compare
✅ Branch has been successfully rebased |
@Mergifyio refresh |
✅ Pull request refreshed |
546c9da
to
cfebab9
Compare
added DNM to get #2669 merged which is second in queue. |
removed DNM as #2669 is merged |
Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Fixed the order of function parameters Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
…me device Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
Add tests for RWX and ROX accessModes for Block and FileSystem Mode PVCs. Fixes: ceph#2262 Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
cfebab9
to
abe6fa7
Compare
Describe what this PR does
Added tests using different accessModes and volumeModes for rbd-nbd
Added helper functions to use deployment kind objects
Note: This PR doesn't consider the new accessMode ReadWriteOncePod [RWOP], this will be a future item to cover tests for both krbd and nbd.
Related issues
Fixes: #2262