diff --git a/.travis.yml b/.travis.yml index fadb17e0..50217784 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,8 +1,8 @@ language: go go: - 1.6.4 -- 1.7.5 -- 1.8.1 +- 1.7.6 +- 1.8.3 install: - go get github.com/nats-io/go-nats - go get github.com/mattn/goveralls diff --git a/server/server.go b/server/server.go index 1495c90e..58c301ed 100644 --- a/server/server.go +++ b/server/server.go @@ -57,6 +57,7 @@ type Server struct { trace bool debug bool running bool + shutdown bool listener net.Listener clients map[uint64]*client routes map[uint64]*client @@ -223,26 +224,12 @@ func (s *Server) Start() { s.logPid() } - // Specifying both HTTP and HTTPS ports is a misconfiguration - if s.opts.HTTPPort != 0 && s.opts.HTTPSPort != 0 { - Fatalf("Can't specify both HTTP (%v) and HTTPs (%v) ports", s.opts.HTTPPort, s.opts.HTTPSPort) + // Start monitoring if needed + if err := s.StartMonitoring(); err != nil { + Fatalf("Can't start monitoring: %v", err) return } - // Start up the http server if needed. - if s.opts.HTTPPort != 0 { - s.StartHTTPMonitoring() - } - - // Start up the https server if needed. - if s.opts.HTTPSPort != 0 { - if s.opts.TLSConfig == nil { - Fatalf("TLS cert and key required for HTTPS") - return - } - s.StartHTTPSMonitoring() - } - // The Routing routine needs to wait for the client listen // port to be opened and potential ephemeral port selected. clientListenReady := make(chan struct{}) @@ -269,11 +256,11 @@ func (s *Server) Shutdown() { s.mu.Lock() // Prevent issues with multiple calls. - if !s.running { + if s.shutdown { s.mu.Unlock() return } - + s.shutdown = true s.running = false s.grMu.Lock() s.grRunning = false @@ -436,15 +423,35 @@ func (s *Server) StartProfiler() { } // StartHTTPMonitoring will enable the HTTP monitoring port. +// DEPRECATED: Should use StartMonitoring. func (s *Server) StartHTTPMonitoring() { s.startMonitoring(false) } // StartHTTPSMonitoring will enable the HTTPS monitoring port. +// DEPRECATED: Should use StartMonitoring. func (s *Server) StartHTTPSMonitoring() { s.startMonitoring(true) } +// StartMonitoring starts the HTTP or HTTPs server if needed. +func (s *Server) StartMonitoring() error { + // Specifying both HTTP and HTTPS ports is a misconfiguration + if s.opts.HTTPPort != 0 && s.opts.HTTPSPort != 0 { + return fmt.Errorf("can't specify both HTTP (%v) and HTTPs (%v) ports", s.opts.HTTPPort, s.opts.HTTPSPort) + } + var err error + if s.opts.HTTPPort != 0 { + err = s.startMonitoring(false) + } else if s.opts.HTTPSPort != 0 { + if s.opts.TLSConfig == nil { + return fmt.Errorf("TLS cert and key required for HTTPS") + } + err = s.startMonitoring(true) + } + return err +} + // HTTP endpoints const ( RootPath = "/" @@ -456,7 +463,7 @@ const ( ) // Start the monitoring server -func (s *Server) startMonitoring(secure bool) { +func (s *Server) startMonitoring(secure bool) error { // Used to track HTTP requests s.httpReqStats = map[string]uint64{ @@ -467,25 +474,27 @@ func (s *Server) startMonitoring(secure bool) { SubszPath: 0, } - var hp string - var err error + var ( + hp string + err error + httpListener net.Listener + ) if secure { hp = net.JoinHostPort(s.opts.HTTPHost, strconv.Itoa(s.opts.HTTPSPort)) Noticef("Starting https monitor on %s", hp) config := util.CloneTLSConfig(s.opts.TLSConfig) config.ClientAuth = tls.NoClientCert - s.http, err = tls.Listen("tcp", hp, config) + httpListener, err = tls.Listen("tcp", hp, config) } else { hp = net.JoinHostPort(s.opts.HTTPHost, strconv.Itoa(s.opts.HTTPPort)) Noticef("Starting http monitor on %s", hp) - s.http, err = net.Listen("tcp", hp) + httpListener, err = net.Listen("tcp", hp) } if err != nil { - Fatalf("Can't listen to the monitor port: %v", err) - return + return fmt.Errorf("can't listen to the monitor port: %v", err) } mux := http.NewServeMux() @@ -513,17 +522,20 @@ func (s *Server) startMonitoring(secure bool) { MaxHeaderBytes: 1 << 20, } s.mu.Lock() + s.http = httpListener s.httpHandler = mux s.mu.Unlock() go func() { - srv.Serve(s.http) + srv.Serve(httpListener) srv.Handler = nil s.mu.Lock() s.httpHandler = nil s.mu.Unlock() s.done <- true }() + + return nil } // HTTPHandler returns the http.Handler object used to handle monitoring diff --git a/test/monitor_test.go b/test/monitor_test.go index 2b011ec5..e81e1c7f 100644 --- a/test/monitor_test.go +++ b/test/monitor_test.go @@ -650,10 +650,14 @@ func TestMonitorNoTLSConfig(t *testing.T) { opts.HTTPSPort = MONITOR_PORT s := server.New(&opts) defer s.Shutdown() + // Check with manually starting the monitoring, which should return an error + if err := s.StartMonitoring(); err == nil || !strings.Contains(err.Error(), "TLS") { + t.Fatalf("Expected error about missing TLS config, got %v", err) + } + // Also check by calling Start(), which should produce a fatal error dl := &dummyLogger{} s.SetLogger(dl, false, false) defer s.SetLogger(nil, false, false) - // This should produce a fatal error due to lack of TLS config s.Start() if !strings.Contains(dl.msg, "TLS") { t.Fatalf("Expected error about missing TLS config, got %v", dl.msg) @@ -670,35 +674,8 @@ func TestMonitorErrorOnListen(t *testing.T) { opts.HTTPPort = MONITOR_PORT s2 := server.New(&opts) defer s2.Shutdown() - dl := &dummyLogger{} - s2.SetLogger(dl, false, false) - defer s2.SetLogger(nil, false, false) - wg := &sync.WaitGroup{} - wg.Add(1) - go func() { - defer wg.Done() - // This will block until server is shutdown - s2.Start() - }() - // Wait for the error to be produced - timeout := time.Now().Add(3 * time.Second) - ok := false - for time.Now().Before(timeout) { - dl.Lock() - msg := dl.msg - dl.Unlock() - if msg == "" { - continue - } - if strings.Contains(msg, "listen") { - ok = true - break - } - } - s2.Shutdown() - wg.Wait() - if !ok { - t.Fatalf("Should have produced a fatal error") + if err := s2.StartMonitoring(); err == nil || !strings.Contains(err.Error(), "listen") { + t.Fatalf("Expected error about not able to start listener, got %v", err) } } @@ -710,12 +687,34 @@ func TestMonitorBothPortsConfigured(t *testing.T) { opts.HTTPSPort = MONITOR_PORT + 1 s := server.New(&opts) defer s.Shutdown() - dl := &dummyLogger{} - s.SetLogger(dl, false, false) - defer s.SetLogger(nil, false, false) - // This should produce a fatal error - s.Start() - if !strings.Contains(dl.msg, "specify both") { - t.Fatalf("Expected error about ports configured, got %v", dl.msg) + if err := s.StartMonitoring(); err == nil || !strings.Contains(err.Error(), "specify both") { + t.Fatalf("Expected error about ports configured, got %v", err) + } +} + +func TestMonitorStop(t *testing.T) { + resetPreviousHTTPConnections() + opts := DefaultTestOptions + opts.Port = CLIENT_PORT + opts.HTTPHost = "localhost" + opts.HTTPPort = MONITOR_PORT + url := fmt.Sprintf("http://%v:%d/", opts.HTTPHost, MONITOR_PORT) + // Create a server instance and start only the monitoring http server. + s := server.New(&opts) + if err := s.StartMonitoring(); err != nil { + t.Fatalf("Error starting monitoring: %v", err) + } + // Make sure http server is started + resp, err := http.Get(url + "varz") + if err != nil { + t.Fatalf("Error on http request: %v", err) + } + resp.Body.Close() + // Although the server itself was not started (we did not call s.Start()), + // Shutdown() should stop the http server. + s.Shutdown() + // HTTP request should now fail + if resp, err := http.Get(url + "varz"); err == nil { + t.Fatalf("Expected error: Got %+v\n", resp) } }