On Tue, Jun 16, 2026 at 04:19:16PM +0000, Stephan, Alexander wrote: > Hi list,
Hi, Thanks for your contribution. I checked the patches and the code quality and implementation seems OK to me. I still have some remarks about the feature itself, see them below. > This series adds support for renaming a running HAProxy server via the > runtime API and a new option server-rename per-backend opt-in directive. > This addresses the long-standing request in GitHub issue #952 [1] for a > runtime server rename command. > Use case: when servers are named after their IP:port or target reference > (e.g., by an ingress controller), a live endpoint replacement means the > server's name becomes stale. Instead of a full > reload, the controller can now issue: > set server <backend>/<oldname> name <newname> > to atomically rename the server in the cebtree while it is in maintenance > mode. A work-in-progress consumer of this feature can be seen here [2]. One can argue that the server rename feature could now be achieved by deleting and adding the server with the new name on the CLI. It's even possible to preserve stats if using stats-file/shm-stats-file and the same GUID. In fact the original feature request #952 dates from haproxy 2.3, just before dynamic servers were made available in 2.4. Also, one of the motivation for server rename is based on server-template or server preprovisioning usage which should be less frequent now with the ability to add/remove servers at runtime. Of course though, a rename command still has merit as it is much simpler to use, in particular because server deletion may take some time when there is long running connections. Thus it can still make sense to have this dedicated rename command if some users still think it is necessary for the latest releases. > The feature requires the backend to opt in with option server-rename (or the > global server-rename tunable) to prevent accidental renames in setups that > depend on stable server names. The > reconciliation of server-state files after a rename is left to the operator, > consistent with the approach discussed in the issue. > All concerns raised in issue #952 should be addressed: cebtree re-indexing > under thread_isolate(), name validation, maintenance-mode requirement, > per-backend opt-in via option server-rename (plus a > global server-rename tunable), and EVENT_HDL_SUB_SERVER_NAME notification on > success. I'm fine with a proxy level setting. However is this really necessary to also have a global one ? With defaults section, it should already be possible to avoid duplication of the configuration in every backend instances. > Three cases are explicitly rejected with an error: > - use-server references — the config text would silently diverge from the > running state. > - track references — the next reload would fail to resolve the old name, a > hard correctness issue. > - Peer-synced stick tables — peers use server names as identifiers; a local > rename would cause silent divergence between peers. > Patch 1: core implementation (srv_update_server_name, CLI command, global > tunable, option server-rename) > Patch 2: reg-test > Patch 3: documentation I have some questions about some of your design choice. In particular you chose to allow renaming servers explicitely referenced in the configuration via ACLs. Your assumption that these rules are based on the server pointer is right so there is no risk of crash here. However, it can be confusing for an administrator to look at his configuration and read server names that are not present anymore in the process memory. Thus, I'm favorable to deny any rename operation if a server is referenced in the configuration. The simplest solution would be to rely on SRV_F_NON_PURGEABLE flag which is already checked by "del server" command, even if this may be too restrictive in some occasion. Currently this would also prevent renaming for servers which relies on resolvers for their address. Finally, I also have an observation about srv_update_server_name(). Every required checks are performed as expected. However, it could be better to refactor some of these checks as these control are also performed by the config parser and the "add server" handler. This would avoid the need to edit the code in three different places, for example for example in case the server naming convention is adjusted. This cleanup could still be implemented in a later patch though. Regards, -- Amaury Denoyelle
