Fixed blocking listen and serve which never returns (#35)

* Fixed blocking listen and serve which never returns

Signed-off-by: Arjun Naik <[email protected]>

* Refactor of StartMetrics() and tests for it

Signed-off-by: Arjun Naik <[email protected]>

pkg/metrics/metrics.go

@@ -16,6 +16,7 @@ package metrics
 
 import (
 	"fmt"
+	"net"
 	"net/http"
 
 	"github.com/prometheus/client_golang/prometheus"
@@ -35,27 +36,26 @@ func StartMetrics(config metricsConfig) error {
 	metricsHandler := promhttp.InstrumentMetricHandler(
 		config.metricsRegisterer, promhttp.HandlerFor(config.metricsGatherer, promhttp.HandlerOpts{}),
 	)
-	http.Handle(config.metricsPath, metricsHandler)
 	log.Info(fmt.Sprintf("Port: %s", config.metricsPort))
-	metricsPort := ":" + (config.metricsPort)
-
-	errc := make(chan error)
-	go ListenAndServeHandle(metricsPort, errc)
-	errMsg := <-errc
-	if errMsg != nil {
-		return errMsg
+	metricsPort := fmt.Sprintf(":%s", config.metricsPort)
+	if free := isPortFree(metricsPort); !free {
+		return fmt.Errorf("port %s is not free", config.metricsPort)
+	}
+	server := &http.Server{
+		Addr:    metricsPort,
+		Handler: metricsHandler,
 	}
+	go server.ListenAndServe()
 	return nil
 }
 
-// ListenAndServeHandle takes in metricsPort and a channel for error
-func ListenAndServeHandle(metricsPort string, errc chan error) error {
-	err := http.ListenAndServe(metricsPort, nil)
+func isPortFree(port string) bool {
+	listener, err := net.Listen("tcp", port)
 	if err != nil {
-		errc <- err
-		return err
+		return false
 	}
-	return nil
+	listener.Close()
+	return true
 }
 
 // RegisterMetrics takes the list of metrics to be registered from the user and

pkg/metrics/metrics_test.go

@@ -0,0 +1,118 @@
+package metrics
+
+import (
+	"fmt"
+	"net"
+	"net/http"
+	"strconv"
+	"testing"
+	"time"
+)
+
+// GetFreePort asks the kernel for a free open port that is ready to use.
+func getFreePort() (string, error) {
+	addr, err := net.ResolveTCPAddr("tcp", "localhost:0")
+	if err != nil {
+		return "", err
+	}
+
+	l, err := net.ListenTCP("tcp", addr)
+	if err != nil {
+		return "", err
+	}
+	defer l.Close()
+	return strconv.Itoa(l.Addr().(*net.TCPAddr).Port), nil
+}
+
+func TestCheckOpenTCPPortUsed(t *testing.T) {
+	port, err := getFreePort()
+	if err != nil {
+		t.Errorf("failed to get free port on host: %v", err)
+		return
+	}
+	freePort := fmt.Sprintf(":%s", port)
+	_, err = net.Listen("tcp", freePort)
+	if err != nil {
+		t.Errorf("failed to listen on free port %s: %v", freePort, err)
+		return
+	}
+	if isPortFree(freePort) {
+		t.Errorf("address %s is is not actually free", freePort)
+	}
+}
+
+func TestCheckOpenTCPPortFree(t *testing.T) {
+	port, err := getFreePort()
+	if err != nil {
+		t.Errorf("failed to get free port")
+		return
+	}
+	freePort := fmt.Sprintf(":%s", port)
+	if !isPortFree(freePort) {
+		t.Errorf("port %s is actually free", freePort)
+	}
+}
+
+func getMetricsResponse(metricsAddr string) (int, error) {
+	resp, err := http.Get(metricsAddr)
+	defer resp.Body.Close()
+	if err != nil {
+		return -1, err
+	}
+	return resp.StatusCode, nil
+}
+
+func TestStartMetricsSuccess(t *testing.T) {
+	freePort, err := getFreePort()
+	if err != nil {
+		t.Errorf("failed to get free port on host: %v", err)
+		return
+	}
+	err = StartMetrics(metricsConfig{
+		metricsPort: freePort,
+		metricsPath: "/metrics",
+	})
+	if err != nil {
+		t.Errorf("failed to start metrics listener: %v", err)
+		return
+	}
+	metricsAddr := fmt.Sprintf("http://localhost:%s/metrics", freePort)
+	var success bool
+	for i := 0; i < 12; i++ {
+		time.Sleep(time.Second * 5)
+		statusCode, err := getMetricsResponse(metricsAddr)
+		if err != nil || statusCode != http.StatusOK {
+			continue
+		}
+		success = true
+		break
+	}
+	if !success {
+		t.Error("failed to get fetch metrics response successfully")
+	}
+}
+
+func TestStartMetricsFailure(t *testing.T) {
+	freePort, err := getFreePort()
+	if err != nil {
+		t.Errorf("failed to get free port on host: %v", err)
+		return
+	}
+	listener, err := net.Listen("tcp", fmt.Sprintf(":%s", freePort))
+	if err != nil {
+		t.Errorf("failed to listen on port %s: %v", freePort, err)
+		return
+	}
+	defer listener.Close()
+	err = StartMetrics(metricsConfig{
+		metricsPort: freePort,
+		metricsPath: "/metrics",
+	})
+	if err == nil {
+		t.Errorf("started metrics listener even when port is bound")
+		return
+	}
+	if err.Error() != fmt.Sprintf("port %s is not free", freePort) {
+		t.Errorf("unexpected error when starting metrics listener")
+	}
+}