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: add additional space for encrypted volumes #4582

Draft
wants to merge 2 commits into
base: devel
Choose a base branch
from
Draft

rbd: add additional space for encrypted volumes #4582

wants to merge 2 commits into from
+322 −4

Conversation

iPraveenParihar
Copy link
Contributor

@iPraveenParihar iPraveenParihar commented Apr 24, 2024 •
edited
Loading

Describe what this PR does

rbd: add additional space for encrypted volumes

issue: when a block-mode pvc is created with encryption enabled
there is some space reserved for the encryption metadata.
Which doesn't allows users to write extact amount of data that
they have requested for.

solution: create pvc with extra space needed for the encryption
metadata.

The extra space is added during the CreateVolume and ExpandVolume
operations. And while returning the response remove the extra space
so the client/user gets the requested size reported.
New metadata is added to images which were created with extra space
for encryption header - rbd.csi.ceph.com/luks2HeaderSize

Test result:

  • Clone from existing PVC
{
  "name": "csi-vol-ca2d5f5c-0da5-4107-a4e0-d0b28eba94cc",
  "id": "b5f4e403bfe5e",
  "size": 1090519040,   // 1GiB + 16MiB
  "objects": 260,
  "order": 22,
  "object_size": 4194304,
  "snapshot_count": 0,
  "block_name_prefix": "rbd_data.b5f4e403bfe5e",
  "format": 2,
  "features": [
    "layering",
    "operations"
  ],
  "op_features": [
    "clone-child"
  ],
  "flags": [],
  "create_timestamp": "Thu Jun 13 07:07:12 2024",
  "access_timestamp": "Thu Jun 13 07:07:12 2024",
  "modify_timestamp": "Thu Jun 13 07:07:12 2024",
  "parent": {
    "pool": "replicapool",
    "pool_namespace": "",
    "image": "csi-vol-ca2d5f5c-0da5-4107-a4e0-d0b28eba94cc-temp",
    "id": "b5f4e6e333281",
    "snapshot": "a8a490a0-3237-48d6-bcaa-859f0aad6ea1",
    "trash": false,
    "overlap": 1073741824    // parent size - 1GiB
  }
}
  • Clone from new PVC
{
  "name": "csi-vol-5b4e7457-eb17-454a-a734-394cf27e7e84",
  "id": "b5f4e2b8beacc",
  "size": 1090519040,     // 1GiB + 16MiB
  "objects": 260,
  "order": 22,
  "object_size": 4194304,
  "snapshot_count": 0,
  "block_name_prefix": "rbd_data.b5f4e2b8beacc",
  "format": 2,
  "features": [
    "layering",
    "operations"
  ],
  "op_features": [
    "clone-child"
  ],
  "flags": [],
  "create_timestamp": "Thu Jun 13 07:11:16 2024",
  "access_timestamp": "Thu Jun 13 07:11:16 2024",
  "modify_timestamp": "Thu Jun 13 07:11:16 2024",
  "parent": {
    "pool": "replicapool",
    "pool_namespace": "",
    "image": "csi-vol-5b4e7457-eb17-454a-a734-394cf27e7e84-temp",
    "id": "b5f4e6517ad39",
    "snapshot": "479b9d82-7044-4597-8316-f7e67bc793f5",
    "trash": false,
    "overlap": 1090519040    // parent size - 1GiB + 16MiB
  }
}
  • Resize new PVC from 1GiB to 2GiB
{
  "name": "csi-vol-00d3a9ff-a76b-4f38-a463-5686478b4347",
  "id": "b5f4e4f96b175",
  "size": 2164260864,    //  2GiB + 16MiB
  "objects": 516,
  "order": 22,
  "object_size": 4194304,
  "snapshot_count": 1,
  "block_name_prefix": "rbd_data.b5f4e4f96b175",
  "format": 2,
  "features": [
    "layering",
    "operations"
  ],
  "op_features": [
    "clone-parent",
    "snap-trash"
  ],
  "flags": [],
  "create_timestamp": "Thu Jun 13 06:47:56 2024",
  "access_timestamp": "Thu Jun 13 06:47:56 2024",
  "modify_timestamp": "Thu Jun 13 06:47:56 2024"
}

  • In above resize scenario after mount to pod. It has exactly 2GiB
$ k exec rbd-pod-e -- blockdev --getsize64 /dev/xvda
2147483648

!
Checklist:

  • Commit Message Formatting: Commit titles and messages follow guidelines in the developer guide.
  • Reviewed the developer guide on Submitting a Pull Request
  • Pending release notes updated with breaking and/or notable changes for the next major release.
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Integration tests have been added, if necessary.
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!)

@iPraveenParihar
Copy link
Contributor Author

minikube testing,

Created a 1GiB block-mode pvc, and then expanded to 2GiB

