-
Notifications
You must be signed in to change notification settings - Fork 929
fix(coderd/healthcheck): add daemon-specific warnings to healthcheck output #11490
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
- Sorts provisioner daemons by name ascending in output - Adds daemon-specific warnings to healthcheck output - Reword some messages
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.
Left a few comments to address/respond to. Otherwise, it is 👍
@@ -26,7 +27,13 @@ type ProvisionerDaemonsReport struct { | |||
Dismissed bool `json:"dismissed"` | |||
Error *string `json:"error"` | |||
|
|||
ProvisionerDaemons []codersdk.ProvisionerDaemon `json:"provisioner_daemons"` | |||
Items []ProvisionerDaemonsReportItem `json:"items"` |
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.
In theory, this is API breaking, right?
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.
Yeah in theory, but the modifications were only merged this morning :-)
|
||
// Ensure stable order for display and for tests | ||
sort.Slice(daemons, func(i, j int) bool { | ||
return daemons[i].Name < daemons[j].Name |
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.
doublecheck: we don't need to sort by other fields? There is no way that the report will contain old entries for provisioners with the same name?
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.
The database schema requires each provisioner daemon to have a unique name.
Follow-up to #11393
Part of #10676