Update handling of custom cluster-cidr

[thomasprade]

Concerning #894

Due to k3s' defaults for cluster-cidr (10.42.0.0/16) and service-cidr (10.43.0.0/16) the creation of more than 42 nodepools and their respective subnets conflicted with these IP ranges.

Changes:

• Added a variable for setting the service-cidr
• Added cluster- and service-cidr to k3s server start arguments
• Extended documentation on theoretical maximum of nodepools

[M4t7e]

Concerning #894 (comment)

I think my assumptions from the referenced comment are right, that K3s should be in charge of IP Address Management (IPAM) and not the CNI, if we want to have a fully working integration into Hetzner with their CCM: https://github.com/hetznercloud/hcloud-cloud-controller-manager/blob/main/README.md?plain=1#L122

When you use the Cloud Controller Manager with networks support, the CCM is in favor of allocating the IPs (& setup the
routing) (Docs: https://kubernetes.io/docs/concepts/architecture/cloud-controller/#route-controller). The CNI plugin you
use needs to support this k8s native functionality (Cilium does it, I don't know about Calico & WeaveNet), so basically
you use the Hetzner Cloud Networks as the underlying networking stack.

Btw, they also use ipam.mode=kubernetes in their testing setup with Cilium.
ipam.mode=kubernetes -> https://docs.cilium.io/en/stable/network/concepts/ipam/kubernetes/#kubernetes-host-scope

The Kubernetes host-scope IPAM mode is enabled with ipam: kubernetes and delegates the address allocation to each individual node in the cluster. IPs are allocated out of the PodCIDR range associated to each node by Kubernetes.

It should be the same story for Calico: https://docs.tigera.io/calico/latest/reference/configure-cni-plugins#using-host-local-ipam

Host local IPAM is generally only used on clusters where integration with the Kubernetes route controller is necessary

This is definitely the reason why the RouteController makes total mess with the Hetzner Network Routes, because the CNI is decoupled from K3s IPAM. I am still not sure about the impact and with the default values (cluster_ipv4_cidr untouched) it looks good in the first place, because K3s and Cilium are using the same network range. But they still work independently from each other and in case the Hetzner network routes for the Nodes Pod ranges are matching, it is pure coincidence or they use the same assignment approach. This really feels like it stands on wobbly legs and I think it should be fixed.

I would suggest to fix it either in this PR or in a new one, that implicitly covers this PR. cluster-cidr needs to be configurable and Calico and Cilium need an adjustment to use Kubernetes host-scope IPAM mode, so that cluster-cidr from K3s is honored.

[mysticaltech]

Thanks for that fix @thomasprade, it's important!

[thomasprade]

@M4t7e afaik the cluster_ipv4_cidr is already passed to cilium in locals.tf:356 (and calico in locals.tf:390 for that matter), so hopefuly fixing the way the cluster- and service cidr is passed to k3s also fixes your problem.

@mysticaltech glad to help 👍

[M4t7e]

@thomasprade Yes it is and that seems to be the problem already. That explicit configuration leads to decoupling the CNI IPAM from K3s IPAM. Then they run independently from each other and only K3s IPAM will set the needed Hetzner network routes to the distinct pod networks for the nodes (you can see them if you open the Hetzner console, select your network and switch to "Routes" tab).

I have tested your change with the current settings in Cilium and it does not hurt. This PR is already the first step to solve that decoupling problem, but it will not fix it. It just "looks" fixed, because of the similar IP assignment rules from K3s and Cilium (and maybe also Calico?). They still can diverge and if that happens, non-masqueraded setups (which are usually preferred due to performance reasons) can break even internally (this depends on many other settings). At least the current defaults lead to masquerading as far as I can see and test. So hopefully it's just a theoretical problem in most cases.

I'm currently crafting something for Cilium to solve that decoupling issue and it looks already very promising. I'll treat this change as precondition, as I can only fix it after this is done. So I'm looking forward for the merge 🙂

[M4t7e]

Wouldn't it be better to put that in the k3s server config files (specified in init.tf and control_planes.tf), together with the other config params?

[thomasprade]

I tried setting the two arguments as an additional parameter in the file provisioner in init.tf:11, but this didn't work.
From what I understand of the server documentation and configuration documentation these options must be passed as an argument to the k3s server command.

[M4t7e]

Starting with K3s v1.19.1+k3s1 all CLI parameters can be set in a configuration file.

You can now configure K3s using a configuration file rather than environment variables or command line arguments

I think it is not enough only setting it in init.tf:11, because this will only be on the first control plane node (first_control_plane resource). It also needs to be in control_planes.tf:95 for the other control plane nodes. See also the comment from https://docs.k3s.io/installation/configuration#configuration-with-binary

It is important to match critical flags on your server nodes. For example, if you use the flag --disable servicelb or --cluster-cidr=10.200.0.0/16 on your master node, but don't set it on other server nodes, the nodes will fail to join.

Maybe @mysticaltech or somebody else could verify that as I am pretty new to the project, but this is what I understood from reverse engineering it.

[mysticaltech]

@thomasprade What you did here is good, but just for consistency, if would be best to implement it indeed like @M4t7e mentioned in both init.tf and control-plane.tf, then we merge! 🙏

[mysticaltech]

I'll do it myself, it's super quick!

[mysticaltech]

Released in v2.4.0 🚀