Repository: nats-io/nats-server (go)

Issue:
NATS LeafNode remote URLs : Config to shuffle, on by default
## Feature Request
Please allow the shuffling of leaf node remote URLs.

#### Use Case:
All leaf nodes start with the same config, the same ordered list of leaf remote URLs. This causes the first server to pick up all leafs by default. Unlike other client connections, which have the option to disable node shuffling, this is on by default.

#### Proposed Change:
Add a config for LeafRemotes "noShuffle" mode.
Add a function to randomize or shuffle the pool of remote URLs.

#### Who Benefits From The Change(s)?
All nats cluster operators with a multitude of leaf-node connections.


Pull request description:
[CHANGED] Randomize Leafnode remote URLs and add option to disable
Currently, leafnode remote URLs are kept in order when parsed. This change randomizes the order of URLs by default and adds an option to disable this behavior, returning you to the way it was before.

```
leafnodes {
  remotes = [
    {
      dont_randomize: true
      url: [
        "nats-leaf://host1:7422"
        "nats-leaf://host2:7422"
        "nats-leaf://host3:7422"
        "nats-leaf://host4:7422"
      ]
      account: fizzbuzz
      credentials: "./fizzbuzz.creds"
    }
  ]
}
```

Resolves #2153

/cc @nats-io/core


Patch:
diff --git a/server/leafnode.go b/server/leafnode.go
index 2a697ee1..3f253372 100644
--- a/server/leafnode.go
+++ b/server/leafnode.go
@@ -21,6 +21,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
+	"math/rand"
 	"net"
 	"net/http"
 	"net/url"
@@ -133,6 +134,17 @@ func validateLeafNode(o *Options) error {
 	if err := validateLeafNodeAuthOptions(o); err != nil {
 		return err
 	}
+
+	for _, rem := range o.LeafNode.Remotes {
+		if rem.NoRandomize {
+			continue
+		}
+
+		rand.Shuffle(len(rem.URLs), func(i, j int) {
+			rem.URLs[i], rem.URLs[j] = rem.URLs[j], rem.URLs[i]
+		})
+	}
+
 	// In local config mode, check that leafnode configuration refers to accounts that exist.
 	if len(o.TrustedOperators) == 0 {
 		accNames := map[string]struct{}{}
diff --git a/server/opts.go b/server/opts.go
index d7ebfdc3..cc5c08ac 100644
--- a/server/opts.go
+++ b/server/opts.go
@@ -139,6 +139,7 @@ type LeafNodeOpts struct {
 // RemoteLeafOpts are options for connecting to a remote server as a leaf node.
 type RemoteLeafOpts struct {
 	LocalAccount string      `json:"local_account,omitempty"`
+	NoRandomize  bool        `json:"-"`
 	URLs         []*url.URL  `json:"urls,omitempty"`
 	Credentials  string      `json:"-"`
 	TLS          bool        `json:"-"`
@@ -1761,6 +1762,8 @@ func parseRemoteLeafNodes(v interface{}, errors *[]error, warnings *[]error) ([]
 		for k, v := range rm {
 			tk, v = unwrapValue(v, &lt)
 			switch strings.ToLower(k) {
+			case "no_randomize", "dont_randomize":
+				remote.NoRandomize = v.(bool)
 			case "url", "urls":
 				switch v := v.(type) {
 				case []interface{}, []string:


Test patch:
diff --git a/server/leafnode_test.go b/server/leafnode_test.go
index e4a32e6d..df989668 100644
--- a/server/leafnode_test.go
+++ b/server/leafnode_test.go
@@ -85,6 +85,59 @@ func TestLeafNodeRandomIP(t *testing.T) {
 	}
 }
 
+func TestLeafNodeRandomRemotes(t *testing.T) {
+	// 16! possible permutations.
+	orderedURLs := make([]*url.URL, 0, 16)
+	for i := 0; i < cap(orderedURLs); i++ {
+		orderedURLs = append(orderedURLs, &url.URL{
+			Scheme: "nats-leaf",
+			Host:   fmt.Sprintf("host%d:7422", i),
+		})
+	}
+
+	o := DefaultOptions()
+	o.LeafNode.Remotes = []*RemoteLeafOpts{
+		{NoRandomize: true},
+		{NoRandomize: false},
+	}
+	o.LeafNode.Remotes[0].URLs = make([]*url.URL, cap(orderedURLs))
+	copy(o.LeafNode.Remotes[0].URLs, orderedURLs)
+	o.LeafNode.Remotes[1].URLs = make([]*url.URL, cap(orderedURLs))
+	copy(o.LeafNode.Remotes[1].URLs, orderedURLs)
+
+	s := RunServer(o)
+	s.Shutdown()
+
+	gotOrdered := o.LeafNode.Remotes[0].URLs
+	if got, want := len(gotOrdered), len(orderedURLs); got != want {
+		t.Fatalf("Unexpected rem0 len URLs, got %d, want %d", got, want)
+	}
+
+	// These should be IN order.
+	for i := range orderedURLs {
+		if got, want := gotOrdered[i].String(), orderedURLs[i].String(); got != want {
+			t.Fatalf("Unexpected ordered url, got %s, want %s", got, want)
+		}
+	}
+
+	gotRandom := o.LeafNode.Remotes[1].URLs
+	if got, want := len(gotRandom), len(orderedURLs); got != want {
+		t.Fatalf("Unexpected rem1 len URLs, got %d, want %d", got, want)
+	}
+
+	// These should be OUT of order.
+	var random bool
+	for i := range orderedURLs {
+		if gotRandom[i].String() != orderedURLs[i].String() {
+			random = true
+			break
+		}
+	}
+	if !random {
+		t.Fatal("Expected urls to be random")
+	}
+}
+
 type testLoopbackResolver struct{}
 
 func (r *testLoopbackResolver) LookupHost(ctx context.Context, host string) ([]string, error) {
@@ -3226,16 +3279,15 @@ func TestLeafNodeLoopDetectionWithMultipleClusters(t *testing.T) {
 
 	checkClusterFormed(t, l1, l2)
 
-	u1, _ := url.Parse(fmt.Sprintf("nats://127.0.0.1:%d", lo1.LeafNode.Port))
-	u2, _ := url.Parse(fmt.Sprintf("nats://127.0.0.1:%d", lo2.LeafNode.Port))
-	urls := []*url.URL{u1, u2}
-
 	ro1 := DefaultOptions()
 	ro1.Cluster.Name = "remote"
 	ro1.Cluster.Host = "127.0.0.1"
 	ro1.Cluster.Port = -1
 	ro1.LeafNode.ReconnectInterval = 50 * time.Millisecond
-	ro1.LeafNode.Remotes = []*RemoteLeafOpts{{URLs: urls}}
+	ro1.LeafNode.Remotes = []*RemoteLeafOpts{{URLs: []*url.URL{
+		{Scheme: "nats", Host: fmt.Sprintf("127.0.0.1:%d", lo1.LeafNode.Port)},
+		{Scheme: "nats", Host: fmt.Sprintf("127.0.0.1:%d", lo2.LeafNode.Port)},
+	}}}
 	r1 := RunServer(ro1)
 	defer r1.Shutdown()
 
@@ -3248,7 +3300,10 @@ func TestLeafNodeLoopDetectionWithMultipleClusters(t *testing.T) {
 	ro2.Cluster.Port = -1
 	ro2.Routes = RoutesFromStr(fmt.Sprintf("nats://127.0.0.1:%d", ro1.Cluster.Port))
 	ro2.LeafNode.ReconnectInterval = 50 * time.Millisecond
-	ro2.LeafNode.Remotes = []*RemoteLeafOpts{{URLs: urls}}
+	ro2.LeafNode.Remotes = []*RemoteLeafOpts{{URLs: []*url.URL{
+		{Scheme: "nats", Host: fmt.Sprintf("127.0.0.1:%d", lo1.LeafNode.Port)},
+		{Scheme: "nats", Host: fmt.Sprintf("127.0.0.1:%d", lo2.LeafNode.Port)},
+	}}}
 	r2 := RunServer(ro2)
 	defer r2.Shutdown()
 
diff --git a/server/opts_test.go b/server/opts_test.go
index 8d27d65f..0c75b8e4 100644
--- a/server/opts_test.go
+++ b/server/opts_test.go
@@ -16,6 +16,7 @@ package server
 import (
 	"bytes"
 	"crypto/tls"
+	"encoding/json"
 	"flag"
 	"fmt"
 	"io/ioutil"
@@ -2383,6 +2384,66 @@ func TestParsingLeafNodeRemotes(t *testing.T) {
 			t.Fatalf("Expected %v, got %v", expected, opts.LeafNode.Remotes[0])
 		}
 	})
+
+	t.Run("url ordering", func(t *testing.T) {
+		// 16! possible permutations.
+		orderedURLs := make([]string, 0, 16)
+		for i := 0; i < cap(orderedURLs); i++ {
+			orderedURLs = append(orderedURLs, fmt.Sprintf("nats-leaf://host%d:7422", i))
+		}
+		confURLs, err := json.Marshal(orderedURLs)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		content := `
+		leafnodes {
+			remotes = [
+				{
+					dont_randomize: true
+					urls: %[1]s
+				}
+				{
+					urls: %[1]s
+				}
+			]
+		}
+		`
+		conf := createConfFile(t, []byte(fmt.Sprintf(content, confURLs)))
+		defer removeFile(t, conf)
+
+		s, o := RunServerWithConfig(conf)
+		s.Shutdown()
+
+		gotOrdered := o.LeafNode.Remotes[0].URLs
+		if got, want := len(gotOrdered), len(orderedURLs); got != want {
+			t.Fatalf("Unexpected rem0 len URLs, got %d, want %d", got, want)
+		}
+
+		// These should be IN order.
+		for i := range orderedURLs {
+			if got, want := gotOrdered[i].String(), orderedURLs[i]; got != want {
+				t.Fatalf("Unexpected ordered url, got %s, want %s", got, want)
+			}
+		}
+
+		gotRandom := o.LeafNode.Remotes[1].URLs
+		if got, want := len(gotRandom), len(orderedURLs); got != want {
+			t.Fatalf("Unexpected rem1 len URLs, got %d, want %d", got, want)
+		}
+
+		// These should be OUT of order.
+		var random bool
+		for i := range orderedURLs {
+			if gotRandom[i].String() != orderedURLs[i] {
+				random = true
+				break
+			}
+		}
+		if !random {
+			t.Fatal("Expected urls to be random")
+		}
+	})
 }
 
 func TestLargeMaxControlLine(t *testing.T) {