search

clastix / kamaji

Kamaji is the Hosted Control Plane Manager for Kubernetes.
https://kamaji.clastix.io
Apache License 2.0
919 stars 81 forks source link

Automatically set dns service address #468

Open kvaps opened 1 month ago

kvaps commented 1 month ago

Kamaji specifies coredns address 10.96.0.10 by default, I think if serviceCidr specified this address should be automatically calculated from the serviceCidr + 10

ref and workaround https://github.com/aenix-io/cozystack/pull/147

prometherion commented 1 month ago

The problem with inferenced defaults is on kubebuilder defaults behaviour.

If you specify a different Service CIDR with no DNS Service IPs, the defaults will be placed. To avoid these kinds of misconfiguration, I would propose a validation to check if those IPs are in the CIDR block, by reporting an error so the user can fix it.

We have safe defaults, I think it's a good trade-off for some customizations on the user side upon values changes: I'm open for discussion.

kvaps commented 1 month ago

Hey @prometherion the DNS server address are always 10th address in servicesCIDR, I think it would be better to calculate dynamic default address and use it if user have not specified another

kvaps commented 1 month ago

would propose a validation to check if those IPs are in the CIDR block, by reporting an error so the user can fix it.

Another problem I see, is that user might want to specify custom DNSes for his cluster

prometherion commented 1 month ago

I think it would be better to calculate dynamic default address and use it if user have not specified another

I get your point, magic autofill is always good, the problem is the OAPIv3 default values.

prometherion: If you specify a different Service CIDR with no DNS Service IPs, the defaults will be placed

Another problem I see, is that user might want to specify custom DNSes for his cluster

Supporting different DNS rather than cluster.local is something we can easily implement, I'd track this in a different feature request, since we're addressing a bug.

kvaps commented 1 month ago

Ah got it. Then it probably should be done by the controller

prometherion commented 1 month ago

I've merged the referenced PR but not marking this as close, since it would require a change in the specification impacting the user experience of Kamaji.

I want to keep this issue open not in the form of a stale issue, but rather to gather interest from the community if it's something we need to address: I don't have a strong opinion on convention over configuration rather than configuration over convention, I'd like to approach to these cum grano salis.

Happy to change my idea and get the implementation the community is expecting: magic defaults are nice, but too much magic could bring issues to the software maintainability.

jds9090 commented 1 month ago

I also had a similar issue. DNS resolution didn’t work because of the misconfiguration on some of tenant clusters. Whenever we changed Service CIDR, we had to change DNS Service IPs. I think it's a tedious task because The virtual IP of the CoreDNS Service is always the 10th IP in the Service network(https://github.com/kubernetes/kubernetes/blob/6911225c3f747e1cd9d109c305436d08b668f086/cmd/kubeadm/app/constants/constants.go#L640). Why don’t we calculate dynamic default address Only if the DNS Service IPs are empty. I think that's appropriate because kamaji is based on kubeadm.

prometherion commented 3 weeks ago

Why don’t we calculate dynamic default address Only if the DNS Service IPs are empty.

As referenced here, we need to change the kubebuilder default behaviour, ensuring a smooth experience even tho we're not specifying those values.

I can take care of that but it would require a bit of time, of course, any contribution here would be really appreciated as usual!