Skip to content

Commit c6577d7

Browse files
committed
Fix internal/http/TestDownload_CancelDownload
The test's server handler used to loop on writes. Once the client canceled the download, the next write hit a broken pipe and failed the test intermittently: - The handler kept looping: write chunk, check request context, short sleep, repeat. - When the test client canceled, the HTTP stack closed the TCP connection immediately. - The handler only noticed cancellation after the select ran, but there was a race between the next write and the select. - If Go scheduled the loop so that it hit another write before the select observed the request context being done, that write ran against a half-closed socket and returned broken pipe. The handler now writes once, flushes to let the client see the written data, waits for the request context being done, and asserts that the request context was canceled, hence it will never write more than once. Fixes: 1a54f6c ("Implement cancellation of stale downloads") Signed-off-by: Tom Wieczorek <[email protected]>
1 parent 453995a commit c6577d7

File tree

1 file changed

+17
-11
lines changed

1 file changed

+17
-11
lines changed

internal/http/download_test.go

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
"strings"
1616
"sync/atomic"
1717
"testing"
18-
"time"
1918

2019
"github.com/k0sproject/k0s/internal/testutil"
2120

@@ -79,18 +78,21 @@ func TestDownload_ExcessContentLength(t *testing.T) {
7978
func TestDownload_CancelDownload(t *testing.T) {
8079
ctx, cancel := context.WithCancelCause(t.Context())
8180

81+
requestDone := make(chan struct{})
8282
baseURL := startFakeDownloadServer(t, false, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
83-
for {
84-
if _, err := w.Write([]byte(t.Name())); !assert.NoError(t, err) {
85-
return
86-
}
87-
88-
select {
89-
case <-r.Context().Done():
90-
return
91-
case <-time.After(time.Microsecond):
92-
}
83+
defer close(requestDone)
84+
if _, err := w.Write([]byte(t.Name())); !assert.NoError(t, err) {
85+
return
9386
}
87+
88+
// Need to flush here, otherwise the internal response buffering will
89+
// prevent the client from receiving the data.
90+
w.(http.Flusher).Flush()
91+
92+
// Wait for the client to cancel the in-flight request.
93+
ctx := r.Context()
94+
<-ctx.Done()
95+
assert.Same(t, context.Canceled, context.Cause(ctx), "HTTP request context wasn't canceled while writing response")
9496
}))
9597

9698
err := internalhttp.Download(ctx, baseURL, internalio.WriterFunc(func(p []byte) (int, error) {
@@ -100,6 +102,10 @@ func TestDownload_CancelDownload(t *testing.T) {
100102

101103
assert.ErrorContains(t, err, "while downloading: ")
102104
assert.ErrorIs(t, err, assert.AnError)
105+
106+
// Make sure the server's HTTP handler has finished
107+
// so all the assertions have been made.
108+
<-requestDone
103109
}
104110

105111
func TestDownload_RedirectLoop(t *testing.T) {

0 commit comments

Comments
 (0)