fix: allow proxy version mismatch (with warning) (#12433)

This commit is contained in:
Dean Sheather
2024-03-20 11:24:18 -07:00
committed by GitHub
parent 4d9fe05f5a
commit 2b773f9034
12 changed files with 93 additions and 145 deletions

2
coderd/apidoc/docs.go generated
View File

@ -13888,7 +13888,6 @@ const docTemplate = `{
"EUNKNOWN",
"EWP01",
"EWP02",
"EWP03",
"EWP04",
"EDB01",
"EDB02",
@ -13909,7 +13908,6 @@ const docTemplate = `{
"CodeUnknown",
"CodeProxyUpdate",
"CodeProxyFetch",
"CodeProxyVersionMismatch",
"CodeProxyUnhealthy",
"CodeDatabasePingFailed",
"CodeDatabasePingSlow",

View File

@ -12632,7 +12632,6 @@
"EUNKNOWN",
"EWP01",
"EWP02",
"EWP03",
"EWP04",
"EDB01",
"EDB02",
@ -12653,7 +12652,6 @@
"CodeUnknown",
"CodeProxyUpdate",
"CodeProxyFetch",
"CodeProxyVersionMismatch",
"CodeProxyUnhealthy",
"CodeDatabasePingFailed",
"CodeDatabasePingSlow",

View File

@ -465,7 +465,6 @@ func New(options *Options) *API {
DERPMap: api.DERPMap(),
},
WorkspaceProxy: healthcheck.WorkspaceProxyReportOptions{
CurrentVersion: buildinfo.Version(),
WorkspaceProxiesFetchUpdater: *(options.WorkspaceProxiesFetchUpdater).Load(),
},
ProvisionerDaemons: healthcheck.ProvisionerDaemonsReportDeps{

View File

@ -15,10 +15,12 @@ const (
// 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"
CodeProxyUpdate Code = "EWP01"
CodeProxyFetch Code = "EWP02"
// CodeProxyVersionMismatch is no longer used as it's no longer a critical
// error.
// CodeProxyVersionMismatch Code = "EWP03"
CodeProxyUnhealthy Code = "EWP04"
CodeDatabasePingFailed Code = "EDB01"
CodeDatabasePingSlow Code = "EDB02"

View File

@ -6,23 +6,16 @@ import (
"sort"
"strings"
"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"
"golang.org/x/xerrors"
)
type WorkspaceProxyReport codersdk.WorkspaceProxyReport
type WorkspaceProxyReportOptions struct {
// CurrentVersion is the current server version.
// We pass this in to make it easier to test.
CurrentVersion string
WorkspaceProxiesFetchUpdater WorkspaceProxiesFetchUpdater
Dismissed bool
Dismissed bool
}
type WorkspaceProxiesFetchUpdater interface {
@ -81,22 +74,27 @@ func (r *WorkspaceProxyReport) Run(ctx context.Context, opts *WorkspaceProxyRepo
return r.WorkspaceProxies.Regions[i].CreatedAt.Before(r.WorkspaceProxies.Regions[j].CreatedAt)
})
var total, healthy int
var total, healthy, warning int
var errs []string
for _, proxy := range r.WorkspaceProxies.Regions {
total++
if proxy.Healthy {
// Warnings in the report are not considered unhealthy, only errors.
healthy++
}
if len(proxy.Status.Report.Warnings) > 0 {
warning++
}
if len(proxy.Status.Report.Errors) > 0 {
for _, err := range proxy.Status.Report.Errors {
errs = append(errs, fmt.Sprintf("%s: %s", proxy.Name, err))
}
for _, err := range proxy.Status.Report.Warnings {
r.Warnings = append(r.Warnings, health.Messagef(health.CodeProxyUnhealthy, "%s: %s", proxy.Name, err))
}
for _, err := range proxy.Status.Report.Errors {
errs = append(errs, fmt.Sprintf("%s: %s", proxy.Name, err))
}
}
r.Severity = calculateSeverity(total, healthy)
r.Severity = calculateSeverity(total, healthy, warning)
r.Healthy = r.Severity.Value() < health.SeverityError.Value()
for _, err := range errs {
switch r.Severity {
@ -106,15 +104,6 @@ func (r *WorkspaceProxyReport) Run(ctx context.Context, opts *WorkspaceProxyRepo
r.appendError(*health.Errorf(health.CodeProxyUnhealthy, err))
}
}
// 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.Severity = health.SeverityError
r.appendError(*health.Errorf(health.CodeProxyVersionMismatch, vErr.Error()))
}
}
}
// appendError appends errs onto r.Error.
@ -129,30 +118,15 @@ func (r *WorkspaceProxyReport) appendError(es ...string) {
r.Error = ptr.Ref(strings.Join(es, "\n"))
}
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.SeverityOK if all proxies are healthy and there are no warnings,
// health.SeverityWarning otherwise.
func calculateSeverity(total, healthy int) health.Severity {
if total == 0 || total == healthy {
func calculateSeverity(total, healthy, warning int) health.Severity {
if total == 0 || (total == healthy && warning == 0) {
return health.SeverityOK
}
if total-healthy == total {
if healthy == 0 {
return health.SeverityError
}
return health.SeverityWarning

View File

@ -72,21 +72,25 @@ func Test_calculateSeverity(t *testing.T) {
for _, tt := range []struct {
total int
healthy int
warning int
expected health.Severity
}{
{0, 0, health.SeverityOK},
{1, 1, health.SeverityOK},
{1, 0, health.SeverityError},
{2, 2, health.SeverityOK},
{2, 1, health.SeverityWarning},
{2, 0, health.SeverityError},
{0, 0, 0, health.SeverityOK},
{1, 1, 0, health.SeverityOK},
{1, 1, 1, health.SeverityWarning},
{1, 0, 0, health.SeverityError},
{2, 2, 0, health.SeverityOK},
{2, 1, 0, health.SeverityWarning},
{2, 1, 1, health.SeverityWarning},
{2, 0, 0, health.SeverityError},
{2, 0, 1, health.SeverityError},
} {
tt := tt
name := fmt.Sprintf("%d total, %d healthy -> %s", tt.total, tt.healthy, tt.expected)
name := fmt.Sprintf("%d total, %d healthy, %d warning -> %s", tt.total, tt.healthy, tt.warning, tt.expected)
t.Run(name, func(t *testing.T) {
t.Parallel()
actual := calculateSeverity(tt.total, tt.healthy)
actual := calculateSeverity(tt.total, tt.healthy, tt.warning)
assert.Equal(t, tt.expected, actual)
})
}

View File

@ -14,12 +14,6 @@ import (
func TestWorkspaceProxies(t *testing.T) {
t.Parallel()
var (
newerPatchVersion = "v2.34.6"
currentVersion = "v2.34.5"
olderVersion = "v2.33.0"
)
for _, tt := range []struct {
name string
fetchWorkspaceProxies func(context.Context) (codersdk.RegionsResponse[codersdk.WorkspaceProxy], error)
@ -43,14 +37,14 @@ func TestWorkspaceProxies(t *testing.T) {
},
{
name: "Enabled/OneHealthy",
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(fakeWorkspaceProxy("alpha", true, currentVersion)),
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(fakeWorkspaceProxy("alpha", true)),
updateProxyHealth: fakeUpdateProxyHealth(nil),
expectedHealthy: true,
expectedSeverity: health.SeverityOK,
},
{
name: "Enabled/OneUnhealthy",
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(fakeWorkspaceProxy("alpha", false, currentVersion)),
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(fakeWorkspaceProxy("alpha", false)),
updateProxyHealth: fakeUpdateProxyHealth(nil),
expectedHealthy: false,
expectedSeverity: health.SeverityError,
@ -66,7 +60,6 @@ func TestWorkspaceProxies(t *testing.T) {
Name: "gone",
Healthy: false,
},
Version: currentVersion,
Status: codersdk.WorkspaceProxyStatus{
Status: codersdk.ProxyUnreachable,
Report: codersdk.ProxyHealthReport{
@ -87,8 +80,8 @@ func TestWorkspaceProxies(t *testing.T) {
{
name: "Enabled/AllHealthy",
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(
fakeWorkspaceProxy("alpha", true, currentVersion),
fakeWorkspaceProxy("beta", true, currentVersion),
fakeWorkspaceProxy("alpha", true),
fakeWorkspaceProxy("beta", true),
),
updateProxyHealth: func(ctx context.Context) error {
return nil
@ -99,8 +92,8 @@ func TestWorkspaceProxies(t *testing.T) {
{
name: "Enabled/OneHealthyOneUnhealthy",
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(
fakeWorkspaceProxy("alpha", false, currentVersion),
fakeWorkspaceProxy("beta", true, currentVersion),
fakeWorkspaceProxy("alpha", false),
fakeWorkspaceProxy("beta", true),
),
updateProxyHealth: fakeUpdateProxyHealth(nil),
expectedHealthy: true,
@ -110,39 +103,18 @@ func TestWorkspaceProxies(t *testing.T) {
{
name: "Enabled/AllUnhealthy",
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(
fakeWorkspaceProxy("alpha", false, currentVersion),
fakeWorkspaceProxy("beta", false, currentVersion),
fakeWorkspaceProxy("alpha", false),
fakeWorkspaceProxy("beta", false),
),
updateProxyHealth: fakeUpdateProxyHealth(nil),
expectedHealthy: false,
expectedSeverity: health.SeverityError,
expectedError: string(health.CodeProxyUnhealthy),
},
{
name: "Enabled/OneOutOfDate",
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(
fakeWorkspaceProxy("alpha", true, currentVersion),
fakeWorkspaceProxy("beta", true, olderVersion),
),
updateProxyHealth: fakeUpdateProxyHealth(nil),
expectedHealthy: false,
expectedSeverity: health.SeverityError,
expectedError: `proxy "beta" version "v2.33.0" does not match primary server version "v2.34.5"`,
},
{
name: "Enabled/OneSlightlyNewerButStillOK",
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(
fakeWorkspaceProxy("alpha", true, currentVersion),
fakeWorkspaceProxy("beta", true, newerPatchVersion),
),
updateProxyHealth: fakeUpdateProxyHealth(nil),
expectedHealthy: true,
expectedSeverity: health.SeverityOK,
},
{
name: "Enabled/NotConnectedYet",
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(
fakeWorkspaceProxy("slowpoke", true, ""),
fakeWorkspaceProxy("slowpoke", true),
),
updateProxyHealth: fakeUpdateProxyHealth(nil),
expectedHealthy: true,
@ -158,7 +130,7 @@ func TestWorkspaceProxies(t *testing.T) {
},
{
name: "Enabled/ErrUpdateProxyHealth",
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(fakeWorkspaceProxy("alpha", true, currentVersion)),
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(fakeWorkspaceProxy("alpha", true)),
updateProxyHealth: fakeUpdateProxyHealth(assert.AnError),
expectedHealthy: true,
expectedSeverity: health.SeverityWarning,
@ -166,20 +138,48 @@ func TestWorkspaceProxies(t *testing.T) {
},
{
name: "Enabled/OneUnhealthyAndDeleted",
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(fakeWorkspaceProxy("alpha", false, currentVersion, func(wp *codersdk.WorkspaceProxy) {
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(fakeWorkspaceProxy("alpha", false, func(wp *codersdk.WorkspaceProxy) {
wp.Deleted = true
})),
updateProxyHealth: fakeUpdateProxyHealth(nil),
expectedHealthy: true,
expectedSeverity: health.SeverityOK,
},
{
name: "Enabled/ProxyWarnings",
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(
fakeWorkspaceProxy("alpha", true, func(wp *codersdk.WorkspaceProxy) {
wp.Status.Report.Warnings = []string{"warning"}
}),
fakeWorkspaceProxy("beta", false),
),
updateProxyHealth: fakeUpdateProxyHealth(nil),
expectedHealthy: true,
expectedSeverity: health.SeverityWarning,
expectedWarningCode: health.CodeProxyUnhealthy,
},
{
name: "Enabled/ProxyWarningsButAllErrored",
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(
fakeWorkspaceProxy("alpha", false),
fakeWorkspaceProxy("beta", false, func(wp *codersdk.WorkspaceProxy) {
wp.Status.Report.Warnings = []string{"warning"}
}),
),
updateProxyHealth: fakeUpdateProxyHealth(nil),
expectedHealthy: false,
expectedError: string(health.CodeProxyUnhealthy),
expectedSeverity: health.SeverityError,
},
} {
tt := tt
if tt.name != "Enabled/ProxyWarnings" {
continue
}
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
var rpt healthcheck.WorkspaceProxyReport
var opts healthcheck.WorkspaceProxyReportOptions
opts.CurrentVersion = currentVersion
if tt.fetchWorkspaceProxies != nil && tt.updateProxyHealth != nil {
opts.WorkspaceProxiesFetchUpdater = &fakeWorkspaceProxyFetchUpdater{
fetchFunc: tt.fetchWorkspaceProxies,
@ -196,7 +196,9 @@ func TestWorkspaceProxies(t *testing.T) {
if tt.expectedError != "" && assert.NotNil(t, rpt.Error) {
assert.Contains(t, *rpt.Error, tt.expectedError)
} else {
assert.Nil(t, rpt.Error)
if !assert.Nil(t, rpt.Error) {
t.Logf("error: %v", *rpt.Error)
}
}
if tt.expectedWarningCode != "" && assert.NotEmpty(t, rpt.Warnings) {
var found bool
@ -245,7 +247,7 @@ func (u *fakeWorkspaceProxyFetchUpdater) Update(ctx context.Context) error {
}
//nolint:revive // yes, this is a control flag, and that is OK in a unit test.
func fakeWorkspaceProxy(name string, healthy bool, version string, mutators ...func(*codersdk.WorkspaceProxy)) codersdk.WorkspaceProxy {
func fakeWorkspaceProxy(name string, healthy bool, mutators ...func(*codersdk.WorkspaceProxy)) codersdk.WorkspaceProxy {
var status codersdk.WorkspaceProxyStatus
if !healthy {
status = codersdk.WorkspaceProxyStatus{
@ -260,8 +262,7 @@ func fakeWorkspaceProxy(name string, healthy bool, version string, mutators ...f
Name: name,
Healthy: healthy,
},
Version: version,
Status: status,
Status: status,
}
for _, f := range mutators {
f(&wsp)