feat: enable btrfs

[motiejus]

This change is enough to provision and run a btrfs filesystems.

Tested workflows:
[x] provisioning (formatting)
[x] disk expansion

/kind feature

Fixes #2000

What this PR does / why we need it:

Enables formatting new PVCs using btrfs filesystem.

Mount options like compress=zstd work as expected.

Does this PR introduce a user-facing change?: YES

ubuntu-only: specifying `fsType: btrfs` in the PVC definition now formats & mounts a btrfs filesystem. btrfs support is experimental.

Non-btrfs users are not impacted by this change.

It is not explicitly supported in GCP docs, and it does not have to be, at least in the interim. This PR allows users easily experiment with the FS.

The integration tests are not updated to run btrfs, because as of writing COS does not support btrfs.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: motiejus / name: Motiejus Jakštys (d51a58d)

[k8s-ci-robot]

Welcome @motiejus!

It looks like this is your first PR to kubernetes-sigs/gcp-compute-persistent-disk-csi-driver 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/gcp-compute-persistent-disk-csi-driver has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @motiejus. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[sunnylovestiramisu]

Hmm so supporting btrfs resizing on Hyperdisk volumes requires creating a smaller volume, migrating data, and deleting the original. This seems like a half supported feature for PDCSI driver, I am not sure if we want to have it enable.

/assign @mattcary

[mattcary]

/ok-to-test

Since this is just adding another option to the storage class and not changing any of the control flow, I'm inclined to go ahead and enable it. Although if resizing is not working the tests probably need to be updated to skip them -- let's see what the CI does.

[mattcary]

