Avoid out of index error in failover (#951)

Fixes panic when using the load balancing client.

api/rest/client/lbclient.go

@@ -24,7 +24,8 @@ type LBStrategy interface {
 	SetClients(clients []Client)
 }
 
-// RoundRobin is a load balancing strategy that would use clients in a sequence.
+// RoundRobin is a load balancing strategy that would use clients in a sequence
+// for all methods, throughout the lifetime of the lb client.
 type RoundRobin struct {
 	clients []Client
 	counter uint32
@@ -44,15 +45,16 @@ func (r *RoundRobin) SetClients(cl []Client) {
 	r.length = uint32(len(cl))
 }
 
-// Failover is a load balancing strategy that would try the local cluster peer first.
-// If the local call fail it would try other client in a round robin fashion.
+// Failover is a load balancing strategy that would try the first cluster peer
+// first. If the first call fails it would try other clients for that call in a
+// round robin fashion.
 type Failover struct {
 	clients []Client
 }
 
 // Next returns the next client to be used.
 func (f *Failover) Next(count int) Client {
-	return f.clients[count]
+	return f.clients[count%len(f.clients)]
 }
 
 // SetClients sets a list of clients for this strategy.

api/rest/client/lbclient_test.go

@@ -2,13 +2,15 @@ package client
 
 import (
 	"context"
+	"fmt"
 	"sync"
 	"testing"
 
+	"github.com/ipfs/ipfs-cluster/api"
 	ma "github.com/multiformats/go-multiaddr"
 )
 
-func TestLBClient(t *testing.T) {
+func TestFailoverConcurrently(t *testing.T) {
 	// Create a load balancing client with 5 empty clients and 5 clients with APIs
 	// say we want to retry the request for at most 5 times
 	cfgs := make([]*Config, 10)
@@ -39,6 +41,47 @@ func TestLBClient(t *testing.T) {
 	testRunManyRequestsConcurrently(t, cfgs, &Failover{}, 200, 5, false)
 }
 
+type dummyClient struct {
+	defaultClient
+	i int
+}
+
+// ID returns dummy client's serial number.
+func (d *dummyClient) ID(ctx context.Context) (*api.ID, error) {
+	return &api.ID{
+		Peername: fmt.Sprintf("%d", d.i),
+	}, nil
+}
+
+func TestRoundRobin(t *testing.T) {
+	var clients []Client
+	// number of clients
+	n := 5
+	// create n dummy clients
+	for i := 0; i < n; i++ {
+		c := &dummyClient{
+			i: i,
+		}
+		clients = append(clients, c)
+	}
+
+	roundRobin := loadBalancingClient{
+		strategy: &RoundRobin{
+			clients: clients,
+			length:  uint32(len(clients)),
+		},
+	}
+
+	// clients should be used in the sequence 1, 2,.., 4, 0.
+	for i := 0; i < n; i++ {
+		id, _ := roundRobin.ID(context.Background())
+		if id.Peername != fmt.Sprintf("%d", (i+1)%n) {
+			t.Errorf("clients are not being tried in sequence, expected client: %d, but found: %s", i, id.Peername)
+		}
+	}
+
+}
+
 func testRunManyRequestsConcurrently(t *testing.T, cfgs []*Config, strategy LBStrategy, requests int, retries int, pass bool) {
 	c, err := NewLBClient(strategy, cfgs, retries)
 	if err != nil {