[pm@dhcp53-176 ceph-csi]$ k get pvc
NAME            STATUS   VOLUME                                     CAPACITY   ACCESS MODES   STORAGECLASS   AGE
raw-block-pvc   Bound    pvc-5d6e808b-5f9c-474b-94a7-ce0f20725048   1Gi        RWO            rbd-sc         7m33s
[pm@dhcp53-176 ceph-csi]$ k exec rook-direct-mount-6b8f99f786-q5qfm -- rbd info replicapool/csi-vol-7c440f1f-35f0-4708-84b5-1d4055c0cc32 --format json | jq
{
  "name": "csi-vol-6feb9874-b24d-4bb7-9b4b-f51732e79708",
  "id": "bceddf10f522a",
  "size": 1090519040,   // --> 1GiB + 16MiB
  "objects": 260,
  "order": 22,
  "object_size": 4194304,
  "snapshot_count": 0,
  "block_name_prefix": "rbd_data.bceddf10f522a",
  "format": 2,
  "features": [
    "layering"
  ],
  "op_features": [],
  "flags": [],
  "create_timestamp": "Wed Apr 24 07:01:43 2024",
  "access_timestamp": "Wed Apr 24 07:01:43 2024",
  "modify_timestamp": "Wed Apr 24 07:01:43 2024"
}



[pm@dhcp53-176 examples]$ k get pvc
NAME            STATUS   VOLUME                                     CAPACITY   ACCESS MODES   STORAGECLASS   AGE
raw-block-pvc   Bound    pvc-5d6e808b-5f9c-474b-94a7-ce0f20725048   2Gi        RWO            rbd-sc         15m
[pm@dhcp53-176 examples]$ k exec rook-direct-mount-6b8f99f786-q5qfm -- rbd info replicapool/csi-vol-7c440f1f-35f0-4708-84b5-1d4055c0cc32 --format json | jq
{
  "name": "csi-vol-7c440f1f-35f0-4708-84b5-1d4055c0cc32",
  "id": "bcedd5fa1dd89",
  "size": 2164260864,   // --> 2GiB + 16MiB
  "objects": 516,
  "order": 22,
  "object_size": 4194304,
  "snapshot_count": 0,
  "block_name_prefix": "rbd_data.bcedd5fa1dd89",
  "format": 2,
  "features": [
    "layering"
  ],
  "op_features": [],
  "flags": [],
  "create_timestamp": "Wed Apr 24 07:23:07 2024",
  "access_timestamp": "Wed Apr 24 07:23:07 2024",
  "modify_timestamp": "Wed Apr 24 07:23:07 2024"
}

Madhu-1
Madhu-1 reviewed Apr 24, 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.

Add E2E checks new size.

@mergify mergify bot added the component/rbd Issues related to RBD label Apr 24, 2024
@iPraveenParihar
Copy link
Contributor Author

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

@iPraveenParihar
Copy link
Contributor Author

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

@iPraveenParihar iPraveenParihar force-pushed the rbd/configure-encrpted-vol-size branch 4 times, most recently from 29e4f91 to 4f0f3fc Compare April 25, 2024 11:12
@iPraveenParihar
Copy link
Contributor Author

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

@nixpanic
Copy link
Member

@Mergifyio rebase

This causes a run of the GitHub CI jobs. Logs for ci/centos/mini-e2e-helm/k8s-1.28 show that e2e passed earlier.

@nixpanic nixpanic force-pushed the rbd/configure-encrpted-vol-size branch from 4f0f3fc to 69aeda7 Compare April 25, 2024 15:41
@mergify Mergify
Copy link
Contributor

mergify bot commented Apr 25, 2024

rebase

✅ Branch has been successfully rebased

@iPraveenParihar iPraveenParihar force-pushed the rbd/configure-encrpted-vol-size branch 2 times, most recently from 227bcf0 to a4eabb3 Compare April 29, 2024 06:38
@iPraveenParihar
Copy link
Contributor Author

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

@iPraveenParihar iPraveenParihar force-pushed the rbd/configure-encrpted-vol-size branch 5 times, most recently from 7b79d41 to ca95618 Compare April 29, 2024 16:54
@iPraveenParihar
Copy link
Contributor Author

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

@iPraveenParihar
Copy link
Contributor Author

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

@iPraveenParihar
Copy link
Contributor Author

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

@karthik-us, is this the correct command to run rbd tests?
its not working for me with above command.

@iPraveenParihar
Copy link
Contributor Author

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

@iPraveenParihar iPraveenParihar force-pushed the rbd/configure-encrpted-vol-size branch from ca95618 to 72252c7 Compare May 2, 2024 04:56
@iPraveenParihar
Copy link
Contributor Author

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

@iPraveenParihar iPraveenParihar force-pushed the rbd/configure-encrpted-vol-size branch from 72252c7 to 9d52790 Compare May 6, 2024 06:01
@iPraveenParihar @nixpanic
rbd: add additional space for encrypted volumes
a5af5a1 
issue: when a block-mode pvc is created with encryption enabled
there is some space reserved for the encryption metadata.
Which doesn't allows users to write extact amount of data that
they have requested for.

