Skip to content

Conversation

@jbaiera
Copy link
Member

@jbaiera jbaiera commented May 3, 2022

This PR introduces an idea of preflight health indicator services to the new health service. Preflight indicators are structurally identical to regular indicators, but they are executed first when calculating health and conditionally block downstream indicators from running on an unstable or unknown cluster state.

The health service is configured with an optional list of preflight indicators. If the preflight indicators are present, the health service will execute them first when calculating a health response. If any of these preflight indicators returns a non-green result status, then the remaining health indicators will not be executed. Instead, an UNKNOWN status will be created for them and returned in the response. If computeDetails flag is set, then UNKNOWN responses will contain details about which preflight indicator blocked its execution.

jbaiera added 5 commits May 3, 2022 14:05
Preflight indicators are run before other health indicators. If any preflight indicators are not GREEN then the remaining indicators are not run. Instead, UNKNOWN results will be returned for each.
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

@elasticsearchmachine
Copy link
Collaborator

Hi @jbaiera, I've created a changelog YAML for you.

Copy link
Contributor

@andreidan andreidan left a comment

Choose a reason for hiding this comment

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

Thanks for working on this Jimmy.

This is looking great.

Can we add one more check that the cluster state is RECOVERED ? If it is not everything should be UNKNOWN and we should recommend the user to retry calling the health api

Comment on lines 190 to 194
var preflight1 = new HealthIndicatorResult("preflight1", "component1", RED, null, null, null, null);
var preflight2 = new HealthIndicatorResult("preflight2", "component2", GREEN, null, null, null, null);
var indicator1 = new HealthIndicatorResult("indicator1", "component1", GREEN, null, null, null, null);
var indicator2 = new HealthIndicatorResult("indicator2", "component1", YELLOW, null, null, null, null);
var indicator3 = new HealthIndicatorResult("indicator3", "component2", GREEN, null, null, null, null);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we generally use real values in tests?

Seeing a failure that has component2 and indicator3 in the error message is rather hard to "parse"

Copy link
Member Author

Choose a reason for hiding this comment

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

How real should these be? I want to make sure that preflight indicators that are in the same component get correctly mixed together in the results, but the master indicator doesn't share a component with anything right now. For now I'll give them some hypothetical names, but if you'd like them to be more real than that we can discuss further.

@jbaiera jbaiera requested a review from andreidan May 4, 2022 20:41
@jbaiera
Copy link
Member Author

jbaiera commented May 4, 2022

@elasticmachine update branch

Copy link
Contributor

@andreidan andreidan left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for working on this Jimmy

@andreidan
Copy link
Contributor

Relates to #84792

jbaiera and others added 3 commits May 5, 2022 10:59
Co-authored-by: Andrei Dan <andrei.dan@elastic.co>
Co-authored-by: Andrei Dan <andrei.dan@elastic.co>
@jbaiera
Copy link
Member Author

jbaiera commented May 5, 2022

@elasticmachine update branch

@jbaiera jbaiera merged commit 8c03df6 into elastic:master May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants