feat: claim prebuilds based on workspace parameters instead of preset id (#19279)

Closes https://github.com/coder/coder/issues/18356.

This change finds and selects a matching preset if one was not chosen
during workspace creation. This solidifies the relationship between
presets and parameters.

When a workspace is created without in explicitly chosen preset, it will
now still be eligible to claim a prebuilt workspace if one is available.
This commit is contained in:
Sas Swart
2025-08-20 11:02:53 +02:00
committed by GitHub
parent 5e84d257b7
commit f9a6adc704
15 changed files with 736 additions and 37 deletions
+8
View File
@@ -1837,6 +1837,14 @@ func (q *querier) FetchVolumesResourceMonitorsUpdatedAfter(ctx context.Context,
return q.db.FetchVolumesResourceMonitorsUpdatedAfter(ctx, updatedAt)
}
func (q *querier) FindMatchingPresetID(ctx context.Context, arg database.FindMatchingPresetIDParams) (uuid.UUID, error) {
_, err := q.GetTemplateVersionByID(ctx, arg.TemplateVersionID)
if err != nil {
return uuid.Nil, err
}
return q.db.FindMatchingPresetID(ctx, arg)
}
func (q *querier) GetAPIKeyByID(ctx context.Context, id string) (database.APIKey, error) {
return fetch(q.log, q.auth, q.db.GetAPIKeyByID)(ctx, id)
}
+16
View File
@@ -4965,6 +4965,22 @@ func (s *MethodTestSuite) TestPrebuilds() {
template, policy.ActionUse,
).Errors(sql.ErrNoRows)
}))
s.Run("FindMatchingPresetID", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) {
t1 := testutil.Fake(s.T(), faker, database.Template{})
tv := testutil.Fake(s.T(), faker, database.TemplateVersion{TemplateID: uuid.NullUUID{UUID: t1.ID, Valid: true}})
dbm.EXPECT().FindMatchingPresetID(gomock.Any(), database.FindMatchingPresetIDParams{
TemplateVersionID: tv.ID,
ParameterNames: []string{"test"},
ParameterValues: []string{"test"},
}).Return(uuid.Nil, nil).AnyTimes()
dbm.EXPECT().GetTemplateVersionByID(gomock.Any(), tv.ID).Return(tv, nil).AnyTimes()
dbm.EXPECT().GetTemplateByID(gomock.Any(), t1.ID).Return(t1, nil).AnyTimes()
check.Args(database.FindMatchingPresetIDParams{
TemplateVersionID: tv.ID,
ParameterNames: []string{"test"},
ParameterValues: []string{"test"},
}).Asserts(tv.RBACObject(t1), policy.ActionRead).Returns(uuid.Nil)
}))
s.Run("GetPrebuildMetrics", s.Subtest(func(_ database.Store, check *expects) {
check.Args().
Asserts(rbac.ResourceWorkspace.All(), policy.ActionRead)
@@ -565,6 +565,13 @@ func (m queryMetricsStore) FetchVolumesResourceMonitorsUpdatedAfter(ctx context.
return r0, r1
}
func (m queryMetricsStore) FindMatchingPresetID(ctx context.Context, arg database.FindMatchingPresetIDParams) (uuid.UUID, error) {
start := time.Now()
r0, r1 := m.s.FindMatchingPresetID(ctx, arg)
m.queryLatencies.WithLabelValues("FindMatchingPresetID").Observe(time.Since(start).Seconds())
return r0, r1
}
func (m queryMetricsStore) GetAPIKeyByID(ctx context.Context, id string) (database.APIKey, error) {
start := time.Now()
apiKey, err := m.s.GetAPIKeyByID(ctx, id)
+15
View File
@@ -1051,6 +1051,21 @@ func (mr *MockStoreMockRecorder) FetchVolumesResourceMonitorsUpdatedAfter(ctx, u
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FetchVolumesResourceMonitorsUpdatedAfter", reflect.TypeOf((*MockStore)(nil).FetchVolumesResourceMonitorsUpdatedAfter), ctx, updatedAt)
}
// FindMatchingPresetID mocks base method.
func (m *MockStore) FindMatchingPresetID(ctx context.Context, arg database.FindMatchingPresetIDParams) (uuid.UUID, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "FindMatchingPresetID", ctx, arg)
ret0, _ := ret[0].(uuid.UUID)
ret1, _ := ret[1].(error)
return ret0, ret1
}
// FindMatchingPresetID indicates an expected call of FindMatchingPresetID.
func (mr *MockStoreMockRecorder) FindMatchingPresetID(ctx, arg any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "FindMatchingPresetID", reflect.TypeOf((*MockStore)(nil).FindMatchingPresetID), ctx, arg)
}
// GetAPIKeyByID mocks base method.
func (m *MockStore) GetAPIKeyByID(ctx context.Context, id string) (database.APIKey, error) {
m.ctrl.T.Helper()
+5
View File
@@ -137,6 +137,11 @@ type sqlcQuerier interface {
FetchNewMessageMetadata(ctx context.Context, arg FetchNewMessageMetadataParams) (FetchNewMessageMetadataRow, error)
FetchVolumesResourceMonitorsByAgentID(ctx context.Context, agentID uuid.UUID) ([]WorkspaceAgentVolumeResourceMonitor, error)
FetchVolumesResourceMonitorsUpdatedAfter(ctx context.Context, updatedAt time.Time) ([]WorkspaceAgentVolumeResourceMonitor, error)
// FindMatchingPresetID finds a preset ID that is the largest exact subset of the provided parameters.
// It returns the preset ID if a match is found, or NULL if no match is found.
// The query finds presets where all preset parameters are present in the provided parameters,
// and returns the preset with the most parameters (largest subset).
FindMatchingPresetID(ctx context.Context, arg FindMatchingPresetIDParams) (uuid.UUID, error)
GetAPIKeyByID(ctx context.Context, id string) (APIKey, error)
// there is no unique constraint on empty token names
GetAPIKeyByName(ctx context.Context, arg GetAPIKeyByNameParams) (APIKey, error)
+41
View File
@@ -7252,6 +7252,47 @@ func (q *sqlQuerier) CountInProgressPrebuilds(ctx context.Context) ([]CountInPro
return items, nil
}
const findMatchingPresetID = `-- name: FindMatchingPresetID :one
WITH provided_params AS (
SELECT
unnest($1::text[]) AS name,
unnest($2::text[]) AS value
),
preset_matches AS (
SELECT
tvp.id AS template_version_preset_id,
COALESCE(COUNT(tvpp.name), 0) AS total_preset_params,
COALESCE(COUNT(pp.name), 0) AS matching_params
FROM template_version_presets tvp
LEFT JOIN template_version_preset_parameters tvpp ON tvpp.template_version_preset_id = tvp.id
LEFT JOIN provided_params pp ON pp.name = tvpp.name AND pp.value = tvpp.value
WHERE tvp.template_version_id = $3
GROUP BY tvp.id
)
SELECT pm.template_version_preset_id
FROM preset_matches pm
WHERE pm.total_preset_params = pm.matching_params -- All preset parameters must match
ORDER BY pm.total_preset_params DESC -- Return the preset with the most parameters
LIMIT 1
`
type FindMatchingPresetIDParams struct {
ParameterNames []string `db:"parameter_names" json:"parameter_names"`
ParameterValues []string `db:"parameter_values" json:"parameter_values"`
TemplateVersionID uuid.UUID `db:"template_version_id" json:"template_version_id"`
}
// FindMatchingPresetID finds a preset ID that is the largest exact subset of the provided parameters.
// It returns the preset ID if a match is found, or NULL if no match is found.
// The query finds presets where all preset parameters are present in the provided parameters,
// and returns the preset with the most parameters (largest subset).
func (q *sqlQuerier) FindMatchingPresetID(ctx context.Context, arg FindMatchingPresetIDParams) (uuid.UUID, error) {
row := q.db.QueryRowContext(ctx, findMatchingPresetID, pq.Array(arg.ParameterNames), pq.Array(arg.ParameterValues), arg.TemplateVersionID)
var template_version_preset_id uuid.UUID
err := row.Scan(&template_version_preset_id)
return template_version_preset_id, err
}
const getPrebuildMetrics = `-- name: GetPrebuildMetrics :many
SELECT
t.name as template_name,
+27
View File
@@ -245,3 +245,30 @@ INNER JOIN organizations o ON o.id = w.organization_id
WHERE NOT t.deleted AND wpb.build_number = 1
GROUP BY t.name, tvp.name, o.name
ORDER BY t.name, tvp.name, o.name;
-- name: FindMatchingPresetID :one
-- FindMatchingPresetID finds a preset ID that is the largest exact subset of the provided parameters.
-- It returns the preset ID if a match is found, or NULL if no match is found.
-- The query finds presets where all preset parameters are present in the provided parameters,
-- and returns the preset with the most parameters (largest subset).
WITH provided_params AS (
SELECT
unnest(@parameter_names::text[]) AS name,
unnest(@parameter_values::text[]) AS value
),
preset_matches AS (
SELECT
tvp.id AS template_version_preset_id,
COALESCE(COUNT(tvpp.name), 0) AS total_preset_params,
COALESCE(COUNT(pp.name), 0) AS matching_params
FROM template_version_presets tvp
LEFT JOIN template_version_preset_parameters tvpp ON tvpp.template_version_preset_id = tvp.id
LEFT JOIN provided_params pp ON pp.name = tvpp.name AND pp.value = tvpp.value
WHERE tvp.template_version_id = @template_version_id
GROUP BY tvp.id
)
SELECT pm.template_version_preset_id
FROM preset_matches pm
WHERE pm.total_preset_params = pm.matching_params -- All preset parameters must match
ORDER BY pm.total_preset_params DESC -- Return the preset with the most parameters
LIMIT 1;
+42
View File
@@ -0,0 +1,42 @@
package prebuilds
import (
"context"
"database/sql"
"errors"
"github.com/google/uuid"
"golang.org/x/xerrors"
"github.com/coder/coder/v2/coderd/database"
)
// FindMatchingPresetID finds a preset ID that matches the provided parameters.
// It returns the preset ID if a match is found, or uuid.Nil if no match is found.
// The function performs a bidirectional comparison to ensure all parameters match exactly.
func FindMatchingPresetID(
ctx context.Context,
store database.Store,
templateVersionID uuid.UUID,
parameterNames []string,
parameterValues []string,
) (uuid.UUID, error) {
if len(parameterNames) != len(parameterValues) {
return uuid.Nil, xerrors.New("parameter names and values must have the same length")
}
result, err := store.FindMatchingPresetID(ctx, database.FindMatchingPresetIDParams{
TemplateVersionID: templateVersionID,
ParameterNames: parameterNames,
ParameterValues: parameterValues,
})
if err != nil {
// Handle the case where no matching preset is found (no rows returned)
if errors.Is(err, sql.ErrNoRows) {
return uuid.Nil, nil
}
return uuid.Nil, xerrors.Errorf("find matching preset ID: %w", err)
}
return result, nil
}
+198
View File
@@ -0,0 +1,198 @@
package prebuilds_test
import (
"testing"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/dbgen"
"github.com/coder/coder/v2/coderd/database/dbtestutil"
"github.com/coder/coder/v2/coderd/prebuilds"
"github.com/coder/coder/v2/testutil"
)
func TestFindMatchingPresetID(t *testing.T) {
t.Parallel()
presetIDs := []uuid.UUID{
uuid.New(),
uuid.New(),
}
// Give each preset a meaningful name in alphabetical order
presetNames := map[uuid.UUID]string{
presetIDs[0]: "development",
presetIDs[1]: "production",
}
tests := []struct {
name string
parameterNames []string
parameterValues []string
presetParameters []database.TemplateVersionPresetParameter
expectedPresetID uuid.UUID
expectError bool
errorContains string
}{
{
name: "exact match",
parameterNames: []string{"region", "instance_type"},
parameterValues: []string{"us-west-2", "t3.medium"},
presetParameters: []database.TemplateVersionPresetParameter{
{TemplateVersionPresetID: presetIDs[0], Name: "region", Value: "us-west-2"},
{TemplateVersionPresetID: presetIDs[0], Name: "instance_type", Value: "t3.medium"},
// antagonist:
{TemplateVersionPresetID: presetIDs[1], Name: "region", Value: "us-west-2"},
{TemplateVersionPresetID: presetIDs[1], Name: "instance_type", Value: "t3.large"},
},
expectedPresetID: presetIDs[0],
expectError: false,
},
{
name: "no match - different values",
parameterNames: []string{"region", "instance_type"},
parameterValues: []string{"us-east-1", "t3.medium"},
presetParameters: []database.TemplateVersionPresetParameter{
{TemplateVersionPresetID: presetIDs[0], Name: "region", Value: "us-west-2"},
{TemplateVersionPresetID: presetIDs[0], Name: "instance_type", Value: "t3.medium"},
// antagonist:
{TemplateVersionPresetID: presetIDs[1], Name: "region", Value: "us-west-2"},
{TemplateVersionPresetID: presetIDs[1], Name: "instance_type", Value: "t3.large"},
},
expectedPresetID: uuid.Nil,
expectError: false,
},
{
name: "no match - fewer provided parameters",
parameterNames: []string{"region"},
parameterValues: []string{"us-west-2"},
presetParameters: []database.TemplateVersionPresetParameter{
{TemplateVersionPresetID: presetIDs[0], Name: "region", Value: "us-west-2"},
{TemplateVersionPresetID: presetIDs[0], Name: "instance_type", Value: "t3.medium"},
// antagonist:
{TemplateVersionPresetID: presetIDs[1], Name: "region", Value: "us-west-2"},
{TemplateVersionPresetID: presetIDs[1], Name: "instance_type", Value: "t3.large"},
},
expectedPresetID: uuid.Nil,
expectError: false,
},
{
name: "subset match - extra provided parameter",
parameterNames: []string{"region", "instance_type", "extra_param"},
parameterValues: []string{"us-west-2", "t3.medium", "extra_value"},
presetParameters: []database.TemplateVersionPresetParameter{
{TemplateVersionPresetID: presetIDs[0], Name: "region", Value: "us-west-2"},
{TemplateVersionPresetID: presetIDs[0], Name: "instance_type", Value: "t3.medium"},
// antagonist:
{TemplateVersionPresetID: presetIDs[1], Name: "region", Value: "us-west-2"},
{TemplateVersionPresetID: presetIDs[1], Name: "instance_type", Value: "t3.large"},
},
expectedPresetID: presetIDs[0], // Should match because all preset parameters are present
expectError: false,
},
{
name: "mismatched parameter names vs values",
parameterNames: []string{"region", "instance_type"},
parameterValues: []string{"us-west-2"},
presetParameters: []database.TemplateVersionPresetParameter{},
expectedPresetID: uuid.Nil,
expectError: true,
errorContains: "parameter names and values must have the same length",
},
{
name: "multiple presets - match first",
parameterNames: []string{"region", "instance_type"},
parameterValues: []string{"us-west-2", "t3.medium"},
presetParameters: []database.TemplateVersionPresetParameter{
{TemplateVersionPresetID: presetIDs[0], Name: "region", Value: "us-west-2"},
{TemplateVersionPresetID: presetIDs[0], Name: "instance_type", Value: "t3.medium"},
{TemplateVersionPresetID: presetIDs[1], Name: "region", Value: "us-east-1"},
{TemplateVersionPresetID: presetIDs[1], Name: "instance_type", Value: "t3.large"},
},
expectedPresetID: presetIDs[0],
expectError: false,
},
{
name: "largest subset match",
parameterNames: []string{"region", "instance_type", "storage_size"},
parameterValues: []string{"us-west-2", "t3.medium", "100gb"},
presetParameters: []database.TemplateVersionPresetParameter{
{TemplateVersionPresetID: presetIDs[0], Name: "region", Value: "us-west-2"},
{TemplateVersionPresetID: presetIDs[0], Name: "instance_type", Value: "t3.medium"},
{TemplateVersionPresetID: presetIDs[1], Name: "region", Value: "us-west-2"},
},
expectedPresetID: presetIDs[0], // Should match the larger subset (2 params vs 1 param)
expectError: false,
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitShort)
db, _ := dbtestutil.NewDB(t)
org := dbgen.Organization(t, db, database.Organization{})
user := dbgen.User(t, db, database.User{})
templateVersion := dbgen.TemplateVersion(t, db, database.TemplateVersion{
OrganizationID: org.ID,
CreatedBy: user.ID,
JobID: uuid.New(),
})
// Group parameters by preset ID and create presets
presetMap := make(map[uuid.UUID][]database.TemplateVersionPresetParameter)
for _, param := range tt.presetParameters {
presetMap[param.TemplateVersionPresetID] = append(presetMap[param.TemplateVersionPresetID], param)
}
// Create presets and insert their parameters
for presetID, params := range presetMap {
// Create the preset
_, err := db.InsertPreset(ctx, database.InsertPresetParams{
ID: presetID,
TemplateVersionID: templateVersion.ID,
Name: presetNames[presetID],
CreatedAt: dbtestutil.NowInDefaultTimezone(),
})
require.NoError(t, err)
// Insert parameters for this preset
names := make([]string, len(params))
values := make([]string, len(params))
for i, param := range params {
names[i] = param.Name
values[i] = param.Value
}
_, err = db.InsertPresetParameters(ctx, database.InsertPresetParametersParams{
TemplateVersionPresetID: presetID,
Names: names,
Values: values,
})
require.NoError(t, err)
}
result, err := prebuilds.FindMatchingPresetID(
ctx,
db,
templateVersion.ID,
tt.parameterNames,
tt.parameterValues,
)
// Assert results
if tt.expectError {
require.Error(t, err)
if tt.errorContains != "" {
assert.Contains(t, err.Error(), tt.errorContains)
}
} else {
require.NoError(t, err)
assert.Equal(t, tt.expectedPresetID, result)
}
})
}
}
+26 -12
View File
@@ -1638,6 +1638,8 @@ func TestPostWorkspaceBuild(t *testing.T) {
t.Run("SetsPresetID", func(t *testing.T) {
t.Parallel()
ctx := testutil.Context(t, testutil.WaitLong)
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
user := coderdtest.CreateFirstUser(t, client)
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{
@@ -1645,9 +1647,20 @@ func TestPostWorkspaceBuild(t *testing.T) {
ProvisionPlan: []*proto.Response{{
Type: &proto.Response_Plan{
Plan: &proto.PlanComplete{
Presets: []*proto.Preset{{
Name: "test",
}},
Presets: []*proto.Preset{
{
Name: "autodetected",
},
{
Name: "manual",
Parameters: []*proto.PresetParameter{
{
Name: "param1",
Value: "value1",
},
},
},
},
},
},
}},
@@ -1655,28 +1668,29 @@ func TestPostWorkspaceBuild(t *testing.T) {
})
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
workspace := coderdtest.CreateWorkspace(t, client, template.ID)
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID)
require.Nil(t, workspace.LatestBuild.TemplateVersionPresetID)
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
presets, err := client.TemplateVersionPresets(ctx, version.ID)
require.NoError(t, err)
require.Equal(t, 1, len(presets))
require.Equal(t, "test", presets[0].Name)
require.Equal(t, 2, len(presets))
require.Equal(t, "autodetected", presets[0].Name)
require.Equal(t, "manual", presets[1].Name)
workspace := coderdtest.CreateWorkspace(t, client, template.ID)
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID)
// Preset ID was detected based on the workspace parameters:
require.Equal(t, presets[0].ID, *workspace.LatestBuild.TemplateVersionPresetID)
build, err := client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{
TemplateVersionID: version.ID,
Transition: codersdk.WorkspaceTransitionStart,
TemplateVersionPresetID: presets[0].ID,
TemplateVersionPresetID: presets[1].ID,
})
require.NoError(t, err)
require.NotNil(t, build.TemplateVersionPresetID)
workspace, err = client.Workspace(ctx, workspace.ID)
require.NoError(t, err)
require.Equal(t, presets[1].ID, *workspace.LatestBuild.TemplateVersionPresetID)
require.Equal(t, build.TemplateVersionPresetID, workspace.LatestBuild.TemplateVersionPresetID)
})
+31 -9
View File
@@ -638,14 +638,35 @@ func createWorkspace(
// Use injected Clock to allow time mocking in tests
now := api.Clock.Now()
// If a template preset was chosen, try claim a prebuilt workspace.
if req.TemplateVersionPresetID != uuid.Nil {
templateVersionPresetID := req.TemplateVersionPresetID
// If no preset was chosen, look for a matching preset by parameter values.
if templateVersionPresetID == uuid.Nil {
parameterNames := make([]string, len(req.RichParameterValues))
parameterValues := make([]string, len(req.RichParameterValues))
for i, parameter := range req.RichParameterValues {
parameterNames[i] = parameter.Name
parameterValues[i] = parameter.Value
}
var err error
templateVersionID := req.TemplateVersionID
if templateVersionID == uuid.Nil {
templateVersionID = template.ActiveVersionID
}
templateVersionPresetID, err = prebuilds.FindMatchingPresetID(ctx, db, templateVersionID, parameterNames, parameterValues)
if err != nil {
return xerrors.Errorf("find matching preset: %w", err)
}
}
// Try to claim a prebuilt workspace.
if templateVersionPresetID != uuid.Nil {
// Try and claim an eligible prebuild, if available.
// On successful claim, initialize all lifecycle fields from template and workspace-level config
// so the newly claimed workspace is properly managed by the lifecycle executor.
claimedWorkspace, err = claimPrebuild(
ctx, prebuildsClaimer, db, api.Logger, now, req, owner,
dbAutostartSchedule, nextStartAt, dbTTL)
ctx, prebuildsClaimer, db, api.Logger, now, req.Name, owner,
templateVersionPresetID, dbAutostartSchedule, nextStartAt, dbTTL)
// If claiming fails with an expected error (no claimable prebuilds or AGPL does not support prebuilds),
// we fall back to creating a new workspace. Otherwise, propagate the unexpected error.
if err != nil {
@@ -654,7 +675,7 @@ func createWorkspace(
fields := []any{
slog.Error(err),
slog.F("workspace_name", req.Name),
slog.F("template_version_preset_id", req.TemplateVersionPresetID),
slog.F("template_version_preset_id", templateVersionPresetID),
}
if !isExpectedError {
@@ -718,8 +739,8 @@ func createWorkspace(
if req.TemplateVersionID != uuid.Nil {
builder = builder.VersionID(req.TemplateVersionID)
}
if req.TemplateVersionPresetID != uuid.Nil {
builder = builder.TemplateVersionPresetID(req.TemplateVersionPresetID)
if templateVersionPresetID != uuid.Nil {
builder = builder.TemplateVersionPresetID(templateVersionPresetID)
}
if claimedWorkspace != nil {
builder = builder.MarkPrebuiltWorkspaceClaim()
@@ -884,13 +905,14 @@ func claimPrebuild(
db database.Store,
logger slog.Logger,
now time.Time,
req codersdk.CreateWorkspaceRequest,
name string,
owner workspaceOwner,
templateVersionPresetID uuid.UUID,
autostartSchedule sql.NullString,
nextStartAt sql.NullTime,
ttl sql.NullInt64,
) (*database.Workspace, error) {
claimedID, err := claimer.Claim(ctx, now, owner.ID, req.Name, req.TemplateVersionPresetID, autostartSchedule, nextStartAt, ttl)
claimedID, err := claimer.Claim(ctx, now, owner.ID, name, templateVersionPresetID, autostartSchedule, nextStartAt, ttl)
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)
+282
View File
@@ -4915,3 +4915,285 @@ func TestUpdateWorkspaceACL(t *testing.T) {
require.Equal(t, cerr.Validations[0].Field, "user_roles")
})
}
func TestWorkspaceCreateWithImplicitPreset(t *testing.T) {
t.Parallel()
// Helper function to create template with presets
createTemplateWithPresets := func(t *testing.T, client *codersdk.Client, user codersdk.CreateFirstUserResponse, presets []*proto.Preset) (codersdk.Template, codersdk.TemplateVersion) {
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{
Parse: echo.ParseComplete,
ProvisionPlan: []*proto.Response{
{
Type: &proto.Response_Plan{
Plan: &proto.PlanComplete{
Presets: presets,
},
},
},
},
})
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
return template, version
}
// Helper function to create workspace and verify preset usage
createWorkspaceAndVerifyPreset := func(t *testing.T, client *codersdk.Client, template codersdk.Template, expectedPresetID *uuid.UUID, params []codersdk.WorkspaceBuildParameter) codersdk.Workspace {
wsName := testutil.GetRandomNameHyphenated(t)
var ws codersdk.Workspace
if len(params) > 0 {
ws = coderdtest.CreateWorkspace(t, client, template.ID, func(cwr *codersdk.CreateWorkspaceRequest) {
cwr.Name = wsName
cwr.RichParameterValues = params
})
} else {
ws = coderdtest.CreateWorkspace(t, client, template.ID, func(cwr *codersdk.CreateWorkspaceRequest) {
cwr.Name = wsName
})
}
require.Equal(t, wsName, ws.Name)
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, ws.LatestBuild.ID)
// Verify the preset was used if expected
if expectedPresetID != nil {
require.NotNil(t, ws.LatestBuild.TemplateVersionPresetID)
require.Equal(t, *expectedPresetID, *ws.LatestBuild.TemplateVersionPresetID)
} else {
require.Nil(t, ws.LatestBuild.TemplateVersionPresetID)
}
return ws
}
t.Run("NoPresets", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
user := coderdtest.CreateFirstUser(t, client)
// Create template with no presets
template, _ := createTemplateWithPresets(t, client, user, []*proto.Preset{})
// Test workspace creation with no parameters
createWorkspaceAndVerifyPreset(t, client, template, nil, nil)
// Test workspace creation with parameters (should still work, no preset matching)
createWorkspaceAndVerifyPreset(t, client, template, nil, []codersdk.WorkspaceBuildParameter{
{Name: "param1", Value: "value1"},
})
})
t.Run("SinglePresetNoParameters", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
user := coderdtest.CreateFirstUser(t, client)
// Create template with single preset that has no parameters
preset := &proto.Preset{
Name: "empty-preset",
Description: "A preset with no parameters",
Parameters: []*proto.PresetParameter{},
}
template, version := createTemplateWithPresets(t, client, user, []*proto.Preset{preset})
// Get the preset ID from the database
ctx := context.Background()
presets, err := client.TemplateVersionPresets(ctx, version.ID)
require.NoError(t, err)
require.Len(t, presets, 1)
presetID := presets[0].ID
// Test workspace creation with no parameters - should match the preset
createWorkspaceAndVerifyPreset(t, client, template, &presetID, nil)
// Test workspace creation with parameters - should not match the preset
createWorkspaceAndVerifyPreset(t, client, template, &presetID, []codersdk.WorkspaceBuildParameter{
{Name: "param1", Value: "value1"},
})
})
t.Run("SinglePresetWithParameters", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
user := coderdtest.CreateFirstUser(t, client)
// Create template with single preset that has parameters
preset := &proto.Preset{
Name: "param-preset",
Description: "A preset with parameters",
Parameters: []*proto.PresetParameter{
{Name: "param1", Value: "value1"},
{Name: "param2", Value: "value2"},
},
}
template, version := createTemplateWithPresets(t, client, user, []*proto.Preset{preset})
// Get the preset ID from the database
ctx := context.Background()
presets, err := client.TemplateVersionPresets(ctx, version.ID)
require.NoError(t, err)
require.Len(t, presets, 1)
presetID := presets[0].ID
// Test workspace creation with no parameters - should not match the preset
createWorkspaceAndVerifyPreset(t, client, template, nil, nil)
// Test workspace creation with exact matching parameters - should match the preset
createWorkspaceAndVerifyPreset(t, client, template, &presetID, []codersdk.WorkspaceBuildParameter{
{Name: "param1", Value: "value1"},
{Name: "param2", Value: "value2"},
})
// Test workspace creation with partial matching parameters - should not match the preset
createWorkspaceAndVerifyPreset(t, client, template, nil, []codersdk.WorkspaceBuildParameter{
{Name: "param1", Value: "value1"},
})
// Test workspace creation with different parameter values - should not match the preset
createWorkspaceAndVerifyPreset(t, client, template, nil, []codersdk.WorkspaceBuildParameter{
{Name: "param1", Value: "value1"},
{Name: "param2", Value: "different"},
})
// Test workspace creation with extra parameters - should match the preset
createWorkspaceAndVerifyPreset(t, client, template, &presetID, []codersdk.WorkspaceBuildParameter{
{Name: "param1", Value: "value1"},
{Name: "param2", Value: "value2"},
{Name: "param3", Value: "value3"},
})
})
t.Run("MultiplePresets", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
user := coderdtest.CreateFirstUser(t, client)
// Create template with multiple presets
preset1 := &proto.Preset{
Name: "empty-preset",
Description: "A preset with no parameters",
Parameters: []*proto.PresetParameter{},
}
preset2 := &proto.Preset{
Name: "single-param-preset",
Description: "A preset with one parameter",
Parameters: []*proto.PresetParameter{
{Name: "param1", Value: "value1"},
},
}
preset3 := &proto.Preset{
Name: "multi-param-preset",
Description: "A preset with multiple parameters",
Parameters: []*proto.PresetParameter{
{Name: "param1", Value: "value1"},
{Name: "param2", Value: "value2"},
},
}
template, version := createTemplateWithPresets(t, client, user, []*proto.Preset{preset1, preset2, preset3})
// Get the preset IDs from the database
ctx := context.Background()
presets, err := client.TemplateVersionPresets(ctx, version.ID)
require.NoError(t, err)
require.Len(t, presets, 3)
// Sort presets by name to get consistent ordering
var emptyPresetID, singleParamPresetID, multiParamPresetID uuid.UUID
for _, p := range presets {
switch p.Name {
case "empty-preset":
emptyPresetID = p.ID
case "single-param-preset":
singleParamPresetID = p.ID
case "multi-param-preset":
multiParamPresetID = p.ID
}
}
// Test workspace creation with no parameters - should match empty preset
createWorkspaceAndVerifyPreset(t, client, template, &emptyPresetID, nil)
// Test workspace creation with single parameter - should match single param preset
createWorkspaceAndVerifyPreset(t, client, template, &singleParamPresetID, []codersdk.WorkspaceBuildParameter{
{Name: "param1", Value: "value1"},
})
// Test workspace creation with multiple parameters - should match multi param preset
createWorkspaceAndVerifyPreset(t, client, template, &multiParamPresetID, []codersdk.WorkspaceBuildParameter{
{Name: "param1", Value: "value1"},
{Name: "param2", Value: "value2"},
})
// Test workspace creation with non-matching parameters - should not match any preset
createWorkspaceAndVerifyPreset(t, client, template, &emptyPresetID, []codersdk.WorkspaceBuildParameter{
{Name: "param1", Value: "different"},
})
})
t.Run("PresetSpecifiedExplicitly", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
user := coderdtest.CreateFirstUser(t, client)
// Create template with multiple presets
preset1 := &proto.Preset{
Name: "preset1",
Description: "First preset",
Parameters: []*proto.PresetParameter{
{Name: "param1", Value: "value1"},
},
}
preset2 := &proto.Preset{
Name: "preset2",
Description: "Second preset",
Parameters: []*proto.PresetParameter{
{Name: "param1", Value: "value2"},
},
}
template, version := createTemplateWithPresets(t, client, user, []*proto.Preset{preset1, preset2})
// Get the preset IDs from the database
ctx := context.Background()
presets, err := client.TemplateVersionPresets(ctx, version.ID)
require.NoError(t, err)
require.Len(t, presets, 2)
var preset1ID, preset2ID uuid.UUID
for _, p := range presets {
switch p.Name {
case "preset1":
preset1ID = p.ID
case "preset2":
preset2ID = p.ID
}
}
// Test workspace creation with preset1 specified explicitly - should use preset1 regardless of parameters
ws := coderdtest.CreateWorkspace(t, client, template.ID, func(cwr *codersdk.CreateWorkspaceRequest) {
cwr.TemplateVersionPresetID = preset1ID
cwr.RichParameterValues = []codersdk.WorkspaceBuildParameter{
{Name: "param1", Value: "value2"}, // This would normally match preset2
}
})
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, ws.LatestBuild.ID)
require.NotNil(t, ws.LatestBuild.TemplateVersionPresetID)
require.Equal(t, preset1ID, *ws.LatestBuild.TemplateVersionPresetID)
// Test workspace creation with preset2 specified explicitly - should use preset2 regardless of parameters
ws2 := coderdtest.CreateWorkspace(t, client, template.ID, func(cwr *codersdk.CreateWorkspaceRequest) {
cwr.TemplateVersionPresetID = preset2ID
cwr.RichParameterValues = []codersdk.WorkspaceBuildParameter{
{Name: "param1", Value: "value1"}, // This would normally match preset1
}
})
coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, ws2.LatestBuild.ID)
require.NotNil(t, ws2.LatestBuild.TemplateVersionPresetID)
require.Equal(t, preset2ID, *ws2.LatestBuild.TemplateVersionPresetID)
})
}
+15 -6
View File
@@ -15,6 +15,7 @@ import (
"github.com/coder/coder/v2/coderd/dynamicparameters"
"github.com/coder/coder/v2/coderd/files"
"github.com/coder/coder/v2/coderd/prebuilds"
"github.com/coder/coder/v2/coderd/rbac/policy"
"github.com/coder/coder/v2/coderd/util/ptr"
"github.com/coder/coder/v2/provisioner/terraform/tfparse"
@@ -442,6 +443,20 @@ func (b *Builder) buildTx(authFunc func(action policy.Action, object rbac.Object
var workspaceBuild database.WorkspaceBuild
err = b.store.InTx(func(store database.Store) error {
names, values, err := b.getParameters()
if err != nil {
// getParameters already wraps errors in BuildError
return err
}
if b.templateVersionPresetID == uuid.Nil {
presetID, err := prebuilds.FindMatchingPresetID(b.ctx, b.store, templateVersionID, names, values)
if err != nil {
return BuildError{http.StatusInternalServerError, "find matching preset", err}
}
b.templateVersionPresetID = presetID
}
err = store.InsertWorkspaceBuild(b.ctx, database.InsertWorkspaceBuildParams{
ID: workspaceBuildID,
CreatedAt: now,
@@ -473,12 +488,6 @@ func (b *Builder) buildTx(authFunc func(action policy.Action, object rbac.Object
return BuildError{code, "insert workspace build", err}
}
names, values, err := b.getParameters()
if err != nil {
// getParameters already wraps errors in BuildError
return err
}
err = store.InsertWorkspaceBuildParameters(b.ctx, database.InsertWorkspaceBuildParametersParams{
WorkspaceBuildID: workspaceBuildID,
Name: names,
+22
View File
@@ -82,6 +82,7 @@ func TestBuilder_NoOptions(t *testing.T) {
}),
withInTx,
expectFindMatchingPresetID(uuid.Nil, sql.ErrNoRows),
expectBuild(func(bld database.InsertWorkspaceBuildParams) {
asrt.Equal(inactiveVersionID, bld.TemplateVersionID)
asrt.Equal(workspaceID, bld.WorkspaceID)
@@ -132,6 +133,7 @@ func TestBuilder_Initiator(t *testing.T) {
asrt.Equal(otherUserID, job.InitiatorID)
}),
withInTx,
expectFindMatchingPresetID(uuid.Nil, sql.ErrNoRows),
expectBuild(func(bld database.InsertWorkspaceBuildParams) {
asrt.Equal(otherUserID, bld.InitiatorID)
}),
@@ -180,6 +182,7 @@ func TestBuilder_Baggage(t *testing.T) {
asrt.Contains(string(job.TraceMetadata.RawMessage), "ip=127.0.0.1")
}),
withInTx,
expectFindMatchingPresetID(uuid.Nil, sql.ErrNoRows),
expectBuild(func(bld database.InsertWorkspaceBuildParams) {
}),
expectBuildParameters(func(params database.InsertWorkspaceBuildParametersParams) {
@@ -219,6 +222,7 @@ func TestBuilder_Reason(t *testing.T) {
expectProvisionerJob(func(_ database.InsertProvisionerJobParams) {
}),
withInTx,
expectFindMatchingPresetID(uuid.Nil, sql.ErrNoRows),
expectBuild(func(bld database.InsertWorkspaceBuildParams) {
asrt.Equal(database.BuildReasonAutostart, bld.Reason)
}),
@@ -261,6 +265,7 @@ func TestBuilder_ActiveVersion(t *testing.T) {
}),
withInTx,
expectFindMatchingPresetID(uuid.Nil, sql.ErrNoRows),
expectBuild(func(bld database.InsertWorkspaceBuildParams) {
asrt.Equal(activeVersionID, bld.TemplateVersionID)
// no previous build...
@@ -386,6 +391,7 @@ func TestWorkspaceBuildWithTags(t *testing.T) {
expectBuildParameters(func(_ database.InsertWorkspaceBuildParametersParams) {
}),
withBuild,
expectFindMatchingPresetID(uuid.Nil, sql.ErrNoRows),
)
fc := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{})
@@ -470,6 +476,7 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
}
}),
withBuild,
expectFindMatchingPresetID(uuid.Nil, sql.ErrNoRows),
)
fc := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{})
@@ -519,6 +526,7 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
}
}),
withBuild,
expectFindMatchingPresetID(uuid.Nil, sql.ErrNoRows),
)
fc := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{})
@@ -661,6 +669,7 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
}
}),
withBuild,
expectFindMatchingPresetID(uuid.Nil, sql.ErrNoRows),
)
fc := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{})
@@ -713,6 +722,7 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
withProvisionerDaemons([]database.GetEligibleProvisionerDaemonsByProvisionerJobIDsRow{}),
// Outputs
expectFindMatchingPresetID(uuid.Nil, sql.ErrNoRows),
expectProvisionerJob(func(job database.InsertProvisionerJobParams) {}),
withInTx,
expectBuild(func(bld database.InsertWorkspaceBuildParams) {}),
@@ -775,6 +785,7 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) {
withProvisionerDaemons([]database.GetEligibleProvisionerDaemonsByProvisionerJobIDsRow{}),
// Outputs
expectFindMatchingPresetID(uuid.Nil, sql.ErrNoRows),
expectProvisionerJob(func(job database.InsertProvisionerJobParams) {}),
withInTx,
expectBuild(func(bld database.InsertWorkspaceBuildParams) {}),
@@ -906,6 +917,7 @@ func TestWorkspaceBuildDeleteOrphan(t *testing.T) {
}),
withInTx,
expectFindMatchingPresetID(uuid.Nil, sql.ErrNoRows),
expectBuild(func(bld database.InsertWorkspaceBuildParams) {
asrt.Equal(inactiveVersionID, bld.TemplateVersionID)
asrt.Equal(workspaceID, bld.WorkspaceID)
@@ -968,6 +980,7 @@ func TestWorkspaceBuildDeleteOrphan(t *testing.T) {
}),
withInTx,
expectFindMatchingPresetID(uuid.Nil, sql.ErrNoRows),
expectBuild(func(bld database.InsertWorkspaceBuildParams) {
asrt.Equal(inactiveVersionID, bld.TemplateVersionID)
asrt.Equal(workspaceID, bld.WorkspaceID)
@@ -1041,6 +1054,7 @@ func TestWorkspaceBuildUsageChecker(t *testing.T) {
// Outputs
expectProvisionerJob(func(job database.InsertProvisionerJobParams) {}),
withInTx,
expectFindMatchingPresetID(uuid.Nil, sql.ErrNoRows),
expectBuild(func(bld database.InsertWorkspaceBuildParams) {}),
withBuild,
expectBuildParameters(func(params database.InsertWorkspaceBuildParametersParams) {}),
@@ -1485,6 +1499,14 @@ func withProvisionerDaemons(provisionerDaemons []database.GetEligibleProvisioner
}
}
func expectFindMatchingPresetID(id uuid.UUID, err error) func(mTx *dbmock.MockStore) {
return func(mTx *dbmock.MockStore) {
mTx.EXPECT().FindMatchingPresetID(gomock.Any(), gomock.Any()).
Times(1).
Return(id, err)
}
}
type fakeUsageChecker struct {
checkBuildUsageFunc func(ctx context.Context, store database.Store, templateVersion *database.TemplateVersion) (wsbuilder.UsageCheckResponse, error)
}
@@ -29,6 +29,7 @@ Prebuilt workspaces are tightly integrated with [workspace presets](./parameters
1. The preset must define all required parameters needed to build the workspace.
1. The preset parameters define the base configuration and are immutable once a prebuilt workspace is provisioned.
1. Parameters that are not defined in the preset can still be customized by users when they claim a workspace.
1. If a user does not select a preset but provides parameters that match one or more presets, Coder will automatically select the most specific matching preset and assign a prebuilt workspace if one is available.
## Prerequisites
@@ -291,16 +292,6 @@ does not reconnect after a template update. This shortcoming is described in [th
and will be addressed before the next release (v2.23). In the interim, a simple workaround is to restart the workspace
when it is in this problematic state.
### Current limitations
The prebuilt workspaces feature has these current limitations:
- **Organizations**
Prebuilt workspaces can only be used with the default organization.
[View issue](https://github.com/coder/internal/issues/364)
### Monitoring and observability
#### Available metrics