MEDIUM: server: add 'set server name' CLI command for runtime server renaming

Add the ability to rename a HAProxy server at runtime via the CLI:

  set server <backend>/<server> name <newname>

This is useful in slot-based dynamic scaling setups where servers are
pre-allocated with generic names (e.g. srv001, srv002) but the operator
wants the names to reflect the current workload (e.g. pod name or
IP:port) for observability and server-state-file consistency.

The implementation:
  - validates the new name: non-empty, passes invalid_char() check
    (allows [A-Za-z0-9_:.-]), and fits in the event data name field
  - requires the server to be administratively in maintenance mode
    (same precondition as 'del server')
  - rejects the rename if the server has SRV_F_NAME_REFD set (use-server
    target, track target, sample-fetch ARGT_SRV referent) - keeps the
    running state consistent with the configuration text
  - re-indexes the server in the name tree under thread_isolate(),
    mirroring the locking pattern used by 'add server' / 'del server'
  - publishes a new EVENT_HDL_SUB_SERVER_NAME event with the old and
    new names so downstream consumers (logs, observability backends)
    can track the rename
  - frees the old name immediately under thread isolation: srv_name
    sample consumers (ACLs, log formats, ...) act on the fetched pointer
    within the current task and do not retain it across wake-ups, so
    no extra deferred-free machinery is needed

There is no opt-in directive: like 'add server' and 'del server', the
operation is gated by the server's properties rather than by a
per-backend toggle. This avoids the runtime-surprise failure mode
where an operator discovers at the CLI that renaming is forbidden by
a missing 'option server-rename' rather than by an actual structural
reference.

This feature was discussed in:
  https://github.com/haproxy/haproxy/issues/952

