Skip to content

feat(coderd/healthcheck): add health check for proxy #10846

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.

Sign up for GitHub

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

Merged
merged 24 commits into from
Nov 24, 2023
Merged

feat(coderd/healthcheck): add health check for proxy #10846

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
a17ec4c
feat(coderd/healthcheck): add health check for proxy
johnstcn Nov 21, 2023
49b6e19
rm logger
johnstcn Nov 23, 2023
c7e202c
wire it up
johnstcn Nov 23, 2023
89bae7e
add severity, convert to table-driven tests
johnstcn Nov 23, 2023
bcdc08c
dbauthz
johnstcn Nov 23, 2023
340a276
make gen; make fmt
johnstcn Nov 23, 2023
d3478c8
Merge remote-tracking branch 'origin/main' into cj/workspaceproxy-hea…
johnstcn Nov 24, 2023
c746021
fix linter import shadowing complaint
johnstcn Nov 24, 2023
05d4b09
fix generated report name
johnstcn Nov 24, 2023
56be002
gen harder
johnstcn Nov 24, 2023
1112b1c
address comments
johnstcn Nov 24, 2023
fed35bd
Merge remote-tracking branch 'origin/main' into cj/workspaceproxy-hea…
johnstcn Nov 24, 2023
7f8760a
rename WorkspaceProxies -> workspace_proxies
johnstcn Nov 24, 2023
5f5c486
fixup! rename WorkspaceProxies -> workspace_proxies
johnstcn Nov 24, 2023
b9cbdd8
use severity value instead of direct comparison
johnstcn Nov 24, 2023
ae30089
more tests
johnstcn Nov 24, 2023
21f52bb
use errors.Join instead of multierror.Append
johnstcn Nov 24, 2023
79dc190
refactor to use interface instead of atomic.Pointers
johnstcn Nov 24, 2023
2530320
rm unnecessary authz
johnstcn Nov 24, 2023
f192488
fix typegen
johnstcn Nov 24, 2023
b45e9c5
Merge remote-tracking branch 'origin/main' into cj/workspaceproxy-hea…
johnstcn Nov 24, 2023
63fbafb
make typescript stop whingeing
johnstcn Nov 24, 2023
6de94bc
Merge remote-tracking branch 'origin/main' into cj/workspaceproxy-hea…
johnstcn Nov 24, 2023
b57cdeb
remove need for gosimple nolint
johnstcn Nov 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions coderd/apidoc/docs.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 26 additions & 0 deletions coderd/apidoc/swagger.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 24 additions & 2 deletions coderd/coderd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,13 @@ type Options struct {
AccessControlStore *atomic.Pointer[dbauthz.AccessControlStore]
// AppSecurityKey is the crypto key used to sign and encrypt tokens related to
// workspace applications. It consists of both a signing and encryption key.
AppSecurityKey workspaceapps.SecurityKey
AppSecurityKey workspaceapps.SecurityKey

// The following two functions are dependencies of HealthcheckFunc but are only implemented
Copy link
Member

Choose a reason for hiding this comment

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

maybe move the comment to the place where you assign these no-op {}

// in enterprise. Stubbing them out here.
FetchWorkspaceProxiesFunc *atomic.Pointer[func(context.Context) (codersdk.RegionsResponse[codersdk.WorkspaceProxy], error)]
UpdateProxyHealthFunc *atomic.Pointer[func(context.Context) error]

HealthcheckFunc func(ctx context.Context, apiKey string) *healthcheck.Report
HealthcheckTimeout time.Duration
HealthcheckRefresh time.Duration
Expand Down Expand Up @@ -396,9 +402,19 @@ func New(options *Options) *API {
*options.UpdateCheckOptions,
)
}

if options.FetchWorkspaceProxiesFunc == nil {
options.FetchWorkspaceProxiesFunc = &atomic.Pointer[func(context.Context) (codersdk.RegionsResponse[codersdk.WorkspaceProxy], error)]{}
Copy link
Member

Choose a reason for hiding this comment

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

Random comment: This looks pretty gnarly (generics within generics), I kind of get why the Go team delayed generics for as long as they did. 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

To be fair, generics within generics look gnarly in any language!

}

if options.UpdateProxyHealthFunc == nil {
options.UpdateProxyHealthFunc = &atomic.Pointer[func(context.Context) error]{}
}

if options.HealthcheckFunc == nil {
options.HealthcheckFunc = func(ctx context.Context, apiKey string) *healthcheck.Report {
return healthcheck.Run(ctx, &healthcheck.ReportOptions{
authCtx := dbauthz.AsSystemRestricted(ctx) //nolint:gocritic // internal function
return healthcheck.Run(authCtx, &healthcheck.ReportOptions{
Database: healthcheck.DatabaseReportOptions{
DB: options.Database,
Threshold: options.DeploymentValues.Healthcheck.ThresholdDatabase.Value(),
Expand All @@ -413,9 +429,15 @@ func New(options *Options) *API {
DerpHealth: derphealth.ReportOptions{
DERPMap: api.DERPMap(),
},
WorkspaceProxy: healthcheck.WorkspaceProxyReportOptions{
CurrentVersion: buildinfo.Version(),
FetchWorkspaceProxies: options.FetchWorkspaceProxiesFunc.Load(),
UpdateProxyHealth: options.UpdateProxyHealthFunc.Load(),
},
})
}
}

if options.HealthcheckTimeout == 0 {
options.HealthcheckTimeout = 30 * time.Second
}
Expand Down
51 changes: 39 additions & 12 deletions coderd/healthcheck/healthcheck.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,19 @@ import (
)

const (
SectionDERP string = "DERP"
SectionAccessURL string = "AccessURL"
SectionWebsocket string = "Websocket"
SectionDatabase string = "Database"
SectionDERP string = "DERP"
SectionAccessURL string = "AccessURL"
SectionWebsocket string = "Websocket"
SectionDatabase string = "Database"
SectionWorkspaceProxy string = "WorkspaceProxy"
)

type Checker interface {
DERP(ctx context.Context, opts *derphealth.ReportOptions) derphealth.Report
AccessURL(ctx context.Context, opts *AccessURLReportOptions) AccessURLReport
Websocket(ctx context.Context, opts *WebsocketReportOptions) WebsocketReport
Database(ctx context.Context, opts *DatabaseReportOptions) DatabaseReport
WorkspaceProxy(ctx context.Context, opts *WorkspaceProxyReportOptions) WorkspaceProxyReport
}

// @typescript-generate Report
Expand All @@ -38,20 +40,22 @@ type Report struct {
// FailingSections is a list of sections that have failed their healthcheck.
FailingSections []string `json:"failing_sections"`

DERP derphealth.Report `json:"derp"`
AccessURL AccessURLReport `json:"access_url"`
Websocket WebsocketReport `json:"websocket"`
Database DatabaseReport `json:"database"`
DERP derphealth.Report `json:"derp"`
AccessURL AccessURLReport `json:"access_url"`
Websocket WebsocketReport `json:"websocket"`
Database DatabaseReport `json:"database"`
WorkspaceProxy WorkspaceProxyReport `json:"workspace_proxy"`

// The Coder version of the server that the report was generated on.
CoderVersion string `json:"coder_version"`
}

type ReportOptions struct {
AccessURL AccessURLReportOptions
Database DatabaseReportOptions
DerpHealth derphealth.ReportOptions
Websocket WebsocketReportOptions
AccessURL AccessURLReportOptions
Database DatabaseReportOptions
DerpHealth derphealth.ReportOptions
Websocket WebsocketReportOptions
WorkspaceProxy WorkspaceProxyReportOptions

Checker Checker
}
Expand All @@ -78,6 +82,11 @@ func (defaultChecker) Database(ctx context.Context, opts *DatabaseReportOptions)
return report
}

func (defaultChecker) WorkspaceProxy(ctx context.Context, opts *WorkspaceProxyReportOptions) (report WorkspaceProxyReport) {
report.Run(ctx, opts)
return report
}

func Run(ctx context.Context, opts *ReportOptions) *Report {
var (
wg sync.WaitGroup
Expand Down Expand Up @@ -136,6 +145,18 @@ func Run(ctx context.Context, opts *ReportOptions) *Report {
report.Database = opts.Checker.Database(ctx, &opts.Database)
}()

wg.Add(1)
go func() {
defer wg.Done()
defer func() {
if err := recover(); err != nil {
report.WorkspaceProxy.Error = ptr.Ref(fmt.Sprint(err))
}
}()

report.WorkspaceProxy = opts.Checker.WorkspaceProxy(ctx, &opts.WorkspaceProxy)
}()

report.CoderVersion = buildinfo.Version()
wg.Wait()

Expand All @@ -153,6 +174,9 @@ func Run(ctx context.Context, opts *ReportOptions) *Report {
if !report.Database.Healthy {
report.FailingSections = append(report.FailingSections, SectionDatabase)
}
if !report.WorkspaceProxy.Healthy {
report.FailingSections = append(report.FailingSections, SectionWorkspaceProxy)
Copy link
Member

Choose a reason for hiding this comment

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

side thought: frontend does not use FailingSections at all. Maybe we should remove it for simplicity.

Copy link
Member Author

@johnstcn johnstcn Nov 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

👍 Will file a follow-up PR

Edit: #10854

Copy link
Member

Choose a reason for hiding this comment

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

I think it's beneficial to have a structure that can be navigated in a generic way.

for (let section of report.failing_sections) {
	console.log(report[section].reason);
}

Given a good design, it will "always works", even when new fields and structures are added. Granted, I'm not a proponent for the current design, just for the ability to do so.

}

report.Healthy = len(report.FailingSections) == 0

Expand All @@ -171,6 +195,9 @@ func Run(ctx context.Context, opts *ReportOptions) *Report {
if report.Database.Severity.Value() > report.Severity.Value() {
report.Severity = report.Database.Severity
}
if report.WorkspaceProxy.Severity.Value() > report.Severity.Value() {
report.Severity = report.WorkspaceProxy.Severity
}
return &report
}

Expand Down
55 changes: 51 additions & 4 deletions coderd/healthcheck/healthcheck_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@ import (
)

type testChecker struct {
DERPReport derphealth.Report
AccessURLReport healthcheck.AccessURLReport
WebsocketReport healthcheck.WebsocketReport
DatabaseReport healthcheck.DatabaseReport
DERPReport derphealth.Report
AccessURLReport healthcheck.AccessURLReport
WebsocketReport healthcheck.WebsocketReport
DatabaseReport healthcheck.DatabaseReport
WorkspaceProxyReport healthcheck.WorkspaceProxyReport
}

func (c *testChecker) DERP(context.Context, *derphealth.ReportOptions) derphealth.Report {
Expand All @@ -33,6 +34,10 @@ func (c *testChecker) Database(context.Context, *healthcheck.DatabaseReportOptio
return c.DatabaseReport
}

func (c *testChecker) WorkspaceProxy(context.Context, *healthcheck.WorkspaceProxyReportOptions) healthcheck.WorkspaceProxyReport {
return c.WorkspaceProxyReport
}

func TestHealthcheck(t *testing.T) {
t.Parallel()

Expand All @@ -56,6 +61,9 @@ func TestHealthcheck(t *testing.T) {
DatabaseReport: healthcheck.DatabaseReport{
Healthy: true,
},
WorkspaceProxyReport: healthcheck.WorkspaceProxyReport{
Healthy: true,
},
},
healthy: true,
failingSections: []string{},
Expand All @@ -74,6 +82,9 @@ func TestHealthcheck(t *testing.T) {
DatabaseReport: healthcheck.DatabaseReport{
Healthy: true,
},
WorkspaceProxyReport: healthcheck.WorkspaceProxyReport{
Healthy: true,
},
},
healthy: false,
failingSections: []string{healthcheck.SectionDERP},
Expand All @@ -93,6 +104,9 @@ func TestHealthcheck(t *testing.T) {
DatabaseReport: healthcheck.DatabaseReport{
Healthy: true,
},
WorkspaceProxyReport: healthcheck.WorkspaceProxyReport{
Healthy: true,
},
},
healthy: true,
failingSections: []string{},
Expand All @@ -111,6 +125,9 @@ func TestHealthcheck(t *testing.T) {
DatabaseReport: healthcheck.DatabaseReport{
Healthy: true,
},
WorkspaceProxyReport: healthcheck.WorkspaceProxyReport{
Healthy: true,
},
},
healthy: false,
failingSections: []string{healthcheck.SectionAccessURL},
Expand All @@ -129,6 +146,9 @@ func TestHealthcheck(t *testing.T) {
DatabaseReport: healthcheck.DatabaseReport{
Healthy: true,
},
WorkspaceProxyReport: healthcheck.WorkspaceProxyReport{
Healthy: true,
},
},
healthy: false,
failingSections: []string{healthcheck.SectionWebsocket},
Expand All @@ -147,9 +167,33 @@ func TestHealthcheck(t *testing.T) {
DatabaseReport: healthcheck.DatabaseReport{
Healthy: false,
},
WorkspaceProxyReport: healthcheck.WorkspaceProxyReport{
Healthy: true,
},
},
healthy: false,
failingSections: []string{healthcheck.SectionDatabase},
}, {
name: "ProxyFail",
checker: &testChecker{
DERPReport: derphealth.Report{
Healthy: true,
},
AccessURLReport: healthcheck.AccessURLReport{
Healthy: true,
},
WebsocketReport: healthcheck.WebsocketReport{
Healthy: true,
},
DatabaseReport: healthcheck.DatabaseReport{
Healthy: true,
},
WorkspaceProxyReport: healthcheck.WorkspaceProxyReport{
Healthy: false,
},
},
healthy: false,
failingSections: []string{healthcheck.SectionWorkspaceProxy},
}, {
name: "AllFail",
checker: &testChecker{},
Expand All @@ -159,6 +203,7 @@ func TestHealthcheck(t *testing.T) {
healthcheck.SectionAccessURL,
healthcheck.SectionWebsocket,
healthcheck.SectionDatabase,
healthcheck.SectionWorkspaceProxy,
},
}} {
c := c
Expand All @@ -175,6 +220,8 @@ func TestHealthcheck(t *testing.T) {
assert.Equal(t, c.checker.DERPReport.Warnings, report.DERP.Warnings)
assert.Equal(t, c.checker.AccessURLReport.Healthy, report.AccessURL.Healthy)
assert.Equal(t, c.checker.WebsocketReport.Healthy, report.Websocket.Healthy)
assert.Equal(t, c.checker.WorkspaceProxyReport.Healthy, report.WorkspaceProxy.Healthy)
assert.Equal(t, c.checker.WorkspaceProxyReport.Warnings, report.WorkspaceProxy.Warnings)
assert.NotZero(t, report.Time)
assert.NotZero(t, report.CoderVersion)
})
Expand Down
Loading