Skip to content

Conversation

@hardys
Copy link

@hardys hardys commented Jan 24, 2020

Some fixes to allow testing with ipv6 enabled controlplane, the related config looks like:

export EXTERNAL_SUBNET="fd2e:6f44:5dd8:c956::/120"
export DNS_VIP="fd2e:6f44:5dd8:c956:0:0:0:2"
export NETWORK_TYPE="OVNKubernetes"
export CLUSTER_SUBNET="fd01::/48"
export CLUSTER_HOST_PREFIX="64"
export SERVICE_SUBNET="fd02::/112"

Note this also requires a special release image and some image customizations atm while related OCP/RHCOS fixes land

Copy link
Member

@russellb russellb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Steven Hardy added 5 commits January 24, 2020 16:19
So we can access it via ipv6
Instead of a regex which only accepts ipv4 addresses
Since the oc adm mirror command appears to reject target registries
and pullspecs that contain ipv6 addresses[1] we can work around this
by using a name instead

[1] openshift/oc#239
This is required to access the local registry by name (and for the
related SSL certs)
Default to the values from the installer for ipv4, but we
can override like this for ipv6:

export CLUSTER_SUBNET="fd01::/48"
export CLUSTER_HOST_PREFIX="64"
export SERVICE_SUBNET="fd02::/112"

Co-Authored-By: Russell Bryant <[email protected]>
@hardys hardys requested a review from stbenjam January 24, 2020 16:31
@hardys hardys added the CI check this PR with CI label Jan 24, 2020
@hardys hardys changed the title WIP changes to enable testing with an IPv6 controlplane Changes to enable testing with an IPv6 controlplane Jan 24, 2020
# IPv6 network config validation
if [[ "${EXTERNAL_SUBNET}" =~ .*:.* ]]; then
if [[ "${NETWORK_TYPE}" != "OVNKubernetes" ]]; then
echo "NETWORK_TYPE must be OVNKubernetes when using IPv6"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we just set this to OVNKubernetes if we detect IPv6?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we could - @russellb do you think anyone is likely to test anything other than OVNKubernetes here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They shouldn't. I'm OK with just setting it instead of telling them to set it ...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's do this in a follow-up

@stbenjam
Copy link
Member

Otherwise LGTM

@metal3ci
Copy link

Build SUCCESS, see build http://10.8.144.11:8080/job/dev-tools/1456/

@russellb russellb merged commit e928e70 into openshift-metal3:master Jan 24, 2020
@hardys hardys mentioned this pull request Jan 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI check this PR with CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants