Merge pull request #46 from Clever/health-check-on-separate-port

health-check: Change health check to run on a separate port.

config/README.md

@@ -18,9 +18,10 @@ yaml
 
 ```go
 type Config struct {
-	Proxy   Proxy
-	Limits  map[string]Limit
-	Storage map[string]string
+	Proxy       Proxy
+	HealthCheck HealthCheck `yaml:"health-check"`
+	Limits      map[string]Limit
+	Storage     map[string]string
 }
 ```
 
@@ -50,6 +51,18 @@ func New(path string) (Config, error)
 ```
 New takes in a path to a configuration yaml and returns a Configuration.
 
+#### type HealthCheck
+
+```go
+type HealthCheck struct {
+	Port     string
+	Endpoint string
+	Enabled  bool
+}
+```
+
+HealthCheck holds the yaml data for how to run the health check service.
+
 #### type Limit
 
 ```go

config/config.go

@@ -13,9 +13,10 @@ import (
 
 // Config holds the yaml data for the config file
 type Config struct {
-	Proxy   Proxy
-	Limits  map[string]Limit
-	Storage map[string]string
+	Proxy       Proxy
+	HealthCheck HealthCheck `yaml:"health-check"`
+	Limits      map[string]Limit
+	Storage     map[string]string
 }
 
 // Proxy holds the yaml data for the proxy option in the config file
@@ -25,6 +26,13 @@ type Proxy struct {
 	Listen  string
 }
 
+// HealthCheck holds the yaml data for how to run the health check service.
+type HealthCheck struct {
+	Port     string
+	Endpoint string
+	Enabled  bool
+}
+
 // Limit holds the yaml data for one of the limits in the config file
 type Limit struct {
 	Interval uint
@@ -61,6 +69,16 @@ func ValidateConfig(config Config) error {
 	if _, err := url.ParseRequestURI(config.Proxy.Host); err != nil {
 		return errors.New("could not parse proxy.host. Must include scheme (eg. https://example.com)")
 	}
+
+	// HealthCheck section is optional.
+	if config.HealthCheck.Enabled {
+		colonIdx := strings.LastIndex(config.Proxy.Listen, ":") + 1
+		proxyPort := config.Proxy.Listen[colonIdx:]
+		if config.HealthCheck.Port == proxyPort {
+			return fmt.Errorf("health service port cannot match proxy.listen port")
+		}
+	}
+
 	if len(config.Limits) < 1 {
 		return fmt.Errorf("no limits definied")
 	}

config/config_test.go

@@ -21,6 +21,14 @@ func TestConfigurationFileLoading(t *testing.T) {
 		t.Error("expected http for Proxy.Handler")
 	}
 
+	if config.HealthCheck.Port != "60002" {
+		t.Error("expected 60002 for HealthCheck.Port")
+	}
+
+	if config.HealthCheck.Endpoint != "/health/check" {
+		t.Error("expected /health/check for HealthCheck.Port")
+	}
+
 	if len(config.Limits) != 4 {
 		t.Error("expected 4 bucket definitions")
 	}
@@ -127,6 +135,25 @@ limits:
 	}
 }
 
+func TestInvalidHealthCheckConfig(t *testing.T) {
+	buf := bytes.NewBufferString(`
+proxy:
+  handler: http
+  host: http://proxy.example.com
+  listen: localhost:8080
+health-check:
+  enabled: true
+  port: 8080
+  endpoint: "/health/check"
+`)
+	_, err := LoadAndValidateYaml(buf.Bytes())
+	if err == nil ||
+		!strings.Contains(err.Error(), "health service port cannot match proxy.listen port") {
+		t.Error("Expected health service port error.")
+	}
+
+}
+
 func TestInvalidStorageConfig(t *testing.T) {
 	baseBuf := bytes.NewBufferString(`
 proxy:

daemon/daemon.go

@@ -17,17 +17,29 @@ type Daemon interface {
 }
 
 type daemon struct {
-	handler http.Handler
-	proxy   config.Proxy
+	handler     http.Handler
+	proxy       config.Proxy
+	healthCheck config.HealthCheck
 }
 
-func (d *daemon) Start() {
-	log.Printf("Listening on %s", d.proxy.Listen)
-	http.HandleFunc("/sphinx/health/check", func(rw http.ResponseWriter, req *http.Request) {
+// setUpHealthCheckService sets up a health check service at the given port
+// that can be pinged at the given endpoint to determine if Sphinx is still
+// running.
+func setUpHealthCheckService(port string, endpoint string) {
+	mux := http.NewServeMux()
+	mux.HandleFunc(endpoint, func(rw http.ResponseWriter, req *http.Request) {
 		rw.WriteHeader(http.StatusOK)
 	})
-	http.Handle("/", d)
-	log.Fatal(http.ListenAndServe(d.proxy.Listen, nil))
+	go http.ListenAndServe(":"+port, mux)
+}
+
+func (d *daemon) Start() {
+	log.Printf("Listening on %s", d.proxy.Listen)
+	// Only set up the health check service if it is enabled.
+	if d.healthCheck.Enabled {
+		setUpHealthCheckService(d.healthCheck.Port, d.healthCheck.Endpoint)
+	}
+	log.Fatal(http.ListenAndServe(d.proxy.Listen, d))
 	return
 }
 
@@ -43,6 +55,7 @@ func (d *daemon) LoadConfig(newConfig config.Config) error {
 	}
 
 	d.proxy = newConfig.Proxy
+	d.healthCheck = newConfig.HealthCheck
 	target, _ := url.Parse(d.proxy.Host) // already tested for invalid Host
 	proxy := httputil.NewSingleHostReverseProxy(target)
 	rateLimiter, err := ratelimiter.New(newConfig)

daemon/daemon_test.go

@@ -1,10 +1,13 @@
 package daemon
 
 import (
+	"fmt"
 	"github.com/Clever/sphinx/config"
 	"io/ioutil"
 	"net/http"
+	"strings"
 	"testing"
+	"time"
 )
 
 func TestConfigReload(t *testing.T) {
@@ -41,11 +44,7 @@ func TestFailedReload(t *testing.T) {
 	}
 }
 
-var localServerPort = ":8081"
-var localServerHost = "http://localhost" + localServerPort
-var localProxyHost = "http://localhost:6634"
-
-func setUpDaemonWithLocalServer() error {
+func setUpDaemonWithLocalServer(conf config.Config) error {
 	// Set up a local server that 404s everywhere except route '/healthyroute'.
 	mux := http.NewServeMux()
 	mux.HandleFunc("/healthyroute", func(rw http.ResponseWriter, req *http.Request) {
@@ -55,15 +54,13 @@ func setUpDaemonWithLocalServer() error {
 		rw.WriteHeader(http.StatusNotFound)
 		rw.Write([]byte("404"))
 	})
-	go http.ListenAndServe(localServerPort, mux)
 
-	// Set up the daemon to proxy to the local server.
-	conf, err := config.New("../example.yaml")
-	if err != nil {
-		return err
-	}
-	conf.Proxy.Host = localServerHost
+	// Start local server on the port config points to.
+	colonIdx := strings.LastIndex(conf.Proxy.Host, ":")
+	localServerListen := conf.Proxy.Host[colonIdx:]
+	go http.ListenAndServe(localServerListen, mux)
 
+	// Set up and start the daemon.
 	daemon, err := New(conf)
 	if err != nil {
 		return err
@@ -75,15 +72,15 @@ func setUpDaemonWithLocalServer() error {
 
 // testProxyRequest calls the proxy server at the given path and verifies that
 // the request returns the given HTTP status and body content.
-func testProxyRequest(t *testing.T, path string, expectedStatus int, expectedBody string) {
-	resp, err := http.Get(localProxyHost + path)
+func testProxyRequest(t *testing.T, url string, expectedStatus int, expectedBody string) {
+	resp, err := http.Get(url)
 	if err != nil {
 		t.Fatalf("Proxy request failed: %s", err.Error())
 	}
 
 	if resp.StatusCode != expectedStatus {
-		t.Fatalf("Response status with path %s does not match expected value.  Actual: %d.  Expected: %d.",
-			path, resp.StatusCode, expectedStatus)
+		t.Fatalf("Response status with url %s does not match expected value.  Actual: %d.  Expected: %d.",
+			url, resp.StatusCode, expectedStatus)
 	}
 
 	body, err := ioutil.ReadAll(resp.Body)
@@ -99,17 +96,60 @@ func testProxyRequest(t *testing.T, path string, expectedStatus int, expectedBod
 }
 
 func TestHealthCheck(t *testing.T) {
-	err := setUpDaemonWithLocalServer()
+	// Set up the daemon config to proxy to the local server.
+	conf, err := config.New("../example.yaml")
+	if err != nil {
+		t.Fatalf("Couldn't load daemon config: %s", err.Error())
+	}
+	conf.Proxy.Host = "http://localhost:8000"
+	conf.Proxy.Listen = ":6634"
+	conf.HealthCheck.Port = "60002"
+	conf.HealthCheck.Enabled = true
+
+	err = setUpDaemonWithLocalServer(conf)
 	if err != nil {
 		t.Fatalf("Test daemon setup failed: %s", err.Error())
 	}
 
+	localProxyURL := "http://localhost" + conf.Proxy.Listen
+
 	// Test a route that should be proxied to 404.
-	testProxyRequest(t, "/helloworld", http.StatusNotFound, "404")
+	testProxyRequest(t, localProxyURL+"/helloworld", http.StatusNotFound, "404")
 
 	// Test a route that should be proxied to a valid response.
-	testProxyRequest(t, "/healthyroute", http.StatusOK, "healthy")
+	testProxyRequest(t, localProxyURL+"/healthyroute", http.StatusOK, "healthy")
+
+	healthCheckURL := fmt.Sprintf("http://localhost:%s/health/check", conf.HealthCheck.Port)
 
 	// Test the health check.
-	testProxyRequest(t, "/sphinx/health/check", http.StatusOK, "")
+	testProxyRequest(t, healthCheckURL, http.StatusOK, "")
+}
+
+func TestDaemonWithNoHealthCheck(t *testing.T) {
+	// Set up the daemon config to proxy to the local server.
+	conf, err := config.New("../example.yaml")
+	if err != nil {
+		t.Fatalf("Couldn't load daemon config: %s", err.Error())
+	}
+	conf.Proxy.Host = "http://localhost:8001"
+	conf.Proxy.Listen = ":6635"
+	conf.HealthCheck.Port = "60003"
+	conf.HealthCheck.Enabled = false
+
+	err = setUpDaemonWithLocalServer(conf)
+	if err != nil {
+		t.Fatalf("Test daemon setup failed: %s", err.Error())
+	}
+
+	// Because so many servers are starting, sleep for a second to make sure
+	// they start.
+	time.Sleep(time.Second)
+
+	localProxyURL := "http://localhost" + conf.Proxy.Listen
+
+	// Test a route that should be proxied to 404.
+	testProxyRequest(t, localProxyURL+"/helloworld", http.StatusNotFound, "404")
+
+	// Test a route that should be proxied to a valid response.
+	testProxyRequest(t, localProxyURL+"/healthyroute", http.StatusOK, "healthy")
 }

deb/sphinx/DEBIAN/control

@@ -1,5 +1,5 @@
 Package: sphinx
-Version: 0.3.0
+Version: 0.3.1
 Section: base
 Priority: optional
 Architecture: amd64

example.yaml

@@ -9,6 +9,11 @@ storage:
   host: localhost # redis hostname. not required for memory
   port: 6379      # redis port.     not required for memory
 
+health-check:
+  enabled: true
+  port: 60002
+  endpoint: "/health/check"
+
 limits:
   bearer-special:
     interval: 15  # in seconds