From 6e3c5e803322d7ae08dbbe6f885899c6204beed5 Mon Sep 17 00:00:00 2001 From: Dev Ojha Date: Sat, 4 Aug 2018 17:35:08 -0700 Subject: [PATCH] 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 --- CHANGELOG_PENDING.md | 3 +- p2p/pex/addrbook.go | 9 +++ p2p/pex/addrbook_test.go | 46 ++++++++++--- p2p/pex/pex_reactor.go | 29 +++++--- p2p/pex/pex_reactor_test.go | 129 ++++++++++++++++++++++-------------- 5 files changed, 145 insertions(+), 71 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 010aea40e..a1d9985ab 100644 --- 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) diff --git a/p2p/pex/addrbook.go b/p2p/pex/addrbook.go index 9596b1d76..ad6e0c00b 100644 --- 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) diff --git a/p2p/pex/addrbook_test.go b/p2p/pex/addrbook_test.go index 761c11874..ade02d490 100644 --- 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,22 +357,48 @@ func TestAddrBookHasAddress(t *testing.T) { assert.False(t, book.HasAddress(addr)) } -func TestPrivatePeers(t *testing.T) { +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()) +} - addrs := make([]*p2p.NetAddress, 10) - for i := 0; i < 10; i++ { - addrs[i] = randIPv4Address(t) - } +func TestPrivatePeers(t *testing.T) { + fname := createTempFileName("addrbook_test") + defer deleteTempFile(fname) - private := make([]string, 10) - for i, addr := range addrs { - private[i] = string(addr.ID) - } + book := NewAddrBook(fname, true) + book.SetLogger(log.TestingLogger()) + + addrs, private := testCreatePrivateAddrs(t, 10) book.AddPrivateIDs(private) // private addrs must not be added diff --git a/p2p/pex/pex_reactor.go b/p2p/pex/pex_reactor.go index c59b3797d..4220dd42f 100644 --- 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 diff --git a/p2p/pex/pex_reactor_test.go b/p2p/pex/pex_reactor_test.go index 36f38c570..f72f81e06 100644 --- 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,67 +451,78 @@ func assertPeersWithTimeout( } } -// 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 { - seed := p2p.MakeSwitch( +// 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, "addrbookSeed.json"), false) + book := NewAddrBook(filepath.Join(dir, fmt.Sprintf("addrbook%d.json", id)), false) book.SetLogger(log.TestingLogger()) - for j := 0; j < len(knownAddrs); j++ { - book.AddAddress(knownAddrs[j], srcAddrs[j]) - book.MarkGood(knownAddrs[j]) - } sw.SetAddrBook(book) sw.SetLogger(log.TestingLogger()) - r := NewPEXReactor(book, &PEXReactorConfig{}) + r := NewPEXReactor( + book, + config, + ) r.SetLogger(log.TestingLogger()) sw.AddReactor("pex", r) return sw }, ) - seed.AddListener( - p2p.NewDefaultListener("tcp://"+seed.NodeInfo().ListenAddr, "", false, log.TestingLogger()), + peer.AddListener( + p2p.NewDefaultListener("tcp://"+peer.NodeInfo().ListenAddr, "", false, log.TestingLogger()), ) - return seed + return peer } -// 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( +// 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 { + seed := 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 := NewAddrBook(filepath.Join(dir, "addrbookSeed.json"), false) book.SetLogger(log.TestingLogger()) + for j := 0; j < len(knownAddrs); j++ { + book.AddAddress(knownAddrs[j], srcAddrs[j]) + book.MarkGood(knownAddrs[j]) + } sw.SetAddrBook(book) sw.SetLogger(log.TestingLogger()) - r := NewPEXReactor( - book, - &PEXReactorConfig{ - Seeds: []string{seed.NodeInfo().NetAddress().String()}, - }, - ) + 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()), + seed.AddListener( + p2p.NewDefaultListener("tcp://"+seed.NodeInfo().ListenAddr, "", false, log.TestingLogger()), ) - return peer + return seed +} + +// 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 { + conf := &PEXReactorConfig{ + Seeds: []string{seed.NodeInfo().NetAddress().String()}, + } + return testCreatePeerWithConfig(dir, id, conf) } func createReactor(conf *PEXReactorConfig) (r *PEXReactor, book *addrBook) {