-
Notifications
You must be signed in to change notification settings - Fork 889
feat(coderd): add parameter to force health check in /debug/health #10677
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
Conversation
4029aed
to
0b8e032
Compare
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.
This is one of these cases, when unit tests are longer than actual implementation.
|
||
// Get cached report if it exists and the requester did not force a refresh. | ||
if !forced { | ||
if report := api.healthCheckCache.Load(); report != nil { |
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.
Do we need an extra safety check to ensure that only 1 request is in-flight?
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.
I belive the healthcheck singleflight group should take care of that.
formatHealthcheck(ctx, rw, r, report) | ||
return | ||
// Check if the forced query parameter is set. | ||
forced := r.URL.Query().Get("force") == "true" |
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.
I get the invalidation concept, but if it does not take too much time, maybe we can also run the health check without caching?
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.
if it does not take too much time
I think this is a pretty big 'if' :-)
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.
nothing blocking here 👍
Part of #8969
Modifies the
/debug/health
endpoint to accept aforce=(true|_)
parameter.Iff set to
true
, will re-run the healthcheck immediately.UI changes will come in a separate PR.