On Wed, Jan 17, 2018 at 05:43:02PM +0100, Olivier Houchard wrote: > Ok you got me convinced, the attached patch don't check for duplicate > cookies for disabled server, until we enable them.
I also prefer this approach. > From cfc333d2b04686a3c488fdcb495cba64dbfec14b Mon Sep 17 00:00:00 2001 > From: Olivier Houchard <ohouch...@haproxy.com> > Date: Wed, 17 Jan 2018 17:39:34 +0100 > Subject: [PATCH] MINOR: servers: Don't report duplicate dyncookies for > disabled servers. > > Especially with server-templates, it can happen servers starts with a > placeholder IP, in the disabled state. In this case, we don't want to report > that the same cookie was generated for multiple servers. So defer the test > until the server is enabled. > > This should be backported to 1.8. > --- > src/server.c | 50 +++++++++++++++++++++++++++++++++++--------------- > 1 file changed, 35 insertions(+), 15 deletions(-) > > diff --git a/src/server.c b/src/server.c > index a37e91968..3901e7d8b 100644 > --- a/src/server.c > +++ b/src/server.c > @@ -86,10 +86,34 @@ int srv_getinter(const struct check *check) > return (check->fastinter)?(check->fastinter):(check->inter); > } > > -void srv_set_dyncookie(struct server *s) > +/* > + * Check that we did not get a hash collision. > + * Unlikely, but it can happen. > + */ > +static inline void srv_check_for_dup_dyncookie(struct server *s) > { > struct proxy *p = s->proxy; > struct server *tmpserv; > + > + for (tmpserv = p->srv; tmpserv != NULL; > + tmpserv = tmpserv->next) { > + if (tmpserv == s) > + continue; > + if (tmpserv->next_admin & SRV_ADMF_FMAINT) > + continue; > + if (tmpserv->cookie && > + strcmp(tmpserv->cookie, s->cookie) == 0) { > + ha_warning("We generated two equal cookies for two > different servers.\n" > + "Please change the secret key for '%s'.\n", > + s->proxy->id); > + } > + } > + > +} > + > +void srv_set_dyncookie(struct server *s) > +{ > + struct proxy *p = s->proxy; > char *tmpbuf; > unsigned long long hash_value; > size_t key_len; > @@ -136,21 +160,13 @@ void srv_set_dyncookie(struct server *s) > if (!s->cookie) > return; > s->cklen = 16; > - /* > - * Check that we did not get a hash collision. > - * Unlikely, but it can happen. > + > + /* Don't bother checking if the dyncookie is duplicated if > + * the server is marked as "disabled", maybe it doesn't have > + * its real IP yet, but just a place holder. > */ > - for (tmpserv = p->srv; tmpserv != NULL; > - tmpserv = tmpserv->next) { > - if (tmpserv == s) > - continue; > - if (tmpserv->cookie && > - strcmp(tmpserv->cookie, s->cookie) == 0) { > - ha_warning("We generated two equal cookies for two > different servers.\n" > - "Please change the secret key for '%s'.\n", > - s->proxy->id); > - } > - } > + if (!(s->next_admin & SRV_ADMF_FMAINT)) > + srv_check_for_dup_dyncookie(s); > } > > /* > @@ -4398,6 +4414,10 @@ static int cli_parse_enable_server(char **args, struct > appctx *appctx, void *pri > return 1; > > srv_adm_set_ready(sv); > + if (!(sv->flags & SRV_F_COOKIESET) > + && (sv->proxy->ck_opts & PR_CK_DYNAMIC) && > + sv->cookie) > + srv_check_for_dup_dyncookie(sv); > return 1; > } > > -- > 2.14.3 >