-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix #712, allow overwriting the public IP of a Kubernetes node #840
Conversation
@alvaroaleman Thanks for this change. If you can add some documentation and ideally a test, I can get it merged. For the documentation, I would suggest adding an "Annotations" section to https://github.com/coreos/flannel/blob/master/Documentation/kubernetes.md |
Added docs. Writing a test seems to be rather complicated, because there are none for the kube subnet manager yet. |
I added some tests for kube subnet manager recently - https://github.com/coreos/flannel/blob/master/dist/functional-test-k8s.sh But I agree, they are still quite complicated! |
Thanks for the hint @tomdee, I added a test. I also moved the check for |
Recreated the PR because travis was failing after having issues pulling the etcd image. The e2e tests run twice btw, because they are called in the travis file and from within the |
OK, this looks good. It would be great if you could add a little more information to the documentation, such as why a user might want to set this, and any other annotations that can be set. Before I merge can you squash your commits so I'm just merging a single commit? |
eaee117
to
d9a370f
Compare
Updated the explanation and squashed my commits. As far as I can tell, the other annotations are not supposed to be altered by users thus it doesn't make sense to document them or am I wrong there? |
Yes, I think you're right! The tests are now failing, I think you lost a commit when doing the squash? Hopefully you can get it back easily then I can merge. |
This may be useful if a nodes public IP can not determined, e.G. because it is behind a nat. Fixes flannel-io#712
Added the missing commit back. |
Description
This PR fixes #712 by allowing to overwrite the public ip of a Kubernetes node via the
flannel.alpha.coreos.com/public-ip-overwrite
annotation.Can anyone point me to the appropriate place for docs, is it just
Documentation/Configuration.md
?Right now the other annotations aren't documented in this repo.
Todos