Skip to content

Conversation

@twz123
Copy link
Member

@twz123 twz123 commented Nov 10, 2025

Description

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 test failures like the following:

--- FAIL: TestDownload_CancelDownload (0.02s)
    download_test.go:84: 
        	Error Trace:	/Users/runner/work/k0s/k0s/internal/http/download_test.go:84
        	            				/Users/runner/hostedtoolcache/go/1.25.4/arm64/src/net/http/server.go:2322
        	            				/Users/runner/hostedtoolcache/go/1.25.4/arm64/src/net/http/server.go:3340
        	            				/Users/runner/hostedtoolcache/go/1.25.4/arm64/src/net/http/server.go:2109
        	            				/Users/runner/hostedtoolcache/go/1.25.4/arm64/src/runtime/asm_arm64.s:1268
        	Error:      	Received unexpected error:
        	            	write tcp 127.0.0.1:49245->127.0.0.1:49246: write: broken pipe
        	Test:       	TestDownload_CancelDownload
2025/11/10 08:42:40 [INFO] generate received request
2025/11/10 08:42:40 [INFO] received CSR
2025/11/10 08:42:40 [INFO] generating key: ecdsa-256
2025/11/10 08:42:40 [INFO] encoded CSR
2025/11/10 08:42:40 [INFO] signed certificate with serial number 300036215146312345792396434938619412852128316097
2025/11/10 08:42:41 http: TLS handshake error from 127.0.0.1:49262: remote error: tls: bad certificate
FAIL
FAIL	github.com/k0sproject/k0s/internal/http	0.414s

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

How Has This Been Tested?

  • Manual test
  • Auto test added

Checklist

  • My code follows the style guidelines of this project
  • My commit messages are signed-off
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings

@twz123 twz123 added bug Something isn't working area/ci labels Nov 10, 2025
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]>
@twz123 twz123 force-pushed the cancel-download-fix branch from 58485a3 to c6577d7 Compare November 10, 2025 12:12
@twz123 twz123 added the backport/release-1.34 PR that needs to be backported/cherrypicked to the release-1.34 branch label Nov 10, 2025
@twz123 twz123 marked this pull request as ready for review November 11, 2025 08:07
@twz123 twz123 requested review from a team as code owners November 11, 2025 08:07
@twz123 twz123 merged commit 8f203c9 into k0sproject:main Nov 19, 2025
484 of 490 checks passed
@twz123 twz123 deleted the cancel-download-fix branch November 19, 2025 13:07
@k0s-bot
Copy link
Contributor

k0s-bot commented Nov 19, 2025

Successfully created backport PR for release-1.34:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/ci backport/release-1.34 PR that needs to be backported/cherrypicked to the release-1.34 branch bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants