sync endpoints for serviceType=LoadBalancer

[ltagliamonte-dd]

We are moving to a flat network using awscni and we will would like to sync in Consul all pods endpoints also the one that atm are type=LoadBalancer.
is there any issue of doing that?
if you are ok i can open a PR for enhance this:
https://github.com/hashicorp/consul-k8s/blob/master/catalog/to-consul/resource.go#L298

[ishustava]

Hey @ltagliamonte-dd,

The main issue I see with that is that it's not clear which address we should register with Consul when we see a service of type LoadBalancer: the ingress IP/Hostname or the endpoints. Right now, the catalog sync assumes you want a routable address to be in Consul, and so if you have a load balancer service, we think that the Load Balancer address should be registered rather than the load balancer endpoints.

If you are switching to a flat network, then all the registered load balancer addresses would still work, so you can switch them eventually by changing a service type to NodePort or ClusterIP over time.

Hope this makes sense.

[ltagliamonte-dd]

thank you for the reply @ishustava but my situation is a bit more complicated and i need to maintain backward compatibility i need to gradually migrate clients to the new consul domain so i need new client address a service via routable IPs and old services still use the load balancer endpoint.

[ishustava]

Got it, thanks for a quick reply!

Do you maybe have an example that you could share with us showing the use-case?

Or maybe you could outline what you're proposing a bit more? I'm mainly confused about how would the sync process decide which address to register when it sees a load balancer service.

[ltagliamonte-dd]

hello @ishustava thank you for the quick reply.

my use cases:
got a mixed env ECS and k8s, ECS services are calling into k8s via ELB.
services in k8s are on flat network and i want for those one to be able to reach directly service in other k8s clusters.

to maintain consul-k8s backward compatibility i propose to add an additional flag like sync-lb-endpoints set to false by default.
What do you think?

[ltagliamonte-dd]

@ishustava i'm thinking of something like this:
master...ltagliamonte-dd:master

just did the make test and it seems that it doesn't break anything, i'm going to test it and if it does work will open a PR. What do you think?

[ltagliamonte-dd]

@ishustava i tested my changes and it looks like it is working, what do you think?

[lkysow]

Hi @ltagliamonte-dd it's a valid use-case so please go ahead and submit a PR but just a warning that we're really backed up on PR reviews so it likely won't get reviewed for a while.

[ltagliamonte-dd]

@lkysow @ishustava PR opened here #257