rbd: add additional space for encrypted volumes #4582

iPraveenParihar · 2024-04-24T08:53:16Z

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. GetLuksHeaderSize() function returns the luks2
encryption metadata(header size).

Related issues

Fixes: #issue_number

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 · 2024-04-24T09:04:28Z

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

Add E2E checks new size.

iPraveenParihar · 2024-04-24T14:47:28Z

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

iPraveenParihar · 2024-04-25T06:43:31Z

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

iPraveenParihar · 2024-04-25T11:12:55Z

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

nixpanic · 2024-04-25T15:41:30Z

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

mergify · 2024-04-25T15:41:51Z

rebase

✅ Branch has been successfully rebased

iPraveenParihar · 2024-04-29T07:13:38Z

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

iPraveenParihar · 2024-04-29T16:58:35Z

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

iPraveenParihar · 2024-04-29T16:58:55Z

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

iPraveenParihar · 2024-04-29T17:00:30Z

/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 · 2024-04-30T03:01:39Z

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

iPraveenParihar · 2024-05-02T04:56:27Z

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

iPraveenParihar · 2024-05-06T06:02:42Z

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

iPraveenParihar · 2024-05-06T11:34:12Z

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

Madhu-1 · 2024-05-06T12:10:04Z

e2e/rbd.go

+
+				//nolint:goconst // The string "1Gi" is used multiple times in rbd.go, so it's not a const value.
+				pvcSize := "1Gi"
+				imageSize := 1*helpers.GiB + util.GetLuksHeaderSize()


check if helper is having any menthods to parse string and convert it to in that avoid bugs or else we need to remember that we need to change size in two places.

Madhu-1 · 2024-05-06T12:10:58Z

e2e/rbd.go

+				}
+			})
+
+			By("resize a encrypted block pvc and verify the image size", func() {


add the resize to above test case, lets not create the PVC and pod again

Madhu-1 · 2024-05-06T12:11:37Z

e2e/rbd_helper.go

+	}
+
+	if imgInfo.Size != imageSize {
+		return fmt.Errorf("size %d does not match expected %d", imgInfo.Size, imageSize)


return pvc name /image name which helps in debugging

Madhu-1 · 2024-05-06T12:12:42Z

internal/util/cryptsetup.go

+	luks2KeySlotsSize          = 32 << 8  // 8192 KiB
+)
+
+func GetLuksHeaderSize() int64 {


return uint64 from here, we dont need to convert int64 to uint64 in multiple places

iPraveenParihar · 2024-05-06T14:54:19Z

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

Madhu-1 · 2024-05-06T16:05:31Z

e2e/rbd.go

+					resizeImageSize uint64
+				)
+				pvcSize := "1Gi"
+				if s, err := helpers.RoundUpToB(resource.MustParse(pvcSize)); err != nil {


it should be err==nil not the other way isn't it?

😀yea, missed it
Thanks!

Madhu-1 · 2024-05-06T16:06:51Z

internal/util/cryptsetup.go

+func GetLuksHeaderSize() uint64 {
+	size := uint64((((2 * luks2MetadataSize) + luks2KeySlotsSize) * helpers.KiB))
+
+	return size
+}


Add unit testing for this helper?

please add unit test for it.

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. `GetLuksHeaderSize()` function returns the luks2 encryption metadata(header size). Signed-off-by: Praveen M <m.praveen@ibm.com>

Signed-off-by: Praveen M <m.praveen@ibm.com>

iPraveenParihar · 2024-05-07T04:46:55Z

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

Rakshith-R · 2024-05-07T09:25:45Z

e2e/rbd.go

@@ -1984,6 +1986,115 @@ var _ = Describe("RBD", func() {
 				}
 			})

+			By("create a encrypted block pvc and verify the image size", func() {


Check snapshot process and size of created snapshot too in the same flow since we had a problem with restore?

Madhu-1 · 2024-05-07T11:32:20Z

e2e/rbd.go

+				if err != nil {
+					framework.Failf("failed to load application: %v", err)
+				}
+				app.Labels = map[string]string{"app": "rbd-pod-block-encrypted"}


labelKey:="app" labelVal:= "rbd-pod-block-encrypted" app.Labels = map[string]string{labelKey:labelVal} opt := metav1.ListOptions{ LabelSelector: fmt.Sprintf("%s=%s",labelKey,labelVal) }

using like above is less error prone, if we check it in one place we don't need to change it in other place

Madhu-1 · 2024-05-07T11:32:51Z

e2e/rbd.go

+				err = deletePVCAndApp("", f, pvc, app)
+				if err != nil {
+					framework.Failf("failed to delete pvc and app: %v", err)
+				}


please add E2E for PVC-PVC clone and snapshot restore as well

Madhu-1 · 2024-05-07T11:33:27Z

e2e/rbd_helper.go

@@ -1067,6 +1067,7 @@ type imageInfo struct {
 	StripeUnit  int    `json:"stripe_unit"`
 	StripeCount int    `json:"stripe_count"`
 	ObjectSize  int    `json:"object_size"`
+	Size        int64  `json:"size"`


can we use uint64 here?

Madhu-1 · 2024-05-07T11:34:08Z

internal/util/cryptsetup.go

+func GetLuksHeaderSize() uint64 {
+	size := uint64((((2 * luks2MetadataSize) + luks2KeySlotsSize) * helpers.KiB))
+
+	return size
+}


please add unit test for it.

Madhu-1 reviewed Apr 24, 2024

View reviewed changes

iPraveenParihar force-pushed the rbd/configure-encrpted-vol-size branch from b7586df to 850dd3a Compare April 24, 2024 09:13

mergify bot added the component/rbd Issues related to RBD label Apr 24, 2024

iPraveenParihar force-pushed the rbd/configure-encrpted-vol-size branch from 850dd3a to 357a727 Compare April 25, 2024 06:35

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

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

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 force-pushed the rbd/configure-encrpted-vol-size branch 5 times, most recently from 7b79d41 to ca95618 Compare April 29, 2024 16:54

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

iPraveenParihar force-pushed the rbd/configure-encrpted-vol-size branch from 72252c7 to 9d52790 Compare May 6, 2024 06:01

iPraveenParihar marked this pull request as ready for review May 6, 2024 07:16

iPraveenParihar requested a review from Madhu-1 May 6, 2024 07:25

Madhu-1 requested changes May 6, 2024

View reviewed changes

Madhu-1 requested review from a team May 6, 2024 12:13

iPraveenParihar force-pushed the rbd/configure-encrpted-vol-size branch from 9d52790 to b8c9e50 Compare May 6, 2024 14:53

Madhu-1 reviewed May 6, 2024

View reviewed changes

iPraveenParihar force-pushed the rbd/configure-encrpted-vol-size branch 2 times, most recently from 5479252 to 624153c Compare May 6, 2024 18:42

e2e: test to verify the encrypted image size

d756a9d

Signed-off-by: Praveen M <m.praveen@ibm.com>

iPraveenParihar force-pushed the rbd/configure-encrpted-vol-size branch from 624153c to d756a9d Compare May 7, 2024 04:46

iPraveenParihar requested a review from Madhu-1 May 7, 2024 07:11

Rakshith-R reviewed May 7, 2024

View reviewed changes

Madhu-1 reviewed May 7, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rbd: add additional space for encrypted volumes #4582

rbd: add additional space for encrypted volumes #4582

iPraveenParihar commented Apr 24, 2024

iPraveenParihar commented Apr 24, 2024

Madhu-1 left a comment

iPraveenParihar commented Apr 24, 2024

iPraveenParihar commented Apr 25, 2024

iPraveenParihar commented Apr 25, 2024

nixpanic commented Apr 25, 2024

mergify bot commented Apr 25, 2024

iPraveenParihar commented Apr 29, 2024

iPraveenParihar commented Apr 29, 2024

iPraveenParihar commented Apr 29, 2024

iPraveenParihar commented Apr 29, 2024

iPraveenParihar commented Apr 30, 2024

iPraveenParihar commented May 2, 2024

iPraveenParihar commented May 6, 2024

iPraveenParihar commented May 6, 2024

Madhu-1 May 6, 2024

Madhu-1 May 6, 2024

Madhu-1 May 6, 2024

Madhu-1 May 6, 2024

iPraveenParihar commented May 6, 2024

Madhu-1 May 6, 2024

iPraveenParihar May 6, 2024

Madhu-1 May 6, 2024

Madhu-1 May 7, 2024

iPraveenParihar commented May 7, 2024

Rakshith-R May 7, 2024

Madhu-1 May 7, 2024

Madhu-1 May 7, 2024

Madhu-1 May 7, 2024

Madhu-1 May 7, 2024

rbd: add additional space for encrypted volumes #4582

Are you sure you want to change the base?

rbd: add additional space for encrypted volumes #4582

Conversation

iPraveenParihar commented Apr 24, 2024

Describe what this PR does

Related issues

iPraveenParihar commented Apr 24, 2024

Madhu-1 left a comment

Choose a reason for hiding this comment

iPraveenParihar commented Apr 24, 2024

iPraveenParihar commented Apr 25, 2024

iPraveenParihar commented Apr 25, 2024

nixpanic commented Apr 25, 2024

mergify bot commented Apr 25, 2024

✅ Branch has been successfully rebased

iPraveenParihar commented Apr 29, 2024

iPraveenParihar commented Apr 29, 2024

iPraveenParihar commented Apr 29, 2024

iPraveenParihar commented Apr 29, 2024

iPraveenParihar commented Apr 30, 2024

iPraveenParihar commented May 2, 2024

iPraveenParihar commented May 6, 2024

iPraveenParihar commented May 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iPraveenParihar commented May 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iPraveenParihar commented May 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment