rbd: add EncryptionLoad2 implementing rbd_encryption_load2 #1061
Conversation
|
I'm looking to add a 2nd test, one that actually tests the use of multiple encryption specs, but I haven't found any good examples yet. In the meantime, while it's still draft, I figured I'd get a test run out of what I have already. |
42c3ed6 to
3aa05cc
Compare
|
Additional test added. |
3aa05cc to
f197e5a
Compare
There was a problem hiding this comment.
I've made few suggestions, all along the same pattern, to some of the length comparison assert statements. Somehow I feel like comparing the length of data read with the length of prior(fixed) data to make more sense than comparing it with the length of read buffer for img.ReadAt(). WDYT?
f197e5a to
bef69a2
Compare
|
There must be something else wrong with this change as other old tests are now panicking a bunch, I filed #1063 just to see if this was something pre-existing but those all passed without so much as a poke needed (earlier this week some of the dnf mirrors were being flaky). Unfortunately, I thought this would be a quick and easy thing to add, but I guess not. |
ansiwen
left a comment
There was a problem hiding this comment.
Here my first review. I will make a seconds pass though. Not sure there is more.
bef69a2 to
db6ae02
Compare
c098ac8 to
2ca6350
Compare
e8c883e to
bdac7dc
Compare
|
@Mergifyio rebase |
✅ Branch has been successfully rebased |
bdac7dc to
5383f06
Compare
anoopcs9
left a comment
There was a problem hiding this comment.
lgtm, see below for a small suggestion.
Fix a couple of comments to make them a bit more accurate. They were probably true at one point in the development of the feature but now referred to functions that don't exist. Signed-off-by: John Mulligan <jmulligan@redhat.com>
Restructure the existing rbd encryption load test function so that it makes use of defer and uses subtests to divide things up more clearly. I needed to make this change in preparation for adding a binding for rbd_encryption_load2 and a similar test - but I couldn't make sense of the existing test in it's more monolithic form. Signed-off-by: John Mulligan <jmulligan@redhat.com>
Add a new Image method EncryptionLoad2 implementing rbd_encryption_load2. This method adds the ability to have different encryption schemes across parent images. Signed-off-by: John Mulligan <jmulligan@redhat.com> Fixes: ceph#1059
Add EncryptionOptionsLUKS type for opening, but not formatting, existing LUKS images generically. Signed-off-by: John Mulligan <jmulligan@redhat.com>
Increase the test timeout from 10m (Go default) to 15m. Adding the three new tests for load 2 and the new generic LUKS type (mainly the longer entryption load2 test) push us over the 10m mark by just about 1 minute :-\ Signed-off-by: John Mulligan <jmulligan@redhat.com>
5383f06 to
6f67ffb
Compare
Pull request has been modified.
ansiwen
left a comment
There was a problem hiding this comment.
LG(ish)TM, give you a chance to see my comments before merging,. Remove the do-not-merge when you read it and want to merge it as is.
Add a new Image method EncryptionLoad2 implementing rbd_encryption_load2.
This method adds the ability to have different encryption schemes across
parent images.
Fixes: #1059
Checklist
//go:build ceph_previewmake api-updateto record new APIsNew or infrequent contributors may want to review the go-ceph Developer's Guide including the section on how we track API Status and the API Stability Plan.
The go-ceph project uses mergify. View the mergify command guide for information on how to interact with mergify. Add a comment with
@Mergifyiorebaseto rebase your PR when github indicates that the PR is out of date with the base branch.