Re: Add agent-host configuration directive and allow changing it and agent-send via socket/CLI
Hi! On Mon, Jan 16, 2017 at 10:57:12AM +0100, Micha?? wrote: > Hello! > Thank you for reviewing. For me agent-addr looks better too, I hope it > won't > be confused with "addr" directive. > > So here are patches with "agent-addr" changes and I added extebded commit > messages to code commits. > > I wrote "Can be backported", because those are not bugfix'es, because they > don't > have to be backported, but on the other hand upgrading to 1.8 won't be > near-future for many projects and backport to 1.7 will spread this feature > to > more people. OK, thank you, I've applied all the series now. Regarding the backport, I prefer not to do it. The reason is that some distros will start to ship with 1.7 soon, and some people might find some articles on the net suggesting to do something with certain options that don't exist for them. We've faced this situation a lot in the past during 1.3 and 1.4 and that's really not nice for distro maintainers who are bugged with wrong reports. Now we release much more often and limit the backport to fixes. However I've kept your comment saying that it can be backported because if someone wants to add your series into his/her tree, it's useful to know that it can be done without risks. Thanks! Willy
Re: Add agent-host configuration directive and allow changing it and agent-send via socket/CLI
Hello! Thank you for reviewing. For me agent-addr looks better too, I hope it won't be confused with "addr" directive. So here are patches with "agent-addr" changes and I added extebded commit messages to code commits. I wrote "Can be backported", because those are not bugfix'es, because they don't have to be backported, but on the other hand upgrading to 1.8 won't be near-future for many projects and backport to 1.7 will spread this feature to more people. Patches files md5: MD5 (./0001-PATCH-MINOR-checks-Add-agent-addr-config-directive.patch) = 555a04df5dc56fa44ab94cb321df469b MD5 (./0002-PATCH-MINOR-cli-Add-possiblity-to-change-agent-confi.patch) = 4b1d732dff72a3349766d74a7f28dcd6 MD5 (./0003-PATCH-MINOR-doc-Add-docs-for-agent-addr-configuratio.patch) = 2421ff01f26936e59461955b4ea5684f MD5 (./0004-PATCH-MINOR-doc-Add-docs-for-agent-addr-and-agent-se.patch) = 971d6c1d23657130db4045d5b9e8cb73 Michał 0001-PATCH-MINOR-checks-Add-agent-addr-config-directive.patch Description: Binary data 0002-PATCH-MINOR-cli-Add-possiblity-to-change-agent-confi.patch Description: Binary data 0003-PATCH-MINOR-doc-Add-docs-for-agent-addr-configuratio.patch Description: Binary data 0004-PATCH-MINOR-doc-Add-docs-for-agent-addr-and-agent-se.patch Description: Binary data
Re: Add agent-host configuration directive and allow changing it and agent-send via socket/CLI
Hi Michal, On Mon, Jan 09, 2017 at 02:00:19PM +0100, Micha?? wrote: > Hello! > 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 > agent-send > variables via CLI/socket. We wonna use this options to dynamically swap > backends > without reloading haproxy. agent-host will allow us to control weight and > status > of servers from single place, so we can enable server when it's ready > (warmed) > 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 welcome! > 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. Thanks! Willy
Add agent-host configuration directive and allow changing it and agent-send via socket/CLI
Hello!
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
agent-send
variables via CLI/socket. We wonna use this options to dynamically swap
backends
without reloading haproxy. agent-host will allow us to control weight and
status
of servers from single place, so we can enable server when it's ready
(warmed)
instead just after healthcheck passing. I think this change isn't touching
any
hot paths and will find adopters, 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.
In attachments there are same patches as below, just in .patch format in
case
mail get corrupted.
MD5 (0001-Add-agent-host-config-directive.patch) =
37a3c156b7a7d213f25b1ea52e37df10
MD5 (0002-Add-possiblity-to-change-agent-config-via-CLI-socket.patch) =
f9919fa73a21bd0a178aa301e1585b73
MD5 (0003-Add-docs-for-agent-host-configuration-variable.patch) =
f7d54b0aaef0ea5034ed80193d6f40b9
MD5 (0004-Add-docs-for-agent-host-and-agent-send-CLI-commands.patch) =
8cb920bbab07d964ae5f6d7c6e1c830b
---
src/server.c | 8
1 file changed, 8 insertions(+)
diff --git a/src/server.c b/src/server.c
index 2f539c9..0ca372a 100644
--- a/src/server.c
+++ b/src/server.c
@@ -1153,6 +1153,14 @@ int parse_server(const char *file, int linenum, char
**args, struct proxy *curpr
newsrv->agent.inter = val;
cur_arg += 2;
}
+ else if (!strcmp(args[cur_arg], "agent-host")) {
+ if(str2ip(args[cur_arg + 1], >agent.addr) == NULL) {
+ Alert("parsing agent-host failed. Check if %s is
correct address.\n", args[cur_arg + 1]);
+ goto out;
+ }
+
+ cur_arg += 2;
+ }
else if (!strcmp(args[cur_arg], "agent-port")) {
global.maxsock++;
newsrv->agent.port = atol(args[cur_arg + 1]);
--
---
doc/configuration.txt | 8
1 file changed, 8 insertions(+)
diff --git a/doc/configuration.txt b/doc/configuration.txt
index ae76ef3..088bba6 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -10684,6 +10684,14 @@ agent-inter
Supported in default-server: Yes
+agent-host
+ The "agent-host" parameter sets address for agent check.
+
+ You can offload agent-check to another target, so you can make single
place
+ managing status and weights of servers defined in haproxy in case you
can't
+ make self-aware and self-managing services. You can specify both IP or
+ hostname, it will be resolved.
+
agent-port
The "agent-port" parameter sets the TCP port used for agent checks.
--
---
src/server.c | 27 +++
1 file changed, 27 insertions(+)
diff --git a/src/server.c b/src/server.c
index 0ca372a..1e6810c 100644
--- a/src/server.c
+++ b/src/server.c
@@ -3467,6 +3467,33 @@ static int cli_parse_set_server(char **args, struct
appctx *appctx, void *privat
appctx->st0 = CLI_ST_PRINT;
}
}
+ else if (strcmp(args[3], "agent-host") == 0) {
+ if(!(sv->agent.state & CHK_ST_ENABLED)) {
+ appctx->ctx.cli.msg = "agent checks are not enabled on this
server.\n";
+ appctx->st0 = CLI_ST_PRINT;
+ } else {
+ if(str2ip(args[4], >agent.addr) == NULL) {
+ appctx->ctx.cli.msg = "incorrect host address given for
agent.\n";
+ appctx->st0 = CLI_ST_PRINT;
+ }
+ }
+ }
+ else if(strcmp(args[3], "agent-send") == 0) {
+ if(!(sv->agent.state & CHK_ST_ENABLED)) {
+ appctx->ctx.cli.msg = "agent checks are not enabled on this
server.\n";
+ appctx->st0 = CLI_ST_PRINT;
+ } else {
+ char *nss = strdup(args[4]);
+ if(!nss) {
+ appctx->ctx.cli.msg = "cannot allocate memory for new
string.\n";
+ appctx->st0 = CLI_ST_PRINT;
+ } else {
+ free(sv->agent.send_string);
+ sv->agent.send_string = nss;
+ sv->agent.send_string_len = strlen(args[4]);
+ }
+ }
+ }
else if (strcmp(args[3], "check-port") == 0) {
int i = 0;
if (strl2irc(args[4], strlen(args[4]), ) != 0) {
--
---
doc/management.txt | 9 +
1 file changed, 9 insertions(+)
diff --git a/doc/management.txt b/doc/management.txt
index dd886de..ec4490f 100644
--- a/doc/management.txt
+++ b/doc/management.txt
@@ -1603,6 +1603,15 @@ set server / agent [ up | down ]
switch a server's state regardless of some slow agent checks for example.
Note that the change is propagated to tracking servers if any.
+set server / agent-host
+ Change addr for servers agent checks. Allows to migrate agent-checks
