p2p/pex: Allow configured seed nodes to not be resolvable over DNS (#2129)

* p2p/pex: Allow configured seed nodes to be offline Previously you couldn't startup tendermint if a seed node was offline. This now allows you to startup tendermint, as long as all seed node addresses are formatted correctly. In the event that all seed nodes are down, and the address book is empty, then it crashes with an informative error msg. (This case doesn't occur if no seeds were specified) Closes #1716 * (Squash this) Address melekes' comments * (squash this) fix package imports * (squash this) fix pex_reactor comment * (squash this) add a test case
2018-08-04 17:35:08 -07:00 · 2018-08-04 17:35:08 -07:00 · 6e3c5e8033
parent 8073e51b04
commit 6e3c5e8033
5 changed files with 145 additions and 71 deletions
--- a/CHANGELOG_PENDING.md
+++ b/CHANGELOG_PENDING.md
@ -26,4 +26,5 @@ BUG FIXES:
 - [common] Safely handle cases where atomic write files already exist [#2109](https://github.com/tendermint/tendermint/issues/2109)
 - [privval] fix a deadline for accepting new connections in socket private
  validator.
- [node] Fully exit when CTRL-C is pressed even if consensus state panics [#2072]
+- [p2p] Allow startup if a configured seed node's IP can't be resolved ([#1716](https://github.com/tendermint/tendermint/issues/1716))
+- [node] Fully exit when CTRL-C is pressed even if consensus state panics [#2072](https://github.com/tendermint/tendermint/issues/2072)
--- a/p2p/pex/addrbook.go
+++ b/p2p/pex/addrbook.go
@ -45,6 +45,9 @@ type AddrBook interface {

 	// Do we need more peers?
 	NeedMoreAddrs() bool
+	// Is Address Book Empty? Answer should not depend on being in your own
+	// address book, or private peers
+	Empty() bool

 	// Pick an address to dial
 	PickAddress(biasTowardsNewAddrs int) *p2p.NetAddress
@ -223,6 +226,12 @@ func (a *addrBook) NeedMoreAddrs() bool {
 	return a.Size() < needAddressThreshold
 }

+// Empty implements AddrBook - returns true if there are no addresses in the address book.
+// Does not count the peer appearing in its own address book, or private peers.
+func (a *addrBook) Empty() bool {
+	return a.Size() == 0
+}
+
 // PickAddress implements AddrBook. It picks an address to connect to.
 // The address is picked randomly from an old or new bucket according
 // to the biasTowardsNewAddrs argument, which must be between [0, 100] (or else is truncated to that range)
--- a/p2p/pex/addrbook_test.go
+++ b/p2p/pex/addrbook_test.go
@ -7,6 +7,8 @@ import (
 	"os"
 	"testing"

+	"github.com/stretchr/testify/require"
+
 	"github.com/stretchr/testify/assert"

 	cmn "github.com/tendermint/tendermint/libs/common"
@ -355,6 +357,40 @@ func TestAddrBookHasAddress(t *testing.T) {
 	assert.False(t, book.HasAddress(addr))
 }

+func testCreatePrivateAddrs(t *testing.T, numAddrs int) ([]*p2p.NetAddress, []string) {
+	addrs := make([]*p2p.NetAddress, numAddrs)
+	for i := 0; i < numAddrs; i++ {
+		addrs[i] = randIPv4Address(t)
+	}
+
+	private := make([]string, numAddrs)
+	for i, addr := range addrs {
+		private[i] = string(addr.ID)
+	}
+	return addrs, private
+}
+
+func TestAddrBookEmpty(t *testing.T) {
+	fname := createTempFileName("addrbook_test")
+	defer deleteTempFile(fname)
+
+	book := NewAddrBook(fname, true)
+	book.SetLogger(log.TestingLogger())
+	// Check that empty book is empty
+	require.True(t, book.Empty())
+	// Check that book with our address is empty
+	book.AddOurAddress(randIPv4Address(t))
+	require.True(t, book.Empty())
+	// Check that book with private addrs is empty
+	_, privateIds := testCreatePrivateAddrs(t, 5)
+	book.AddPrivateIDs(privateIds)
+	require.True(t, book.Empty())
+
+	// Check that book with address is not empty
+	book.AddAddress(randIPv4Address(t), randIPv4Address(t))
+	require.False(t, book.Empty())
+}
+
 func TestPrivatePeers(t *testing.T) {
 	fname := createTempFileName("addrbook_test")
 	defer deleteTempFile(fname)
@ -362,15 +398,7 @@ func TestPrivatePeers(t *testing.T) {
 	book := NewAddrBook(fname, true)
 	book.SetLogger(log.TestingLogger())

-	addrs := make([]*p2p.NetAddress, 10)
-	for i := 0; i < 10; i++ {
-		addrs[i] = randIPv4Address(t)
-	}
-
-	private := make([]string, 10)
-	for i, addr := range addrs {
-		private[i] = string(addr.ID)
-	}
+	addrs, private := testCreatePrivateAddrs(t, 10)
 	book.AddPrivateIDs(private)

 	// private addrs must not be added
--- a/p2p/pex/pex_reactor.go
+++ b/p2p/pex/pex_reactor.go
@ -7,9 +7,10 @@ import (
 	"sync"
 	"time"

+	"github.com/pkg/errors"
+
 	amino "github.com/tendermint/go-amino"
 	cmn "github.com/tendermint/tendermint/libs/common"
-
 	"github.com/tendermint/tendermint/p2p"
 	"github.com/tendermint/tendermint/p2p/conn"
 )
@ -120,11 +121,11 @@ func (r *PEXReactor) OnStart() error {
 		return err
 	}

-	// return err if user provided a bad seed address
-	// or a host name that we cant resolve
-	seedAddrs, err := r.checkSeeds()
+	numOnline, seedAddrs, err := r.checkSeeds()
 	if err != nil {
 		return err
+	} else if numOnline == 0 && r.book.Empty() {
+		return errors.New("Address book is empty, and could not connect to any seed nodes")
 	}

 	r.seedAddrs = seedAddrs
@ -478,19 +479,27 @@ func (r *PEXReactor) dialPeer(addr *p2p.NetAddress) {
 	}
 }

-// check seed addresses are well formed
-func (r *PEXReactor) checkSeeds() ([]*p2p.NetAddress, error) {
+// checkSeeds checks that addresses are well formed.
+// Returns number of seeds we can connect to, along with all seeds addrs.
+// return err if user provided any badly formatted seed addresses.
+// Doesn't error if the seed node can't be reached.
+// numOnline returns -1 if no seed nodes were in the initial configuration.
+func (r *PEXReactor) checkSeeds() (numOnline int, netAddrs []*p2p.NetAddress, err error) {
 	lSeeds := len(r.config.Seeds)
 	if lSeeds == 0 {
-		return nil, nil
+		return -1, nil, nil
 	}
 	netAddrs, errs := p2p.NewNetAddressStrings(r.config.Seeds)
+	numOnline = lSeeds - len(errs)
 	for _, err := range errs {
-		if err != nil {
-			return nil, err
+		switch e := err.(type) {
+		case p2p.ErrNetAddressLookup:
+			r.Logger.Error("Connecting to seed failed", "err", e)
+		default:
+			return 0, nil, errors.Wrap(e, "seed node configuration has error")
 		}
 	}
-	return netAddrs, nil
+	return
 }

 // randomly dial seeds until we connect to one or exhaust them
--- a/p2p/pex/pex_reactor_test.go
+++ b/p2p/pex/pex_reactor_test.go
@ -204,6 +204,45 @@ func TestPEXReactorAddrsMessageAbuse(t *testing.T) {
 	assert.False(t, sw.Peers().Has(peer.ID()))
 }

+func TestCheckSeeds(t *testing.T) {
+	// directory to store address books
+	dir, err := ioutil.TempDir("", "pex_reactor")
+	require.Nil(t, err)
+	defer os.RemoveAll(dir) // nolint: errcheck
+
+	// 1. test creating peer with no seeds works
+	peer := testCreateDefaultPeer(dir, 0)
+	require.Nil(t, peer.Start())
+	peer.Stop()
+
+	// 2. create seed
+	seed := testCreateSeed(dir, 1, []*p2p.NetAddress{}, []*p2p.NetAddress{})
+
+	// 3. test create peer with online seed works
+	peer = testCreatePeerWithSeed(dir, 2, seed)
+	require.Nil(t, peer.Start())
+	peer.Stop()
+
+	// 4. test create peer with all seeds having unresolvable DNS fails
+	badPeerConfig := &PEXReactorConfig{
+		Seeds: []string{"ed3dfd27bfc4af18f67a49862f04cc100696e84d@bad.network.addr:26657",
+			"d824b13cb5d40fa1d8a614e089357c7eff31b670@anotherbad.network.addr:26657"},
+	}
+	peer = testCreatePeerWithConfig(dir, 2, badPeerConfig)
+	require.Error(t, peer.Start())
+	peer.Stop()
+
+	// 5. test create peer with one good seed address succeeds
+	badPeerConfig = &PEXReactorConfig{
+		Seeds: []string{"ed3dfd27bfc4af18f67a49862f04cc100696e84d@bad.network.addr:26657",
+			"d824b13cb5d40fa1d8a614e089357c7eff31b670@anotherbad.network.addr:26657",
+			seed.NodeInfo().NetAddress().String()},
+	}
+	peer = testCreatePeerWithConfig(dir, 2, badPeerConfig)
+	require.Nil(t, peer.Start())
+	peer.Stop()
+}
+
 func TestPEXReactorUsesSeedsIfNeeded(t *testing.T) {
 	// directory to store address books
 	dir, err := ioutil.TempDir("", "pex_reactor")
@ -231,30 +270,7 @@ func TestConnectionSpeedForPeerReceivedFromSeed(t *testing.T) {
 	defer os.RemoveAll(dir) // nolint: errcheck

 	// 1. create peer
-	peer := p2p.MakeSwitch(
-		cfg,
-		1,
-		"127.0.0.1",
-		"123.123.123",
-		func(i int, sw *p2p.Switch) *p2p.Switch {
-			book := NewAddrBook(filepath.Join(dir, "addrbook1.json"), false)
-			book.SetLogger(log.TestingLogger())
-			sw.SetAddrBook(book)
-
-			sw.SetLogger(log.TestingLogger())
-
-			r := NewPEXReactor(
-				book,
-				&PEXReactorConfig{},
-			)
-			r.SetLogger(log.TestingLogger())
-			sw.AddReactor("pex", r)
-			return sw
-		},
-	)
-	peer.AddListener(
-		p2p.NewDefaultListener("tcp://"+peer.NodeInfo().ListenAddr, "", false, log.TestingLogger()),
-	)
+	peer := testCreateDefaultPeer(dir, 1)
 	require.Nil(t, peer.Start())
 	defer peer.Stop()

@ -435,6 +451,40 @@ func assertPeersWithTimeout(
 	}
 }

+// Creates a peer with the provided config
+func testCreatePeerWithConfig(dir string, id int, config *PEXReactorConfig) *p2p.Switch {
+	peer := p2p.MakeSwitch(
+		cfg,
+		id,
+		"127.0.0.1",
+		"123.123.123",
+		func(i int, sw *p2p.Switch) *p2p.Switch {
+			book := NewAddrBook(filepath.Join(dir, fmt.Sprintf("addrbook%d.json", id)), false)
+			book.SetLogger(log.TestingLogger())
+			sw.SetAddrBook(book)
+
+			sw.SetLogger(log.TestingLogger())
+
+			r := NewPEXReactor(
+				book,
+				config,
+			)
+			r.SetLogger(log.TestingLogger())
+			sw.AddReactor("pex", r)
+			return sw
+		},
+	)
+	peer.AddListener(
+		p2p.NewDefaultListener("tcp://"+peer.NodeInfo().ListenAddr, "", false, log.TestingLogger()),
+	)
+	return peer
+}
+
+// Creates a peer with the default config
+func testCreateDefaultPeer(dir string, id int) *p2p.Switch {
+	return testCreatePeerWithConfig(dir, id, &PEXReactorConfig{})
+}
+
 // Creates a seed which knows about the provided addresses / source address pairs.
 // Starting and stopping the seed is left to the caller
 func testCreateSeed(dir string, id int, knownAddrs, srcAddrs []*p2p.NetAddress) *p2p.Switch {
@ -469,33 +519,10 @@ func testCreateSeed(dir string, id int, knownAddrs, srcAddrs []*p2p.NetAddress)
 // Creates a peer which knows about the provided seed.
 // Starting and stopping the peer is left to the caller
 func testCreatePeerWithSeed(dir string, id int, seed *p2p.Switch) *p2p.Switch {
-	peer := p2p.MakeSwitch(
-		cfg,
-		id,
-		"127.0.0.1",
-		"123.123.123",
-		func(i int, sw *p2p.Switch) *p2p.Switch {
-			book := NewAddrBook(filepath.Join(dir, fmt.Sprintf("addrbook%d.json", id)), false)
-			book.SetLogger(log.TestingLogger())
-			sw.SetAddrBook(book)
-
-			sw.SetLogger(log.TestingLogger())
-
-			r := NewPEXReactor(
-				book,
-				&PEXReactorConfig{
-					Seeds: []string{seed.NodeInfo().NetAddress().String()},
-				},
-			)
-			r.SetLogger(log.TestingLogger())
-			sw.AddReactor("pex", r)
-			return sw
-		},
-	)
-	peer.AddListener(
-		p2p.NewDefaultListener("tcp://"+peer.NodeInfo().ListenAddr, "", false, log.TestingLogger()),
-	)
-	return peer
+	conf := &PEXReactorConfig{
+		Seeds: []string{seed.NodeInfo().NetAddress().String()},
+	}
+	return testCreatePeerWithConfig(dir, id, conf)
 }

 func createReactor(conf *PEXReactorConfig) (r *PEXReactor, book *addrBook) {