ccremer / kubernetes-zfs-provisioner

Dynamic ZFS persistent volume provisioner for Kubernetes
Apache License 2.0
74 stars 7 forks source link

Optional space reservation (thin provisioning) #102

Closed Foxite closed 1 year ago

Foxite commented 1 year ago

Fixes #101

I'm going to test this next, but you can have a look at the code before I'm done. Has been tested.

Also I'm going to send a separate PR in which I will add the parameter to the example config in the helm chart. #103

Summary

I've added an optional parameter to the storage classes, called "reserveSpace", which is true by default. When specified as "false", any datasets created when provisioning PVs using that storage class will not have a RefReservation set.

Checklist

For Code changes

ccremer commented 1 year ago

Thanks for your contribution! code-wise LGTM, I like how you keep the default behaviour to avoid breaking change.

Documentation in the Readme is still missing (explain what the implication of thin-provisioning is).

To test it, you can run make test:integration (which creates a 1gb zpool in file in project dir, requires zfs on linux) and see if the properties get attached/removed correctly.

Foxite commented 1 year ago

I have added integration tests for thin provisioning and assertions that the refreservation property is actually correct, in both cases.

To test this it was necessary to upgrade go-zfs, because the currently used version has a bug that makes it impossible to get dataset properties.

Also don't look at the individual commits too much :')

Foxite commented 1 year ago

Just realized I forgot to update the Readme.