From 7d6744c739b25ae27a42f6fcf235e0d6bfbb3e61 Mon Sep 17 00:00:00 2001
From: Oran Agra <oran@redislabs.com>
Date: Wed, 20 Oct 2021 15:40:28 +0300
Subject: [PATCH] fix new cluster tests issues (#9657)

Following #9483 the daily CI exposed a few problems.

* The cluster creation code (uses redis-cli) is complicated to test with TLS enabled.
  for now i'm just skipping them since the tests we run there don't really need that kind of coverage
* cluster port binding failures
  note that `find_available_port` already looks for a free cluster port
  but the code in `wait_server_started` couldn't detect the failure of binding
  (the text it greps for wasn't found in the log)
---
 src/cluster.c                    |  2 ++
 src/server.c                     |  2 ++
 tests/unit/cluster.tcl           | 12 +++++++++---
 tests/unit/moduleapi/cluster.tcl | 17 ++++++++++-------
 4 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/src/cluster.c b/src/cluster.c
index 740127537..310d30136 100644
--- a/src/cluster.c
+++ b/src/cluster.c
@@ -584,6 +584,8 @@ void clusterInit(void) {
     }
     int cport = server.cluster_port ? server.cluster_port : port + CLUSTER_PORT_INCR;
     if (listenToPort(cport, &server.cfd) == C_ERR ) {
+        /* Note: the following log text is matched by the test suite. */
+        serverLog(LL_WARNING, "Failed listening on port %u (cluster), aborting.", cport);
         exit(1);
     }
     
diff --git a/src/server.c b/src/server.c
index 17672fbd6..7894be358 100644
--- a/src/server.c
+++ b/src/server.c
@@ -4199,11 +4199,13 @@ void initServer(void) {
     /* Open the TCP listening socket for the user commands. */
     if (server.port != 0 &&
         listenToPort(server.port,&server.ipfd) == C_ERR) {
+        /* Note: the following log text is matched by the test suite. */
         serverLog(LL_WARNING, "Failed listening on port %u (TCP), aborting.", server.port);
         exit(1);
     }
     if (server.tls_port != 0 &&
         listenToPort(server.tls_port,&server.tlsfd) == C_ERR) {
+        /* Note: the following log text is matched by the test suite. */
         serverLog(LL_WARNING, "Failed listening on port %u (TLS), aborting.", server.tls_port);
         exit(1);
     }
diff --git a/tests/unit/cluster.tcl b/tests/unit/cluster.tcl
index 41efc985a..885bf76a2 100644
--- a/tests/unit/cluster.tcl
+++ b/tests/unit/cluster.tcl
@@ -21,10 +21,14 @@ proc csi {args} {
 # make sure the test infra won't use SELECT
 set ::singledb 1
 
+# cluster creation is complicated with TLS, and the current tests don't really need that coverage
+tags {tls:skip external:skip cluster} {
+
 # start three servers
-start_server {overrides {cluster-enabled yes cluster-node-timeout 1} tags {"external:skip cluster"}} {
-start_server {overrides {cluster-enabled yes cluster-node-timeout 1} tags {"external:skip cluster"}} {
-start_server {overrides {cluster-enabled yes cluster-node-timeout 1} tags {"external:skip cluster"}} {
+set base_conf [list cluster-enabled yes cluster-node-timeout 1]
+start_server [list overrides $base_conf] {
+start_server [list overrides $base_conf] {
+start_server [list overrides $base_conf] {
 
     set node1 [srv 0 client]
     set node2 [srv -1 client]
@@ -149,3 +153,5 @@ start_server {overrides {cluster-enabled yes cluster-node-timeout 1} tags {"exte
 }
 }
 }
+
+} ;# tags
\ No newline at end of file
diff --git a/tests/unit/moduleapi/cluster.tcl b/tests/unit/moduleapi/cluster.tcl
index 22841df57..b2d2df899 100644
--- a/tests/unit/moduleapi/cluster.tcl
+++ b/tests/unit/moduleapi/cluster.tcl
@@ -24,20 +24,21 @@ set testmodule_nokey [file normalize tests/modules/blockonbackground.so]
 # make sure the test infra won't use SELECT
 set ::singledb 1
 
+# cluster creation is complicated with TLS, and the current tests don't really need that coverage
+tags {tls:skip external:skip cluster modules} {
+
 # start three servers
-start_server {overrides {cluster-enabled yes cluster-node-timeout 1} tags {"external:skip cluster modules"}} {
-start_server {overrides {cluster-enabled yes cluster-node-timeout 1} tags {"external:skip cluster modules"}} {
-start_server {overrides {cluster-enabled yes cluster-node-timeout 1} tags {"external:skip cluster modules"}} {
+set base_conf [list cluster-enabled yes cluster-node-timeout 1 loadmodule $testmodule]
+start_server [list overrides $base_conf] {
+start_server [list overrides $base_conf] {
+start_server [list overrides $base_conf] {
 
     set node1 [srv 0 client]
     set node2 [srv -1 client]
     set node3 [srv -2 client]
     set node3_pid [srv -2 pid]
 
-    $node1 module load $testmodule
-    $node2 module load $testmodule
-    $node3 module load $testmodule
-
+    # the "overrides" mechanism can only support one "loadmodule" directive
     $node1 module load $testmodule_nokey
     $node2 module load $testmodule_nokey
     $node3 module load $testmodule_nokey
@@ -200,3 +201,5 @@ start_server {overrides {cluster-enabled yes cluster-node-timeout 1} tags {"exte
 }
 }
 }
+
+} ;# tags
\ No newline at end of file