Fix cluster myself CLUSTER SLOTS/NODES wrong port after updating port/tls-port (valkey-io#2186)

When modifying port or tls-port through config set, we need to call
clusterUpdateMyselfAnnouncedPorts to update myself's port, otherwise
CLUSTER SLOTS/NODES will be old information from myself's perspective.

In addition, in some places, such as clusterUpdateMyselfAnnouncedPorts
and clusterUpdateMyselfIp, beforeSleep save is added so that the
new ip info can be updated to nodes.conf.

Remove clearCachedClusterSlotsResponse in updateClusterAnnouncedPort
since now we add beforeSleep save in clusterUpdateMyselfAnnouncedPorts,
and it will call clearCachedClusterSlotsResponse.

Signed-off-by: Binbin <[email protected]>

Author: enjoy-binbin

src/cluster_legacy.c

@@ -1024,6 +1024,7 @@ void clusterUpdateMyselfFlags(void) {
 void clusterUpdateMyselfAnnouncedPorts(void) {
     if (!myself) return;
     deriveAnnouncedPorts(&myself->tcp_port, &myself->tls_port, &myself->cport);
+    clusterDoBeforeSleep(CLUSTER_TODO_SAVE_CONFIG);
 }
 
 /* We want to take myself->ip in sync with the cluster-announce-ip option.
@@ -1054,6 +1055,7 @@ void clusterUpdateMyselfIp(void) {
         } else {
             myself->ip[0] = '\0'; /* Force autodetection. */
         }
+        clusterDoBeforeSleep(CLUSTER_TODO_SAVE_CONFIG);
     }
 }
 

src/config.c

@@ -2473,6 +2473,7 @@ static int updatePort(const char **err) {
     listener->bindaddr_count = server.bindaddr_count;
     listener->port = server.port;
     listener->ct = connectionByType(CONN_TYPE_SOCKET);
+    clusterUpdateMyselfAnnouncedPorts();
     if (changeListener(listener) == C_ERR) {
         *err = "Unable to listen on this port. Check server logs.";
         return 0;
@@ -2660,7 +2661,6 @@ int updateClusterFlags(const char **err) {
 static int updateClusterAnnouncedPort(const char **err) {
     UNUSED(err);
     clusterUpdateMyselfAnnouncedPorts();
-    clearCachedClusterSlotsResponse();
     return 1;
 }
 
@@ -2719,6 +2719,7 @@ static int applyTLSPort(const char **err) {
     listener->bindaddr_count = server.bindaddr_count;
     listener->port = server.tls_port;
     listener->ct = connectionByType(CONN_TYPE_TLS);
+    clusterUpdateMyselfAnnouncedPorts();
     if (changeListener(listener) == C_ERR) {
         *err = "Unable to listen on this port. Check server logs.";
         return 0;

tests/unit/cluster/announced-endpoints.tcl

@@ -53,4 +53,43 @@ start_cluster 2 2 {tags {external:skip cluster}} {
         R 0 config set cluster-announce-bus-port 0
         assert_match "*@$base_bus_port *" [R 0 CLUSTER NODES]
     }
+
+    test "Test change port and tls-port on runtime" {
+        if {$::tls} {
+            set baseport [lindex [R 0 config get tls-port] 1]
+        } else {
+            set baseport [lindex [R 0 config get port] 1]
+        }
+        set count [expr [llength $::servers] + 1]
+        set used_port [find_available_port $baseport $count]
+
+        # We execute CLUSTER SLOTS command to trigger the `debugServerAssertWithInfo` in `clusterCommandSlots` function, ensuring
+        # that the cached response is invalidated upon updating any of port or tls-port.
+        R 0 CLUSTER SLOTS
+        R 1 CLUSTER SLOTS
+
+        # Set port or tls-port to ensure changes are consistent across the cluster.
+        if {$::tls} {
+            R 0 config set tls-port $used_port
+        } else {
+            R 0 config set port $used_port
+        }
+        # Make sure changes in myself node's view are consistent.
+        assert_match "*:$used_port@*" [R 0 CLUSTER NODES]
+        assert_match "*$used_port*" [R 0 CLUSTER SLOTS]
+        # Make sure changes in other node's view are consistent.
+        wait_for_condition 50 100 {
+            [string match "*:$used_port@*" [R 1 CLUSTER NODES]] &&
+            [string match "*$used_port*" [R 1 CLUSTER SLOTS]]
+        } else {
+            fail "Node port was not propagated via gossip"
+        }
+
+        # Restore the original configuration item value.
+        if {$::tls} {
+            R 0 config set tls-port $baseport
+        } else {
+            R 0 config set port $baseport
+        }
+    }
 }