solution: create pvc with extra space needed for the encryption
metadata.

The extra space is added during the CreateVolume and ExpandVolume
operations. And while returning the response remove the extra space
so the client/user gets the requested size reported.

Signed-off-by: Praveen M <[email protected]>
@iPraveenParihar @nixpanic
e2e: test to verify the encrypted image size
8438381 
This commit adds testcase for the create/resize/clone/restore operations
for a RBD block PVC and validate imageSize and deviceSize.

Signed-off-by: Praveen M <[email protected]>
@mergify Mergify
Copy link
Contributor

mergify bot commented Aug 13, 2024

rebase

✅ Branch has been successfully rebased

@mergify Mergify
Copy link
Contributor

mergify bot commented Aug 13, 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 nixpanic force-pushed the rbd/configure-encrpted-vol-size branch from 63c72c0 to 8438381 Compare August 13, 2024 13:12
@mergify mergify bot added the ok-to-test Label to trigger E2E tests label Aug 13, 2024
@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-cephfs

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-rbd

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

@ceph-csi-bot
Copy link
Collaborator

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

@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-helm/k8s-1.28

@ceph-csi-bot
Copy link
Collaborator

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

@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-helm/k8s-1.30

@ceph-csi-bot
Copy link
Collaborator

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

@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 ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Aug 13, 2024
@iPraveenParihar
Copy link
Contributor Author

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

@iPraveenParihar
Copy link
Contributor Author

failed test cases are not same in all CI runs, but all fails at creating snapshot.

failed test - create ROX PVC clone and mount it to multiple pods

  E0819 11:44:29.654152       1 snapshot_controller_base.go:357] could not sync content "snapcontent-3c487efa-b938-4aa5-9a30-cee647e7539a": failed to take snapshot of the volume 0001-0024-61c6f69b-3550-4c89-8b36-3e1f45ca7749-0000000000000004-5c6aefd5-9700-4bd9-a0b6-8599443432ac: "rpc error: code = Aborted desc = an operation with the given Volume ID snapshot-3c487efa-b938-4aa5-9a30-cee647e7539a already exists"
  I0819 11:48:45.654891       1 snapshot_controller.go:308] createSnapshotWrapper: Creating snapshot for content snapcontent-3c487efa-b938-4aa5-9a30-cee647e7539a through the plugin ...
  I0819 11:48:45.710540       1 snapshot_controller.go:342] createSnapshotWrapper: CreateSnapshot for content snapcontent-3c487efa-b938-4aa5-9a30-cee647e7539a returned error: rpc error: code = Aborted desc = an operation with the given Volume ID snapshot-3c487efa-b938-4aa5-9a30-cee647e7539a already exists
  E0819 11:48:45.710571       1 snapshot_controller.go:142] checkandUpdateContentStatus [snapcontent-3c487efa-b938-4aa5-9a30-cee647e7539a]: error occurred failed to take snapshot of the volume 0001-0024-61c6f69b-3550-4c89-8b36-3e1f45ca7749-0000000000000004-5c6aefd5-9700-4bd9-a0b6-8599443432ac: "rpc error: code = Aborted desc = an operation with the given Volume ID snapshot-3c487efa-b938-4aa5-9a30-cee647e7539a already exists"
  E0819 11:48:45.710584       1 snapshot_controller_base.go:357] could not sync content "snapcontent-3c487efa-b938-4aa5-9a30-cee647e7539a": failed to take snapshot of the volume 0001-0024-61c6f69b-3550-4c89-8b36-3e1f45ca7749-0000000000000004-5c6aefd5-9700-4bd9-a0b6-8599443432ac: "rpc error: code = Aborted desc = an operation with the given Volume ID snapshot-3c487efa-b938-4aa5-9a30-cee647e7539a already exists"

will debug and update.

@github-actions GitHub Actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in two weeks if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Sep 19, 2024
@Madhu-1
Copy link
Collaborator

Madhu-1 commented Sep 20, 2024

@iPraveenParihar can you please check this one? this is already approved PR and getting stalled.

@github-actions github-actions bot removed the stale label Sep 20, 2024
@mergify Mergify
Copy link
Contributor

mergify bot commented Sep 20, 2024

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

@iPraveenParihar
Copy link
Contributor Author

@iPraveenParihar can you please check this one? this is already approved PR and getting stalled.

There seems to be inconsistent CI test failures. Not sure, if its because of this PR change or another.
I'll take care of this.

@iPraveenParihar iPraveenParihar marked this pull request as draft October 11, 2024 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nixpanic nixpanic nixpanic approved these changes

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

@Rakshith-R Rakshith-R Awaiting requested review from Rakshith-R

Assignees

@iPraveenParihar iPraveenParihar

Labels
component/rbd Issues related to RBD
Projects
None yet
Milestone
No milestone
Development

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