diff --git a/pkg/apiserver/handlers_test.go b/pkg/apiserver/handlers_test.go index ea0a916d537..d15e0cf8100 100644 --- a/pkg/apiserver/handlers_test.go +++ b/pkg/apiserver/handlers_test.go @@ -17,6 +17,7 @@ limitations under the License. package apiserver import ( + "fmt" "io/ioutil" "net/http" "net/http/httptest" @@ -41,15 +42,15 @@ func (fakeRL) Stop() {} func (f fakeRL) TryAccept() bool { return bool(f) } func (f fakeRL) Accept() {} -func expectHTTP(url string, code int, t *testing.T) { +func expectHTTP(url string, code int) error { r, err := http.Get(url) if err != nil { - t.Errorf("unexpected error: %v", err) - return + return fmt.Errorf("unexpected error: %v", err) } if r.StatusCode != code { - t.Errorf("unexpected response: %v", r.StatusCode) + return fmt.Errorf("unexpected response: %v", r.StatusCode) } + return nil } func getPath(resource, namespace, name string) string { @@ -82,6 +83,13 @@ func TestMaxInFlight(t *testing.T) { // 'short' accounted ones. calls := &sync.WaitGroup{} calls.Add(AllowedInflightRequestsNo * 2) + + // Responses is used to wait until all responses are + // received. This prevents some async requests getting EOF + // errors from prematurely closing the server + responses := sync.WaitGroup{} + responses.Add(AllowedInflightRequestsNo * 2) + // Block is used to keep requests in flight for as long as we need to. All requests will // be unblocked at the same time. block := sync.WaitGroup{} @@ -109,21 +117,25 @@ func TestMaxInFlight(t *testing.T) { for i := 0; i < AllowedInflightRequestsNo; i++ { // These should hang waiting on block... go func() { - expectHTTP(server.URL+"/foo/bar/watch", http.StatusOK, t) + if err := expectHTTP(server.URL+"/foo/bar/watch", http.StatusOK); err != nil { + t.Error(err) + } + responses.Done() }() } // Check that sever is not saturated by not-accounted calls - expectHTTP(server.URL+"/dontwait", http.StatusOK, t) - oneAccountedFinished := sync.WaitGroup{} - oneAccountedFinished.Add(1) - var once sync.Once + if err := expectHTTP(server.URL+"/dontwait", http.StatusOK); err != nil { + t.Error(err) + } // These should hang and be accounted, i.e. saturate the server for i := 0; i < AllowedInflightRequestsNo; i++ { // These should hang waiting on block... go func() { - expectHTTP(server.URL, http.StatusOK, t) - once.Do(oneAccountedFinished.Done) + if err := expectHTTP(server.URL, http.StatusOK); err != nil { + t.Error(err) + } + responses.Done() }() } // We wait for all calls to be received by the server @@ -133,18 +145,24 @@ func TestMaxInFlight(t *testing.T) { // Do this multiple times to show that it rate limit rejected requests don't block. for i := 0; i < 2; i++ { - expectHTTP(server.URL, errors.StatusTooManyRequests, t) + if err := expectHTTP(server.URL, errors.StatusTooManyRequests); err != nil { + t.Error(err) + } } // Validate that non-accounted URLs still work - expectHTTP(server.URL+"/dontwait/watch", http.StatusOK, t) + if err := expectHTTP(server.URL+"/dontwait/watch", http.StatusOK); err != nil { + t.Error(err) + } // Let all hanging requests finish block.Done() // Show that we recover from being blocked up. // Too avoid flakyness we need to wait until at least one of the requests really finishes. - oneAccountedFinished.Wait() - expectHTTP(server.URL, http.StatusOK, t) + responses.Wait() + if err := expectHTTP(server.URL, http.StatusOK); err != nil { + t.Error(err) + } } func TestReadOnly(t *testing.T) {