kubernetes
/
apiserver
mirror of https://github.com/kubernetes/apiserver.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #102511 from tkashem/issue-102016 

apf: fix data race in test

Kubernetes-commit: ff6f9e009a176884989c42e43c4deba7fa5ed0c9
This commit is contained in:
Kubernetes Publisher 2021-06-07 00:52:51 -07:00
parent 71dfa70b21 9421ee44c4
commit e19cdaa216
4 changed files with 71 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
Godeps/Godeps.json generated
View File

@ -944,7 +944,7 @@
},
{
"ImportPath": "k8s.io/component-base",
"Rev": "1946a51be3de"
"Rev": "89ef37005142"
},
{
"ImportPath": "k8s.io/gengo",

4
go.mod
View File

@ -41,7 +41,7 @@ require (
k8s.io/api v0.0.0-20210604195109-9f22d1265651
k8s.io/apimachinery v0.0.0-20210604114423-aec8116c445f
k8s.io/client-go v0.0.0-20210604195650-ded678f91ed5
k8s.io/component-base v0.0.0-20210605195000-1946a51be3de
k8s.io/component-base v0.0.0-20210607075009-89ef37005142
k8s.io/klog/v2 v2.9.0
k8s.io/kube-openapi v0.0.0-20210421082810-95288971da7e
k8s.io/utils v0.0.0-20210521133846-da695404a2bc
@ -54,5 +54,5 @@ replace (
k8s.io/api => k8s.io/api v0.0.0-20210604195109-9f22d1265651
k8s.io/apimachinery => k8s.io/apimachinery v0.0.0-20210604114423-aec8116c445f
k8s.io/client-go => k8s.io/client-go v0.0.0-20210604195650-ded678f91ed5
k8s.io/component-base => k8s.io/component-base v0.0.0-20210605195000-1946a51be3de
k8s.io/component-base => k8s.io/component-base v0.0.0-20210607075009-89ef37005142
)

4
go.sum
View File

@ -701,8 +701,8 @@ k8s.io/apimachinery v0.0.0-20210604114423-aec8116c445f h1:DmoZH3nTdy0sXQ7iZ6Gd3b
k8s.io/apimachinery v0.0.0-20210604114423-aec8116c445f/go.mod h1:5zcgojGmAy5Bo3S4mgZWAt6HwoKzaSh4MV3ITvlcOVM=
k8s.io/client-go v0.0.0-20210604195650-ded678f91ed5 h1:zb0G/VmukmYiPgkwoAJ8wn41hwO3mySwMny0C1XkTCo=
k8s.io/client-go v0.0.0-20210604195650-ded678f91ed5/go.mod h1:kSx8A96VUSpBA4jTX1ogcPiKm8hb7r1mbnUMpk0g/1w=
k8s.io/component-base v0.0.0-20210605195000-1946a51be3de h1:fXG9daybdpGcUPH1GRSzEW5Y/XnDwdGQfUYTRup1/G4=
k8s.io/component-base v0.0.0-20210605195000-1946a51be3de/go.mod h1:crEIsgh9DV2JslpwAGBUoOyYTr2nj2UH4xFZjghQE34=
k8s.io/component-base v0.0.0-20210607075009-89ef37005142 h1:O8oAedO9/fHf4DwxWWD/vwfQnOZFSVZM0SQZMzVvPzU=
k8s.io/component-base v0.0.0-20210607075009-89ef37005142/go.mod h1:crEIsgh9DV2JslpwAGBUoOyYTr2nj2UH4xFZjghQE34=
k8s.io/gengo v0.0.0-20200413195148-3a45101e95ac/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0=
k8s.io/klog/v2 v2.0.0/go.mod h1:PBfzABfn139FHAV07az/IF9Wp1bkk3vpT2XSJ76fSDE=
k8s.io/klog/v2 v2.9.0 h1:D7HV+n1V57XeZ0m6tdRkfknthUaM06VFbWldOFh8kzM=

132
pkg/server/filters/priority-and-fairness_test.go
View File

@ -594,8 +594,10 @@ func TestPriorityAndFairnessWithPanicRecoveryAndTimeoutFilter(t *testing.T) {
server, requestGetter := newHTTP2ServerWithClient(handler, requestTimeout*2)
defer server.Close()
var err error
_, err = requestGetter(firstRequestPathPanic)
// we send two requests synchronously, one at a time
// - first request is expected to panic as designed
// - second request is expected to success
_, err := requestGetter(firstRequestPathPanic)
if !executed {
t.Errorf("Expected inner handler to be executed for request: %q", firstRequestPathPanic)
}
@ -660,11 +662,8 @@ func TestPriorityAndFairnessWithPanicRecoveryAndTimeoutFilter(t *testing.T) {
server, requestGetter := newHTTP2ServerWithClient(handler, requestTimeout*2)
defer server.Close()
go hardStop(t, stopCh, requestTimeout*3, func() {
// the client timed out unexpectedly, let's clean up and abort.
close(reqHandlerCompletedCh)
})
// send a request synchronously with a client timeout of requestTimeout*2 seconds
// this ensures the test does not block indefinitely if the server does not respond.
var (
response *http.Response
err error
@ -715,6 +714,7 @@ func TestPriorityAndFairnessWithPanicRecoveryAndTimeoutFilter(t *testing.T) {
stopCh := make(chan struct{})
controller, controllerCompletedCh := startAPFController(t, stopCh, apfConfiguration, serverConcurrency, requestTimeout/4, plName, plConcurrency)
var innerHandlerWriteErr error
reqHandlerCompletedCh, callerRoundTripDoneCh := make(chan struct{}), make(chan struct{})
rquestTimesOutPath := "/request/time-out-as-designed"
requestHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@ -726,9 +726,7 @@ func TestPriorityAndFairnessWithPanicRecoveryAndTimeoutFilter(t *testing.T) {
// we expect the timeout handler to have timed out this request by now and any attempt
// to write to the response should return a http.ErrHandlerTimeout error.
if _, err := w.Write([]byte("foo")); err != http.ErrHandlerTimeout {
t.Fatalf("Expected error: %#v, but got: %#v", http.ErrHandlerTimeout, err)
}
_, innerHandlerWriteErr = w.Write([]byte("foo"))
panic(http.ErrAbortHandler)
}
@ -738,11 +736,8 @@ func TestPriorityAndFairnessWithPanicRecoveryAndTimeoutFilter(t *testing.T) {
server, requestGetter := newHTTP2ServerWithClient(handler, requestTimeout*2)
defer server.Close()
go hardStop(t, stopCh, requestTimeout*3, func() {
// the client timed out unexpectedly, let's clean up and abort.
close(reqHandlerCompletedCh)
})
// send a request synchronously with a client timeout of requestTimeout*2 seconds
// this ensures the test does not block indefinitely if the server does not respond.
var (
response *http.Response
err error
@ -759,6 +754,9 @@ func TestPriorityAndFairnessWithPanicRecoveryAndTimeoutFilter(t *testing.T) {
t.Logf("Waiting for the inner handler of the request: %q to complete", rquestTimesOutPath)
<-reqHandlerCompletedCh
if innerHandlerWriteErr != http.ErrHandlerTimeout {
t.Fatalf("Expected error: %#v, but got: %#v", http.ErrHandlerTimeout, err)
}
if err != nil {
t.Fatalf("Expected request: %q to get a response, but got error: %#v", rquestTimesOutPath, err)
}
@ -789,6 +787,7 @@ func TestPriorityAndFairnessWithPanicRecoveryAndTimeoutFilter(t *testing.T) {
stopCh := make(chan struct{})
controller, controllerCompletedCh := startAPFController(t, stopCh, apfConfiguration, serverConcurrency, requestTimeout/4, plName, plConcurrency)
var innerHandlerWriteErr error
rquestTimesOutPath := "/request/time-out-as-designed"
reqHandlerCompletedCh, callerRoundTripDoneCh := make(chan struct{}), make(chan struct{})
requestHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@ -803,9 +802,7 @@ func TestPriorityAndFairnessWithPanicRecoveryAndTimeoutFilter(t *testing.T) {
// we expect the timeout handler to have timed out this request by now and any attempt
// to write to the response should return a http.ErrHandlerTimeout error.
if _, err := w.Write([]byte("foo")); err != http.ErrHandlerTimeout {
t.Fatalf("Expected error: %#v, but got: %#v", http.ErrHandlerTimeout, err)
}
_, innerHandlerWriteErr = w.Write([]byte("foo"))
}
})
handler := newHandlerChain(t, requestHandler, controller, userName, requestTimeout)
@ -813,11 +810,6 @@ func TestPriorityAndFairnessWithPanicRecoveryAndTimeoutFilter(t *testing.T) {
server, requestGetter := newHTTP2ServerWithClient(handler, requestTimeout*2)
defer server.Close()
go hardStop(t, stopCh, requestTimeout*3, func() {
// the client timed out unexpectedly, let's clean up and abort.
close(reqHandlerCompletedCh)
})
var err error
func() {
defer close(callerRoundTripDoneCh)
@ -831,6 +823,9 @@ func TestPriorityAndFairnessWithPanicRecoveryAndTimeoutFilter(t *testing.T) {
t.Logf("Waiting for the inner handler of the request: %q to complete", rquestTimesOutPath)
<-reqHandlerCompletedCh
if innerHandlerWriteErr != http.ErrHandlerTimeout {
t.Fatalf("Expected error: %#v, but got: %#v", http.ErrHandlerTimeout, err)
}
expectResetStreamError(t, err)
close(stopCh)
@ -844,6 +839,12 @@ func TestPriorityAndFairnessWithPanicRecoveryAndTimeoutFilter(t *testing.T) {
t.Run("priority level concurrency is set to 1, queue length is 1, first request should time out and second (enqueued) request should time out as well", func(t *testing.T) {
t.Parallel()
type result struct {
err error
response *http.Response
}
const (
requestTimeout = 5 * time.Second
userName = "alice"
@ -856,6 +857,8 @@ func TestPriorityAndFairnessWithPanicRecoveryAndTimeoutFilter(t *testing.T) {
stopCh := make(chan struct{})
controller, controllerCompletedCh := startAPFController(t, stopCh, apfConfiguration, serverConcurrency, requestTimeout/4, plName, plConcurrency)
var firstRequestInnerHandlerWriteErr error
var secondRequestExecuted bool
firstRequestTimesOutPath := "/request/first/time-out-as-designed"
secondRequestEnqueuedPath := "/request/second/enqueued-as-designed"
firstReqHandlerCompletedCh, firstReqInProgressCh := make(chan struct{}), make(chan struct{})
@ -875,15 +878,13 @@ func TestPriorityAndFairnessWithPanicRecoveryAndTimeoutFilter(t *testing.T) {
// we expect the timeout handler to have timed out this request by now and any attempt
// to write to the response should return a http.ErrHandlerTimeout error.
if _, err := w.Write([]byte("foo")); err != http.ErrHandlerTimeout {
t.Fatalf("Expected error: %#v, but got: %#v", http.ErrHandlerTimeout, err)
}
_, firstRequestInnerHandlerWriteErr = w.Write([]byte("foo"))
return
}
if r.URL.Path == secondRequestEnqueuedPath {
// we expect the concurrency to be set to 1 and so this request should never be executed.
t.Fatalf("Expected request to be enqueued: %q", secondRequestEnqueuedPath)
secondRequestExecuted = true
}
})
handler := newHandlerChain(t, requestHandler, controller, userName, requestTimeout)
@ -891,52 +892,61 @@ func TestPriorityAndFairnessWithPanicRecoveryAndTimeoutFilter(t *testing.T) {
server, requestGetter := newHTTP2ServerWithClient(handler, requestTimeout*2)
defer server.Close()
go hardStop(t, stopCh, requestTimeout*3, func() {
// the client timed out unexpectedly, let's clean up and abort.
close(firstReqInProgressCh)
close(firstReqHandlerCompletedCh)
})
var firstReqErr, secondReqErr error
var resp1, resp2 *http.Response
// we send two requests, each with a client timeout of requestTimeout*2 seconds
// this ensures the test does not block indefinitely if the server does not respond.
// - first request (expected to timeout as designed) sent from a new goroutine
// - second request (expected to be enqueued) is sent from a new goroutine
firstReqResultCh, secondReqResultCh := make(chan result, 1), make(chan result, 1)
go func() {
defer close(firstReqRoundTripDoneCh)
t.Logf("Waiting for the request: %q to time out", firstRequestTimesOutPath)
resp1, firstReqErr = requestGetter(firstRequestTimesOutPath)
if isClientTimeout(firstReqErr) {
t.Fatalf("the client has unexpectedly timed out - request: %q error: %s", firstRequestTimesOutPath, firstReqErr.Error())
}
t.Logf("Sending request: %q", firstRequestTimesOutPath)
resp, err := requestGetter(firstRequestTimesOutPath)
t.Logf("RoundTrip of request: %q has completed", firstRequestTimesOutPath)
firstReqResultCh <- result{err: err, response: resp}
}()
func() {
go func() {
// we must wait for the "first" request to start executing before
// we can initiate the "second".
defer close(secondReqRoundTripDoneCh)
// we must wait for the "first" request to start executing first
<-firstReqInProgressCh
resp2, secondReqErr = requestGetter(secondRequestEnqueuedPath)
if isClientTimeout(secondReqErr) {
t.Fatalf("the client has unexpectedly timed out - request: %q error: %s", secondRequestEnqueuedPath, secondReqErr.Error())
}
t.Logf("Sending request: %q", secondRequestEnqueuedPath)
resp, err := requestGetter(secondRequestEnqueuedPath)
t.Logf("RoundTrip of request: %q has completed", secondRequestEnqueuedPath)
secondReqResultCh <- result{err: err, response: resp}
}()
<-firstReqRoundTripDoneCh
firstReqResult := <-firstReqResultCh
if isClientTimeout(firstReqResult.err) {
t.Fatalf("the client has unexpectedly timed out - request: %q error: %s", firstRequestTimesOutPath, firstReqResult.err.Error())
}
t.Logf("Waiting for the inner handler of the request: %q to complete", firstRequestTimesOutPath)
<-firstReqHandlerCompletedCh
// first request is expected to time out.
if firstReqErr != nil {
t.Fatalf("Expected request: %q to get a response, but got error: %#v", firstRequestTimesOutPath, firstReqErr)
if firstRequestInnerHandlerWriteErr != http.ErrHandlerTimeout {
t.Fatalf("Expected error: %#v, but got: %#v", http.ErrHandlerTimeout, firstRequestInnerHandlerWriteErr)
}
if resp1.StatusCode != http.StatusGatewayTimeout {
t.Errorf("Expected HTTP status code: %d for request: %q, but got: %#v", http.StatusGatewayTimeout, firstRequestTimesOutPath, resp1)
if firstReqResult.err != nil {
t.Fatalf("Expected request: %q to get a response, but got error: %#v", firstRequestTimesOutPath, firstReqResult.err)
}
if firstReqResult.response.StatusCode != http.StatusGatewayTimeout {
t.Errorf("Expected HTTP status code: %d for request: %q, but got: %#v", http.StatusGatewayTimeout, firstRequestTimesOutPath, firstReqResult.response)
}
// second request is expected to either be rejected (ideal behavior) or time out (current approximation of the ideal behavior)
if secondReqErr != nil {
t.Fatalf("Expected request: %q to get a response, but got error: %#v", secondRequestEnqueuedPath, secondReqErr)
secondReqResult := <-secondReqResultCh
if isClientTimeout(secondReqResult.err) {
t.Fatalf("the client has unexpectedly timed out - request: %q error: %s", secondRequestEnqueuedPath, secondReqResult.err.Error())
}
if !(resp2.StatusCode == http.StatusTooManyRequests || resp2.StatusCode == http.StatusGatewayTimeout) {
t.Errorf("Expected HTTP status code: %d or %d for request: %q, but got: %#v", http.StatusTooManyRequests, http.StatusGatewayTimeout, secondRequestEnqueuedPath, resp2)
if secondRequestExecuted {
t.Errorf("Expected second request to be enqueued: %q", secondRequestEnqueuedPath)
}
if secondReqResult.err != nil {
t.Fatalf("Expected request: %q to get a response, but got error: %#v", secondRequestEnqueuedPath, secondReqResult.err)
}
if !(secondReqResult.response.StatusCode == http.StatusTooManyRequests || secondReqResult.response.StatusCode == http.StatusGatewayTimeout) {
t.Errorf("Expected HTTP status code: %d or %d for request: %q, but got: %#v", http.StatusTooManyRequests, http.StatusGatewayTimeout, secondRequestEnqueuedPath, secondReqResult.response)
}
close(stopCh)
@ -1212,13 +1222,3 @@ func isClientTimeout(err error) bool {
}
return false
}
func hardStop(t *testing.T, stopCh <-chan struct{}, testTimeout time.Duration, cleanup func()) {
select {
case <-stopCh:
// The test has completed normally.
case <-time.After(testTimeout):
cleanup()
t.Fatalf("the test did not finish within %s", testTimeout)
}
}