Skip to content
This repository was archived by the owner on Jan 9, 2020. It is now read-only.

Conversation

@foxish
Copy link
Member

@foxish foxish commented Feb 14, 2017

Should fix minikube issue in #112

@foxish foxish requested a review from mccheah February 14, 2017 18:52
@mccheah
Copy link

mccheah commented Feb 14, 2017

Looks ok to fix Minikube, although it would be good to follow up on the Minikube side to see why it's returning the new IP type in the first place. @foxish any idea why that is the case now?

Copy link

@mccheah mccheah left a comment

Choose a reason for hiding this comment

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

Looks fine but would like to know why this started showing up in this Minikube version. ExternalIP seems like the correct classification. Plus if a node advertises both an ExternalIP and a LegacyHostIP, should we pick only one or the other, or pick both?

@foxish
Copy link
Member Author

foxish commented Feb 14, 2017

It is deprecated, so, Minikube should move away from it and set the External/Internal IP. I'm not sure why they're still setting that parameter instead.
If both are provided, I think the order is ExternalIP first, then LegacyHostIP.

@mccheah mccheah merged commit 8b31beb into k8s-support-alternate-incremental Feb 14, 2017
@mccheah mccheah deleted the add-legacy-host-ip branch February 14, 2017 22:33
@kimoonkim
Copy link
Member

We have an on-premiss kubernetes 1.4 cluster that also has LegacyHostIP/InternalIP. So this fix helps us.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants