d2iq-archive / mesos-dns

DNS-based service discovery for Mesos.
https://mesosphere.github.com/mesos-dns
Apache License 2.0
483 stars 137 forks source link

add non-zero weight for returned SRV records #515

Closed lamdor closed 6 years ago

lamdor commented 6 years ago

If this is zero, services like haproxy see that as a backend that's in maintenance. I couldn't see any problem with making it non-zero.

mesosphere-ci commented 6 years ago

Can one of the admins verify this patch?

jdef commented 6 years ago

ok to test

jdef commented 6 years ago

thanks @rubbish can you provide a link to the relevant haproxy docs?

lamdor commented 6 years ago

@jdef I couldn't find it the haproxy docs (https://cbonte.github.io/haproxy-dconv/1.8/configuration.html#5.3.1), this is something I found in my own testing.

But looking at the haproxy source, http://git.haproxy.org/?p=haproxy-1.8.git;a=blob;f=src/dns.c;h=a957710edb2f92e31069844a15338cc8834e8402;hb=HEAD#l527 it has:

Make sure weight is at least 1, so that the server will be used.

jdef commented 6 years ago

OK, thanks. My gut says that the default weight should be configurable. Maybe we make that default weight 1 so that SRVs play nice with haproxy by default. The reason we make it configurable is so that people can roll back to using 0 if the new default of 1 causes problems in their cluster.

How does that sound?

On Fri, Jan 26, 2018 at 12:02 PM, Luke Amdor notifications@github.com wrote:

@jdef https://github.com/jdef I couldn't find it the haproxy docs ( https://cbonte.github.io/haproxy-dconv/1.8/configuration.html#5.3.1), this is something I found in my own testing.

But looking at the haproxy source, http://git.haproxy.org/?p= haproxy-1.8.git;a=blob;f=src/dns.c;h=a957710edb2f92e31069844a15338c c8834e8402;hb=HEAD#l527 it has:

Make sure weight is at least 1, so that the server will be used.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mesosphere/mesos-dns/pull/515#issuecomment-360843091, or mute the thread https://github.com/notifications/unsubscribe-auth/ACPVLLkVXNkAtiZDNsjCVHTcwtWX7D2bks5tOgVDgaJpZM4RujvT .

lamdor commented 6 years ago

@jdef Sounds good. Added in 2f04f61

lamdor commented 6 years ago

should be gtg now

jdef commented 6 years ago

retest this please