From 7aacba4bda25c5ab8f97fd2960dd6db5cfc0601b Mon Sep 17 00:00:00 2001 From: Phil Pennock Date: Wed, 27 Oct 2021 13:46:43 -0400 Subject: [PATCH 1/2] tests: don't allocate fixed ports from random range When a socket is bound for IP stack protocols with port == 0, the kernel picks a free port in a specific range and binds it; on Linux, the range can be seen (and modified) with `sysctl net.ipv4.ip_local_port_range` or looking in `/proc/sys/net/ipv4/ip_local_port_range`. This defaults to 32768:60999. When binding explicit ports (for tests), don't use a port number from that range, or there will be flaky tests as periodically that port will already be in use from another test. This renumbers all the JS clustering tests I found binding in that range to be beneath that range; I checked the code to ensure the new port wasn't already in use. --- server/jetstream_cluster_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/server/jetstream_cluster_test.go b/server/jetstream_cluster_test.go index 9c859c5d..b81defde 100644 --- a/server/jetstream_cluster_test.go +++ b/server/jetstream_cluster_test.go @@ -5440,13 +5440,13 @@ func TestJetStreamClusterSuperClusterInterestOnlyMode(t *testing.T) { ` storeDir1 := createDir(t, JetStreamStoreDir) conf1 := createConfFile(t, []byte(fmt.Sprintf(template, - "S1", storeDir1, "", 33222, "A", 33222, "A", 11222, "B", 11223))) + "S1", storeDir1, "", 23222, "A", 23222, "A", 11222, "B", 11223))) s1, o1 := RunServerWithConfig(conf1) defer s1.Shutdown() storeDir2 := createDir(t, JetStreamStoreDir) conf2 := createConfFile(t, []byte(fmt.Sprintf(template, - "S2", storeDir2, "", 33223, "B", 33223, "B", 11223, "A", 11222))) + "S2", storeDir2, "", 23223, "B", 23223, "B", 11223, "A", 11222))) s2, o2 := RunServerWithConfig(conf2) defer s2.Shutdown() @@ -5497,9 +5497,9 @@ func TestJetStreamClusterSuperClusterInterestOnlyMode(t *testing.T) { // Now change account "two" to enable JS changeCurrentConfigContentWithNewContent(t, conf1, []byte(fmt.Sprintf(template, - "S1", storeDir1, "jetstream: enabled", 33222, "A", 33222, "A", 11222, "B", 11223))) + "S1", storeDir1, "jetstream: enabled", 23222, "A", 23222, "A", 11222, "B", 11223))) changeCurrentConfigContentWithNewContent(t, conf2, []byte(fmt.Sprintf(template, - "S2", storeDir2, "jetstream: enabled", 33223, "B", 33223, "B", 11223, "A", 11222))) + "S2", storeDir2, "jetstream: enabled", 23223, "B", 23223, "B", 11223, "A", 11222))) if err := s1.Reload(); err != nil { t.Fatalf("Error on s1 reload: %v", err) @@ -6826,7 +6826,7 @@ func TestJetStreamClusterDomainsAndAPIResponses(t *testing.T) { // Now create spoke LN cluster. tmpl = strings.Replace(jsClusterTemplWithLeafNode, "store_dir:", "domain: SPOKE, store_dir:", 1) - lnc := c.createLeafNodesWithTemplateAndStartPort(tmpl, "SPOKE", 5, 33913) + lnc := c.createLeafNodesWithTemplateAndStartPort(tmpl, "SPOKE", 5, 23913) defer lnc.shutdown() lnc.waitOnClusterReady() @@ -7190,7 +7190,7 @@ func TestJetStreamClusterSuperClusterPullConsumerAndHeaders(t *testing.T) { } func TestJetStreamClusterLeafDifferentAccounts(t *testing.T) { - c := createJetStreamCluster(t, jsClusterAccountsTempl, "HUB", _EMPTY_, 2, 33133, false) + c := createJetStreamCluster(t, jsClusterAccountsTempl, "HUB", _EMPTY_, 2, 23133, false) defer c.shutdown() ln := c.createLeafNodesWithStartPort("LN", 2, 22110) From 635c98a04b11ea49374565d714129294e80bff04 Mon Sep 17 00:00:00 2001 From: Phil Pennock Date: Wed, 27 Oct 2021 14:01:20 -0400 Subject: [PATCH 2/2] tests: hard-reject bad ports for JS clusters --- server/jetstream_cluster_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/server/jetstream_cluster_test.go b/server/jetstream_cluster_test.go index b81defde..feb18efb 100644 --- a/server/jetstream_cluster_test.go +++ b/server/jetstream_cluster_test.go @@ -9754,6 +9754,21 @@ func createJetStreamCluster(t *testing.T, tmpl string, clusterName, snPre string t.Fatalf("Bad params") } + // Flaky test prevention: + // Binding a socket to IP stack port 0 will bind an ephemeral port from an OS-specific range. + // If someone passes in to us a port spec which would cover that range, the test would be flaky. + // Adjust these ports to be the most inclusive across the port runner OSes. + // Linux: /proc/sys/net/ipv4/ip_local_port_range : 32768:60999 + // is useful, and shows there's no safe available range without OS-specific tuning. + // Our tests are usually run on Linux. Folks who care about other OSes: if you can't tune your test-runner OS to match, please + // propose a viable alternative. + const prohibitedPortFirst = 32768 + const prohibitedPortLast = 60999 + if (portStart >= prohibitedPortFirst && portStart <= prohibitedPortLast) || + (portStart+numServers-1 >= prohibitedPortFirst && portStart+numServers-1 <= prohibitedPortLast) { + t.Fatalf("test setup failure: may not specify a cluster port range which falls within %d:%d", prohibitedPortFirst, prohibitedPortLast) + } + // Build out the routes that will be shared with all configs. var routes []string for cp := portStart; cp < portStart+numServers; cp++ {