✨ Allow FQDN as the control plane endpoint host #108
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Until now, we assumed the control plane endpoint host to be an IP, but that's an unnecessary restriction. The template now consumes a new environment variable: `CONTROL_PLANE_ENDPOINT_HOST` It's optional and defaults to the value of the existing `CONTROL_PLANE_ENDPOINT_IP` environment variable. As the host is required to resolve to that IP, we'd actually not need it anymore to be given explicitly. However, as we still render the kube-vip manifest in the template and don't have anything nice there to do the translation automatically, we keep the explicit IP for now. The controller code is not aware of the `CONTROL_PLANE_ENDPOINT_IP` value, though. It only sees the control plane endpoint host, which can now be either an IP or an FQDN, so the controller must be able to resolve the name to use the corresponding IP to find the matching IP block and to find the correct IP failover group. Unit tests for DNS resolving were not as convenient as I expected. The stdlib tests don't provide any means to mock it, they even rely on external network reachability to resolve some Google domain names. They pointed to golang/go#12503, though. There's an interesting link to https://github.com/ncruces/go-dns, showing how to inject custom behavior using a custom dialer, enabling things such as caching. It's overkill for us, though, so our used approach for unit testing is simpler.
piepmatz
requested review from
lubedacht,
gfariasalves-ionos,
mcbenjemaa,
wikkyk and
jriedel-ionos
as code owners
piepmatz
changed the title
lubedacht
suggested changes
Apr 30, 2024
|
lubedacht
approved these changes
May 2, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is the purpose of this pull request/Why do we need it?
Until now, we assumed the control plane endpoint host to be an IP, but that's an unnecessary restriction.
Description of changes:
The template now consumes a new environment variable:
CONTROL_PLANE_ENDPOINT_HOSTIt's optional and defaults to the value of the existing
CONTROL_PLANE_ENDPOINT_IPenvironment variable.As the host is required to resolve to that IP, we'd actually not need it anymore to be given explicitly. However, as we still render the kube-vip manifest in the template and don't have anything nice there to do the translation automatically, we keep the explicit IP for now.
The controller code is not aware of the
CONTROL_PLANE_ENDPOINT_IPvalue, though. It only sees the control plane endpoint host, which can now be either an IP or an FQDN, so the controller must be able to resolve the name to use the corresponding IP to find the matching IP block and to find the correct IP failover group.Special notes for your reviewer:
Unit tests for DNS resolving were not as convenient as I expected. The stdlib tests don't provide any means to mock it, they even rely on external network reachability to resolve some Google domain names. They pointed to golang/go#12503, though. There's an interesting link to https://github.com/ncruces/go-dns, showing how to inject custom behavior using a custom dialer, enabling things such as caching. It's overkill for us, though, so our used approach for unit testing is simpler.
Checklist: