From b0a025e9f8d5a3f25ececd7c79f0a6349bf28857 Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Mon, 28 Apr 2025 17:34:17 +0000 Subject: [PATCH 1/6] refactor: use specific error for agpl and prebuilds --- coderd/prebuilds/api.go | 5 ++++- coderd/prebuilds/noop.go | 2 +- coderd/workspaces.go | 7 +++++-- enterprise/coderd/prebuilds/claim_test.go | 10 +++++++++- 4 files changed, 19 insertions(+), 5 deletions(-) diff --git a/coderd/prebuilds/api.go b/coderd/prebuilds/api.go index 2342da5d62c07..fc4bf2e774334 100644 --- a/coderd/prebuilds/api.go +++ b/coderd/prebuilds/api.go @@ -9,7 +9,10 @@ import ( "github.com/coder/coder/v2/coderd/database" ) -var ErrNoClaimablePrebuiltWorkspaces = xerrors.New("no claimable prebuilt workspaces found") +var ( + ErrNoClaimablePrebuiltWorkspaces = xerrors.New("no claimable prebuilt workspaces found") + ErrAGPLDoesNotSupportPrebuilds = xerrors.New("prebuild-related functionality is not supported under the AGPL license") +) // ReconciliationOrchestrator manages the lifecycle of prebuild reconciliation. // It runs a continuous loop to check and reconcile prebuild states, and can be stopped gracefully. diff --git a/coderd/prebuilds/noop.go b/coderd/prebuilds/noop.go index e3dc0597b169b..d320980ceccc8 100644 --- a/coderd/prebuilds/noop.go +++ b/coderd/prebuilds/noop.go @@ -27,7 +27,7 @@ type NoopClaimer struct{} func (NoopClaimer) Claim(context.Context, uuid.UUID, string, uuid.UUID) (*uuid.UUID, error) { // Not entitled to claim prebuilds in AGPL version. - return nil, ErrNoClaimablePrebuiltWorkspaces + return nil, ErrAGPLDoesNotSupportPrebuilds } func (NoopClaimer) Initiator() uuid.UUID { diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 12b3787acf3d8..292dbbb2d66fd 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -650,8 +650,11 @@ func createWorkspace( if req.TemplateVersionPresetID != uuid.Nil { // Try and claim an eligible prebuild, if available. claimedWorkspace, err = claimPrebuild(ctx, prebuildsClaimer, db, api.Logger, req, owner) - if err != nil && !errors.Is(err, prebuilds.ErrNoClaimablePrebuiltWorkspaces) { - return xerrors.Errorf("claim prebuild: %w", err) + if err != nil && + !errors.Is(err, prebuilds.ErrNoClaimablePrebuiltWorkspaces) && + !errors.Is(err, prebuilds.ErrAGPLDoesNotSupportPrebuilds) { + + return xerrors.Errorf("failed to claim prebuilt workspace: %w", err) } } diff --git a/enterprise/coderd/prebuilds/claim_test.go b/enterprise/coderd/prebuilds/claim_test.go index 5d75b7463471d..fb2c351f3069d 100644 --- a/enterprise/coderd/prebuilds/claim_test.go +++ b/enterprise/coderd/prebuilds/claim_test.go @@ -111,6 +111,11 @@ func TestClaimPrebuild(t *testing.T) { markPrebuildsClaimable: true, claimingErr: agplprebuilds.ErrNoClaimablePrebuiltWorkspaces, }, + "AGPL does not support prebuilds error is returned": { + expectPrebuildClaimed: false, + markPrebuildsClaimable: true, + claimingErr: agplprebuilds.ErrAGPLDoesNotSupportPrebuilds, + }, "unexpected claiming error is returned": { expectPrebuildClaimed: false, markPrebuildsClaimable: true, @@ -224,8 +229,11 @@ func TestClaimPrebuild(t *testing.T) { TemplateVersionPresetID: presets[0].ID, }) + isNoPrebuiltWorkspaces := errors.Is(tc.claimingErr, agplprebuilds.ErrNoClaimablePrebuiltWorkspaces) + isUnsupported := errors.Is(tc.claimingErr, agplprebuilds.ErrAGPLDoesNotSupportPrebuilds) + switch { - case tc.claimingErr != nil && errors.Is(tc.claimingErr, agplprebuilds.ErrNoClaimablePrebuiltWorkspaces): + case tc.claimingErr != nil && (isNoPrebuiltWorkspaces || isUnsupported): require.NoError(t, err) coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, userWorkspace.LatestBuild.ID) From 71d66af44ee50226562344dd17c3e4775d104bfa Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Mon, 28 Apr 2025 18:29:33 +0000 Subject: [PATCH 2/6] refactor: add minor comment --- coderd/workspaces.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 292dbbb2d66fd..a29d3e16df86a 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -650,6 +650,8 @@ func createWorkspace( if req.TemplateVersionPresetID != uuid.Nil { // Try and claim an eligible prebuild, if available. claimedWorkspace, err = claimPrebuild(ctx, prebuildsClaimer, db, api.Logger, req, owner) + // If claiming fails with an expected error (no claimable prebuilds or AGPL does not support prebuilds), + // we fall back to creating a new workspace. Otherwise, propagate the unexpected error. if err != nil && !errors.Is(err, prebuilds.ErrNoClaimablePrebuiltWorkspaces) && !errors.Is(err, prebuilds.ErrAGPLDoesNotSupportPrebuilds) { From bec0fdcae4ba28ceef60ce0271294f8f405fdcba Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Mon, 28 Apr 2025 18:37:50 +0000 Subject: [PATCH 3/6] refactor: fix linter --- coderd/workspaces.go | 1 - 1 file changed, 1 deletion(-) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index a29d3e16df86a..d5fd77d6c52fc 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -655,7 +655,6 @@ func createWorkspace( if err != nil && !errors.Is(err, prebuilds.ErrNoClaimablePrebuiltWorkspaces) && !errors.Is(err, prebuilds.ErrAGPLDoesNotSupportPrebuilds) { - return xerrors.Errorf("failed to claim prebuilt workspace: %w", err) } } From 522a28a13f8ab2049c9499f33bae5808224707a8 Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Mon, 28 Apr 2025 18:59:27 +0000 Subject: [PATCH 4/6] CR's fixes --- coderd/prebuilds/api.go | 4 ++-- coderd/prebuilds/noop.go | 2 +- coderd/workspaces.go | 2 +- enterprise/coderd/prebuilds/claim_test.go | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/coderd/prebuilds/api.go b/coderd/prebuilds/api.go index fc4bf2e774334..00129eae37491 100644 --- a/coderd/prebuilds/api.go +++ b/coderd/prebuilds/api.go @@ -10,8 +10,8 @@ import ( ) var ( - ErrNoClaimablePrebuiltWorkspaces = xerrors.New("no claimable prebuilt workspaces found") - ErrAGPLDoesNotSupportPrebuilds = xerrors.New("prebuild-related functionality is not supported under the AGPL license") + ErrNoClaimablePrebuiltWorkspaces = xerrors.New("no claimable prebuilt workspaces found") + ErrAGPLDoesNotSupportPrebuiltWorkspaces = xerrors.New("prebuilt workspaces functionality is not supported under the AGPL license") ) // ReconciliationOrchestrator manages the lifecycle of prebuild reconciliation. diff --git a/coderd/prebuilds/noop.go b/coderd/prebuilds/noop.go index d320980ceccc8..6fb3f7c6a5f1f 100644 --- a/coderd/prebuilds/noop.go +++ b/coderd/prebuilds/noop.go @@ -27,7 +27,7 @@ type NoopClaimer struct{} func (NoopClaimer) Claim(context.Context, uuid.UUID, string, uuid.UUID) (*uuid.UUID, error) { // Not entitled to claim prebuilds in AGPL version. - return nil, ErrAGPLDoesNotSupportPrebuilds + return nil, ErrAGPLDoesNotSupportPrebuiltWorkspaces } func (NoopClaimer) Initiator() uuid.UUID { diff --git a/coderd/workspaces.go b/coderd/workspaces.go index d5fd77d6c52fc..2bfcb81f8d6cf 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -654,7 +654,7 @@ func createWorkspace( // we fall back to creating a new workspace. Otherwise, propagate the unexpected error. if err != nil && !errors.Is(err, prebuilds.ErrNoClaimablePrebuiltWorkspaces) && - !errors.Is(err, prebuilds.ErrAGPLDoesNotSupportPrebuilds) { + !errors.Is(err, prebuilds.ErrAGPLDoesNotSupportPrebuiltWorkspaces) { return xerrors.Errorf("failed to claim prebuilt workspace: %w", err) } } diff --git a/enterprise/coderd/prebuilds/claim_test.go b/enterprise/coderd/prebuilds/claim_test.go index fb2c351f3069d..145095e6533e7 100644 --- a/enterprise/coderd/prebuilds/claim_test.go +++ b/enterprise/coderd/prebuilds/claim_test.go @@ -114,7 +114,7 @@ func TestClaimPrebuild(t *testing.T) { "AGPL does not support prebuilds error is returned": { expectPrebuildClaimed: false, markPrebuildsClaimable: true, - claimingErr: agplprebuilds.ErrAGPLDoesNotSupportPrebuilds, + claimingErr: agplprebuilds.ErrAGPLDoesNotSupportPrebuiltWorkspaces, }, "unexpected claiming error is returned": { expectPrebuildClaimed: false, @@ -230,7 +230,7 @@ func TestClaimPrebuild(t *testing.T) { }) isNoPrebuiltWorkspaces := errors.Is(tc.claimingErr, agplprebuilds.ErrNoClaimablePrebuiltWorkspaces) - isUnsupported := errors.Is(tc.claimingErr, agplprebuilds.ErrAGPLDoesNotSupportPrebuilds) + isUnsupported := errors.Is(tc.claimingErr, agplprebuilds.ErrAGPLDoesNotSupportPrebuiltWorkspaces) switch { case tc.claimingErr != nil && (isNoPrebuiltWorkspaces || isUnsupported): From a4954e4e174bd1ceb24052fcfbc9304007c880e2 Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Mon, 28 Apr 2025 19:04:56 +0000 Subject: [PATCH 5/6] CR's fixes --- coderd/workspaces.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 2bfcb81f8d6cf..6c8a4ccbca460 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -655,6 +655,9 @@ func createWorkspace( if err != nil && !errors.Is(err, prebuilds.ErrNoClaimablePrebuiltWorkspaces) && !errors.Is(err, prebuilds.ErrAGPLDoesNotSupportPrebuiltWorkspaces) { + api.Logger.Error(ctx, "failed to claim prebuilt workspace", slog.Error(err), + slog.F("workspace_name", req.Name), slog.F("template_version_preset_id", req.TemplateVersionPresetID)) + return xerrors.Errorf("failed to claim prebuilt workspace: %w", err) } } From ffac93361f891fc38ef0cce8ee645d4ecbd5b8aa Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Mon, 28 Apr 2025 19:49:34 +0000 Subject: [PATCH 6/6] CR's fixes --- coderd/workspaces.go | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 6c8a4ccbca460..2ac432d905ae6 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -652,13 +652,26 @@ func createWorkspace( claimedWorkspace, err = claimPrebuild(ctx, prebuildsClaimer, db, api.Logger, req, owner) // If claiming fails with an expected error (no claimable prebuilds or AGPL does not support prebuilds), // we fall back to creating a new workspace. Otherwise, propagate the unexpected error. - if err != nil && - !errors.Is(err, prebuilds.ErrNoClaimablePrebuiltWorkspaces) && - !errors.Is(err, prebuilds.ErrAGPLDoesNotSupportPrebuiltWorkspaces) { - api.Logger.Error(ctx, "failed to claim prebuilt workspace", slog.Error(err), - slog.F("workspace_name", req.Name), slog.F("template_version_preset_id", req.TemplateVersionPresetID)) + if err != nil { + isExpectedError := errors.Is(err, prebuilds.ErrNoClaimablePrebuiltWorkspaces) || + errors.Is(err, prebuilds.ErrAGPLDoesNotSupportPrebuiltWorkspaces) + fields := []any{ + slog.Error(err), + slog.F("workspace_name", req.Name), + slog.F("template_version_preset_id", req.TemplateVersionPresetID), + } + + if !isExpectedError { + // if it's an unexpected error - use error log level + api.Logger.Error(ctx, "failed to claim prebuilt workspace", fields...) + + return xerrors.Errorf("failed to claim prebuilt workspace: %w", err) + } + + // if it's an expected error - use warn log level + api.Logger.Warn(ctx, "failed to claim prebuilt workspace", fields...) - return xerrors.Errorf("failed to claim prebuilt workspace: %w", err) + // fall back to creating a new workspace } }