From ce79f524be0de830eb43bc908215e1f8e73bb28e Mon Sep 17 00:00:00 2001 From: Ivan Kozlovic Date: Thu, 11 Feb 2016 17:20:33 -0700 Subject: [PATCH] Fix scheme for routes returned When server returns routes through INFO, use "nats-route://" scheme. A test was checking that. Add test to check that hostname is replaced with IP. --- server/route.go | 22 ++++++----- test/route_discovery_test.go | 74 ++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 9 deletions(-) diff --git a/server/route.go b/server/route.go index 5f7c3f47..d3530115 100644 --- a/server/route.go +++ b/server/route.go @@ -216,18 +216,22 @@ func (s *Server) createRoute(conn net.Conn, rURL *url.URL) *client { info := s.routeInfo for _, r := range s.routes { r.mu.Lock() - if r.route.url != nil { + // race condition where connection can be closed (r.nc == nil) + // and route still in s.routes[]. + if r.nc == nil { + r.mu.Unlock() + continue + } + if r.route.url != nil { var rurl string - if r.nc != nil { - _, rport, err := net.SplitHostPort(r.route.url.Host) - if err == nil { - // We will send the url but based on the route's ip address. - if ip, ok := r.nc.(*net.TCPConn); ok { - addr := ip.RemoteAddr().(*net.TCPAddr) - rurl = fmt.Sprintf("nats://%s:%s", addr.IP.String(), rport) - } + _, rport, err := net.SplitHostPort(r.route.url.Host) + if err == nil { + // We will send the url but based on the route's ip address. + if ip, ok := r.nc.(*net.TCPConn); ok { + addr := ip.RemoteAddr().(*net.TCPAddr) + rurl = fmt.Sprintf("nats-route://%s:%s/", addr.IP.String(), rport) } } diff --git a/test/route_discovery_test.go b/test/route_discovery_test.go index a0269428..900e3cab 100644 --- a/test/route_discovery_test.go +++ b/test/route_discovery_test.go @@ -10,6 +10,7 @@ import ( "net/http" "runtime" "strconv" + "strings" "testing" "time" @@ -353,3 +354,76 @@ func readHttpRoutez(t *testing.T, url string) *server.Routez { } return &r } + +func TestSeedReturnIPInsteadOfURL(t *testing.T) { + s, opts := runSeedServer(t) + defer s.Shutdown() + + rc1 := createRouteConn(t, opts.ClusterHost, opts.ClusterPort) + defer rc1.Close() + + routeSend1, route1Expect := setupRoute(t, rc1, opts) + route1Expect(infoRe) + + rc1ID := "2222" + rc1Port := 22 + rc1Host := "localhost" + + // register ourselves via INFO + r1Info := server.Info{ID: rc1ID, Host: rc1Host, Port: rc1Port} + b, _ := json.Marshal(r1Info) + infoJSON := fmt.Sprintf(server.InfoProto, b) + routeSend1(infoJSON) + routeSend1("PING\r\n") + route1Expect(pongRe) + + rc2 := createRouteConn(t, opts.ClusterHost, opts.ClusterPort) + defer rc2.Close() + + routeSend2, route2Expect := setupRoute(t, rc2, opts) + + rc2ID := "2224" + rc2Port := 24 + rc2Host := "localhost" + + // register ourselves via INFO + r2Info := server.Info{ID: rc2ID, Host: rc2Host, Port: rc2Port} + b, _ = json.Marshal(r2Info) + infoJSON = fmt.Sprintf(server.InfoProto, b) + routeSend2(infoJSON) + + // Now read back out the info from the seed route + buf := route2Expect(infoRe) + + info := server.Info{} + if err := json.Unmarshal(buf[4:], &info); err != nil { + t.Fatalf("Could not unmarshal route info: %v", err) + } + + if len(info.Routes) != 1 { + t.Fatalf("Expected len of []Routes to be 1 vs %d", len(info.Routes)) + } + + route := info.Routes[0] + if route.RemoteID != rc1ID { + t.Fatalf("Expected RemoteID of \"22\", got %q", route.RemoteID) + } + if route.URL == "" { + t.Fatal("Expected a URL for the implicit route") + } + rurl := strings.TrimPrefix(route.URL, "nats-route://") + rhost, _, err := net.SplitHostPort(rurl) + if err != nil { + t.Fatalf("Error getting host information from: %v, err=%v", route.URL, err) + } + if rhost == rc1Host { + t.Fatalf("Expected route url to include IP address, got %s", rhost) + } + addr, ok := rc1.RemoteAddr().(*net.TCPAddr) + if !ok { + t.Fatal("Unable to get IP address from route") + } + if rhost != addr.IP.String() { + t.Fatalf("Expected IP %s, got %s", addr.IP.String(), rhost) + } +}