Skip to content
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

Adding support for setting the virtual IP that points to the control plane #5

Merged
merged 6 commits into from
May 13, 2019

Conversation

hyperbolic2346
Copy link
Contributor

This allows for someone with an external load balancer to specify the IP that the workers will use to talk to the control plane.

@hyperbolic2346
Copy link
Contributor Author

@Cynerva
Copy link
Contributor

Cynerva commented Apr 15, 2019

This allows for someone with an external load balancer to specify the IP that the workers will use to talk to the control plane.

Not quite. This affects kubeconfig files generated on kubernetes-master, but not on kubernetes-worker. The kubernetes-worker charm creates its own kubeconfigs using addresses it receives over the kube-api-endpoint relation, which might be related directly to kubernetes-master, but is usually related to kubeapi-load-balancer instead.

Added a config change hook to kick off changes instead of waiting for the next hook run
Fixed a minor flake8 issue
Added a check for empty data, which can occur if the relation is going away
@hyperbolic2346
Copy link
Contributor Author

A good point, sir. I've updated the PR to include updating the relation data. I was on the fence about doing something like this for the kube-api-loadbalancer charm, but it is possible to have a virtual IP floating on the load balancers outside of juju and wanting to use that. As a result, I am going to add a similar PR to the load balancer charm. This will only work if there isn't a load balancer. I updated the config.yaml to spell that out.

@evilnick I would love your input on the wording on the config.yaml section if you have any.

Note that all units will send all the IPs, but the interface will cull duplicates.
Pulling a fix for a condition in favor of a bug and fix just for that.
Pulling config changed handler since we don't need it as cynerva pointed out in review
@Cynerva
Copy link
Contributor

Cynerva commented Apr 29, 2019

LGTM

@Cynerva
Copy link
Contributor

Cynerva commented Apr 29, 2019

Feel free to merge this. I left it unmerged in case you wanted to wait for feedback on the config description.

config.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@evilnick evilnick left a comment

Choose a reason for hiding this comment

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

Only one spelling mistake though, so 🍖

config.yaml Outdated Show resolved Hide resolved
@tvansteenburgh
Copy link
Contributor

@hyperbolic2346 We should add something about this to the High Availability section of the docs (https://www.ubuntu.com/kubernetes/docs/high-availability)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants