Yelp / nerve-tools

Tools for configuring SmartStack's Nerve based on local sources
Apache License 2.0
3 stars 12 forks source link

Fix multiple smartstack registrations #62

Closed kaisen closed 5 years ago

kaisen commented 5 years ago

Just check that an Envoy ingress port is set up for a local service's actual port when determining whether we should output Envoy nerve configs for a service. This ensures that services with multiple registrations will have all the registrations healthchecked.

Previously, a mapping of "{service_name}.{service_instance}.{local_host_port}" -> "Envoy ingress port" was used, but this would cause services with multiple registrations (like "service.main" and "service.alt_name") to only have one Envoy nerve config outputted. This is because Envoy's /listeners admin endpoint only outputs listener information under the listener name, and only one listener is set up for each service, even if it has multiple registrations.

By only checking the local host port, we lose some guarantees that the nerve configs will match the service name and instance. This can be a problem if another service gets assigned the same local host port by marathon after configure_nerve.py has already read information about locally running services, but the good thing is that healthchecks through Envoy require the correct Host header to be set, so in this case, the healthchecks will fail and the wrong service will never be registered in ZooKeeper. The next configure_nerve.py run will pick up the right information about locally running services and output the right Envoy nerve configs.