avoid port re-use across unit test runs (#4374)

In Jenkins CI we run two instances of unit tests concurrently.
This can trigger CI failure when the same port numbers are re-used
by the different test instances. Fixed one more issue of this by
allowing user configuration of the base port number.

Makefile

@@ -159,17 +159,28 @@ libbacktrace:
 # EXECUTOR_NUMBER: [0, 1] (depends on max number of concurrent CI jobs)
 #
 # The following port ranges are allocated (entire continuous range):
+#
+# Unit tests:
+# - NIMBUS_TEST_KEYMANAGER_BASE_PORT + [0, 4)
+#
+# REST tests:
+# - --base-port
+# - --base-rest-port
+# - --base-metrics-port
+#
+# Local testnets (entire continuous range):
 # - --base-port + [0, --nodes + --light-clients)
 # - --base-rest-port + [0, --nodes)
 # - --base-metrics-port + [0, --nodes)
 # - --base-vc-metrics-port + [0, --nodes]
 # - --base-remote-signer-port + [0, --remote-signers)
 #
-# If --run-geth or --run-nimbus is specified (only these ports):
+# Local testnets with --run-geth or --run-nimbus (only these ports):
 # - --base-el-net-port + --el-port-offset * [0, --nodes + --light-clients)
 # - --base-el-http-port + --el-port-offset * [0, --nodes + --light-clients)
 # - --base-el-ws-port + --el-port-offset * [0, --nodes + --light-clients)
 # - --base-el-auth-rpc-port + --el-port-offset * [0, --nodes + --light-clients)
+UNIT_TEST_BASE_PORT := 9950
 
 restapi-test:
 	./tests/simulation/restapi.sh \
@@ -315,7 +326,11 @@ endif
 	for TEST_BINARY in $(XML_TEST_BINARIES); do \
 		PARAMS="--xml:build/$${TEST_BINARY}.xml --console"; \
 		echo -e "\nRunning $${TEST_BINARY} $${PARAMS}\n"; \
-		build/$${TEST_BINARY} $${PARAMS} || { echo -e "\n$${TEST_BINARY} $${PARAMS} failed; Last 50 lines from the log:"; tail -n50 "$${TEST_BINARY}.log"; exit 1; }; \
+		NIMBUS_TEST_KEYMANAGER_BASE_PORT=$$(( $(UNIT_TEST_BASE_PORT) + EXECUTOR_NUMBER * 25 )) \
+			build/$${TEST_BINARY} $${PARAMS} || { \
+				echo -e "\n$${TEST_BINARY} $${PARAMS} failed; Last 50 lines from the log:"; \
+				tail -n50 "$${TEST_BINARY}.log"; exit 1; \
+			}; \
 		done; \
 		rm -rf 0000-*.json t_slashprot_migration.* *.log block_sim_db
 	for TEST_BINARY in $(TEST_BINARIES); do \
@@ -324,7 +339,10 @@ endif
 		elif [[ "$${TEST_BINARY}" == "block_sim" ]]; then PARAMS="--validators=8000 --slots=160"; \
 		fi; \
 		echo -e "\nRunning $${TEST_BINARY} $${PARAMS}\n"; \
-		build/$${TEST_BINARY} $${PARAMS} || { echo -e "\n$${TEST_BINARY} $${PARAMS} failed; Last 50 lines from the log:"; tail -n50 "$${TEST_BINARY}.log"; exit 1; }; \
+		build/$${TEST_BINARY} $${PARAMS} || { \
+			echo -e "\n$${TEST_BINARY} $${PARAMS} failed; Last 50 lines from the log:"; \
+			tail -n50 "$${TEST_BINARY}.log"; exit 1; \
+		}; \
 		done; \
 		rm -rf 0000-*.json t_slashprot_migration.* *.log block_sim_db
 

tests/test_keymanager_api.nim

@@ -28,10 +28,18 @@ import
 
 type
   KeymanagerToTest = object
+    ident: string
     port: int
     validatorsDir: string
     secretsDir: string
 
+  # Individual port numbers derived by adding `ord` to configurable base port
+  PortKind {.pure.} = enum
+    PeerToPeer,
+    Metrics,
+    KeymanagerBN,
+    KeymanagerVC
+
 const
   simulationDepositsCount = 128
   dataDir = "./test_keymanager_api"
@@ -41,8 +49,7 @@ const
   genesisFile = dataDir / "genesis.ssz"
   bootstrapEnrFile = dataDir / "bootstrap_node.enr"
   tokenFilePath = dataDir / "keymanager-token.txt"
-  keymanagerPortBN = 47000
+  defaultBasePort = 49000
-  keymanagerPortVC = 48000
   correctTokenValue = "some secret token"
   defaultFeeRecipient = Eth1Address.fromHex("0x000000000000000000000000000000000000DEAD")
 
@@ -96,16 +103,6 @@ const
   vcValidatorsDir = vcDataDir / "validators"
   vcSecretsDir = vcDataDir / "secrets"
 
-  beaconNodeKeymanager = KeymanagerToTest(
-    port: keymanagerPortBN,
-    validatorsDir: nodeValidatorsDir,
-    secretsDir: nodeSecretsDir)
-
-  validatorClientKeymanager = KeymanagerToTest(
-    port: keymanagerPortVC,
-    validatorsDir: vcValidatorsDir,
-    secretsDir: vcSecretsDir)
-
 func specifiedFeeRecipient(x: int): Eth1Address =
   copyMem(addr result, unsafeAddr x, sizeof x)
 
@@ -258,28 +255,28 @@ proc addPreTestRemoteKeystores(validatorsDir: string) =
             err = res.error
       quit 1
 
-proc startBeaconNode {.raises: [Defect, CatchableError].} =
+proc startBeaconNode(basePort: int) {.raises: [Defect, CatchableError].} =
   let rng = keys.newRng()
 
   copyHalfValidators(nodeDataDir, true)
   addPreTestRemoteKeystores(nodeValidatorsDir)
 
   let runNodeConf = try: BeaconNodeConf.load(cmdLine = mapIt([
-    "--tcp-port=49000",
+    "--tcp-port=" & $(basePort + PortKind.PeerToPeer.ord),
-    "--udp-port=49000",
+    "--udp-port=" & $(basePort + PortKind.PeerToPeer.ord),
     "--discv5=off",
     "--network=" & dataDir,
     "--data-dir=" & nodeDataDir,
     "--validators-dir=" & nodeValidatorsDir,
     "--secrets-dir=" & nodeSecretsDir,
     "--metrics-address=127.0.0.1",
-    "--metrics-port=48008",
+    "--metrics-port=" & $(basePort + PortKind.Metrics.ord),
     "--rest=true",
     "--rest-address=127.0.0.1",
-    "--rest-port=" & $keymanagerPortBN,
+    "--rest-port=" & $(basePort + PortKind.KeymanagerBN.ord),
     "--keymanager=true",
     "--keymanager-address=127.0.0.1",
-    "--keymanager-port=" & $keymanagerPortBN,
+    "--keymanager-port=" & $(basePort + PortKind.KeymanagerBN.ord),
     "--keymanager-token-file=" & tokenFilePath,
     "--suggested-fee-recipient=" & $defaultFeeRecipient,
     "--doppelganger-detection=off"], it))
@@ -303,21 +300,21 @@ proc startBeaconNode {.raises: [Defect, CatchableError].} =
 
   # os.removeDir dataDir
 
-proc startValidatorClient {.async, thread.} =
+proc startValidatorClient(basePort: int) {.async, thread.} =
   let rng = keys.newRng()
 
   copyHalfValidators(vcDataDir, false)
   addPreTestRemoteKeystores(vcValidatorsDir)
 
   let runValidatorClientConf = try: ValidatorClientConf.load(cmdLine = mapIt([
-    "--beacon-node=http://127.0.0.1:47000",
+    "--beacon-node=http://127.0.0.1:" & $(basePort + PortKind.KeymanagerBN.ord),
     "--data-dir=" & vcDataDir,
     "--validators-dir=" & vcValidatorsDir,
     "--secrets-dir=" & vcSecretsDir,
     "--suggested-fee-recipient=" & $defaultFeeRecipient,
     "--keymanager=true",
     "--keymanager-address=127.0.0.1",
-    "--keymanager-port=" & $keymanagerPortVC,
+    "--keymanager-port=" & $(basePort + PortKind.KeymanagerVC.ord),
     "--keymanager-token-file=" & tokenFilePath], it))
   except:
     quit 1
@@ -499,13 +496,7 @@ proc runTests(keymanager: KeymanagerToTest) {.async.} =
           ]
         )
 
-    keymanagerKind =
+    testFlavour = " [" & keymanager.ident & "]" & preset()
-      if keymanager.port == keymanagerPortBN:
-        " [Beacon Node]"
-      else:
-        " [Validator Client]"
-
-    testFlavour = keymanagerKind & preset()
 
   suite "Serialization/deserialization" & testFlavour:
     proc `==`(a, b: Kdf): bool =
@@ -1278,11 +1269,24 @@ proc runTests(keymanager: KeymanagerToTest) {.async.} =
         response.status == 403
         responseJson["message"].getStr() == InvalidAuthorizationError
 
-proc delayedTests {.async.} =
+proc delayedTests(basePort: int) {.async.} =
+  let
+    beaconNodeKeymanager = KeymanagerToTest(
+      ident: "Beacon Node",
+      port: basePort + PortKind.KeymanagerBN.ord,
+      validatorsDir: nodeValidatorsDir,
+      secretsDir: nodeSecretsDir)
+
+    validatorClientKeymanager = KeymanagerToTest(
+      ident: "Validator Client",
+      port: basePort + PortKind.KeymanagerVC.ord,
+      validatorsDir: vcValidatorsDir,
+      secretsDir: vcSecretsDir)
+
   while bnStatus != BeaconNodeStatus.Running:
     await sleepAsync(1.seconds)
 
-  asyncSpawn startValidatorClient()
+  asyncSpawn startValidatorClient(basePort)
 
   await sleepAsync(2.seconds)
 
@@ -1296,13 +1300,26 @@ proc delayedTests {.async.} =
 
   bnStatus = BeaconNodeStatus.Stopping
 
-proc main() {.async.} =
+proc main(basePort: int) {.async.} =
   if dirExists(dataDir):
     os.removeDir dataDir
 
-  asyncSpawn delayedTests()
+  asyncSpawn delayedTests(basePort)
 
   prepareNetwork()
-  startBeaconNode()
+  startBeaconNode(basePort)
 
-waitFor main()
+let
+  basePortStr = os.getEnv("NIMBUS_TEST_KEYMANAGER_BASE_PORT", $defaultBasePort)
+  basePort =
+    try:
+      let val = parseInt(basePortStr)
+      if val < 0 or val > (uint16.high.int - PortKind.high.ord):
+        fatal "Invalid base port arg", basePort = basePortStr
+        quit 1
+      val
+    except ValueError as exc:
+      fatal "Invalid base port arg", basePort = basePortStr, exc = exc.msg
+      quit 1
+
+waitFor main(basePort)