Support raw block volume

[slaperche-scality]

Component:

operator, salt

Why this is needed:

There are some specialized applications (e.g.: database, SDS system, …) that require direct access to a block device because, for example, the file system layer introduces unneeded overhead.

To address this use case, we should be able to manage Volume that will backed by PersistentVolume with volumeMode set to Block.

What should be done:

Currently, the volumeMode is hardcoded to Filesystem in the operator, this need to change.
In the same vein, the formatting step is always executed by the Salt state, this also need to be changed.

Implementation proposal:

We need a way to know if we want to format the volume or not.
This could be either at the Volume level or at the StorageClass level.

I suggest that we control that at the StorageClass level, because:

• that's already the place where we configure the formatting, so it's consistent
• if we put it at the Volume level, we will have to sanity check against illogical combination (a StorageClass with ext4 on a non-formatted Volume)
• by setting it as the StorageClass level it could apply to both SparseLoopDevice and RawBlockDevice.

The idea is to accept a special value "none" for the parameter fsType in the StorageClass.
When fsType is set to "none" then:

• PersistentVolume are created with volumeMode set to Block
• Salt state skip the formatting.

We could have made the field optional instead of using a special value, but I think that it's better to be explicit here.

Test plan:

If we can add support for SparseLoopDevice (may be tricky because we can't store the ID in the FS in that case), then we can add tests with SparseLoopDevice: create a volume and check that the PV has the right mode and the backing storage is not formatted.

Otherwise manuel testing I guess (since CI doesn't attach "real" volume to the VM).

[slaperche-scality]

Comments are welcome! @NicolasT @gdemonet @benoit-a

Or I can make a design section in the doc, like what I've done for volume v1 (will be volume v1.1 this time 😛 )

[gdemonet]

Comments are welcome! @NicolasT @gdemonet @benoit-a

I don't have any strong opinion - the implementation proposed looks acceptable to me, especially since it's a feature supported by PersistentVolumes in the first place.

Or I can make a design section in the doc, like what I've done for volume v1 (will be volume v1.1 this time )

On this point, I think we need to adjust the Volume design document to match the other "Architecture documents" we have in the repo. We found that having a set of "KEP-style" docs was too heavy for our needs, and simply editing the content of such docs seems enough for us (rather than creating new docs to invalidate/supersede the initial design).

This format would need some standardization however, to make it simpler when external contributors want to suggest a new design or modifications to an existing one.

[slaperche-scality]

Hum, in the end I'm no longer convinced that putting it into the StorageClass is the right approach…

Without formatting, it seems impossible to have a constant device path for a SparseLoopDevice (for RawBlockDevice, we can fallback on the UUID of the device itself, but that won't work for a sparse file…).

If this parameter becomes RawBlockDevice-only, then it's better to add it into the RawBlockDevice spec rather than using the StorageClass.

So I think I'll move back this new settings inside the Volume object itself (it will be optional with a default value that will keep the current behavior (use formatting), thus it shouldn't break the compat)

---

Another issue: if one enter /dev/sda as devicePath, when creating the PV I dont' want to use such path and should use a by-uuid/… path instead
=> how do I get the UUID of a given volume?

Until now if wasn't an issue because we were using the Volume UUID as FS UUID and everything worked fine: the operator can compute the path from the Volume alone.
But that's no longer the case…

I can add an extra Salt call during the Volume provisionning (using disk.blkid), but then where do I store this info between the moment I get it and the moment I create the PV?
The CR is the only persistence I have accross operator restart/reconciliation iteration.
I can:

• patch devicePath, but this may surprise the user…
• add a condition: could help monitoring the progress of the deployment, but conditions are more about "problem" than "progression"
• something else?

---

Edit: Hum, after some thoughts I think I'll go for a hashmap (volume, storage uuid).
It will avoid packing more stuff into the CR status, and will be persistent accross reconciliation iteration.

If we're unlucky enough to crash/restart between the time we fetch the UUID and the creation of the PV, we only have to call disk.blkid again (it's call that doesn't modify anything, no problem if we call it multiple times) and resume the creation: it should be unlikely enough to be acceptable.

If this works well for UUID, I may use the same approach for disk.dump and revert my hackish implementation that pack stuff as string into the Salt Job Handle…

[gdemonet]

We could add more status fields to the CR for the Operator to use as state (incl. block device UUID, size...), no?
For SparseLoopDevice _Volume_s, if using the "raw block" option, a consumer could read the device path from status.devicePath.

[slaperche-scality]

We could add more status fields to the CR for the Operator to use as state (incl. block device UUID, size...), no?

In the first implementation of Volume (which never got merged, because I rewrote it according to review's feedback), we had a lot of "computed" state inside the status and I think that's not something we want.

For SparseLoopDevice _Volume_s, if using the "raw block" option, a consumer could read the device path from status.devicePath.

But you would need to refresh this info at some point, because after a reboot your sparse loop device may be loop3 instead of loop1. And then you probably need to update the PV as well, otherwise kubelet will mount the wrong device…
Sounds like a lot of work for a feature we're not supposed to use in prod.

[NicolasT]

• I think choosing between block or file should be done at the Volume level, not in the StorageClass. Indeed, we specify formatting options in the StorageClass, which may be a slight stretch, but it kinda fits. Remember a StorageClass ought to define the 'class' of storage (performance characteristics, local/attached,...), not how this storage is consumed.

• For the device identifiers: we currently use FS UUIDs for File Volumes which works great: it allows us to uniquely identify a single file volume (in /dev/disk/by-uuid/), ensure we (the automation) formatted it properly,... We want to keep the same benefits for block volumes. In general, a plain block device (unpartitioned, unformatted) doesn't have any associated UUID.

In a rawBlockDevice Volume that'll become a block PersistentVolume, we are passed the path to a device. This can either be a raw device (/dev/sda, /dev/mapper/vg-data,...) or a partition (/dev/sda1).

In case of a raw device, we could enforce this device to have a related World Wide Name, i.e., an entry pointing to it in /dev/disk/by-id/wwn-*, and use this as the device path in the generated PersistentVolume.
For LVM LVs, we can resolve the given 'device' to the entry in /dev/disk/by-id/dm-uuid-LVM-*, relying on LVMs UUID mechanism.
Finally, if we're passed a 'regular' partition, we could decide to not support MBR/BIOS disks (heck, this is 2020...) and only support GPT partitioning, in which case the partition has a GPT UUID, and the volume can use /dev/disk/by-partuuid/... to reference it (or have our automation change its GPT UUID?)

In the GPT case, we could even enforce the use of specific GPT partition types.

Now, when using sparseLoopDevice, we can

• Create the sparse file
• Create a GPT partition on it
• Create a single GPT partition in it, setting the Volume UUID as its partition UUID
• losetup --find --partscan ...
• Use /dev/disk/by-partuuid in the PV

[slaperche-scality]

Thanks for the feedback.

Let's put the selection between File/Block at the Volume level then (which make more sense indeed, same as volumeMode is at the PV level, there is some symmetry here 🙂 )

---

About the reliable device path, IIUC instead of using disk.blkid, I can write a custom Salt module that would resolve the devicePath of a volume, e.g:

• /dev/sda returns /dev/disk/by-id/wwn-0x5001b444a4bbea26
• /dev/dm-2 returns /dev/disk/by-id/dm-uuid-LVM-ALKa3ucymYV7FMBOAGWXpvsGdYz90e5cE3UzfO24GNUidBmOtPpCb47HqL6ekqxa
• /dev/sda1 returns /dev/disk/by-partuuid/b8b36029-01

And we don't support MBR/BIOS partition.

(and I like the idea for SparseLoopDevice 👍 )

Is that right?

---

As to where to store device size/device path until we create the PV, I guess the volatile hashtable is a good enough solution, right?

There isn't much point in persisting those in the CR (even if it's in the status) as they are only needed between the time we fetch them and the time we create the PV (at which point they become persisted in the PV).

[NicolasT]

Let's put the selection between File/Block at the Volume level then (which make more sense indeed, same as volumeMode is at the PV level, there is some symmetry here )

Yup.

About the reliable device path, IIUC instead of using disk.blkid, I can write a custom Salt module that would resolve the devicePath of a volume, e.g:

• /dev/sda returns /dev/disk/by-id/wwn-0x5001b444a4bbea26
• /dev/dm-2 returns /dev/disk/by-id/dm-uuid-LVM-ALKa3ucymYV7FMBOAGWXpvsGdYz90e5cE3UzfO24GNUidBmOtPpCb47HqL6ekqxa
• /dev/sda1 returns /dev/disk/by-partuuid/b8b36029-01

Well, the input is not necessarily /dev/dm-2, for example (I'd likely use /dev/mapper/vg-lv or /dev/vg/lv myself as input... The /dev/dm-* nodes are not really something a human would use, right?), so this modules would first need to resolve symlinks to an actual block device node, then figure out the relevant /dev/disk/by-id path of it.

And we don't support MBR/BIOS partition.

We need to validate this with TS.

(and I like the idea for SparseLoopDevice )

Yeah, having sparse support for block volumes is a must, for test and dev at least.

As to where to store device size/device path until we create the PV, I guess the volatile hashtable is a good enough solution, right?

There isn't much point in persisting those in the CR (even if it's in the status) as they are only needed between the time we fetch them and the time we create the PV (at which point they become persisted in the PV).

Indeed. One can't know at which exact point in time we look those up anyway.

[NicolasT]

(As an aside: even when a whole disk is to be used, I think a GPT table should be created on it, with a single partition spanning block 2048 to the end of the disk, and using a partuuid, instead of using the raw disk)

[slaperche-scality]

Well, the input is not necessarily /dev/dm-2, for example (I'd likely use /dev/mapper/vg-lv or /dev/vg/lv myself as input... The /dev/dm-* nodes are not really something a human would use, right?), so this modules would first need to resolve symlinks to an actual block device node, then figure out the relevant /dev/disk/by-id path of it.

Yup indeed, we also have this indirection to handle.

And we don't support MBR/BIOS partition.

We need to validate this with TS.

Will wait for a confirmation then.

(As an aside: even when a whole disk is to be used, I think a GPT table should be created on it, with a single partition spanning block 2048 to the end of the disk, and using a partuuid, instead of using the raw disk)

Yeah, we could do it that way.
WWW ID aren't reliable? Or it's just to limit the number different of cases?

[xaf-scality]

For me, GPT is a good solution for that. We should not use boot drive => no need to support MBR/BIOS partition and in some case (PoC) if we need to simulate multiple drive on one drive LVM+GPT will do the trick.

[NicolasT]

WWW ID aren't reliable? Or it's just to limit the number different of cases?

WWNs are reliable (at least that's the idea). However, indeed, since not all cases can be covered by using WWNs, maybe using an alternative that covers all cases by construction is 'better'.

[slaperche-scality]

So, to sum up the feedback we got (here and in the mail thread):

• RawBlockDevice with a real disk => use GPT partition + a single partition (by-partuuid/<volume-uuid> as path)
• RawBlockDevice with LVM => use LVM UUID (by-id/dm-uuid-LVM-… as path)
• RawBlockDevice with a partition => require GPT (error out if not) and use partition UUID from GPT (by-partuuid/<partition-uuid> as path)
• SparseLoopDevice => create a sparse file and create a GPT partition on it (by-partuuid/<volume-UUID> as path)