From e50fbf55ae883403d757c5544fe40464bad48e43 Mon Sep 17 00:00:00 2001
From: Alexander Yastrebov <alexander.yastrebov@zalando.de>
Date: Sun, 16 Jul 2023 00:12:19 +0200
Subject: [PATCH] skipper: fix shutdown test

Make test request in parallel to ensure another request is refused
not only after shutdown started but also while shutdown is still in progress
(i.e. while test request is still in progress).

Verify shutdown completes after test request is finished.

Rename tests to have the same prefix.

Updates #988 #1865

Signed-off-by: Alexander Yastrebov <alexander.yastrebov@zalando.de>
---
 skipper_test.go | 53 +++++++++++++++++++++++++++++++++++--------------
 1 file changed, 38 insertions(+), 15 deletions(-)

diff --git a/skipper_test.go b/skipper_test.go
index 0996dd9e0c..f9b21674fa 100644
--- a/skipper_test.go
+++ b/skipper_test.go
@@ -283,12 +283,12 @@ func TestHTTPServer(t *testing.T) {
 	}
 }
 
-func TestHTTPServerShutdown(t *testing.T) {
+func TestServerShutdownHTTP(t *testing.T) {
 	o := &Options{}
 	testServerShutdown(t, o, "http")
 }
 
-func TestHTTPSServerShutdown(t *testing.T) {
+func TestServerShutdownHTTPS(t *testing.T) {
 	o := &Options{
 		CertPathTLS: "fixtures/test.crt",
 		KeyPathTLS:  "fixtures/test.key",
@@ -296,6 +296,11 @@ func TestHTTPSServerShutdown(t *testing.T) {
 	testServerShutdown(t, o, "https")
 }
 
+type responseOrError struct {
+	rsp *http.Response
+	err error
+}
+
 func testServerShutdown(t *testing.T, o *Options, scheme string) {
 	const shutdownDelay = 1 * time.Second
 
@@ -319,8 +324,9 @@ func testServerShutdown(t *testing.T, o *Options, scheme string) {
 	defer proxy.Close()
 
 	sigs := make(chan os.Signal, 1)
+	done := make(chan struct{})
 	go func() {
-		err := listenAndServeQuit(proxy, o, sigs, nil, nil, nil)
+		err := listenAndServeQuit(proxy, o, sigs, done, nil, nil)
 		require.NoError(t, err)
 	}()
 
@@ -329,22 +335,39 @@ func testServerShutdown(t *testing.T, o *Options, scheme string) {
 
 	time.Sleep(shutdownDelay / 2)
 
-	t.Logf("ongoing request passing in before shutdown")
-	r, err := waitConnGet(testUrl)
-	require.NoError(t, err)
-	require.Equal(t, 200, r.StatusCode)
+	t.Logf("Make request in parallel before shutdown started")
 
-	defer r.Body.Close()
+	roeCh := make(chan responseOrError)
+	go func() {
+		rsp, err := waitConnGet(testUrl)
+		roeCh <- responseOrError{rsp, err}
+	}()
 
-	body, err := io.ReadAll(r.Body)
-	require.NoError(t, err)
-	require.Equal(t, "OK", string(body))
+	time.Sleep(shutdownDelay)
 
-	time.Sleep(shutdownDelay / 2)
+	t.Logf("We are 1.5x past the shutdown delay, so the server should be shutting down now")
 
-	t.Logf("request after shutdown should fail")
-	_, err = waitConnGet(testUrl)
-	require.Error(t, err)
+	select {
+	case <-roeCh:
+		t.Fatalf("Request should still be in progress after shutdown started")
+	default:
+		_, err = waitConnGet(testUrl)
+		assert.ErrorContains(t, err, "connection refused", "Another request should fail after shutdown started")
+	}
+
+	roe := <-roeCh
+	require.NoError(t, roe.err, "Request started before shutdown must succeed")
+	defer roe.rsp.Body.Close()
+
+	body, err := io.ReadAll(roe.rsp.Body)
+	require.NoError(t, err)
+	assert.Equal(t, "OK", string(body))
+
+	select {
+	case <-done:
+	case <-time.After(1 * time.Second):
+		t.Errorf("Shutdown takes too long after request is finished")
+	}
 }
 
 type (