Fix typo in the Note section of load balancer allocator #1835
Conversation
| @@ -83,4 +83,4 @@ When running the controller outside a pod, both `POD_NAME` and `POD_NAMESPACE` m | |||
|
|
|||
| ## Notes | |||
|
|
|||
| It's not possible to specify the addresses for the load balancer services. A externalIP service can be used instead. | |||
| If it is not possible to specify the addresses for the load balancer services, an [externalIP service](https://kubernetes.io/docs/concepts/services-networking/service/#external-ips) can be used instead. | |||
There was a problem hiding this comment.
Most of this edit is fine, however, we should not add an If to the front of this. It's not up to the user on whether or not to specify an IP address on the Load Balancer. kube-router currently doesn't allow for this in its LoadBalancer implementation.
If users want to have full control over the IP address a service gets, then they must use an ExternalIP.
There was a problem hiding this comment.
Oh! In my case I assigned an ip address in the loadBalancerIP field of the service manifest and saw that the IP got shown under the "External IP" column. Hence I thought in some cases it works. Then this Pull Request is not even required I think.
There was a problem hiding this comment.
Hmm... I don't see any handling of the loadBalancerIP field in the actual Load Balancer Allocation controller (https://github.com/cloudnativelabs/kube-router/blob/master/pkg/controllers/lballoc/lballoc.go) so I'd be a bit surprised if this is working the way that you're expecting.
That being said, I see references to it in other controllers like the services controller, so its possible that its just working anyway even without the Load Balancer Allocator handling it properly.
Are you able to show me the full YAML of your service? It would be kind of interesting to see how its setup to see if I can make more sense of it.
|
Thanks for reading the docs and contributing! I left one comment, and also as a slight nitpic, it would be nice if you could update your commit message to match the existing commit message structure of this repo. Something like |
I think the Notes section below had a typo. This PR fixes that.