From f0c01ebc4e0994466b21359541a996763721dead Mon Sep 17 00:00:00 2001
From: Abhilash Gnan <abhilashgnan@gmail.com>
Date: Sun, 16 Jun 2019 17:55:34 +0200
Subject: [PATCH] handle query svc errors on SIGINT

Signed-off-by: Abhilash Gnan <abhilashgnan@gmail.com>
---
 cmd/flags/admin.go           |  5 ++++-
 cmd/query/app/server.go      | 15 ++++++++++++---
 cmd/query/app/server_test.go | 27 ++++++++++++++++++++++++++-
 3 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/cmd/flags/admin.go b/cmd/flags/admin.go
index bf34109558e..5f088b1d299 100644
--- a/cmd/flags/admin.go
+++ b/cmd/flags/admin.go
@@ -113,7 +113,10 @@ func (s *AdminServer) serveWithListener(l net.Listener) {
 	s.server = &http.Server{Handler: recoveryHandler(s.mux)}
 	s.logger.Info("Starting admin HTTP server", zap.Int("http-port", s.adminPort))
 	go func() {
-		if err := s.server.Serve(l); err != nil {
+		switch err := s.server.Serve(l); err {
+		case nil, http.ErrServerClosed:
+			// normal exit, nothing to do
+		default:
 			s.logger.Error("failed to serve", zap.Error(err))
 			s.hc.Set(healthcheck.Broken)
 		}
diff --git a/cmd/query/app/server.go b/cmd/query/app/server.go
index bf68320db39..cddcc7bf168 100644
--- a/cmd/query/app/server.go
+++ b/cmd/query/app/server.go
@@ -18,9 +18,10 @@ import (
 	"fmt"
 	"net"
 	"net/http"
+	"strings"
 
 	"github.com/gorilla/handlers"
-	"github.com/opentracing/opentracing-go"
+	opentracing "github.com/opentracing/opentracing-go"
 	"github.com/soheilhy/cmux"
 	"go.uber.org/zap"
 	"google.golang.org/grpc"
@@ -105,7 +106,11 @@ func (s *Server) Start() error {
 
 	go func() {
 		s.svc.Logger.Info("Starting HTTP server", zap.Int("port", s.queryOptions.Port))
-		if err := s.httpServer.Serve(httpListener); err != nil {
+
+		switch err := s.httpServer.Serve(httpListener); err {
+		case nil, http.ErrServerClosed, cmux.ErrListenerClosed:
+			// normal exit, nothing to do
+		default:
 			s.svc.Logger.Error("Could not start HTTP server", zap.Error(err))
 		}
 		s.svc.SetHealthCheckStatus(healthcheck.Unavailable)
@@ -114,6 +119,7 @@ func (s *Server) Start() error {
 	// Start GRPC server concurrently
 	go func() {
 		s.svc.Logger.Info("Starting GRPC server", zap.Int("port", s.queryOptions.Port))
+
 		if err := s.grpcServer.Serve(grpcListener); err != nil {
 			s.svc.Logger.Error("Could not start GRPC server", zap.Error(err))
 		}
@@ -123,7 +129,10 @@ func (s *Server) Start() error {
 	// Start cmux server concurrently.
 	go func() {
 		s.svc.Logger.Info("Starting CMUX server", zap.Int("port", s.queryOptions.Port))
-		if err := cmuxServer.Serve(); err != nil {
+
+		err := cmuxServer.Serve()
+		// TODO: Remove string comparision when https://github.com/soheilhy/cmux/pull/69 is merged
+		if err != nil && !strings.Contains(err.Error(), "use of closed network connection") {
 			s.svc.Logger.Error("Could not start multiplexed server", zap.Error(err))
 		}
 		s.svc.SetHealthCheckStatus(healthcheck.Unavailable)
diff --git a/cmd/query/app/server_test.go b/cmd/query/app/server_test.go
index 2cb9b656ab0..f854db4d11c 100644
--- a/cmd/query/app/server_test.go
+++ b/cmd/query/app/server_test.go
@@ -15,12 +15,14 @@
 package app
 
 import (
+	"fmt"
 	"testing"
 	"time"
 
-	"github.com/opentracing/opentracing-go"
+	opentracing "github.com/opentracing/opentracing-go"
 	"github.com/stretchr/testify/assert"
 	"go.uber.org/zap"
+	"go.uber.org/zap/zaptest/observer"
 
 	"github.com/jaegertracing/jaeger/cmd/flags"
 	"github.com/jaegertracing/jaeger/cmd/query/app/querysvc"
@@ -59,3 +61,26 @@ func TestServer(t *testing.T) {
 	}
 	assert.Equal(t, healthcheck.Unavailable, server.svc.HC().Get())
 }
+
+func TestServerGracefulExit(t *testing.T) {
+	flagsSvc := flags.NewService(ports.AgentAdminHTTP)
+
+	zapCore, logs := observer.New(zap.ErrorLevel)
+	assert.Equal(t, 0, logs.Len(), "Expected initial ObservedLogs to have zero length.")
+
+	flagsSvc.Logger = zap.New(zapCore)
+
+	querySvc := &querysvc.QueryService{}
+	tracer := opentracing.NoopTracer{}
+	server := NewServer(flagsSvc, querySvc, &QueryOptions{Port: ports.QueryAdminHTTP}, tracer)
+	assert.NoError(t, server.Start())
+
+	// Wait for servers to come up before we can call .Close()
+	time.Sleep(1 * time.Second)
+	server.Close()
+
+	for _, logEntry := range logs.All() {
+		assert.True(t, logEntry.Level != zap.ErrorLevel,
+			fmt.Sprintf("Error log found on server exit: %v", logEntry))
+	}
+}