ceph: add support for encrypted metadata pvc #5977

leseb · 2020-08-05T11:36:36Z

Description of your changes:

Now the metadata pvc will also be encrypted if the storageClassDeviceSet
has the "encrypted" flag turned on.

Signed-off-by: Sébastien Han seb@redhat.com

Checklist:

// known CI issue
[skip ci]

travisn

holding for testing

travisn · 2020-08-07T19:50:29Z

pkg/operator/ceph/cluster/osd/osd.go

-			errMsg := fmt.Sprintf("failed to validate storageClassDeviceSet %q. min required ceph version to support encryption is %q", volume.Name, cephVolumeRawEncryptionModeMinCephVersion.String())
+		if osdProps.encrypted &&
+			!c.clusterInfo.CephVersion.IsAtLeast(cephVolumeRawEncryptionModeMinNautilusCephVersion) &&
+			!c.clusterInfo.CephVersion.IsAtLeast(cephVolumeRawEncryptionModeMinOctopusCephVersion) {


This doesn't look like it will allow for any nautilus version. The octopus check will override the nautilus check. How about a helper function to check the version and then unit test it?

travisn · 2020-08-07T19:55:15Z

pkg/operator/ceph/cluster/osd/spec.go

+
+func (c *Cluster) getPVCEncryptionOpenInitContainerActivate(osdProps osdProperties) []v1.Container {
+	containers := []v1.Container{}
+	containers = append(containers, c.generateEncryptionOpenBlockContainer(osdProps.resources, blockEncryptionOpenInitContainer, osdProps.pvc.ClaimName))


nit: initialize the container with all its properties to a local var, then append it to the containers slice. This way, you wouldn't have to reference the slice with containers[0] or containers[1] below. It's more readable IMO that way.

mergify · 2020-08-11T11:59:13Z

This pull request has merge conflicts that must be resolved before it can be merged. @leseb please rebase it. https://rook.io/docs/rook/master/development-flow.html#updating-your-fork

travisn

just a small suggestion

pkg/operator/ceph/cluster/osd/config.go

The c-v encryption is part of 15.2.5 on Octopus. Signed-off-by: Sébastien Han <seb@redhat.com>

satoru-takeuchi · 2020-08-12T19:46:11Z

tests/scripts/localPathPV.sh

-lsblk
+# dump data of the disk to check whether it is an osd or not
+sudo dd if="$test_scratch_device" of=data bs=4k count=1
+sudo cat data


It prints the raw binary data to stdout. It's difficult to read and it would break the terminal's setting when running in the local environment. How about using the binary dump tools like hexdump or xxd? In addition, I consider 4K is too long and printing "bluestore" signature and fsid is enough.

Agreed, this was mostly for quick debug, this commit is going away :)

Now the metadata pvc will also be encrypted if the storageClassDeviceSet has the "encrypted" flag turned on. Signed-off-by: Sébastien Han <seb@redhat.com>

Small refactor to use the common function helper callCephVolume() to build ceph-volume arguments. Signed-off-by: Sébastien Han <seb@redhat.com>

If the device is empty let's not use an empty space as an argument for the c-v call. Signed-off-by: Sébastien Han <seb@redhat.com>

So that the operator can print it. Signed-off-by: Sébastien Han <seb@redhat.com>

satoru-takeuchi · 2020-08-13T00:14:09Z

@leseb Could you tell me whether dropping the following change is intentional or not?

c23840b#diff-d1ca60bbccae22948f1847a6b9747cd5L445-R447

In my local environment, MultiClusterTestSuite fails as before. Here is the log of failed osd prepare pod

2020-08-13 00:17:55.061801 I | cephosd: discovering hardware
2020-08-13 00:17:55.061809 D | exec: Running command: lsblk /mnt/set1-data-0-qg7c6 --bytes --nodeps --pairs --paths --output SIZE,ROTA,RO,TYPE,PKNAME,NAME,KNAME
2020-08-13 00:17:55.066795 D | exec: Running command: sgdisk --print /mnt/set1-data-0-qg7c6
2020-08-13 00:17:55.070501 D | exec: Running command: udevadm info --query=property /dev/sdb
2020-08-13 00:17:55.089585 I | cephosd: creating and starting the osds
2020-08-13 00:17:55.094524 D | cephosd: No Drive Groups configured.
2020-08-13 00:17:55.094573 D | cephosd: desiredDevices are [{Name:/mnt/set1-data-0-qg7c6 OSDsPerDevice:1 MetadataDevice: DatabaseSizeMB:0 DeviceClass: IsFilter:false IsDevicePathFilter:false}]
2020-08-13 00:17:55.094581 D | cephosd: context.Devices are [0xc0003f0240]
2020-08-13 00:17:55.095366 D | exec: Running command: stdbuf -oL ceph-volume --log-path /tmp/ceph-log raw list /mnt/set1-data-0-qg7c6 --format json
2020-08-13 00:17:55.795997 D | cephosd: stderr: unable to read label for /mnt/set1-data-0-qg7c6: (2) No such file or directory
{}
2020-08-13 00:17:55.796059 I | cephosd: skipping device "/mnt/set1-data-0-qg7c6": failed to detect if there is already an osd. failed to unmarshal ceph-volume raw list results: invalid character 's' looking for beginning of value.
2020-08-13 00:17:55.796120 I | cephosd: configuring osd devices: {"Entries":{}}
2020-08-13 00:17:55.796124 I | cephosd: no new devices to configure. returning devices already configured with ceph-volume.
2020-08-13 00:17:55.796129 D | exec: Running command: pvdisplay -C -o lvpath --noheadings /mnt/set1-data-0-qg7c6
2020-08-13 00:17:55.932887 W | cephosd: failed to retrieve logical volume path for "/mnt/set1-data-0-qg7c6". exit status 5
2020-08-13 00:17:55.932927 D | exec: Running command: stdbuf -oL ceph-volume --log-path /tmp/ceph-log lvm list  --format json
2020-08-13 00:17:56.514691 D | cephosd: {}
2020-08-13 00:17:56.514743 I | cephosd: 0 ceph-volume lvm osd devices configured on this node
2020-08-13 00:17:56.514758 D | exec: Running command: stdbuf -oL ceph-volume --log-path /tmp/ceph-log raw list /mnt/set1-data-0-qg7c6 --format json
2020-08-13 00:17:57.013571 D | cephosd: stderr: unable to read label for /mnt/set1-data-0-qg7c6: (2) No such file or directory
{}
2020-08-13 00:17:57.013618 I | cephosd: failed to get device already provisioned by ceph-volume raw. failed to unmarshal ceph-volume raw list results: invalid character 's' looking for beginning of value
2020-08-13 00:17:57.013624 W | cephosd: skipping OSD configuration as no devices matched the storage settings for this node "set1-data-0-qg7c6"

Although this PR was merged without CI, I guess the integration test still fail in upstream/master too.

satoru-takeuchi · 2020-08-13T01:06:23Z

Hm, my integration test in the local environment also failed even with the following change.

c23840b#diff-d1ca60bbccae22948f1847a6b9747cd5L445-R447

The log of OSD prepare pod.

2020-08-13 01:00:05.276775 I | cephosd: discovering hardware
2020-08-13 01:00:05.276786 D | exec: Running command: lsblk /mnt/set1-data-0-ks77x --bytes --nodeps --pairs --paths --output SIZE,ROTA,RO,TYPE,PKNAME,NAME,KNAME
2020-08-13 01:00:05.280903 D | exec: Running command: sgdisk --print /mnt/set1-data-0-ks77x
2020-08-13 01:00:05.284097 D | exec: Running command: udevadm info --query=property /dev/sdb
2020-08-13 01:00:05.288536 I | cephosd: creating and starting the osds
2020-08-13 01:00:05.296175 D | cephosd: No Drive Groups configured.
2020-08-13 01:00:05.296234 D | cephosd: desiredDevices are [{Name:/mnt/set1-data-0-ks77x OSDsPerDevice:1 MetadataDevice: DatabaseSizeMB:0 DeviceClass: IsFilter:false IsDevicePathFilter:false}]
2020-08-13 01:00:05.296244 D | cephosd: context.Devices are [0xc0002017a0]
2020-08-13 01:00:05.296988 D | exec: Running command: stdbuf -oL ceph-volume --log-path /tmp/ceph-log raw list /mnt/set1-data-0-ks77x --format json
2020-08-13 01:00:05.861963 D | cephosd: stderr: unable to read label for /mnt/set1-data-0-ks77x: (2) No such file or directory
{}
2020-08-13 01:00:05.862134 I | cephosd: skipping device "/mnt/set1-data-0-ks77x": failed to detect if there is already an osd. failed to unmarshal ceph-volume raw list results: invalid character 's' looking for beginning of value.
2020-08-13 01:00:05.862310 I | cephosd: configuring osd devices: {"Entries":{}}
2020-08-13 01:00:05.862335 I | cephosd: no new devices to configure. returning devices already configured with ceph-volume.
2020-08-13 01:00:05.862343 D | exec: Running command: pvdisplay -C -o lvpath --noheadings /mnt/set1-data-0-ks77x
2020-08-13 01:00:05.998355 W | cephosd: failed to retrieve logical volume path for "/mnt/set1-data-0-ks77x". exit status 5
2020-08-13 01:00:05.998453 D | exec: Running command: stdbuf -oL ceph-volume --log-path /tmp/ceph-log lvm list  --format json
2020-08-13 01:00:06.607446 D | cephosd: {}
2020-08-13 01:00:06.607488 I | cephosd: 0 ceph-volume lvm osd devices configured on this node
2020-08-13 01:00:06.607506 D | exec: Running command: stdbuf -oL ceph-volume --log-path /tmp/ceph-log raw list /mnt/set1-data-0-ks77x --format json
2020-08-13 01:00:07.096787 D | cephosd: stderr: unable to read label for /mnt/set1-data-0-ks77x: (2) No such file or directory
{}
2020-08-13 01:00:07.096841 I | cephosd: failed to get device already provisioned by ceph-volume raw. failed to unmarshal ceph-volume raw list results: invalid character 's' looking for beginning of value
2020-08-13 01:00:07.096846 W | cephosd: skipping OSD configuration as no devices matched the storage settings for this node "set1-data-0-ks77x"

leseb · 2020-08-13T07:38:11Z

Hm, my integration test in the local environment also failed even with the following change.

c23840b#diff-d1ca60bbccae22948f1847a6b9747cd5L445-R447

The log of OSD prepare pod.

2020-08-13 01:00:05.276775 I | cephosd: discovering hardware
2020-08-13 01:00:05.276786 D | exec: Running command: lsblk /mnt/set1-data-0-ks77x --bytes --nodeps --pairs --paths --output SIZE,ROTA,RO,TYPE,PKNAME,NAME,KNAME
2020-08-13 01:00:05.280903 D | exec: Running command: sgdisk --print /mnt/set1-data-0-ks77x
2020-08-13 01:00:05.284097 D | exec: Running command: udevadm info --query=property /dev/sdb
2020-08-13 01:00:05.288536 I | cephosd: creating and starting the osds
2020-08-13 01:00:05.296175 D | cephosd: No Drive Groups configured.
2020-08-13 01:00:05.296234 D | cephosd: desiredDevices are [{Name:/mnt/set1-data-0-ks77x OSDsPerDevice:1 MetadataDevice: DatabaseSizeMB:0 DeviceClass: IsFilter:false IsDevicePathFilter:false}]
2020-08-13 01:00:05.296244 D | cephosd: context.Devices are [0xc0002017a0]
2020-08-13 01:00:05.296988 D | exec: Running command: stdbuf -oL ceph-volume --log-path /tmp/ceph-log raw list /mnt/set1-data-0-ks77x --format json
2020-08-13 01:00:05.861963 D | cephosd: stderr: unable to read label for /mnt/set1-data-0-ks77x: (2) No such file or directory
{}
2020-08-13 01:00:05.862134 I | cephosd: skipping device "/mnt/set1-data-0-ks77x": failed to detect if there is already an osd. failed to unmarshal ceph-volume raw list results: invalid character 's' looking for beginning of value.
2020-08-13 01:00:05.862310 I | cephosd: configuring osd devices: {"Entries":{}}
2020-08-13 01:00:05.862335 I | cephosd: no new devices to configure. returning devices already configured with ceph-volume.
2020-08-13 01:00:05.862343 D | exec: Running command: pvdisplay -C -o lvpath --noheadings /mnt/set1-data-0-ks77x
2020-08-13 01:00:05.998355 W | cephosd: failed to retrieve logical volume path for "/mnt/set1-data-0-ks77x". exit status 5
2020-08-13 01:00:05.998453 D | exec: Running command: stdbuf -oL ceph-volume --log-path /tmp/ceph-log lvm list  --format json
2020-08-13 01:00:06.607446 D | cephosd: {}
2020-08-13 01:00:06.607488 I | cephosd: 0 ceph-volume lvm osd devices configured on this node
2020-08-13 01:00:06.607506 D | exec: Running command: stdbuf -oL ceph-volume --log-path /tmp/ceph-log raw list /mnt/set1-data-0-ks77x --format json
2020-08-13 01:00:07.096787 D | cephosd: stderr: unable to read label for /mnt/set1-data-0-ks77x: (2) No such file or directory
{}
2020-08-13 01:00:07.096841 I | cephosd: failed to get device already provisioned by ceph-volume raw. failed to unmarshal ceph-volume raw list results: invalid character 's' looking for beginning of value
2020-08-13 01:00:07.096846 W | cephosd: skipping OSD configuration as no devices matched the storage settings for this node "set1-data-0-ks77x"

I have the fix, PR soon :)

leseb · 2020-08-13T08:00:56Z

@satoru-takeuchi #6059

Similar to rook#5977 but for wal devices. So if a cluster is deployed on PVC and wal devices, those will be encrypted as well. Signed-off-by: Sébastien Han <seb@redhat.com>

leseb · 2020-08-13T08:47:39Z

@satoru-takeuchi and yes dropping this c23840b#diff-d1ca60bbccae22948f1847a6b9747cd5L445-R447 was intentional as I don't think it was fixing anything, NAME always exists... Still digging!

Similar to rook#5977 but for wal devices. So if a cluster is deployed on PVC and wal devices, those will be encrypted as well. Signed-off-by: Sébastien Han <seb@redhat.com>

ceph: add support for encrypted metadata pvc (bp #5977)

Similar to #5977 but for wal devices. So if a cluster is deployed on PVC and wal devices, those will be encrypted as well. Signed-off-by: Sébastien Han <seb@redhat.com> (cherry picked from commit 4f9435d)

leseb added ceph main ceph tag backport-release-1.4 labels Aug 5, 2020

leseb requested a review from travisn August 5, 2020 11:36

travisn requested changes Aug 5, 2020

View reviewed changes

leseb mentioned this pull request Aug 5, 2020

ceph: add encryption support for osd pvc #5873

Merged

11 tasks

leseb force-pushed the encryption-metadata-pvc branch 5 times, most recently from dce34f7 to 7414463 Compare August 6, 2020 20:55

travisn requested changes Aug 7, 2020

View reviewed changes

leseb force-pushed the encryption-metadata-pvc branch 2 times, most recently from eaa13f1 to d110df0 Compare August 10, 2020 14:57

leseb force-pushed the encryption-metadata-pvc branch 2 times, most recently from 27ff4d9 to cffbe7e Compare August 11, 2020 16:05

travisn approved these changes Aug 11, 2020

View reviewed changes

pkg/operator/ceph/cluster/osd/config.go Outdated Show resolved Hide resolved

ceph: add octopus validation for pvc encryption

7d6974b

The c-v encryption is part of 15.2.5 on Octopus. Signed-off-by: Sébastien Han <seb@redhat.com>

leseb mentioned this pull request Aug 12, 2020

c-v raw fails to list existing OSD (blocks CI) #5996

Closed

leseb force-pushed the encryption-metadata-pvc branch 2 times, most recently from c23840b to 964ffe7 Compare August 12, 2020 14:07

satoru-takeuchi reviewed Aug 12, 2020

View reviewed changes

leseb force-pushed the encryption-metadata-pvc branch from 964ffe7 to bdf3d49 Compare August 12, 2020 19:57

satoru-takeuchi approved these changes Aug 12, 2020

View reviewed changes

leseb added 4 commits August 12, 2020 22:24

ceph: add support for encrypted metadata pvc

a91c96c

Now the metadata pvc will also be encrypted if the storageClassDeviceSet has the "encrypted" flag turned on. Signed-off-by: Sébastien Han <seb@redhat.com>

ceph: use callCephVolume() to list osds

9738c2b

Small refactor to use the common function helper callCephVolume() to build ceph-volume arguments. Signed-off-by: Sébastien Han <seb@redhat.com>

ceph: do not list on empty device

f75a6e9

If the device is empty let's not use an empty space as an argument for the c-v call. Signed-off-by: Sébastien Han <seb@redhat.com>

ceph: return cv debug logs

dd2166b

So that the operator can print it. Signed-off-by: Sébastien Han <seb@redhat.com>

leseb force-pushed the encryption-metadata-pvc branch from bdf3d49 to dd2166b Compare August 12, 2020 20:24

travisn merged commit 52a6071 into rook:master Aug 12, 2020

tenzen-y mentioned this pull request Aug 13, 2020

WIP: ci: check ceph-volume.log #6036

Closed

10 tasks

leseb deleted the encryption-metadata-pvc branch August 13, 2020 06:47

leseb mentioned this pull request Aug 13, 2020

ceph: add support for encrypted metadata pvc (bp #5977) #6058

Merged

leseb mentioned this pull request Aug 13, 2020

ceph: add support for wal encrypted device on pvc #6062

Merged

10 tasks

travisn added a commit that referenced this pull request Aug 13, 2020

Merge pull request #6058 from leseb/bkp-5977

0bd5d47

ceph: add support for encrypted metadata pvc (bp #5977)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ceph: add support for encrypted metadata pvc #5977

ceph: add support for encrypted metadata pvc #5977

leseb commented Aug 5, 2020 •

edited by travisn

travisn left a comment

travisn Aug 7, 2020 •

edited

travisn Aug 7, 2020

mergify bot commented Aug 11, 2020

travisn left a comment

satoru-takeuchi Aug 12, 2020

leseb Aug 12, 2020

satoru-takeuchi commented Aug 13, 2020 •

edited

satoru-takeuchi commented Aug 13, 2020

leseb commented Aug 13, 2020

leseb commented Aug 13, 2020

leseb commented Aug 13, 2020

ceph: add support for encrypted metadata pvc #5977

ceph: add support for encrypted metadata pvc #5977

Conversation

leseb commented Aug 5, 2020 • edited by travisn

travisn left a comment

Choose a reason for hiding this comment

travisn Aug 7, 2020 • edited

Choose a reason for hiding this comment

travisn Aug 7, 2020

Choose a reason for hiding this comment

mergify bot commented Aug 11, 2020

travisn left a comment

Choose a reason for hiding this comment

satoru-takeuchi Aug 12, 2020

Choose a reason for hiding this comment

leseb Aug 12, 2020

Choose a reason for hiding this comment

satoru-takeuchi commented Aug 13, 2020 • edited

satoru-takeuchi commented Aug 13, 2020

leseb commented Aug 13, 2020

leseb commented Aug 13, 2020

leseb commented Aug 13, 2020

leseb commented Aug 5, 2020 •

edited by travisn

travisn Aug 7, 2020 •

edited

satoru-takeuchi commented Aug 13, 2020 •

edited