Re: [PATCH] [MEDIUM] dns: Add resolve-opts "ignore-weight"
On Thu, Nov 21, 2019 at 05:18:58PM +0100, Willy Tarreau wrote: > On Thu, Nov 21, 2019 at 02:12:09PM +0100, Baptiste wrote: > > Hi there, > > > > Since a short term reliable solution can't be found, we can apply this > > patch as a workaround. > > Yep, as discussed on the github issue I think it remains the most > reasonable short-term approach. Patch now merged, thanks guys. Willy
Re: [PATCH] [MEDIUM] dns: Add resolve-opts "ignore-weight"
On Thu, Nov 21, 2019 at 02:12:09PM +0100, Baptiste wrote: > Hi there, > > Since a short term reliable solution can't be found, we can apply this > patch as a workaround. Yep, as discussed on the github issue I think it remains the most reasonable short-term approach. Thanks, Willy
Re: [PATCH] [MEDIUM] dns: Add resolve-opts "ignore-weight"
Hi there, Since a short term reliable solution can't be found, we can apply this patch as a workaround. Baptiste >
Re: [PATCH] [MEDIUM] dns: Add resolve-opts "ignore-weight"
On Mon, Nov 18, 2019 at 2:37 PM Daniel Corbett wrote: > Hello, > > > On 11/18/19 7:05 AM, Willy Tarreau wrote: > > On Mon, Nov 18, 2019 at 12:06:08PM +0100, Baptiste wrote: > >> When we first designed this feature, we did it with this in mind "if > admins > >> can update a SRV record in a DNS server, they can adjust the weight > >> accordingly". > >> > >> I understand the need, but the response is way too short. It's a global > >> question of precedence in HAProxy from my point of view. > >> I am scared that if we start to adjust things this way, we'll end up > with > >> 1000s of flags overlapping each others and adding complexity on top of > >> complexity. > >> > >> The real question is "what prevents an admin from updating a DNS > record?" > >> Or why they don't failover to A/ records only? > > I must admit I understand a valid use case : have the DNS set up to > advertise > > the list of servers, and let the agent adjust the servers' health based > on > > their load, the fact that they're running backup or OS updates etc. Thus > in > > my opinion, the *use case* makes sense. What I'm unsure about is the > proper > > way to do it, because as you mention, it's more a matter of overall > > consistency between all sources. We could very well instead have a > per-backend > > setting indicating what source to fetch the weight from (agent, dns, > health, > > other?), where to fetch the maxconn from etc. Some may even want to > combine > > these (average, multiply, ...). I'm fine if you prefer to postpone it. > If in > > the end we decide to merge it as-is we could also backport it, and if we > > decide to address it differently, at least we won't have to maintain one > > extra short-lived flag. > > > > Thanks, > > Willy > > > I'm open to ideas on implementation method, definitely not stuck on this > method :)To be honest I was trying to find some "good first issues" > to tackle. > > GitHub request here: https://github.com/haproxy/haproxy/issues/48 > > > Thanks for taking the time to review and provide your input guys! > > > Thanks, > > -- Daniel > > > I replied back on the github issue to re-start conversation on the topic. Based on the answer, I'll give you my go or not :) If the go happens after the release, we can still backport this quick change if this is really useful to people. Baptiste
Re: [PATCH] [MEDIUM] dns: Add resolve-opts "ignore-weight"
Hello, On 11/18/19 7:05 AM, Willy Tarreau wrote: On Mon, Nov 18, 2019 at 12:06:08PM +0100, Baptiste wrote: When we first designed this feature, we did it with this in mind "if admins can update a SRV record in a DNS server, they can adjust the weight accordingly". I understand the need, but the response is way too short. It's a global question of precedence in HAProxy from my point of view. I am scared that if we start to adjust things this way, we'll end up with 1000s of flags overlapping each others and adding complexity on top of complexity. The real question is "what prevents an admin from updating a DNS record?" Or why they don't failover to A/ records only? I must admit I understand a valid use case : have the DNS set up to advertise the list of servers, and let the agent adjust the servers' health based on their load, the fact that they're running backup or OS updates etc. Thus in my opinion, the *use case* makes sense. What I'm unsure about is the proper way to do it, because as you mention, it's more a matter of overall consistency between all sources. We could very well instead have a per-backend setting indicating what source to fetch the weight from (agent, dns, health, other?), where to fetch the maxconn from etc. Some may even want to combine these (average, multiply, ...). I'm fine if you prefer to postpone it. If in the end we decide to merge it as-is we could also backport it, and if we decide to address it differently, at least we won't have to maintain one extra short-lived flag. Thanks, Willy I'm open to ideas on implementation method, definitely not stuck on this method :) To be honest I was trying to find some "good first issues" to tackle. GitHub request here: https://github.com/haproxy/haproxy/issues/48 Thanks for taking the time to review and provide your input guys! Thanks, -- Daniel
Re: [PATCH] [MEDIUM] dns: Add resolve-opts "ignore-weight"
On Mon, Nov 18, 2019 at 12:06:08PM +0100, Baptiste wrote: > When we first designed this feature, we did it with this in mind "if admins > can update a SRV record in a DNS server, they can adjust the weight > accordingly". > > I understand the need, but the response is way too short. It's a global > question of precedence in HAProxy from my point of view. > I am scared that if we start to adjust things this way, we'll end up with > 1000s of flags overlapping each others and adding complexity on top of > complexity. > > The real question is "what prevents an admin from updating a DNS record?" > Or why they don't failover to A/ records only? I must admit I understand a valid use case : have the DNS set up to advertise the list of servers, and let the agent adjust the servers' health based on their load, the fact that they're running backup or OS updates etc. Thus in my opinion, the *use case* makes sense. What I'm unsure about is the proper way to do it, because as you mention, it's more a matter of overall consistency between all sources. We could very well instead have a per-backend setting indicating what source to fetch the weight from (agent, dns, health, other?), where to fetch the maxconn from etc. Some may even want to combine these (average, multiply, ...). I'm fine if you prefer to postpone it. If in the end we decide to merge it as-is we could also backport it, and if we decide to address it differently, at least we won't have to maintain one extra short-lived flag. Thanks, Willy
Re: [PATCH] [MEDIUM] dns: Add resolve-opts "ignore-weight"
On Mon, Nov 18, 2019 at 11:57 AM Willy Tarreau wrote: > Hi Daniel, > > On Sun, Nov 17, 2019 at 10:06:32AM -0500, Daniel Corbett wrote: > > Hello, > > > > > > I realize that new features are not preferred at the moment but I think > this > > might be a usability issue and hopefully it can be considered for > 2.1-dev, > > however, it's perfectly fine if it's decided to wait till next. > > > > It was noted in GitHub issue #48 that there are times when a > configuration > > may use the server-template directive with SRV records and simultaneously > > want to control weights separately using an agent-check or through the > > runtime api. This patch adds a new option "ignore-weight" to the > > "resolve-opts" directive. > > > > When specified, any weight indicated within an SRV record will be > ignored. > > This is for both initial resolution and ongoing resolution. > > In my opinion it's small enough to be mergeable. However I have no opinion > whether this is the best way to handle it or not, so I'll leave it to > others > to judge. For example it could be imagined that some would want to keep the > weight zero as special to take a server out of the farm maybe (though this > could complicate the logic). If others agree with the patch, I'm fine with > getting it merged even this late given that it doesn't seem to have side > effects. > > > I wanted to include VTC test with this, however, I could not think of an > > appropriate way to do it as I suspect we may need a "fake dns server" > > similar to what was made for syslog. > > vtest is really made to test proxies, i.e. have a client on one side, a > server on the other one, and synchronize them to make sure that what is > observed precisely corresponds to what is tested, which is the hardest > part to test on a proxy. It's really not suitable to run other types of > tests at the moment. Maybe we could imagine improving it to implement a > DNS responder, but even by doing this we'd start to add some entropy > (timing between checks causing ordering issues) making the tests harder > to reproduce. > > If we want to implement some testability for anything related to DNS, > we'll need to specify in very fine details how we want it to work I > guess. > > Thanks, > Willy > > Hi, When we first designed this feature, we did it with this in mind "if admins can update a SRV record in a DNS server, they can adjust the weight accordingly". I understand the need, but the response is way too short. It's a global question of precedence in HAProxy from my point of view. I am scared that if we start to adjust things this way, we'll end up with 1000s of flags overlapping each others and adding complexity on top of complexity. The real question is "what prevents an admin from updating a DNS record?" Or why they don't failover to A/ records only? Baptiste
Re: [PATCH] [MEDIUM] dns: Add resolve-opts "ignore-weight"
Hi Daniel, On Sun, Nov 17, 2019 at 10:06:32AM -0500, Daniel Corbett wrote: > Hello, > > > I realize that new features are not preferred at the moment but I think this > might be a usability issue and hopefully it can be considered for 2.1-dev, > however, it's perfectly fine if it's decided to wait till next. > > It was noted in GitHub issue #48 that there are times when a configuration > may use the server-template directive with SRV records and simultaneously > want to control weights separately using an agent-check or through the > runtime api. This patch adds a new option "ignore-weight" to the > "resolve-opts" directive. > > When specified, any weight indicated within an SRV record will be ignored. > This is for both initial resolution and ongoing resolution. In my opinion it's small enough to be mergeable. However I have no opinion whether this is the best way to handle it or not, so I'll leave it to others to judge. For example it could be imagined that some would want to keep the weight zero as special to take a server out of the farm maybe (though this could complicate the logic). If others agree with the patch, I'm fine with getting it merged even this late given that it doesn't seem to have side effects. > I wanted to include VTC test with this, however, I could not think of an > appropriate way to do it as I suspect we may need a "fake dns server" > similar to what was made for syslog. vtest is really made to test proxies, i.e. have a client on one side, a server on the other one, and synchronize them to make sure that what is observed precisely corresponds to what is tested, which is the hardest part to test on a proxy. It's really not suitable to run other types of tests at the moment. Maybe we could imagine improving it to implement a DNS responder, but even by doing this we'd start to add some entropy (timing between checks causing ordering issues) making the tests harder to reproduce. If we want to implement some testability for anything related to DNS, we'll need to specify in very fine details how we want it to work I guess. Thanks, Willy
