Hi Amaury, Thanks for the thorough review. I addressed your suggestions in the following way:
1. I removed the global server-rename keyword. You are right that a defaults section already covers the use case of enabling renaming across all backends without repeating the option. The global tunable and its parser entry have been dropped entirely. Actually, also my ingress controller implementation didn’t make use of it. 1. SRV_F_NON_PURGEABLE check. The individual use-server loop and srv->trackers check have been replaced with a single SRV_F_NON_PURGEABLE flag check, consistent with del server. This also implicitly covers resolver-based servers as you noted. The error message has been updated to reflect the broader scope of the rejection. 1. Name-validation refactored into a shared helper. A new srv_validate_name() function consolidates the empty/length/#-prefix/charset checks. It is now called by both srv_update_server_name() and cli_parse_add_server(), so the checks no longer need to be maintained in multiple places. Note that the character set intentionally differs from invalid_char() used by the config parser: the runtime helper allows characters such as : so that IP:port strings (e.g., 172.17.0.4:8080) remain valid server names at runtime, which is a primary use case for this feature. One more remark regarding formatting. The CLI keyword table entry for set server was realigned to match the surrounding entries. checkpatch warns about the line length due to tab expansion, but the alignment is intentional and consistent with the existing style in that table. I append the updates patch files. Please have another look. Best regards, Alexander From: Amaury Denoyelle <[email protected]> Date: Wednesday, 17. June 2026 at 09:15 To: Stephan, Alexander <[email protected]> Cc: [email protected] <[email protected]> Subject: Re: [PATCH 1/3 2/3 3/3] MINOR: server: add 'option server-rename' and 'set server name' CLI command [You don't often get email from [email protected]. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] 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
0001-MINOR-server-add-option-server-rename-and-set-server.patch
Description: 0001-MINOR-server-add-option-server-rename-and-set-server.patch
0002-REGTESTS-server-add-test-for-set-server-name-CLI-com.patch
Description: 0002-REGTESTS-server-add-test-for-set-server-name-CLI-com.patch
0003-DOC-server-document-option-server-rename-and-set-ser.patch
Description: 0003-DOC-server-document-option-server-rename-and-set-ser.patch
