fix(coderd/debug): fix caching issue with dismissed sections (#11051)

This commit is contained in:
Cian Johnston 2023-12-06 08:38:03 +00:00 committed by GitHub
parent 53453c06a1
commit 38ed816207
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 64 additions and 12 deletions

View File

@ -25,7 +25,6 @@ import (
"github.com/prometheus/client_golang/prometheus"
httpSwagger "github.com/swaggo/http-swagger/v2"
"go.opentelemetry.io/otel/trace"
"golang.org/x/exp/slices"
"golang.org/x/xerrors"
"google.golang.org/api/idtoken"
"storj.io/drpc/drpcmux"
@ -408,30 +407,26 @@ func New(options *Options) *API {
if options.HealthcheckFunc == nil {
options.HealthcheckFunc = func(ctx context.Context, apiKey string) *healthcheck.Report {
dismissedHealthchecks := loadDismissedHealthchecks(ctx, options.Database, options.Logger)
// NOTE: dismissed healthchecks are marked in formatHealthcheck.
// Not here, as this result gets cached.
return healthcheck.Run(ctx, &healthcheck.ReportOptions{
Database: healthcheck.DatabaseReportOptions{
DB: options.Database,
Threshold: options.DeploymentValues.Healthcheck.ThresholdDatabase.Value(),
Dismissed: slices.Contains(dismissedHealthchecks, codersdk.HealthSectionDatabase),
},
Websocket: healthcheck.WebsocketReportOptions{
AccessURL: options.AccessURL,
APIKey: apiKey,
Dismissed: slices.Contains(dismissedHealthchecks, codersdk.HealthSectionWebsocket),
},
AccessURL: healthcheck.AccessURLReportOptions{
AccessURL: options.AccessURL,
Dismissed: slices.Contains(dismissedHealthchecks, codersdk.HealthSectionAccessURL),
},
DerpHealth: derphealth.ReportOptions{
DERPMap: api.DERPMap(),
Dismissed: slices.Contains(dismissedHealthchecks, codersdk.HealthSectionDERP),
DERPMap: api.DERPMap(),
},
WorkspaceProxy: healthcheck.WorkspaceProxyReportOptions{
CurrentVersion: buildinfo.Version(),
WorkspaceProxiesFetchUpdater: *(options.WorkspaceProxiesFetchUpdater).Load(),
Dismissed: slices.Contains(dismissedHealthchecks, codersdk.HealthSectionWorkspaceProxy),
},
})
}

View File

@ -59,6 +59,11 @@ func (api *API) debugDeploymentHealth(rw http.ResponseWriter, r *http.Request) {
ctx, cancel := context.WithTimeout(r.Context(), api.Options.HealthcheckTimeout)
defer cancel()
// Load sections previously marked as dismissed.
// We hydrate this here as we cache the healthcheck and hydrating in the
// healthcheck function itself can lead to stale results.
dismissed := loadDismissedHealthchecks(ctx, api.Database, api.Logger)
// Check if the forced query parameter is set.
forced := r.URL.Query().Get("force") == "true"
@ -66,7 +71,7 @@ func (api *API) debugDeploymentHealth(rw http.ResponseWriter, r *http.Request) {
if !forced {
if report := api.healthCheckCache.Load(); report != nil {
if time.Since(report.Time) < api.Options.HealthcheckRefresh {
formatHealthcheck(ctx, rw, r, report)
formatHealthcheck(ctx, rw, r, *report, dismissed...)
return
}
}
@ -89,12 +94,36 @@ func (api *API) debugDeploymentHealth(rw http.ResponseWriter, r *http.Request) {
})
return
case res := <-resChan:
formatHealthcheck(ctx, rw, r, res.Val)
report := res.Val
if report == nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "There was an unknown error completing the healthcheck.",
Detail: "nil report from healthcheck result channel",
})
return
}
formatHealthcheck(ctx, rw, r, *report, dismissed...)
return
}
}
func formatHealthcheck(ctx context.Context, rw http.ResponseWriter, r *http.Request, hc *healthcheck.Report) {
func formatHealthcheck(ctx context.Context, rw http.ResponseWriter, r *http.Request, hc healthcheck.Report, dismissed ...codersdk.HealthSection) {
// Mark any sections previously marked as dismissed.
for _, d := range dismissed {
switch d {
case codersdk.HealthSectionAccessURL:
hc.AccessURL.Dismissed = true
case codersdk.HealthSectionDERP:
hc.DERP.Dismissed = true
case codersdk.HealthSectionDatabase:
hc.Database.Dismissed = true
case codersdk.HealthSectionWebsocket:
hc.Websocket.Dismissed = true
case codersdk.HealthSectionWorkspaceProxy:
hc.WorkspaceProxy.Dismissed = true
}
}
format := r.URL.Query().Get("format")
switch format {
case "text":
@ -269,7 +298,7 @@ func loadDismissedHealthchecks(ctx context.Context, db database.Store, logger sl
}
}
if err != nil && !xerrors.Is(err, sql.ErrNoRows) {
logger.Error(ctx, "unable to fetch health settings: %w", err)
logger.Error(ctx, "unable to fetch health settings", slog.Error(err))
}
return dismissedHealthchecks
}

View File

@ -2,6 +2,7 @@ package coderd_test
import (
"context"
"encoding/json"
"io"
"net/http"
"sync/atomic"
@ -11,6 +12,8 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"cdr.dev/slog/sloggers/slogtest"
"github.com/coder/coder/v2/coderd/coderdtest"
"github.com/coder/coder/v2/coderd/healthcheck"
"github.com/coder/coder/v2/coderd/healthcheck/derphealth"
@ -90,8 +93,11 @@ func TestDebugHealth(t *testing.T) {
t.Parallel()
var (
// Need to ignore errors due to ctx timeout
logger = slogtest.Make(t, &slogtest.Options{IgnoreErrors: true})
ctx, cancel = context.WithTimeout(context.Background(), testutil.WaitShort)
client = coderdtest.New(t, &coderdtest.Options{
Logger: &logger,
HealthcheckTimeout: time.Microsecond,
HealthcheckFunc: func(context.Context, string) *healthcheck.Report {
t := time.NewTimer(time.Second)
@ -276,6 +282,17 @@ func TestHealthSettings(t *testing.T) {
settings, err := adminClient.HealthSettings(ctx)
require.NoError(t, err)
require.Equal(t, expected, settings)
// then
res, err := adminClient.Request(ctx, "GET", "/api/v2/debug/health", nil)
require.NoError(t, err)
bs, err := io.ReadAll(res.Body)
require.NoError(t, err)
defer res.Body.Close()
var hc healthcheck.Report
require.NoError(t, json.Unmarshal(bs, &hc))
require.True(t, hc.DERP.Dismissed)
require.True(t, hc.Websocket.Dismissed)
})
t.Run("UnDismissSection", func(t *testing.T) {
@ -307,6 +324,17 @@ func TestHealthSettings(t *testing.T) {
settings, err := adminClient.HealthSettings(ctx)
require.NoError(t, err)
require.Equal(t, expected, settings)
// then
res, err := adminClient.Request(ctx, "GET", "/api/v2/debug/health", nil)
require.NoError(t, err)
bs, err := io.ReadAll(res.Body)
require.NoError(t, err)
defer res.Body.Close()
var hc healthcheck.Report
require.NoError(t, json.Unmarshal(bs, &hc))
require.True(t, hc.DERP.Dismissed)
require.False(t, hc.Websocket.Dismissed)
})
t.Run("NotModified", func(t *testing.T) {