Skip to content

Commit c0246f4

Browse files
evgeniy-scherbinaevgeniy-scherbina
committed
refactor: make sure InProgress works on preset level as well
1 parent a79fe4c commit c0246f4

File tree

5 files changed

+179
-156
lines changed

5 files changed

+179
-156
lines changed

‎coderd/database/queries.sql.go

+5-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/prebuilds.sql

+3-2
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ WHERE (b.transition = 'start'::workspace_transition
5959
-- name: CountInProgressPrebuilds :many
6060
-- CountInProgressPrebuilds returns the number of in-progress prebuilds, grouped by template version ID and transition.
6161
-- Prebuild considered in-progress if it's in the "starting", "stopping", or "deleting" state.
62-
SELECT t.id AS template_id, wpb.template_version_id, wpb.transition, COUNT(wpb.transition)::int AS count
62+
SELECT t.id AS template_id, wpb.template_version_id, wpb.transition, COUNT(wpb.transition)::int AS count, tvp.id as preset_id
6363
FROM workspace_latest_builds wlb
6464
INNER JOIN workspace_prebuild_builds wpb ON wpb.id = wlb.id
6565
-- We only need these counts for active template versions.
@@ -69,8 +69,9 @@ FROM workspace_latest_builds wlb
6969
-- running prebuilds for inactive template versions, and we ignore
7070
-- prebuilds that are still building.
7171
INNER JOIN templates t ON t.active_version_id = wlb.template_version_id
72+
INNER JOIN template_version_presets tvp ON wlb.template_version_preset_id = tvp.id
7273
WHERE wlb.job_status IN ('pending'::provisioner_job_status, 'running'::provisioner_job_status)
73-
GROUP BY t.id, wpb.template_version_id, wpb.transition;
74+
GROUP BY t.id, wpb.template_version_id, wpb.transition, tvp.id;
7475

7576
-- GetPresetsBackoff groups workspace builds by preset ID.
7677
-- Each preset is associated with exactly one template version ID.

‎coderd/prebuilds/global_snapshot.go

+2-6
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,8 @@ func (s GlobalSnapshot) FilterByPreset(presetID uuid.UUID) (*PresetSnapshot, err
4545
return prebuild.CurrentPresetID.UUID == preset.ID
4646
})
4747

48-
// These aren't preset-specific, but they need to inhibit all presets of this template from operating since they could
49-
// be in-progress builds which might impact another preset. For example, if a template goes from no defined prebuilds to defined prebuilds
50-
// and back, or a template is updated from one version to another.
51-
// We group by the template so that all prebuilds being provisioned for a prebuild are inhibited if any prebuild for
52-
// any preset in that template are in progress, to prevent clobbering.
5348
inProgress := slice.Filter(s.PrebuildsInProgress, func(prebuild database.CountInProgressPrebuildsRow) bool {
54-
return prebuild.TemplateID == preset.TemplateID
49+
return prebuild.PresetID == preset.ID
5550
})
5651

5752
var backoffPtr *database.GetPresetsBackoffRow
@@ -60,6 +55,7 @@ func (s GlobalSnapshot) FilterByPreset(presetID uuid.UUID) (*PresetSnapshot, err
6055
})
6156
if found {
6257
backoffPtr = &backoff
58+
6359
}
6460

6561
return &PresetSnapshot{

‎coderd/prebuilds/preset_snapshot_test.go

+83-59
Original file line numberDiff line numberDiff line change
@@ -63,64 +63,6 @@ var opts = map[uint]options{
6363
},
6464
}
6565

