Skip to content

Commit 2b773f9

Browse files
deansheatherdeansheather
authored
fix: allow proxy version mismatch (with warning) (#12433)
1 parent 4d9fe05 commit 2b773f9

File tree

12 files changed

+93
-145
lines changed

12 files changed

+93
-145
lines changed

‎coderd/apidoc/docs.go

-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/coderd.go

-1
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,6 @@ func New(options *Options) *API {
465465
DERPMap: api.DERPMap(),
466466
},
467467
WorkspaceProxy: healthcheck.WorkspaceProxyReportOptions{
468-
CurrentVersion: buildinfo.Version(),
469468
WorkspaceProxiesFetchUpdater: *(options.WorkspaceProxiesFetchUpdater).Load(),
470469
},
471470
ProvisionerDaemons: healthcheck.ProvisionerDaemonsReportDeps{

‎coderd/healthcheck/health/model.go

+6-4
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,12 @@ const (
1515
// CodeUnknown is a catch-all health code when something unexpected goes wrong (for example, a panic).
1616
CodeUnknown Code = "EUNKNOWN"
1717

18-
CodeProxyUpdate Code = "EWP01"
19-
CodeProxyFetch Code = "EWP02"
20-
CodeProxyVersionMismatch Code = "EWP03"
21-
CodeProxyUnhealthy Code = "EWP04"
18+
CodeProxyUpdate Code = "EWP01"
19+
CodeProxyFetch Code = "EWP02"
20+
// CodeProxyVersionMismatch is no longer used as it's no longer a critical
21+
// error.
22+
// CodeProxyVersionMismatch Code = "EWP03"
23+
CodeProxyUnhealthy Code = "EWP04"
2224

2325
CodeDatabasePingFailed Code = "EDB01"
2426
CodeDatabasePingSlow Code = "EDB02"

‎coderd/healthcheck/workspaceproxy.go

+16-42
Original file line numberDiff line numberDiff line change
@@ -6,23 +6,16 @@ import (
66
"sort"
77
"strings"
88

9-
"github.com/coder/coder/v2/buildinfo"
109
"github.com/coder/coder/v2/coderd/healthcheck/health"
1110
"github.com/coder/coder/v2/coderd/util/ptr"
1211
"github.com/coder/coder/v2/codersdk"
13-
14-
"golang.org/x/xerrors"
1512
)
1613

1714
type WorkspaceProxyReport codersdk.WorkspaceProxyReport
1815

1916
type WorkspaceProxyReportOptions struct {
20-
// CurrentVersion is the current server version.
21-
// We pass this in to make it easier to test.
22-
CurrentVersion string
2317
WorkspaceProxiesFetchUpdater WorkspaceProxiesFetchUpdater
24-
25-
Dismissed bool
18+
Dismissed bool
2619
}
2720

2821
type WorkspaceProxiesFetchUpdater interface {
@@ -81,22 +74,27 @@ func (r *WorkspaceProxyReport) Run(ctx context.Context, opts *WorkspaceProxyRepo
8174
return r.WorkspaceProxies.Regions[i].CreatedAt.Before(r.WorkspaceProxies.Regions[j].CreatedAt)
8275
})
8376

84-
var total, healthy int
77+
var total, healthy, warning int
8578
var errs []string
8679
for _, proxy := range r.WorkspaceProxies.Regions {
8780
total++
8881
if proxy.Healthy {
82+
// Warnings in the report are not considered unhealthy, only errors.
8983
healthy++
9084
}
85+
if len(proxy.Status.Report.Warnings) > 0 {
86+
warning++
87+
}
9188

92-
if len(proxy.Status.Report.Errors) > 0 {
93-
for _, err := range proxy.Status.Report.Errors {
94-
errs = append(errs, fmt.Sprintf("%s: %s", proxy.Name, err))
95-
}
89+
for _, err := range proxy.Status.Report.Warnings {
90+
r.Warnings = append(r.Warnings, health.Messagef(health.CodeProxyUnhealthy, "%s: %s", proxy.Name, err))
91+
}
92+
for _, err := range proxy.Status.Report.Errors {
93+
errs = append(errs, fmt.Sprintf("%s: %s", proxy.Name, err))
9694
}
9795
}
9896

99-
r.Severity = calculateSeverity(total, healthy)
97+
r.Severity = calculateSeverity(total, healthy, warning)
10098
r.Healthy = r.Severity.Value() < health.SeverityError.Value()
10199
for _, err := range errs {
102100
switch r.Severity {
@@ -106,15 +104,6 @@ func (r *WorkspaceProxyReport) Run(ctx context.Context, opts *WorkspaceProxyRepo
106104
r.appendError(*health.Errorf(health.CodeProxyUnhealthy, err))
107105
}
108106
}
109-
110-
// Versions _must_ match. Perform this check last. This will clobber any other severity.
111-
for _, proxy := range r.WorkspaceProxies.Regions {
112-
if vErr := checkVersion(proxy, opts.CurrentVersion); vErr != nil {
113-
r.Healthy = false
114-
r.Severity = health.SeverityError
115-
r.appendError(*health.Errorf(health.CodeProxyVersionMismatch, vErr.Error()))
116-
}
117-
}
118107
}
119108

120109
// appendError appends errs onto r.Error.
@@ -129,30 +118,15 @@ func (r *WorkspaceProxyReport) appendError(es ...string) {
129118
r.Error = ptr.Ref(strings.Join(es, "\n"))
130119
}
131120

132-
func checkVersion(proxy codersdk.WorkspaceProxy, currentVersion string) error {
133-
if proxy.Version == "" {
134-
return nil // may have not connected yet, this is OK
135-
}
136-
if buildinfo.VersionsMatch(proxy.Version, currentVersion) {
137-
return nil
138-
}
139-
140-
return xerrors.Errorf("proxy %q version %q does not match primary server version %q",
141-
proxy.Name,
142-
proxy.Version,
143-
currentVersion,
144-
)
145-
}
146-
147121
// calculateSeverity returns:
148122
// health.SeverityError if all proxies are unhealthy,
149-
// health.SeverityOK if all proxies are healthy,
123+
// health.SeverityOK if all proxies are healthy and there are no warnings,
150124
// health.SeverityWarning otherwise.
151-
func calculateSeverity(total, healthy int) health.Severity {
152-
if total == 0 || total == healthy {
125+
func calculateSeverity(total, healthy, warning int) health.Severity {
126+
if total == 0 || (total == healthy && warning == 0) {
153127
return health.SeverityOK
154128
}
155-
if total-healthy == total {
129+
if healthy == 0 {
156130
return health.SeverityError
157131
}
158132
return health.SeverityWarning

‎coderd/healthcheck/workspaceproxy_internal_test.go

+12-8
Original file line numberDiff line numberDiff line change
@@ -72,21 +72,25 @@ func Test_calculateSeverity(t *testing.T) {
7272
for _, tt := range []struct {
7373
total int
7474
healthy int
75+
warning int
7576
expected health.Severity
7677
}{
77-
{0, 0, health.SeverityOK},
78-
{1, 1, health.SeverityOK},
79-
{1, 0, health.SeverityError},
80-
{2, 2, health.SeverityOK},
81-
{2, 1, health.SeverityWarning},
82-
{2, 0, health.SeverityError},
78+
{0, 0, 0, health.SeverityOK},
79+
{1, 1, 0, health.SeverityOK},
80+
{1, 1, 1, health.SeverityWarning},
81+
{1, 0, 0, health.SeverityError},
82+
{2, 2, 0, health.SeverityOK},
83+
{2, 1, 0, health.SeverityWarning},
84+
{2, 1, 1, health.SeverityWarning},
85+
{2, 0, 0, health.SeverityError},
86+
{2, 0, 1, health.SeverityError},
8387
} {
8488
tt := tt
85-
name := fmt.Sprintf("%d total, %d healthy -> %s", tt.total, tt.healthy, tt.expected)
89+
name := fmt.Sprintf("%d total, %d healthy, %d warning -> %s", tt.total, tt.healthy, tt.warning, tt.expected)
8690
t.Run(name, func(t *testing.T) {
8791
t.Parallel()
8892

89-
actual := calculateSeverity(tt.total, tt.healthy)
93+
actual := calculateSeverity(tt.total, tt.healthy, tt.warning)
9094
assert.Equal(t, tt.expected, actual)
9195
})
9296
}

‎coderd/healthcheck/workspaceproxy_test.go

+45-44
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,6 @@ import (
1414
func TestWorkspaceProxies(t *testing.T) {
1515
t.Parallel()
1616

17-
var (
18-
newerPatchVersion = "v2.34.6"
19-
currentVersion = "v2.34.5"
20-
olderVersion = "v2.33.0"
21-
)
22-
2317
for _, tt := range []struct {
2418
name string
2519
fetchWorkspaceProxies func(context.Context) (codersdk.RegionsResponse[codersdk.WorkspaceProxy], error)
@@ -43,14 +37,14 @@ func TestWorkspaceProxies(t *testing.T) {
4337
},
4438
{
4539
name: "Enabled/OneHealthy",
46-
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(fakeWorkspaceProxy("alpha", true, currentVersion)),
40+
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(fakeWorkspaceProxy("alpha", true)),
4741
updateProxyHealth: fakeUpdateProxyHealth(nil),
4842
expectedHealthy: true,
4943
expectedSeverity: health.SeverityOK,
5044
},
5145
{
5246
name: "Enabled/OneUnhealthy",
53-
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(fakeWorkspaceProxy("alpha", false, currentVersion)),
47+
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(fakeWorkspaceProxy("alpha", false)),
5448
updateProxyHealth: fakeUpdateProxyHealth(nil),
5549
expectedHealthy: false,
5650
expectedSeverity: health.SeverityError,
@@ -66,7 +60,6 @@ func TestWorkspaceProxies(t *testing.T) {
6660
Name: "gone",
6761
Healthy: false,
6862
},
69-
Version: currentVersion,
7063
Status: codersdk.WorkspaceProxyStatus{
7164
Status: codersdk.ProxyUnreachable,
7265
Report: codersdk.ProxyHealthReport{
@@ -87,8 +80,8 @@ func TestWorkspaceProxies(t *testing.T) {
8780
{
8881
name: "Enabled/AllHealthy",
8982
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(
90-
fakeWorkspaceProxy("alpha", true, currentVersion),
91-
fakeWorkspaceProxy("beta", true, currentVersion),
83+
fakeWorkspaceProxy("alpha", true),
84+
fakeWorkspaceProxy("beta", true),
9285
),
9386
updateProxyHealth: func(ctx context.Context) error {
9487
return nil
@@ -99,8 +92,8 @@ func TestWorkspaceProxies(t *testing.T) {
9992
{
10093
name: "Enabled/OneHealthyOneUnhealthy",
10194
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(
102-
fakeWorkspaceProxy("alpha", false, currentVersion),
103-
fakeWorkspaceProxy("beta", true, currentVersion),
95+
fakeWorkspaceProxy("alpha", false),
96+
fakeWorkspaceProxy("beta", true),
10497
),
10598
updateProxyHealth: fakeUpdateProxyHealth(nil),
10699
expectedHealthy: true,
@@ -110,39 +103,18 @@ func TestWorkspaceProxies(t *testing.T) {
110103
{
111104
name: "Enabled/AllUnhealthy",
112105
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(
113-
fakeWorkspaceProxy("alpha", false, currentVersion),
114-
fakeWorkspaceProxy("beta", false, currentVersion),
106+
fakeWorkspaceProxy("alpha", false),
107+
fakeWorkspaceProxy("beta", false),
115108
),
116109
updateProxyHealth: fakeUpdateProxyHealth(nil),
117110
expectedHealthy: false,
118111
expectedSeverity: health.SeverityError,
119112
expectedError: string(health.CodeProxyUnhealthy),
120113
},
121-
{
122-
name: "Enabled/OneOutOfDate",
123-
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(
124-
fakeWorkspaceProxy("alpha", true, currentVersion),
125-
fakeWorkspaceProxy("beta", true, olderVersion),
126-
),
127-
updateProxyHealth: fakeUpdateProxyHealth(nil),
128-
expectedHealthy: false,
129-
expectedSeverity: health.SeverityError,
130-
expectedError: `proxy "beta" version "v2.33.0" does not match primary server version "v2.34.5"`,
131-
},
132-
{
133-
name: "Enabled/OneSlightlyNewerButStillOK",
134-
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(
135-
fakeWorkspaceProxy("alpha", true, currentVersion),
136-
fakeWorkspaceProxy("beta", true, newerPatchVersion),
137-
),
138-
updateProxyHealth: fakeUpdateProxyHealth(nil),
139-
expectedHealthy: true,
140-
expectedSeverity: health.SeverityOK,
141-
},
142114
{
143115
name: "Enabled/NotConnectedYet",
144116
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(
145-
fakeWorkspaceProxy("slowpoke", true, ""),
117+
fakeWorkspaceProxy("slowpoke", true),
146118
),
147119
updateProxyHealth: fakeUpdateProxyHealth(nil),
148120
expectedHealthy: true,
@@ -158,28 +130,56 @@ func TestWorkspaceProxies(t *testing.T) {
158130
},
159131
{
160132
name: "Enabled/ErrUpdateProxyHealth",
161-
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(fakeWorkspaceProxy("alpha", true, currentVersion)),
133+
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(fakeWorkspaceProxy("alpha", true)),
162134
updateProxyHealth: fakeUpdateProxyHealth(assert.AnError),
163135
expectedHealthy: true,
164136
expectedSeverity: health.SeverityWarning,
165137
expectedWarningCode: health.CodeProxyUpdate,
166138
},
167139
{
168140
name: "Enabled/OneUnhealthyAndDeleted",
169-
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(fakeWorkspaceProxy("alpha", false, currentVersion, func(wp *codersdk.WorkspaceProxy) {
141+
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(fakeWorkspaceProxy("alpha", false, func(wp *codersdk.WorkspaceProxy) {
170142
wp.Deleted = true
171143
})),
172144
updateProxyHealth: fakeUpdateProxyHealth(nil),
173145
expectedHealthy: true,
174146
expectedSeverity: health.SeverityOK,
175147
},
148+
{
149+
name: "Enabled/ProxyWarnings",
150+
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(
151+
fakeWorkspaceProxy("alpha", true, func(wp *codersdk.WorkspaceProxy) {
152+
wp.Status.Report.Warnings = []string{"warning"}
153+
}),
154+
fakeWorkspaceProxy("beta", false),
155+
),
156+
updateProxyHealth: fakeUpdateProxyHealth(nil),
157+
expectedHealthy: true,
158+
expectedSeverity: health.SeverityWarning,
159+
expectedWarningCode: health.CodeProxyUnhealthy,
160+
},
161+
{
162+
name: "Enabled/ProxyWarningsButAllErrored",
163+
fetchWorkspaceProxies: fakeFetchWorkspaceProxies(
164+
fakeWorkspaceProxy("alpha", false),
165+
fakeWorkspaceProxy("beta", false, func(wp *codersdk.WorkspaceProxy) {
166+
wp.Status.Report.Warnings = []string{"warning"}
167+
}),
168+
),
169+
updateProxyHealth: fakeUpdateProxyHealth(nil),
170+
expectedHealthy: false,
171+
expectedError: string(health.CodeProxyUnhealthy),
172+
expectedSeverity: health.SeverityError,
173+
},
176174
} {
177175
tt := tt
176+
if tt.name != "Enabled/ProxyWarnings" {
177+
continue
178+
}
178179
t.Run(tt.name, func(t *testing.T) {
179180
t.Parallel()
180181
var rpt healthcheck.WorkspaceProxyReport
181182
var opts healthcheck.WorkspaceProxyReportOptions
182-
opts.CurrentVersion = currentVersion
183183
if tt.fetchWorkspaceProxies != nil && tt.updateProxyHealth != nil {
184184
opts.WorkspaceProxiesFetchUpdater = &fakeWorkspaceProxyFetchUpdater{
185185
fetchFunc: tt.fetchWorkspaceProxies,
@@ -196,7 +196,9 @@ func TestWorkspaceProxies(t *testing.T) {
196196
if tt.expectedError != "" && assert.NotNil(t, rpt.Error) {
197197
assert.Contains(t, *rpt.Error, tt.expectedError)
198198
} else {
199-
assert.Nil(t, rpt.Error)
199+
if !assert.Nil(t, rpt.Error) {
200+
t.Logf("error: %v", *rpt.Error)
201+
}
200202
}
201203
if tt.expectedWarningCode != "" && assert.NotEmpty(t, rpt.Warnings) {
202204
var found bool
@@ -245,7 +247,7 @@ func (u *fakeWorkspaceProxyFetchUpdater) Update(ctx context.Context) error {
245247
}
246248

247249
//nolint:revive // yes, this is a control flag, and that is OK in a unit test.
248-
func fakeWorkspaceProxy(name string, healthy bool, version string, mutators ...func(*codersdk.WorkspaceProxy)) codersdk.WorkspaceProxy {
250+
func fakeWorkspaceProxy(name string, healthy bool, mutators ...func(*codersdk.WorkspaceProxy)) codersdk.WorkspaceProxy {
249251
var status codersdk.WorkspaceProxyStatus
250252
if !healthy {
251253
status = codersdk.WorkspaceProxyStatus{
@@ -260,8 +262,7 @@ func fakeWorkspaceProxy(name string, healthy bool, version string, mutators ...f
260262
Name: name,
261263
Healthy: healthy,
262264
},
263-
Version: version,
264-
Status: status,
265+
Status: status,
265266
}
266267
for _, f := range mutators {
267268
f(&wsp)

0 commit comments

Comments
 (0)