Cluster nodename feature 20221124 clone

redis.conf

@@ -1742,6 +1742,10 @@ aof-timestamp-enabled no
 #
 # cluster-announce-hostname ""
 
+# Clusters can configure their announced nodename using this config. The nodename can be a human-readable name
+# which will be exposed in places like info and logging for debugging purposes.
+# cluster-announce-nodename ""
+
 # Clusters can advertise how clients should connect to them using either their IP address,
 # a user defined hostname, or by declaring they have no endpoint. Which endpoint is
 # shown as the preferred endpoint is set by using the cluster-preferred-endpoint-type

src/cluster.h

@@ -138,6 +138,7 @@ typedef struct clusterNode {
     long long repl_offset;      /* Last known repl offset for this node. */
     char ip[NET_IP_STR_LEN];    /* Latest known IP address of this node */
     sds hostname;               /* The known hostname for this node */
+    sds nodename;               /* The known human readable nodename for this node */
     int port;                   /* Latest known clients port (TLS or plain). */
     int pport;                  /* Latest known clients plaintext port. Only used
                                    if the main clients port is for TLS. */
@@ -254,6 +255,7 @@ typedef struct {
 typedef enum {
     CLUSTERMSG_EXT_TYPE_HOSTNAME,
     CLUSTERMSG_EXT_TYPE_FORGOTTEN_NODE,
+    CLUSTERMSG_EXT_TYPE_NODENAME,
 } clusterMsgPingtypes; 
 
 /* Helper function for making sure extensions are eight byte aligned. */
@@ -270,13 +272,18 @@ typedef struct {
 
 static_assert(sizeof(clusterMsgPingExtForgottenNode) % 8 == 0, "");
 
+typedef struct {
+    char nodename[1]; /* The announced nodename, ends with \0. */
+} clusterMsgPingExtNodename;
+
 typedef struct {
     uint32_t length; /* Total length of this extension message (including this header) */
     uint16_t type; /* Type of this extension message (see clusterMsgPingExtTypes) */
     uint16_t unused; /* 16 bits of padding to make this structure 8 byte aligned. */
     union {
         clusterMsgPingExtHostname hostname;
         clusterMsgPingExtForgottenNode forgotten_node;
+        clusterMsgPingExtNodename nodename;
     } ext[]; /* Actual extension information, formatted so that the data is 8 
               * byte aligned, regardless of its content. */
 } clusterMsgPingExt;
@@ -407,5 +414,6 @@ void clusterUpdateMyselfIp(void);
 void slotToChannelAdd(sds channel);
 void slotToChannelDel(sds channel);
 void clusterUpdateMyselfHostname(void);
+void clusterUpdateMyselfNodename(void);
 
 #endif /* __CLUSTER_H */

src/config.c

@@ -2603,6 +2603,12 @@ int updateClusterHostname(const char **err) {
     return 1;
 }
 
+int updateClusterNodename(const char **err) {
+    UNUSED(err);
+    clusterUpdateMyselfNodename();
+    return 1;
+}
+
 static int applyTlsCfg(const char **err) {
     UNUSED(err);
 
@@ -3028,6 +3034,7 @@ standardConfig static_configs[] = {
     createStringConfig("cluster-announce-ip", NULL, MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.cluster_announce_ip, NULL, NULL, updateClusterIp),
     createStringConfig("cluster-config-file", NULL, IMMUTABLE_CONFIG, ALLOW_EMPTY_STRING, server.cluster_configfile, "nodes.conf", NULL, NULL),
     createStringConfig("cluster-announce-hostname", NULL, MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.cluster_announce_hostname, NULL, isValidAnnouncedHostname, updateClusterHostname),
+    createStringConfig("cluster-announce-nodename", NULL, MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.cluster_announce_nodename, NULL, isValidAnnouncedHostname, updateClusterNodename),
     createStringConfig("syslog-ident", NULL, IMMUTABLE_CONFIG, ALLOW_EMPTY_STRING, server.syslog_ident, "redis", NULL, NULL),
     createStringConfig("dbfilename", NULL, MODIFIABLE_CONFIG | PROTECTED_CONFIG, ALLOW_EMPTY_STRING, server.rdb_filename, "dump.rdb", isValidDBfilename, NULL),
     createStringConfig("appendfilename", NULL, IMMUTABLE_CONFIG, ALLOW_EMPTY_STRING, server.aof_filename, "appendonly.aof", isValidAOFfilename, NULL),

src/server.h

@@ -1857,7 +1857,8 @@ struct redisServer {
     int cluster_slave_no_failover;  /* Prevent slave from starting a failover
                                        if the master is in failure state. */
     char *cluster_announce_ip;  /* IP address to announce on cluster bus. */
-    char *cluster_announce_hostname;  /* hostname to announce on cluster bus. */
+    char *cluster_announce_hostname;  /* IP address to announce on cluster bus. */
+    char *cluster_announce_nodename;  /* Human readable name assigned to a node. */
     int cluster_preferred_endpoint_type; /* Use the announced hostname when available. */
     int cluster_announce_port;     /* base port to announce on cluster bus. */
     int cluster_announce_tls_port; /* TLS port to announce on cluster bus. */

tests/cluster/cluster.tcl

@@ -218,6 +218,17 @@ proc are_hostnames_propagated {match_string} {
     return 1
 }
 
+# Check if cluster's view of nodename is consistent
+proc are_nodenames_propagated {match_string} {
+    for {set j 0} {$j < $::cluster_master_nodes + $::cluster_replica_nodes} {incr j} {
+        set cfg [R $j cluster nodes]
+        if {! [string match $match_string $cfg]} {
+            return 0
+        }
+        return 1
+    }
+}
+
 # Returns a parsed CLUSTER LINKS output of the instance identified
 # by the given `id` as a list of dictionaries, with each dictionary
 # corresponds to a link.

tests/unit/cluster/hostnames.tcl

@@ -59,6 +59,22 @@ test "Set cluster hostnames and verify they are propagated" {
     wait_for_cluster_propagation
 }
 
+test "Set cluster nodenames and verify they are propagated" {
+
+    #for {set j 0} {$j < $::cluster_master_nodes + $::cluster_replica_nodes} {incr j} {
+    #    R $j config set cluster-announce-nodename "node-$j.com"
+    #}
+
+    #wait_for_condition 50 100 {
+    #    [are_nodenames_propagated "*node-*.com*"] eq 1
+    #} else {
+    #    fail "cluster nodenames were not propagated"
+    #}
+
+    # Now that everything is propagated, assert everyone agrees
+    #wait_for_cluster_propagation
+}
+
 test "Update hostnames and make sure they are all eventually propagated" {
     for {set j 0} {$j < [llength $::servers]} {incr j} {
         R $j config set cluster-announce-hostname "host-updated-$j.com"
@@ -74,6 +90,21 @@ test "Update hostnames and make sure they are all eventually propagated" {
     wait_for_cluster_propagation
 }
 
+test "Update nodenames and make sure they are all eventually propagated" {
+    #for {set j 0} {$j < $::cluster_master_nodes + $::cluster_replica_nodes} {incr j} {
+    #    R $j config set cluster-announce-nodename "node-updated-$j.com"
+    #}
+
+    #wait_for_condition 50 100 {
+    #    [are_nodenames_propagated "*node-updated-*.com*"] eq 1
+    #} else {
+    #    fail "cluster nodenames were not propagated"
+    #}
+
+    # Now that everything is propagated, assert everyone agrees
+    #wait_for_cluster_propagation
+}
+
 test "Remove hostnames and make sure they are all eventually propagated" {
     for {set j 0} {$j < [llength $::servers]} {incr j} {
         R $j config set cluster-announce-hostname ""
@@ -241,4 +272,11 @@ test "Test hostname validation" {
     # Note this isn't a valid hostname, but it passes our internal validation
     R 0 config set cluster-announce-hostname "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-."
 }
+
+test "Test nodename validation" {
+    #catch {R 0 config set cluster-announce-nodename [string repeat x 256]} err
+    #assert_match "*must be less than 256 characters*" $err
+    #catch {R 0 config set cluster-announce-nodename "?.com"} err
+    #assert_match "*may only contain alphanumeric characters, hyphens or dots*" $err
+}
 }