Skip to content

Allow machineid.AutoUpdateVersionReporter to shut down correctly#60219

Merged
boxofrad merged 2 commits intomasterfrom
boxofrad/fix-semaphore-stop
Oct 14, 2025
Merged

Allow machineid.AutoUpdateVersionReporter to shut down correctly#60219
boxofrad merged 2 commits intomasterfrom
boxofrad/fix-semaphore-stop

Conversation

@boxofrad
Copy link
Copy Markdown
Contributor

Fixes an issue where, because we were starting the leader election routine inside auth.NewServer, nothing was waiting for it to shut down correctly, and we were failing to release the semaphore.

There wasn't a huge user-facing impact (as the semaphore lease would eventually expire) but it leads to a confusing error message in the logs and delays things unnecessarily.

Credit to @marcoandredinis for finding it.

Fixes an issue where, because we were starting the leader election routine
inside `auth.NewServer`, nothing was waiting for it to shut down correctly,
and we were failing to release the semaphore.

There wasn't a huge user-facing impact (as the semaphore lease would eventually
expire) but it leads to a confusing error message in the logs and delays things
unnecessarily.
@boxofrad boxofrad force-pushed the boxofrad/fix-semaphore-stop branch from 87d0f4c to 5f751ec Compare October 14, 2025 15:01
Comment thread lib/auth/machineid/machineidv1/auto_update_version_reporter.go Outdated
Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
@boxofrad boxofrad added this pull request to the merge queue Oct 14, 2025
Merged via the queue into master with commit 7b3a563 Oct 14, 2025
40 checks passed
@boxofrad boxofrad deleted the boxofrad/fix-semaphore-stop branch October 14, 2025 15:56
@backport-bot-workflows
Copy link
Copy Markdown
Contributor

@boxofrad See the table below for backport results.

Branch Result
branch/v17 Failed
branch/v18 Failed

github-merge-queue bot pushed a commit that referenced this pull request Oct 15, 2025
* MWI: Generate `AutoUpdateBotInstanceReport` resource (#59738)

* MWI: Add `tctl` get and delete mappings for `AutoUpdateBotInstanceReport` (#60017)

* MWI: Add `teleport_bot_instances` metric (#59774)

* MWI: Log on `AutoUpdateBotInstanceReport` generation failure (#60191)

* Fix passing lock by value

* Allow `machineid.AutoUpdateVersionReporter` to shut down correctly (#60219)
github-merge-queue bot pushed a commit that referenced this pull request Oct 15, 2025
* MWI: Generate `AutoUpdateBotInstanceReport` resource (#59738)

* MWI: Add `tctl` get and delete mappings for `AutoUpdateBotInstanceReport` (#60017)

* MWI: Add `teleport_bot_instances` metric (#59774)

* MWI: Log on `AutoUpdateBotInstanceReport` generation failure (#60191)

* Allow `machineid.AutoUpdateVersionReporter` to shut down correctly (#60219)
mmcallister pushed a commit that referenced this pull request Nov 6, 2025
…60219)

* Allow `machineid.AutoUpdateVersionReporter` to shut down correctly

Fixes an issue where, because we were starting the leader election routine
inside `auth.NewServer`, nothing was waiting for it to shut down correctly,
and we were failing to release the semaphore.

There wasn't a huge user-facing impact (as the semaphore lease would eventually
expire) but it leads to a confusing error message in the logs and delays things
unnecessarily.

* Use `Clock.Since` instead of manually subtracting time

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

---------

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
rhammonds-teleport pushed a commit that referenced this pull request Nov 6, 2025
…60219)

* Allow `machineid.AutoUpdateVersionReporter` to shut down correctly

Fixes an issue where, because we were starting the leader election routine
inside `auth.NewServer`, nothing was waiting for it to shut down correctly,
and we were failing to release the semaphore.

There wasn't a huge user-facing impact (as the semaphore lease would eventually
expire) but it leads to a confusing error message in the logs and delays things
unnecessarily.

* Use `Clock.Since` instead of manually subtracting time

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

---------

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
mmcallister pushed a commit that referenced this pull request Nov 19, 2025
…60219)

* Allow `machineid.AutoUpdateVersionReporter` to shut down correctly

Fixes an issue where, because we were starting the leader election routine
inside `auth.NewServer`, nothing was waiting for it to shut down correctly,
and we were failing to release the semaphore.

There wasn't a huge user-facing impact (as the semaphore lease would eventually
expire) but it leads to a confusing error message in the logs and delays things
unnecessarily.

* Use `Clock.Since` instead of manually subtracting time

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

---------

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
mmcallister pushed a commit that referenced this pull request Nov 20, 2025
…60219)

* Allow `machineid.AutoUpdateVersionReporter` to shut down correctly

Fixes an issue where, because we were starting the leader election routine
inside `auth.NewServer`, nothing was waiting for it to shut down correctly,
and we were failing to release the semaphore.

There wasn't a huge user-facing impact (as the semaphore lease would eventually
expire) but it leads to a confusing error message in the logs and delays things
unnecessarily.

* Use `Clock.Since` instead of manually subtracting time

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>

---------

Co-authored-by: Zac Bergquist <zac.bergquist@goteleport.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants