From e5a15c87e9d83ee19ba93aa356dfbb7b33a013c8 Mon Sep 17 00:00:00 2001 From: Ben Luddy Date: Wed, 7 Jun 2023 12:48:33 -0400 Subject: [PATCH] Ensure timeout test handlers don't complete before timing out. TestTimeoutRequestHeaders and TestTimeoutWithLogging are designed to catch data races on request headers and include an HTTP handler that triggers timeout then repeatedly mutates request headers. Sometimes, the request header mutation loop could complete before the timeout filter observed the timeout, resulting in a test failure. The mutation loop now runs until the test ends. --- .../pkg/server/filters/timeout_test.go | 30 ++++++++++++++----- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/server/filters/timeout_test.go b/staging/src/k8s.io/apiserver/pkg/server/filters/timeout_test.go index cf4203f72a6..78b4ba7a89b 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/filters/timeout_test.go +++ b/staging/src/k8s.io/apiserver/pkg/server/filters/timeout_test.go @@ -272,6 +272,8 @@ func TestTimeoutRequestHeaders(t *testing.T) { }) } + testDone := make(chan struct{}) + defer close(testDone) ts := httptest.NewServer( withDeadline( WithTimeoutForNonLongRunningRequests( @@ -280,8 +282,13 @@ func TestTimeoutRequestHeaders(t *testing.T) { cancel() // mutate request Headers // Authorization filter does it for example - for j := 0; j < 10000; j++ { - req.Header.Set("Test", "post") + for { + select { + case <-testDone: + return + default: + req.Header.Set("Test", "post") + } } }), func(r *http.Request, requestInfo *request.RequestInfo) bool { @@ -301,8 +308,8 @@ func TestTimeoutRequestHeaders(t *testing.T) { if err != nil { t.Fatal(err) } - if res.StatusCode != http.StatusGatewayTimeout { - t.Errorf("got res.StatusCde %d; expected %d", res.StatusCode, http.StatusServiceUnavailable) + if actual, expected := res.StatusCode, http.StatusGatewayTimeout; actual != expected { + t.Errorf("got status code %d; expected %d", actual, expected) } res.Body.Close() } @@ -323,6 +330,8 @@ func TestTimeoutWithLogging(t *testing.T) { }) } + testDone := make(chan struct{}) + defer close(testDone) ts := httptest.NewServer( WithHTTPLogging( withDeadline( @@ -332,8 +341,13 @@ func TestTimeoutWithLogging(t *testing.T) { cancel() // mutate request Headers // Authorization filter does it for example - for j := 0; j < 10000; j++ { - req.Header.Set("Test", "post") + for { + select { + case <-testDone: + return + default: + req.Header.Set("Test", "post") + } } }), func(r *http.Request, requestInfo *request.RequestInfo) bool { @@ -354,8 +368,8 @@ func TestTimeoutWithLogging(t *testing.T) { if err != nil { t.Fatal(err) } - if res.StatusCode != http.StatusGatewayTimeout { - t.Errorf("got res.StatusCode %d; expected %d", res.StatusCode, http.StatusServiceUnavailable) + if actual, expected := res.StatusCode, http.StatusGatewayTimeout; actual != expected { + t.Errorf("got status code %d; expected %d", actual, expected) } res.Body.Close() }