[WIP] Enable testing dual-stack scenarios #59
Conversation
| external_network_bridge_ip_v6: "{{ external_network_cidr_v6|ansible.utils.nthhost(1)|default('', true) }}" | ||
| external_network_dhcp_range_v6: | ||
| - "{{ external_network_cidr_v6|ansible.utils.nthhost(200)|default('', true) }}" | ||
| - "{{ external_network_cidr_v6|ansible.utils.nthhost(250)|default('', true) }}" |
There was a problem hiding this comment.
We could use potentially different values here, but it probably makes sense to keep IPv4 and IPv6 aligned for the dual-stack case.
There was a problem hiding this comment.
Yeah for debugging it seemed easier to just keep them the same, but users can override the values if they prefer something else
| {% endfor %} | ||
| </dhcp> | ||
| {% endif %} | ||
| </ip> |
There was a problem hiding this comment.
IPv6 does not include a DNS mapping as IPv4 (potentially) does. Not a must have, but to make IPv4 and IPv6 identical I'd include it, either now or later. What do you think?
Edit: sorry Steve, I did not check the documentation first, so if I got it right, the dns element adds additional records, while the A/AAAA records for the hosts are set through the ip/dhcp/host element. If this is correct, I would simply add the IPv6 section between the IPv4 and DNS elements rather than at the end, if you agree.
There was a problem hiding this comment.
I moved the dns stanza out of the v4/v6 conditional blocks which I think does the right thing but I need to test it - let me know if you think that's the right approach 👍
| @@ -15,6 +15,9 @@ libvirt_nic_model: virtio | |||
| # Set to false for DHCP-less testing with static-ips | |||
| libvirt_network_dhcp: true | |||
|
|
|||
| # Set to true for dual-stack testing with ipv6 | |||
| libvirt_network_ipv6: false | |||
There was a problem hiding this comment.
I'd feel courageous here and enable it by default probably. And maybe add two flags to disable IPv4 and IPv6 when needed, so that we can easily enable single stack testing for both. What do you think?
There was a problem hiding this comment.
I would prefer to leave it v4 only by default, just so for debugging the environment is as simple as possible
| @@ -0,0 +1,8 @@ | |||
| cni: {{cluster_cni}} | |||
| {% if libvirt_network_ipv6|bool %} | |||
There was a problem hiding this comment.
Maybe we can already put in place support for single v4, single v6 and dual here.
PS: fundamentally the more I think about it the more I think I'd take the time to enable all of the three combinations right now, it should be just a couple more ifs.
There was a problem hiding this comment.
I have added initial support for single-stack v6, not yet tested though and we know e.g Rancher doesn't yet support it so the management cluster at least probably has to remain dual-stack
This aligns with the upstream metal3-dev-env naming for easier reuse of the same roles/templates
When ipv6 is enabled we configure the management cluster for dual stack
The network defaults are unused, and the networkData is not needed since we're not deploying an image which can interpret this format
Add variable to optionally disable the ipv4 network
|
@mchiappero this is ready for another review pass when you get a moment - I think the dual-stack configuration should work now, I've not yet had time to validate the single-stack ipv6 case for downstream clusters (we may need to add variables for this so we can enable a dual-stack management cluster with single-stack downstream cluster example-manifests, or just render both I guess) |
|
Looks like this is working for dual stack: I noticed we don't template the CNI though so will add a varable for that in the example-manifests |
Depends on #58 - adds an ipv6 network and management cluster configuration, remaining configuration of the management cluster still TODO so marking this as draft