Skip to content

Fix flaky test TestChaosUpload#62189

Merged
Joerger merged 8 commits intomasterfrom
joerger/fix-flaky-test-TestChaosUpload
Dec 18, 2025
Merged

Fix flaky test TestChaosUpload#62189
Joerger merged 8 commits intomasterfrom
joerger/fix-flaky-test-TestChaosUpload

Conversation

@Joerger
Copy link
Copy Markdown
Contributor

@Joerger Joerger commented Dec 12, 2025

Change:

  • Make the test run magnitudes faster and allow the --race flag.
    • From my local testing, it doesn't not seem like the slow scan period was not warranted and may have just been covering up underlying issues with the test. That may change after seeing CI run it.
  • Don't treat empty uploads as a sessionError in uploadEncrypted, mirroring the default upload method.
    • It's unclear whether this should actually be working this way in either case. We have had issues in the recent past where recordings are truncated and never cleaned up - e.g. moved to the corrupted folder with an .error file if relevant. However, this can be handled in a follow up PR if the legacy behavior is deemed undesirable rather than blocking this high priority flaky test fix.
  • Add a missing FS unlock when uploadEncrypted hits an unexpected error. This flake appeared once ^ was fixed.

Closes #33099

@Joerger Joerger requested a review from eriktate December 12, 2025 00:56
@github-actions github-actions bot added the audit-log Issues related to Teleports Audit Log label Dec 12, 2025
@github-actions github-actions bot requested a review from tigrato December 12, 2025 00:57
@Joerger Joerger added the no-changelog Indicates that a PR does not require a changelog entry label Dec 12, 2025
@Joerger Joerger force-pushed the joerger/fix-flaky-test-TestChaosUpload branch from bb68b45 to a7dbadd Compare December 12, 2025 01:13
@Joerger Joerger force-pushed the joerger/fix-flaky-test-TestChaosUpload branch from a7dbadd to 59ac781 Compare December 12, 2025 01:15
Comment thread lib/events/filesessions/fileasync.go Outdated
@Joerger Joerger requested a review from zmb3 December 12, 2025 18:21
Comment thread lib/events/filesessions/fileasync.go
@Joerger Joerger requested a review from tigrato December 12, 2025 18:29
@Joerger
Copy link
Copy Markdown
Contributor Author

Joerger commented Dec 17, 2025

@rosstimothy @zmb3 Can you give this a flaky test skip? Looks like it's just timing out, takes just under 6s locally:

> go test -run ^TestChaosUpload$ github.com/gravitational/teleport/lib/events/filesessions -tags=libfido2,piv --count=1 --race              
ok  	github.com/gravitational/teleport/lib/events/filesessions	5.515s

@rosstimothy
Copy link
Copy Markdown
Contributor

/excludeflake *

@Joerger Joerger enabled auto-merge December 18, 2025 19:35
@Joerger Joerger added this pull request to the merge queue Dec 18, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 18, 2025
@Joerger Joerger added this pull request to the merge queue Dec 18, 2025
Merged via the queue into master with commit c5bd2b0 Dec 18, 2025
41 checks passed
@Joerger Joerger deleted the joerger/fix-flaky-test-TestChaosUpload branch December 18, 2025 21:09
@backport-bot-workflows
Copy link
Copy Markdown
Contributor

@Joerger See the table below for backport results.

Branch Result
branch/v18 Failed

rosstimothy pushed a commit that referenced this pull request Jan 5, 2026
* Significantly speed up TestChaosUpload with a more aggressive scan period.

* Add missing close for encrypted upload error.

* Don't treat empty uploads as a session error.

* Defer close.

* Add comment.
21KennethTran pushed a commit that referenced this pull request Jan 6, 2026
* Significantly speed up TestChaosUpload with a more aggressive scan period.

* Add missing close for encrypted upload error.

* Don't treat empty uploads as a session error.

* Defer close.

* Add comment.
github-merge-queue bot pushed a commit that referenced this pull request Jan 7, 2026
* Significantly speed up TestChaosUpload with a more aggressive scan period.

* Add missing close for encrypted upload error.

* Don't treat empty uploads as a session error.

* Defer close.

* Add comment.

Co-authored-by: Brian Joerger <bjoerger@goteleport.com>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it feasible to backport these changes to v17? I just got a hit on v17. I tried backporting this PR myself, but it doesn't backport cleanly.

Joerger added a commit that referenced this pull request Mar 11, 2026
* Significantly speed up TestChaosUpload with a more aggressive scan period.

* Add missing close for encrypted upload error.

* Don't treat empty uploads as a session error.

* Defer close.

* Add comment.
github-merge-queue bot pushed a commit that referenced this pull request Apr 2, 2026
* Significantly speed up TestChaosUpload with a more aggressive scan period.

* Add missing close for encrypted upload error.

* Don't treat empty uploads as a session error.

* Defer close.

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

Labels

audit-log Issues related to Teleports Audit Log backport/branch/v18 no-changelog Indicates that a PR does not require a changelog entry size/sm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TestChaosUpload flakiness

6 participants