Re: [PATCH v3 0/5] cli commands for checks and agent
On Fri, Feb 12, 2021 at 3:06 PM Christopher Faulet wrote: > I just slightly amended the 3rd patch to handle the v2 in > apply_server_state(). > There is a test on the version when a state-file is local to a proxy. Just a > minor change. ok, thanks for that. > And in the last one, I removed the "chunk_appendf(msg, "\n");" to > move the LF in ha_warning() calls. yes ok it is probably clearer that way. Glad to see the end of this series, now I'm ready to receive related bug reports ;) -- William
Re: [PATCH v3 0/5] cli commands for checks and agent
Le 11/02/2021 à 22:51, William Dauchy a écrit :
Hello Christopher,
Here is some work to finish you week.
I believe I addressed all the points raised:
- warning are no longer emitted when we have "0" or "-" values
- I enhanced the warning output as well, and understood my mistake for
my previous CLEANUP patch removing a space... so I removed this patch
as well.
- Fixed all the chunk_appendf issues.
- I was a bit lazy to address the partial vs complete failure in parsing
as I was a bit puzzled about the approach to take. I think it would be
sad to duplicate the code for pre validation, but on the other hand I
agree it was clear to assume the whole line was not applied at all. I
however concluded it was acceptable to simply know the line was not
fully applied. I believe in that case the user should worry. We can
probably add a way to show where it stopped, but I felt discouraged
with the srv resolution, in the middle of srv port, where it is harder
to have a kinda generic way to know where we stopped.
William Dauchy (5):
MEDIUM: cli: add check-addr command
MEDIUM: cli: add agent-port command
MEDIUM: server: add server-states version 2
MEDIUM: server: support {check,agent}_addr, agent_port in server state
MINOR: server: enhance error precision when applying server state
Ok, it is good for me. I will push it soon. Thanks William! And don't be too
worry about the loading of server-state files. This part is a mess and should be
refactored, at least to be readable and also to fix bugs on error path. I'm
tempted to do so and a bit afraid too.
I just slightly amended the 3rd patch to handle the v2 in apply_server_state().
There is a test on the version when a state-file is local to a proxy. Just a
minor change. And in the last one, I removed the "chunk_appendf(msg, "\n");" to
move the LF in ha_warning() calls.
--
Christopher Faulet
[PATCH v3 0/5] cli commands for checks and agent
Hello Christopher,
Here is some work to finish you week.
I believe I addressed all the points raised:
- warning are no longer emitted when we have "0" or "-" values
- I enhanced the warning output as well, and understood my mistake for
my previous CLEANUP patch removing a space... so I removed this patch
as well.
- Fixed all the chunk_appendf issues.
- I was a bit lazy to address the partial vs complete failure in parsing
as I was a bit puzzled about the approach to take. I think it would be
sad to duplicate the code for pre validation, but on the other hand I
agree it was clear to assume the whole line was not applied at all. I
however concluded it was acceptable to simply know the line was not
fully applied. I believe in that case the user should worry. We can
probably add a way to show where it stopped, but I felt discouraged
with the srv resolution, in the middle of srv port, where it is harder
to have a kinda generic way to know where we stopped.
William Dauchy (5):
MEDIUM: cli: add check-addr command
MEDIUM: cli: add agent-port command
MEDIUM: server: add server-states version 2
MEDIUM: server: support {check,agent}_addr, agent_port in server state
MINOR: server: enhance error precision when applying server state
doc/management.txt| 15 +-
include/haproxy/server-t.h| 16 +-
.../checks/1be_40srv_odd_health_checks.vtc| 2 +-
.../checks/40be_2srv_odd_health_checks.vtc| 2 +-
reg-tests/checks/4be_1srv_health_checks.vtc | 6 +-
src/proxy.c | 41 +-
src/server.c | 971 ++
7 files changed, 612 insertions(+), 441 deletions(-)
--
2.30.0
