Backport #11725 #11249 #11799 to branch/v9 #11795
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
April 7, 2022 13:52
* Throw startup error if `TeleportReadyEvent` is not emitted Before this commit, the `TeleportReadyEvent` was only waited for when a process reload occurred. Thus, if a bug exists in the code that emits this event (as it's currently the case since the `MetricsReady` and `WindowsDesktopReady` events are never emitted), such a bug may go unnoticed for a while. This commit ensures that the `TeleportReadyEvent` is always waited for on startup, and throws an error if the event is not emitted (after some timeout). This commit also: - removes the `MetricsReady` event (as this is not produced by a component that sends heartbeats, which is the case of every other event required by the `TeleportReadyEvent` event mapping) - ensures that `WindowsDesktopReady` event is emitted - refactors some of the code in `lib/service/supervisor.go` - moves the event mapping registration to a new `registerTeleportReadyEvent` function
…#11249) Fixes #11065. This commit: - ensures that `TeleportReadyEvent` is only produced when all components that send heartbeats (i.e. call [`process.onHeartbeat`](https://github.com/gravitational/teleport/blob/16bf416556f337b045b66dc9c3f5a3e16f8cc988/lib/service/service.go#L358-L366)) are ready - changes `TeleportProcess.registerTeleportReadyEvent` so that it returns a count of these components (let's call it `componentCount`) - uses `componentCount` to also ensure that `stateOK` is only reported when all the components have sent their heartbeat, thus fixing #11065 Since it seems difficult to know when `TeleportProcess.registerTeleportReadyEvent` should be updated, with the goal of quickly detecting a bug when it's introduced we have that: 1. if `componentCount` is lower than it should, then the service fails to start (due to #11725) 2. if `componentCount` is higher than it should, then an error is logged in function `processState.getStateLocked`.
github-actions
bot
requested review from
espadolini,
r0mant,
smallinsky,
xacrimon and
zmb3
espadolini
approved these changes
Apr 7, 2022
xacrimon
approved these changes
Apr 7, 2022
* Make `PortList.Pop()` thread-safe * Store a `[]int` instead of `[]string` in `PortList`
vitorenesduarte
changed the title
zmb3
approved these changes
Apr 8, 2022
Closed
r0mant
added a commit
that referenced
this pull request
Apr 26, 2022
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Combined backport:
TeleportReadyEventis not emitted #11725PortList.Pop()thread-safe #11799