Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix bootstrapping bug and add real test for bootstrapping #2210

Merged
merged 2 commits into from
Jan 17, 2016
Merged

fix bootstrapping bug and add real test for bootstrapping #2210

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
52da1e3
fix bootstrapping bug and add real test for bootstrapping
whyrusleeping Jan 17, 2016
39d0b33
address CR feedback
whyrusleeping Jan 17, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 4 additions & 17 deletions core/bootstrap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
math2 "github.com/ipfs/go-ipfs/thirdparty/math2"
lgbl "github.com/ipfs/go-ipfs/util/eventlog/loggables"

ma "github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/jbenet/go-multiaddr"
goprocess "github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/jbenet/goprocess"
procctx "github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/jbenet/goprocess/context"
periodicproc "github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/jbenet/goprocess/periodic"
Expand Down Expand Up @@ -202,7 +201,7 @@ func bootstrapConnect(ctx context.Context, ph host.Host, peers []peer.PeerInfo)
return nil
}

func toPeerInfos(bpeers []config.BootstrapPeer) []peer.PeerInfo {
func toPeerInfos(bpeers []config.BootstrapPeer) ([]peer.PeerInfo, error) {
Copy link
Member

Choose a reason for hiding this comment

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

where do you make use of the error?

Copy link
Member

Choose a reason for hiding this comment

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

i only see return peers, nil

Copy link
Member Author

Choose a reason for hiding this comment

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

oh whoops... I was using the iaddr.ParseMultiaddr to parse the /ipfs/Qmasdasd part off of the addresses, then i realized that was already being done for me.

Copy link
Member

Choose a reason for hiding this comment

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

also https://travis-ci.org/ipfs/go-ipfs/jobs/102864391#L183

pinfos := make(map[peer.ID]*peer.PeerInfo)
for _, bootstrap := range bpeers {
pinfo, ok := pinfos[bootstrap.ID()]
Expand All @@ -211,28 +210,16 @@ func toPeerInfos(bpeers []config.BootstrapPeer) []peer.PeerInfo {
pinfos[bootstrap.ID()] = pinfo
pinfo.ID = bootstrap.ID()
}
pinfo.Addrs = append(pinfo.Addrs, bootstrap.Multiaddr())

pinfo.Addrs = append(pinfo.Addrs, bootstrap.Transport())
}

var peers []peer.PeerInfo
for _, pinfo := range pinfos {
peers = append(peers, *pinfo)
}

return peers
}

func toPeerInfo(bp config.BootstrapPeer) peer.PeerInfo {
// for now, we drop the "ipfs addr" part of the multiaddr. the rest
// of the codebase currently uses addresses without the peerid part.
m := bp.Multiaddr()
s := ma.Split(m)
m = ma.Join(s[:len(s)-1]...)

return peer.PeerInfo{
ID: bp.ID(),
Addrs: []ma.Multiaddr{m},
}
return peers, nil
}

func randomSubsetOfPeers(in []peer.PeerInfo, max int) []peer.PeerInfo {
Expand Down
2 changes: 1 addition & 1 deletion core/core.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ func (n *IpfsNode) loadBootstrapPeers() ([]peer.PeerInfo, error) {
if err != nil {
return nil, err
}
return toPeerInfos(parsed), nil
return toPeerInfos(parsed)
}

func (n *IpfsNode) loadFilesRoot() error {
Expand Down
65 changes: 65 additions & 0 deletions test/sharness/t0121-bootstrap-iptb.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
#!/bin/sh
#
# Copyright (c) 2016 Jeromy Johnson
Copy link
Member Author

Choose a reason for hiding this comment

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

i stopped and admired the fact that the script i copied this from was 2014. I had to bump it two years forward

Copy link
Member

Choose a reason for hiding this comment

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

👍

# MIT Licensed; see the LICENSE file in this repository.
#

# changing the bootstrap peers will require changing it in two places :)
test_description="test node bootstrapping"

. lib/test-lib.sh

test_init_ipfs

test_expect_success "disable mdns" '
ipfs config Discovery.MDNS.Enabled false --json
'

test_launch_ipfs_daemon

export IPTB_ROOT="`pwd`/.iptb"

ipfsi() {
dir="$1"
shift
IPFS_PATH="$IPTB_ROOT/$dir" ipfs $@
}

check_has_connection() {
node=$1
ipfsi $node swarm peers | grep ipfs > /dev/null
}

test_expect_success "setup iptb nodes" '
iptb init -n 5 -f --bootstrap=none --port=0
'

Copy link
Member

Choose a reason for hiding this comment

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

let's try this here:

test_expect_success "start up iptb nodes" '
    iptb start --wait
'

test_expect_success "check peers works" '
    ipfs swarm peers >peers_out
'

test_expect_success "correct number of peers" '
    test_empty $(cat peers_out)
'

Copy link
Member Author

Choose a reason for hiding this comment

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

why is that better?

test_expect_success "set bootstrap addrs" '
bsn_peer_id=$(ipfs id -f "<id>") &&
BADDR="/ip4/127.0.0.1/tcp/$PORT_SWARM/ipfs/$bsn_peer_id" &&
ipfsi 0 bootstrap add $BADDR &&
ipfsi 1 bootstrap add $BADDR &&
ipfsi 2 bootstrap add $BADDR &&
ipfsi 3 bootstrap add $BADDR &&
ipfsi 4 bootstrap add $BADDR
'

test_expect_success "start up iptb nodes" '
iptb start --wait
'

test_expect_success "check peers works" '
ipfs swarm peers > peers_out
'

test_expect_success "correct number of peers" '
test `cat peers_out | wc -l` == 5
Copy link
Member

Choose a reason for hiding this comment

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

'

test_kill_ipfs_daemon

test_expect_success "bring down iptb nodes" '
iptb stop
'

test_done