66-
func TestMultiplePresetsPerTemplateVersion(t *testing.T) {
67-
t.Parallel()
68-
69-
templateID := uuid.New()
70-
templateVersionID := uuid.New()
71-
presetOpts1 := options{
72-
templateID: templateID,
73-
templateVersionID: templateVersionID,
74-
presetID: uuid.New(),
75-
presetName: "my-preset-1",
76-
prebuildID: uuid.New(),
77-
workspaceName: "prebuilds1",
78-
}
79-
presetOpts2 := options{
80-
templateID: templateID,
81-
templateVersionID: templateVersionID,
82-
presetID: uuid.New(),
83-
presetName: "my-preset-2",
84-
prebuildID: uuid.New(),
85-
workspaceName: "prebuilds2",
86-
}
87-
88-
clock := quartz.NewMock(t)
89-
90-
presets := []database.GetTemplatePresetsWithPrebuildsRow{
91-
preset(true, 0, presetOpts1),
92-
preset(true, 0, presetOpts2),
93-
}
94-
95-
inProgress := []database.CountInProgressPrebuildsRow{
96-
{
97-
TemplateID: templateID,
98-
TemplateVersionID: templateVersionID,
99-
Transition: database.WorkspaceTransitionStart,
100-
Count: 1,
101-
},
102-
}
103-
104-
snapshot := prebuilds.NewGlobalSnapshot(presets, nil, inProgress, nil)
105-
106-
for _, presetID := range []uuid.UUID{presetOpts1.presetID, presetOpts2.presetID} {
107-
ps, err := snapshot.FilterByPreset(presetID)
108-
require.NoError(t, err)
109-
110-
state := ps.CalculateState()
111-
actions, err := ps.CalculateActions(clock, backoffInterval)
112-
require.NoError(t, err)
113-
114-
validateState(t, prebuilds.ReconciliationState{
115-
Starting: 1,
116-
}, *state)
117-
validateActions(t, prebuilds.ReconciliationActions{
118-
ActionType: prebuilds.ActionTypeCreate,
119-
Create: 0,
120-
}, *actions)
121-
}
122-
}
123-
12466
// A new template version with a preset without prebuilds configured should result in no prebuilds being created.
12567
func TestNoPrebuilds(t *testing.T) {
12668
t.Parallel()
@@ -414,8 +356,9 @@ func TestInProgressActions(t *testing.T) {
414356
t.Parallel()
415357

416358
// GIVEN: a preset.
359+
defaultPreset := preset(true, tc.desired, current)
417360
presets := []database.GetTemplatePresetsWithPrebuildsRow{
418-
preset(true, tc.desired, current),
361+
defaultPreset,
419362
}
420363

421364
// GIVEN: a running prebuild for the preset.
@@ -441,6 +384,7 @@ func TestInProgressActions(t *testing.T) {
441384
TemplateVersionID: current.templateVersionID,
442385
Transition: tc.transition,
443386
Count: tc.inProgress,
387+
PresetID: defaultPreset.ID,
444388
},
445389
}
446390

@@ -628,6 +572,86 @@ func TestLatestBuildFailed(t *testing.T) {
628572
}, *actions)
629573
}
630574

575+
func TestMultiplePresetsPerTemplateVersion(t *testing.T) {
576+
t.Parallel()
577+
578+
templateID := uuid.New()
579+
templateVersionID := uuid.New()
580+
presetOpts1 := options{
581+
templateID: templateID,
582+
templateVersionID: templateVersionID,
583+
presetID: uuid.New(),
584+
presetName: "my-preset-1",
585+
prebuildID: uuid.New(),
586+
workspaceName: "prebuilds1",
587+
}
588+
presetOpts2 := options{
589+
templateID: templateID,
590+
templateVersionID: templateVersionID,
591+
presetID: uuid.New(),
592+
presetName: "my-preset-2",
593+
prebuildID: uuid.New(),
594+
workspaceName: "prebuilds2",
595+
}
596+
597+
clock := quartz.NewMock(t)
598+
599+
presets := []database.GetTemplatePresetsWithPrebuildsRow{
600+
preset(true, 1, presetOpts1),
601+
preset(true, 1, presetOpts2),
602+
}
603+
604+
inProgress := []database.CountInProgressPrebuildsRow{
605+
{
606+
TemplateID: templateID,
607+
TemplateVersionID: templateVersionID,
608+
Transition: database.WorkspaceTransitionStart,
609+
Count: 1,
610+
PresetID: presetOpts1.presetID,
611+
},
612+
}
613+
614+
snapshot := prebuilds.NewGlobalSnapshot(presets, nil, inProgress, nil)
615+
616+
// Nothing has to be created for preset 1.
617+
{
618+
ps, err := snapshot.FilterByPreset(presetOpts1.presetID)
619+
require.NoError(t, err)
620+
621+
state := ps.CalculateState()
622+
actions, err := ps.CalculateActions(clock, backoffInterval)
623+
require.NoError(t, err)
624+
625+
validateState(t, prebuilds.ReconciliationState{
626+
Starting: 1,
627+
Desired: 1,
628+
}, *state)
629+
validateActions(t, prebuilds.ReconciliationActions{
630+
ActionType: prebuilds.ActionTypeCreate,
631+
Create: 0,
632+
}, *actions)
633+
}
634+
635+
// One prebuild has to be created for preset 2. Make sure preset 1 doesn't block preset 2.
636+
{
637+
ps, err := snapshot.FilterByPreset(presetOpts2.presetID)
638+
require.NoError(t, err)
639+
640+
state := ps.CalculateState()
641+
actions, err := ps.CalculateActions(clock, backoffInterval)
642+
require.NoError(t, err)
643+
644+
validateState(t, prebuilds.ReconciliationState{
645+
Starting: 0,
646+
Desired: 1,
647+
}, *state)
648+
validateActions(t, prebuilds.ReconciliationActions{
649+
ActionType: prebuilds.ActionTypeCreate,
650+
Create: 1,
651+
}, *actions)
652+
}
653+
}
654+
631655
func preset(active bool, instances int32, opts options, muts ...func(row database.GetTemplatePresetsWithPrebuildsRow) database.GetTemplatePresetsWithPrebuildsRow) database.GetTemplatePresetsWithPrebuildsRow {
632656
entry := database.GetTemplatePresetsWithPrebuildsRow{
633657
TemplateID: opts.templateID,

0 commit comments

Comments
 (0)