Skip to content

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

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 4 commits into from
Mar 20, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5e01b03
fix: allow proxy version mismatch (with warning)
deansheather Mar 5, 2024
fe5a794
Improve healthcheck page
deansheather Mar 18, 2024
7c862f2
fixup! Improve healthcheck page
deansheather Mar 18, 2024
56a12ac
fixup! Improve healthcheck page
deansheather Mar 20, 2024
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
2 changes: 0 additions & 2 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.

2 changes: 0 additions & 2 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.

1 change: 0 additions & 1 deletion coderd/coderd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,6 @@ func New(options *Options) *API {
DERPMap: api.DERPMap(),
},
WorkspaceProxy: healthcheck.WorkspaceProxyReportOptions{
CurrentVersion: buildinfo.Version(),
WorkspaceProxiesFetchUpdater: *(options.WorkspaceProxiesFetchUpdater).Load(),
},
ProvisionerDaemons: healthcheck.ProvisionerDaemonsReportDeps{
Expand Down
10 changes: 6 additions & 4 deletions coderd/healthcheck/health/model.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
58 changes: 16 additions & 42 deletions coderd/healthcheck/workspaceproxy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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.
Expand All @@ -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
Expand Down
20 changes: 12 additions & 8 deletions coderd/healthcheck/workspaceproxy_internal_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
Expand Down
89 changes: 45 additions & 44 deletions coderd/healthcheck/workspaceproxy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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,
Expand All @@ -66,7 +60,6 @@ func TestWorkspaceProxies(t *testing.T) {
Name: "gone",
Healthy: false,
},
Version: currentVersion,
Status: codersdk.WorkspaceProxyStatus{
Status: codersdk.ProxyUnreachable,
Report: codersdk.ProxyHealthReport{
Expand All @@ -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
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -158,28 +130,56 @@ 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,
expectedWarningCode: health.CodeProxyUpdate,
},
{
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,
Expand All @@ -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
Expand Down Expand Up @@ -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{
Expand All @@ -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)
Expand Down
Loading