Avoid port conflicts in acceptance tests

acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/jsonrpc/Perm.java

@@ -12,7 +12,9 @@
  */
 package tech.pegasys.pantheon.tests.acceptance.dsl.jsonrpc;
 
-import tech.pegasys.pantheon.ethereum.permissioning.WhitelistPersistor;
+import static java.util.Arrays.asList;
+
+import tech.pegasys.pantheon.ethereum.permissioning.WhitelistPersistor.WHITELIST_TYPE;
 import tech.pegasys.pantheon.tests.acceptance.dsl.condition.Condition;
 import tech.pegasys.pantheon.tests.acceptance.dsl.condition.perm.AddAccountsToWhitelistSuccessfully;
 import tech.pegasys.pantheon.tests.acceptance.dsl.condition.perm.AddNodeSuccess;
@@ -21,12 +23,14 @@
 import tech.pegasys.pantheon.tests.acceptance.dsl.condition.perm.RemoveAccountsFromWhitelistSuccessfully;
 import tech.pegasys.pantheon.tests.acceptance.dsl.condition.perm.RemoveNodeSuccess;
 import tech.pegasys.pantheon.tests.acceptance.dsl.condition.perm.WhiteListContainsKeyAndValue;
+import tech.pegasys.pantheon.tests.acceptance.dsl.node.Node;
+import tech.pegasys.pantheon.tests.acceptance.dsl.node.RunnableNode;
 import tech.pegasys.pantheon.tests.acceptance.dsl.transaction.Transactions;
 
 import java.nio.file.Path;
-import java.util.Arrays;
-import java.util.Collection;
 import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 public class Perm {
 
@@ -47,14 +51,32 @@ public Condition removeAccountsFromWhitelist(final String... accounts) {
 
   public Condition expectAccountsWhitelist(final String... expectedAccounts) {
     return new GetExpectedAccountsWhitelist(
-        transactions.getAccountsWhiteList(), Arrays.asList(expectedAccounts));
+        transactions.getAccountsWhiteList(), asList(expectedAccounts));
+  }
+
+  public Condition addNodesToWhitelist(final String... nodes) {
+    return addNodesToWhitelist(asList(nodes));
   }
 
-  public Condition addNodesToWhitelist(final List<String> enodeList) {
+  public Condition addNodesToWhitelist(final Node... nodes) {
+    final List<String> enodeList = toEnodeUris(nodes);
+    return addNodesToWhitelist(enodeList);
+  }
+
+  private Condition addNodesToWhitelist(final List<String> enodeList) {
     return new AddNodeSuccess(transactions.addNodesToWhitelist(enodeList));
   }
 
-  public Condition removeNodesFromWhitelist(final List<String> enodeList) {
+  public Condition removeNodesFromWhitelist(final String... nodes) {
+    return removeNodesFromWhitelist(asList(nodes));
+  }
+
+  public Condition removeNodesFromWhitelist(final Node... nodes) {
+    final List<String> enodeList = toEnodeUris(nodes);
+    return removeNodesFromWhitelist(enodeList);
+  }
+
+  private Condition removeNodesFromWhitelist(final List<String> enodeList) {
     return new RemoveNodeSuccess(transactions.removeNodesFromWhitelist(enodeList));
   }
 
@@ -63,9 +85,14 @@ public Condition getNodesWhitelist(final int expectedNodeNum) {
   }
 
   public Condition expectPermissioningWhitelistFileKeyValue(
-      final WhitelistPersistor.WHITELIST_TYPE whitelistType,
-      final Collection<String> val,
-      final Path configFilePath) {
-    return new WhiteListContainsKeyAndValue(whitelistType, val, configFilePath);
+      final WHITELIST_TYPE whitelistType, final Path configFilePath, final String... val) {
+    return new WhiteListContainsKeyAndValue(whitelistType, asList(val), configFilePath);
+  }
+
+  private List<String> toEnodeUris(final Node[] nodes) {
+    return Stream.of(nodes)
+        .map(node -> (RunnableNode) node)
+        .map(RunnableNode::enodeUrl)
+        .collect(Collectors.toList());
   }
 }

acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/Node.java

@@ -18,6 +18,7 @@
 import tech.pegasys.pantheon.tests.acceptance.dsl.waitcondition.WaitCondition;
 
 public interface Node {
+
   <T> T execute(Transaction<T> transaction);
 
   <T> T executeHttpTransaction(HttpTransaction<T> transaction);

acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/NodeConfiguration.java

@@ -20,8 +20,6 @@
 
 public interface NodeConfiguration {
 
-  String enodeUrl();
-
   void bootnodes(List<String> bootnodes);
 
   List<URI> bootnodes();
@@ -42,5 +40,7 @@ public interface NodeConfiguration {
 
   void ethNetworkConfig(Optional<EthNetworkConfig> ethNetworkConfig);
 
-  boolean isBootnode();
+  boolean isP2pEnabled();
+
+  boolean isDiscoveryEnabled();
 }

acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/PantheonNode.java

@@ -67,14 +67,13 @@
 import org.web3j.protocol.websocket.WebSocketService;
 import org.web3j.utils.Async;
 
-public class PantheonNode implements Node, NodeConfiguration, RunnableNode, AutoCloseable {
+public class PantheonNode implements NodeConfiguration, RunnableNode, AutoCloseable {
 
   private static final String LOCALHOST = "127.0.0.1";
   private static final Logger LOG = getLogger();
 
   private final Path homeDirectory;
   private final KeyPair keyPair;
-  private final int p2pPort;
   private final Properties portsProperties = new Properties();
   private final Boolean p2pEnabled;
 
@@ -88,7 +87,6 @@ public class PantheonNode implements Node, NodeConfiguration, RunnableNode, Auto
   private final GenesisConfigProvider genesisConfigProvider;
   private final boolean devMode;
   private final boolean discoveryEnabled;
-  private final boolean isBootnode;
   private final List<String> bootnodes;
 
   private JsonRequestFactories jsonRequestFactories;
@@ -107,15 +105,12 @@ public PantheonNode(
       final Optional<PermissioningConfiguration> permissioningConfiguration,
       final boolean devMode,
       final GenesisConfigProvider genesisConfigProvider,
-      final int p2pPort,
       final Boolean p2pEnabled,
       final boolean discoveryEnabled,
-      final boolean isBootnode,
       final List<String> bootnodes)
       throws IOException {
     this.homeDirectory = Files.createTempDirectory("acctest");
     this.keyPair = KeyPairUtil.loadKeyPair(homeDirectory);
-    this.p2pPort = p2pPort;
     this.name = name;
     this.miningParameters = miningParameters;
     this.privacyParameters = privacyParameters;
@@ -127,7 +122,6 @@ public PantheonNode(
     this.devMode = devMode;
     this.p2pEnabled = p2pEnabled;
     this.discoveryEnabled = discoveryEnabled;
-    this.isBootnode = isBootnode;
     this.bootnodes = bootnodes;
     LOG.info("Created PantheonNode {}", this.toString());
   }
@@ -147,12 +141,31 @@ public String getName() {
 
   @Override
   public String enodeUrl() {
+    final String discport = isDiscoveryEnabled() ? "?discport=" + getDiscoveryPort() : "";
     return "enode://"
         + keyPair.getPublicKey().toString().substring(2)
         + "@"
         + LOCALHOST
         + ":"
-        + p2pPort;
+        + getP2pPort()
+        + discport;
+  }
+
+  private String getP2pPort() {
+    final String port = portsProperties.getProperty("p2p");
+    if (port == null) {
+      throw new IllegalStateException("Requested p2p port before ports properties was written");
+    }
+    return port;
+  }
+
+  private String getDiscoveryPort() {
+    final String port = portsProperties.getProperty("discovery");
+    if (port == null) {
+      throw new IllegalStateException(
+          "Requested discovery port before ports properties was written");
+    }
+    return port;
   }
 
   private Optional<String> jsonRpcBaseUrl() {
@@ -218,7 +231,7 @@ private JsonRequestFactories jsonRequestFactories() {
         web3jService = new WebSocketService(wsClient, false);
         try {
           ((WebSocketService) web3jService).connect();
-        } catch (ConnectException e) {
+        } catch (final ConnectException e) {
           throw new RuntimeException(e);
         }
 
@@ -299,10 +312,6 @@ public void useAuthenticationTokenInHeaderForJsonRpc(final String token) {
     this.token = token;
   }
 
-  public KeyPair getKeyPair() {
-    return keyPair;
-  }
-
   private void checkIfWebSocketEndpointIsAvailable(final String url) {
     final WebSocketClient webSocketClient = new WebSocketClient(URI.create(url));
     // Web3j implementation always invoke the listener (even when one hasn't been set). We are using
@@ -359,6 +368,7 @@ private void loadPortsFile() {
     try (final FileInputStream fis =
         new FileInputStream(new File(homeDirectory.toFile(), "pantheon.ports"))) {
       portsProperties.load(fis);
+      LOG.info("Ports for node {}: {}", name, portsProperties);
     } catch (final IOException e) {
       throw new RuntimeException("Error reading Pantheon ports file", e);
     }
@@ -418,27 +428,17 @@ MetricsConfiguration metricsConfiguration() {
     return metricsConfiguration;
   }
 
-  int p2pPort() {
-    return p2pPort;
-  }
-
   String p2pListenHost() {
     return LOCALHOST;
   }
 
-  int p2pListenPort() {
-    return p2pPort;
-  }
-
   @Override
   public List<URI> bootnodes() {
-    return bootnodes.stream()
-        .filter(node -> !node.equals(this.enodeUrl()))
-        .map(URI::create)
-        .collect(Collectors.toList());
+    return bootnodes.stream().map(URI::create).collect(Collectors.toList());
   }
 
-  Boolean p2pEnabled() {
+  @Override
+  public boolean isP2pEnabled() {
     return p2pEnabled;
   }
 
@@ -460,15 +460,11 @@ public boolean isDevMode() {
     return devMode;
   }
 
+  @Override
   public boolean isDiscoveryEnabled() {
     return discoveryEnabled;
   }
 
-  @Override
-  public boolean isBootnode() {
-    return isBootnode;
-  }
-
   Optional<PermissioningConfiguration> getPermissioningConfiguration() {
     return permissioningConfiguration;
   }
@@ -477,7 +473,6 @@ Optional<PermissioningConfiguration> getPermissioningConfiguration() {
   public String toString() {
     return MoreObjects.toStringHelper(this)
         .add("name", name)
-        .add("p2pPort", p2pPort)
         .add("homeDirectory", homeDirectory)
         .add("keyPair", keyPair)
         .add("p2pEnabled", p2pEnabled)

acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/ProcessPantheonNodeRunner.java

@@ -18,8 +18,11 @@
 import tech.pegasys.pantheon.ethereum.jsonrpc.RpcApi;
 import tech.pegasys.pantheon.ethereum.jsonrpc.RpcApis;
 
+import java.io.BufferedReader;
 import java.io.File;
 import java.io.IOException;
+import java.io.InputStreamReader;
+import java.lang.ProcessBuilder.Redirect;
 import java.net.URI;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -28,6 +31,8 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 
@@ -38,8 +43,10 @@
 public class ProcessPantheonNodeRunner implements PantheonNodeRunner {
 
   private final Logger LOG = LogManager.getLogger();
+  private final Logger PROCESS_LOG = LogManager.getLogger("tech.pegasys.pantheon.SubProcessLog");
 
   private final Map<String, Process> pantheonProcesses = new HashMap<>();
+  private final ExecutorService outputProcessorExecutor = Executors.newCachedThreadPool();
 
   ProcessPantheonNodeRunner() {
     Runtime.getRuntime().addShutdownHook(new Thread(this::shutdown));
@@ -51,6 +58,7 @@ public void startNode(final PantheonNode node) {
 
     final List<String> params = new ArrayList<>();
     params.add("build/install/pantheon/bin/pantheon");
+
     params.add("--data-path");
     params.add(dataDir.toAbsolutePath().toString());
 
@@ -66,7 +74,7 @@ public void startNode(final PantheonNode node) {
     params.add(node.p2pListenHost());
 
     params.add("--p2p-port");
-    params.add(Integer.toString(node.p2pListenPort()));
+    params.add("0");
 
     if (node.getMiningParameters().isMiningEnabled()) {
       params.add("--miner-enabled");
@@ -133,7 +141,7 @@ public void startNode(final PantheonNode node) {
       params.add(Integer.toString(ethNetworkConfig.getNetworkId()));
     }
 
-    if (!node.p2pEnabled()) {
+    if (!node.isP2pEnabled()) {
       params.add("--p2p-enabled");
       params.add("false");
     }
@@ -157,10 +165,12 @@ public void startNode(final PantheonNode node) {
     final ProcessBuilder processBuilder =
         new ProcessBuilder(params)
             .directory(new File(System.getProperty("user.dir")).getParentFile())
-            .inheritIO();
+            .redirectErrorStream(true)
+            .redirectInput(Redirect.INHERIT);
 
     try {
       final Process process = processBuilder.start();
+      outputProcessorExecutor.submit(() -> printOutput(node, process));
       pantheonProcesses.put(node.getName(), process);
     } catch (final IOException e) {
       LOG.error("Error starting PantheonNode process", e);
@@ -169,6 +179,19 @@ public void startNode(final PantheonNode node) {
     waitForPortsFile(dataDir);
   }
 
+  private void printOutput(final PantheonNode node, final Process process) {
+    try (final BufferedReader in =
+        new BufferedReader(new InputStreamReader(process.getInputStream(), UTF_8))) {
+      String line = in.readLine();
+      while (line != null) {
+        PROCESS_LOG.info("{}: {}", node.getName(), line);
+        line = in.readLine();
+      }
+    } catch (final IOException e) {
+      LOG.error("Failed to read output from process", e);
+    }
+  }
+
   private Path createGenesisFile(final PantheonNode node, final EthNetworkConfig ethNetworkConfig) {
     try {
       final Path genesisFile = Files.createTempFile(node.homeDirectory(), "genesis", "");
@@ -197,6 +220,15 @@ public void stopNode(final PantheonNode node) {
   public synchronized void shutdown() {
     final HashMap<String, Process> localMap = new HashMap<>(pantheonProcesses);
     localMap.forEach(this::killPantheonProcess);
+    outputProcessorExecutor.shutdown();
+    try {
+      if (!outputProcessorExecutor.awaitTermination(5, TimeUnit.SECONDS)) {
+        LOG.error("Output processor executor did not shutdown cleanly.");
+      }
+    } catch (final InterruptedException e) {
+      LOG.error("Interrupted while already shutting down", e);
+      Thread.currentThread().interrupt();
+    }
   }
 
   @Override

acceptance-tests/src/test/java/tech/pegasys/pantheon/tests/acceptance/dsl/node/RunnableNode.java

@@ -30,4 +30,6 @@ public interface RunnableNode extends Node {
   String getName();
 
   Address getAddress();
+
+  String enodeUrl();
 }