try both 4001 and 2379 etcd ports in SD if not configured #3351

xvello · 2017-05-29T13:07:37Z

What does this PR do?

if a port is configured, only try this one
else, try on 4001 then 2379

If connection fails, raise a ValueError that is caught in sd_docker_backend

Motivation

This allows a smooth update path to users that upgrade to new versions of etcd and didn't configure their etcd port for SD

Testing Guidelines

Tested manually with:

an invalid host (both ports fails, exception raised)
no conf and a server listening only on 2379 (4001 will fail, 2379 will succeed)
port 2379 configured (only this one is tried)

- if a port is configured, only try this one - else, try on 4001 then 2379 If connection fails, raise a ValueError that is caught in sd_docker_backend

hkaj

nice, lgtm

handle both 4001 and 2379 etcd ports in SD if not configured

58d3935

- if a port is configured, only try this one - else, try on 4001 then 2379 If connection fails, raise a ValueError that is caught in sd_docker_backend

xvello requested a review from hkaj May 29, 2017 13:08

xvello added this to the 5.15 milestone May 29, 2017

hkaj approved these changes May 29, 2017

View reviewed changes

xvello merged commit d290afe into master Jun 6, 2017

xvello deleted the xvello/sd_etcd_port_2379 branch June 6, 2017 11:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

try both 4001 and 2379 etcd ports in SD if not configured #3351

try both 4001 and 2379 etcd ports in SD if not configured #3351

xvello commented May 29, 2017 •

edited

hkaj left a comment

try both 4001 and 2379 etcd ports in SD if not configured #3351

try both 4001 and 2379 etcd ports in SD if not configured #3351

Conversation

xvello commented May 29, 2017 • edited

What does this PR do?

Motivation

Testing Guidelines

hkaj left a comment

Choose a reason for hiding this comment

xvello commented May 29, 2017 •

edited