add severity, convert to table-driven tests

coder · johnstcn · Nov 24, 2023 · Nov 21, 2023 · Nov 23, 2023 · Nov 23, 2023
commit 89bae7effe51ceeb40f36249cf2d606bb7518ac0
diff --git a/coderd/coderd.go b/coderd/coderd.go
@@ -430,8 +430,8 @@ func New(options *Options) *API {
 				},
 				WorkspaceProxy: healthcheck.WorkspaceProxyReportOptions{
 					CurrentVersion:        buildinfo.Version(),
-					FetchWorkspaceProxies: *options.FetchWorkspaceProxiesFunc.Load(),
-					UpdateProxyHealth:     *options.UpdateProxyHealthFunc.Load(),
+					FetchWorkspaceProxies: options.FetchWorkspaceProxiesFunc.Load(),
+					UpdateProxyHealth:     options.UpdateProxyHealthFunc.Load(),
 				},
 			})
 		}

diff --git a/coderd/healthcheck/healthcheck.go b/coderd/healthcheck/healthcheck.go
@@ -195,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
 }
 

diff --git a/coderd/healthcheck/workspaceproxy.go b/coderd/healthcheck/workspaceproxy.go
@@ -2,75 +2,136 @@ package healthcheck
 
 import (
 	"context"
-	"fmt"
+
+	"golang.org/x/xerrors"
 
 	"github.com/coder/coder/v2/buildinfo"
+	"github.com/coder/coder/v2/coderd/healthcheck/health"
 	"github.com/coder/coder/v2/coderd/util/ptr"
 	"github.com/coder/coder/v2/codersdk"
+
+	"github.com/hashicorp/go-multierror"
 )
 
 type WorkspaceProxyReportOptions struct {
 	// CurrentVersion is the current server version.
 	// We pass this in to make it easier to test.
 	CurrentVersion string
 	// FetchWorkspaceProxies is a function that returns the available workspace proxies.
-	FetchWorkspaceProxies func(context.Context) (codersdk.RegionsResponse[codersdk.WorkspaceProxy], error)
+	FetchWorkspaceProxies *func(context.Context) (codersdk.RegionsResponse[codersdk.WorkspaceProxy], error)
 	// UpdateProxyHealth is a function called when healthcheck is run.
 	// This would normally be ProxyHealth.ForceUpdate().
 	// We do this because if someone mashes the healthcheck refresh button
 	// they would expect up-to-date data.
-	UpdateProxyHealth func(context.Context) error
+	UpdateProxyHealth *func(context.Context) error
 }
 
 // @typescript-generate Report
 type WorkspaceProxyReport struct {
-	Healthy  bool     `json:"healthy"`
-	Warnings []string `json:"warnings"`
-	Error    *string  `json:"error"`
+	Healthy  bool            `json:"healthy"`
+	Severity health.Severity `json:"severity"`
+	Warnings []string        `json:"warnings"`
+	Error    *string         `json:"error"`
 
 	WorkspaceProxies codersdk.RegionsResponse[codersdk.WorkspaceProxy]
 }
 
 func (r *WorkspaceProxyReport) Run(ctx context.Context, opts *WorkspaceProxyReportOptions) {
 	r.Healthy = true
+	r.Severity = health.SeverityOK
 	r.Warnings = []string{}
 
 	if opts.FetchWorkspaceProxies == nil {
 		return
 	}
+	fetchWorkspaceProxiesFunc := *opts.FetchWorkspaceProxies
 
 	if opts.UpdateProxyHealth == nil {
 		err := "opts.UpdateProxyHealth must not be nil if opts.FetchWorkspaceProxies is not nil"
 		r.Error = ptr.Ref(err)
 		return
 	}
+	updateProxyHealthFunc := *opts.UpdateProxyHealth
 
-	if err := opts.UpdateProxyHealth(ctx); err != nil {
-		r.Error = ptr.Ref(err.Error())
+	// If this fails, just mark it as a warning. It is still updated in the background.
+	if err := updateProxyHealthFunc(ctx); err != nil {
+		r.Severity = health.SeverityWarning
+		r.Warnings = append(r.Warnings, xerrors.Errorf("update proxy health: %w", err).Error())
 		return
 	}
 
-	proxies, err := opts.FetchWorkspaceProxies(ctx)
+	proxies, err := fetchWorkspaceProxiesFunc(ctx)
 	if err != nil {
 		r.Healthy = false
+		r.Severity = health.SeverityError
 		r.Error = ptr.Ref(err.Error())
 		return
 	}
 
 	r.WorkspaceProxies = proxies
 
-	var numProxies int
-	var healthyProxies int
+	var total, healthy int
 	for _, proxy := range r.WorkspaceProxies.Regions {
-		numProxies++
+		total++
 		if proxy.Healthy {
-			healthyProxies++
+			healthy++
+		}
+
+		if len(proxy.Status.Report.Errors) > 0 {
+			for _, err := range proxy.Status.Report.Errors {
+				r.appendError(xerrors.New(err))
+			}
 		}
+	}
+
+	r.Severity = calculateSeverity(total, healthy)
+	r.Healthy = r.Severity != health.SeverityError
 
-		// check versions
-		if !buildinfo.VersionsMatch(proxy.Version, opts.CurrentVersion) {
+	// Versions _must_ match. Perform this check last. This will clobber any other severity.
+	for _, proxy := range r.WorkspaceProxies.Regions {
+		if vErr := checkVersion(proxy, opts.CurrentVersion); vErr != nil {
 			r.Healthy = false
-			r.Warnings = append(r.Warnings, fmt.Sprintf("Proxy %q version %q does not match primary server version %q", proxy.Name, proxy.Version, opts.CurrentVersion))
+			r.Severity = health.SeverityError
+			r.appendError(vErr)
 		}
 	}
 }
+
+// appendError multierror-appends err onto r.Error.
+// We only have one error, so multiple errors need to be squashed in there.
+func (r *WorkspaceProxyReport) appendError(errs ...error) {
+	var prevErr error
+	if r.Error != nil {
+		prevErr = xerrors.New(*r.Error)
+	}
+	r.Error = ptr.Ref(multierror.Append(prevErr, errs...).Error())
+}
+
+func checkVersion(proxy codersdk.WorkspaceProxy, currentVersion string) error {
+	if proxy.Version == "" {
+		return nil // may have not connected yet, this is OK
+	}
+	if buildinfo.VersionsMatch(proxy.Version, currentVersion) {
+		return nil
+	}
+
+	return xerrors.Errorf("proxy %q version %q does not match primary server version %q",
+		proxy.Name,
+		proxy.Version,
+		currentVersion,
+	)
+}
+
+// calculateSeverity returns:
+// health.SeverityError if all proxies are unhealthy,
+// health.SeverityOK if all proxies are healthy,
+// health.SeverityWarning otherwise.
+func calculateSeverity(total, healthy int) health.Severity {
+	if total == 0 || total == healthy {
+		return health.SeverityOK
+	}
+	if total-healthy == total {
+		return health.SeverityError
+	}
+	return health.SeverityWarning
+}