-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Query: Fix panic on stores endpoint #4754
Query: Fix panic on stores endpoint #4754
Conversation
- Skip an endpoint from stores if component type is nil - Simplify - pass only endpoint status func instead of endpoints Signed-off-by: Matej Gera <matejgera@gmail.com>
Signed-off-by: Matej Gera <matejgera@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Is it better to add this to the current release?
I'd leave that to the maintainers' discretion, I can point this to the release as well if that's preferred. For me, this is not that critical, since it requires a specific scenario with multiple unreachable / misconfigured stores. I think so far we've seen only one report of the issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…s-invalid-endpoint Signed-off-by: Matej Gera <matejgera@gmail.com>
Changes
As was reported in #4743, it is possible that if stores provided to querier are not available (e.g. the store is down, the address is incorrect etc.), trying to call the
/stores
endpoint can result in panic. This is due to the fact that the status of the endpoint is missing information on component type. The suggested way to resolve this is to ignore such stores.Resolves #4743.
Verification
Added tests to verify
/stores
behaves as expected.