✨ Support both IPv4 and IPv6 addresses in startup scripts and configuration files #789
Conversation
This file contains hidden or 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
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
metal3-io-bot
added
the
needs-ok-to-test
Contributor
|
Hi @mchiappero. Thanks for your PR. I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
metal3-io-bot
added
the
size/L
mchiappero
force-pushed
the
ironic_ipv6
branch
3 times, most recently
from
3daa66c to
379c0a3
Compare
2 tasks
Member
|
Before I even take a look, please squash the commits, there is no reason to have 12. |
The value of host_ip is determined twice within the ironic.conf.j2 template file, by means of a relatively hard to read set of conditions. Avoid this duplication and improve readability by exporting the correct value once in scripts/configure-ironic.sh. This also leave more room for more complex evaluations should these be needed in the future (e.g. IPv6) Signed-off-by: Marco Chiappero <[email protected]>
mchiappero
force-pushed
the
ironic_ipv6
branch
from
379c0a3 to
56092b9
Compare
Member
|
/hold |
metal3-io-bot
added
the
do-not-merge/hold
Whether IRONIC_IP or PROVISIONIG_IP is provided to ironic-image, any such address should be checked for validity and, if needed, properly formatted. For this reason, this commit introduces parse_ip_address to be consumed inside wait_for_interface_or_ip. Signed-off-by: Marco Chiappero <[email protected]>
Add a new get_interface_of_ip function that returns the name of the interface of a given IP, and use it to determine if the PROVISIONING_IP address is actually present on a network interface. This improves the code readability and enables additional debugging output. Signed-off-by: Marco Chiappero <[email protected]>
Commit 2742439 added logic to tentatively identify the interface name in get_provisioning_interface if the PROVISIONING_IP is provided. However the same process in then repeated in wait_for_interface_or_ip. Signed-off-by: Marco Chiappero <[email protected]>
Introduce an explicit check for IRONIC_IP, allowing PROVISIONING_INTERFACE to be tested separately, to improve quality and readability. Moreover, whenever PROVISIONING_INTERFACE is not set by the user, function get_provisioning_interface attempts to determine one, or provide "provisioning" as default value. However this can cause confusing errors down the line. So, also remove this default value and fail gracefully, with proper logging, if the PROVISIONING_INTERFACE value is not detected. Signed-off-by: Marco Chiappero <[email protected]>
Up to now wait_for_interface_or_ip has parse the values in the following order: PROVISIONING_IP, IRONIC_IP, PROVISIONING_INTERFACE. However IRONIC_IP should likely be considered as overriding any PROVISIONING_* value. Thus, make sure IRONIC_IP is evaluated at the beginning of the chain. Signed-off-by: Marco Chiappero <[email protected]>
The ironic scripts either use PROVISIONING_IP as an input or try to determine an IP address to bind the sockets to. This results in IRONIC_IP being defined once the process is complete, and it can carry either an IPv4 or an IPv6 address. Likely, the assumption is that on Linux, by default, IPv4-mapped IPv6 addresses can be leveraged to serve both IPv4 and IPv6 through a single socket. However this is not a good practice and two separate sockets should be used instead, whenever possible. This change modifies such logic by - introducing the variable IRONIC_IPV6 alongside the existing IRONIC_IP - matching IRONIC_IP and attempting to populate both variables Please note that hostname based URLs, with both A and AAAA records, are also required for a fully working dual-stack configuration. Signed-off-by: Marco Chiappero <[email protected]>
As per the Ironic documentation: "This field [my_ip] does accept an IPv6 address as an override for templates and URLs, however it is recommended that [DEFAULT]my_ipv6 is used along with DNS names for service URLs for dual-stack environments." Fill my_ipv6 when an IPv6 address has been found for binding. Signed-off-by: Marco Chiappero <[email protected]>
Prioritize IPv6 over IPv4 when available to set host_ip in ironic.conf when LISTEN_ALL_INTERFACES is not set to true. Signed-off-by: Marco Chiappero <[email protected]>
When LISTEN_ALL_INTERFACES is not set, Apache should make Ironic API avaiable on either or both IPv4 and IPv6 sockets, depending on the addresses requested or found on the system. Make sure to set the "Listen" directive according to ENABLE_IPV4 and ENABLE_IPV4, and the VirtualHost when IRONIC_URL_HOSTNAME is present. Signed-off-by: Marco Chiappero <[email protected]>
Enable the use of individual IPv4 and IPv6 sockets when the respective IP is detected and LISTEN_ALL_INTERFACES is not set to true. This allows to correctly bind to both the IPv4 and IPv6 addresses found and not just one of them. Signed-off-by: Marco Chiappero <[email protected]>
Enable the use of two separate sockets for IPv4 and IPv6 when LISTEN_ALL_INTERFACES is set to true. While desirable, on Linux Apache uses IPv4-mapped IPv6 addresses by default, thus leveraging a single IPv6 socket for IPv4 connections as well. This behaviour is far from being desirable and can be disabled at compile time via the "--disable-v4-mapped" flag, so make sure both an ANY address Listen directive is present for both IPv4 and IPv6. When Apache is compiled with "--enable-v4-mapped", the IPv4 socket will be simply ignored. Please see https://httpd.apache.org/docs/2.4/bind.html for more information. Signed-off-by: Marco Chiappero <[email protected]>
mchiappero
force-pushed
the
ironic_ipv6
branch
from
56092b9 to
5e0ff03
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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 this PR does / why we need it:
Currently the configuration and startup scripts rely on the assumption that all the services should either be exposed on an IPv4 or an IPv6 address. Dual-stack support is somewhat provided by leveraging IPv4-mapped IPv6 addresses, that is, only an IPv6 socket is allocated (which is a discouraged practice). Enable true dual-stack support, both in ironic.conf and all the Apache instances, by introducing IRONIC_IPV6 alongside the existing IRONIC_IP, so that either or both IPs can be leverage when defined or detected.
To avoid breaking changes and keep the PR simple, externally provided IRONIC_IP and PROVISIONING_IP values will result in either an IPv4 or an IPv6 socket allocated and not both. However, when PROVISIONING_INTERFACE must be used, the new code will attempt to detect both IP versions, potentially resulting in a proper dual-stack configuration on a dual-stack system. In this case, IPv6 will however take precedence for URL generation.
This PR paves the way to a future PR, where a hostname, resolving to both IPv4 and IPv6 addresses, can be leveraged for "dual-stack URLs" and, potentially, in a dual socket configuration. Besides this possibility, an initial conversation on other approaches or improvements can also get started here.
NOTE: based on PR #788
Checklist: