Skip to content

Make StartRotation concurrency safe#34186

Merged
sgress454 merged 4 commits intomainfrom
sgress454/fix-race-condition-in-rotation
Oct 15, 2025
Merged

Make StartRotation concurrency safe#34186
sgress454 merged 4 commits intomainfrom
sgress454/fix-race-condition-in-rotation

Conversation

@sgress454
Copy link
Copy Markdown
Contributor

Related issue: For #33111

Fixes a possible race condition introduced in #33884 which saw a test failure here.

Checklist for submitter

Testing

  • Added/updated automated tests
    Existing test is sufficient, will circle back if it fails again.

  • QA'd all new/changed functionality manually
    Verified that Fleet Desktop still opens My Device page correctly and token rotation logs are still seen.

For unreleased bug fixes in a release candidate, one of:

  • Confirmed that the fix is not expected to adversely impact load test results

lucasmrod
lucasmrod previously approved these changes Oct 14, 2025
@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 14, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.22%. Comparing base (dcefbc4) to head (ce303cb).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #34186      +/-   ##
==========================================
+ Coverage   64.21%   64.22%   +0.01%     
==========================================
  Files        2060     2060              
  Lines      207193   207279      +86     
  Branches     6895     6895              
==========================================
+ Hits       133046   133125      +79     
- Misses      63712    63717       +5     
- Partials    10435    10437       +2     
Flag Coverage Δ
backend 65.35% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sgress454
Copy link
Copy Markdown
Contributor Author

@lucasmrod Turns out I was not testing in race-condition mode like I thought, and once I fixed that it still failed because the actual issue causing the failure was in the test itself (although the patches to the rotation code are still needed!).

I patched the test using atomic and also fixed the name because rotater is apparently not a thing 🙃 . Verified with

go test -race -timeout 5m -tags full,fts5 -run ^TestRotator$ github.com/fleetdm/fleet/v4/orbit/pkg/token

@sgress454 sgress454 merged commit cbde3f7 into main Oct 15, 2025
64 of 85 checks passed
@sgress454 sgress454 deleted the sgress454/fix-race-condition-in-rotation branch October 15, 2025 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants