Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

Avoid port conflicts in acceptance tests #1025

Merged
merged 27 commits into from
Mar 4, 2019

Conversation

ajsutton
Copy link
Contributor

@ajsutton ajsutton commented Mar 3, 2019

PR description

Acceptance tests were attempting to find an available port by starting a socket server with port 0, stopping it then assuming that port would remain unused which led to a lot of test failures when Pantheon failed to start up because of a port conflict.

This PR changes the behaviour so that Pantheon is always started with port 0 as the p2p port and then the port is read out of the pantheon.ports file. As a result, we need to start a cluster by first starting one node as the bootnode, getting it's enode URL (which requires the port) and then starting the others using that bootnode. The peer discovery process then ensures all peers in the cluster wind up connected to each other without requiring them all to be in the bootnode list.

Note this builds on the fixes in #1021, #1020, #1019 and #1018. The changeset will be much easier to review once they've been merged.

…wn before we write the ports file

Include the P2P TCP port in ports file even when peer discovery is disabled.
Load information from the advertised peer rather than the discovery listening socket.
Fix admin_nodeInfo to include the ?discport param in the enode URI when the discovery port differs from the P2P port.
…e which isn't responding doesn't return an error. P2P disabled nodes never open the port so this is equivalent.
@ajsutton ajsutton marked this pull request as ready for review March 4, 2019 04:11
Copy link
Contributor

@lucassaldanha lucassaldanha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. 🚀

@ajsutton ajsutton merged commit f284efe into PegaSysEng:master Mar 4, 2019
@ajsutton ajsutton deleted the port-conflicts branch March 4, 2019 23:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants