Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

feat(coderd/healthcheck): add access URL error codes and healthcheck doc#10915

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
johnstcn merged 17 commits intomainfromcj/health-resolutions
Nov 30, 2023

Conversation

johnstcn
Copy link
Member

@johnstcnjohnstcn commentedNov 28, 2023•
edited
Loading

Relates to#8965

  • Added error codes for separate code paths in health checks
  • Prefixed errors and warnings with error code prefixes
  • Added a docs page with details on each code, cause and solution

Future work:

  • add a convenience function insite/src/api to create a troubleshooting link from an error that extracts the error code prefix matching^([A-Z0-9]+):.
  • Refactor both.error and.warnings[] to be a shape of{ message: string, code: string}. I started on this but realised it makes the diff too large, so will do it in a follow-up.

@BrunoQuaresma
Copy link
Collaborator

What would be the expected shape here? Something like:

{"warnings": ["code":"W001","message":"This is a warning"  ],"resolutions": {"W001":"https://docs.coder.com/warning/W001"  }}

@johnstcn
Copy link
MemberAuthor

johnstcn commentedNov 29, 2023•
edited
Loading

What would be the expected shape here? Something like:

{"warnings": ["code":"W001","message":"This is a warning"  ],"resolutions": {"W001":"https://docs.coder.com/warning/W001"  }}

I had envisioned{ warnings: [ { code: string, message: string } ... ] }, without the resolutions field.
Idea is that the FE would automatically construct this from${CODER_DOCS_URL}/path/to/doc#W001.

BrunoQuaresma reacted with thumbs up emoji

@BrunoQuaresma
Copy link
Collaborator

@johnstcn sounds good!

Comment on lines +13 to +34
// CodeUnknown is a catch-all health code when something unexpected goes wrong (for example, a panic).
CodeUnknown Code = "EUNKNOWN"

CodeProxyUpdate Code = "EWP01"
CodeProxyFetch Code = "EWP02"
CodeProxyVersionMismatch Code = "EWP03"
CodeProxyUnhealthy Code = "EWP04"

CodeDatabasePingFailed Code = "EDB01"
CodeDatabasePingSlow Code = "EDB02"

CodeWebsocketDial Code = "EWS01"
CodeWebsocketEcho Code = "EWS02"
CodeWebsocketMsg Code = "EWS03"

CodeAccessURLNotSet Code = "EACS01"
CodeAccessURLInvalid Code = "EACS02"
CodeAccessURLFetch Code = "EACS03"
CodeAccessURLNotOK Code = "EACS04"

CodeDERPNodeUsesWebsocket Code = `EDERP01`
CodeDERPOneNodeUnhealthy Code = `EDERP02`
Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

review: these all have to go in here to be generated properly

Co-authored-by: Muhammad Atif Ali <atif@coder.com>
Copy link
Member

@matifalimatifali left a comment•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

A few comments but this is Awesome <3

johnstcn reacted with heart emoji
@johnstcnjohnstcn marked this pull request as ready for reviewNovember 30, 2023 11:30
@mtojekmtojek self-requested a reviewNovember 30, 2023 12:00
Copy link
Member

@mtojekmtojek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

👍

@johnstcnjohnstcnenabled auto-merge (squash)November 30, 2023 12:14
@johnstcnjohnstcn merged commit4f92928 intomainNov 30, 2023
@johnstcnjohnstcn deleted the cj/health-resolutions branchNovember 30, 2023 12:15
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsNov 30, 2023
Copy link
Contributor

@spikecurtisspikecurtis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Suggestions inline

> [forced websocket connections for DERP](../cli/server.md#--derp-force-websockets).

**Solution:** ensure that any configured reverse proxy does not strip the
`Upgrade: derp` header.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

ensure that any proxies you use allow connection upgrade with theUpgrade: derp header.

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

👍

performance may be impacted for clients closest to the unhealthy DERP server.

**Solution:** Ensure that the DERP server is available and reachable over the
network on port 443, for example:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Ensure that the DERP server is available and reachable over the network, for example:

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

👍

network on port 443, for example:

```shell
curl -v "https://coder.company.com:443/derp"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

:443 is redundant if you use https://

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

👍

running Coder, using standard troubleshooting tools like `curl`:

```shell
curl -v "https://coder.company.com:443/"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

:443 is redundant with https://

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

👍

```

2. Ensure that any reverse proxy that is sitting in front of Coder's configured
access URL is not stripping the HTTP header `Upgrade: websocket`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Ensure that any reverse proxy that is serving Coder's configured access URL allows connection upgrade with the headerUpgrade: websocket

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

👍

**Problem:** One or more workspace proxies are not reachable.

**Solution:** Ensure that Coder can establish a connection to the configured
workspace proxies on port 443.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Ensure that Coder can establish a connection to the configured
workspace proxies

Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

👍

Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@spikecurtisspikecurtisspikecurtis left review comments

@matifalimatifalimatifali approved these changes

@mtojekmtojekmtojek approved these changes

Assignees

@johnstcnjohnstcn

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

5 participants
@johnstcn@BrunoQuaresma@spikecurtis@matifali@mtojek
[8]ページ先頭
©2009-2025 Movatter.jp