Skip to content
This repository has been archived by the owner on Jul 30, 2021. It is now read-only.

Use spec.nodeName as hostname-override, not podIP #151

Merged
merged 1 commit into from
Dec 30, 2016
Merged

Use spec.nodeName as hostname-override, not podIP #151

merged 1 commit into from
Dec 30, 2016

Conversation

dghubble
Copy link
Contributor

@dghubble dghubble commented Oct 12, 2016 •
edited

  • Respect names given to nodes, don't assume IP
  • Allow DNS node names to be used
  • kube-apiserver uses host's default iface for advertise-address

We're using this patch for bare-metal and AWS clusters to allow DNS names to be used. Kubernetes 1.4, introduced the spec.nodeName downward api field. Pods can make use of the node name which may be either an IP or a DNS name.

rel: #90

@dghubble dghubble mentioned this pull request Oct 19, 2016
Merged
@aaronlevy aaronlevy changed the title Use host's default iface for kube-apiserver advertise-address [Do Not Merge] Use host's default iface for kube-apiserver advertise-address Oct 24, 2016
@khrisrichardson
Copy link

@dghubble it appears that spec.nodeName has landed and been vendored in. Are you still intending to merge this? It could simplify the coreos-baremetal instructions for bootkube.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 18, 2016
@dghubble dghubble changed the title [Do Not Merge] Use host's default iface for kube-apiserver advertise-address Use spec.nodeName as hostname-override, not podIP Dec 18, 2016
@dghubble dghubble mentioned this pull request Dec 18, 2016
Merged
@dghubble
Copy link
Contributor Author

dghubble commented Dec 18, 2016 •
edited

Here's the story, we use this patch on bare-metal and AWS clusters or anywhere else we'd like to name nodes by either DNS name or by IP. Using spec.NodeName is a now a great way to stop requiring nodes be given IP names. Its less about customization and more about choosing a more flexible flag.

The only setup I haven't verified is the vagrant/Virtualbox scripts used by this project. I suspect it may rely on:

- --advertise-address=$(MY_POD_IP)

but perhaps the default would suffice. I'll have to get a setup to try that out.

coreos-baremetal has been updated to v1.4.7 and this required bootkube patch has been updated accordingly.

@khrisrichardson @aaronlevy

@dghubble
Use spec.nodeName as hostname-override, not podIP
c2804c0 
* Respect names given to nodes, don't assume IP
* Allow DNS node names to be used
* kube-apiserver uses host's default iface for advertise-address
@Quentin-M
Copy link
Contributor

Tested against the multi-node deployment on master. An nginx Pod was able to reach another nginx Pod running on another worker using: service name (DNS), service IP, service NodePort, direct pod IP.

cc @dghubble

@dghubble
Copy link
Contributor Author

dghubble commented Dec 21, 2016 •
edited

I spun up the hack/multinode example in a Vagrant / Virtualbox setup and the apiserver detects an address to use. Nothing seems amiss. So this patch seems to allow DNS names, while not harming vagrant / local examples. It would mean we can stop carrying this patch and also coreos-baremetal can use upstream again :)

genericapiserver.go:606] Will report 172.17.4.101 as public IP address.

@aaronlevy

aaronlevy
aaronlevy approved these changes Dec 30, 2016
View reviewed changes
Copy link
Contributor

@aaronlevy aaronlevy left a comment

Choose a reason for hiding this comment

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

lgtm

@aaronlevy aaronlevy merged commit a4739ed into kubernetes-retired:master Dec 30, 2016
dghubble added a commit to poseidon/matchbox that referenced this pull request Dec 30, 2016
@dghubble
examples: Upgrade self-hosted Kubernetes to v1.5.1
2b220bc 
* Update bootkube version to stop requiring the forked
binary; bootkube now allows DNS names
* kubernetes-retired/bootkube#151
@dghubble dghubble mentioned this pull request Dec 30, 2016
Merged
dghubble added a commit to poseidon/matchbox that referenced this pull request Dec 30, 2016
@dghubble
examples: Upgrade self-hosted Kubernetes to v1.5.1
2997718 
* Update bootkube version to stop requiring the forked
binary; bootkube now allows DNS names
* kubernetes-retired/bootkube#151
@dghubble
Copy link
Contributor Author

Now that this has merged, poseidon/matchbox#407 updates coreos-baremetal docs to use kubernetes-incubator/bootkube. @khrisrichardson

@khrisrichardson
Copy link

@dghubble @aaronlevy @Quentin-M thanks much for following through on this

dghubble added a commit to poseidon/matchbox that referenced this pull request Dec 31, 2016
@dghubble
examples: Upgrade self-hosted Kubernetes to v1.5.1
20df52c 
* Update bootkube version to stop requiring the forked
binary; bootkube now allows DNS names
* kubernetes-retired/bootkube#151
dghubble added a commit to poseidon/matchbox that referenced this pull request Dec 31, 2016
@dghubble
examples: Upgrade self-hosted Kubernetes to v1.5.1
b9f0f61 
* Update bootkube version to stop requiring the forked
binary; bootkube now allows DNS names
* kubernetes-retired/bootkube#151
@aaronlevy aaronlevy mentioned this pull request Jan 4, 2017
Merged
@aaronlevy aaronlevy mentioned this pull request Jan 18, 2017
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@aaronlevy aaronlevy aaronlevy approved these changes

Assignees
No one assigned
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@dghubble @khrisrichardson @Quentin-M @aaronlevy @k8s-ci-robot