engine: do not start local resources with invalid probe specs #4158
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
Original behavior tried to fail gracefully, i.e. let the resource start and just mark it as ready with a warning logged that the probe spec was bad. There was a bug, however, which meant that dependent resources would still see it as not ready and not start. In the end, it seems that this behavior was more confusing than helpful, so it's better to just prevent the resource from startup if the probe spec is invalid. Some validation has also been pushed down to the `Tiltfile` level, which will provide better errors with line numbers. It's worth noting, however, that there will likely always be some validation/ cases that we cannot catch during `Tiltfile` parsing, so the logic in the controller is still important.
|
This pull request has been linked to Clubhouse Story #11119: local resources with invalid readiness probe specs cause dependent resources to never start. |
landism
approved these changes
Feb 4, 2021
milas
deleted the
milas/ch11119/local-resources-with-invalid-readiness-probe
branch
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.
Original behavior tried to fail gracefully, i.e. let the resource
start and just mark it as ready with a warning logged that the probe
spec was bad. There was a bug, however, which meant that dependent
resources would still see it as not ready and not start.
In the end, it seems that this behavior was more confusing than
helpful, so it's better to just prevent the resource from startup
if the probe spec is invalid.
Some validation has also been pushed down to the
Tiltfilelevel,which will provide better errors with line numbers. It's worth
noting, however, that there will likely always be some validation/
cases that we cannot catch during
Tiltfileparsing, so the logicin the controller is still important.
Closes #4156.