CSI block-device mount_options not working as expected when using volume_mount #9021
Comments
|
Thank you for opening this, we just realized this was happening in our deployment a couple weeks ago as well and I haven't had a chance to dig into it more to open the issue. I'm on mobile and away from the work computer but I can verify that we are seeing the behavior below:
At this point, I would assume an error for this mount failure would be passed back up but that doesn't appear to be happening so Nomad is assuming the volume had been mounted successfully and proceeds with starting the allocation and reporting everything being deployed successfully. I was able to dig in a little bit and remember coming across the logic for the publish volume command and it looked like it may be applying the mount options only for "file-system" type registrations instead of for "block-device"(I recall the comments indicating it was intended to apply to block-devices). I feel like the cinder CSI driver should be reporting the publish operation as a failure but isn't for some reason which is likely a big in the driver itself. I'm in the middle of wrapping up the quarter and getting prepared for the next so things are a bit busy ATM but I can make time to assist with testing/debugging. |
|
Hi @acornies! Thanks for opening this issue. And thanks for jumping into the discussion @rigrassm!
Is that before or after the volume is claimed by an allocation (just trying to figure out if we're overwriting that value incorrectly somehow).
It might help here if you could provide the node plugin's logs from when the consuming task is being started. That might help narrow down whether the issue is in Nomad or the cinder driver or both. |
|
Here's a log from one of "node" tasks: Log from a controller:
Before. Nomad doesn't appear to persist the mount options before an alloc utilizes it. Maybe I can get some more verbose logging from the driver... |
Hrm, ok... lemme see if I can reproduce that outside of a cinder plugin.
Yeah typically for this sort of thing we'll want debug-level logs, as the plugin developers don't seem to log individual volume operations at any higher level than that. By the values we've got here I'd say the plugin is defaulting to info-level. |
Unfortunately (and this is one of the reasons I had to hold off on trying to deep dive any further at the time) I couldn't find any way to enable debug level logging after looking through the source either. I just took a look at the k8s manifests they use for deploying the driver again and noticed a couple things in the definition for the node-plugin definition that differ from what is in the nomad example job. The two things that stand out to me are:
|
|
Re:
I believe this is what I remember seeing when I initially looked into the mount options not being set. Line 510 in c195f93 Specifically: The mount options defined in this struct only appear to be attached to Volumes of type "VolumeAccessTypeMount" (ie. file-system) and don't get attached associated with block-device types. I'm not certain that I'm seeing the whole picture here so I figured i'd just jot down what I saw and the assumptions in hopes that someone more familiar with the code base and the CSI spec could determine if it's actually an issue or not lol. |
|
Sorry for the comment spam, just came across this issue which states that you can change the log level for the node plugin using the flag |
|
Thanks @rigrassm - I was able to relaunch with Something that stands out is |
|
That looks promising, it seems odd that there aren't any errors being reported when mounting the device but it may be expected behaviour from the drivers perspective if no mount options are provided. I think @tgross will be able to get a good idea of what's going on here with this information though. |
|
Perfect, thanks folks. @rigrassm wrote:
We bind in the staging directories already in the @rigrassm wrote:
and @acornies wrote:
Good eye on noticing that, but if you search thru the spec for the definition of // Specify a capability of a volume.
message VolumeCapability {
// Indicate that the volume will be accessed via the block device API.
message BlockVolume {
// Intentionally empty, for now.
}
// Indicate that the volume will be accessed via the filesystem API.
message MountVolume {
// The filesystem type. This field is OPTIONAL.
// An empty string is equal to an unspecified field value.
string fs_type = 1;
// The mount options that can be used for the volume. This field is
// OPTIONAL. `mount_flags` MAY contain sensitive information.
// Therefore, the CO and the Plugin MUST NOT leak this information
// to untrusted entities. The total size of this repeated field
// SHALL NOT exceed 4 KiB.
repeated string mount_flags = 2;
}
...I'm wondering if this is a validation issue... are we sure that the cinder plugin is supposed to support these options? |
|
My assumption given Nomad's own docs was that this is possible for |
|
Well that's embarrassing! 😀 I'll have to double-check the origin PR for those docs and see if I can track down why. I probably won't get a chance to circle back to this until tomorrow though. |
|
Let's see who wrote those docs... oh dear, it was me 🤦 #7439 Ok, well if I follow the path of the
With a bit more digging I'm remembering that having mount options associated with a specific volume isn't part of the CSI spec, but something Nomad is doing for operator convenience -- it's setting a default value for that volume. The volume's mount options get merged with the So I think there are three fixes here:
|
|
From a higher level, what are your suggestions on how to use Nomad with cinder-csi? Is it even possible? Not being a CSI expert, I'm wondering how the k8s ecosystem utilizes it and how that could be applied here (if at all). |
It should be, with the caveat that we don't support the
It looks like they have an example of a consumer job for us to work from: a MySQL job that they've split for some reason between the split between the |
|
BTW, thanks very much for spending time on this - really appreciated. I guess I'm wondering where the disconnect is (could very well be my understanding) because the cinder csi examples use I use the ID from this terraform openstack resource: @rigrassm Did you have this working at some point with a Docker-based job? |
|
@acornies Unfortunately no, I believed it was working due to everything in the process going through as expected (including nomad reporting the volume being attached to the alloc) after starting the job. It wasn't until we had a job get rescheduled which ended up losing it's persisted data that I realized that the volume want actually being mounted. Re: using This absolutely should be possible though. I'm going to be a little tired up the rest of this week but I'll try and poke at things some more if I get some free time. It may be worth opening an issue over at the cloud-provider-openstack project to see if they can help us make sense of what's going on here (I think this is likely going to be needed at some point anyways since the driver not actually mounting the device should be an error which is not being thrown and I consider a bug in the driver). If you would like to open an issue over there and tag me I'll be happy to help over there. If not, I'll try and open one up when I'm able to get to seeing things up again in the test environment. |
|
So I this after searching around a bit about mount options in the volume registration, this page indicates that there are specific cases where mount_options may apply to block devices. https://kubernetes.io/docs/concepts/storage/persistent-volumes/#mount-options |
This is where the nomenclature they've chosen is super unfortunate, because it's overlapping an existing term of art. I'm fairly certain the references there to "block devices" mean in the sense of the operating system, and not in the two APIs available in CSI. As in a block device vs a character device (totally unsupported) vs an object store (external HTTP API). |
Yes, others in our organization are using this exact driver with k8s successfully. Putting the spec aside for a moment, would passing in the mount options for block devices in this context work in theory? Is this a case where Nomad has strictly adhered to the spec, not taking into account a big use case which is OpenStack Cinder? |
|
Breaking it down a little further: I thought I wouldn't have to do something like: |
That's certainly a possibility, so let's take a look at the Cinder plugin code. Looks like the filesystem for mounting (ext4 by default) is being determined and formatted in the node plugin at the end of And if we look at where we translate from Nomad's internal representation to the CSI protobuf in
Agreed. That being said, given that code I'm not sure why you need to pass the If leaving it off doesn't work, we can fix it... let's enumerate our options and their tradeoffs: (1) Allow mount options to be passed in for block volume plugins.
(2) Validate the mount options strictly against the spec:
I think I'm leaning towards allowing the mount options to be passed in for block volume then, and being a little more loose in spec compliance. I want to grab @schmichael for his thoughts on that though. |
|
@acornies I think I missed this earlier, but the plugin does treat block device capability and mount capability separately. If it finds the block device capability it just exits that stage early Maybe the error you ran into earlier had to with how the volume was created in OpenStack? Can you provide a redacted command line for |
|
Also, if we look a little more closely at the protobuf: https://github.com/container-storage-interface/spec/blob/v1.3.0/csi.proto#L387 we can't pass both the |
I don't see any other options I can use for volume creation (v3) in our OpenStack APIs. When using these "block-device" volumes on a VM, I have to use This comment on the bottom of the OpenShift docs looks relevant: https://docs.openshift.com/enterprise/3.1/install_config/persistent_storage/persistent_storage_cinder.html#volume-format-cinder Is this what the csi driver is supposed to handle? Are there any other code paths, maybe with |
Yes, filesystem formatting is entirely controlled by the That made me realize something about the error you showed earlier: This is coming from The spec tells us that the CO (Nomad, in this case):
But I think we misread this and are being overly aggressive in validating the response we get back from the plugin. My reading of this is that we should be checking the return for the specifically-validated capabilities, but that we should then only use the capabilities that we've validated. Which in this case would mean not passing the mount options along and letting the default behavior of the plugin take over. If k8s is closely following the spec here, that's probably why the Cinder plugin is working there. But the Cinder plugin should be returning any fields it wants to use, as far as I can tell. You could test this theory by using the Cinder plugin with k8s and setting the |
|
That being said, I want to check in with the CSI folks to ensure my interpretation is correct here before we go making changes. |
|
I've opened container-storage-interface/spec#458 to get some clarification. |
|
More data points:
|
|
I've discovered something unfortunate about why k8s works with the Cinder driver... it's because they don't actually do the |
|
I've got an exactly failing test for the case you're getting, @acornies, so the fix to loosen the validation should be pretty trivial at this point: diff --git a/plugins/csi/client_test.go b/plugins/csi/client_test.go
index 265d234b8..061e3b193 100644
--- a/plugins/csi/client_test.go
+++ b/plugins/csi/client_test.go
@@ -564,6 +564,23 @@ func TestClient_RPC_ControllerValidateVolume(t *testing.T) {
// this is a multierror
ExpectedErr: fmt.Errorf("volume capability validation failed: 1 error occurred:\n\t* requested mount flags did not match available capabilities\n\n"),
},
+
+ {
+ Name: "handles incomplete confirmed response",
+ Response: &csipbv1.ValidateVolumeCapabilitiesResponse{
+ Confirmed: &csipbv1.ValidateVolumeCapabilitiesResponse_Confirmed{
+ VolumeCapabilities: []*csipbv1.VolumeCapability{
+ {
+ AccessMode: &csipbv1.VolumeCapability_AccessMode{
+ Mode: csipbv1.VolumeCapability_AccessMode_MULTI_NODE_MULTI_WRITER,
+ },
+ },
+ },
+ },
+ },
+ ResponseErr: nil,
+ ExpectedErr: nil,
+ },
}
for _, tc := range cases { |
|
Thanks for the investigation @tgross! Looks like we're moving forward with relaxing the validation rules which is super exciting! Working CSI with OpenStack Cinder is a game changer for us. Are you folks moving forward with the change in a specific branch, or are you accepting PRs for this? |
I should have a PR up by end-of-day (East Coast US). That'll get shipped in the upcoming 0.13. |
|
Heck Yeah, thank you @tgross! |
|
I've just merged #9049 and that will ship in 0.13. |
|
Really late to the party here but clarification about the file-system vs block-device question. In this case it’s about how the storage is exposed to the container, not what’s backing it per-se. ie: csi supports exposing volumes as raw block devices to containers which could hypothetically then do their own formatting etc...it’s not heavily used but that is the context of the option. As an example I could create a nfs based csi driver which allows block access...in which case the driver would dd a bunch of 0s to a file on the nfs share and then mount the file into the container as if it were a ‘real’ block device like an iscsi drive etc. |
|
@travisghansen Yeah, I think that's where the confusion originated from. In my case, I was interpreting the In hindsight, it seems like a silly misinterpretation but this thread should serve as a good source of information for when others are trying to figure things out. |
|
Yes, the terminology is definitely confusing! Hopefully this helps a little for folks who stumble upon this! |
|
I'm going to lock this issue because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active issues. |
Nomad version
Nomad v0.12.5+ent (99ba9e9)
Operating system and Environment details
Issue
We are using Nomad Ent with OpenStack and Cinder CSI. We came across this guide: https://github.com/hashicorp/nomad/tree/master/demo/csi/cinder-csi-plugin which seems to almost do the trick, but the volume fails to mount properly as a ext4 filesystem when using the Docker driver.
When inspecting the volume, the
Mount Optionsare always<none>even though we've registered the volume with options like so:The cinder csi node and controllers appear to be working as expected and the device does get attached properly from a node perspective (/dev/vdb). Wondering if there's something I'm missing here? I also noticed that on a job level, you can specify the same mount options:
I've tried both ways with no luck getting these volumes working correctly with Docker. Tagging @rigrassm in case he ran into similar issues.
Reproduction steps
Job file (if appropriate)
Thanks for your time.
The text was updated successfully, but these errors were encountered: