From 70a7674e20998acac9280ed0f1ca7f9d2a7acf19 Mon Sep 17 00:00:00 2001
From: Christian Provenzano <18606244+caproven@users.noreply.github.com>
Date: Tue, 10 May 2022 23:36:14 -0400
Subject: [PATCH] server: Director can be stopped

Goroutine for new directors would live past director scope. Tests
could occassionally fail if this goroutine had log events after
test execution should have ended.
---
 server/proxy/httpproxy/director.go      | 13 ++++++++++---
 server/proxy/httpproxy/director_test.go | 10 +++++++++-
 server/proxy/httpproxy/proxy.go         |  2 +-
 3 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/server/proxy/httpproxy/director.go b/server/proxy/httpproxy/director.go
index e20e2226a..ad50dd07b 100644
--- a/server/proxy/httpproxy/director.go
+++ b/server/proxy/httpproxy/director.go
@@ -33,7 +33,7 @@ func init() {
 	rand.Seed(time.Now().UnixNano())
 }
 
-func newDirector(lg *zap.Logger, urlsFunc GetProxyURLs, failureWait time.Duration, refreshInterval time.Duration) *director {
+func newDirector(lg *zap.Logger, urlsFunc GetProxyURLs, failureWait time.Duration, refreshInterval time.Duration, stop <-chan struct{}, donec chan<- struct{}) *director {
 	if lg == nil {
 		lg = zap.NewNop()
 	}
@@ -44,6 +44,9 @@ func newDirector(lg *zap.Logger, urlsFunc GetProxyURLs, failureWait time.Duratio
 	}
 	d.refresh()
 	go func() {
+		if donec != nil {
+			defer close(donec)
+		}
 		// In order to prevent missing proxy endpoints in the first try:
 		// when given refresh interval of defaultRefreshInterval or greater
 		// and whenever there is no available proxy endpoints,
@@ -65,8 +68,12 @@ func newDirector(lg *zap.Logger, urlsFunc GetProxyURLs, failureWait time.Duratio
 					lg.Info("endpoints found", zap.Strings("endpoints", sl))
 				})
 			}
-			time.Sleep(ri)
-			d.refresh()
+			select {
+			case <-time.After(ri):
+				d.refresh()
+			case <-stop:
+				return
+			}
 		}
 	}()
 	return d
diff --git a/server/proxy/httpproxy/director_test.go b/server/proxy/httpproxy/director_test.go
index 9bba5987c..7eb089a1f 100644
--- a/server/proxy/httpproxy/director_test.go
+++ b/server/proxy/httpproxy/director_test.go
@@ -55,7 +55,8 @@ func TestNewDirectorScheme(t *testing.T) {
 		uf := func() []string {
 			return tt.urls
 		}
-		got := newDirector(zaptest.NewLogger(t), uf, time.Minute, time.Minute)
+		stop, donec := make(chan struct{}), make(chan struct{})
+		got := newDirector(zaptest.NewLogger(t), uf, time.Minute, time.Minute, stop, donec)
 
 		var gep []string
 		for _, ep := range got.ep {
@@ -66,6 +67,13 @@ func TestNewDirectorScheme(t *testing.T) {
 		if !reflect.DeepEqual(tt.want, gep) {
 			t.Errorf("#%d: want endpoints = %#v, got = %#v", i, tt.want, gep)
 		}
+
+		close(stop)
+		select {
+		case <-donec:
+		case <-time.After(time.Second):
+			t.Fatalf("done took too long")
+		}
 	}
 }
 
diff --git a/server/proxy/httpproxy/proxy.go b/server/proxy/httpproxy/proxy.go
index c8f27bf01..6874604e8 100644
--- a/server/proxy/httpproxy/proxy.go
+++ b/server/proxy/httpproxy/proxy.go
@@ -58,7 +58,7 @@ func NewHandler(lg *zap.Logger, t *http.Transport, urlsFunc GetProxyURLs, failur
 
 	p := &reverseProxy{
 		lg:        lg,
-		director:  newDirector(lg, urlsFunc, failureWait, refreshInterval),
+		director:  newDirector(lg, urlsFunc, failureWait, refreshInterval, nil, nil),
 		transport: t,
 	}