Skip to content

fix(coderd/healthcheck): add daemon-specific warnings to healthcheck output#11490

Merged
johnstcn merged 2 commits into
mainfrom
cj/provisioner-healthcheck-fixes
Jan 8, 2024
Merged

fix(coderd/healthcheck): add daemon-specific warnings to healthcheck output#11490
johnstcn merged 2 commits into
mainfrom
cj/provisioner-healthcheck-fixes

Conversation

@johnstcn

@johnstcn johnstcn commented Jan 8, 2024

Copy link
Copy Markdown
Member

Follow-up to #11393
Part of #10676

  • Sorts provisioner daemons by name ascending in output
  • Adds daemon-specific warnings to healthcheck output
  • Reword some messages

- Sorts provisioner daemons by name ascending in output
- Adds daemon-specific warnings to healthcheck output
- Reword some messages
@johnstcn johnstcn self-assigned this Jan 8, 2024
@johnstcn johnstcn changed the title fix(coderd/healthcheck): scope warnings in ProvisionerDaemonsReport fix(coderd/healthcheck): sort provisioner daemons by name in ProvisionerDaemonsReport Jan 8, 2024
@johnstcn johnstcn requested review from mafredri and mtojek January 8, 2024 13:30
Comment thread coderd/healthcheck/provisioner.go Outdated

@mtojek mtojek left a comment

Copy link
Copy Markdown
Member

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 👍

Error *string `json:"error"`

ProvisionerDaemons []codersdk.ProvisionerDaemon `json:"provisioner_daemons"`
Items []ProvisionerDaemonsReportItem `json:"items"`

Copy link
Copy Markdown
Member

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?

Copy link
Copy Markdown
Member Author

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

Copy link
Copy Markdown
Member

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?

Copy link
Copy Markdown
Member Author

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.

Comment thread coderd/healthcheck/provisioner.go
@johnstcn johnstcn changed the title fix(coderd/healthcheck): sort provisioner daemons by name in ProvisionerDaemonsReport fix(coderd/healthcheck): add daemon-specific warnings to healthcheck output Jan 8, 2024
@johnstcn johnstcn merged commit 93cf5dc into main Jan 8, 2024
@johnstcn johnstcn deleted the cj/provisioner-healthcheck-fixes branch January 8, 2024 13:55
@github-actions github-actions Bot locked and limited conversation to collaborators Jan 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants