✨ Allow IRONIC_IP, IRONIC_IPV6 and URLs to be derived from a DNS record #791
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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
metal3-io-bot
added
the
do-not-merge/work-in-progress
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. DetailsInstructions 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
needs-ok-to-test
Author
|
I'm setting it to WIP as I haven't thoroughly tested yet (and the documentation hasn't been updated), but should be mostly bug free. Feedback on the design is very welcome in the meantime. |
mchiappero
commented
Nov 14, 2025
| {% endif %} | ||
| {% if env.IRONIC_URL_HOSTNAME is defined and env.IRONIC_URL_HOSTNAME|length %} | ||
| <VirtualHost {{ env.IRONIC_URL_HOSTNAME }}:{{ env.IRONIC_LISTEN_PORT }}> | ||
| {% else %} |
Author
There was a problem hiding this comment.
I guess we might also include the IP based VirtualHosts rather than have mutual exclusion. What do you think? Need to check if multiple VirtualHost directives would work...
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_ip-hostname
branch
from
ddef60a to
e0e58ed
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]>
In a dual-stack scenario, especially when deploying in direct mode via virtual media, it might be useful to 1) use a hostname to enable "dual IP" URLs 2) have ironic bind to those two addresses, if found on the system. To make this possible, this commit introduces: - a new user environment variable named IRONIC_URL_HOSTNAME, to be used as immutable external only input, to derive IRONIC_URL_HOST and the IP addresses to bind on - a new utility function named "get_ip_of_hostname" to help look up the A and AAAA records - additional logic to look for the returned address on the system, for binding the processes; this new logic has lower priority than PROVISIONING_IP (which can then be used to enforce one specific IP version) and PROVISIONING_INTERFACE Note, while IRONIC_URL_HOSTNAME and PROVISIONING_IP are considered to be mutually exclusive, IRONIC_URL_HOSTNAME and PROVISIONING_INTERFACE are not. Signed-off-by: Marco Chiappero <[email protected]>
Whenever listening addresses and/or URLs are based on an hostname contained in IRONIC_URL_HOSTNAME, update the Apache configuration for API server with an hostname based VirtualHost entry. Signed-off-by: Marco Chiappero <[email protected]>
mchiappero
force-pushed
the
ironic_ip-hostname
branch
from
e0e58ed to
969e216
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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:
PR #789 introduced the ability to create IPv4 and IPv6 sockets for Apache, but is limited in the ability to provide true dual-stack behavior with respect to the Ironic API endpoints. Introduce the a new environment variable, IRONIC_URL_HOST, which can contain an hostname which resolves to at least an IPv4 or an IPv6 address. When both are present, the client has the option to connect with whichever version is actually in use or preferred.
This design allows to also bind the servers to the IP addresses the hostname resolves to, when LISTEN_ALL_INTERFACES is not true, which means that such IPs will be searched for on the system when starting. Setting LISTEN_ALL_INTERFACES to true results in Apache binding on IPv4 and/or IPv6 according to the resolved IPs, but not to look for those IPs on the systems. This is to mimic the behavior of IRONIC_IP.
NOTE: based on PR #789.
Checklist: