Fixed some leafnode issues introduced from JS cluster work

Also fixed a flapper.

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>

server/client.go

@@ -4631,9 +4631,7 @@ func (c *client) getClientInfo(detailed bool) *ClientInfo {
 	}
 	// Server name. Defaults to server ID if not set explicitly.
 	var sn string
-	if c.kind == LEAF {
-		sn = c.leaf.remoteServer
-	} else {
+	if detailed && c.kind != LEAF {
 		sn = c.srv.Name()
 	}
 
@@ -4644,6 +4642,9 @@ func (c *client) getClientInfo(detailed bool) *ClientInfo {
 	ci.RTT = c.rtt
 	// Detailed signals additional opt in.
 	if detailed {
+		if c.kind == LEAF {
+			sn = c.leaf.remoteServer
+		}
 		ci.Start = &c.start
 		ci.Host = c.host
 		ci.ID = c.cid

server/leafnode.go

@@ -993,7 +993,14 @@ func (c *client) processLeafnodeInfo(info *Info) error {
 		c.headers = supportsHeaders && info.Headers
 
 		// Remember the remote server.
-		c.leaf.remoteServer = info.Name
+		// Pre 2.2.0 servers are not sending their server name.
+		// In that case, use info.ID, which, for those servers, matches
+		// the content of the field `Name` in the leafnode CONNECT protocol.
+		if info.Name == _EMPTY_ {
+			c.leaf.remoteServer = info.ID
+		} else {
+			c.leaf.remoteServer = info.Name
+		}
 	}
 	// For both initial INFO and async INFO protocols, Possibly
 	// update our list of remote leafnode URLs we can connect to.
@@ -1098,7 +1105,7 @@ func (s *Server) setLeafNodeInfoHostPortAndIP() error {
 
 // Add the connection to the map of leaf nodes.
 // If `checkForDup` is true (invoked when a leafnode is accepted), then we check
-// if a connection already exists for the same server name (ID) and account.
+// if a connection already exists for the same server name and account.
 // That can happen when the remote is attempting to reconnect while the accepting
 // side did not detect the connection as broken yet.
 // But it can also happen when there is a misconfiguration and the remote is
@@ -1119,11 +1126,15 @@ func (s *Server) addLeafNodeConnection(c *client, srvName string, checkForDup bo
 
 	var old *client
 	s.mu.Lock()
-	if checkForDup {
+	// We check for empty because in some test we may send empty CONNECT{}
+	if checkForDup && srvName != _EMPTY_ {
 		for _, ol := range s.leafs {
 			ol.mu.Lock()
-			// We check for empty because in some test we may send empty CONNECT{}
-			if srvName != _EMPTY_ && ol.leaf.remoteServer == srvName && ol.acc.Name == accName {
+			// We care here only about non solicited Leafnode. This function
+			// is more about replacing stale connections than detecting loops.
+			// We have code for the loop detection elsewhere, which also delays
+			// attempt to reconnect.
+			if !ol.isSolicitedLeafNode() && ol.leaf.remoteServer == srvName && ol.acc.Name == accName {
 				old = ol
 			}
 			ol.mu.Unlock()
@@ -1218,7 +1229,15 @@ func (c *client) processLeafNodeConnect(s *Server, arg []byte, lang string) erro
 	c.headers = supportHeaders && proto.Headers
 
 	// Remember the remote server.
-	c.leaf.remoteServer = proto.Name
+	// We changed the leafnode CONNECT.Name json tag from "name" to "server_name",
+	// so for pre 2.2.0 servers, proto.Name (which is json "server_name") will be empty.
+	// However, CONNECT goes through common processing and the old json tag "name" is
+	// decoded into c.opts.Name (clientOpts struct), so use that if proto.Name is empty.
+	if proto.Name == _EMPTY_ {
+		c.leaf.remoteServer = c.opts.Name
+	} else {
+		c.leaf.remoteServer = proto.Name
+	}
 
 	// If the other side has declared itself a hub, so we will take on the spoke role.
 	if proto.Hub {

server/leafnode_test.go

@@ -762,9 +762,6 @@ func (l *loopDetectedLogger) Errorf(format string, v ...interface{}) {
 }
 
 func TestLeafNodeLoop(t *testing.T) {
-	// FIXME(dlc) - Broken for some reason.
-	t.SkipNow()
-
 	// This test requires that we set the port to known value because
 	// we want A point to B and B to A.
 	oa := DefaultOptions()
@@ -1892,17 +1889,16 @@ func TestLeafNodeNoDuplicateWithinCluster(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Error parsing url: %v", err)
 	}
-	remoteLeafs := []*RemoteLeafOpts{&RemoteLeafOpts{URLs: []*url.URL{u}}}
 
 	oLeaf1 := DefaultOptions()
-	oLeaf1.LeafNode.Remotes = remoteLeafs
+	oLeaf1.LeafNode.Remotes = []*RemoteLeafOpts{&RemoteLeafOpts{URLs: []*url.URL{u}}}
 	leaf1 := RunServer(oLeaf1)
 	defer leaf1.Shutdown()
 
 	leaf1ClusterURL := fmt.Sprintf("nats://127.0.0.1:%d", oLeaf1.Cluster.Port)
 
 	oLeaf2 := DefaultOptions()
-	oLeaf2.LeafNode.Remotes = remoteLeafs
+	oLeaf2.LeafNode.Remotes = []*RemoteLeafOpts{&RemoteLeafOpts{URLs: []*url.URL{u}}}
 	oLeaf2.Routes = RoutesFromStr(leaf1ClusterURL)
 	leaf2 := RunServer(oLeaf2)
 	defer leaf2.Shutdown()

test/leafnode_test.go

@@ -4262,31 +4262,35 @@ func TestLeafnodeHeaders(t *testing.T) {
 	leaf, _ := runSolicitLeafServer(opts)
 	defer leaf.Shutdown()
 
+	checkLeafNodeConnected(t, srv)
+	checkLeafNodeConnected(t, leaf)
+
 	snc, err := nats.Connect(srv.ClientURL())
 	if err != nil {
 		t.Fatalf(err.Error())
 	}
 	defer snc.Close()
-	ssub, err := snc.SubscribeSync("test")
-	if err != nil {
-		t.Fatalf("subscribe failed: %s", err)
-	}
 
 	lnc, err := nats.Connect(leaf.ClientURL())
 	if err != nil {
 		t.Fatalf(err.Error())
 	}
 	defer lnc.Close()
+
+	// Start with subscription on leaf so that we check that srv has the interest
+	// (since we are going to publish from srv)
 	lsub, err := lnc.SubscribeSync("test")
 	if err != nil {
 		t.Fatalf("subscribe failed: %s", err)
 	}
 	lnc.Flush()
-
-	checkLeafNodeConnected(t, srv)
-	checkLeafNodeConnected(t, leaf)
 	checkSubInterest(t, srv, "$G", "test", time.Second)
 
+	ssub, err := snc.SubscribeSync("test")
+	if err != nil {
+		t.Fatalf("subscribe failed: %s", err)
+	}
+
 	msg := nats.NewMsg("test")
 	msg.Header.Add("Test", "Header")
 	if len(msg.Header) == 0 {