On Mon, Jan 09, 2017 at 02:00:19PM +0100, Micha?? wrote:
> It's my first PR to haproxy, so please tell me if anything still wrong.
> I've read CONTRIBUTING.
> This patches implements possiblity to define different host (agent-host) for
> agent checks in config and they also allow changing agent-host and
> variables via CLI/socket. We wonna use this options to dynamically swap
> without reloading haproxy. agent-host will allow us to control weight and
> of servers from single place, so we can enable server when it's ready
> instead just after healthcheck passing.
I remember a recent discussion on this subject, I don't remember if it was
here on the list or with Baptiste or someone else, but that's definitely
> I think this change isn't touching any
> hot paths and will find adopters,
I think it will be useful to make it easier to control server statuses from
a central place.
> because reloading haproxy with all SSL stuff
> and losing statstics for less dynamic backends is pain.
> In my opinion changes in code don't require any comments. I documented those
> features of course, so everyone can use them.
Indeed, however a short description of the changes in the commit messages
would help. Please keep in mind that merged patches are review again later :
- when backporting fixes and the rare few minor improvements
- when bisecting to find the cause of a bug.
In this case it's really important that all the useful information is
visible in the git log to help take the appropriate decision (eg: backport
yes/no, or this is totally unrelated to my problem).
I'd personally prefer if the "agent-host" was renamed to "agent-addr" to
be more consistent with the "addr" parameter we already have for the
checks and which is used at a few places on the CLI. Also in the HTTP
world, "host" is a bit connoted as the string found in the header field
carrying the same name :-)
Otherwise your changes look fine, I'm willing to merge them, I don't
see any risk there.