diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 2e8e5c5d74..c6c8be1d58 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -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", diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 8d6e7ccd6b..605d9f8c76 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -12632,7 +12632,6 @@ "EUNKNOWN", "EWP01", "EWP02", - "EWP03", "EWP04", "EDB01", "EDB02", @@ -12653,7 +12652,6 @@ "CodeUnknown", "CodeProxyUpdate", "CodeProxyFetch", - "CodeProxyVersionMismatch", "CodeProxyUnhealthy", "CodeDatabasePingFailed", "CodeDatabasePingSlow", diff --git a/coderd/coderd.go b/coderd/coderd.go index 6b89f4d095..264901b811 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -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{ diff --git a/coderd/healthcheck/health/model.go b/coderd/healthcheck/health/model.go index 9eae390aa0..9f853864fd 100644 --- a/coderd/healthcheck/health/model.go +++ b/coderd/healthcheck/health/model.go @@ -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" diff --git a/coderd/healthcheck/workspaceproxy.go b/coderd/healthcheck/workspaceproxy.go index c5ff1c7082..5302eea76e 100644 --- a/coderd/healthcheck/workspaceproxy.go +++ b/coderd/healthcheck/workspaceproxy.go @@ -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 diff --git a/coderd/healthcheck/workspaceproxy_internal_test.go b/coderd/healthcheck/workspaceproxy_internal_test.go index 605971e233..5a7875518d 100644 --- a/coderd/healthcheck/workspaceproxy_internal_test.go +++ b/coderd/healthcheck/workspaceproxy_internal_test.go @@ -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) }) } diff --git a/coderd/healthcheck/workspaceproxy_test.go b/coderd/healthcheck/workspaceproxy_test.go index fd4c127cfb..a5fab6c63b 100644 --- a/coderd/healthcheck/workspaceproxy_test.go +++ b/coderd/healthcheck/workspaceproxy_test.go @@ -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) diff --git a/docs/admin/healthcheck.md b/docs/admin/healthcheck.md index 62a7de6197..8712dae2a5 100644 --- a/docs/admin/healthcheck.md +++ b/docs/admin/healthcheck.md @@ -246,18 +246,6 @@ from the database. **Solution:** This may be a transient issue. If it persists, it could signify an issue with Coder's configured database. -### EWP03 - -_Workspace Proxy Version Mismatch_ - -**Problem:** One or more workspace proxies are more than one major or minor -version out of date with the main deployment. It is important that workspace -proxies are updated at the same time as the main deployment to minimize the risk -of API incompatibility. - -**Solution:** Update the workspace proxy to match the currently running version -of Coder. - ### EWP04 _One or more Workspace Proxies Unhealthy_ diff --git a/docs/api/schemas.md b/docs/api/schemas.md index 60722a0ab9..710f7d93fb 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -8411,7 +8411,6 @@ If the schedule is empty, the user will be updated to use the default schedule.| | `EUNKNOWN` | | `EWP01` | | `EWP02` | -| `EWP03` | | `EWP04` | | `EDB01` | | `EDB02` | diff --git a/enterprise/coderd/workspaceproxy.go b/enterprise/coderd/workspaceproxy.go index c229903ada..379d01ad43 100644 --- a/enterprise/coderd/workspaceproxy.go +++ b/enterprise/coderd/workspaceproxy.go @@ -4,7 +4,6 @@ import ( "context" "crypto/sha256" "database/sql" - "flag" "fmt" "net/http" "net/url" @@ -15,7 +14,6 @@ import ( "golang.org/x/xerrors" "cdr.dev/slog" - "github.com/coder/coder/v2/buildinfo" agpl "github.com/coder/coder/v2/coderd" "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/database" @@ -552,36 +550,18 @@ func (api *API) workspaceProxyRegister(rw http.ResponseWriter, r *http.Request) var ( ctx = r.Context() proxy = httpmw.WorkspaceProxy(r) - // TODO: This audit log does not work because it has no user id - // associated with it. The audit log commitAudit() function ignores - // the audit log if there is no user id. We should find a solution - // to make sure this event is tracked. - // auditor = api.AGPL.Auditor.Load() - // aReq, commitAudit = audit.InitRequest[database.WorkspaceProxy](rw, &audit.RequestParams{ - // Audit: *auditor, - // Log: api.Logger, - // Request: r, - // Action: database.AuditActionWrite, - // }) ) - // aReq.Old = proxy - // defer commitAudit() var req wsproxysdk.RegisterWorkspaceProxyRequest if !httpapi.Read(ctx, rw, r, &req) { return } - // Version check should be forced in non-dev builds and when running in - // tests. Only Major + minor versions are checked. - shouldForceVersion := !buildinfo.IsDev() || flag.Lookup("test.v") != nil - if shouldForceVersion && !buildinfo.VersionsMatch(req.Version, buildinfo.Version()) { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Version mismatch.", - Detail: fmt.Sprintf("Proxy version %q does not match primary server version %q", req.Version, buildinfo.Version()), - }) - return - } + // NOTE: we previously enforced version checks when registering, but this + // will cause proxies to enter crash loop backoff if the server is updated + // and the proxy is not. Most releases do not make backwards-incompatible + // changes to the proxy API, so instead of blocking requests we will show + // healthcheck warnings. if err := validateProxyURL(req.AccessURL); err != nil { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ @@ -718,7 +698,6 @@ func (api *API) workspaceProxyRegister(rw http.ResponseWriter, r *http.Request) siblingsRes = append(siblingsRes, convertReplica(replica)) } - // aReq.New = updatedProxy httpapi.Write(ctx, rw, http.StatusCreated, wsproxysdk.RegisterWorkspaceProxyResponse{ AppSecurityKey: api.AppSecurityKey.String(), DERPMeshKey: api.DERPServer.MeshKey(), diff --git a/enterprise/wsproxy/wsproxy.go b/enterprise/wsproxy/wsproxy.go index 3766064608..a8b05123a1 100644 --- a/enterprise/wsproxy/wsproxy.go +++ b/enterprise/wsproxy/wsproxy.go @@ -168,7 +168,6 @@ func New(ctx context.Context, opts *Options) (*Server, error) { client.SDKClient.HTTPClient = opts.HTTPClient } - // TODO: Probably do some version checking here info, err := client.SDKClient.BuildInfo(ctx) if err != nil { return nil, xerrors.Errorf("buildinfo: %w", errors.Join( @@ -179,6 +178,15 @@ func New(ctx context.Context, opts *Options) (*Server, error) { if info.WorkspaceProxy { return nil, xerrors.Errorf("%q is a workspace proxy, not a primary coderd instance", opts.DashboardURL) } + // We don't want to crash the proxy if the versions don't match because + // it'll enter crash loop backoff (and most patches don't make any backwards + // incompatible changes to the proxy API anyways) + if !buildinfo.VersionsMatch(info.Version, buildinfo.Version()) { + opts.Logger.Warn(ctx, "workspace proxy version doesn't match Minor.Major version of the primary, please keep them in sync", + slog.F("primary_version", info.Version), + slog.F("proxy_version", buildinfo.Version()), + ) + } meshTLSConfig, err := replicasync.CreateDERPMeshTLSConfig(opts.AccessURL.Hostname(), opts.TLSCertificates) if err != nil { diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index e4f2d1f386..deb4e06363 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -2459,7 +2459,6 @@ export type HealthCode = | "EUNKNOWN" | "EWP01" | "EWP02" - | "EWP03" | "EWP04" | "EWS01" | "EWS02" @@ -2479,7 +2478,6 @@ export const HealthCodes: HealthCode[] = [ "EUNKNOWN", "EWP01", "EWP02", - "EWP03", "EWP04", "EWS01", "EWS02",