From ee6a35d9b564f9b5ed382c7bf8c5f9987df83d13 Mon Sep 17 00:00:00 2001 From: Kishan Mohanbhai Sagathiya Date: Sun, 11 Aug 2019 21:15:43 +0530 Subject: [PATCH 1/6] Sort addresses in `/id` and thus in GET `/peers` as well --- cluster.go | 7 +++++++ pstoremgr/pstoremgr.go | 3 +++ utils/utils.go | 12 ++++++++++++ 3 files changed, 22 insertions(+) create mode 100644 utils/utils.go diff --git a/cluster.go b/cluster.go index 2e6f875ba..840c11e2f 100644 --- a/cluster.go +++ b/cluster.go @@ -727,7 +727,14 @@ func (c *Cluster) ID(ctx context.Context) *api.ID { for _, addr := range c.host.Addrs() { addrsSet[addr.String()] = struct{}{} } + + var addrSorted []string for k := range addrsSet { + addrSorted = append(addrSorted, k) + } + sort.Strings(addrSorted) + + for _, k := range addrSorted { addr, _ := api.NewMultiaddr(k) addrs = append(addrs, api.MustLibp2pMultiaddrJoin(addr, c.id)) } diff --git a/pstoremgr/pstoremgr.go b/pstoremgr/pstoremgr.go index ee6dfd82f..c9e107845 100644 --- a/pstoremgr/pstoremgr.go +++ b/pstoremgr/pstoremgr.go @@ -15,6 +15,7 @@ import ( "time" logging "github.com/ipfs/go-log" + "github.com/ipfs/ipfs-cluster/utils" host "github.com/libp2p/go-libp2p-core/host" net "github.com/libp2p/go-libp2p-core/network" peer "github.com/libp2p/go-libp2p-core/peer" @@ -139,9 +140,11 @@ func (pm *Manager) filteredPeerAddrs(p peer.ID) []ma.Multiaddr { } if len(peerDNSAddrs) > 0 { + sort.Sort(utils.ByString(peerDNSAddrs)) return peerDNSAddrs } + sort.Sort(utils.ByString(peerAddrs)) return peerAddrs } diff --git a/utils/utils.go b/utils/utils.go new file mode 100644 index 000000000..d0872402f --- /dev/null +++ b/utils/utils.go @@ -0,0 +1,12 @@ +package utils + +import ( + ma "github.com/multiformats/go-multiaddr" +) + +// ByString can sort multiaddresses by its string +type ByString []ma.Multiaddr + +func (m ByString) Len() int { return len(m) } +func (m ByString) Swap(i, j int) { m[i], m[j] = m[j], m[i] } +func (m ByString) Less(i, j int) bool { return m[i].String() < m[j].String() } From e6d183fd70c1706e84401a0861163a81703b8bed Mon Sep 17 00:00:00 2001 From: Kishan Mohanbhai Sagathiya Date: Fri, 16 Aug 2019 16:50:28 +0530 Subject: [PATCH 2/6] Addressed reviews --- api/util.go | 14 -------------- cluster.go | 19 ++++--------------- pstoremgr/pstoremgr.go | 3 +-- 3 files changed, 5 insertions(+), 31 deletions(-) diff --git a/api/util.go b/api/util.go index 9f8a38aec..0cff0acda 100644 --- a/api/util.go +++ b/api/util.go @@ -2,7 +2,6 @@ package api import ( peer "github.com/libp2p/go-libp2p-core/peer" - ma "github.com/multiformats/go-multiaddr" ) // PeersToStrings IDB58Encodes a list of peers. @@ -29,16 +28,3 @@ func StringsToPeers(strs []string) []peer.ID { } return peers } - -// MustLibp2pMultiaddrJoin takes a LibP2P multiaddress and a peer ID and -// encapsulates a new /ipfs/ address. It will panic if the given -// peer ID is bad. -func MustLibp2pMultiaddrJoin(addr Multiaddr, p peer.ID) Multiaddr { - v := addr.Value() - pidAddr, err := ma.NewMultiaddr("/ipfs/" + peer.IDB58Encode(p)) - // let this break badly - if err != nil { - panic("called MustLibp2pMultiaddrJoin with bad peer!") - } - return Multiaddr{Multiaddr: v.Encapsulate(pidAddr)} -} diff --git a/cluster.go b/cluster.go index cb12334af..7a2d6fa34 100644 --- a/cluster.go +++ b/cluster.go @@ -721,22 +721,11 @@ func (c *Cluster) ID(ctx context.Context) *api.ID { Error: err.Error(), } } - var addrs []api.Multiaddr - - addrsSet := make(map[string]struct{}) // to filter dups - for _, addr := range c.host.Addrs() { - addrsSet[addr.String()] = struct{}{} - } - var addrSorted []string - for k := range addrsSet { - addrSorted = append(addrSorted, k) - } - sort.Strings(addrSorted) - - for _, k := range addrSorted { - addr, _ := api.NewMultiaddr(k) - addrs = append(addrs, api.MustLibp2pMultiaddrJoin(addr, c.id)) + var addrs []api.Multiaddr + mAddrs, _ := peer.AddrInfoToP2pAddrs(&peer.AddrInfo{ID: c.id, Addrs: c.host.Addrs()}) + for _, mAddr := range mAddrs { + addrs = append(addrs, api.NewMultiaddrWithValue(mAddr)) } peers := []peer.ID{} diff --git a/pstoremgr/pstoremgr.go b/pstoremgr/pstoremgr.go index c9e107845..1735fbc61 100644 --- a/pstoremgr/pstoremgr.go +++ b/pstoremgr/pstoremgr.go @@ -15,7 +15,7 @@ import ( "time" logging "github.com/ipfs/go-log" - "github.com/ipfs/ipfs-cluster/utils" + utils "github.com/ipfs/ipfs-cluster/utils" host "github.com/libp2p/go-libp2p-core/host" net "github.com/libp2p/go-libp2p-core/network" peer "github.com/libp2p/go-libp2p-core/peer" @@ -140,7 +140,6 @@ func (pm *Manager) filteredPeerAddrs(p peer.ID) []ma.Multiaddr { } if len(peerDNSAddrs) > 0 { - sort.Sort(utils.ByString(peerDNSAddrs)) return peerDNSAddrs } From c109a01343d75110dbb1413ea9178e3cd2c140c3 Mon Sep 17 00:00:00 2001 From: Kishan Mohanbhai Sagathiya Date: Mon, 26 Aug 2019 18:27:17 +0530 Subject: [PATCH 3/6] Sort peers for crdt consensus.Peers --- cluster.go | 15 +++++++++++---- consensus/crdt/consensus.go | 3 +++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/cluster.go b/cluster.go index 7a2d6fa34..48750a137 100644 --- a/cluster.go +++ b/cluster.go @@ -723,9 +723,11 @@ func (c *Cluster) ID(ctx context.Context) *api.ID { } var addrs []api.Multiaddr - mAddrs, _ := peer.AddrInfoToP2pAddrs(&peer.AddrInfo{ID: c.id, Addrs: c.host.Addrs()}) - for _, mAddr := range mAddrs { - addrs = append(addrs, api.NewMultiaddrWithValue(mAddr)) + mAddrs, err := peer.AddrInfoToP2pAddrs(&peer.AddrInfo{ID: c.id, Addrs: c.host.Addrs()}) + if err == nil { + for _, mAddr := range mAddrs { + addrs = append(addrs, api.NewMultiaddrWithValue(mAddr)) + } } peers := []peer.ID{} @@ -747,7 +749,7 @@ func (c *Cluster) ID(ctx context.Context) *api.ID { } } - return &api.ID{ + id := &api.ID{ ID: c.id, //PublicKey: c.host.Peerstore().PubKey(c.id), Addresses: addrs, @@ -758,6 +760,11 @@ func (c *Cluster) ID(ctx context.Context) *api.ID { IPFS: ipfsID, Peername: c.config.Peername, } + if err != nil { + id.Error = err.Error() + } + + return id } // PeerAdd adds a new peer to this Cluster. diff --git a/consensus/crdt/consensus.go b/consensus/crdt/consensus.go index c7d8d6ec7..02f530574 100644 --- a/consensus/crdt/consensus.go +++ b/consensus/crdt/consensus.go @@ -3,6 +3,7 @@ package crdt import ( "context" "errors" + "sort" "sync" "time" @@ -399,6 +400,8 @@ func (css *Consensus) Peers(ctx context.Context) ([]peer.ID, error) { peers = append(peers, css.host.ID()) } + sort.Sort(peer.IDSlice(peers)) + return peers, nil } From eddf4a1e4db01be216fd9c609639e6701076ac28 Mon Sep 17 00:00:00 2001 From: Kishan Mohanbhai Sagathiya Date: Tue, 27 Aug 2019 10:09:05 +0530 Subject: [PATCH 4/6] Added sharness test for peers ls --- sharness/t0033-ctl-peers.sh | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100755 sharness/t0033-ctl-peers.sh diff --git a/sharness/t0033-ctl-peers.sh b/sharness/t0033-ctl-peers.sh new file mode 100755 index 000000000..8c3e6819c --- /dev/null +++ b/sharness/t0033-ctl-peers.sh @@ -0,0 +1,29 @@ +#!/bin/bash + +test_description="Test cluster-ctl's peers command" + +. lib/test-lib.sh + +test_ipfs_init +test_cluster_init + +test_expect_success IPFS,CLUSTER "peers ls gives same results always (crdt)" ' + ipfs-cluster-ctl peers ls > peers1-crdt.txt + ipfs-cluster-ctl peers ls > peers2-crdt.txt + test_cmp peers1-crdt.txt peers2-crdt.txt +' + +cluster_kill +sleep 5 +test_cluster_init "" raft + +test_expect_success IPFS,CLUSTER "peers ls gives same results always (raft)" ' + ipfs-cluster-ctl peers ls > peers1-raft.txt + ipfs-cluster-ctl peers ls > peers2-raft.txt + test_cmp peers1-raft.txt peers2-raft.txt +' + +test_clean_ipfs +test_clean_cluster + +test_done From c9e72e235976608087f052425caa2c18d5904db6 Mon Sep 17 00:00:00 2001 From: Kishan Mohanbhai Sagathiya Date: Wed, 28 Aug 2019 09:55:03 +0530 Subject: [PATCH 5/6] Avoid using sharness tests when possible --- ipfscluster_test.go | 25 +++++++++++++++++++++++++ sharness/t0033-ctl-peers.sh | 29 ----------------------------- 2 files changed, 25 insertions(+), 29 deletions(-) delete mode 100755 sharness/t0033-ctl-peers.sh diff --git a/ipfscluster_test.go b/ipfscluster_test.go index 17fb208bd..96df8a008 100644 --- a/ipfscluster_test.go +++ b/ipfscluster_test.go @@ -1,7 +1,9 @@ package ipfscluster import ( + "bytes" "context" + "encoding/json" "errors" "flag" "fmt" @@ -565,6 +567,29 @@ func TestClustersPeers(t *testing.T) { } } +func TestClustersPeersRetainOrder(t *testing.T) { + ctx := context.Background() + clusters, mock := createClusters(t) + defer shutdownClusters(t, clusters, mock) + + delay() + + j := rand.Intn(nClusters) // choose a random cluster peer + peers1, err := json.Marshal(clusters[j].Peers(ctx)) + if err != nil { + t.Fatal(err) + } + + peers2, err := json.Marshal(clusters[j].Peers(ctx)) + if err != nil { + t.Fatal(err) + } + + if bytes.Compare(peers1, peers2) != 0 { + t.Error("expected both results to be same") + } +} + func TestClustersPin(t *testing.T) { ctx := context.Background() clusters, mock := createClusters(t) diff --git a/sharness/t0033-ctl-peers.sh b/sharness/t0033-ctl-peers.sh deleted file mode 100755 index 8c3e6819c..000000000 --- a/sharness/t0033-ctl-peers.sh +++ /dev/null @@ -1,29 +0,0 @@ -#!/bin/bash - -test_description="Test cluster-ctl's peers command" - -. lib/test-lib.sh - -test_ipfs_init -test_cluster_init - -test_expect_success IPFS,CLUSTER "peers ls gives same results always (crdt)" ' - ipfs-cluster-ctl peers ls > peers1-crdt.txt - ipfs-cluster-ctl peers ls > peers2-crdt.txt - test_cmp peers1-crdt.txt peers2-crdt.txt -' - -cluster_kill -sleep 5 -test_cluster_init "" raft - -test_expect_success IPFS,CLUSTER "peers ls gives same results always (raft)" ' - ipfs-cluster-ctl peers ls > peers1-raft.txt - ipfs-cluster-ctl peers ls > peers2-raft.txt - test_cmp peers1-raft.txt peers2-raft.txt -' - -test_clean_ipfs -test_clean_cluster - -test_done From 08d7b274af6eabe2487df8a1a75423b126392de0 Mon Sep 17 00:00:00 2001 From: Kishan Mohanbhai Sagathiya Date: Sat, 31 Aug 2019 13:46:19 +0530 Subject: [PATCH 6/6] Addressed reviews in test --- ipfscluster_test.go | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/ipfscluster_test.go b/ipfscluster_test.go index 96df8a008..7710ddd81 100644 --- a/ipfscluster_test.go +++ b/ipfscluster_test.go @@ -574,19 +574,24 @@ func TestClustersPeersRetainOrder(t *testing.T) { delay() - j := rand.Intn(nClusters) // choose a random cluster peer - peers1, err := json.Marshal(clusters[j].Peers(ctx)) - if err != nil { - t.Fatal(err) - } + for i := 0; i < 5; i++ { + j := rand.Intn(nClusters) // choose a random cluster peer + peers1, err := json.Marshal(clusters[j].Peers(ctx)) + if err != nil { + t.Fatal(err) + } - peers2, err := json.Marshal(clusters[j].Peers(ctx)) - if err != nil { - t.Fatal(err) - } + waitForLeaderAndMetrics(t, clusters) + + k := rand.Intn(nClusters) + peers2, err := json.Marshal(clusters[k].Peers(ctx)) + if err != nil { + t.Fatal(err) + } - if bytes.Compare(peers1, peers2) != 0 { - t.Error("expected both results to be same") + if bytes.Compare(peers1, peers2) != 0 { + t.Error("expected both results to be same") + } } }