diff --git a/include/haproxy/event_hdl-t.h b/include/haproxy/event_hdl-t.h
index d969291447b6fbb11e54acdfa879e6508fbd463b..e15bc3f958fd31fbadc59d7e49219be50f437893 100644 (file)
--- a/include/haproxy/event_hdl-t.h
+++ b/include/haproxy/event_hdl-t.h
@@ -279,6 +279,8 @@ struct event_hdl_sub {
 #define EVENT_HDL_SUB_SERVER_CHECK                      EVENT_HDL_SUB_TYPE(1,7)
 /* server inet addr (addr:svc_port tuple) change event */
 #define EVENT_HDL_SUB_SERVER_INETADDR                   EVENT_HDL_SUB_TYPE(1,8)
+/* server name change event */
+#define EVENT_HDL_SUB_SERVER_NAME                       EVENT_HDL_SUB_TYPE(1,9)
 
 /* PAT_REF family, only published in pat ref subscription list
  * (not published in global subscription list for performance reasons)

diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h
index ca332d4a1469d6affa2bc224f063530099bcee42..c2bd6c7db9d5662badae0a9e6491c93e989ad023 100644 (file)
--- a/include/haproxy/server-t.h
+++ b/include/haproxy/server-t.h
@@ -339,6 +339,7 @@ struct path_parameters {
 };
 
 struct proxy;
+
 struct server {
        /* mostly config or admin stuff, doesn't change often */
        enum obj_type obj_type;                 /* object type == OBJ_TYPE_SERVER */
@@ -572,6 +573,7 @@ struct event_hdl_cb_data_server {
         *   EVENT_HDL_SUB_SERVER_ADMIN
         *   EVENT_HDL_SUB_SERVER_CHECK
         *   EVENT_HDL_SUB_SERVER_INETADDR
+        *   EVENT_HDL_SUB_SERVER_NAME
         */
        struct {
                /* safe data can be safely used from both
@@ -775,6 +777,24 @@ struct event_hdl_cb_data_server_inetaddr {
        /* no unsafe data */
 };
 
+/* data provided to EVENT_HDL_SUB_SERVER_NAME handlers through
+ * event_hdl facility
+ *
+ * Note that this may be casted to regular event_hdl_cb_data_server if
+ * you don't care about name related optional info
+ */
+struct event_hdl_cb_data_server_name {
+       /* provided by:
+        *   EVENT_HDL_SUB_SERVER_NAME
+        */
+       struct event_hdl_cb_data_server server;                 /* must be at the beginning */
+       struct {
+               char old_name[64];
+               char new_name[64];
+       } safe;
+       /* no unsafe data */
+};
+
 /* Storage structure to load server-state lines from a flat file into
  * an ebtree, for faster processing
  */

diff --git a/src/event_hdl.c b/src/event_hdl.c
index a149c747158b5a60e688c3f199306f8842340b79..b26c024349da779b216095013e28cdd87ddc914c 100644 (file)
--- a/src/event_hdl.c
+++ b/src/event_hdl.c
@@ -42,6 +42,7 @@ static struct event_hdl_sub_type_map event_hdl_sub_type_map[] = {
        {"SERVER_ADMIN",        EVENT_HDL_SUB_SERVER_ADMIN},
        {"SERVER_CHECK",        EVENT_HDL_SUB_SERVER_CHECK},
        {"SERVER_INETADDR",     EVENT_HDL_SUB_SERVER_INETADDR},
+       {"SERVER_NAME",         EVENT_HDL_SUB_SERVER_NAME},
        {"PAT_REF",             EVENT_HDL_SUB_PAT_REF},
        {"PAT_REF_ADD",         EVENT_HDL_SUB_PAT_REF_ADD},
        {"PAT_REF_DEL",         EVENT_HDL_SUB_PAT_REF_DEL},

diff --git a/src/server.c b/src/server.c
index 7f8422f75a0c481779f776cca298087e3c7da56c..eef885b9583e1bc3fb624bb89ffd2afba87c9585 100644 (file)
--- a/src/server.c
+++ b/src/server.c
@@ -30,6 +30,7 @@
 #include <haproxy/counters.h>
 #include <haproxy/dict-t.h>
 #include <haproxy/errors.h>
+#include <haproxy/event_hdl.h>
 #include <haproxy/global.h>
 #include <haproxy/guid.h>
 #include <haproxy/log.h>
@@ -56,7 +57,6 @@
 #include <haproxy/time.h>
 #include <haproxy/tools.h>
 #include <haproxy/xxhash.h>
-#include <haproxy/event_hdl.h>
 
 static void srv_update_status(struct server *s, int type, int cause);
 static int srv_apply_lastaddr(struct server *srv, int *err_code);
@@ -3296,6 +3296,7 @@ struct server *srv_drop(struct server *srv)
        task_destroy(srv->srvrq_check);
 
        free(srv->id);
+
 #ifdef USE_QUIC
        if (srv->per_thr) {
                for (i = 0; i < global.nbthread; i++)
@@ -5518,6 +5519,118 @@ const char *srv_update_fqdn(struct server *server, const char *fqdn, const char
        return msg->area;
 }
 
+/* Rename a server at runtime. This function handles all precondition checks,
+ * tree re-indexing under thread_isolate(), and event publication.
+ *
+ * The caller must NOT hold any lock — this function uses thread_isolate()
+ * internally for tree manipulation.
+ *
+ * Preconditions enforced:
+ *   - server must be administratively in maintenance
+ *   - new name must not conflict with an existing server in the backend
+ *   - new name must be syntactically valid (invalid_char() rules, non-empty,
+ *     fits in event data name field)
+ *   - server must not have SRV_F_NAME_REFD set: this covers servers whose
+ *     name is referenced statically in the running configuration
+ *     ('use-server' rules, 'track' chains, sample-fetch ARGT_SRV
+ *     arguments). Renaming such a server would leave the config text
+ *     inconsistent with the running state. Note that sample fetches with
+ *     ARGT_SRV arguments are resolved to pointers at config-check time and
+ *     are flagged via SRV_F_NAME_REFD by the arg resolver.
+ *
+ * Returns NULL on success, or a pointer to a static/trash error message
+ * string on failure. On success, a ha_notice() is emitted and the
+ * EVENT_HDL_SUB_SERVER_NAME event is published.
+ */
+static const char *srv_update_server_name(struct server *srv, const char *new_name)
+{
+       struct proxy *be = srv->proxy;
+       char *old_name;
+       char *dup;
+       const char *err;
+
+       /* validate name syntax: must be non-empty, valid chars, not too long */
+       if (!*new_name)
+               return "Require a server name.\n";
+
+       err = invalid_char(new_name);
+       if (err)
+               return "Server name contains invalid characters.\n";
+
+       if (strlen(new_name) >= sizeof(((struct event_hdl_cb_data_server_name *)0)->safe.new_name))
+               return "Server name too long.\n";
+
+       /* server must be administratively down (in maintenance) */
+       if (!(srv->cur_admin & SRV_ADMF_MAINT))
+               return "Server must be in maintenance mode to be renamed (set server <b>/<s> state maint).\n";
+
+       /* reject if the server's name is statically referenced in the running
+        * config ('use-server' rules, 'track' chains, sample-fetch ARGT_SRV
+        * arguments): renaming would leave the running state inconsistent with
+        * the configuration text.
+        */
+       if (srv->flags & SRV_F_NAME_REFD)
+               return "Cannot rename: server's name is statically referenced (use-server, track, or sample argument).\n";
+
+       /* same name is a no-op success */
+       if (strcmp(srv->id, new_name) == 0)
+               return NULL;
+
+       /* allocate new name before taking isolation */
+       dup = strdup(new_name);
+       if (!dup)
+               return "Out of memory allocating new server name.\n";
+
+       /* tree manipulation requires thread isolation (same pattern as
+        * add/del server). This is rare enough that the cost is acceptable.
+        */
+       thread_isolate();
+
+       /* re-check for name conflict under isolation — another rename or
+        * add server could have raced before we isolated.
+        */
+       if (server_find_by_name(be, new_name)) {
+               thread_release();
+               free(dup);
+               return "A server with the same name already exists in this backend.\n";
+       }
+
+       /* --- point of no return --- */
+
+       old_name = srv->id;
+
+       /* re-index in the name tree */
+       cebis_item_delete(&be->conf.used_server_name, conf.name_node, id, srv);
+       srv->id = dup;
+       cebis_item_insert(&be->conf.used_server_name, conf.name_node, id, srv);
+
+       /* publish rename event with both old and new names */
+       {
+               struct event_hdl_cb_data_server_name cb_data;
+
+               _srv_event_hdl_prepare(&cb_data.server, srv, 1);
+               snprintf(cb_data.safe.old_name, sizeof(cb_data.safe.old_name), "%s", old_name);
+               snprintf(cb_data.safe.new_name, sizeof(cb_data.safe.new_name), "%s", new_name);
+               _srv_event_hdl_publish(EVENT_HDL_SUB_SERVER_NAME, cb_data, srv);
+       }
+
+       /* Emit the rename notice while old_name is still valid. */
+       ha_notice("Server %s/%s renamed from '%s'.\n", be->id, srv->id, old_name);
+
+       /* The old name string is no longer reachable from any structure: the
+        * tree has been re-indexed and srv->id now points at the new copy.
+        * Sample consumers retrieve srv->id via smp_fetch_srv_name and act on
+        * it within the current task (ACL evaluation, log format expansion,
+        * etc.), without retaining the raw pointer across wake-ups, so it is
+        * safe to release the old name immediately under thread isolation.
+        */
+       free(old_name);
+
+       thread_release();
+
+       return NULL;
+}
+
 
 /* Expects to find a backend and a server in <arg> under the form <backend>/<server>,
  * and returns the pointer to the server. Otherwise, display adequate error messages
@@ -5778,11 +5891,26 @@ static int cli_parse_set_server(char **args, char *payload, struct appctx *appct
 #else
                cli_msg(appctx, LOG_NOTICE, "server ssl setting not supported.\n");
 #endif
-       } else {
+       }
+       else if (strcmp(args[3], "name") == 0) {
+               if (!*args[4]) {
+                       cli_err(appctx, "set server <b>/<s> name requires a new name.\n");
+                       goto out;
+               }
+               /* srv_update_server_name() handles its own locking via
+                * thread_isolate(), so do NOT hold the server lock here.
+                */
+               warning = srv_update_server_name(sv, args[4]);
+               if (warning)
+                       cli_err(appctx, warning);
+               else
+                       cli_msg(appctx, LOG_NOTICE, "Server name updated.\n");
+       }
+       else {
                cli_err(appctx,
                        "usage: set server <backend>/<server> "
                        "addr | agent | agent-addr | agent-port | agent-send | "
-                       "check-addr | check-port | fqdn | health | ssl | "
+                       "check-addr | check-port | fqdn | health | name | ssl | "
                        "state | weight\n");
        }
  out:
@@ -6684,7 +6812,7 @@ static struct cli_kw_list cli_kws = {{ },{
        { { "enable", "health",  NULL },         "enable health                           : enable health checks",                                        cli_parse_enable_health, NULL },
        { { "enable", "server",  NULL },         "enable server  (DEPRECATED)             : enable a disabled server (use 'set server' instead)",         cli_parse_enable_server, NULL },
        { { "set", "maxconn", "server",  NULL }, "set maxconn server <bk>/<srv>           : change a server's maxconn setting",                           cli_parse_set_maxconn_server, NULL },
-       { { "set", "server", NULL },             "set server <bk>/<srv> [opts]            : change a server's state, weight, address or ssl",             cli_parse_set_server },
+       { { "set", "server", NULL },             "set server <bk>/<srv> [opts]            : change a server's state, weight, address, name or ssl",       cli_parse_set_server },
        { { "get", "weight", NULL },             "get weight <bk>/<srv>                   : report a server's current weight",                            cli_parse_get_weight },
        { { "set", "weight", NULL },             "set weight <bk>/<srv>  (DEPRECATED)     : change a server's weight (use 'set server' instead)",         cli_parse_set_weight },
        { { "add", "server", NULL },             "add server <bk>/<srv>                   : create a new server",                                         cli_parse_add_server, cli_io_handler_add_server },