terraform-hcloud-kube-hetzner
terraform-hcloud-kube-hetzner copied to clipboard
Added longhorn variables for fstype and default replica count
I added two variables to accommodate my needs with mongodb (which strongly recommends the xfs file system). I imagine a better solution would be expanding PR #244 and allowing these options to be set during volume creation instead of as a global in the longhorn.yml file, but I wanted to get his out there regardless in case it does prove useful.
Thanks!
@TimHeckel Thanks for this; it's very useful indeed. One thing I am tempted to do is to implement what was done for what was done for Cilium, i.e., allow the addition of an optional file that contains the helm values at the root of the module next to the kube.tf
. This would give infinite possibilities for those that want to change the defaults.
In the meantime, we could use your values as defaults, both xfs
and the replica count of 3; what do you think? xfs
is better for storage than ext4
, no?
@karbowiak FYI above.
Thanks @mysticaltech - I am guessing most people prefer the ext4 file system, xfs is better in many specialized cases, but ext4 is generally preferred from what I understand.
That said, the current implementation is probably just fine to keep (defaults to ext4 and 3 replicas). If we went the route of an optional file, that would be consistent with the Cilium implementation, but I'm not familiar enough with longhorn to know if there are many other variables you'd want to change, so the separate file might be overkill, which would make this PR potentially viable? (I was able to deploy using this PR, i'm unclear why the validation is failing?)
I also did some reading and it's unclear whether you can switch between file systems at the volume level with longhorn? Meaning, do you have to chose either xfs or ext4 when first deploying, or could they be set at the volume level?
The validation can be fixed with 'terraform format'.
The file option is to set helm values, definitely for first deploy. It's actually easy to implement.
Am just considering if everything cannot be simplified via that option first before merging (and potentially causing more backward incompatibility later down the line).
But please do fix the validation 🙏 and thank you so much, all of this is very valuable and important if the value file option turns out to be impractical.
Thanks - done!
@TimHeckel Sorry for the delay, I was on a break and let this pass, my apologies. Am going to merge it ASAP, but please do the suggested tweaks above. Thanks!
Will do the changes myself.
@TimHeckel FYI, lots of exciting changes were released. All of the above still stands, but more goodness was added, like the ability to add a custom longhorn_values.yaml file and the possibility to use Hetzner volumes. More info in the kube.tf.example
file.