@motiejus can you update the README with details of support? Including any quirks around usage, like resize as Sunny mentioned (I haven't verified any of this myself).

[motiejus]

Hmm so supporting btrfs resizing on Hyperdisk volumes requires creating a smaller volume, migrating data, and deleting the original. This seems like a half supported feature for PDCSI driver, I am not sure if we want to have it enable.

AFAIK CSI API only supports expansion? From here:

Note that, although you can specify a lower amount of storage than what was requested previously,
the new value must still be higher than .status.capacity. Kubernetes does not support shrinking a PVC
to less than its current size.

Let me know if this is wrong, I would be very happy to stand corrected. As far as I understand, btrfs is the only filesystem out of the 3 (others of course being xfs and ext4) that supports online shrinking. If there is an API I can hook into, I would be more than happy to implement it for btrfs.

@motiejus can you update the README with details of support? Including any quirks around usage, like resize as Sunny mentioned (I haven't verified any of this myself).

As far as I have tried, all the features present in ext4 and xfs are working just as well in btrfs, thanks to already-present support in mount-utils.

[motiejus]

Those failing tests seem like flakes? Windows-2019 and Windows-2022.

To @mattcary 's comment

Including any quirks around usage

Early in testing we realized that the default 50MiB RAM limit is not enough to mount larger btrfs volumes (1TiB++). So we bumped the limit to 256MiB internally and didn't have any issues since. It is different from #1782, since that was the mount call that caused the OOM (the linked ticket is fsck).

Given that the memory limits are not part of this repository, do you think this warrants an entry in the README?

[mattcary]

I think updating the readme to say that btrfs is available, but it's new and has not been production-tested. Things like increased memory needs would also be helpful.

[motiejus]

I think updating the readme to say that btrfs is available, but it's new and has not been production-tested. Things like increased memory needs would also be helpful.

Added a note in the README. If you prefer other wording, please let me know how, so I'll update it, or feel free to change it yourself.

Thanks!

[mattcary]

I think updating the readme to say that btrfs is available, but it's new and has not been production-tested. Things like increased memory needs would also be helpful.

Added a note in the README. If you prefer other wording, please let me know how, so I'll update it, or feel free to change it yourself.

Thanks!

That looks great, thanks

[mattcary]

Hmm, it didn't look like the integration test ran with your sc-btrfs. I'm not sure why, can you take a look?

[motiejus]

Hmm, it didn't look like the integration test ran with your sc-btrfs. I'm not sure why, can you take a look?

Tried to take a look. tests/run-k8s-integration-ci.sh wasn't executed, thus xfs tests also didn't run. Seems like we have a bigger problem, but I don't know where to check: I can't find an entrypoint to the integration tests: .github directory does not have anything related.

Are we missing a big chunk of integration tests? Anyhow, this seems to be a bigger problem than btrfs. Happy to help, but my knowledge of the test setup is lacking.

[sunnylovestiramisu]

Hmm, it didn't look like the integration test ran with your sc-btrfs. I'm not sure why, can you take a look?

Tried to take a look. tests/run-k8s-integration-ci.sh wasn't executed, thus xfs tests also didn't run. Seems like we have a bigger problem, but I don't know where to check: I can't find an entrypoint to the integration tests: .github directory does not have anything related.

Are we missing a big chunk of integration tests? Anyhow, this seems to be a bigger problem than btrfs. Happy to help, but my knowledge of the test setup is lacking.

You should try to replace the --storageclass-files=sc-balanced.yaml in the run-k8s-integration.sh and see if the integ tests run.

[motiejus]

The integration test failed with this:

  Mounting arguments: -t btrfs -o defaults /dev/disk/by-id/google-pvc-85a0c313-7a48-4daa-988a-9a4a5aeeabfb /var/lib/kubelet/plugins/kubernetes.io/csi/pd.csi.storage.gke.io/747f7ed65f8237a1726b5c15909179a6908fa3157bc2bfaf1aaf06a4e0362f27/globalmount
  Output: mount: /var/lib/kubelet/plugins/kubernetes.io/csi/pd.csi.storage.gke.io/747f7ed65f8237a1726b5c15909179a6908fa3157bc2bfaf1aaf06a4e0362f27/globalmount: unknown filesystem type 'btrfs'.
         dmesg(1) may have more information after failed mount system call. 

cos_containerd does not have a btrfs kernel module (we also stumbled upon this internally). I am changing to ubuntu_containerd for the sake of the test.

[motiejus]

Integration test doesn't like non-COS:

setting environment vars for bringing up ubuntu cluster on GCE is unimplemented

i will add a note in the README that CSI does not currently have btrfs kernel driver, thus only Ubuntu is supported (from the list of GCP-blessed OS images). If the OCI team asks, I would appreciate if you could support the clause.

[k8s-ci-robot]

@motiejus: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-gcp-compute-persistent-disk-csi-driver-e2e-windows-2019	d51a58d	link	false	/test pull-gcp-compute-persistent-disk-csi-driver-e2e-windows-2019
pull-gcp-compute-persistent-disk-csi-driver-e2e-windows-2022	d51a58d	link	false	/test pull-gcp-compute-persistent-disk-csi-driver-e2e-windows-2022

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

[mattcary]

i will add a note in the README that CSI does not currently have btrfs kernel driver, thus only Ubuntu is supported (from the list of GCP-blessed OS images). If the OCI team asks, I would appreciate if you could support the clause.

We've started the process internally, we'll see what happens.

Tried to take a look. tests/run-k8s-integration-ci.sh wasn't executed, thus xfs tests also didn't run.

If you go to the prow job yaml from the build details page, you see the following:

So it's the run-integration-test.sh script that's executed for presubmit. The -ci script is run periodically to make the following: https://testgrid.k8s.io/provider-gcp-compute-persistent-disk-csi-driver. Sorry, I misremembered what was what.

Anyway it looks like for the presubmit we only run on one storage class anyway, to save presubmit time. That's probably a good idea, ie we wouldn't want to add btrfs to the presumbit anyway. Thank you for arranging a one-off run to confirm the COS issue.

Since we've listed this support as experimental, and the presubmit run does confirm that enabling btrfs doesn't break anything else (not that we would have expected to but that's what testing is for....), I think we can submit this.

Thanks for your contribution!

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mattcary, motiejus

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [mattcary]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[mattcary]

@motiejus are you on slack? I have a couple questions to ask. Thanks!

[motiejus]

@motiejus are you on slack? I have a couple questions to ask. Thanks!

Sent you an email to your @google.com address.