From b0405ada49e4b18bb107f011c2a0bc48abb3a1d2 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Thu, 7 Aug 2025 10:31:21 +0000 Subject: [PATCH 1/2] Initial, inefficient attempt at preset parameter matching --- coderd/wsbuilder/wsbuilder.go | 57 +++++++++++++++++++++++++++++++---- 1 file changed, 51 insertions(+), 6 deletions(-) diff --git a/coderd/wsbuilder/wsbuilder.go b/coderd/wsbuilder/wsbuilder.go index 73e449ee5bb93..00bced630df38 100644 --- a/coderd/wsbuilder/wsbuilder.go +++ b/coderd/wsbuilder/wsbuilder.go @@ -442,6 +442,57 @@ 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 { + parameterMap := make(map[string]string) + for i, name := range names { + parameterMap[name] = values[i] + } + + presetParameters, err := b.store.GetPresetParametersByTemplateVersionID(b.ctx, templateVersionID) + if err != nil { + return BuildError{http.StatusInternalServerError, "get preset parameters", err} + } + + presetMap := make(map[uuid.UUID]map[string]string) + for _, presetParameter := range presetParameters { + if _, ok := presetMap[presetParameter.TemplateVersionPresetID]; !ok { + presetMap[presetParameter.TemplateVersionPresetID] = make(map[string]string) + } + presetMap[presetParameter.TemplateVersionPresetID][presetParameter.Name] = presetParameter.Value + } + + // Compare each preset's parameters to the provided parameters to find any matches + for presetID, presetParams := range presetMap { + isMatch := true + // Check that all preset parameters match the provided parameters + for paramName, presetValue := range presetParams { + if providedValue, exists := parameterMap[paramName]; !exists || providedValue != presetValue { + isMatch = false + break + } + } + // Check that all provided parameters match the preset parameters + if isMatch { + for paramName, providedValue := range parameterMap { + if presetValue, exists := presetParams[paramName]; !exists || providedValue != presetValue { + isMatch = false + break + } + } + } + if isMatch { + b.templateVersionPresetID = presetID + break + } + } + } + err = store.InsertWorkspaceBuild(b.ctx, database.InsertWorkspaceBuildParams{ ID: workspaceBuildID, CreatedAt: now, @@ -473,12 +524,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, From 8e2e62ef8dea481e532fc6e09dc185c9d89d1527 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Mon, 11 Aug 2025 08:35:48 +0000 Subject: [PATCH 2/2] WIP: claim prebuilds based on parameters --- coderd/database/dbauthz/dbauthz.go | 8 + coderd/database/dbmetrics/querymetrics.go | 14 + coderd/database/dbmock/dbmock.go | 30 ++ coderd/database/dump.sql | 39 ++- ...356_add_parameter_hash_to_presets.down.sql | 9 + ...00356_add_parameter_hash_to_presets.up.sql | 60 ++++ coderd/database/models.go | 2 + coderd/database/querier.go | 2 + coderd/database/querier_test.go | 283 ++++++++++++++++++ coderd/database/queries.sql.go | 69 ++++- coderd/database/queries/presets.sql | 12 + coderd/database/queries/workspacebuilds.sql | 18 ++ .../provisionerdserver/provisionerdserver.go | 6 + coderd/wsbuilder/wsbuilder.go | 62 +--- 14 files changed, 558 insertions(+), 56 deletions(-) create mode 100644 coderd/database/migrations/000356_add_parameter_hash_to_presets.down.sql create mode 100644 coderd/database/migrations/000356_add_parameter_hash_to_presets.up.sql diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 99dd9833fa5d6..ecc8d05bc75c4 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -4255,6 +4255,10 @@ func (q *querier) RevokeDBCryptKey(ctx context.Context, activeKeyDigest string) return q.db.RevokeDBCryptKey(ctx, activeKeyDigest) } +func (q *querier) SetWorkspaceBuildPresetByParameterHash(ctx context.Context, workspaceBuildID uuid.UUID) (database.SetWorkspaceBuildPresetByParameterHashRow, error) { + panic("not implemented") +} + func (q *querier) TryAcquireLock(ctx context.Context, id int64) (bool, error) { return q.db.TryAcquireLock(ctx, id) } @@ -4452,6 +4456,10 @@ func (q *querier) UpdateOrganizationDeletedByID(ctx context.Context, arg databas return deleteQ(q.log, q.auth, q.db.GetOrganizationByID, deleteF)(ctx, arg.ID) } +func (q *querier) UpdatePresetParameterHash(ctx context.Context, id uuid.UUID) (database.UpdatePresetParameterHashRow, error) { + panic("not implemented") +} + func (q *querier) UpdatePresetPrebuildStatus(ctx context.Context, arg database.UpdatePresetPrebuildStatusParams) error { preset, err := q.db.GetPresetByID(ctx, arg.PresetID) if err != nil { diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index d0cd0d1ab797d..513781307dbff 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -2616,6 +2616,13 @@ func (m queryMetricsStore) RevokeDBCryptKey(ctx context.Context, activeKeyDigest return r0 } +func (m queryMetricsStore) SetWorkspaceBuildPresetByParameterHash(ctx context.Context, workspaceBuildID uuid.UUID) (database.SetWorkspaceBuildPresetByParameterHashRow, error) { + start := time.Now() + r0 := m.s.SetWorkspaceBuildPresetByParameterHash(ctx, workspaceBuildID) + m.queryLatencies.WithLabelValues("SetWorkspaceBuildPresetByParameterHash").Observe(time.Since(start).Seconds()) + return r0 +} + func (m queryMetricsStore) TryAcquireLock(ctx context.Context, pgTryAdvisoryXactLock int64) (bool, error) { start := time.Now() ok, err := m.s.TryAcquireLock(ctx, pgTryAdvisoryXactLock) @@ -2756,6 +2763,13 @@ func (m queryMetricsStore) UpdateOrganizationDeletedByID(ctx context.Context, ar return r0 } +func (m queryMetricsStore) UpdatePresetParameterHash(ctx context.Context, id uuid.UUID) (database.UpdatePresetParameterHashRow, error) { + start := time.Now() + r0 := m.s.UpdatePresetParameterHash(ctx, id) + m.queryLatencies.WithLabelValues("UpdatePresetParameterHash").Observe(time.Since(start).Seconds()) + return r0 +} + func (m queryMetricsStore) UpdatePresetPrebuildStatus(ctx context.Context, arg database.UpdatePresetPrebuildStatusParams) error { start := time.Now() r0 := m.s.UpdatePresetPrebuildStatus(ctx, arg) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index e88763ba1eb74..c5d598d0fce64 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -5608,6 +5608,21 @@ func (mr *MockStoreMockRecorder) RevokeDBCryptKey(ctx, activeKeyDigest any) *gom return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RevokeDBCryptKey", reflect.TypeOf((*MockStore)(nil).RevokeDBCryptKey), ctx, activeKeyDigest) } +// SetWorkspaceBuildPresetByParameterHash mocks base method. +func (m *MockStore) SetWorkspaceBuildPresetByParameterHash(ctx context.Context, workspaceBuildID uuid.UUID) (database.SetWorkspaceBuildPresetByParameterHashRow, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SetWorkspaceBuildPresetByParameterHash", ctx, workspaceBuildID) + ret0, _ := ret[0].(database.SetWorkspaceBuildPresetByParameterHashRow) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// SetWorkspaceBuildPresetByParameterHash indicates an expected call of SetWorkspaceBuildPresetByParameterHash. +func (mr *MockStoreMockRecorder) SetWorkspaceBuildPresetByParameterHash(ctx, workspaceBuildID any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetWorkspaceBuildPresetByParameterHash", reflect.TypeOf((*MockStore)(nil).SetWorkspaceBuildPresetByParameterHash), ctx, workspaceBuildID) +} + // TryAcquireLock mocks base method. func (m *MockStore) TryAcquireLock(ctx context.Context, pgTryAdvisoryXactLock int64) (bool, error) { m.ctrl.T.Helper() @@ -5901,6 +5916,21 @@ func (mr *MockStoreMockRecorder) UpdateOrganizationDeletedByID(ctx, arg any) *go return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateOrganizationDeletedByID", reflect.TypeOf((*MockStore)(nil).UpdateOrganizationDeletedByID), ctx, arg) } +// UpdatePresetParameterHash mocks base method. +func (m *MockStore) UpdatePresetParameterHash(ctx context.Context, id uuid.UUID) (database.UpdatePresetParameterHashRow, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdatePresetParameterHash", ctx, id) + ret0, _ := ret[0].(database.UpdatePresetParameterHashRow) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// UpdatePresetParameterHash indicates an expected call of UpdatePresetParameterHash. +func (mr *MockStoreMockRecorder) UpdatePresetParameterHash(ctx, id any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdatePresetParameterHash", reflect.TypeOf((*MockStore)(nil).UpdatePresetParameterHash), ctx, id) +} + // UpdatePresetPrebuildStatus mocks base method. func (m *MockStore) UpdatePresetPrebuildStatus(ctx context.Context, arg database.UpdatePresetPrebuildStatusParams) error { m.ctrl.T.Helper() diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 053b5302d3e38..769324121d9ba 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -468,6 +468,38 @@ BEGIN END; $$; +CREATE FUNCTION generate_parameter_hash(names text[], param_values text[]) RETURNS text + LANGUAGE plpgsql + AS $$ +DECLARE + param_pairs TEXT[] := '{}'; + sorted_pairs TEXT[] := '{}'; + hash_value TEXT; + i INTEGER; +BEGIN + -- Validate input arrays have same length + IF array_length(names, 1) != array_length(param_values, 1) THEN + RAISE EXCEPTION 'Names and values arrays must have the same length'; + END IF; + + -- Build parameter pairs from arrays + FOR i IN 1..array_length(names, 1) LOOP + param_pairs := array_append(param_pairs, names[i] || '=' || param_values[i]); + END LOOP; + + -- Sort the pairs for consistent hashing + SELECT array_agg(pair ORDER BY pair) INTO sorted_pairs + FROM unnest(param_pairs) AS pair; + + -- Generate hash from sorted pairs + SELECT encode(sha256(array_to_string(sorted_pairs, '|')::bytea), 'hex') INTO hash_value; + + RETURN hash_value; +END; +$$; + +COMMENT ON FUNCTION generate_parameter_hash(names text[], param_values text[]) IS 'Generates SHA256 hash of sorted parameter name-value pairs from arrays. First parameter is names array, second is values array.'; + CREATE FUNCTION inhibit_enqueue_if_disabled() RETURNS trigger LANGUAGE plpgsql AS $$ @@ -1632,13 +1664,16 @@ CREATE TABLE template_version_presets ( scheduling_timezone text DEFAULT ''::text NOT NULL, is_default boolean DEFAULT false NOT NULL, description character varying(128) DEFAULT ''::character varying NOT NULL, - icon character varying(256) DEFAULT ''::character varying NOT NULL + icon character varying(256) DEFAULT ''::character varying NOT NULL, + parameter_hash text ); COMMENT ON COLUMN template_version_presets.description IS 'Short text describing the preset (max 128 characters).'; COMMENT ON COLUMN template_version_presets.icon IS 'URL or path to an icon representing the preset (max 256 characters).'; +COMMENT ON COLUMN template_version_presets.parameter_hash IS 'SHA256 hash of sorted parameter name-value pairs for efficient preset matching'; + CREATE TABLE template_version_terraform_values ( template_version_id uuid NOT NULL, updated_at timestamp with time zone DEFAULT now() NOT NULL, @@ -2826,6 +2861,8 @@ CREATE INDEX idx_tailnet_tunnels_src_id ON tailnet_tunnels USING hash (src_id); CREATE UNIQUE INDEX idx_template_version_presets_default ON template_version_presets USING btree (template_version_id) WHERE (is_default = true); +CREATE INDEX idx_template_version_presets_parameter_hash ON template_version_presets USING btree (template_version_id, parameter_hash); + CREATE INDEX idx_template_versions_has_ai_task ON template_versions USING btree (has_ai_task); CREATE UNIQUE INDEX idx_unique_preset_name ON template_version_presets USING btree (name, template_version_id); diff --git a/coderd/database/migrations/000356_add_parameter_hash_to_presets.down.sql b/coderd/database/migrations/000356_add_parameter_hash_to_presets.down.sql new file mode 100644 index 0000000000000..41c175e8f513c --- /dev/null +++ b/coderd/database/migrations/000356_add_parameter_hash_to_presets.down.sql @@ -0,0 +1,9 @@ +-- Drop the index first +DROP INDEX IF EXISTS idx_template_version_presets_parameter_hash; + +-- Remove the parameter_hash column +ALTER TABLE template_version_presets +DROP COLUMN IF EXISTS parameter_hash; + +-- Drop the function +DROP FUNCTION IF EXISTS generate_parameter_hash(TEXT[], TEXT[]); diff --git a/coderd/database/migrations/000356_add_parameter_hash_to_presets.up.sql b/coderd/database/migrations/000356_add_parameter_hash_to_presets.up.sql new file mode 100644 index 0000000000000..e3f2f898b01b2 --- /dev/null +++ b/coderd/database/migrations/000356_add_parameter_hash_to_presets.up.sql @@ -0,0 +1,60 @@ +-- Add parameter_hash column to template_version_presets table +ALTER TABLE template_version_presets +ADD COLUMN parameter_hash TEXT; + +-- Create a generic function to generate parameter hash from arrays +CREATE OR REPLACE FUNCTION generate_parameter_hash(names TEXT[], param_values TEXT[]) +RETURNS TEXT AS $$ +DECLARE + param_pairs TEXT[] := '{}'; + sorted_pairs TEXT[] := '{}'; + hash_value TEXT; + i INTEGER; +BEGIN + -- Validate input arrays have same length + IF array_length(names, 1) != array_length(param_values, 1) THEN + RAISE EXCEPTION 'Names and values arrays must have the same length'; + END IF; + + -- Build parameter pairs from arrays + FOR i IN 1..array_length(names, 1) LOOP + param_pairs := array_append(param_pairs, names[i] || '=' || param_values[i]); + END LOOP; + + -- Sort the pairs for consistent hashing + SELECT array_agg(pair ORDER BY pair) INTO sorted_pairs + FROM unnest(param_pairs) AS pair; + + -- Generate hash from sorted pairs + SELECT encode(sha256(array_to_string(sorted_pairs, '|')::bytea), 'hex') INTO hash_value; + + RETURN hash_value; +END; +$$ LANGUAGE plpgsql; + +-- Populate parameter_hash for existing presets +UPDATE template_version_presets +SET parameter_hash = generate_parameter_hash( + (SELECT array_agg(name ORDER BY name) FROM template_version_preset_parameters WHERE template_version_preset_id = template_version_presets.id), + (SELECT array_agg(value ORDER BY name) FROM template_version_preset_parameters WHERE template_version_preset_id = template_version_presets.id) +) +WHERE id IN ( + SELECT DISTINCT template_version_preset_id + FROM template_version_preset_parameters +); + +-- Make parameter_hash NOT NULL after populating +-- ALTER TABLE template_version_presets +-- ALTER COLUMN parameter_hash SET NOT NULL; + +-- Create index for efficient parameter hash lookups +CREATE INDEX idx_template_version_presets_parameter_hash +ON template_version_presets (template_version_id, parameter_hash); + +-- Add comment explaining the column +COMMENT ON COLUMN template_version_presets.parameter_hash IS 'SHA256 hash of sorted parameter name-value pairs for efficient preset matching'; + +-- Keep the function for use by the application +COMMENT ON FUNCTION generate_parameter_hash(TEXT[], TEXT[]) IS 'Generates SHA256 hash of sorted parameter name-value pairs from arrays. First parameter is names array, second is values array.'; + + diff --git a/coderd/database/models.go b/coderd/database/models.go index 8b13c8a8af057..7019525e96462 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -3689,6 +3689,8 @@ type TemplateVersionPreset struct { Description string `db:"description" json:"description"` // URL or path to an icon representing the preset (max 256 characters). Icon string `db:"icon" json:"icon"` + // SHA256 hash of sorted parameter name-value pairs for efficient preset matching + ParameterHash sql.NullString `db:"parameter_hash" json:"parameter_hash"` } type TemplateVersionPresetParameter struct { diff --git a/coderd/database/querier.go b/coderd/database/querier.go index a50bb0bb2192a..498da2babd0ef 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -563,6 +563,7 @@ type sqlcQuerier interface { RemoveUserFromAllGroups(ctx context.Context, userID uuid.UUID) error RemoveUserFromGroups(ctx context.Context, arg RemoveUserFromGroupsParams) ([]uuid.UUID, error) RevokeDBCryptKey(ctx context.Context, activeKeyDigest string) error + SetWorkspaceBuildPresetByParameterHash(ctx context.Context, workspaceBuildID uuid.UUID) (SetWorkspaceBuildPresetByParameterHashRow, error) // Non blocking lock. Returns true if the lock was acquired, false otherwise. // // This must be called from within a transaction. The lock will be automatically @@ -588,6 +589,7 @@ type sqlcQuerier interface { UpdateOAuth2ProviderAppSecretByID(ctx context.Context, arg UpdateOAuth2ProviderAppSecretByIDParams) (OAuth2ProviderAppSecret, error) UpdateOrganization(ctx context.Context, arg UpdateOrganizationParams) (Organization, error) UpdateOrganizationDeletedByID(ctx context.Context, arg UpdateOrganizationDeletedByIDParams) error + UpdatePresetParameterHash(ctx context.Context, id uuid.UUID) (UpdatePresetParameterHashRow, error) UpdatePresetPrebuildStatus(ctx context.Context, arg UpdatePresetPrebuildStatusParams) error UpdateProvisionerDaemonLastSeenAt(ctx context.Context, arg UpdateProvisionerDaemonLastSeenAtParams) error UpdateProvisionerJobByID(ctx context.Context, arg UpdateProvisionerJobByIDParams) error diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 9c88b9b3db679..ccc7f9c3fd895 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -6003,3 +6003,286 @@ func TestGetRunningPrebuiltWorkspaces(t *testing.T) { require.Len(t, runningPrebuilds, 1, "expected only one running prebuilt workspace") require.Equal(t, runningPrebuild.ID, runningPrebuilds[0].ID, "expected the running prebuilt workspace to be returned") } + +func TestSetWorkspaceBuildPresetByParameterHash(t *testing.T) { + t.Parallel() + + if !dbtestutil.WillUsePostgres() { + t.SkipNow() + } + + now := dbtime.Now() + orgID := uuid.New() + userID := uuid.New() + + t.Run("MatchingPreset", func(t *testing.T) { + t.Parallel() + + db, _ := dbtestutil.NewDB(t) + ctx := testutil.Context(t, testutil.WaitShort) + + // Create organization and user + dbgen.Organization(t, db, database.Organization{ + ID: orgID, + }) + dbgen.User(t, db, database.User{ + ID: userID, + }) + + // Create template and template version + tmpl := createTemplate(t, db, orgID, userID) + tmplV1 := createTmplVersionAndPreset(t, db, tmpl, tmpl.ActiveVersionID, now, nil) + + // Create workspace and workspace build + workspace := dbgen.Workspace(t, db, database.WorkspaceTable{ + TemplateID: tmpl.ID, + OwnerID: userID, + }) + + // Create a provisioner job for the workspace build + job := dbgen.ProvisionerJob(t, db, nil, database.ProvisionerJob{ + OrganizationID: orgID, + Type: database.ProvisionerJobTypeWorkspaceBuild, + }) + + workspaceBuild := dbgen.WorkspaceBuild(t, db, database.WorkspaceBuild{ + WorkspaceID: workspace.ID, + TemplateVersionID: tmpl.ActiveVersionID, + JobID: job.ID, + BuildNumber: 1, + Transition: database.WorkspaceTransitionStart, + }) + + // Insert workspace build parameters that match the preset + err := db.InsertWorkspaceBuildParameters(ctx, database.InsertWorkspaceBuildParametersParams{ + WorkspaceBuildID: workspaceBuild.ID, + Name: []string{"param1", "param2"}, + Value: []string{"value1", "value2"}, + }) + require.NoError(t, err) + + // Insert preset parameters with the same values + _, err = db.InsertPresetParameters(ctx, database.InsertPresetParametersParams{ + TemplateVersionPresetID: tmplV1.preset.ID, + Names: []string{"param1", "param2"}, + Values: []string{"value1", "value2"}, + }) + require.NoError(t, err) + + // Create antagonist preset 1: Different template version with same parameters + differentTemplateVersion := dbgen.TemplateVersion(t, db, database.TemplateVersion{ + TemplateID: uuid.NullUUID{UUID: tmpl.ID, Valid: true}, + OrganizationID: orgID, + CreatedBy: userID, + }) + antagonistPreset1 := dbgen.Preset(t, db, database.InsertPresetParams{ + TemplateVersionID: differentTemplateVersion.ID, + Name: "antagonist_different_template", + }) + _, err = db.InsertPresetParameters(ctx, database.InsertPresetParametersParams{ + TemplateVersionPresetID: antagonistPreset1.ID, + Names: []string{"param1", "param2"}, + Values: []string{"value1", "value2"}, + }) + require.NoError(t, err) + + // Create antagonist preset 2: Same template version but different parameters + antagonistPreset2 := dbgen.Preset(t, db, database.InsertPresetParams{ + TemplateVersionID: tmpl.ActiveVersionID, + Name: "antagonist_different_params", + }) + _, err = db.InsertPresetParameters(ctx, database.InsertPresetParametersParams{ + TemplateVersionPresetID: antagonistPreset2.ID, + Names: []string{"param1", "param2"}, + Values: []string{"different_value1", "different_value2"}, + }) + require.NoError(t, err) + + // Verify that the parameters are actually equal before testing the query + buildParams, err := db.GetWorkspaceBuildParameters(ctx, workspaceBuild.ID) + require.NoError(t, err) + presetParams, err := db.GetPresetParametersByPresetID(ctx, tmplV1.preset.ID) + require.NoError(t, err) + + // Convert to maps for easy comparison + buildParamMap := make(map[string]string) + for _, param := range buildParams { + buildParamMap[param.Name] = param.Value + } + presetParamMap := make(map[string]string) + for _, param := range presetParams { + presetParamMap[param.Name] = param.Value + } + + // Test case precondition: Build parameters should match preset parameters + require.Equal(t, presetParamMap, buildParamMap, "Build parameters should match preset parameters") + + presetHash, err := db.UpdatePresetParameterHash(ctx, tmplV1.preset.ID) + require.NoError(t, err) + + // Detect and set the preset that matches the build parameters + buildHash, err := db.SetWorkspaceBuildPresetByParameterHash(ctx, workspaceBuild.ID) + require.NoError(t, err) + + require.Equal(t, presetHash.ParameterHash, buildHash.ParameterHash, "Hashes of the parameter sets for the preset and the build should match") + + // Verify the preset was set + updatedBuild, err := db.GetWorkspaceBuildByID(ctx, workspaceBuild.ID) + require.NoError(t, err) + require.True(t, updatedBuild.TemplateVersionPresetID.Valid) + require.Equal(t, tmplV1.preset.ID, updatedBuild.TemplateVersionPresetID.UUID) + + // Verify antagonist presets do not match + antagonist1Hash, err := db.UpdatePresetParameterHash(ctx, antagonistPreset1.ID) + require.NoError(t, err) + antagonist2Hash, err := db.UpdatePresetParameterHash(ctx, antagonistPreset2.ID) + require.NoError(t, err) + + // Antagonist 1 should have same hash but different template version (should not match due to template version filter) + require.Equal(t, buildHash.ParameterHash, antagonist1Hash.ParameterHash, "Antagonist 1 should have same parameter hash") + require.NotEqual(t, antagonistPreset1.ID, updatedBuild.TemplateVersionPresetID.UUID, "Antagonist 1 should not match due to different template version") + + // Antagonist 2 should have different hash (should not match due to different parameters) + require.NotEqual(t, buildHash.ParameterHash, antagonist2Hash.ParameterHash, "Antagonist 2 should have different parameter hash") + require.NotEqual(t, antagonistPreset2.ID, updatedBuild.TemplateVersionPresetID.UUID, "Antagonist 2 should not match due to different parameters") + }) + + t.Run("NoMatchingPreset", func(t *testing.T) { + t.Parallel() + + db, _ := dbtestutil.NewDB(t) + ctx := testutil.Context(t, testutil.WaitShort) + + // Create organization and user + dbgen.Organization(t, db, database.Organization{ + ID: orgID, + }) + dbgen.User(t, db, database.User{ + ID: userID, + }) + + // Create template and template version + tmpl := createTemplate(t, db, orgID, userID) + tmplV1 := createTmplVersionAndPreset(t, db, tmpl, tmpl.ActiveVersionID, now, nil) + + // Create workspace and workspace build + workspace := dbgen.Workspace(t, db, database.WorkspaceTable{ + TemplateID: tmpl.ID, + OwnerID: userID, + }) + + // Create a provisioner job for the workspace build + job := dbgen.ProvisionerJob(t, db, nil, database.ProvisionerJob{ + OrganizationID: orgID, + Type: database.ProvisionerJobTypeWorkspaceBuild, + }) + + workspaceBuild := dbgen.WorkspaceBuild(t, db, database.WorkspaceBuild{ + WorkspaceID: workspace.ID, + TemplateVersionID: tmpl.ActiveVersionID, + JobID: job.ID, + BuildNumber: 1, + Transition: database.WorkspaceTransitionStart, + }) + + // Insert workspace build parameters that don't match any preset + err := db.InsertWorkspaceBuildParameters(ctx, database.InsertWorkspaceBuildParametersParams{ + WorkspaceBuildID: workspaceBuild.ID, + Name: []string{"different_param"}, + Value: []string{"different_value"}, + }) + require.NoError(t, err) + + // Insert preset parameters with different values + _, err = db.InsertPresetParameters(ctx, database.InsertPresetParametersParams{ + TemplateVersionPresetID: tmplV1.preset.ID, + Names: []string{"param1", "param2"}, + Values: []string{"value1", "value2"}, + }) + require.NoError(t, err) + + presetHash, err := db.UpdatePresetParameterHash(ctx, tmplV1.preset.ID) + require.NoError(t, err) + + // Run the query to set the preset + buildHash, err := db.SetWorkspaceBuildPresetByParameterHash(ctx, workspaceBuild.ID) + require.NoError(t, err) + + require.NotEqual(t, presetHash.ParameterHash, buildHash.ParameterHash, "Preset and build hashes should not match") + + // Verify no preset was set (should be NULL) + updatedBuild, err := db.GetWorkspaceBuildByID(ctx, workspaceBuild.ID) + require.NoError(t, err) + require.Equal(t, uuid.Nil, updatedBuild.TemplateVersionPresetID.UUID) + }) + + t.Run("ParameterOrderDoesNotMatter", func(t *testing.T) { + t.Parallel() + + db, _ := dbtestutil.NewDB(t) + ctx := testutil.Context(t, testutil.WaitShort) + + // Create organization and user + dbgen.Organization(t, db, database.Organization{ + ID: orgID, + }) + dbgen.User(t, db, database.User{ + ID: userID, + }) + + // Create template and template version + tmpl := createTemplate(t, db, orgID, userID) + tmplV1 := createTmplVersionAndPreset(t, db, tmpl, tmpl.ActiveVersionID, now, nil) + + // Create workspace and workspace build + workspace := dbgen.Workspace(t, db, database.WorkspaceTable{ + TemplateID: tmpl.ID, + OwnerID: userID, + }) + + // Create a provisioner job for the workspace build + job := dbgen.ProvisionerJob(t, db, nil, database.ProvisionerJob{ + OrganizationID: orgID, + Type: database.ProvisionerJobTypeWorkspaceBuild, + }) + + workspaceBuild := dbgen.WorkspaceBuild(t, db, database.WorkspaceBuild{ + WorkspaceID: workspace.ID, + TemplateVersionID: tmpl.ActiveVersionID, + JobID: job.ID, + BuildNumber: 1, + Transition: database.WorkspaceTransitionStart, + }) + + // Insert workspace build parameters in one order + err := db.InsertWorkspaceBuildParameters(ctx, database.InsertWorkspaceBuildParametersParams{ + WorkspaceBuildID: workspaceBuild.ID, + Name: []string{"param2", "param1"}, // Different order + Value: []string{"value2", "value1"}, // Different order + }) + require.NoError(t, err) + + // Insert preset parameters in different order + _, err = db.InsertPresetParameters(ctx, database.InsertPresetParametersParams{ + TemplateVersionPresetID: tmplV1.preset.ID, + Names: []string{"param1", "param2"}, // Different order + Values: []string{"value1", "value2"}, // Different order + }) + require.NoError(t, err) + + presetHash, err := db.UpdatePresetParameterHash(ctx, tmplV1.preset.ID) + require.NoError(t, err) + + // Run the query to set the preset + buildHash, err := db.SetWorkspaceBuildPresetByParameterHash(ctx, workspaceBuild.ID) + require.NoError(t, err) + + require.Equal(t, presetHash.ParameterHash, buildHash.ParameterHash, "Preset and build hashes should match") + + // Verify the preset was set (hash should match regardless of order) + updatedBuild, err := db.GetWorkspaceBuildByID(ctx, workspaceBuild.ID) + require.NoError(t, err) + require.True(t, updatedBuild.TemplateVersionPresetID.Valid) + require.Equal(t, tmplV1.preset.ID, updatedBuild.TemplateVersionPresetID.UUID) + }) +} diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index b078e2dbb29c0..4b6283659e4f0 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -7628,7 +7628,7 @@ func (q *sqlQuerier) GetActivePresetPrebuildSchedules(ctx context.Context) ([]Te } const getPresetByID = `-- name: GetPresetByID :one -SELECT tvp.id, tvp.template_version_id, tvp.name, tvp.created_at, tvp.desired_instances, tvp.invalidate_after_secs, tvp.prebuild_status, tvp.scheduling_timezone, tvp.is_default, tvp.description, tvp.icon, tv.template_id, tv.organization_id FROM +SELECT tvp.id, tvp.template_version_id, tvp.name, tvp.created_at, tvp.desired_instances, tvp.invalidate_after_secs, tvp.prebuild_status, tvp.scheduling_timezone, tvp.is_default, tvp.description, tvp.icon, tvp.parameter_hash, tv.template_id, tv.organization_id FROM template_version_presets tvp INNER JOIN template_versions tv ON tvp.template_version_id = tv.id WHERE tvp.id = $1 @@ -7646,6 +7646,7 @@ type GetPresetByIDRow struct { IsDefault bool `db:"is_default" json:"is_default"` Description string `db:"description" json:"description"` Icon string `db:"icon" json:"icon"` + ParameterHash sql.NullString `db:"parameter_hash" json:"parameter_hash"` TemplateID uuid.NullUUID `db:"template_id" json:"template_id"` OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` } @@ -7665,6 +7666,7 @@ func (q *sqlQuerier) GetPresetByID(ctx context.Context, presetID uuid.UUID) (Get &i.IsDefault, &i.Description, &i.Icon, + &i.ParameterHash, &i.TemplateID, &i.OrganizationID, ) @@ -7673,7 +7675,7 @@ func (q *sqlQuerier) GetPresetByID(ctx context.Context, presetID uuid.UUID) (Get const getPresetByWorkspaceBuildID = `-- name: GetPresetByWorkspaceBuildID :one SELECT - template_version_presets.id, template_version_presets.template_version_id, template_version_presets.name, template_version_presets.created_at, template_version_presets.desired_instances, template_version_presets.invalidate_after_secs, template_version_presets.prebuild_status, template_version_presets.scheduling_timezone, template_version_presets.is_default, template_version_presets.description, template_version_presets.icon + template_version_presets.id, template_version_presets.template_version_id, template_version_presets.name, template_version_presets.created_at, template_version_presets.desired_instances, template_version_presets.invalidate_after_secs, template_version_presets.prebuild_status, template_version_presets.scheduling_timezone, template_version_presets.is_default, template_version_presets.description, template_version_presets.icon, template_version_presets.parameter_hash FROM template_version_presets INNER JOIN workspace_builds ON workspace_builds.template_version_preset_id = template_version_presets.id @@ -7696,6 +7698,7 @@ func (q *sqlQuerier) GetPresetByWorkspaceBuildID(ctx context.Context, workspaceB &i.IsDefault, &i.Description, &i.Icon, + &i.ParameterHash, ) return i, err } @@ -7777,7 +7780,7 @@ func (q *sqlQuerier) GetPresetParametersByTemplateVersionID(ctx context.Context, const getPresetsByTemplateVersionID = `-- name: GetPresetsByTemplateVersionID :many SELECT - id, template_version_id, name, created_at, desired_instances, invalidate_after_secs, prebuild_status, scheduling_timezone, is_default, description, icon + id, template_version_id, name, created_at, desired_instances, invalidate_after_secs, prebuild_status, scheduling_timezone, is_default, description, icon, parameter_hash FROM template_version_presets WHERE @@ -7805,6 +7808,7 @@ func (q *sqlQuerier) GetPresetsByTemplateVersionID(ctx context.Context, template &i.IsDefault, &i.Description, &i.Icon, + &i.ParameterHash, ); err != nil { return nil, err } @@ -7843,7 +7847,7 @@ VALUES ( $8, $9, $10 -) RETURNING id, template_version_id, name, created_at, desired_instances, invalidate_after_secs, prebuild_status, scheduling_timezone, is_default, description, icon +) RETURNING id, template_version_id, name, created_at, desired_instances, invalidate_after_secs, prebuild_status, scheduling_timezone, is_default, description, icon, parameter_hash ` type InsertPresetParams struct { @@ -7885,6 +7889,7 @@ func (q *sqlQuerier) InsertPreset(ctx context.Context, arg InsertPresetParams) ( &i.IsDefault, &i.Description, &i.Icon, + &i.ParameterHash, ) return i, err } @@ -7964,6 +7969,31 @@ func (q *sqlQuerier) InsertPresetPrebuildSchedule(ctx context.Context, arg Inser return i, err } +const updatePresetParameterHash = `-- name: UpdatePresetParameterHash :one +WITH preset_hash AS ( + SELECT generate_parameter_hash( + (SELECT array_agg(tvpp.name ORDER BY tvpp.name) FROM template_version_preset_parameters tvpp WHERE tvpp.template_version_preset_id = $1), + (SELECT array_agg(tvpp.value ORDER BY tvpp.name) FROM template_version_preset_parameters tvpp WHERE tvpp.template_version_preset_id = $1) + ) as parameter_hash +) +UPDATE template_version_presets +SET parameter_hash = (SELECT parameter_hash FROM preset_hash) +WHERE template_version_presets.id = $1 +RETURNING template_version_presets.id, (SELECT parameter_hash FROM preset_hash) as parameter_hash +` + +type UpdatePresetParameterHashRow struct { + ID uuid.UUID `db:"id" json:"id"` + ParameterHash string `db:"parameter_hash" json:"parameter_hash"` +} + +func (q *sqlQuerier) UpdatePresetParameterHash(ctx context.Context, id uuid.UUID) (UpdatePresetParameterHashRow, error) { + row := q.db.QueryRowContext(ctx, updatePresetParameterHash, id) + var i UpdatePresetParameterHashRow + err := row.Scan(&i.ID, &i.ParameterHash) + return i, err +} + const updatePresetPrebuildStatus = `-- name: UpdatePresetPrebuildStatus :exec UPDATE template_version_presets SET prebuild_status = $1 @@ -18917,6 +18947,37 @@ func (q *sqlQuerier) InsertWorkspaceBuild(ctx context.Context, arg InsertWorkspa return err } +const setWorkspaceBuildPresetByParameterHash = `-- name: SetWorkspaceBuildPresetByParameterHash :one +WITH build_hash AS ( + SELECT generate_parameter_hash( + (SELECT array_agg(wbp.name ORDER BY wbp.name) FROM workspace_build_parameters wbp WHERE wbp.workspace_build_id = $1), + (SELECT array_agg(wbp.value ORDER BY wbp.name) FROM workspace_build_parameters wbp WHERE wbp.workspace_build_id = $1) + ) as parameter_hash +) +UPDATE workspace_builds +SET template_version_preset_id = ( + SELECT tvp.id + FROM template_version_presets tvp, build_hash + WHERE tvp.template_version_id = workspace_builds.template_version_id + AND tvp.parameter_hash = build_hash.parameter_hash + LIMIT 1 +) +WHERE workspace_builds.id = $1 +RETURNING workspace_builds.id, (SELECT parameter_hash FROM build_hash) as parameter_hash +` + +type SetWorkspaceBuildPresetByParameterHashRow struct { + ID uuid.UUID `db:"id" json:"id"` + ParameterHash string `db:"parameter_hash" json:"parameter_hash"` +} + +func (q *sqlQuerier) SetWorkspaceBuildPresetByParameterHash(ctx context.Context, workspaceBuildID uuid.UUID) (SetWorkspaceBuildPresetByParameterHashRow, error) { + row := q.db.QueryRowContext(ctx, setWorkspaceBuildPresetByParameterHash, workspaceBuildID) + var i SetWorkspaceBuildPresetByParameterHashRow + err := row.Scan(&i.ID, &i.ParameterHash) + return i, err +} + const updateWorkspaceBuildAITaskByID = `-- name: UpdateWorkspaceBuildAITaskByID :exec UPDATE workspace_builds diff --git a/coderd/database/queries/presets.sql b/coderd/database/queries/presets.sql index e6edcb4c59c1f..df7c76a4504ce 100644 --- a/coderd/database/queries/presets.sql +++ b/coderd/database/queries/presets.sql @@ -50,6 +50,18 @@ UPDATE template_version_presets SET prebuild_status = @status WHERE id = @preset_id; +-- name: UpdatePresetParameterHash :one +WITH preset_hash AS ( + SELECT generate_parameter_hash( + (SELECT array_agg(tvpp.name ORDER BY tvpp.name) FROM template_version_preset_parameters tvpp WHERE tvpp.template_version_preset_id = @id), + (SELECT array_agg(tvpp.value ORDER BY tvpp.name) FROM template_version_preset_parameters tvpp WHERE tvpp.template_version_preset_id = @id) + ) as parameter_hash +) +UPDATE template_version_presets +SET parameter_hash = (SELECT parameter_hash FROM preset_hash) +WHERE template_version_presets.id = @id +RETURNING template_version_presets.id, (SELECT parameter_hash FROM preset_hash) as parameter_hash; + -- name: GetPresetsByTemplateVersionID :many SELECT * diff --git a/coderd/database/queries/workspacebuilds.sql b/coderd/database/queries/workspacebuilds.sql index be76b6642df1f..6c3083a05f7fb 100644 --- a/coderd/database/queries/workspacebuilds.sql +++ b/coderd/database/queries/workspacebuilds.sql @@ -253,3 +253,21 @@ WHERE AND pj.job_status = 'failed' ORDER BY tv.name ASC, wb.build_number DESC; + +-- name: SetWorkspaceBuildPresetByParameterHash :one +WITH build_hash AS ( + SELECT generate_parameter_hash( + (SELECT array_agg(wbp.name ORDER BY wbp.name) FROM workspace_build_parameters wbp WHERE wbp.workspace_build_id = @workspace_build_id), + (SELECT array_agg(wbp.value ORDER BY wbp.name) FROM workspace_build_parameters wbp WHERE wbp.workspace_build_id = @workspace_build_id) + ) as parameter_hash +) +UPDATE workspace_builds +SET template_version_preset_id = ( + SELECT tvp.id + FROM template_version_presets tvp, build_hash + WHERE tvp.template_version_id = workspace_builds.template_version_id + AND tvp.parameter_hash = build_hash.parameter_hash + LIMIT 1 +) +WHERE workspace_builds.id = @workspace_build_id +RETURNING workspace_builds.id, (SELECT parameter_hash FROM build_hash) as parameter_hash; diff --git a/coderd/provisionerdserver/provisionerdserver.go b/coderd/provisionerdserver/provisionerdserver.go index 94173703c467d..230b0ffb354cc 100644 --- a/coderd/provisionerdserver/provisionerdserver.go +++ b/coderd/provisionerdserver/provisionerdserver.go @@ -2365,6 +2365,12 @@ func InsertWorkspacePresetAndParameters(ctx context.Context, db database.Store, presetParameterNames = append(presetParameterNames, parameter.Name) presetParameterValues = append(presetParameterValues, parameter.Value) } + + _, err = tx.UpdatePresetParameterHash(ctx, dbPreset.ID) + if err != nil { + return xerrors.Errorf("update preset parameter hash: %w", err) + } + _, err = tx.InsertPresetParameters(ctx, database.InsertPresetParametersParams{ TemplateVersionPresetID: dbPreset.ID, Names: presetParameterNames, diff --git a/coderd/wsbuilder/wsbuilder.go b/coderd/wsbuilder/wsbuilder.go index 00bced630df38..56799442900a6 100644 --- a/coderd/wsbuilder/wsbuilder.go +++ b/coderd/wsbuilder/wsbuilder.go @@ -442,57 +442,6 @@ 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 { - parameterMap := make(map[string]string) - for i, name := range names { - parameterMap[name] = values[i] - } - - presetParameters, err := b.store.GetPresetParametersByTemplateVersionID(b.ctx, templateVersionID) - if err != nil { - return BuildError{http.StatusInternalServerError, "get preset parameters", err} - } - - presetMap := make(map[uuid.UUID]map[string]string) - for _, presetParameter := range presetParameters { - if _, ok := presetMap[presetParameter.TemplateVersionPresetID]; !ok { - presetMap[presetParameter.TemplateVersionPresetID] = make(map[string]string) - } - presetMap[presetParameter.TemplateVersionPresetID][presetParameter.Name] = presetParameter.Value - } - - // Compare each preset's parameters to the provided parameters to find any matches - for presetID, presetParams := range presetMap { - isMatch := true - // Check that all preset parameters match the provided parameters - for paramName, presetValue := range presetParams { - if providedValue, exists := parameterMap[paramName]; !exists || providedValue != presetValue { - isMatch = false - break - } - } - // Check that all provided parameters match the preset parameters - if isMatch { - for paramName, providedValue := range parameterMap { - if presetValue, exists := presetParams[paramName]; !exists || providedValue != presetValue { - isMatch = false - break - } - } - } - if isMatch { - b.templateVersionPresetID = presetID - break - } - } - } - err = store.InsertWorkspaceBuild(b.ctx, database.InsertWorkspaceBuildParams{ ID: workspaceBuildID, CreatedAt: now, @@ -524,6 +473,12 @@ 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, @@ -533,6 +488,11 @@ func (b *Builder) buildTx(authFunc func(action policy.Action, object rbac.Object return BuildError{http.StatusInternalServerError, "insert workspace build parameters: %w", err} } + _, err = store.SetWorkspaceBuildPresetByParameterHash(b.ctx, workspaceBuildID) + if err != nil { + return BuildError{http.StatusInternalServerError, "set workspace build preset by parameter hash", err} + } + workspaceBuild, err = store.GetWorkspaceBuildByID(b.ctx, workspaceBuildID) if err != nil { return BuildError{http.StatusInternalServerError, "get workspace build", err}