Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
• If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
• If you have done the above and are still having issues with the CLA being reported as unsigned, please email the CNCF helpdesk: helpdesk@rt.linuxfoundation.org

/assign @leblancd
/assign @anguslees

@zioproto: GitHub didn't allow me to assign the following users: leblancd.

Note that only kubernetes members and repo collaborators can be assigned.

/ok-to-test

reasonable approach i think until we are ready. can you please add a TODO/FIXME stating that we need to revisit this when we enable ipv6 support for openstack provider?

lgtm, although someone more recently familiar with OpenStack IPv6 addressing might be able to confirm that the assumption is valid that all IPv6 addresses processed here are externally accessible (i.e are in the GUA address range and are globally routed). Otherwise, looks like a good fix.

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: zioproto
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: anguslees

Assign the PR to them by writing /assign @anguslees in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

/test pull-kubernetes-e2e-kops-aws

/hold

/test cla/linuxfoundation

Hello all,
I tested the patch it works as expected:

Addresses:
  InternalIP:  10.8.10.7
  ExternalIP:  2001:620:5ca1:4005:f816:3eff:fe7d:f44d
  Hostname:    k8s-master

In the short term I am happy with this patch, because I can publish services in IPv6 using the nginx-ingress-controller because the VM running that container has an IPv6 address.

Of course in case of an IPv6 pod network this solution will not work.

There is a foundamental thing to decide before proposing any new patch. An IPv6 address assigned to a Openstack VM, should be threated as InternalIP or ExternalIP ?

thank you

@zioproto - That's a very good question! Again, I haven't worked with OpenStack IPv6 in a while, but this would be my take. As a straw man proposal, I would propose that the best we can do is to classify IPv6 VM addresses as internal/external based upon address range: LLA's get ignored (I assume these are already filtered out?), ULAs are internal, and GUA range addresses are external. So the overall logic would be:

if (addr is floatingIP) or (network is public) or (addr is IPv6 && is GUA range) {
    mark as external
} else {
    mark as internal
}

My reasoning is as follows. For IPv4, it's clear that only floating IPs or IPs on a public network would be externally accessible. For IPv6, it's a bit more fuzzy. Floating IPs are not typically (never?) used for IPv6 VMs (it used to be that IPv6 floating IPs weren't supported. Usually you want to avoid NAT66 translation). IPv6 addresses on a VM could be assigned in a variety of ways, if I remember correctly:

• Prefix delegation (using SLAAC)
• DHCP (stateless and stateful)
• Provider net
• Subnet pool (e.g. with external IPAM)
  Whichever way that a VM's IP address might get assigned, the question of whether it's externally accessible would come down to whether:
• Address is in the Global unicast range, AND...
• The corresponding subnet is routed from the outside world (either by prefix delegation or by the provider network)
  I would think that an operator would make sure that these bullets would be consistent, i.e. if there's an external route, then use GUA, If not, use ULA.

Typically, I think VM IPv6 addresses are going to be global (to avoid NAT66), so all addresses would get classified as externalIPs (except for the corner case where a VM subnet needs to be truly private).

@anguslees - Do you happen to know what would happen if the OpenStack cloud provider plugin only returns external IPs (internal IP list is empty)?

Hello @anguslees ,
I am implementing a getCIDRIpVersion function to figure out if I need an IPv4 or IPv6 nexthop for the route, and filter the proposed addresses as needed.

once I get the addresses from Openstack and I figure out if they are IPv4 and IPv6 it would be great to pass on the IPVersion information here:
https://github.com/zioproto/kubernetes/blob/issues/59421/pkg/cloudprovider/providers/openstack/openstack.go#L475

However, who write the data type for the addresses did no think about an IPVersion field:

https://github.com/kubernetes/kubernetes/blob/master/pkg/apis/core/types.go#L3553-L3566

Should I add it here in types.go ? It is my first patch into Kubernetes and I am not sure what structs is okay to patch.

thanks

However, who write the data type for the addresses did no think about an IPVersion field

On the golang side, you can guess by checking if net.IP.{To4.To16} returns non-nil (will do conversion to/from v4-mapped addresses, so the results aren't mutually exclusive). Alternatively, just checking for a : should be sufficient (since we know these are address literal strings).

@anguslees - Do you happen to know what would happen if the OpenStack cloud provider plugin only returns external IPs (internal IP list is empty)?

I believe it will all work fine. The whole InternalIP vs ExternalIP thing was never specified at all and nobody has been able to tell me what actual semantics are expected. The various bits of code I've seen (and written myself) just prefers InternalIPs over ExternalIPs, on the naive assumption that everyone has a split network infrastructure and billing model like AWS (ie: traffic to "internal" addresses is free).

According to what we discussed in this pull request a proposed a different new pull request do address the problem. #59749
thank you

/close

in favor of #59749