From cdd77fe7e0d1eeeea201e73431f0984d4093585e Mon Sep 17 00:00:00 2001 From: Colin Adler Date: Mon, 17 Apr 2023 14:36:43 -0500 Subject: [PATCH 1/2] feat(healthcheck): add accessurl check --- coderd/healthcheck/accessurl.go | 63 +++++++++++++++ coderd/healthcheck/accessurl_test.go | 113 +++++++++++++++++++++++++++ coderd/healthcheck/derp.go | 48 +++++------- coderd/healthcheck/derp_test.go | 9 +-- coderd/healthcheck/healthcheck.go | 47 ++++++++--- coderd/healthcheck/websocket.go | 9 +++ 6 files changed, 244 insertions(+), 45 deletions(-) create mode 100644 coderd/healthcheck/accessurl.go create mode 100644 coderd/healthcheck/accessurl_test.go diff --git a/coderd/healthcheck/accessurl.go b/coderd/healthcheck/accessurl.go new file mode 100644 index 0000000000000..fd55fc7634203 --- /dev/null +++ b/coderd/healthcheck/accessurl.go @@ -0,0 +1,63 @@ +package healthcheck + +import ( + "context" + "io" + "net/http" + "net/url" + "time" + + "golang.org/x/xerrors" +) + +type AccessURLReport struct { + Healthy bool + Reachable bool + StatusCode int + HealthzResponse string + Err error +} + +type AccessURLOptions struct { + AccessURL *url.URL + Client *http.Client +} + +func (r *AccessURLReport) Run(ctx context.Context, opts *AccessURLOptions) { + ctx, cancel := context.WithTimeout(ctx, 5*time.Second) + defer cancel() + + if opts.Client == nil { + opts.Client = http.DefaultClient + } + + accessURL, err := opts.AccessURL.Parse("/healthz") + if err != nil { + r.Err = xerrors.Errorf("parse healthz endpoint: %w", err) + return + } + + req, err := http.NewRequestWithContext(ctx, "GET", accessURL.String(), nil) + if err != nil { + r.Err = xerrors.Errorf("create healthz request: %w", err) + return + } + + res, err := opts.Client.Do(req) + if err != nil { + r.Err = xerrors.Errorf("get healthz endpoint: %w", err) + return + } + defer res.Body.Close() + + body, err := io.ReadAll(res.Body) + if err != nil { + r.Err = xerrors.Errorf("read healthz response: %w", err) + return + } + + r.Reachable = true + r.Healthy = res.StatusCode == http.StatusOK + r.StatusCode = res.StatusCode + r.HealthzResponse = string(body) +} diff --git a/coderd/healthcheck/accessurl_test.go b/coderd/healthcheck/accessurl_test.go new file mode 100644 index 0000000000000..808888771e2f4 --- /dev/null +++ b/coderd/healthcheck/accessurl_test.go @@ -0,0 +1,113 @@ +package healthcheck_test + +import ( + "context" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/xerrors" + + "github.com/coder/coder/coderd/coderdtest" + "github.com/coder/coder/coderd/healthcheck" +) + +func TestAccessURL(t *testing.T) { + t.Parallel() + + t.Run("OK", func(t *testing.T) { + t.Parallel() + + var ( + ctx, cancel = context.WithCancel(context.Background()) + report healthcheck.AccessURLReport + client = coderdtest.New(t, nil) + ) + defer cancel() + + report.Run(ctx, &healthcheck.AccessURLOptions{ + AccessURL: client.URL, + }) + + assert.True(t, report.Healthy) + assert.True(t, report.Reachable) + assert.Equal(t, http.StatusOK, report.StatusCode) + assert.Equal(t, "OK", report.HealthzResponse) + assert.NoError(t, report.Err) + }) + + t.Run("404", func(t *testing.T) { + t.Parallel() + + var ( + ctx, cancel = context.WithCancel(context.Background()) + report healthcheck.AccessURLReport + resp = []byte("NOT OK") + srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + w.Write(resp) + })) + ) + defer cancel() + defer srv.Close() + + u, err := url.Parse(srv.URL) + require.NoError(t, err) + + report.Run(ctx, &healthcheck.AccessURLOptions{ + Client: srv.Client(), + AccessURL: u, + }) + + assert.False(t, report.Healthy) + assert.True(t, report.Reachable) + assert.Equal(t, http.StatusNotFound, report.StatusCode) + assert.Equal(t, string(resp), report.HealthzResponse) + assert.NoError(t, report.Err) + }) + + t.Run("ClientErr", func(t *testing.T) { + t.Parallel() + + var ( + ctx, cancel = context.WithCancel(context.Background()) + report healthcheck.AccessURLReport + resp = []byte("OK") + srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write(resp) + })) + client = srv.Client() + ) + defer cancel() + defer srv.Close() + + expErr := xerrors.New("client error") + client.Transport = roundTripFunc(func(r *http.Request) (*http.Response, error) { + return nil, expErr + }) + + u, err := url.Parse(srv.URL) + require.NoError(t, err) + + report.Run(ctx, &healthcheck.AccessURLOptions{ + Client: client, + AccessURL: u, + }) + + assert.False(t, report.Healthy) + assert.False(t, report.Reachable) + assert.Equal(t, 0, report.StatusCode) + assert.Equal(t, "", report.HealthzResponse) + assert.ErrorIs(t, report.Err, expErr) + }) +} + +type roundTripFunc func(r *http.Request) (*http.Response, error) + +func (rt roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { + return rt(r) +} diff --git a/coderd/healthcheck/derp.go b/coderd/healthcheck/derp.go index c3b3f1991991b..3e4580aa8ea7d 100644 --- a/coderd/healthcheck/derp.go +++ b/coderd/healthcheck/derp.go @@ -30,6 +30,7 @@ type DERPReport struct { Regions map[int]*DERPRegionReport `json:"regions"` Netcheck *netcheck.Report `json:"netcheck"` + NetcheckErr error `json:"netcheck_err"` NetcheckLogs []string `json:"netcheck_logs"` } @@ -66,23 +67,22 @@ type DERPReportOptions struct { DERPMap *tailcfg.DERPMap } -func (r *DERPReport) Run(ctx context.Context, opts *DERPReportOptions) error { +func (r *DERPReport) Run(ctx context.Context, opts *DERPReportOptions) { r.Healthy = true r.Regions = map[int]*DERPRegionReport{} - eg, ctx := errgroup.WithContext(ctx) + wg := &sync.WaitGroup{} + wg.Add(len(opts.DERPMap.Regions)) for _, region := range opts.DERPMap.Regions { region := region - eg.Go(func() error { + go func() { + defer wg.Done() regionReport := DERPRegionReport{ Region: region, } - err := regionReport.Run(ctx) - if err != nil { - return xerrors.Errorf("run region report: %w", err) - } + regionReport.Run(ctx) r.mu.Lock() r.Regions[region.RegionID] = ®ionReport @@ -90,8 +90,7 @@ func (r *DERPReport) Run(ctx context.Context, opts *DERPReportOptions) error { r.Healthy = false } r.mu.Unlock() - return nil - }) + }() } ncLogf := func(format string, args ...interface{}) { @@ -103,32 +102,29 @@ func (r *DERPReport) Run(ctx context.Context, opts *DERPReportOptions) error { PortMapper: portmapper.NewClient(tslogger.WithPrefix(ncLogf, "portmap: "), nil), Logf: tslogger.WithPrefix(ncLogf, "netcheck: "), } - ncReport, err := nc.GetReport(ctx, opts.DERPMap) - if err != nil { - return xerrors.Errorf("run netcheck: %w", err) - } - r.Netcheck = ncReport + r.Netcheck, r.NetcheckErr = nc.GetReport(ctx, opts.DERPMap) - return eg.Wait() + wg.Wait() } -func (r *DERPRegionReport) Run(ctx context.Context) error { +func (r *DERPRegionReport) Run(ctx context.Context) { r.Healthy = true r.NodeReports = []*DERPNodeReport{} - eg, ctx := errgroup.WithContext(ctx) + wg := &sync.WaitGroup{} + + wg.Add(len(r.Region.Nodes)) for _, node := range r.Region.Nodes { node := node - eg.Go(func() error { + go func() { + defer wg.Done() + nodeReport := DERPNodeReport{ Node: node, Healthy: true, } - err := nodeReport.Run(ctx) - if err != nil { - return xerrors.Errorf("run node report: %w", err) - } + nodeReport.Run(ctx) r.mu.Lock() r.NodeReports = append(r.NodeReports, &nodeReport) @@ -136,11 +132,10 @@ func (r *DERPRegionReport) Run(ctx context.Context) error { r.Healthy = false } r.mu.Unlock() - return nil - }) + }() } - return eg.Wait() + wg.Wait() } func (r *DERPNodeReport) derpURL() *url.URL { @@ -159,7 +154,7 @@ func (r *DERPNodeReport) derpURL() *url.URL { return derpURL } -func (r *DERPNodeReport) Run(ctx context.Context) error { +func (r *DERPNodeReport) Run(ctx context.Context) { ctx, cancel := context.WithTimeout(ctx, 10*time.Second) defer cancel() @@ -179,7 +174,6 @@ func (r *DERPNodeReport) Run(ctx context.Context) error { r.STUN.Error != nil { r.Healthy = false } - return nil } func (r *DERPNodeReport) doExchangeMessage(ctx context.Context) { diff --git a/coderd/healthcheck/derp_test.go b/coderd/healthcheck/derp_test.go index 83579ee36b683..e48da8160028c 100644 --- a/coderd/healthcheck/derp_test.go +++ b/coderd/healthcheck/derp_test.go @@ -58,8 +58,7 @@ func TestDERP(t *testing.T) { } ) - err := report.Run(ctx, opts) - require.NoError(t, err) + report.Run(ctx, opts) assert.True(t, report.Healthy) for _, region := range report.Regions { @@ -100,8 +99,7 @@ func TestDERP(t *testing.T) { // Only include the Dallas region opts.DERPMap.Regions = map[int]*tailcfg.DERPRegion{9: opts.DERPMap.Regions[9]} - err := report.Run(ctx, opts) - require.NoError(t, err) + report.Run(ctx, opts) assert.True(t, report.Healthy) for _, region := range report.Regions { @@ -215,8 +213,7 @@ func TestDERP(t *testing.T) { } ) - err := report.Run(ctx, opts) - require.NoError(t, err) + report.Run(ctx, opts) assert.True(t, report.Healthy) for _, region := range report.Regions { diff --git a/coderd/healthcheck/healthcheck.go b/coderd/healthcheck/healthcheck.go index e48ac4fa72712..26dedaa5a97a8 100644 --- a/coderd/healthcheck/healthcheck.go +++ b/coderd/healthcheck/healthcheck.go @@ -2,9 +2,11 @@ package healthcheck import ( "context" + "net/http" + "net/url" + "sync" "time" - "golang.org/x/xerrors" "tailscale.com/tailcfg" ) @@ -14,28 +16,49 @@ type Report struct { // Healthy is true if the report returns no errors. Healthy bool `json:"pass"` - DERP DERPReport `json:"derp"` + DERP DERPReport `json:"derp"` + AccessURL AccessURLReport `json:"access_url"` - // TODO - // AccessURL AccessURLReport - // Websocket WebsocketReport + // TODO: + // Websocket WebsocketReport `json:"websocket"` } type ReportOptions struct { // TODO: support getting this over HTTP? - DERPMap *tailcfg.DERPMap + DERPMap *tailcfg.DERPMap + AccessURL *url.URL + Client *http.Client } func Run(ctx context.Context, opts *ReportOptions) (*Report, error) { var report Report - err := report.DERP.Run(ctx, &DERPReportOptions{ - DERPMap: opts.DERPMap, - }) - if err != nil { - return nil, xerrors.Errorf("run derp: %w", err) - } + wg := &sync.WaitGroup{} + wg.Add(1) + go func() { + defer wg.Done() + report.DERP.Run(ctx, &DERPReportOptions{ + DERPMap: opts.DERPMap, + }) + }() + + wg.Add(1) + go func() { + defer wg.Done() + report.AccessURL.Run(ctx, &AccessURLOptions{ + AccessURL: opts.AccessURL, + Client: opts.Client, + }) + }() + + // wg.Add(1) + // go func() { + // defer wg.Done() + // report.Websocket.Run(ctx, opts.AccessURL) + // }() + + wg.Wait() report.Time = time.Now() report.Healthy = report.DERP.Healthy return &report, nil diff --git a/coderd/healthcheck/websocket.go b/coderd/healthcheck/websocket.go index 628a2723982fe..a8063f3f6a9ee 100644 --- a/coderd/healthcheck/websocket.go +++ b/coderd/healthcheck/websocket.go @@ -1,3 +1,12 @@ package healthcheck +import ( + "context" + "net/url" +) + type WebsocketReport struct{} + +func (*WebsocketReport) Run(ctx context.Context, accessURL *url.URL) { + _, _ = ctx, accessURL +} From 21995078ecde36a171f79f3a8f095635378448db Mon Sep 17 00:00:00 2001 From: Colin Adler Date: Tue, 18 Apr 2023 14:24:04 -0500 Subject: [PATCH 2/2] gen --- coderd/apidoc/docs.go | 22 +++++++++++++++++++ coderd/apidoc/swagger.json | 22 +++++++++++++++++++ docs/api/debug.md | 8 +++++++ docs/api/schemas.md | 43 +++++++++++++++++++++++++++++++++----- 4 files changed, 90 insertions(+), 5 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 135cdf1f7e9b6..c4b0c00ea7cc5 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -9749,6 +9749,24 @@ const docTemplate = `{ "ParameterSourceSchemeData" ] }, + "healthcheck.AccessURLReport": { + "type": "object", + "properties": { + "err": {}, + "healthy": { + "type": "boolean" + }, + "healthzResponse": { + "type": "string" + }, + "reachable": { + "type": "boolean" + }, + "statusCode": { + "type": "integer" + } + } + }, "healthcheck.DERPNodeReport": { "type": "object", "properties": { @@ -9814,6 +9832,7 @@ const docTemplate = `{ "netcheck": { "$ref": "#/definitions/netcheck.Report" }, + "netcheck_err": {}, "netcheck_logs": { "type": "array", "items": { @@ -9843,6 +9862,9 @@ const docTemplate = `{ "healthcheck.Report": { "type": "object", "properties": { + "access_url": { + "$ref": "#/definitions/healthcheck.AccessURLReport" + }, "derp": { "$ref": "#/definitions/healthcheck.DERPReport" }, diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 31acf01b313b3..3e6d3097d6703 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -8812,6 +8812,24 @@ "ParameterSourceSchemeData" ] }, + "healthcheck.AccessURLReport": { + "type": "object", + "properties": { + "err": {}, + "healthy": { + "type": "boolean" + }, + "healthzResponse": { + "type": "string" + }, + "reachable": { + "type": "boolean" + }, + "statusCode": { + "type": "integer" + } + } + }, "healthcheck.DERPNodeReport": { "type": "object", "properties": { @@ -8877,6 +8895,7 @@ "netcheck": { "$ref": "#/definitions/netcheck.Report" }, + "netcheck_err": {}, "netcheck_logs": { "type": "array", "items": { @@ -8906,6 +8925,9 @@ "healthcheck.Report": { "type": "object", "properties": { + "access_url": { + "$ref": "#/definitions/healthcheck.AccessURLReport" + }, "derp": { "$ref": "#/definitions/healthcheck.DERPReport" }, diff --git a/docs/api/debug.md b/docs/api/debug.md index 88946156d88d3..5ac870e67a7fc 100644 --- a/docs/api/debug.md +++ b/docs/api/debug.md @@ -39,6 +39,13 @@ curl -X GET http://coder-server:8080/api/v2/debug/health \ ```json { + "access_url": { + "err": null, + "healthy": true, + "healthzResponse": "string", + "reachable": true, + "statusCode": 0 + }, "derp": { "healthy": true, "netcheck": { @@ -71,6 +78,7 @@ curl -X GET http://coder-server:8080/api/v2/debug/health \ "udp": true, "upnP": "string" }, + "netcheck_err": null, "netcheck_logs": ["string"], "regions": { "property1": { diff --git a/docs/api/schemas.md b/docs/api/schemas.md index be0010ec439f9..b6cb93e911cbf 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -5563,6 +5563,28 @@ Parameter represents a set value for the scope. | `none` | | `data` | +## healthcheck.AccessURLReport + +```json +{ + "err": null, + "healthy": true, + "healthzResponse": "string", + "reachable": true, + "statusCode": 0 +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +| ----------------- | ------- | -------- | ------------ | ----------- | +| `err` | any | false | | | +| `healthy` | boolean | false | | | +| `healthzResponse` | string | false | | | +| `reachable` | boolean | false | | | +| `statusCode` | integer | false | | | + ## healthcheck.DERPNodeReport ```json @@ -5711,6 +5733,7 @@ Parameter represents a set value for the scope. "udp": true, "upnP": "string" }, + "netcheck_err": null, "netcheck_logs": ["string"], "regions": { "property1": { @@ -5833,6 +5856,7 @@ Parameter represents a set value for the scope. | ------------------ | ------------------------------------------------------------ | -------- | ------------ | ----------- | | `healthy` | boolean | false | | | | `netcheck` | [netcheck.Report](#netcheckreport) | false | | | +| `netcheck_err` | any | false | | | | `netcheck_logs` | array of string | false | | | | `regions` | object | false | | | | ยป `[any property]` | [healthcheck.DERPRegionReport](#healthcheckderpregionreport) | false | | | @@ -5859,6 +5883,13 @@ Parameter represents a set value for the scope. ```json { + "access_url": { + "err": null, + "healthy": true, + "healthzResponse": "string", + "reachable": true, + "statusCode": 0 + }, "derp": { "healthy": true, "netcheck": { @@ -5891,6 +5922,7 @@ Parameter represents a set value for the scope. "udp": true, "upnP": "string" }, + "netcheck_err": null, "netcheck_logs": ["string"], "regions": { "property1": { @@ -6012,11 +6044,12 @@ Parameter represents a set value for the scope. ### Properties -| Name | Type | Required | Restrictions | Description | -| ------ | ------------------------------------------------ | -------- | ------------ | ------------------------------------------------ | -| `derp` | [healthcheck.DERPReport](#healthcheckderpreport) | false | | | -| `pass` | boolean | false | | Healthy is true if the report returns no errors. | -| `time` | string | false | | Time is the time the report was generated at. | +| Name | Type | Required | Restrictions | Description | +| ------------ | ---------------------------------------------------------- | -------- | ------------ | ------------------------------------------------ | +| `access_url` | [healthcheck.AccessURLReport](#healthcheckaccessurlreport) | false | | | +| `derp` | [healthcheck.DERPReport](#healthcheckderpreport) | false | | | +| `pass` | boolean | false | | Healthy is true if the report returns no errors. | +| `time` | string | false | | Time is the time the report was generated at. | ## netcheck.Report