On Thu, Jun 22, 2017 at 08:55:03AM -0700, Joseph Lynch wrote:
> > If it's about the stats page, I think we'd rather emit the FQDN there,
> > what do you think ?
> It is about the stats page, but also about our automation's ability to
> take information from a service registry (which has host+port pairs +
> health information) and identify which server in a HAProxy backend
> maps to that service instance.
>
> For example "enable server <b>/<s>" or "disable server <b>/<s>" has to
> take the name, so if we can't rely on the server name being some
> combination of host+port then for every up/down we'd have to "show
> stat" and map from host+port to name for each update (as opposed to
> issuing one socket "disable server". We also have lots of automation
> (e.g. synapse, service monitoring scripts) which parse out the svname
> to find the host+port, which while I can fix, is rather difficult.
> Also there are a few stat commands (like "show servers state") which
> don't show the host+port and only show the name.
The server state is a perfect example of huge breakage that would be
caused by such a change. In short, you start with the names from the
configuration file, which are looked up in the state file, then the
names are changed, and once dumped, they are different. So upon next
reload, nothing matches at all.
> > I'm having a few problems with changing the server's name :
> > - it's a unique identifier, so changing it after unicity has been
> > verified can be a problem and can become confusing in logs, stats
> > or anything else.
> I could verify that the name is unique perhaps? I don't feel like
> logs/stats would be an issue because the only time you'd update the
> name would be if the server is in MAINT (basically a free slot to
> dynamically update to a new server)? Also logs usually have the actual
> address information in addition to the name?
For me it's a more general problem of inconsistency between the configuration
and what is running. For example if you have this :
backend foo
use-server srv1 if rule1
use-server srv2 if rule2
server srv1 1.1.1.1:80
server srv2 2.2.2.2:80
Do you know what happens once you'll switch the names over the CLI ? (hint:
I know). And it will even be inconsistent with what use_backend does because
use_backend *can* resolve at runtime instead of config time. The logs will
return the current name depending on the last change, possibly conflicting
with what is in the configuration. Another example :
backend foo1
server srv1 1.1.1.1:80 track foo2/srv1
server srv2 2.2.2.2:80 track foo2/srv2
backend foo2
server srv1 1.1.1.1:80 check
server srv2 2.2.2.2:80 check
Here after a renaming of the tracked servers, it becomes a mess, you'll
constantly have to know what's taken at startup time and what's taken at
runtime.
(...)
> > Given your intent is to provide human-readable information and that the
> > server name is a config primary key (not always the most readable one
> > by the way), better add a "desc <name>" config option that you can change
> > at will on the CLI (set server <b>/<s> desc <desc>), and add it to the
> > stats page (only when stats-show-desc is set), and let's add a sample
> > fetch function to return it (logs, headers, anything). What do you think ?
> I'll try it out, but that is a larger change that has backwards
> compatibility concerns,
On the opposite, there's zero backwards compatibility concerns to have
with adding a description field because we don't have it for servers for
now, only for backends and frontends.
> and I'm still concerned about how does
> automation go from address to name going forward.
Just a question, wouldn't your requirement in fact be to lookup a server
by its address ? I mean, wouldn't it be easier for your use case ? If so
I'm pretty sure that's something we could easily apply. Either we apply
this to the first server that matches the address, or we apply it to all
those which match, depending on the difficulty.
For example wouldn't it be easier for you to be able to do :
set server addr bk/@1.1.1.1:80 2.2.2.2:80
or :
disable server bk/@1.1.1.1:80
?
> If I verified that
> the name was unique would that be an ok solution?
As explained above for me it's far from being a valid solution due to the
major inconsistencies between the configuration, the state file, the logs,
the stats etc... In the end everything suddenly becomes unobservable and
undebuggable, and state file cannot be used anymore.
I'm pretty sure you have a very valid use case that we probably need to
discuss more to find the largest intersection with what can reasonably be
done here without breaking existing setups. We know that the management
socket needs to be significantly improved for some maintenance operations,
so it's even more important to know what your ideal usage scenario would
be to see how it could be mapped here.
> >> +const char *update_server_name(struct server *server, const char *name,
> >> const char *updater)
> >> +{
> >> + struct chunk *msg;
> >> +
> >> + msg = get_trash_chunk();
> >> + chunk_reset(msg);
> >> +
> >> + if (!strcmp(name, server->id)) {
> >> + chunk_appendf(msg, "no need to change the name");
> >> + goto out;
> >> + }
> >> +
> >> + char* old_name = server->id;
> >
> > This will not build, please avoid inline declarations.
> I thought I declared it in /include/proto/server.h, oops I will double check!
It's unrelated, I meant the "char *old_name" in the middle of the code.
Cheers!
Willy
