Add cluster-announce-client-(port|tls-port) configs

[marvin-roesch]

New config options:

• cluster-announce-client-port
• cluster-announce-client-tls-port

If enabled, clients will always get to see the configured port for a node instead of the internally announced port(s), the same way that cluster-announce-client-ipv4 and cluster-announce-client-ipv6 work. Cluster-internal communication uses the non-client variant of these options.

The configuration is propagated throughout the cluster using new ping extensions.

Closes #2377

[codecov]

Codecov Report

❌ Patch coverage is 62.71186% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.21%. Comparing base (a50690f) to head (df8eaa4).
⚠️ Report is 96 commits behind head on unstable.

Files with missing lines	Patch %	Lines
src/cluster_legacy.c	62.71%	22 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable    #2429      +/-   ##
============================================
+ Coverage     71.52%   72.21%   +0.69%     
============================================
  Files           123      127       +4     
  Lines         67455    70777    +3322     
============================================
+ Hits          48246    51111    +2865     
- Misses        19209    19666     +457     

Files with missing lines	Coverage Δ
src/cluster.c	89.98% <ø> (-0.40%)	⬇️
src/config.c	78.66% <ø> (+0.19%)	⬆️
src/module.c	9.70% <ø> (+0.14%)	⬆️
src/server.h	100.00% <ø> (ø)
src/cluster_legacy.c	87.40% <62.71%> (+0.52%)	⬆️

... and 41 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[madolson]

Viktor from the issue thread:

I like these new configs, but I think we should skip the cluster bus port. The bus is not for clients.

We already have cluster-announce-bus-port that is used together with cluster-announce-ip for the cluster bus.

[marvin-roesch]

I went ahead and added an easily reversible commit that removes the bus port announcement change again. The main rationale behind including it in the first place was that the bus port is visible to clients as part of the CLUSTER NODES response.

I'm not sure why the external tests were failing. They ran fine for me locally and I could not see anything related to my changes in the failed test logs.

[zuiderkwast]

It's a very well-written PR. Thank you. I have only a few comments.

[marvin-roesch]

I'm not super familiar with Tcl, but from what I could find, the macOS tests might fail due to an old-ish version being installed where lset will not expand a list for the set index to fit. Since the tests don't rely on the indices of the list items, I went for lappend instead.

[zuiderkwast]

I'm not super familiar with Tcl, but from what I could find, the macOS tests might fail due to an old-ish version being installed where lset will not expand a list for the set index to fit. Since the tests don't rely on the indices of the list items, I went for lappend instead.

Sounds reasonable! None of us are TCL experts, but I know the macOS worker runs 8.5 while other workers run 8.6.

[madolson]

Didn't do a deep review, just wanted to check some things we have missed in the past, but it all looked pretty good to me.

[zuiderkwast]

The Tcl refactoring looks great.

[rjd15372]

@marvin-roesch I'm about to merge this PR but if you intend to still apply the suggestion in https://github.com/valkey-io/valkey/pull/2429/files/0c0d599099e0f69e3f060cb8c6bb5a2d5b34dd20#diff-2515500619600c5a1e7a8d9aaa8761a6071314cd226c3b7ee4d0e054691f0a8e please let me know soon.

[marvin-roesch]

Thanks for merging! Sorry about not commenting on that suggestion before. My intention was mostly to avoid repeating conditions, but comparing generated assembly, the compiler at least produced shorter output for the suggested (and now applied) changes.

[zuiderkwast]

My intention was mostly to avoid repeating conditions, but comparing generated assembly, the compiler at least produced shorter output for the suggested (and now applied) changes.

OK, good to know. I didn't check. :) This is not a very hot code path, so we can prefer readability over micro optimizations here.