Set InternalIP machine status.addresses

[andrewsykim]

/kind feature

Describe the solution you'd like
Almost always a VM will have an address internal to the datacenter. In the case where we don't know if an address is external or internal to the datacenter, we should assume it is internal and set the type as InternalIP. Today we always set it as an ExternalIP

Anything else you would like to add:
[Miscellaneous information that will assist in solving the issue.]

Environment:

• Cluster-api-provider-vsphere version:
• Kubernetes version: (use kubectl version):
• OS (e.g. from /etc/os-release):

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[andrewsykim]

/remove-lifecycle stale

This is still pretty important from my perspective, in general we shouldn't label private addresses spaces as ExternalIP. It's also not consistent with how we address node addresses.

[yastij]

@andrewsykim - Agreed. As this is a breaking changing my thinking was to include this in the set of breaking changes of v1a4

[andrewsykim]

As this is a breaking changing my thinking was to include this in the set of breaking changes of v1a4.

It's only a breaking change if we stop setting ExternalIP. I don't think it would be a breaking change to start setting InternalIP now though?

[yastij]

The interesting bit is that we don't have a way to distinguish between InternalIP and ExternalIP. Are you suggesting to set both ?

[andrewsykim]

If we can't distinguish between the two, we should set the type based on what is most common -- which from my understanding is InternalIP, since almost always a VM's machine IP is internal to a vCenter. In an ideal world we would just set InternalIP, but removing ExternalIP is a breaking change.

My suggestion would be to set both InternalIP and ExternalIP for the remainder of v1alpha3 and remove ExternalIP starting v1alpha4. Or even better, set the type based on some well known heuristic like checking if the address is in one of the well-known private address spaces.

[yastij]

that makes sense wrt timelines

[srm09]

@yastij This still seems to be the case for Machine addresses, something that we should look into again?
/help

[k8s-ci-robot]

@srm09:
This request has been marked as needing help from a contributor.

Guidelines

Please ensure that the issue body includes answers to the following questions:

• Why are we solving this issue?
• To address this issue, are there any code changes? If there are code changes, what needs to be done in the code and what places can the assignee treat as reference points?
• Does this issue have zero to low barrier of entry?
• How can the assignee reach out to you for help?

For more details on the requirements of such an issue, please see here and ensure that they are met.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

@yastij This still seems to be the case for Machine addresses, something that we should look into again?
/help

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/test-infra repository.

[flyik]

Any updates? I can implement it if thoughts about it are still the same

[CharlieR-o-o-t]

Have no idea why provider always sets ExternalIP , this is incorrect behavior.
Have created fix PR: #3379