From 8b9e30d0f1b990ca6e1ca52d5b7ae602865f76a7 Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Thu, 17 Apr 2025 17:56:39 +0000 Subject: [PATCH 01/34] feat: implement claiming of prebuilt workspaces --- coderd/coderd.go | 3 + coderd/prebuilds/api.go | 33 ++ coderd/prebuilds/noop.go | 15 + .../provisionerdserver/provisionerdserver.go | 16 +- coderd/workspaces.go | 116 ++++- coderd/wsbuilder/wsbuilder.go | 30 +- codersdk/deployment.go | 49 +- codersdk/organizations.go | 7 +- enterprise/coderd/coderd.go | 43 ++ enterprise/coderd/prebuilds/claim.go | 64 +++ enterprise/coderd/prebuilds/claim_test.go | 427 ++++++++++++++++++ 11 files changed, 769 insertions(+), 34 deletions(-) create mode 100644 enterprise/coderd/prebuilds/claim.go create mode 100644 enterprise/coderd/prebuilds/claim_test.go diff --git a/coderd/coderd.go b/coderd/coderd.go index 72ebce81120fa..7d4142d9ea7db 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -9,6 +9,7 @@ import ( "expvar" "flag" "fmt" + "github.com/coder/coder/v2/coderd/prebuilds" "io" "net/http" "net/url" @@ -595,6 +596,7 @@ func New(options *Options) *API { f := appearance.NewDefaultFetcher(api.DeploymentValues.DocsURL.String()) api.AppearanceFetcher.Store(&f) api.PortSharer.Store(&portsharing.DefaultPortSharer) + api.PrebuildsClaimer.Store(&prebuilds.DefaultClaimer) buildInfo := codersdk.BuildInfoResponse{ ExternalURL: buildinfo.ExternalURL(), Version: buildinfo.Version(), @@ -1562,6 +1564,7 @@ type API struct { AccessControlStore *atomic.Pointer[dbauthz.AccessControlStore] PortSharer atomic.Pointer[portsharing.PortSharer] FileCache files.Cache + PrebuildsClaimer atomic.Pointer[prebuilds.Claimer] UpdatesProvider tailnet.WorkspaceUpdatesProvider diff --git a/coderd/prebuilds/api.go b/coderd/prebuilds/api.go index 6ebfb8acced44..b0acaf9cb71b0 100644 --- a/coderd/prebuilds/api.go +++ b/coderd/prebuilds/api.go @@ -2,6 +2,10 @@ package prebuilds import ( "context" + + "github.com/google/uuid" + + "github.com/coder/coder/v2/coderd/database" ) // ReconciliationOrchestrator manages the lifecycle of prebuild reconciliation. @@ -19,9 +23,38 @@ type ReconciliationOrchestrator interface { Stop(ctx context.Context, cause error) } +// Reconciler defines the core operations for managing prebuilds. +// It provides both high-level orchestration (ReconcileAll) and lower-level operations +// for more fine-grained control (SnapshotState, ReconcilePreset, CalculateActions). +// All database operations must be performed within repeatable-read transactions +// to ensure consistency. type Reconciler interface { // ReconcileAll orchestrates the reconciliation of all prebuilds across all templates. // It takes a global snapshot of the system state and then reconciles each preset // in parallel, creating or deleting prebuilds as needed to reach their desired states. + // For more fine-grained control, you can use the lower-level methods SnapshotState + // and ReconcilePreset directly. ReconcileAll(ctx context.Context) error + + // SnapshotState captures the current state of all prebuilds across templates. + // It creates a global database snapshot that can be viewed as a collection of PresetSnapshots, + // each representing the state of prebuilds for a specific preset. + // MUST be called inside a repeatable-read transaction. + SnapshotState(ctx context.Context, store database.Store) (*GlobalSnapshot, error) + + // ReconcilePreset handles a single PresetSnapshot, determining and executing + // the required actions (creating or deleting prebuilds) based on the current state. + // MUST be called inside a repeatable-read transaction. + ReconcilePreset(ctx context.Context, snapshot PresetSnapshot) error + + // CalculateActions determines what actions are needed to reconcile a preset's prebuilds + // to their desired state. This includes creating new prebuilds, deleting excess ones, + // or waiting due to backoff periods. + // MUST be called inside a repeatable-read transaction. + CalculateActions(ctx context.Context, state PresetSnapshot) (*ReconciliationActions, error) +} + +type Claimer interface { + Claim(ctx context.Context, store database.Store, userID uuid.UUID, name string, presetID uuid.UUID) (*uuid.UUID, error) + Initiator() uuid.UUID } diff --git a/coderd/prebuilds/noop.go b/coderd/prebuilds/noop.go index ffe4e7b442af9..4232e1f2654b8 100644 --- a/coderd/prebuilds/noop.go +++ b/coderd/prebuilds/noop.go @@ -3,6 +3,8 @@ package prebuilds import ( "context" + "github.com/google/uuid" + "github.com/coder/coder/v2/coderd/database" ) @@ -33,3 +35,16 @@ func (NoopReconciler) CalculateActions(context.Context, PresetSnapshot) (*Reconc } var _ ReconciliationOrchestrator = NoopReconciler{} + +type AGPLPrebuildClaimer struct{} + +func (c AGPLPrebuildClaimer) Claim(context.Context, database.Store, uuid.UUID, string, uuid.UUID) (*uuid.UUID, error) { + // Not entitled to claim prebuilds in AGPL version. + return nil, nil +} + +func (c AGPLPrebuildClaimer) Initiator() uuid.UUID { + return uuid.Nil +} + +var DefaultClaimer Claimer = AGPLPrebuildClaimer{} diff --git a/coderd/provisionerdserver/provisionerdserver.go b/coderd/provisionerdserver/provisionerdserver.go index a4e28741ce988..ba224fd4e850c 100644 --- a/coderd/provisionerdserver/provisionerdserver.go +++ b/coderd/provisionerdserver/provisionerdserver.go @@ -2462,10 +2462,18 @@ type TemplateVersionImportJob struct { // WorkspaceProvisionJob is the payload for the "workspace_provision" job type. type WorkspaceProvisionJob struct { - WorkspaceBuildID uuid.UUID `json:"workspace_build_id"` - DryRun bool `json:"dry_run"` - IsPrebuild bool `json:"is_prebuild,omitempty"` - LogLevel string `json:"log_level,omitempty"` + WorkspaceBuildID uuid.UUID `json:"workspace_build_id"` + DryRun bool `json:"dry_run"` + IsPrebuild bool `json:"is_prebuild,omitempty"` + PrebuildClaimedByUser uuid.UUID `json:"prebuild_claimed_by,omitempty"` + // RunningWorkspaceAgentID is *only* used for prebuilds. We pass it down when we want to rebuild a prebuilt workspace + // but not generate a new agent token. The provisionerdserver will retrieve this token and push it down to + // the provisioner (and ultimately to the `coder_agent` resource in the Terraform provider) where it will be + // reused. Context: the agent token is often used in immutable attributes of workspace resource (e.g. VM/container) + // to initialize the agent, so if that value changes it will necessitate a replacement of that resource, thus + // obviating the whole point of the prebuild. + RunningWorkspaceAgentID uuid.UUID `json:"running_workspace_agent_id"` + LogLevel string `json:"log_level,omitempty"` } // TemplateVersionDryRunJob is the payload for the "template_version_dry_run" job type. diff --git a/coderd/workspaces.go b/coderd/workspaces.go index a654597faeadd..71d02c4d8b7c7 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "fmt" + "github.com/coder/coder/v2/coderd/prebuilds" "net/http" "slices" "strconv" @@ -635,34 +636,71 @@ func createWorkspace( provisionerJob *database.ProvisionerJob workspaceBuild *database.WorkspaceBuild provisionerDaemons []database.GetEligibleProvisionerDaemonsByProvisionerJobIDsRow + + runningWorkspaceAgentID uuid.UUID ) + + prebuilds := (*api.PrebuildsClaimer.Load()).(prebuilds.Claimer) + err = api.Database.InTx(func(db database.Store) error { - now := dbtime.Now() - // Workspaces are created without any versions. - minimumWorkspace, err := db.InsertWorkspace(ctx, database.InsertWorkspaceParams{ - ID: uuid.New(), - CreatedAt: now, - UpdatedAt: now, - OwnerID: owner.ID, - OrganizationID: template.OrganizationID, - TemplateID: template.ID, - Name: req.Name, - AutostartSchedule: dbAutostartSchedule, - NextStartAt: nextStartAt, - Ttl: dbTTL, - // The workspaces page will sort by last used at, and it's useful to - // have the newly created workspace at the top of the list! - LastUsedAt: dbtime.Now(), - AutomaticUpdates: dbAU, - }) - if err != nil { - return xerrors.Errorf("insert workspace: %w", err) + var ( + workspaceID uuid.UUID + claimedWorkspace *database.Workspace + ) + + // If a template preset was chosen, try claim a prebuild. + if req.TemplateVersionPresetID != uuid.Nil { + // Try and claim an eligible prebuild, if available. + claimedWorkspace, err = claimPrebuild(ctx, prebuilds, db, api.Logger, req, owner) + if err != nil { + return xerrors.Errorf("claim prebuild: %w", err) + } + } + + // No prebuild found; regular flow. + if claimedWorkspace == nil { + now := dbtime.Now() + // Workspaces are created without any versions. + minimumWorkspace, err := db.InsertWorkspace(ctx, database.InsertWorkspaceParams{ + ID: uuid.New(), + CreatedAt: now, + UpdatedAt: now, + OwnerID: owner.ID, + OrganizationID: template.OrganizationID, + TemplateID: template.ID, + Name: req.Name, + AutostartSchedule: dbAutostartSchedule, + NextStartAt: nextStartAt, + Ttl: dbTTL, + // The workspaces page will sort by last used at, and it's useful to + // have the newly created workspace at the top of the list! + LastUsedAt: dbtime.Now(), + AutomaticUpdates: dbAU, + }) + if err != nil { + return xerrors.Errorf("insert workspace: %w", err) + } + workspaceID = minimumWorkspace.ID + } else { + // Prebuild found! + workspaceID = claimedWorkspace.ID + initiatorID = prebuilds.Initiator() + agents, err := db.GetWorkspaceAgentsInLatestBuildByWorkspaceID(ctx, claimedWorkspace.ID) + if err != nil { + // TODO: comment about best-effort, workspace can be restarted if this fails... + api.Logger.Error(ctx, "failed to retrieve running agents of claimed prebuilt workspace", + slog.F("workspace_id", claimedWorkspace.ID), slog.Error(err)) + } + if len(agents) >= 1 { + // TODO: handle multiple agents + runningWorkspaceAgentID = agents[0].ID + } } // We have to refetch the workspace for the joined in fields. // TODO: We can use WorkspaceTable for the builder to not require // this extra fetch. - workspace, err = db.GetWorkspaceByID(ctx, minimumWorkspace.ID) + workspace, err = db.GetWorkspaceByID(ctx, workspaceID) if err != nil { return xerrors.Errorf("get workspace by ID: %w", err) } @@ -672,10 +710,18 @@ func createWorkspace( Initiator(initiatorID). ActiveVersion(). RichParameterValues(req.RichParameterValues). - TemplateVersionPresetID(req.TemplateVersionPresetID) + TemplateVersionPresetID(req.TemplateVersionPresetID). + RunningWorkspaceAgentID(runningWorkspaceAgentID) if req.TemplateVersionID != uuid.Nil { builder = builder.VersionID(req.TemplateVersionID) } + if req.TemplateVersionPresetID != uuid.Nil { + builder = builder.TemplateVersionPresetID(req.TemplateVersionPresetID) + } + + if claimedWorkspace != nil { + builder = builder.MarkPrebuildClaimedBy(owner.ID) + } workspaceBuild, provisionerJob, provisionerDaemons, err = builder.Build( ctx, @@ -839,6 +885,32 @@ func requestTemplate(ctx context.Context, rw http.ResponseWriter, req codersdk.C return template, true } +func claimPrebuild(ctx context.Context, claimer prebuilds.Claimer, db database.Store, logger slog.Logger, req codersdk.CreateWorkspaceRequest, owner workspaceOwner) (*database.Workspace, error) { + prebuildsCtx := dbauthz.AsPrebuildsOrchestrator(ctx) + + // TODO: do we need a timeout here? + claimCtx, cancel := context.WithTimeout(prebuildsCtx, time.Second*10) + defer cancel() + + claimedID, err := claimer.Claim(claimCtx, db, owner.ID, req.Name, req.TemplateVersionPresetID) + if err != nil { + // TODO: enhance this by clarifying whether this *specific* prebuild failed or whether there are none to claim. + return nil, xerrors.Errorf("claim prebuild: %w", err) + } + + // No prebuild available. + if claimedID == nil { + return nil, nil + } + + lookup, err := db.GetWorkspaceByID(prebuildsCtx, *claimedID) + if err != nil { + logger.Error(ctx, "unable to find claimed workspace by ID", slog.Error(err), slog.F("claimed_prebuild_id", (*claimedID).String())) + return nil, xerrors.Errorf("find claimed workspace by ID %q: %w", (*claimedID).String(), err) + } + return &lookup, err +} + func (api *API) notifyWorkspaceCreated( ctx context.Context, receiverID uuid.UUID, diff --git a/coderd/wsbuilder/wsbuilder.go b/coderd/wsbuilder/wsbuilder.go index fa7c00861202d..136afe8906d06 100644 --- a/coderd/wsbuilder/wsbuilder.go +++ b/coderd/wsbuilder/wsbuilder.go @@ -75,7 +75,9 @@ type Builder struct { parameterValues *[]string templateVersionPresetParameterValues []database.TemplateVersionPresetParameter - prebuild bool + prebuild bool + prebuildClaimedBy uuid.UUID + runningWorkspaceAgentID uuid.UUID verifyNoLegacyParametersOnce bool } @@ -178,6 +180,19 @@ func (b Builder) MarkPrebuild() Builder { return b } +func (b Builder) MarkPrebuildClaimedBy(userID uuid.UUID) Builder { + // nolint: revive + b.prebuildClaimedBy = userID + return b +} + +// RunningWorkspaceAgentID is only used for prebuilds; see the associated field in `provisionerdserver.WorkspaceProvisionJob`. +func (b Builder) RunningWorkspaceAgentID(id uuid.UUID) Builder { + // nolint: revive + b.runningWorkspaceAgentID = id + return b +} + // SetLastWorkspaceBuildInTx prepopulates the Builder's cache with the last workspace build. This allows us // to avoid a repeated database query when the Builder's caller also needs the workspace build, e.g. auto-start & // auto-stop. @@ -309,9 +324,11 @@ func (b *Builder) buildTx(authFunc func(action policy.Action, object rbac.Object workspaceBuildID := uuid.New() input, err := json.Marshal(provisionerdserver.WorkspaceProvisionJob{ - WorkspaceBuildID: workspaceBuildID, - LogLevel: b.logLevel, - IsPrebuild: b.prebuild, + WorkspaceBuildID: workspaceBuildID, + LogLevel: b.logLevel, + IsPrebuild: b.prebuild, + PrebuildClaimedByUser: b.prebuildClaimedBy, + RunningWorkspaceAgentID: b.runningWorkspaceAgentID, }) if err != nil { return nil, nil, nil, BuildError{ @@ -624,6 +641,11 @@ func (b *Builder) findNewBuildParameterValue(name string) *codersdk.WorkspaceBui } func (b *Builder) getLastBuildParameters() ([]database.WorkspaceBuildParameter, error) { + // TODO: exclude preset params from this list instead of returning nothing? + if b.prebuildClaimedBy != uuid.Nil { + return nil, nil + } + if b.lastBuildParameters != nil { return *b.lastBuildParameters, nil } diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 8b447e2c96e06..17d3f5990ea9c 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -81,6 +81,7 @@ const ( FeatureControlSharedPorts FeatureName = "control_shared_ports" FeatureCustomRoles FeatureName = "custom_roles" FeatureMultipleOrganizations FeatureName = "multiple_organizations" + FeatureWorkspacePrebuilds FeatureName = "workspace_prebuilds" ) // FeatureNames must be kept in-sync with the Feature enum above. @@ -103,6 +104,7 @@ var FeatureNames = []FeatureName{ FeatureControlSharedPorts, FeatureCustomRoles, FeatureMultipleOrganizations, + FeatureWorkspacePrebuilds, } // Humanize returns the feature name in a human-readable format. @@ -132,6 +134,7 @@ func (n FeatureName) AlwaysEnable() bool { FeatureHighAvailability: true, FeatureCustomRoles: true, FeatureMultipleOrganizations: true, + FeatureWorkspacePrebuilds: true, }[n] } @@ -393,6 +396,7 @@ type DeploymentValues struct { TermsOfServiceURL serpent.String `json:"terms_of_service_url,omitempty" typescript:",notnull"` Notifications NotificationsConfig `json:"notifications,omitempty" typescript:",notnull"` AdditionalCSPPolicy serpent.StringArray `json:"additional_csp_policy,omitempty" typescript:",notnull"` + Prebuilds PrebuildsConfig `json:"workspace_prebuilds,omitempty" typescript:",notnull"` WorkspaceHostnameSuffix serpent.String `json:"workspace_hostname_suffix,omitempty" typescript:",notnull"` Config serpent.YAMLConfigPath `json:"config,omitempty" typescript:",notnull"` @@ -1034,6 +1038,11 @@ func (c *DeploymentValues) Options() serpent.OptionSet { Parent: &deploymentGroupNotifications, YAML: "webhook", } + deploymentGroupPrebuilds = serpent.Group{ + Name: "Workspace Prebuilds", + YAML: "workspace_prebuilds", + Description: "Configure how workspace prebuilds behave.", + } deploymentGroupInbox = serpent.Group{ Name: "Inbox", Parent: &deploymentGroupNotifications, @@ -3029,6 +3038,41 @@ Write out the current server config as YAML to stdout.`, Annotations: serpent.Annotations{}.Mark(annotationFormatDuration, "true"), Hidden: true, // Hidden because most operators should not need to modify this. }, + { + Name: "Reconciliation Interval", + Description: "How often to reconcile workspace prebuilds state.", + Flag: "workspace-prebuilds-reconciliation-interval", + Env: "CODER_WORKSPACE_PREBUILDS_RECONCILIATION_INTERVAL", + Value: &c.Prebuilds.ReconciliationInterval, + Default: (time.Second * 15).String(), + Group: &deploymentGroupPrebuilds, + YAML: "reconciliation_interval", + Annotations: serpent.Annotations{}.Mark(annotationFormatDuration, "true"), + }, + { + Name: "Reconciliation Backoff Interval", + Description: "Interval to increase reconciliation backoff by when unrecoverable errors occur.", + Flag: "workspace-prebuilds-reconciliation-backoff-interval", + Env: "CODER_WORKSPACE_PREBUILDS_RECONCILIATION_BACKOFF_INTERVAL", + Value: &c.Prebuilds.ReconciliationBackoffInterval, + Default: (time.Second * 15).String(), + Group: &deploymentGroupPrebuilds, + YAML: "reconciliation_backoff_interval", + Annotations: serpent.Annotations{}.Mark(annotationFormatDuration, "true"), + Hidden: true, + }, + { + Name: "Reconciliation Backoff Lookback Period", + Description: "Interval to look back to determine number of failed builds, which influences backoff.", + Flag: "workspace-prebuilds-reconciliation-backoff-lookback-period", + Env: "CODER_WORKSPACE_PREBUILDS_RECONCILIATION_BACKOFF_LOOKBACK_PERIOD", + Value: &c.Prebuilds.ReconciliationBackoffLookback, + Default: (time.Hour).String(), // TODO: use https://pkg.go.dev/github.com/jackc/pgtype@v1.12.0#Interval + Group: &deploymentGroupPrebuilds, + YAML: "reconciliation_backoff_lookback_period", + Annotations: serpent.Annotations{}.Mark(annotationFormatDuration, "true"), + Hidden: true, + }, // Push notifications. } @@ -3254,6 +3298,7 @@ const ( ExperimentAutoFillParameters Experiment = "auto-fill-parameters" // This should not be taken out of experiments until we have redesigned the feature. ExperimentNotifications Experiment = "notifications" // Sends notifications via SMTP and webhooks following certain events. ExperimentWorkspaceUsage Experiment = "workspace-usage" // Enables the new workspace usage tracking. + ExperimentWorkspacePrebuilds Experiment = "workspace-prebuilds" // Enables the new workspace prebuilds feature. ExperimentWebPush Experiment = "web-push" // Enables web push notifications through the browser. ExperimentDynamicParameters Experiment = "dynamic-parameters" // Enables dynamic parameters when creating a workspace. ) @@ -3262,7 +3307,9 @@ const ( // users to opt-in to via --experimental='*'. // Experiments that are not ready for consumption by all users should // not be included here and will be essentially hidden. -var ExperimentsAll = Experiments{} +var ExperimentsAll = Experiments{ + ExperimentWorkspacePrebuilds, +} // Experiments is a list of experiments. // Multiple experiments may be enabled at the same time. diff --git a/codersdk/organizations.go b/codersdk/organizations.go index b880f25e15a2c..4e588770a989a 100644 --- a/codersdk/organizations.go +++ b/codersdk/organizations.go @@ -224,9 +224,10 @@ type CreateWorkspaceRequest struct { TTLMillis *int64 `json:"ttl_ms,omitempty"` // RichParameterValues allows for additional parameters to be provided // during the initial provision. - RichParameterValues []WorkspaceBuildParameter `json:"rich_parameter_values,omitempty"` - AutomaticUpdates AutomaticUpdates `json:"automatic_updates,omitempty"` - TemplateVersionPresetID uuid.UUID `json:"template_version_preset_id,omitempty" format:"uuid"` + RichParameterValues []WorkspaceBuildParameter `json:"rich_parameter_values,omitempty"` + AutomaticUpdates AutomaticUpdates `json:"automatic_updates,omitempty"` + TemplateVersionPresetID uuid.UUID `json:"template_version_preset_id,omitempty" format:"uuid"` + ClaimPrebuildIfAvailable bool `json:"claim_prebuild_if_available,omitempty"` } func (c *Client) OrganizationByName(ctx context.Context, name string) (Organization, error) { diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index 6b45bc65e2c3f..73cce4a4e38e7 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -4,6 +4,9 @@ import ( "context" "crypto/ed25519" "fmt" + agplprebuilds "github.com/coder/coder/v2/coderd/prebuilds" + "github.com/coder/coder/v2/enterprise/coderd/prebuilds" + "github.com/coder/quartz" "math" "net/http" "net/url" @@ -628,6 +631,8 @@ type API struct { licenseMetricsCollector *license.MetricsCollector tailnetService *tailnet.ClientService + + PrebuildsReconciler agplprebuilds.ReconciliationOrchestrator } // writeEntitlementWarningsHeader writes the entitlement warnings to the response header @@ -658,6 +663,14 @@ func (api *API) Close() error { if api.Options.CheckInactiveUsersCancelFunc != nil { api.Options.CheckInactiveUsersCancelFunc() } + + if api.PrebuildsReconciler != nil { + ctx, giveUp := context.WithTimeoutCause(context.Background(), time.Second*30, xerrors.New("gave up waiting for reconciler to stop")) + defer giveUp() + // TODO: determine root cause (requires changes up the stack, though). + api.PrebuildsReconciler.Stop(ctx, xerrors.New("api closed")) + } + return api.AGPL.Close() } @@ -860,6 +873,20 @@ func (api *API) updateEntitlements(ctx context.Context) error { api.AGPL.PortSharer.Store(&ps) } + if initial, changed, enabled := featureChanged(codersdk.FeatureWorkspacePrebuilds); shouldUpdate(initial, changed, enabled) || api.PrebuildsReconciler == nil { + reconciler, claimer := api.setupPrebuilds(enabled) + if api.PrebuildsReconciler != nil { + stopCtx, giveUp := context.WithTimeoutCause(context.Background(), time.Second*30, xerrors.New("gave up waiting for reconciler to stop")) + defer giveUp() + api.PrebuildsReconciler.Stop(stopCtx, xerrors.New("entitlements change")) + } + + api.PrebuildsReconciler = reconciler + go reconciler.RunLoop(context.Background()) + + api.AGPL.PrebuildsClaimer.Store(&claimer) + } + // External token encryption is soft-enforced featureExternalTokenEncryption := reloadedEntitlements.Features[codersdk.FeatureExternalTokenEncryption] featureExternalTokenEncryption.Enabled = len(api.ExternalTokenEncryption) > 0 @@ -1128,3 +1155,19 @@ func (api *API) runEntitlementsLoop(ctx context.Context) { func (api *API) Authorize(r *http.Request, action policy.Action, object rbac.Objecter) bool { return api.AGPL.HTTPAuth.Authorize(r, action, object) } + +func (api *API) setupPrebuilds(entitled bool) (agplprebuilds.ReconciliationOrchestrator, agplprebuilds.Claimer) { + enabled := api.AGPL.Experiments.Enabled(codersdk.ExperimentWorkspacePrebuilds) + if !enabled || !entitled { + api.Logger.Debug(context.Background(), "prebuilds not enabled", + slog.F("experiment_enabled", enabled), slog.F("entitled", entitled)) + + return agplprebuilds.NewNoopReconciler(), agplprebuilds.DefaultClaimer + } + + reconciler := prebuilds.NewStoreReconciler(api.Database, api.Pubsub, api.DeploymentValues.Prebuilds, + api.Logger.Named("prebuilds"), quartz.NewReal()) + + return reconciler, + prebuilds.EnterpriseClaimer{} +} diff --git a/enterprise/coderd/prebuilds/claim.go b/enterprise/coderd/prebuilds/claim.go new file mode 100644 index 0000000000000..af4095eb5fba3 --- /dev/null +++ b/enterprise/coderd/prebuilds/claim.go @@ -0,0 +1,64 @@ +package prebuilds + +import ( + "context" + "database/sql" + "errors" + + "github.com/coder/coder/v2/coderd/prebuilds" + + "github.com/google/uuid" + "golang.org/x/xerrors" + + "github.com/coder/coder/v2/coderd/database" +) + +type EnterpriseClaimer struct{} + +func (_ EnterpriseClaimer) Claim( + ctx context.Context, + store database.Store, + userID uuid.UUID, + name string, + presetID uuid.UUID, +) (*uuid.UUID, error) { + var prebuildID *uuid.UUID + err := store.InTx(func(db database.Store) error { + // TODO: do we need this? + //// Ensure no other replica can claim a prebuild for this user simultaneously. + // err := store.AcquireLock(ctx, database.GenLockID(fmt.Sprintf("prebuild-user-claim-%s", userID.String()))) + // if err != nil { + // return xerrors.Errorf("acquire claim lock for user %q: %w", userID.String(), err) + //} + + result, err := db.ClaimPrebuiltWorkspace(ctx, database.ClaimPrebuiltWorkspaceParams{ + NewUserID: userID, + NewName: name, + PresetID: presetID, + }) + if err != nil { + switch { + // No eligible prebuilds found + case errors.Is(err, sql.ErrNoRows): + // Exit, this will result in a nil prebuildID being returned, which is fine + return nil + default: + return xerrors.Errorf("claim prebuild for user %q: %w", userID.String(), err) + } + } + + prebuildID = &result.ID + + return nil + }, &database.TxOptions{ + TxIdentifier: "prebuild-claim", + }) + + return prebuildID, err +} + +func (_ EnterpriseClaimer) Initiator() uuid.UUID { + return prebuilds.SystemUserID +} + +var _ prebuilds.Claimer = &EnterpriseClaimer{} diff --git a/enterprise/coderd/prebuilds/claim_test.go b/enterprise/coderd/prebuilds/claim_test.go new file mode 100644 index 0000000000000..b290fadd3fe22 --- /dev/null +++ b/enterprise/coderd/prebuilds/claim_test.go @@ -0,0 +1,427 @@ +package prebuilds_test + +import ( + "context" + "database/sql" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/coder/serpent" + + "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbauthz" + "github.com/coder/coder/v2/coderd/database/dbtestutil" + "github.com/coder/coder/v2/coderd/rbac" + "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" + "github.com/coder/coder/v2/enterprise/coderd/license" + "github.com/coder/coder/v2/enterprise/coderd/prebuilds" + "github.com/coder/coder/v2/provisioner/echo" + "github.com/coder/coder/v2/provisionersdk/proto" + "github.com/coder/coder/v2/testutil" +) + +type storeSpy struct { + database.Store + + claims *atomic.Int32 + claimParams *atomic.Pointer[database.ClaimPrebuiltWorkspaceParams] + claimedWorkspace *atomic.Pointer[database.ClaimPrebuiltWorkspaceRow] +} + +func newStoreSpy(db database.Store) *storeSpy { + return &storeSpy{ + Store: db, + claims: &atomic.Int32{}, + claimParams: &atomic.Pointer[database.ClaimPrebuiltWorkspaceParams]{}, + claimedWorkspace: &atomic.Pointer[database.ClaimPrebuiltWorkspaceRow]{}, + } +} + +func (m *storeSpy) InTx(fn func(store database.Store) error, opts *database.TxOptions) error { + // Pass spy down into transaction store. + return m.Store.InTx(func(store database.Store) error { + spy := newStoreSpy(store) + spy.claims = m.claims + spy.claimParams = m.claimParams + spy.claimedWorkspace = m.claimedWorkspace + + return fn(spy) + }, opts) +} + +func (m *storeSpy) ClaimPrebuiltWorkspace(ctx context.Context, arg database.ClaimPrebuiltWorkspaceParams) (database.ClaimPrebuiltWorkspaceRow, error) { + m.claims.Add(1) + m.claimParams.Store(&arg) + result, err := m.Store.ClaimPrebuiltWorkspace(ctx, arg) + if err == nil { + m.claimedWorkspace.Store(&result) + } + return result, err +} + +func TestClaimPrebuild(t *testing.T) { + t.Parallel() + + if !dbtestutil.WillUsePostgres() { + t.Skip("This test requires postgres") + } + + const ( + desiredInstances = 1 + presetCount = 2 + ) + + cases := map[string]struct { + entitlementEnabled bool + experimentEnabled bool + attemptPrebuildClaim bool + expectPrebuildClaimed bool + markPrebuildsClaimable bool + expectedPrebuildsCount int + }{ + "without the experiment enabled, prebuilds will not provisioned": { + experimentEnabled: false, + entitlementEnabled: true, + attemptPrebuildClaim: false, + expectedPrebuildsCount: 0, + }, + "without the entitlement, prebuilds will not provisioned": { + experimentEnabled: true, + entitlementEnabled: false, + attemptPrebuildClaim: false, + expectedPrebuildsCount: 0, + }, + "with everything enabled, but no eligible prebuilds to claim": { + entitlementEnabled: true, + experimentEnabled: true, + attemptPrebuildClaim: true, + expectPrebuildClaimed: false, + markPrebuildsClaimable: false, + expectedPrebuildsCount: desiredInstances * presetCount, + }, + "with everything enabled, claiming an eligible prebuild should succeed": { + entitlementEnabled: true, + experimentEnabled: true, + attemptPrebuildClaim: true, + expectPrebuildClaimed: true, + markPrebuildsClaimable: true, + expectedPrebuildsCount: desiredInstances * presetCount, + }, + } + + for name, tc := range cases { + tc := tc + + t.Run(name, func(t *testing.T) { + t.Parallel() + + // Setup. // TODO: abstract? + + ctx := testutil.Context(t, testutil.WaitMedium) + db, pubsub := dbtestutil.NewDB(t) + spy := newStoreSpy(db) + + var prebuildsEntitled int64 + if tc.entitlementEnabled { + prebuildsEntitled = 1 + } + + client, _, api, owner := coderdenttest.NewWithAPI(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + IncludeProvisionerDaemon: true, + Database: spy, + Pubsub: pubsub, + DeploymentValues: coderdtest.DeploymentValues(t, func(values *codersdk.DeploymentValues) { + values.Prebuilds.ReconciliationInterval = serpent.Duration(time.Hour) // We will kick off a reconciliation manually. + + if tc.experimentEnabled { + values.Experiments = serpent.StringArray{string(codersdk.ExperimentWorkspacePrebuilds)} + } + }), + }, + + EntitlementsUpdateInterval: time.Second, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureWorkspacePrebuilds: prebuildsEntitled, + }, + }, + }) + reconciler := api.PrebuildsReconciler + + // The entitlements will need to refresh before the reconciler is set. + require.Eventually(t, func() bool { + if tc.entitlementEnabled && tc.experimentEnabled { + assert.IsType(t, &prebuilds.StoreReconciler{}, reconciler) + } + + return reconciler != nil + }, testutil.WaitSuperLong, testutil.IntervalFast) + + version := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, templateWithAgentAndPresetsWithPrebuilds(desiredInstances)) + _ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) + coderdtest.CreateTemplate(t, client, owner.OrganizationID, version.ID) + presets, err := client.TemplateVersionPresets(ctx, version.ID) + require.NoError(t, err) + require.Len(t, presets, presetCount) + + userClient, user := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.RoleMember()) + + ctx = dbauthz.AsPrebuildsOrchestrator(ctx) + + // Given: the reconciliation state is snapshot. + state, err := reconciler.SnapshotState(ctx, spy) + require.NoError(t, err) + + // When: the experiment or entitlement is not preset, there should be nothing to reconcile. + if !tc.entitlementEnabled || !tc.experimentEnabled { + require.Len(t, state.Presets, 0) + return + } + + require.Len(t, state.Presets, presetCount) + // When: a reconciliation is setup for each preset. + for _, preset := range presets { + ps, err := state.FilterByPreset(preset.ID) + require.NoError(t, err) + require.NotNil(t, ps) + actions, err := reconciler.CalculateActions(ctx, *ps) + require.NoError(t, err) + require.NotNil(t, actions) + + require.NoError(t, reconciler.ReconcilePreset(ctx, *ps)) + } + + // Given: a set of running, eligible prebuilds eventually starts up. + runningPrebuilds := make(map[uuid.UUID]database.GetRunningPrebuiltWorkspacesRow, desiredInstances*presetCount) + require.Eventually(t, func() bool { + rows, err := spy.GetRunningPrebuiltWorkspaces(ctx) + require.NoError(t, err) + + for _, row := range rows { + runningPrebuilds[row.CurrentPresetID.UUID] = row + + if !tc.markPrebuildsClaimable { + continue + } + + agents, err := db.GetWorkspaceAgentsInLatestBuildByWorkspaceID(ctx, row.ID) + require.NoError(t, err) + + for _, agent := range agents { + require.NoError(t, db.UpdateWorkspaceAgentLifecycleStateByID(ctx, database.UpdateWorkspaceAgentLifecycleStateByIDParams{ + ID: agent.ID, + LifecycleState: database.WorkspaceAgentLifecycleStateReady, + StartedAt: sql.NullTime{Time: time.Now().Add(time.Hour), Valid: true}, + ReadyAt: sql.NullTime{Time: time.Now().Add(-1 * time.Hour), Valid: true}, + })) + } + } + + t.Logf("found %d running prebuilds so far, want %d", len(runningPrebuilds), tc.expectedPrebuildsCount) + + return len(runningPrebuilds) == tc.expectedPrebuildsCount + }, testutil.WaitSuperLong, testutil.IntervalSlow) + + // When: a user creates a new workspace with a preset for which prebuilds are configured. + workspaceName := strings.ReplaceAll(testutil.GetRandomName(t), "_", "-") + params := database.ClaimPrebuiltWorkspaceParams{ + NewUserID: user.ID, + NewName: workspaceName, + PresetID: presets[0].ID, + } + userWorkspace, err := userClient.CreateUserWorkspace(ctx, user.Username, codersdk.CreateWorkspaceRequest{ + TemplateVersionID: version.ID, + Name: workspaceName, + TemplateVersionPresetID: presets[0].ID, + ClaimPrebuildIfAvailable: true, // TODO: doesn't do anything yet; it probably should though. + }) + require.NoError(t, err) + coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, userWorkspace.LatestBuild.ID) + + // Then: if we're not expecting any prebuild claims to succeed, handle this specifically. + if !tc.attemptPrebuildClaim { + require.EqualValues(t, spy.claims.Load(), 0) + require.Nil(t, spy.claimedWorkspace.Load()) + + currentPrebuilds, err := spy.GetRunningPrebuiltWorkspaces(ctx) + require.NoError(t, err) + // The number of prebuilds should NOT change. + require.Equal(t, len(currentPrebuilds), len(runningPrebuilds)) + return + } + + // Then: a prebuild should have been claimed. + require.EqualValues(t, spy.claims.Load(), 1) + require.NotNil(t, spy.claims.Load()) + require.EqualValues(t, *spy.claimParams.Load(), params) + + if !tc.expectPrebuildClaimed { + require.Nil(t, spy.claimedWorkspace.Load()) + return + } + + require.NotNil(t, spy.claimedWorkspace.Load()) + claimed := *spy.claimedWorkspace.Load() + require.NotEqual(t, claimed, uuid.Nil) + + // Then: the claimed prebuild must now be owned by the requester. + workspace, err := spy.GetWorkspaceByID(ctx, claimed.ID) + require.NoError(t, err) + require.Equal(t, user.ID, workspace.OwnerID) + + // Then: the number of running prebuilds has changed since one was claimed. + currentPrebuilds, err := spy.GetRunningPrebuiltWorkspaces(ctx) + require.NoError(t, err) + require.NotEqual(t, len(currentPrebuilds), len(runningPrebuilds)) + + // Then: the claimed prebuild is now missing from the running prebuilds set. + current, err := spy.GetRunningPrebuiltWorkspaces(ctx) + require.NoError(t, err) + + var found bool + for _, prebuild := range current { + if prebuild.ID == claimed.ID { + found = true + break + } + } + require.False(t, found, "claimed prebuild should not still be considered a running prebuild") + + // Then: reconciling at this point will provision a new prebuild to replace the claimed one. + { + // Given: the reconciliation state is snapshot. + state, err = reconciler.SnapshotState(ctx, spy) + require.NoError(t, err) + + // When: a reconciliation is setup for each preset. + for _, preset := range presets { + ps, err := state.FilterByPreset(preset.ID) + require.NoError(t, err) + + // Then: the reconciliation takes place without error. + require.NoError(t, reconciler.ReconcilePreset(ctx, *ps)) + } + } + + require.Eventually(t, func() bool { + rows, err := spy.GetRunningPrebuiltWorkspaces(ctx) + require.NoError(t, err) + + t.Logf("found %d running prebuilds so far, want %d", len(rows), tc.expectedPrebuildsCount) + + return len(runningPrebuilds) == tc.expectedPrebuildsCount + }, testutil.WaitSuperLong, testutil.IntervalSlow) + + // Then: when restarting the created workspace (which claimed a prebuild), it should not try and claim a new prebuild. + // Prebuilds should ONLY be used for net-new workspaces. + // This is expected by default anyway currently since new workspaces and operations on existing workspaces + // take different code paths, but it's worth validating. + + spy.claims.Store(0) // Reset counter because we need to check if any new claim requests happen. + + wp, err := userClient.WorkspaceBuildParameters(ctx, userWorkspace.LatestBuild.ID) + require.NoError(t, err) + + stopBuild, err := userClient.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{ + TemplateVersionID: version.ID, + Transition: codersdk.WorkspaceTransitionStop, + RichParameterValues: wp, + }) + require.NoError(t, err) + coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, stopBuild.ID) + + startBuild, err := userClient.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{ + TemplateVersionID: version.ID, + Transition: codersdk.WorkspaceTransitionStart, + RichParameterValues: wp, + }) + require.NoError(t, err) + coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, startBuild.ID) + + require.Zero(t, spy.claims.Load()) + }) + } +} + +func templateWithAgentAndPresetsWithPrebuilds(desiredInstances int32) *echo.Responses { + return &echo.Responses{ + Parse: echo.ParseComplete, + ProvisionPlan: []*proto.Response{ + { + Type: &proto.Response_Plan{ + Plan: &proto.PlanComplete{ + Resources: []*proto.Resource{ + { + Type: "compute", + Name: "main", + Agents: []*proto.Agent{ + { + Name: "smith", + OperatingSystem: "linux", + Architecture: "i386", + }, + }, + }, + }, + Presets: []*proto.Preset{ + { + Name: "preset-a", + Parameters: []*proto.PresetParameter{ + { + Name: "k1", + Value: "v1", + }, + }, + Prebuild: &proto.Prebuild{ + Instances: desiredInstances, + }, + }, + { + Name: "preset-b", + Parameters: []*proto.PresetParameter{ + { + Name: "k1", + Value: "v2", + }, + }, + Prebuild: &proto.Prebuild{ + Instances: desiredInstances, + }, + }, + }, + }, + }, + }, + }, + ProvisionApply: []*proto.Response{ + { + Type: &proto.Response_Apply{ + Apply: &proto.ApplyComplete{ + Resources: []*proto.Resource{ + { + Type: "compute", + Name: "main", + Agents: []*proto.Agent{ + { + Name: "smith", + OperatingSystem: "linux", + Architecture: "i386", + }, + }, + }, + }, + }, + }, + }, + }, + } +} From 217e46fc35220f673fe4ccab5e9f1227b92f24d7 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Mon, 21 Apr 2025 18:20:02 +0200 Subject: [PATCH 02/34] Reverting unncessary changes Signed-off-by: Danny Kopping --- coderd/apidoc/docs.go | 3 ++ coderd/apidoc/swagger.json | 3 ++ .../provisionerdserver/provisionerdserver.go | 9 +--- coderd/workspaces.go | 19 ++----- coderd/wsbuilder/wsbuilder.go | 9 ---- codersdk/deployment.go | 49 +------------------ docs/reference/api/schemas.md | 22 +++++---- docs/reference/api/workspaces.md | 2 + enterprise/coderd/coderd.go | 43 ---------------- enterprise/coderd/prebuilds/claim_test.go | 37 +++++++------- site/src/api/typesGenerated.ts | 1 + 11 files changed, 45 insertions(+), 152 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 268cfd7a894ba..fb572298d032a 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -11453,6 +11453,9 @@ const docTemplate = `{ "autostart_schedule": { "type": "string" }, + "claim_prebuild_if_available": { + "type": "boolean" + }, "name": { "type": "string" }, diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index e973f11849547..990723b28b694 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -10211,6 +10211,9 @@ "autostart_schedule": { "type": "string" }, + "claim_prebuild_if_available": { + "type": "boolean" + }, "name": { "type": "string" }, diff --git a/coderd/provisionerdserver/provisionerdserver.go b/coderd/provisionerdserver/provisionerdserver.go index c409339371d9e..2fbd3bdcbeb57 100644 --- a/coderd/provisionerdserver/provisionerdserver.go +++ b/coderd/provisionerdserver/provisionerdserver.go @@ -2473,14 +2473,7 @@ type WorkspaceProvisionJob struct { DryRun bool `json:"dry_run"` IsPrebuild bool `json:"is_prebuild,omitempty"` PrebuildClaimedByUser uuid.UUID `json:"prebuild_claimed_by,omitempty"` - // RunningWorkspaceAgentID is *only* used for prebuilds. We pass it down when we want to rebuild a prebuilt workspace - // but not generate a new agent token. The provisionerdserver will retrieve this token and push it down to - // the provisioner (and ultimately to the `coder_agent` resource in the Terraform provider) where it will be - // reused. Context: the agent token is often used in immutable attributes of workspace resource (e.g. VM/container) - // to initialize the agent, so if that value changes it will necessitate a replacement of that resource, thus - // obviating the whole point of the prebuild. - RunningWorkspaceAgentID uuid.UUID `json:"running_workspace_agent_id"` - LogLevel string `json:"log_level,omitempty"` + LogLevel string `json:"log_level,omitempty"` } // TemplateVersionDryRunJob is the payload for the "template_version_dry_run" job type. diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 71d02c4d8b7c7..8cfd45c8d2fb7 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -6,12 +6,13 @@ import ( "encoding/json" "errors" "fmt" - "github.com/coder/coder/v2/coderd/prebuilds" "net/http" "slices" "strconv" "time" + "github.com/coder/coder/v2/coderd/prebuilds" + "github.com/dustin/go-humanize" "github.com/go-chi/chi/v5" "github.com/google/uuid" @@ -19,6 +20,7 @@ import ( "golang.org/x/xerrors" "cdr.dev/slog" + "github.com/coder/coder/v2/agent/proto" "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/database" @@ -636,8 +638,6 @@ func createWorkspace( provisionerJob *database.ProvisionerJob workspaceBuild *database.WorkspaceBuild provisionerDaemons []database.GetEligibleProvisionerDaemonsByProvisionerJobIDsRow - - runningWorkspaceAgentID uuid.UUID ) prebuilds := (*api.PrebuildsClaimer.Load()).(prebuilds.Claimer) @@ -685,16 +685,6 @@ func createWorkspace( // Prebuild found! workspaceID = claimedWorkspace.ID initiatorID = prebuilds.Initiator() - agents, err := db.GetWorkspaceAgentsInLatestBuildByWorkspaceID(ctx, claimedWorkspace.ID) - if err != nil { - // TODO: comment about best-effort, workspace can be restarted if this fails... - api.Logger.Error(ctx, "failed to retrieve running agents of claimed prebuilt workspace", - slog.F("workspace_id", claimedWorkspace.ID), slog.Error(err)) - } - if len(agents) >= 1 { - // TODO: handle multiple agents - runningWorkspaceAgentID = agents[0].ID - } } // We have to refetch the workspace for the joined in fields. @@ -710,8 +700,7 @@ func createWorkspace( Initiator(initiatorID). ActiveVersion(). RichParameterValues(req.RichParameterValues). - TemplateVersionPresetID(req.TemplateVersionPresetID). - RunningWorkspaceAgentID(runningWorkspaceAgentID) + TemplateVersionPresetID(req.TemplateVersionPresetID) if req.TemplateVersionID != uuid.Nil { builder = builder.VersionID(req.TemplateVersionID) } diff --git a/coderd/wsbuilder/wsbuilder.go b/coderd/wsbuilder/wsbuilder.go index 136afe8906d06..c854f1ba01f4f 100644 --- a/coderd/wsbuilder/wsbuilder.go +++ b/coderd/wsbuilder/wsbuilder.go @@ -77,7 +77,6 @@ type Builder struct { prebuild bool prebuildClaimedBy uuid.UUID - runningWorkspaceAgentID uuid.UUID verifyNoLegacyParametersOnce bool } @@ -186,13 +185,6 @@ func (b Builder) MarkPrebuildClaimedBy(userID uuid.UUID) Builder { return b } -// RunningWorkspaceAgentID is only used for prebuilds; see the associated field in `provisionerdserver.WorkspaceProvisionJob`. -func (b Builder) RunningWorkspaceAgentID(id uuid.UUID) Builder { - // nolint: revive - b.runningWorkspaceAgentID = id - return b -} - // SetLastWorkspaceBuildInTx prepopulates the Builder's cache with the last workspace build. This allows us // to avoid a repeated database query when the Builder's caller also needs the workspace build, e.g. auto-start & // auto-stop. @@ -328,7 +320,6 @@ func (b *Builder) buildTx(authFunc func(action policy.Action, object rbac.Object LogLevel: b.logLevel, IsPrebuild: b.prebuild, PrebuildClaimedByUser: b.prebuildClaimedBy, - RunningWorkspaceAgentID: b.runningWorkspaceAgentID, }) if err != nil { return nil, nil, nil, BuildError{ diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 17d3f5990ea9c..8b447e2c96e06 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -81,7 +81,6 @@ const ( FeatureControlSharedPorts FeatureName = "control_shared_ports" FeatureCustomRoles FeatureName = "custom_roles" FeatureMultipleOrganizations FeatureName = "multiple_organizations" - FeatureWorkspacePrebuilds FeatureName = "workspace_prebuilds" ) // FeatureNames must be kept in-sync with the Feature enum above. @@ -104,7 +103,6 @@ var FeatureNames = []FeatureName{ FeatureControlSharedPorts, FeatureCustomRoles, FeatureMultipleOrganizations, - FeatureWorkspacePrebuilds, } // Humanize returns the feature name in a human-readable format. @@ -134,7 +132,6 @@ func (n FeatureName) AlwaysEnable() bool { FeatureHighAvailability: true, FeatureCustomRoles: true, FeatureMultipleOrganizations: true, - FeatureWorkspacePrebuilds: true, }[n] } @@ -396,7 +393,6 @@ type DeploymentValues struct { TermsOfServiceURL serpent.String `json:"terms_of_service_url,omitempty" typescript:",notnull"` Notifications NotificationsConfig `json:"notifications,omitempty" typescript:",notnull"` AdditionalCSPPolicy serpent.StringArray `json:"additional_csp_policy,omitempty" typescript:",notnull"` - Prebuilds PrebuildsConfig `json:"workspace_prebuilds,omitempty" typescript:",notnull"` WorkspaceHostnameSuffix serpent.String `json:"workspace_hostname_suffix,omitempty" typescript:",notnull"` Config serpent.YAMLConfigPath `json:"config,omitempty" typescript:",notnull"` @@ -1038,11 +1034,6 @@ func (c *DeploymentValues) Options() serpent.OptionSet { Parent: &deploymentGroupNotifications, YAML: "webhook", } - deploymentGroupPrebuilds = serpent.Group{ - Name: "Workspace Prebuilds", - YAML: "workspace_prebuilds", - Description: "Configure how workspace prebuilds behave.", - } deploymentGroupInbox = serpent.Group{ Name: "Inbox", Parent: &deploymentGroupNotifications, @@ -3038,41 +3029,6 @@ Write out the current server config as YAML to stdout.`, Annotations: serpent.Annotations{}.Mark(annotationFormatDuration, "true"), Hidden: true, // Hidden because most operators should not need to modify this. }, - { - Name: "Reconciliation Interval", - Description: "How often to reconcile workspace prebuilds state.", - Flag: "workspace-prebuilds-reconciliation-interval", - Env: "CODER_WORKSPACE_PREBUILDS_RECONCILIATION_INTERVAL", - Value: &c.Prebuilds.ReconciliationInterval, - Default: (time.Second * 15).String(), - Group: &deploymentGroupPrebuilds, - YAML: "reconciliation_interval", - Annotations: serpent.Annotations{}.Mark(annotationFormatDuration, "true"), - }, - { - Name: "Reconciliation Backoff Interval", - Description: "Interval to increase reconciliation backoff by when unrecoverable errors occur.", - Flag: "workspace-prebuilds-reconciliation-backoff-interval", - Env: "CODER_WORKSPACE_PREBUILDS_RECONCILIATION_BACKOFF_INTERVAL", - Value: &c.Prebuilds.ReconciliationBackoffInterval, - Default: (time.Second * 15).String(), - Group: &deploymentGroupPrebuilds, - YAML: "reconciliation_backoff_interval", - Annotations: serpent.Annotations{}.Mark(annotationFormatDuration, "true"), - Hidden: true, - }, - { - Name: "Reconciliation Backoff Lookback Period", - Description: "Interval to look back to determine number of failed builds, which influences backoff.", - Flag: "workspace-prebuilds-reconciliation-backoff-lookback-period", - Env: "CODER_WORKSPACE_PREBUILDS_RECONCILIATION_BACKOFF_LOOKBACK_PERIOD", - Value: &c.Prebuilds.ReconciliationBackoffLookback, - Default: (time.Hour).String(), // TODO: use https://pkg.go.dev/github.com/jackc/pgtype@v1.12.0#Interval - Group: &deploymentGroupPrebuilds, - YAML: "reconciliation_backoff_lookback_period", - Annotations: serpent.Annotations{}.Mark(annotationFormatDuration, "true"), - Hidden: true, - }, // Push notifications. } @@ -3298,7 +3254,6 @@ const ( ExperimentAutoFillParameters Experiment = "auto-fill-parameters" // This should not be taken out of experiments until we have redesigned the feature. ExperimentNotifications Experiment = "notifications" // Sends notifications via SMTP and webhooks following certain events. ExperimentWorkspaceUsage Experiment = "workspace-usage" // Enables the new workspace usage tracking. - ExperimentWorkspacePrebuilds Experiment = "workspace-prebuilds" // Enables the new workspace prebuilds feature. ExperimentWebPush Experiment = "web-push" // Enables web push notifications through the browser. ExperimentDynamicParameters Experiment = "dynamic-parameters" // Enables dynamic parameters when creating a workspace. ) @@ -3307,9 +3262,7 @@ const ( // users to opt-in to via --experimental='*'. // Experiments that are not ready for consumption by all users should // not be included here and will be essentially hidden. -var ExperimentsAll = Experiments{ - ExperimentWorkspacePrebuilds, -} +var ExperimentsAll = Experiments{} // Experiments is a list of experiments. // Multiple experiments may be enabled at the same time. diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index 79d7a411bf98c..82682a3708a0f 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -1462,6 +1462,7 @@ None { "automatic_updates": "always", "autostart_schedule": "string", + "claim_prebuild_if_available": true, "name": "string", "rich_parameter_values": [ { @@ -1480,16 +1481,17 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o ### Properties -| Name | Type | Required | Restrictions | Description | -|------------------------------|-------------------------------------------------------------------------------|----------|--------------|---------------------------------------------------------------------------------------------------------| -| `automatic_updates` | [codersdk.AutomaticUpdates](#codersdkautomaticupdates) | false | | | -| `autostart_schedule` | string | false | | | -| `name` | string | true | | | -| `rich_parameter_values` | array of [codersdk.WorkspaceBuildParameter](#codersdkworkspacebuildparameter) | false | | Rich parameter values allows for additional parameters to be provided during the initial provision. | -| `template_id` | string | false | | Template ID specifies which template should be used for creating the workspace. | -| `template_version_id` | string | false | | Template version ID can be used to specify a specific version of a template for creating the workspace. | -| `template_version_preset_id` | string | false | | | -| `ttl_ms` | integer | false | | | +| Name | Type | Required | Restrictions | Description | +|-------------------------------|-------------------------------------------------------------------------------|----------|--------------|---------------------------------------------------------------------------------------------------------| +| `automatic_updates` | [codersdk.AutomaticUpdates](#codersdkautomaticupdates) | false | | | +| `autostart_schedule` | string | false | | | +| `claim_prebuild_if_available` | boolean | false | | | +| `name` | string | true | | | +| `rich_parameter_values` | array of [codersdk.WorkspaceBuildParameter](#codersdkworkspacebuildparameter) | false | | Rich parameter values allows for additional parameters to be provided during the initial provision. | +| `template_id` | string | false | | Template ID specifies which template should be used for creating the workspace. | +| `template_version_id` | string | false | | Template version ID can be used to specify a specific version of a template for creating the workspace. | +| `template_version_preset_id` | string | false | | | +| `ttl_ms` | integer | false | | | ## codersdk.CryptoKey diff --git a/docs/reference/api/workspaces.md b/docs/reference/api/workspaces.md index 5e727cee297fe..b5c5c6dfa524d 100644 --- a/docs/reference/api/workspaces.md +++ b/docs/reference/api/workspaces.md @@ -25,6 +25,7 @@ of the template will be used. { "automatic_updates": "always", "autostart_schedule": "string", + "claim_prebuild_if_available": true, "name": "string", "rich_parameter_values": [ { @@ -605,6 +606,7 @@ of the template will be used. { "automatic_updates": "always", "autostart_schedule": "string", + "claim_prebuild_if_available": true, "name": "string", "rich_parameter_values": [ { diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index 73cce4a4e38e7..6b45bc65e2c3f 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -4,9 +4,6 @@ import ( "context" "crypto/ed25519" "fmt" - agplprebuilds "github.com/coder/coder/v2/coderd/prebuilds" - "github.com/coder/coder/v2/enterprise/coderd/prebuilds" - "github.com/coder/quartz" "math" "net/http" "net/url" @@ -631,8 +628,6 @@ type API struct { licenseMetricsCollector *license.MetricsCollector tailnetService *tailnet.ClientService - - PrebuildsReconciler agplprebuilds.ReconciliationOrchestrator } // writeEntitlementWarningsHeader writes the entitlement warnings to the response header @@ -663,14 +658,6 @@ func (api *API) Close() error { if api.Options.CheckInactiveUsersCancelFunc != nil { api.Options.CheckInactiveUsersCancelFunc() } - - if api.PrebuildsReconciler != nil { - ctx, giveUp := context.WithTimeoutCause(context.Background(), time.Second*30, xerrors.New("gave up waiting for reconciler to stop")) - defer giveUp() - // TODO: determine root cause (requires changes up the stack, though). - api.PrebuildsReconciler.Stop(ctx, xerrors.New("api closed")) - } - return api.AGPL.Close() } @@ -873,20 +860,6 @@ func (api *API) updateEntitlements(ctx context.Context) error { api.AGPL.PortSharer.Store(&ps) } - if initial, changed, enabled := featureChanged(codersdk.FeatureWorkspacePrebuilds); shouldUpdate(initial, changed, enabled) || api.PrebuildsReconciler == nil { - reconciler, claimer := api.setupPrebuilds(enabled) - if api.PrebuildsReconciler != nil { - stopCtx, giveUp := context.WithTimeoutCause(context.Background(), time.Second*30, xerrors.New("gave up waiting for reconciler to stop")) - defer giveUp() - api.PrebuildsReconciler.Stop(stopCtx, xerrors.New("entitlements change")) - } - - api.PrebuildsReconciler = reconciler - go reconciler.RunLoop(context.Background()) - - api.AGPL.PrebuildsClaimer.Store(&claimer) - } - // External token encryption is soft-enforced featureExternalTokenEncryption := reloadedEntitlements.Features[codersdk.FeatureExternalTokenEncryption] featureExternalTokenEncryption.Enabled = len(api.ExternalTokenEncryption) > 0 @@ -1155,19 +1128,3 @@ func (api *API) runEntitlementsLoop(ctx context.Context) { func (api *API) Authorize(r *http.Request, action policy.Action, object rbac.Objecter) bool { return api.AGPL.HTTPAuth.Authorize(r, action, object) } - -func (api *API) setupPrebuilds(entitled bool) (agplprebuilds.ReconciliationOrchestrator, agplprebuilds.Claimer) { - enabled := api.AGPL.Experiments.Enabled(codersdk.ExperimentWorkspacePrebuilds) - if !enabled || !entitled { - api.Logger.Debug(context.Background(), "prebuilds not enabled", - slog.F("experiment_enabled", enabled), slog.F("entitled", entitled)) - - return agplprebuilds.NewNoopReconciler(), agplprebuilds.DefaultClaimer - } - - reconciler := prebuilds.NewStoreReconciler(api.Database, api.Pubsub, api.DeploymentValues.Prebuilds, - api.Logger.Named("prebuilds"), quartz.NewReal()) - - return reconciler, - prebuilds.EnterpriseClaimer{} -} diff --git a/enterprise/coderd/prebuilds/claim_test.go b/enterprise/coderd/prebuilds/claim_test.go index b290fadd3fe22..e13978a8bdb0e 100644 --- a/enterprise/coderd/prebuilds/claim_test.go +++ b/enterprise/coderd/prebuilds/claim_test.go @@ -8,12 +8,11 @@ import ( "testing" "time" + "github.com/coder/quartz" "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/coder/serpent" - "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbauthz" @@ -21,7 +20,6 @@ import ( "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" - "github.com/coder/coder/v2/enterprise/coderd/license" "github.com/coder/coder/v2/enterprise/coderd/prebuilds" "github.com/coder/coder/v2/provisioner/echo" "github.com/coder/coder/v2/provisionersdk/proto" @@ -129,33 +127,34 @@ func TestClaimPrebuild(t *testing.T) { db, pubsub := dbtestutil.NewDB(t) spy := newStoreSpy(db) - var prebuildsEntitled int64 - if tc.entitlementEnabled { - prebuildsEntitled = 1 - } + //var prebuildsEntitled int64 + //if tc.entitlementEnabled { + // prebuildsEntitled = 1 + //} - client, _, api, owner := coderdenttest.NewWithAPI(t, &coderdenttest.Options{ + logger := testutil.Logger(t) + client, _, _, owner := coderdenttest.NewWithAPI(t, &coderdenttest.Options{ Options: &coderdtest.Options{ IncludeProvisionerDaemon: true, Database: spy, Pubsub: pubsub, DeploymentValues: coderdtest.DeploymentValues(t, func(values *codersdk.DeploymentValues) { - values.Prebuilds.ReconciliationInterval = serpent.Duration(time.Hour) // We will kick off a reconciliation manually. - - if tc.experimentEnabled { - values.Experiments = serpent.StringArray{string(codersdk.ExperimentWorkspacePrebuilds)} - } + //values.Prebuilds.ReconciliationInterval = serpent.Duration(time.Hour) // We will kick off a reconciliation manually. + // + //if tc.experimentEnabled { + // values.Experiments = serpent.StringArray{string(codersdk.ExperimentWorkspacePrebuilds)} + //} }), }, EntitlementsUpdateInterval: time.Second, - LicenseOptions: &coderdenttest.LicenseOptions{ - Features: license.Features{ - codersdk.FeatureWorkspacePrebuilds: prebuildsEntitled, - }, - }, + //LicenseOptions: &coderdenttest.LicenseOptions{ + // Features: license.Features{ + // codersdk.FeatureWorkspacePrebuilds: prebuildsEntitled, + // }, + //}, }) - reconciler := api.PrebuildsReconciler + reconciler := prebuilds.NewStoreReconciler(spy, pubsub, codersdk.PrebuildsConfig{}, logger, quartz.NewMock(t)) // The entitlements will need to refresh before the reconciler is set. require.Eventually(t, func() bool { diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index d160b7683e92e..df437ae7db4e6 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -467,6 +467,7 @@ export interface CreateWorkspaceRequest { readonly rich_parameter_values?: readonly WorkspaceBuildParameter[]; readonly automatic_updates?: AutomaticUpdates; readonly template_version_preset_id?: string; + readonly claim_prebuild_if_available?: boolean; } // From codersdk/deployment.go From c4595336d23e59f2de7d3d475ce67d257251dabf Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Tue, 22 Apr 2025 09:51:54 +0200 Subject: [PATCH 03/34] Refactoring Signed-off-by: Danny Kopping --- coderd/workspaces.go | 2 +- enterprise/coderd/prebuilds/claim_test.go | 99 +++++------------------ 2 files changed, 21 insertions(+), 80 deletions(-) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 8cfd45c8d2fb7..ea2c8a8ae3baf 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -640,7 +640,7 @@ func createWorkspace( provisionerDaemons []database.GetEligibleProvisionerDaemonsByProvisionerJobIDsRow ) - prebuilds := (*api.PrebuildsClaimer.Load()).(prebuilds.Claimer) + prebuilds := *api.PrebuildsClaimer.Load() err = api.Database.InTx(func(db database.Store) error { var ( diff --git a/enterprise/coderd/prebuilds/claim_test.go b/enterprise/coderd/prebuilds/claim_test.go index e13978a8bdb0e..2eabc2baea26c 100644 --- a/enterprise/coderd/prebuilds/claim_test.go +++ b/enterprise/coderd/prebuilds/claim_test.go @@ -10,13 +10,13 @@ import ( "github.com/coder/quartz" "github.com/google/uuid" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbtestutil" + agplprebuilds "github.com/coder/coder/v2/coderd/prebuilds" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/enterprise/coderd/coderdenttest" @@ -78,40 +78,20 @@ func TestClaimPrebuild(t *testing.T) { ) cases := map[string]struct { - entitlementEnabled bool - experimentEnabled bool - attemptPrebuildClaim bool expectPrebuildClaimed bool markPrebuildsClaimable bool - expectedPrebuildsCount int }{ - "without the experiment enabled, prebuilds will not provisioned": { - experimentEnabled: false, - entitlementEnabled: true, - attemptPrebuildClaim: false, - expectedPrebuildsCount: 0, - }, - "without the entitlement, prebuilds will not provisioned": { - experimentEnabled: true, - entitlementEnabled: false, - attemptPrebuildClaim: false, - expectedPrebuildsCount: 0, - }, - "with everything enabled, but no eligible prebuilds to claim": { - entitlementEnabled: true, - experimentEnabled: true, - attemptPrebuildClaim: true, + "no eligible prebuilds to claim": { expectPrebuildClaimed: false, markPrebuildsClaimable: false, - expectedPrebuildsCount: desiredInstances * presetCount, }, - "with everything enabled, claiming an eligible prebuild should succeed": { - entitlementEnabled: true, - experimentEnabled: true, - attemptPrebuildClaim: true, + "claiming an eligible prebuild should succeed": { + expectPrebuildClaimed: true, + markPrebuildsClaimable: true, + }, + "claiming an eligible prebuild results in error": { expectPrebuildClaimed: true, markPrebuildsClaimable: true, - expectedPrebuildsCount: desiredInstances * presetCount, }, } @@ -121,49 +101,26 @@ func TestClaimPrebuild(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - // Setup. // TODO: abstract? - + // Setup. ctx := testutil.Context(t, testutil.WaitMedium) db, pubsub := dbtestutil.NewDB(t) spy := newStoreSpy(db) - - //var prebuildsEntitled int64 - //if tc.entitlementEnabled { - // prebuildsEntitled = 1 - //} + expectedPrebuildsCount := desiredInstances * presetCount logger := testutil.Logger(t) - client, _, _, owner := coderdenttest.NewWithAPI(t, &coderdenttest.Options{ + client, _, api, owner := coderdenttest.NewWithAPI(t, &coderdenttest.Options{ Options: &coderdtest.Options{ IncludeProvisionerDaemon: true, Database: spy, Pubsub: pubsub, - DeploymentValues: coderdtest.DeploymentValues(t, func(values *codersdk.DeploymentValues) { - //values.Prebuilds.ReconciliationInterval = serpent.Duration(time.Hour) // We will kick off a reconciliation manually. - // - //if tc.experimentEnabled { - // values.Experiments = serpent.StringArray{string(codersdk.ExperimentWorkspacePrebuilds)} - //} - }), }, EntitlementsUpdateInterval: time.Second, - //LicenseOptions: &coderdenttest.LicenseOptions{ - // Features: license.Features{ - // codersdk.FeatureWorkspacePrebuilds: prebuildsEntitled, - // }, - //}, }) - reconciler := prebuilds.NewStoreReconciler(spy, pubsub, codersdk.PrebuildsConfig{}, logger, quartz.NewMock(t)) - - // The entitlements will need to refresh before the reconciler is set. - require.Eventually(t, func() bool { - if tc.entitlementEnabled && tc.experimentEnabled { - assert.IsType(t, &prebuilds.StoreReconciler{}, reconciler) - } - return reconciler != nil - }, testutil.WaitSuperLong, testutil.IntervalFast) + reconciler := prebuilds.NewStoreReconciler(spy, pubsub, codersdk.PrebuildsConfig{}, logger, quartz.NewMock(t)) + var claimer agplprebuilds.Claimer = &prebuilds.EnterpriseClaimer{} + api.AGPL.PrebuildsClaimer.Store(&claimer) version := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, templateWithAgentAndPresetsWithPrebuilds(desiredInstances)) _ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) @@ -179,14 +136,8 @@ func TestClaimPrebuild(t *testing.T) { // Given: the reconciliation state is snapshot. state, err := reconciler.SnapshotState(ctx, spy) require.NoError(t, err) - - // When: the experiment or entitlement is not preset, there should be nothing to reconcile. - if !tc.entitlementEnabled || !tc.experimentEnabled { - require.Len(t, state.Presets, 0) - return - } - require.Len(t, state.Presets, presetCount) + // When: a reconciliation is setup for each preset. for _, preset := range presets { ps, err := state.FilterByPreset(preset.ID) @@ -215,6 +166,7 @@ func TestClaimPrebuild(t *testing.T) { agents, err := db.GetWorkspaceAgentsInLatestBuildByWorkspaceID(ctx, row.ID) require.NoError(t, err) + // Workspaces are eligible once its agent is marked "ready". for _, agent := range agents { require.NoError(t, db.UpdateWorkspaceAgentLifecycleStateByID(ctx, database.UpdateWorkspaceAgentLifecycleStateByIDParams{ ID: agent.ID, @@ -225,9 +177,9 @@ func TestClaimPrebuild(t *testing.T) { } } - t.Logf("found %d running prebuilds so far, want %d", len(runningPrebuilds), tc.expectedPrebuildsCount) + t.Logf("found %d running prebuilds so far, want %d", len(runningPrebuilds), expectedPrebuildsCount) - return len(runningPrebuilds) == tc.expectedPrebuildsCount + return len(runningPrebuilds) == expectedPrebuildsCount }, testutil.WaitSuperLong, testutil.IntervalSlow) // When: a user creates a new workspace with a preset for which prebuilds are configured. @@ -243,21 +195,10 @@ func TestClaimPrebuild(t *testing.T) { TemplateVersionPresetID: presets[0].ID, ClaimPrebuildIfAvailable: true, // TODO: doesn't do anything yet; it probably should though. }) + require.NoError(t, err) coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, userWorkspace.LatestBuild.ID) - // Then: if we're not expecting any prebuild claims to succeed, handle this specifically. - if !tc.attemptPrebuildClaim { - require.EqualValues(t, spy.claims.Load(), 0) - require.Nil(t, spy.claimedWorkspace.Load()) - - currentPrebuilds, err := spy.GetRunningPrebuiltWorkspaces(ctx) - require.NoError(t, err) - // The number of prebuilds should NOT change. - require.Equal(t, len(currentPrebuilds), len(runningPrebuilds)) - return - } - // Then: a prebuild should have been claimed. require.EqualValues(t, spy.claims.Load(), 1) require.NotNil(t, spy.claims.Load()) @@ -315,9 +256,9 @@ func TestClaimPrebuild(t *testing.T) { rows, err := spy.GetRunningPrebuiltWorkspaces(ctx) require.NoError(t, err) - t.Logf("found %d running prebuilds so far, want %d", len(rows), tc.expectedPrebuildsCount) + t.Logf("found %d running prebuilds so far, want %d", len(rows), expectedPrebuildsCount) - return len(runningPrebuilds) == tc.expectedPrebuildsCount + return len(runningPrebuilds) == expectedPrebuildsCount }, testutil.WaitSuperLong, testutil.IntervalSlow) // Then: when restarting the created workspace (which claimed a prebuild), it should not try and claim a new prebuild. From f9052192a1934c5f44e78b3905a1bab000d793e3 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Tue, 22 Apr 2025 09:54:39 +0200 Subject: [PATCH 04/34] Removing unnecessary field Signed-off-by: Danny Kopping --- coderd/apidoc/docs.go | 3 --- coderd/apidoc/swagger.json | 3 --- codersdk/organizations.go | 1 - docs/reference/api/schemas.md | 22 ++++++++++------------ docs/reference/api/workspaces.md | 2 -- enterprise/coderd/prebuilds/claim_test.go | 1 - site/src/api/typesGenerated.ts | 1 - 7 files changed, 10 insertions(+), 23 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index fb572298d032a..268cfd7a894ba 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -11453,9 +11453,6 @@ const docTemplate = `{ "autostart_schedule": { "type": "string" }, - "claim_prebuild_if_available": { - "type": "boolean" - }, "name": { "type": "string" }, diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 990723b28b694..e973f11849547 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -10211,9 +10211,6 @@ "autostart_schedule": { "type": "string" }, - "claim_prebuild_if_available": { - "type": "boolean" - }, "name": { "type": "string" }, diff --git a/codersdk/organizations.go b/codersdk/organizations.go index 4e588770a989a..afe8a423c9199 100644 --- a/codersdk/organizations.go +++ b/codersdk/organizations.go @@ -227,7 +227,6 @@ type CreateWorkspaceRequest struct { RichParameterValues []WorkspaceBuildParameter `json:"rich_parameter_values,omitempty"` AutomaticUpdates AutomaticUpdates `json:"automatic_updates,omitempty"` TemplateVersionPresetID uuid.UUID `json:"template_version_preset_id,omitempty" format:"uuid"` - ClaimPrebuildIfAvailable bool `json:"claim_prebuild_if_available,omitempty"` } func (c *Client) OrganizationByName(ctx context.Context, name string) (Organization, error) { diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index 82682a3708a0f..79d7a411bf98c 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -1462,7 +1462,6 @@ None { "automatic_updates": "always", "autostart_schedule": "string", - "claim_prebuild_if_available": true, "name": "string", "rich_parameter_values": [ { @@ -1481,17 +1480,16 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o ### Properties -| Name | Type | Required | Restrictions | Description | -|-------------------------------|-------------------------------------------------------------------------------|----------|--------------|---------------------------------------------------------------------------------------------------------| -| `automatic_updates` | [codersdk.AutomaticUpdates](#codersdkautomaticupdates) | false | | | -| `autostart_schedule` | string | false | | | -| `claim_prebuild_if_available` | boolean | false | | | -| `name` | string | true | | | -| `rich_parameter_values` | array of [codersdk.WorkspaceBuildParameter](#codersdkworkspacebuildparameter) | false | | Rich parameter values allows for additional parameters to be provided during the initial provision. | -| `template_id` | string | false | | Template ID specifies which template should be used for creating the workspace. | -| `template_version_id` | string | false | | Template version ID can be used to specify a specific version of a template for creating the workspace. | -| `template_version_preset_id` | string | false | | | -| `ttl_ms` | integer | false | | | +| Name | Type | Required | Restrictions | Description | +|------------------------------|-------------------------------------------------------------------------------|----------|--------------|---------------------------------------------------------------------------------------------------------| +| `automatic_updates` | [codersdk.AutomaticUpdates](#codersdkautomaticupdates) | false | | | +| `autostart_schedule` | string | false | | | +| `name` | string | true | | | +| `rich_parameter_values` | array of [codersdk.WorkspaceBuildParameter](#codersdkworkspacebuildparameter) | false | | Rich parameter values allows for additional parameters to be provided during the initial provision. | +| `template_id` | string | false | | Template ID specifies which template should be used for creating the workspace. | +| `template_version_id` | string | false | | Template version ID can be used to specify a specific version of a template for creating the workspace. | +| `template_version_preset_id` | string | false | | | +| `ttl_ms` | integer | false | | | ## codersdk.CryptoKey diff --git a/docs/reference/api/workspaces.md b/docs/reference/api/workspaces.md index b5c5c6dfa524d..5e727cee297fe 100644 --- a/docs/reference/api/workspaces.md +++ b/docs/reference/api/workspaces.md @@ -25,7 +25,6 @@ of the template will be used. { "automatic_updates": "always", "autostart_schedule": "string", - "claim_prebuild_if_available": true, "name": "string", "rich_parameter_values": [ { @@ -606,7 +605,6 @@ of the template will be used. { "automatic_updates": "always", "autostart_schedule": "string", - "claim_prebuild_if_available": true, "name": "string", "rich_parameter_values": [ { diff --git a/enterprise/coderd/prebuilds/claim_test.go b/enterprise/coderd/prebuilds/claim_test.go index 2eabc2baea26c..58822f1866d84 100644 --- a/enterprise/coderd/prebuilds/claim_test.go +++ b/enterprise/coderd/prebuilds/claim_test.go @@ -193,7 +193,6 @@ func TestClaimPrebuild(t *testing.T) { TemplateVersionID: version.ID, Name: workspaceName, TemplateVersionPresetID: presets[0].ID, - ClaimPrebuildIfAvailable: true, // TODO: doesn't do anything yet; it probably should though. }) require.NoError(t, err) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index df437ae7db4e6..d160b7683e92e 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -467,7 +467,6 @@ export interface CreateWorkspaceRequest { readonly rich_parameter_values?: readonly WorkspaceBuildParameter[]; readonly automatic_updates?: AutomaticUpdates; readonly template_version_preset_id?: string; - readonly claim_prebuild_if_available?: boolean; } // From codersdk/deployment.go From 826633897a268205af9a7883446c3687829e2505 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Tue, 22 Apr 2025 11:54:39 +0200 Subject: [PATCH 05/34] make fmt Signed-off-by: Danny Kopping --- coderd/coderd.go | 3 ++- coderd/wsbuilder/wsbuilder.go | 12 ++++++------ codersdk/organizations.go | 6 +++--- enterprise/coderd/prebuilds/claim_test.go | 9 +++++---- 4 files changed, 16 insertions(+), 14 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index d7c56c183b735..c7b2c5b3cf4d3 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -9,7 +9,6 @@ import ( "expvar" "flag" "fmt" - "github.com/coder/coder/v2/coderd/prebuilds" "io" "net/http" "net/url" @@ -20,6 +19,8 @@ import ( "sync/atomic" "time" + "github.com/coder/coder/v2/coderd/prebuilds" + "github.com/andybalholm/brotli" "github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5/middleware" diff --git a/coderd/wsbuilder/wsbuilder.go b/coderd/wsbuilder/wsbuilder.go index c854f1ba01f4f..39162d5c978e4 100644 --- a/coderd/wsbuilder/wsbuilder.go +++ b/coderd/wsbuilder/wsbuilder.go @@ -75,8 +75,8 @@ type Builder struct { parameterValues *[]string templateVersionPresetParameterValues []database.TemplateVersionPresetParameter - prebuild bool - prebuildClaimedBy uuid.UUID + prebuild bool + prebuildClaimedBy uuid.UUID verifyNoLegacyParametersOnce bool } @@ -316,10 +316,10 @@ func (b *Builder) buildTx(authFunc func(action policy.Action, object rbac.Object workspaceBuildID := uuid.New() input, err := json.Marshal(provisionerdserver.WorkspaceProvisionJob{ - WorkspaceBuildID: workspaceBuildID, - LogLevel: b.logLevel, - IsPrebuild: b.prebuild, - PrebuildClaimedByUser: b.prebuildClaimedBy, + WorkspaceBuildID: workspaceBuildID, + LogLevel: b.logLevel, + IsPrebuild: b.prebuild, + PrebuildClaimedByUser: b.prebuildClaimedBy, }) if err != nil { return nil, nil, nil, BuildError{ diff --git a/codersdk/organizations.go b/codersdk/organizations.go index afe8a423c9199..b880f25e15a2c 100644 --- a/codersdk/organizations.go +++ b/codersdk/organizations.go @@ -224,9 +224,9 @@ type CreateWorkspaceRequest struct { TTLMillis *int64 `json:"ttl_ms,omitempty"` // RichParameterValues allows for additional parameters to be provided // during the initial provision. - RichParameterValues []WorkspaceBuildParameter `json:"rich_parameter_values,omitempty"` - AutomaticUpdates AutomaticUpdates `json:"automatic_updates,omitempty"` - TemplateVersionPresetID uuid.UUID `json:"template_version_preset_id,omitempty" format:"uuid"` + RichParameterValues []WorkspaceBuildParameter `json:"rich_parameter_values,omitempty"` + AutomaticUpdates AutomaticUpdates `json:"automatic_updates,omitempty"` + TemplateVersionPresetID uuid.UUID `json:"template_version_preset_id,omitempty" format:"uuid"` } func (c *Client) OrganizationByName(ctx context.Context, name string) (Organization, error) { diff --git a/enterprise/coderd/prebuilds/claim_test.go b/enterprise/coderd/prebuilds/claim_test.go index 58822f1866d84..4893062a75efc 100644 --- a/enterprise/coderd/prebuilds/claim_test.go +++ b/enterprise/coderd/prebuilds/claim_test.go @@ -8,10 +8,11 @@ import ( "testing" "time" - "github.com/coder/quartz" "github.com/google/uuid" "github.com/stretchr/testify/require" + "github.com/coder/quartz" + "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbauthz" @@ -190,9 +191,9 @@ func TestClaimPrebuild(t *testing.T) { PresetID: presets[0].ID, } userWorkspace, err := userClient.CreateUserWorkspace(ctx, user.Username, codersdk.CreateWorkspaceRequest{ - TemplateVersionID: version.ID, - Name: workspaceName, - TemplateVersionPresetID: presets[0].ID, + TemplateVersionID: version.ID, + Name: workspaceName, + TemplateVersionPresetID: presets[0].ID, }) require.NoError(t, err) From 4098ed713ce7fb2ddf30d5e9fc369513e8e98d6e Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Tue, 22 Apr 2025 17:10:21 +0000 Subject: [PATCH 06/34] CR's fixes --- coderd/coderd.go | 3 +-- coderd/workspaces.go | 5 ++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index c7b2c5b3cf4d3..3779971efae12 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -19,8 +19,6 @@ import ( "sync/atomic" "time" - "github.com/coder/coder/v2/coderd/prebuilds" - "github.com/andybalholm/brotli" "github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5/middleware" @@ -47,6 +45,7 @@ import ( "github.com/coder/coder/v2/coderd/entitlements" "github.com/coder/coder/v2/coderd/files" "github.com/coder/coder/v2/coderd/idpsync" + "github.com/coder/coder/v2/coderd/prebuilds" "github.com/coder/coder/v2/coderd/runtimeconfig" "github.com/coder/coder/v2/coderd/webpush" diff --git a/coderd/workspaces.go b/coderd/workspaces.go index ea2c8a8ae3baf..075caa92be594 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -11,8 +11,6 @@ import ( "strconv" "time" - "github.com/coder/coder/v2/coderd/prebuilds" - "github.com/dustin/go-humanize" "github.com/go-chi/chi/v5" "github.com/google/uuid" @@ -31,6 +29,7 @@ import ( "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/coderd/httpmw" "github.com/coder/coder/v2/coderd/notifications" + "github.com/coder/coder/v2/coderd/prebuilds" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/coderd/rbac/policy" "github.com/coder/coder/v2/coderd/schedule" @@ -897,7 +896,7 @@ func claimPrebuild(ctx context.Context, claimer prebuilds.Claimer, db database.S logger.Error(ctx, "unable to find claimed workspace by ID", slog.Error(err), slog.F("claimed_prebuild_id", (*claimedID).String())) return nil, xerrors.Errorf("find claimed workspace by ID %q: %w", (*claimedID).String(), err) } - return &lookup, err + return &lookup, nil } func (api *API) notifyWorkspaceCreated( From 50d23e4780a863f90a2c36192285febb2dd97302 Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Tue, 22 Apr 2025 17:47:35 +0000 Subject: [PATCH 07/34] CR's fixes --- enterprise/coderd/prebuilds/claim.go | 49 +++++++++------------------- 1 file changed, 16 insertions(+), 33 deletions(-) diff --git a/enterprise/coderd/prebuilds/claim.go b/enterprise/coderd/prebuilds/claim.go index af4095eb5fba3..1a647501dccaf 100644 --- a/enterprise/coderd/prebuilds/claim.go +++ b/enterprise/coderd/prebuilds/claim.go @@ -5,12 +5,11 @@ import ( "database/sql" "errors" - "github.com/coder/coder/v2/coderd/prebuilds" - "github.com/google/uuid" "golang.org/x/xerrors" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/prebuilds" ) type EnterpriseClaimer struct{} @@ -22,39 +21,23 @@ func (_ EnterpriseClaimer) Claim( name string, presetID uuid.UUID, ) (*uuid.UUID, error) { - var prebuildID *uuid.UUID - err := store.InTx(func(db database.Store) error { - // TODO: do we need this? - //// Ensure no other replica can claim a prebuild for this user simultaneously. - // err := store.AcquireLock(ctx, database.GenLockID(fmt.Sprintf("prebuild-user-claim-%s", userID.String()))) - // if err != nil { - // return xerrors.Errorf("acquire claim lock for user %q: %w", userID.String(), err) - //} - - result, err := db.ClaimPrebuiltWorkspace(ctx, database.ClaimPrebuiltWorkspaceParams{ - NewUserID: userID, - NewName: name, - PresetID: presetID, - }) - if err != nil { - switch { - // No eligible prebuilds found - case errors.Is(err, sql.ErrNoRows): - // Exit, this will result in a nil prebuildID being returned, which is fine - return nil - default: - return xerrors.Errorf("claim prebuild for user %q: %w", userID.String(), err) - } - } - - prebuildID = &result.ID - - return nil - }, &database.TxOptions{ - TxIdentifier: "prebuild-claim", + result, err := store.ClaimPrebuiltWorkspace(ctx, database.ClaimPrebuiltWorkspaceParams{ + NewUserID: userID, + NewName: name, + PresetID: presetID, }) + if err != nil { + switch { + // No eligible prebuilds found + case errors.Is(err, sql.ErrNoRows): + // Exit, this will result in a nil prebuildID being returned, which is fine + return nil, nil + default: + return nil, xerrors.Errorf("claim prebuild for user %q: %w", userID.String(), err) + } + } - return prebuildID, err + return &result.ID, nil } func (_ EnterpriseClaimer) Initiator() uuid.UUID { From 546b7ae70c5fc675001d612c2dd521514bfa630a Mon Sep 17 00:00:00 2001 From: Yevhenii Shcherbina Date: Tue, 22 Apr 2025 13:50:46 -0400 Subject: [PATCH 08/34] Update coderd/workspaces.go Co-authored-by: Danny Kopping --- coderd/workspaces.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 075caa92be594..e9ff19f714cdc 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -647,7 +647,7 @@ func createWorkspace( claimedWorkspace *database.Workspace ) - // If a template preset was chosen, try claim a prebuild. + // If a template preset was chosen, try claim a prebuilt workspace. if req.TemplateVersionPresetID != uuid.Nil { // Try and claim an eligible prebuild, if available. claimedWorkspace, err = claimPrebuild(ctx, prebuilds, db, api.Logger, req, owner) From 85f79268c998af62d0acfc7953df891b51daa65a Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Tue, 22 Apr 2025 19:57:06 +0000 Subject: [PATCH 09/34] CR's fixes --- coderd/prebuilds/api.go | 3 +++ coderd/workspaces.go | 13 ++++--------- enterprise/coderd/prebuilds/claim.go | 4 +--- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/coderd/prebuilds/api.go b/coderd/prebuilds/api.go index b0acaf9cb71b0..edcfca0b6ddce 100644 --- a/coderd/prebuilds/api.go +++ b/coderd/prebuilds/api.go @@ -2,12 +2,15 @@ package prebuilds import ( "context" + "errors" "github.com/google/uuid" "github.com/coder/coder/v2/coderd/database" ) +var ErrNoClaimablePrebuiltWorkspaces = errors.New("no claimable prebuilt workspaces found") + // ReconciliationOrchestrator manages the lifecycle of prebuild reconciliation. // It runs a continuous loop to check and reconcile prebuild states, and can be stopped gracefully. type ReconciliationOrchestrator interface { diff --git a/coderd/workspaces.go b/coderd/workspaces.go index e9ff19f714cdc..0667e88a3092e 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -639,7 +639,7 @@ func createWorkspace( provisionerDaemons []database.GetEligibleProvisionerDaemonsByProvisionerJobIDsRow ) - prebuilds := *api.PrebuildsClaimer.Load() + prebuildsClaimer := *api.PrebuildsClaimer.Load() err = api.Database.InTx(func(db database.Store) error { var ( @@ -650,8 +650,8 @@ func createWorkspace( // If a template preset was chosen, try claim a prebuilt workspace. if req.TemplateVersionPresetID != uuid.Nil { // Try and claim an eligible prebuild, if available. - claimedWorkspace, err = claimPrebuild(ctx, prebuilds, db, api.Logger, req, owner) - if err != nil { + 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) } } @@ -683,7 +683,7 @@ func createWorkspace( } else { // Prebuild found! workspaceID = claimedWorkspace.ID - initiatorID = prebuilds.Initiator() + initiatorID = prebuildsClaimer.Initiator() } // We have to refetch the workspace for the joined in fields. @@ -886,11 +886,6 @@ func claimPrebuild(ctx context.Context, claimer prebuilds.Claimer, db database.S return nil, xerrors.Errorf("claim prebuild: %w", err) } - // No prebuild available. - if claimedID == nil { - return nil, nil - } - lookup, err := db.GetWorkspaceByID(prebuildsCtx, *claimedID) if err != nil { logger.Error(ctx, "unable to find claimed workspace by ID", slog.Error(err), slog.F("claimed_prebuild_id", (*claimedID).String())) diff --git a/enterprise/coderd/prebuilds/claim.go b/enterprise/coderd/prebuilds/claim.go index 1a647501dccaf..48ad4d425f9a6 100644 --- a/enterprise/coderd/prebuilds/claim.go +++ b/enterprise/coderd/prebuilds/claim.go @@ -4,7 +4,6 @@ import ( "context" "database/sql" "errors" - "github.com/google/uuid" "golang.org/x/xerrors" @@ -30,8 +29,7 @@ func (_ EnterpriseClaimer) Claim( switch { // No eligible prebuilds found case errors.Is(err, sql.ErrNoRows): - // Exit, this will result in a nil prebuildID being returned, which is fine - return nil, nil + return nil, prebuilds.ErrNoClaimablePrebuiltWorkspaces default: return nil, xerrors.Errorf("claim prebuild for user %q: %w", userID.String(), err) } From ee9908cc02eb1c026b1f1c7da0217d58445b0d38 Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Tue, 22 Apr 2025 20:10:51 +0000 Subject: [PATCH 10/34] CR's fixes --- enterprise/coderd/prebuilds/claim_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/enterprise/coderd/prebuilds/claim_test.go b/enterprise/coderd/prebuilds/claim_test.go index 4893062a75efc..6c88edd0cfe8d 100644 --- a/enterprise/coderd/prebuilds/claim_test.go +++ b/enterprise/coderd/prebuilds/claim_test.go @@ -90,10 +90,6 @@ func TestClaimPrebuild(t *testing.T) { expectPrebuildClaimed: true, markPrebuildsClaimable: true, }, - "claiming an eligible prebuild results in error": { - expectPrebuildClaimed: true, - markPrebuildsClaimable: true, - }, } for name, tc := range cases { From 70bf1795962f59c863b59e030b1ef0998252a9f4 Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Tue, 22 Apr 2025 20:16:11 +0000 Subject: [PATCH 11/34] CR's fixes --- enterprise/coderd/prebuilds/claim_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/enterprise/coderd/prebuilds/claim_test.go b/enterprise/coderd/prebuilds/claim_test.go index 6c88edd0cfe8d..efe84f65aa290 100644 --- a/enterprise/coderd/prebuilds/claim_test.go +++ b/enterprise/coderd/prebuilds/claim_test.go @@ -197,7 +197,6 @@ func TestClaimPrebuild(t *testing.T) { // Then: a prebuild should have been claimed. require.EqualValues(t, spy.claims.Load(), 1) - require.NotNil(t, spy.claims.Load()) require.EqualValues(t, *spy.claimParams.Load(), params) if !tc.expectPrebuildClaimed { @@ -207,7 +206,7 @@ func TestClaimPrebuild(t *testing.T) { require.NotNil(t, spy.claimedWorkspace.Load()) claimed := *spy.claimedWorkspace.Load() - require.NotEqual(t, claimed, uuid.Nil) + require.NotEqual(t, claimed.ID, uuid.Nil) // Then: the claimed prebuild must now be owned by the requester. workspace, err := spy.GetWorkspaceByID(ctx, claimed.ID) From 7a72d034ed7dda884ef0a8f46543ffdca8cb0844 Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Tue, 22 Apr 2025 20:23:38 +0000 Subject: [PATCH 12/34] CR's fixes --- enterprise/coderd/prebuilds/claim_test.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/enterprise/coderd/prebuilds/claim_test.go b/enterprise/coderd/prebuilds/claim_test.go index efe84f65aa290..94e36ef4c13e8 100644 --- a/enterprise/coderd/prebuilds/claim_test.go +++ b/enterprise/coderd/prebuilds/claim_test.go @@ -216,14 +216,11 @@ func TestClaimPrebuild(t *testing.T) { // Then: the number of running prebuilds has changed since one was claimed. currentPrebuilds, err := spy.GetRunningPrebuiltWorkspaces(ctx) require.NoError(t, err) - require.NotEqual(t, len(currentPrebuilds), len(runningPrebuilds)) + require.Equal(t, expectedPrebuildsCount-1, len(currentPrebuilds)) // Then: the claimed prebuild is now missing from the running prebuilds set. - current, err := spy.GetRunningPrebuiltWorkspaces(ctx) - require.NoError(t, err) - var found bool - for _, prebuild := range current { + for _, prebuild := range currentPrebuilds { if prebuild.ID == claimed.ID { found = true break From b246589a72804dc8c764059364b1cb89e444977b Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Tue, 22 Apr 2025 21:08:56 +0000 Subject: [PATCH 13/34] fix tests by updating noop.go --- coderd/prebuilds/noop.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/prebuilds/noop.go b/coderd/prebuilds/noop.go index 4232e1f2654b8..c177ec614e8b3 100644 --- a/coderd/prebuilds/noop.go +++ b/coderd/prebuilds/noop.go @@ -40,7 +40,7 @@ type AGPLPrebuildClaimer struct{} func (c AGPLPrebuildClaimer) Claim(context.Context, database.Store, uuid.UUID, string, uuid.UUID) (*uuid.UUID, error) { // Not entitled to claim prebuilds in AGPL version. - return nil, nil + return nil, ErrNoClaimablePrebuiltWorkspaces } func (c AGPLPrebuildClaimer) Initiator() uuid.UUID { From fcdbba8ecc1892af0d94951444023b4dfcd491ac Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Wed, 23 Apr 2025 14:21:34 +0000 Subject: [PATCH 14/34] test: add failure testcase scenario --- enterprise/coderd/prebuilds/claim_test.go | 197 ++++++++++++++++++++++ 1 file changed, 197 insertions(+) diff --git a/enterprise/coderd/prebuilds/claim_test.go b/enterprise/coderd/prebuilds/claim_test.go index 94e36ef4c13e8..33de6bc2bbdb0 100644 --- a/enterprise/coderd/prebuilds/claim_test.go +++ b/enterprise/coderd/prebuilds/claim_test.go @@ -3,6 +3,7 @@ package prebuilds_test import ( "context" "database/sql" + "errors" "strings" "sync/atomic" "testing" @@ -66,6 +67,32 @@ func (m *storeSpy) ClaimPrebuiltWorkspace(ctx context.Context, arg database.Clai return result, err } +type errorStore struct { + claimingErr error + + database.Store +} + +func newErrorStore(db database.Store, claimingErr error) *errorStore { + return &errorStore{ + Store: db, + claimingErr: claimingErr, + } +} + +func (es *errorStore) InTx(fn func(store database.Store) error, opts *database.TxOptions) error { + // Pass failure store down into transaction store. + return es.Store.InTx(func(store database.Store) error { + newES := newErrorStore(store, es.claimingErr) + + return fn(newES) + }, opts) +} + +func (es *errorStore) ClaimPrebuiltWorkspace(ctx context.Context, arg database.ClaimPrebuiltWorkspaceParams) (database.ClaimPrebuiltWorkspaceRow, error) { + return database.ClaimPrebuiltWorkspaceRow{}, es.claimingErr +} + func TestClaimPrebuild(t *testing.T) { t.Parallel() @@ -284,6 +311,176 @@ func TestClaimPrebuild(t *testing.T) { } } +func TestClaimPrebuild_CheckDifferentErrors(t *testing.T) { + t.Parallel() + + if !dbtestutil.WillUsePostgres() { + t.Skip("This test requires postgres") + } + + const ( + desiredInstances = 1 + presetCount = 2 + + expectedPrebuildsCount = desiredInstances * presetCount + ) + + cases := map[string]struct { + claimingErr error + checkFn func( + t *testing.T, + ctx context.Context, + store database.Store, + userClient *codersdk.Client, + user codersdk.User, + templateVersionID uuid.UUID, + presetID uuid.UUID, + ) + }{ + "ErrNoClaimablePrebuiltWorkspaces is returned": { + claimingErr: agplprebuilds.ErrNoClaimablePrebuiltWorkspaces, + checkFn: func( + t *testing.T, + ctx context.Context, + store database.Store, + userClient *codersdk.Client, + user codersdk.User, + templateVersionID uuid.UUID, + presetID uuid.UUID, + ) { + // When: a user creates a new workspace with a preset for which prebuilds are configured. + workspaceName := strings.ReplaceAll(testutil.GetRandomName(t), "_", "-") + userWorkspace, err := userClient.CreateUserWorkspace(ctx, user.Username, codersdk.CreateWorkspaceRequest{ + TemplateVersionID: templateVersionID, + Name: workspaceName, + TemplateVersionPresetID: presetID, + }) + + require.NoError(t, err) + coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, userWorkspace.LatestBuild.ID) + + // Then: the number of running prebuilds hasn't changed because claiming prebuild is failed and we fallback to creating new workspace. + currentPrebuilds, err := store.GetRunningPrebuiltWorkspaces(ctx) + require.NoError(t, err) + require.Equal(t, expectedPrebuildsCount, len(currentPrebuilds)) + }, + }, + "unexpected error during claim is returned": { + claimingErr: errors.New("unexpected error during claim"), + checkFn: func( + t *testing.T, + ctx context.Context, + store database.Store, + userClient *codersdk.Client, + user codersdk.User, + templateVersionID uuid.UUID, + presetID uuid.UUID, + ) { + // When: a user creates a new workspace with a preset for which prebuilds are configured. + workspaceName := strings.ReplaceAll(testutil.GetRandomName(t), "_", "-") + _, err := userClient.CreateUserWorkspace(ctx, user.Username, codersdk.CreateWorkspaceRequest{ + TemplateVersionID: templateVersionID, + Name: workspaceName, + TemplateVersionPresetID: presetID, + }) + + // Then: unexpected error happened and was propagated all the way to the caller + require.Error(t, err) + require.ErrorContains(t, err, "unexpected error during claim") + + // Then: the number of running prebuilds hasn't changed because claiming prebuild is failed. + currentPrebuilds, err := store.GetRunningPrebuiltWorkspaces(ctx) + require.NoError(t, err) + require.Equal(t, expectedPrebuildsCount, len(currentPrebuilds)) + }, + }, + } + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + t.Parallel() + + // Setup. + ctx := testutil.Context(t, testutil.WaitMedium) + db, pubsub := dbtestutil.NewDB(t) + errorStore := newErrorStore(db, tc.claimingErr) + + logger := testutil.Logger(t) + client, _, api, owner := coderdenttest.NewWithAPI(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + IncludeProvisionerDaemon: true, + Database: errorStore, + Pubsub: pubsub, + }, + + EntitlementsUpdateInterval: time.Second, + }) + + reconciler := prebuilds.NewStoreReconciler(errorStore, pubsub, codersdk.PrebuildsConfig{}, logger, quartz.NewMock(t)) + var claimer agplprebuilds.Claimer = &prebuilds.EnterpriseClaimer{} + api.AGPL.PrebuildsClaimer.Store(&claimer) + + version := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, templateWithAgentAndPresetsWithPrebuilds(desiredInstances)) + _ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) + coderdtest.CreateTemplate(t, client, owner.OrganizationID, version.ID) + presets, err := client.TemplateVersionPresets(ctx, version.ID) + require.NoError(t, err) + require.Len(t, presets, presetCount) + + userClient, user := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.RoleMember()) + + ctx = dbauthz.AsPrebuildsOrchestrator(ctx) + + // Given: the reconciliation state is snapshot. + state, err := reconciler.SnapshotState(ctx, errorStore) + require.NoError(t, err) + require.Len(t, state.Presets, presetCount) + + // When: a reconciliation is setup for each preset. + for _, preset := range presets { + ps, err := state.FilterByPreset(preset.ID) + require.NoError(t, err) + require.NotNil(t, ps) + actions, err := reconciler.CalculateActions(ctx, *ps) + require.NoError(t, err) + require.NotNil(t, actions) + + require.NoError(t, reconciler.ReconcilePreset(ctx, *ps)) + } + + // Given: a set of running, eligible prebuilds eventually starts up. + runningPrebuilds := make(map[uuid.UUID]database.GetRunningPrebuiltWorkspacesRow, desiredInstances*presetCount) + require.Eventually(t, func() bool { + rows, err := errorStore.GetRunningPrebuiltWorkspaces(ctx) + require.NoError(t, err) + + for _, row := range rows { + runningPrebuilds[row.CurrentPresetID.UUID] = row + + agents, err := db.GetWorkspaceAgentsInLatestBuildByWorkspaceID(ctx, row.ID) + require.NoError(t, err) + + // Workspaces are eligible once its agent is marked "ready". + for _, agent := range agents { + require.NoError(t, db.UpdateWorkspaceAgentLifecycleStateByID(ctx, database.UpdateWorkspaceAgentLifecycleStateByIDParams{ + ID: agent.ID, + LifecycleState: database.WorkspaceAgentLifecycleStateReady, + StartedAt: sql.NullTime{Time: time.Now().Add(time.Hour), Valid: true}, + ReadyAt: sql.NullTime{Time: time.Now().Add(-1 * time.Hour), Valid: true}, + })) + } + } + + t.Logf("found %d running prebuilds so far, want %d", len(runningPrebuilds), expectedPrebuildsCount) + + return len(runningPrebuilds) == expectedPrebuildsCount + }, testutil.WaitSuperLong, testutil.IntervalSlow) + + tc.checkFn(t, ctx, errorStore, userClient, user, version.ID, presets[0].ID) + }) + } +} + func templateWithAgentAndPresetsWithPrebuilds(desiredInstances int32) *echo.Responses { return &echo.Responses{ Parse: echo.ParseComplete, From e7b7f80287d598afe41664d375525e94a219a2a7 Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Wed, 23 Apr 2025 15:35:57 +0000 Subject: [PATCH 15/34] refactor: revert interface changes --- coderd/prebuilds/api.go | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/coderd/prebuilds/api.go b/coderd/prebuilds/api.go index edcfca0b6ddce..1e7183f7ddb1d 100644 --- a/coderd/prebuilds/api.go +++ b/coderd/prebuilds/api.go @@ -26,35 +26,11 @@ type ReconciliationOrchestrator interface { Stop(ctx context.Context, cause error) } -// Reconciler defines the core operations for managing prebuilds. -// It provides both high-level orchestration (ReconcileAll) and lower-level operations -// for more fine-grained control (SnapshotState, ReconcilePreset, CalculateActions). -// All database operations must be performed within repeatable-read transactions -// to ensure consistency. type Reconciler interface { // ReconcileAll orchestrates the reconciliation of all prebuilds across all templates. // It takes a global snapshot of the system state and then reconciles each preset // in parallel, creating or deleting prebuilds as needed to reach their desired states. - // For more fine-grained control, you can use the lower-level methods SnapshotState - // and ReconcilePreset directly. ReconcileAll(ctx context.Context) error - - // SnapshotState captures the current state of all prebuilds across templates. - // It creates a global database snapshot that can be viewed as a collection of PresetSnapshots, - // each representing the state of prebuilds for a specific preset. - // MUST be called inside a repeatable-read transaction. - SnapshotState(ctx context.Context, store database.Store) (*GlobalSnapshot, error) - - // ReconcilePreset handles a single PresetSnapshot, determining and executing - // the required actions (creating or deleting prebuilds) based on the current state. - // MUST be called inside a repeatable-read transaction. - ReconcilePreset(ctx context.Context, snapshot PresetSnapshot) error - - // CalculateActions determines what actions are needed to reconcile a preset's prebuilds - // to their desired state. This includes creating new prebuilds, deleting excess ones, - // or waiting due to backoff periods. - // MUST be called inside a repeatable-read transaction. - CalculateActions(ctx context.Context, state PresetSnapshot) (*ReconciliationActions, error) } type Claimer interface { From e7455db207e10c24012b2d1b658efee38ce6c661 Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Wed, 23 Apr 2025 17:12:47 +0000 Subject: [PATCH 16/34] refactor: simplify signature of claim method --- coderd/prebuilds/api.go | 4 +--- coderd/prebuilds/noop.go | 2 +- coderd/workspaces.go | 2 +- enterprise/coderd/prebuilds/claim.go | 15 +++++++++++---- enterprise/coderd/prebuilds/claim_test.go | 4 ++-- 5 files changed, 16 insertions(+), 11 deletions(-) diff --git a/coderd/prebuilds/api.go b/coderd/prebuilds/api.go index 1e7183f7ddb1d..a82b68629089a 100644 --- a/coderd/prebuilds/api.go +++ b/coderd/prebuilds/api.go @@ -5,8 +5,6 @@ import ( "errors" "github.com/google/uuid" - - "github.com/coder/coder/v2/coderd/database" ) var ErrNoClaimablePrebuiltWorkspaces = errors.New("no claimable prebuilt workspaces found") @@ -34,6 +32,6 @@ type Reconciler interface { } type Claimer interface { - Claim(ctx context.Context, store database.Store, userID uuid.UUID, name string, presetID uuid.UUID) (*uuid.UUID, error) + Claim(ctx context.Context, userID uuid.UUID, name string, presetID uuid.UUID) (*uuid.UUID, error) Initiator() uuid.UUID } diff --git a/coderd/prebuilds/noop.go b/coderd/prebuilds/noop.go index c177ec614e8b3..375cd0d62b196 100644 --- a/coderd/prebuilds/noop.go +++ b/coderd/prebuilds/noop.go @@ -38,7 +38,7 @@ var _ ReconciliationOrchestrator = NoopReconciler{} type AGPLPrebuildClaimer struct{} -func (c AGPLPrebuildClaimer) Claim(context.Context, database.Store, uuid.UUID, string, uuid.UUID) (*uuid.UUID, error) { +func (c AGPLPrebuildClaimer) Claim(context.Context, uuid.UUID, string, uuid.UUID) (*uuid.UUID, error) { // Not entitled to claim prebuilds in AGPL version. return nil, ErrNoClaimablePrebuiltWorkspaces } diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 0667e88a3092e..52ba2b6ebe54f 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -880,7 +880,7 @@ func claimPrebuild(ctx context.Context, claimer prebuilds.Claimer, db database.S claimCtx, cancel := context.WithTimeout(prebuildsCtx, time.Second*10) defer cancel() - claimedID, err := claimer.Claim(claimCtx, db, owner.ID, req.Name, req.TemplateVersionPresetID) + claimedID, err := claimer.Claim(claimCtx, owner.ID, req.Name, req.TemplateVersionPresetID) if err != nil { // TODO: enhance this by clarifying whether this *specific* prebuild failed or whether there are none to claim. return nil, xerrors.Errorf("claim prebuild: %w", err) diff --git a/enterprise/coderd/prebuilds/claim.go b/enterprise/coderd/prebuilds/claim.go index 48ad4d425f9a6..547bcd2b1281e 100644 --- a/enterprise/coderd/prebuilds/claim.go +++ b/enterprise/coderd/prebuilds/claim.go @@ -11,16 +11,23 @@ import ( "github.com/coder/coder/v2/coderd/prebuilds" ) -type EnterpriseClaimer struct{} +type EnterpriseClaimer struct { + store database.Store +} + +func NewEnterpriseClaimer(store database.Store) *EnterpriseClaimer { + return &EnterpriseClaimer{ + store: store, + } +} -func (_ EnterpriseClaimer) Claim( +func (c EnterpriseClaimer) Claim( ctx context.Context, - store database.Store, userID uuid.UUID, name string, presetID uuid.UUID, ) (*uuid.UUID, error) { - result, err := store.ClaimPrebuiltWorkspace(ctx, database.ClaimPrebuiltWorkspaceParams{ + result, err := c.store.ClaimPrebuiltWorkspace(ctx, database.ClaimPrebuiltWorkspaceParams{ NewUserID: userID, NewName: name, PresetID: presetID, diff --git a/enterprise/coderd/prebuilds/claim_test.go b/enterprise/coderd/prebuilds/claim_test.go index 33de6bc2bbdb0..60c2afc7b8c84 100644 --- a/enterprise/coderd/prebuilds/claim_test.go +++ b/enterprise/coderd/prebuilds/claim_test.go @@ -143,7 +143,7 @@ func TestClaimPrebuild(t *testing.T) { }) reconciler := prebuilds.NewStoreReconciler(spy, pubsub, codersdk.PrebuildsConfig{}, logger, quartz.NewMock(t)) - var claimer agplprebuilds.Claimer = &prebuilds.EnterpriseClaimer{} + var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(spy) api.AGPL.PrebuildsClaimer.Store(&claimer) version := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, templateWithAgentAndPresetsWithPrebuilds(desiredInstances)) @@ -417,7 +417,7 @@ func TestClaimPrebuild_CheckDifferentErrors(t *testing.T) { }) reconciler := prebuilds.NewStoreReconciler(errorStore, pubsub, codersdk.PrebuildsConfig{}, logger, quartz.NewMock(t)) - var claimer agplprebuilds.Claimer = &prebuilds.EnterpriseClaimer{} + var claimer agplprebuilds.Claimer = prebuilds.NewEnterpriseClaimer(errorStore) api.AGPL.PrebuildsClaimer.Store(&claimer) version := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, templateWithAgentAndPresetsWithPrebuilds(desiredInstances)) From 077d7f09897695f2827f678331dfc393e617312b Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Wed, 23 Apr 2025 17:24:28 +0000 Subject: [PATCH 17/34] refactor: CR's fixes --- coderd/workspaces.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 52ba2b6ebe54f..cedf4b2cbf6f9 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -639,12 +639,11 @@ func createWorkspace( provisionerDaemons []database.GetEligibleProvisionerDaemonsByProvisionerJobIDsRow ) - prebuildsClaimer := *api.PrebuildsClaimer.Load() - err = api.Database.InTx(func(db database.Store) error { var ( workspaceID uuid.UUID claimedWorkspace *database.Workspace + prebuildsClaimer = *api.PrebuildsClaimer.Load() ) // If a template preset was chosen, try claim a prebuilt workspace. From 663a8c0c233135e18a7e79e809588148a4457218 Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Wed, 23 Apr 2025 17:35:38 +0000 Subject: [PATCH 18/34] refactor: CR's fixes --- coderd/workspaces.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 9b80b1b6b2d08..dcd7adcec7e38 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -878,11 +878,7 @@ func requestTemplate(ctx context.Context, rw http.ResponseWriter, req codersdk.C func claimPrebuild(ctx context.Context, claimer prebuilds.Claimer, db database.Store, logger slog.Logger, req codersdk.CreateWorkspaceRequest, owner workspaceOwner) (*database.Workspace, error) { prebuildsCtx := dbauthz.AsPrebuildsOrchestrator(ctx) - // TODO: do we need a timeout here? - claimCtx, cancel := context.WithTimeout(prebuildsCtx, time.Second*10) - defer cancel() - - claimedID, err := claimer.Claim(claimCtx, owner.ID, req.Name, req.TemplateVersionPresetID) + claimedID, err := claimer.Claim(prebuildsCtx, owner.ID, req.Name, req.TemplateVersionPresetID) if err != nil { // TODO: enhance this by clarifying whether this *specific* prebuild failed or whether there are none to claim. return nil, xerrors.Errorf("claim prebuild: %w", err) From bc31fac9c6ef27b5475fdd7a6ba9f9b55f677153 Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Wed, 23 Apr 2025 17:59:44 +0000 Subject: [PATCH 19/34] refactor: make lint --- coderd/prebuilds/api.go | 4 ++-- enterprise/coderd/prebuilds/claim.go | 1 + enterprise/coderd/prebuilds/claim_test.go | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/coderd/prebuilds/api.go b/coderd/prebuilds/api.go index a82b68629089a..ebc4c39c89b50 100644 --- a/coderd/prebuilds/api.go +++ b/coderd/prebuilds/api.go @@ -2,12 +2,12 @@ package prebuilds import ( "context" - "errors" "github.com/google/uuid" + "golang.org/x/xerrors" ) -var ErrNoClaimablePrebuiltWorkspaces = errors.New("no claimable prebuilt workspaces found") +var ErrNoClaimablePrebuiltWorkspaces = xerrors.New("no claimable prebuilt workspaces found") // 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/enterprise/coderd/prebuilds/claim.go b/enterprise/coderd/prebuilds/claim.go index 547bcd2b1281e..69ba1fa107952 100644 --- a/enterprise/coderd/prebuilds/claim.go +++ b/enterprise/coderd/prebuilds/claim.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "errors" + "github.com/google/uuid" "golang.org/x/xerrors" diff --git a/enterprise/coderd/prebuilds/claim_test.go b/enterprise/coderd/prebuilds/claim_test.go index 60c2afc7b8c84..7eccc7ef52b74 100644 --- a/enterprise/coderd/prebuilds/claim_test.go +++ b/enterprise/coderd/prebuilds/claim_test.go @@ -3,7 +3,6 @@ package prebuilds_test import ( "context" "database/sql" - "errors" "strings" "sync/atomic" "testing" @@ -11,6 +10,7 @@ import ( "github.com/google/uuid" "github.com/stretchr/testify/require" + "golang.org/x/xerrors" "github.com/coder/quartz" @@ -366,7 +366,7 @@ func TestClaimPrebuild_CheckDifferentErrors(t *testing.T) { }, }, "unexpected error during claim is returned": { - claimingErr: errors.New("unexpected error during claim"), + claimingErr: xerrors.New("unexpected error during claim"), checkFn: func( t *testing.T, ctx context.Context, From 087bd209f8547a45df1cffed07e4d1f75517ea51 Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Wed, 23 Apr 2025 19:16:42 +0000 Subject: [PATCH 20/34] refactor: fix linter --- coderd/prebuilds/noop.go | 4 ++-- coderd/workspaces.go | 4 ++-- enterprise/coderd/prebuilds/claim.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/coderd/prebuilds/noop.go b/coderd/prebuilds/noop.go index 375cd0d62b196..d122a61ebb9c6 100644 --- a/coderd/prebuilds/noop.go +++ b/coderd/prebuilds/noop.go @@ -38,12 +38,12 @@ var _ ReconciliationOrchestrator = NoopReconciler{} type AGPLPrebuildClaimer struct{} -func (c AGPLPrebuildClaimer) Claim(context.Context, uuid.UUID, string, uuid.UUID) (*uuid.UUID, error) { +func (AGPLPrebuildClaimer) Claim(context.Context, uuid.UUID, string, uuid.UUID) (*uuid.UUID, error) { // Not entitled to claim prebuilds in AGPL version. return nil, ErrNoClaimablePrebuiltWorkspaces } -func (c AGPLPrebuildClaimer) Initiator() uuid.UUID { +func (AGPLPrebuildClaimer) Initiator() uuid.UUID { return uuid.Nil } diff --git a/coderd/workspaces.go b/coderd/workspaces.go index dcd7adcec7e38..94e43c67128d7 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -886,8 +886,8 @@ func claimPrebuild(ctx context.Context, claimer prebuilds.Claimer, db database.S lookup, err := db.GetWorkspaceByID(prebuildsCtx, *claimedID) if err != nil { - logger.Error(ctx, "unable to find claimed workspace by ID", slog.Error(err), slog.F("claimed_prebuild_id", (*claimedID).String())) - return nil, xerrors.Errorf("find claimed workspace by ID %q: %w", (*claimedID).String(), err) + logger.Error(ctx, "unable to find claimed workspace by ID", slog.Error(err), slog.F("claimed_prebuild_id", claimedID.String())) + return nil, xerrors.Errorf("find claimed workspace by ID %q: %w", claimedID.String(), err) } return &lookup, nil } diff --git a/enterprise/coderd/prebuilds/claim.go b/enterprise/coderd/prebuilds/claim.go index 69ba1fa107952..f040ee756e678 100644 --- a/enterprise/coderd/prebuilds/claim.go +++ b/enterprise/coderd/prebuilds/claim.go @@ -46,7 +46,7 @@ func (c EnterpriseClaimer) Claim( return &result.ID, nil } -func (_ EnterpriseClaimer) Initiator() uuid.UUID { +func (EnterpriseClaimer) Initiator() uuid.UUID { return prebuilds.SystemUserID } From 9f66169e506141d8e3466abcabd8eeae9d9857e4 Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Wed, 23 Apr 2025 19:26:35 +0000 Subject: [PATCH 21/34] refactor: remove PrebuildsOrchestrator context from claim tests --- enterprise/coderd/prebuilds/claim_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/enterprise/coderd/prebuilds/claim_test.go b/enterprise/coderd/prebuilds/claim_test.go index 7eccc7ef52b74..562b48b5ec72e 100644 --- a/enterprise/coderd/prebuilds/claim_test.go +++ b/enterprise/coderd/prebuilds/claim_test.go @@ -16,7 +16,6 @@ import ( "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" - "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbtestutil" agplprebuilds "github.com/coder/coder/v2/coderd/prebuilds" "github.com/coder/coder/v2/coderd/rbac" @@ -155,8 +154,6 @@ func TestClaimPrebuild(t *testing.T) { userClient, user := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.RoleMember()) - ctx = dbauthz.AsPrebuildsOrchestrator(ctx) - // Given: the reconciliation state is snapshot. state, err := reconciler.SnapshotState(ctx, spy) require.NoError(t, err) @@ -429,8 +426,6 @@ func TestClaimPrebuild_CheckDifferentErrors(t *testing.T) { userClient, user := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.RoleMember()) - ctx = dbauthz.AsPrebuildsOrchestrator(ctx) - // Given: the reconciliation state is snapshot. state, err := reconciler.SnapshotState(ctx, errorStore) require.NoError(t, err) From 7f25f241ec76b82ba8c5f14d503df6906a14a2df Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Wed, 23 Apr 2025 19:59:28 +0000 Subject: [PATCH 22/34] refactor: don't fail test in a goroutine --- enterprise/coderd/prebuilds/claim_test.go | 34 +++++++++++++++++------ 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/enterprise/coderd/prebuilds/claim_test.go b/enterprise/coderd/prebuilds/claim_test.go index 562b48b5ec72e..02ee874fd48fd 100644 --- a/enterprise/coderd/prebuilds/claim_test.go +++ b/enterprise/coderd/prebuilds/claim_test.go @@ -175,7 +175,9 @@ func TestClaimPrebuild(t *testing.T) { runningPrebuilds := make(map[uuid.UUID]database.GetRunningPrebuiltWorkspacesRow, desiredInstances*presetCount) require.Eventually(t, func() bool { rows, err := spy.GetRunningPrebuiltWorkspaces(ctx) - require.NoError(t, err) + if err != nil { + return false + } for _, row := range rows { runningPrebuilds[row.CurrentPresetID.UUID] = row @@ -185,16 +187,21 @@ func TestClaimPrebuild(t *testing.T) { } agents, err := db.GetWorkspaceAgentsInLatestBuildByWorkspaceID(ctx, row.ID) - require.NoError(t, err) + if err != nil { + return false + } // Workspaces are eligible once its agent is marked "ready". for _, agent := range agents { - require.NoError(t, db.UpdateWorkspaceAgentLifecycleStateByID(ctx, database.UpdateWorkspaceAgentLifecycleStateByIDParams{ + err = db.UpdateWorkspaceAgentLifecycleStateByID(ctx, database.UpdateWorkspaceAgentLifecycleStateByIDParams{ ID: agent.ID, LifecycleState: database.WorkspaceAgentLifecycleStateReady, StartedAt: sql.NullTime{Time: time.Now().Add(time.Hour), Valid: true}, ReadyAt: sql.NullTime{Time: time.Now().Add(-1 * time.Hour), Valid: true}, - })) + }) + if err != nil { + return false + } } } @@ -270,7 +277,9 @@ func TestClaimPrebuild(t *testing.T) { require.Eventually(t, func() bool { rows, err := spy.GetRunningPrebuiltWorkspaces(ctx) - require.NoError(t, err) + if err != nil { + return false + } t.Logf("found %d running prebuilds so far, want %d", len(rows), expectedPrebuildsCount) @@ -447,22 +456,29 @@ func TestClaimPrebuild_CheckDifferentErrors(t *testing.T) { runningPrebuilds := make(map[uuid.UUID]database.GetRunningPrebuiltWorkspacesRow, desiredInstances*presetCount) require.Eventually(t, func() bool { rows, err := errorStore.GetRunningPrebuiltWorkspaces(ctx) - require.NoError(t, err) + if err != nil { + return false + } for _, row := range rows { runningPrebuilds[row.CurrentPresetID.UUID] = row agents, err := db.GetWorkspaceAgentsInLatestBuildByWorkspaceID(ctx, row.ID) - require.NoError(t, err) + if err != nil { + return false + } // Workspaces are eligible once its agent is marked "ready". for _, agent := range agents { - require.NoError(t, db.UpdateWorkspaceAgentLifecycleStateByID(ctx, database.UpdateWorkspaceAgentLifecycleStateByIDParams{ + err = db.UpdateWorkspaceAgentLifecycleStateByID(ctx, database.UpdateWorkspaceAgentLifecycleStateByIDParams{ ID: agent.ID, LifecycleState: database.WorkspaceAgentLifecycleStateReady, StartedAt: sql.NullTime{Time: time.Now().Add(time.Hour), Valid: true}, ReadyAt: sql.NullTime{Time: time.Now().Add(-1 * time.Hour), Valid: true}, - })) + }) + if err != nil { + return false + } } } From ff9eeb42278054ee18dfa907597ac142a2f97b90 Mon Sep 17 00:00:00 2001 From: Edward Angert Date: Wed, 23 Apr 2025 16:13:13 -0400 Subject: [PATCH 23/34] docs: fix ssh coder example in testing-templates doc (#17550) from @NickSquangler and @angrycub on Slack Co-authored-by: EdwardAngert <17991901+EdwardAngert@users.noreply.github.com> --- docs/tutorials/testing-templates.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/tutorials/testing-templates.md b/docs/tutorials/testing-templates.md index c3572286049e0..45250a6a71aac 100644 --- a/docs/tutorials/testing-templates.md +++ b/docs/tutorials/testing-templates.md @@ -105,7 +105,7 @@ jobs: coder create -t $TEMPLATE_NAME --template-version ${{ steps.name.outputs.version_name }} test-${{ steps.name.outputs.version_name }} --yes coder config-ssh --yes # run some example commands - coder ssh test-${{ steps.name.outputs.version_name }} -- make build + ssh coder.test-${{ steps.name.outputs.version_name }} -- make build - name: Delete the test workspace if: always() From 31b873fa57ac4b8c7fbd01f60c3eef55db8f923b Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Thu, 24 Apr 2025 00:19:25 +0100 Subject: [PATCH 24/34] fix: add websocket close handling (#17548) resolves #17508 Display an error in the UI that the websocket closed if the user is still interacting with the dynamic parameters form Screenshot 2025-04-23 at 17 57 25 --- site/src/api/api.ts | 6 +++ .../CreateWorkspacePageExperimental.tsx | 13 +++++- ...eWorkspacePageViewExperimental.stories.tsx | 40 +++++++++++++++++++ 3 files changed, 57 insertions(+), 2 deletions(-) create mode 100644 site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.stories.tsx diff --git a/site/src/api/api.ts b/site/src/api/api.ts index fa62afadee608..b3ce8bd0cf471 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -1015,9 +1015,11 @@ class ApiMethods { { onMessage, onError, + onClose, }: { onMessage: (response: TypesGen.DynamicParametersResponse) => void; onError: (error: Error) => void; + onClose: () => void; }, ): WebSocket => { const socket = createWebSocket( @@ -1033,6 +1035,10 @@ class ApiMethods { socket.close(); }); + socket.addEventListener("close", () => { + onClose(); + }); + return socket; }; diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx index 03da3bd477745..c02529c5d9446 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx @@ -1,4 +1,4 @@ -import type { ApiErrorResponse } from "api/errors"; +import { type ApiErrorResponse, DetailedError } from "api/errors"; import { checkAuthorization } from "api/queries/authCheck"; import { templateByName, @@ -107,6 +107,15 @@ const CreateWorkspacePageExperimental: FC = () => { onError: (error) => { setWsError(error); }, + onClose: () => { + // There is no reason for the websocket to close while a user is on the page + setWsError( + new DetailedError( + "Websocket connection for dynamic parameters unexpectedly closed.", + "Refresh the page to reset the form.", + ), + ); + }, }, ); @@ -141,7 +150,7 @@ const CreateWorkspacePageExperimental: FC = () => { } = useExternalAuth(realizedVersionId); const isLoadingFormData = - ws.current?.readyState !== WebSocket.OPEN || + ws.current?.readyState === WebSocket.CONNECTING || templateQuery.isLoading || permissionsQuery.isLoading; const loadFormDataError = templateQuery.error ?? permissionsQuery.error; diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.stories.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.stories.tsx new file mode 100644 index 0000000000000..a41e3a48c0ad9 --- /dev/null +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.stories.tsx @@ -0,0 +1,40 @@ +import type { Meta, StoryObj } from "@storybook/react"; +import { DetailedError } from "api/errors"; +import { chromatic } from "testHelpers/chromatic"; +import { MockTemplate, MockUser } from "testHelpers/entities"; +import { CreateWorkspacePageViewExperimental } from "./CreateWorkspacePageViewExperimental"; + +const meta: Meta = { + title: "Pages/CreateWorkspacePageViewExperimental", + parameters: { chromatic }, + component: CreateWorkspacePageViewExperimental, + args: { + autofillParameters: [], + diagnostics: [], + defaultName: "", + defaultOwner: MockUser, + externalAuth: [], + externalAuthPollingState: "idle", + hasAllRequiredExternalAuth: true, + mode: "form", + parameters: [], + permissions: { + createWorkspaceForAny: true, + }, + presets: [], + sendMessage: () => {}, + template: MockTemplate, + }, +}; + +export default meta; +type Story = StoryObj; + +export const WebsocketError: Story = { + args: { + error: new DetailedError( + "Websocket connection for dynamic parameters unexpectedly closed.", + "Refresh the page to reset the form.", + ), + }, +}; From 4fe770dd55ca00df51a4201fd446a0dc8aa5a2c5 Mon Sep 17 00:00:00 2001 From: Ethan <39577870+ethanndickson@users.noreply.github.com> Date: Thu, 24 Apr 2025 13:02:57 +1000 Subject: [PATCH 25/34] ci: move go install tools to separate action (#17552) I think using an older version of mockgen on the schmoder CI broke the workflow, so I'm gonna sync it via this action, like we do with the other `make build` dependencies. --- .github/actions/setup-go-tools/action.yaml | 14 ++++++++++++++ .github/workflows/ci.yaml | 14 ++------------ 2 files changed, 16 insertions(+), 12 deletions(-) create mode 100644 .github/actions/setup-go-tools/action.yaml diff --git a/.github/actions/setup-go-tools/action.yaml b/.github/actions/setup-go-tools/action.yaml new file mode 100644 index 0000000000000..9c08a7d417b13 --- /dev/null +++ b/.github/actions/setup-go-tools/action.yaml @@ -0,0 +1,14 @@ +name: "Setup Go tools" +description: | + Set up tools for `make gen`, `offlinedocs` and Schmoder CI. +runs: + using: "composite" + steps: + - name: go install tools + shell: bash + run: | + go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.30 + go install storj.io/drpc/cmd/protoc-gen-go-drpc@v0.0.34 + go install golang.org/x/tools/cmd/goimports@v0.31.0 + go install github.com/mikefarah/yq/v4@v4.44.3 + go install go.uber.org/mock/mockgen@v0.5.0 diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 54239330f2a4f..6a0d3b621cf0f 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -249,12 +249,7 @@ jobs: uses: ./.github/actions/setup-tf - name: go install tools - run: | - go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.30 - go install storj.io/drpc/cmd/protoc-gen-go-drpc@v0.0.34 - go install golang.org/x/tools/cmd/goimports@v0.31.0 - go install github.com/mikefarah/yq/v4@v4.44.3 - go install go.uber.org/mock/mockgen@v0.5.0 + uses: ./.github/actions/setup-go-tools - name: Install Protoc run: | @@ -860,12 +855,7 @@ jobs: uses: ./.github/actions/setup-go - name: Install go tools - run: | - go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.30 - go install storj.io/drpc/cmd/protoc-gen-go-drpc@v0.0.34 - go install golang.org/x/tools/cmd/goimports@v0.31.0 - go install github.com/mikefarah/yq/v4@v4.44.3 - go install go.uber.org/mock/mockgen@v0.5.0 + uses: ./.github/actions/setup-go-tools - name: Setup sqlc uses: ./.github/actions/setup-sqlc From 8d8fc9938958c2dce4a4ee6da6b26f8de496af35 Mon Sep 17 00:00:00 2001 From: M Atif Ali Date: Thu, 24 Apr 2025 12:21:31 +0500 Subject: [PATCH 26/34] chore(dogfood): allow provider minor version updates (#17554) --- dogfood/coder/main.tf | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dogfood/coder/main.tf b/dogfood/coder/main.tf index e9df01a4a12f3..275a4f4b6f9fc 100644 --- a/dogfood/coder/main.tf +++ b/dogfood/coder/main.tf @@ -2,11 +2,11 @@ terraform { required_providers { coder = { source = "coder/coder" - version = "2.3.0" + version = "~> 2.0" } docker = { source = "kreuzwerker/docker" - version = "~> 3.0.0" + version = "~> 3.0" } } } From af3232529986987ad4501b044cedb33c93cf7a48 Mon Sep 17 00:00:00 2001 From: Aericio <16523741+Aericio@users.noreply.github.com> Date: Wed, 23 Apr 2025 21:21:35 -1000 Subject: [PATCH 27/34] fix(examples/templates/docker-devcontainer): update folder path and provider version constraint (#17553) Co-authored-by: M Atif Ali --- examples/templates/docker-devcontainer/main.tf | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/examples/templates/docker-devcontainer/main.tf b/examples/templates/docker-devcontainer/main.tf index d0f328ea46f38..52877214caa7c 100644 --- a/examples/templates/docker-devcontainer/main.tf +++ b/examples/templates/docker-devcontainer/main.tf @@ -2,7 +2,7 @@ terraform { required_providers { coder = { source = "coder/coder" - version = "~> 1.0.0" + version = "~> 2.0" } docker = { source = "kreuzwerker/docker" @@ -340,11 +340,11 @@ module "jetbrains_gateway" { source = "registry.coder.com/modules/jetbrains-gateway/coder" # JetBrains IDEs to make available for the user to select - jetbrains_ides = ["IU", "PY", "WS", "PS", "RD", "CL", "GO", "RM"] + jetbrains_ides = ["IU", "PS", "WS", "PY", "CL", "GO", "RM", "RD", "RR"] default = "IU" # Default folder to open when starting a JetBrains IDE - folder = "/home/coder" + folder = "/workspaces" # This ensures that the latest version of the module gets downloaded, you can also pin the module version to prevent breaking changes in production. version = ">= 1.0.0" From 0826e8a5c838e0a632051dda4bb03565771a17f5 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Thu, 24 Apr 2025 09:54:00 +0200 Subject: [PATCH 28/34] feat: enable masking password inputs instead of blocking echo (#17469) Closes #17059 --- cli/cliui/prompt.go | 73 +++++++++++++++++++++++++++++++++++----- cli/cliui/prompt_test.go | 66 +++++++++++++++++++++++++++--------- go.mod | 1 - go.sum | 2 -- 4 files changed, 116 insertions(+), 26 deletions(-) diff --git a/cli/cliui/prompt.go b/cli/cliui/prompt.go index b432f75afeaaf..264ebf2939780 100644 --- a/cli/cliui/prompt.go +++ b/cli/cliui/prompt.go @@ -1,6 +1,7 @@ package cliui import ( + "bufio" "bytes" "encoding/json" "fmt" @@ -8,19 +9,21 @@ import ( "os" "os/signal" "strings" + "unicode" - "github.com/bgentry/speakeasy" "github.com/mattn/go-isatty" "golang.org/x/xerrors" + "github.com/coder/coder/v2/pty" "github.com/coder/pretty" "github.com/coder/serpent" ) // PromptOptions supply a set of options to the prompt. type PromptOptions struct { - Text string - Default string + Text string + Default string + // When true, the input will be masked with asterisks. Secret bool IsConfirm bool Validate func(string) error @@ -88,14 +91,13 @@ func Prompt(inv *serpent.Invocation, opts PromptOptions) (string, error) { var line string var err error + signal.Notify(interrupt, os.Interrupt) + defer signal.Stop(interrupt) + inFile, isInputFile := inv.Stdin.(*os.File) if opts.Secret && isInputFile && isatty.IsTerminal(inFile.Fd()) { - // we don't install a signal handler here because speakeasy has its own - line, err = speakeasy.Ask("") + line, err = readSecretInput(inFile, inv.Stdout) } else { - signal.Notify(interrupt, os.Interrupt) - defer signal.Stop(interrupt) - line, err = readUntil(inv.Stdin, '\n') // Check if the first line beings with JSON object or array chars. @@ -204,3 +206,58 @@ func readUntil(r io.Reader, delim byte) (string, error) { } } } + +// readSecretInput reads secret input from the terminal rune-by-rune, +// masking each character with an asterisk. +func readSecretInput(f *os.File, w io.Writer) (string, error) { + // Put terminal into raw mode (no echo, no line buffering). + oldState, err := pty.MakeInputRaw(f.Fd()) + if err != nil { + return "", err + } + defer func() { + _ = pty.RestoreTerminal(f.Fd(), oldState) + }() + + reader := bufio.NewReader(f) + var runes []rune + + for { + r, _, err := reader.ReadRune() + if err != nil { + return "", err + } + + switch { + case r == '\r' || r == '\n': + // Finish on Enter + if _, err := fmt.Fprint(w, "\r\n"); err != nil { + return "", err + } + return string(runes), nil + + case r == 3: + // Ctrl+C + return "", ErrCanceled + + case r == 127 || r == '\b': + // Backspace/Delete: remove last rune + if len(runes) > 0 { + // Erase the last '*' on the screen + if _, err := fmt.Fprint(w, "\b \b"); err != nil { + return "", err + } + runes = runes[:len(runes)-1] + } + + default: + // Only mask printable, non-control runes + if !unicode.IsControl(r) { + runes = append(runes, r) + if _, err := fmt.Fprint(w, "*"); err != nil { + return "", err + } + } + } + } +} diff --git a/cli/cliui/prompt_test.go b/cli/cliui/prompt_test.go index 5ac0d906caae8..8b5a3e98ea1f7 100644 --- a/cli/cliui/prompt_test.go +++ b/cli/cliui/prompt_test.go @@ -6,6 +6,7 @@ import ( "io" "os" "os/exec" + "runtime" "testing" "github.com/stretchr/testify/assert" @@ -13,7 +14,6 @@ import ( "golang.org/x/xerrors" "github.com/coder/coder/v2/cli/cliui" - "github.com/coder/coder/v2/pty" "github.com/coder/coder/v2/pty/ptytest" "github.com/coder/coder/v2/testutil" "github.com/coder/serpent" @@ -181,6 +181,48 @@ func TestPrompt(t *testing.T) { resp := testutil.TryReceive(ctx, t, doneChan) require.Equal(t, "valid", resp) }) + + t.Run("MaskedSecret", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitShort) + ptty := ptytest.New(t) + doneChan := make(chan string) + go func() { + resp, err := newPrompt(ctx, ptty, cliui.PromptOptions{ + Text: "Password:", + Secret: true, + }, nil) + assert.NoError(t, err) + doneChan <- resp + }() + ptty.ExpectMatch("Password: ") + + ptty.WriteLine("test") + + resp := testutil.TryReceive(ctx, t, doneChan) + require.Equal(t, "test", resp) + }) + + t.Run("UTF8Password", func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitShort) + ptty := ptytest.New(t) + doneChan := make(chan string) + go func() { + resp, err := newPrompt(ctx, ptty, cliui.PromptOptions{ + Text: "Password:", + Secret: true, + }, nil) + assert.NoError(t, err) + doneChan <- resp + }() + ptty.ExpectMatch("Password: ") + + ptty.WriteLine("和製漢字") + + resp := testutil.TryReceive(ctx, t, doneChan) + require.Equal(t, "和製漢字", resp) + }) } func newPrompt(ctx context.Context, ptty *ptytest.PTY, opts cliui.PromptOptions, invOpt func(inv *serpent.Invocation)) (string, error) { @@ -209,13 +251,12 @@ func TestPasswordTerminalState(t *testing.T) { passwordHelper() return } + if runtime.GOOS == "windows" { + t.Skip("Skipping on windows. PTY doesn't read ptty.Write correctly.") + } t.Parallel() ptty := ptytest.New(t) - ptyWithFlags, ok := ptty.PTY.(pty.WithFlags) - if !ok { - t.Skip("unable to check PTY local echo on this platform") - } cmd := exec.Command(os.Args[0], "-test.run=TestPasswordTerminalState") //nolint:gosec cmd.Env = append(os.Environ(), "TEST_SUBPROCESS=1") @@ -229,21 +270,16 @@ func TestPasswordTerminalState(t *testing.T) { defer process.Kill() ptty.ExpectMatch("Password: ") - - require.Eventually(t, func() bool { - echo, err := ptyWithFlags.EchoEnabled() - return err == nil && !echo - }, testutil.WaitShort, testutil.IntervalMedium, "echo is on while reading password") + ptty.Write('t') + ptty.Write('e') + ptty.Write('s') + ptty.Write('t') + ptty.ExpectMatch("****") err = process.Signal(os.Interrupt) require.NoError(t, err) _, err = process.Wait() require.NoError(t, err) - - require.Eventually(t, func() bool { - echo, err := ptyWithFlags.EchoEnabled() - return err == nil && echo - }, testutil.WaitShort, testutil.IntervalMedium, "echo is off after reading password") } // nolint:unused diff --git a/go.mod b/go.mod index 521ff2c44ddf6..59dcaf99a291e 100644 --- a/go.mod +++ b/go.mod @@ -83,7 +83,6 @@ require ( github.com/armon/circbuf v0.0.0-20190214190532-5111143e8da2 github.com/awalterschulze/gographviz v2.0.3+incompatible github.com/aws/smithy-go v1.22.3 - github.com/bgentry/speakeasy v0.2.0 github.com/bramvdbogaerde/go-scp v1.5.0 github.com/briandowns/spinner v1.23.0 github.com/cakturk/go-netstat v0.0.0-20200220111822-e5b49efee7a5 diff --git a/go.sum b/go.sum index fdcc9bc5b0296..809be5163de62 100644 --- a/go.sum +++ b/go.sum @@ -815,8 +815,6 @@ github.com/bep/tmc v0.5.1/go.mod h1:tGYHN8fS85aJPhDLgXETVKp+PR382OvFi2+q2GkGsq0= github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d h1:xDfNPAt8lFiC1UJrqV3uuy861HCTo708pDMbjHHdCas= github.com/bgentry/go-netrc v0.0.0-20140422174119-9fd32a8b3d3d/go.mod h1:6QX/PXZ00z/TKoufEY6K/a0k6AhaJrQKdFe6OfVXsa4= github.com/bgentry/speakeasy v0.1.0/go.mod h1:+zsyZBPWlz7T6j88CTgSN5bM796AkVf0kBD4zp0CCIs= -github.com/bgentry/speakeasy v0.2.0 h1:tgObeVOf8WAvtuAX6DhJ4xks4CFNwPDZiqzGqIHE51E= -github.com/bgentry/speakeasy v0.2.0/go.mod h1:+zsyZBPWlz7T6j88CTgSN5bM796AkVf0kBD4zp0CCIs= github.com/bmatcuk/doublestar/v4 v4.8.1 h1:54Bopc5c2cAvhLRAzqOGCYHYyhcDHsFF4wWIR5wKP38= github.com/bmatcuk/doublestar/v4 v4.8.1/go.mod h1:xBQ8jztBU6kakFMg+8WGxn0c6z1fTSPVIjEY1Wr7jzc= github.com/bool64/shared v0.1.5 h1:fp3eUhBsrSjNCQPcSdQqZxxh9bBwrYiZ+zOKFkM0/2E= From cbb6c121e78527116b655478a9a201f8a67147ff Mon Sep 17 00:00:00 2001 From: M Atif Ali Date: Thu, 24 Apr 2025 12:56:07 +0500 Subject: [PATCH 29/34] fix(examples/templates/kubernetes-devcontainer): update coder provider (#17555) --- examples/templates/kubernetes-devcontainer/main.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/templates/kubernetes-devcontainer/main.tf b/examples/templates/kubernetes-devcontainer/main.tf index c9a86f08df6d2..69e53565d3c78 100644 --- a/examples/templates/kubernetes-devcontainer/main.tf +++ b/examples/templates/kubernetes-devcontainer/main.tf @@ -2,7 +2,7 @@ terraform { required_providers { coder = { source = "coder/coder" - version = "~> 1.0.0" + version = "~> 2.0" } kubernetes = { source = "hashicorp/kubernetes" From e4668442e05c2c633fca07e8f8ef1391f1657f21 Mon Sep 17 00:00:00 2001 From: M Atif Ali Date: Thu, 24 Apr 2025 13:52:34 +0500 Subject: [PATCH 30/34] docs: add automatic release calendar updates in docs (#17531) --- .github/workflows/release.yaml | 52 ++++++++ docs/install/releases/index.md | 24 ++-- scripts/update-release-calendar.sh | 205 +++++++++++++++++++++++++++++ 3 files changed, 269 insertions(+), 12 deletions(-) create mode 100755 scripts/update-release-calendar.sh diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 94d7b6f9ae5e4..040054eb84cbc 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -924,3 +924,55 @@ jobs: continue-on-error: true run: | make sqlc-push + + update-calendar: + name: "Update release calendar in docs" + runs-on: "ubuntu-latest" + needs: [release, publish-homebrew, publish-winget, publish-sqlc] + if: ${{ !inputs.dry_run }} + permissions: + contents: write + pull-requests: write + steps: + - name: Harden Runner + uses: step-security/harden-runner@c6295a65d1254861815972266d5933fd6e532bdf # v2.11.1 + with: + egress-policy: audit + + - name: Checkout repository + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + with: + fetch-depth: 0 # Needed to get all tags for version calculation + + - name: Set up Git + run: | + git config user.name "Coder CI" + git config user.email "cdrci@coder.com" + + - name: Run update script + run: | + ./scripts/update-release-calendar.sh + make fmt/markdown + + - name: Check for changes + id: check_changes + run: | + if git diff --quiet docs/install/releases/index.md; then + echo "No changes detected in release calendar." + echo "changes=false" >> $GITHUB_OUTPUT + else + echo "Changes detected in release calendar." + echo "changes=true" >> $GITHUB_OUTPUT + fi + + - name: Create Pull Request + if: steps.check_changes.outputs.changes == 'true' + uses: peter-evans/create-pull-request@ff45666b9427631e3450c54a1bcbee4d9ff4d7c0 # v3.0.0 + with: + commit-message: "docs: update release calendar" + title: "docs: update release calendar" + body: | + This PR automatically updates the release calendar in the docs. + branch: bot/update-release-calendar + delete-branch: true + labels: docs diff --git a/docs/install/releases/index.md b/docs/install/releases/index.md index d0ab0d1a05d5e..09aca9f37cb9b 100644 --- a/docs/install/releases/index.md +++ b/docs/install/releases/index.md @@ -53,18 +53,18 @@ Best practices for installing Coder can be found on our [install](../index.md) pages. ## Release schedule - -| Release name | Release Date | Status | -|--------------|--------------------|------------------| -| 2.12.x | June 04, 2024 | Not Supported | -| 2.13.x | July 02, 2024 | Not Supported | -| 2.14.x | August 06, 2024 | Not Supported | -| 2.15.x | September 03, 2024 | Not Supported | -| 2.16.x | October 01, 2024 | Not Supported | -| 2.17.x | November 05, 2024 | Not Supported | -| 2.18.x | December 03, 2024 | Security Support | -| 2.19.x | February 04, 2024 | Stable | -| 2.20.x | March 05, 2024 | Mainline | + + +| Release name | Release Date | Status | Latest Release | +|------------------------------------------------|-------------------|------------------|----------------------------------------------------------------| +| [2.16](https://coder.com/changelog/coder-2-16) | November 05, 2024 | Not Supported | [v2.16.1](https://github.com/coder/coder/releases/tag/v2.16.1) | +| [2.17](https://coder.com/changelog/coder-2-17) | December 03, 2024 | Not Supported | [v2.17.3](https://github.com/coder/coder/releases/tag/v2.17.3) | +| [2.18](https://coder.com/changelog/coder-2-18) | February 04, 2025 | Not Supported | [v2.18.5](https://github.com/coder/coder/releases/tag/v2.18.5) | +| [2.19](https://coder.com/changelog/coder-2-19) | February 04, 2025 | Security Support | [v2.19.1](https://github.com/coder/coder/releases/tag/v2.19.1) | +| [2.20](https://coder.com/changelog/coder-2-20) | March 04, 2025 | Stable | [v2.20.2](https://github.com/coder/coder/releases/tag/v2.20.2) | +| [2.21](https://coder.com/changelog/coder-2-21) | April 01, 2025 | Mainline | [v2.21.1](https://github.com/coder/coder/releases/tag/v2.21.1) | +| 2.22 | May 07, 2024 | Not Released | N/A | + > [!TIP] > We publish a diff --git a/scripts/update-release-calendar.sh b/scripts/update-release-calendar.sh new file mode 100755 index 0000000000000..a9fe6e54d605d --- /dev/null +++ b/scripts/update-release-calendar.sh @@ -0,0 +1,205 @@ +#!/bin/bash + +set -euo pipefail + +# This script automatically updates the release calendar in docs/install/releases/index.md +# It calculates the releases based on the first Tuesday of each month rule +# and updates the status of each release (Not Supported, Security Support, Stable, Mainline, Not Released) + +DOCS_FILE="docs/install/releases/index.md" + +# Define unique markdown comments as anchors +CALENDAR_START_MARKER="" +CALENDAR_END_MARKER="" + +# Get current date +current_date=$(date +"%Y-%m-%d") +current_month=$(date +"%m") +current_year=$(date +"%Y") + +# Function to get the first Tuesday of a given month and year +get_first_tuesday() { + local year=$1 + local month=$2 + local first_day + local days_until_tuesday + local first_tuesday + + # Find the first day of the month + first_day=$(date -d "$year-$month-01" +"%u") + + # Calculate days until first Tuesday (if day 1 is Tuesday, first_day=2) + days_until_tuesday=$((first_day == 2 ? 0 : (9 - first_day) % 7)) + + # Get the date of the first Tuesday + first_tuesday=$(date -d "$year-$month-01 +$days_until_tuesday days" +"%Y-%m-%d") + + echo "$first_tuesday" +} + +# Function to format date as "Month DD, YYYY" +format_date() { + date -d "$1" +"%B %d, %Y" +} + +# Function to get the latest patch version for a minor release +get_latest_patch() { + local version_major=$1 + local version_minor=$2 + local tags + local latest + + # Get all tags for this minor version + tags=$(cd "$(git rev-parse --show-toplevel)" && git tag | grep "^v$version_major\\.$version_minor\\." | sort -V) + + # Get the latest one + latest=$(echo "$tags" | tail -1) + + if [ -z "$latest" ]; then + # If no tags found, return empty + echo "" + else + # Return without the v prefix + echo "${latest#v}" + fi +} + +# Generate releases table showing: +# - 3 previous unsupported releases +# - 1 security support release (n-2) +# - 1 stable release (n-1) +# - 1 mainline release (n) +# - 1 next release (n+1) +generate_release_calendar() { + local result="" + local version_major=2 + local latest_version + local version_minor + local start_minor + + # Find the current minor version by looking at the last mainline release tag + latest_version=$(cd "$(git rev-parse --show-toplevel)" && git tag | grep '^v[0-9]*\.[0-9]*\.[0-9]*$' | sort -V | tail -1) + version_minor=$(echo "$latest_version" | cut -d. -f2) + + # Start with 3 unsupported releases back + start_minor=$((version_minor - 5)) + + # Initialize the calendar table with an additional column for latest release + result="| Release name | Release Date | Status | Latest Release |\n" + result+="|--------------|--------------|--------|----------------|\n" + + # Generate rows for each release (7 total: 3 unsupported, 1 security, 1 stable, 1 mainline, 1 next) + for i in {0..6}; do + # Calculate release minor version + local rel_minor=$((start_minor + i)) + # Format release name without the .x + local version_name="$version_major.$rel_minor" + local release_date + local formatted_date + local latest_patch + local patch_link + local status + local formatted_version_name + + # Calculate release month and year based on release pattern + # This is a simplified calculation assuming monthly releases + local rel_month=$(((current_month - (5 - i) + 12) % 12)) + [[ $rel_month -eq 0 ]] && rel_month=12 + local rel_year=$current_year + if [[ $rel_month -gt $current_month ]]; then + rel_year=$((rel_year - 1)) + fi + if [[ $rel_month -lt $current_month && $i -gt 5 ]]; then + rel_year=$((rel_year + 1)) + fi + + # Skip January releases starting from 2025 + if [[ $rel_month -eq 1 && $rel_year -ge 2025 ]]; then + rel_month=2 + # No need to reassign rel_year to itself + fi + + # Get release date (first Tuesday of the month) + release_date=$(get_first_tuesday "$rel_year" "$(printf "%02d" "$rel_month")") + formatted_date=$(format_date "$release_date") + + # Get latest patch version + latest_patch=$(get_latest_patch "$version_major" "$rel_minor") + if [ -n "$latest_patch" ]; then + patch_link="[v${latest_patch}](https://github.com/coder/coder/releases/tag/v${latest_patch})" + else + patch_link="N/A" + fi + + # Determine status + if [[ "$release_date" > "$current_date" ]]; then + status="Not Released" + elif [[ $i -eq 6 ]]; then + status="Not Released" + elif [[ $i -eq 5 ]]; then + status="Mainline" + elif [[ $i -eq 4 ]]; then + status="Stable" + elif [[ $i -eq 3 ]]; then + status="Security Support" + else + status="Not Supported" + fi + + # Format version name and patch link based on release status + # No links for unreleased versions + if [[ "$status" == "Not Released" ]]; then + formatted_version_name="$version_name" + patch_link="N/A" + else + formatted_version_name="[$version_name](https://coder.com/changelog/coder-$version_major-$rel_minor)" + fi + + # Add row to table + result+="| $formatted_version_name | $formatted_date | $status | $patch_link |\n" + done + + echo -e "$result" +} + +# Check if the markdown comments exist in the file +if ! grep -q "$CALENDAR_START_MARKER" "$DOCS_FILE" || ! grep -q "$CALENDAR_END_MARKER" "$DOCS_FILE"; then + echo "Error: Markdown comment anchors not found in $DOCS_FILE" + echo "Please add the following anchors around the release calendar table:" + echo " $CALENDAR_START_MARKER" + echo " $CALENDAR_END_MARKER" + exit 1 +fi + +# Generate the new calendar table content +NEW_CALENDAR=$(generate_release_calendar) + +# Update the file while preserving the rest of the content +awk -v start_marker="$CALENDAR_START_MARKER" \ + -v end_marker="$CALENDAR_END_MARKER" \ + -v new_calendar="$NEW_CALENDAR" \ + ' + BEGIN { found_start = 0; found_end = 0; print_line = 1; } + $0 ~ start_marker { + print; + print new_calendar; + found_start = 1; + print_line = 0; + next; + } + $0 ~ end_marker { + found_end = 1; + print_line = 1; + print; + next; + } + print_line || !found_start || found_end { print } + ' "$DOCS_FILE" >"${DOCS_FILE}.new" + +# Replace the original file with the updated version +mv "${DOCS_FILE}.new" "$DOCS_FILE" + +# run make fmt/markdown +make fmt/markdown + +echo "Successfully updated release calendar in $DOCS_FILE" From f24557bf455093b9afd7abe45e7233ef502e2612 Mon Sep 17 00:00:00 2001 From: M Atif Ali Date: Thu, 24 Apr 2025 14:14:50 +0500 Subject: [PATCH 31/34] chore(dogfood): add windsurf module (#17558) --- dogfood/coder/main.tf | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/dogfood/coder/main.tf b/dogfood/coder/main.tf index 275a4f4b6f9fc..92f25cb13f62b 100644 --- a/dogfood/coder/main.tf +++ b/dogfood/coder/main.tf @@ -225,6 +225,14 @@ module "cursor" { folder = local.repo_dir } +module "windsurf" { + count = data.coder_workspace.me.start_count + source = "registry.coder.com/modules/windsurf/coder" + version = ">= 1.0.0" + agent_id = coder_agent.dev.id + folder = local.repo_dir +} + module "zed" { count = data.coder_workspace.me.start_count source = "./zed" From 53b38d7ec70b9c057a869ed2fa0742a6ba7cbc54 Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Thu, 24 Apr 2025 12:40:00 +0000 Subject: [PATCH 32/34] fix: use userCtx instead of prebuildCtx for claiming prebuild --- coderd/workspaces.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 94e43c67128d7..12b3787acf3d8 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -876,15 +876,13 @@ func requestTemplate(ctx context.Context, rw http.ResponseWriter, req codersdk.C } func claimPrebuild(ctx context.Context, claimer prebuilds.Claimer, db database.Store, logger slog.Logger, req codersdk.CreateWorkspaceRequest, owner workspaceOwner) (*database.Workspace, error) { - prebuildsCtx := dbauthz.AsPrebuildsOrchestrator(ctx) - - claimedID, err := claimer.Claim(prebuildsCtx, owner.ID, req.Name, req.TemplateVersionPresetID) + claimedID, err := claimer.Claim(ctx, owner.ID, req.Name, req.TemplateVersionPresetID) if err != nil { // TODO: enhance this by clarifying whether this *specific* prebuild failed or whether there are none to claim. return nil, xerrors.Errorf("claim prebuild: %w", err) } - lookup, err := db.GetWorkspaceByID(prebuildsCtx, *claimedID) + lookup, err := db.GetWorkspaceByID(ctx, *claimedID) if err != nil { logger.Error(ctx, "unable to find claimed workspace by ID", slog.Error(err), slog.F("claimed_prebuild_id", claimedID.String())) return nil, xerrors.Errorf("find claimed workspace by ID %q: %w", claimedID.String(), err) From 6c41e03f90b95635aee4e7b967a6cb446b903198 Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Thu, 24 Apr 2025 12:48:57 +0000 Subject: [PATCH 33/34] refactor: CR's fixes --- coderd/wsbuilder/wsbuilder.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/coderd/wsbuilder/wsbuilder.go b/coderd/wsbuilder/wsbuilder.go index 3a51a07c51491..942829004309c 100644 --- a/coderd/wsbuilder/wsbuilder.go +++ b/coderd/wsbuilder/wsbuilder.go @@ -647,11 +647,6 @@ func (b *Builder) findNewBuildParameterValue(name string) *codersdk.WorkspaceBui } func (b *Builder) getLastBuildParameters() ([]database.WorkspaceBuildParameter, error) { - // TODO: exclude preset params from this list instead of returning nothing? - if b.prebuildClaimedBy != uuid.Nil { - return nil, nil - } - if b.lastBuildParameters != nil { return *b.lastBuildParameters, nil } From 9173e9a5c4dd2913f93cfb4b8b1f82b21782568e Mon Sep 17 00:00:00 2001 From: evgeniy-scherbina Date: Thu, 24 Apr 2025 13:25:17 +0000 Subject: [PATCH 34/34] CR's fixes --- enterprise/coderd/prebuilds/claim_test.go | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/enterprise/coderd/prebuilds/claim_test.go b/enterprise/coderd/prebuilds/claim_test.go index 02ee874fd48fd..4f398724b8265 100644 --- a/enterprise/coderd/prebuilds/claim_test.go +++ b/enterprise/coderd/prebuilds/claim_test.go @@ -3,6 +3,7 @@ package prebuilds_test import ( "context" "database/sql" + "slices" "strings" "sync/atomic" "testing" @@ -125,7 +126,7 @@ func TestClaimPrebuild(t *testing.T) { t.Parallel() // Setup. - ctx := testutil.Context(t, testutil.WaitMedium) + ctx := testutil.Context(t, testutil.WaitSuperLong) db, pubsub := dbtestutil.NewDB(t) spy := newStoreSpy(db) expectedPrebuildsCount := desiredInstances * presetCount @@ -250,13 +251,9 @@ func TestClaimPrebuild(t *testing.T) { require.Equal(t, expectedPrebuildsCount-1, len(currentPrebuilds)) // Then: the claimed prebuild is now missing from the running prebuilds set. - var found bool - for _, prebuild := range currentPrebuilds { - if prebuild.ID == claimed.ID { - found = true - break - } - } + found := slices.ContainsFunc(currentPrebuilds, func(prebuild database.GetRunningPrebuiltWorkspacesRow) bool { + return prebuild.ID == claimed.ID + }) require.False(t, found, "claimed prebuild should not still be considered a running prebuild") // Then: reconciling at this point will provision a new prebuild to replace the claimed one. @@ -407,7 +404,7 @@ func TestClaimPrebuild_CheckDifferentErrors(t *testing.T) { t.Parallel() // Setup. - ctx := testutil.Context(t, testutil.WaitMedium) + ctx := testutil.Context(t, testutil.WaitSuperLong) db, pubsub := dbtestutil.NewDB(t) errorStore := newErrorStore(db, tc.claimingErr)