cluster-api-provider-vsphere icon indicating copy to clipboard operation
cluster-api-provider-vsphere copied to clipboard

Set InternalIP machine status.addresses

Open andrewsykim opened this issue 5 years ago • 11 comments
trafficstars

/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):

andrewsykim avatar Jul 01 '20 15:07 andrewsykim

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

fejta-bot avatar Oct 22 '20 14:10 fejta-bot

/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.

andrewsykim avatar Oct 22 '20 14:10 andrewsykim

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

yastij avatar Oct 26 '20 10:10 yastij

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?

andrewsykim avatar Oct 26 '20 13:10 andrewsykim

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

yastij avatar Oct 26 '20 15:10 yastij

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.

andrewsykim avatar Oct 26 '20 15:10 andrewsykim

that makes sense wrt timelines

yastij avatar Oct 27 '20 18:10 yastij

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

srm09 avatar Jan 30 '22 21:01 srm09

@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.

k8s-ci-robot avatar Jan 30 '22 21:01 k8s-ci-robot

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

flyik avatar Jun 06 '22 02:06 flyik