terraform-hcloud-kube-hetzner icon indicating copy to clipboard operation
terraform-hcloud-kube-hetzner copied to clipboard

Added longhorn variables for fstype and default replica count

Open TimHeckel opened this issue 2 years ago • 5 comments

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 avatar Aug 07 '22 21:08 TimHeckel

@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?

mysticaltech avatar Aug 08 '22 08:08 mysticaltech

@karbowiak FYI above.

mysticaltech avatar Aug 08 '22 08:08 mysticaltech

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?

TimHeckel avatar Aug 08 '22 13:08 TimHeckel

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.

mysticaltech avatar Aug 08 '22 14:08 mysticaltech

Thanks - done!

TimHeckel avatar Aug 09 '22 02:08 TimHeckel

@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!

mysticaltech avatar Aug 27 '22 08:08 mysticaltech

Will do the changes myself.

mysticaltech avatar Aug 27 '22 16:08 mysticaltech

@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.

mysticaltech avatar Aug 30 '22 10:08 mysticaltech