Skip to content

Commit 27bc60d

Browse files
evgeniy-scherbinaSasSwartdannykoppingdeansheatherspikecurtis
authored
feat: implement reconciliation loop (coder#17261)
Closes coder/internal#510 <details> <summary> Refactoring Summary </summary> ### 1) `CalculateActions` Function #### Issues Before Refactoring: - Large function (~150 lines), making it difficult to read and maintain. - The control flow is hard to follow due to complex conditional logic. - The `ReconciliationActions` struct was partially initialized early, then mutated in multiple places, making the flow error-prone. Original source: https://github.com/coder/coder/blob/fe60b569ad754245e28bac71e0ef3c83536631bb/coderd/prebuilds/state.go#L13-L167 #### Improvements After Refactoring: - Simplified and broken down into smaller, focused helper methods. - The flow of the function is now more linear and easier to understand. - Struct initialization is cleaner, avoiding partial and incremental mutations. Refactored function: https://github.com/coder/coder/blob/eeb0407d783cdda71ec2418c113f325542c47b1c/coderd/prebuilds/state.go#L67-L84 --- ### 2) `ReconciliationActions` Struct #### Issues Before Refactoring: - The struct mixed both actionable decisions and diagnostic state, which blurred its purpose. - It was unclear which fields were necessary for reconciliation logic, and which were purely for logging/observability. #### Improvements After Refactoring: - Split into two clear, purpose-specific structs: - **`ReconciliationActions`** — defines the intended reconciliation action. - **`ReconciliationState`** — captures runtime state and metadata, primarily for logging and diagnostics. Original struct: https://github.com/coder/coder/blob/fe60b569ad754245e28bac71e0ef3c83536631bb/coderd/prebuilds/reconcile.go#L29-L41 </details> --------- Signed-off-by: Danny Kopping <dannykopping@gmail.com> Co-authored-by: Sas Swart <sas.swart.cdk@gmail.com> Co-authored-by: Danny Kopping <dannykopping@gmail.com> Co-authored-by: Dean Sheather <dean@deansheather.com> Co-authored-by: Spike Curtis <spike@coder.com> Co-authored-by: Danny Kopping <danny@coder.com>
1 parent 6a79965 commit 27bc60d

File tree

16 files changed

+2834
-9
lines changed

16 files changed

+2834
-9
lines changed

‎coderd/database/lock.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,7 @@ const (
1212
LockIDDBPurge
1313
LockIDNotificationsReportGenerator
1414
LockIDCryptoKeyRotation
15-
LockIDReconcileTemplatePrebuilds
16-
LockIDDeterminePrebuildsState
15+
LockIDReconcilePrebuilds
1716
)
1817

1918
// GenLockID generates a unique and consistent lock ID from a given string.

‎coderd/database/querier.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go

Lines changed: 5 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/prebuilds.sql

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,9 @@ WHERE (b.transition = 'start'::workspace_transition
5757
AND b.job_status = 'succeeded'::provisioner_job_status);
5858

5959
-- name: CountInProgressPrebuilds :many
60-
-- CountInProgressPrebuilds returns the number of in-progress prebuilds, grouped by template version ID and transition.
60+
-- CountInProgressPrebuilds returns the number of in-progress prebuilds, grouped by preset 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, wlb.template_version_preset_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.
@@ -70,7 +70,7 @@ FROM workspace_latest_builds wlb
7070
-- prebuilds that are still building.
7171
INNER JOIN templates t ON t.active_version_id = wlb.template_version_id
7272
WHERE wlb.job_status IN ('pending'::provisioner_job_status, 'running'::provisioner_job_status)
73-
GROUP BY t.id, wpb.template_version_id, wpb.transition;
73+
GROUP BY t.id, wpb.template_version_id, wpb.transition, wlb.template_version_preset_id;
7474

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

‎coderd/prebuilds/api.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package prebuilds
2+
3+
import (
4+
"context"
5+
)
6+
7+
// ReconciliationOrchestrator manages the lifecycle of prebuild reconciliation.
8+
// It runs a continuous loop to check and reconcile prebuild states, and can be stopped gracefully.
9+
type ReconciliationOrchestrator interface {
10+
Reconciler
11+
12+
// RunLoop starts a continuous reconciliation loop that periodically calls ReconcileAll
13+
// to ensure all prebuilds are in their desired states. The loop runs until the context
14+
// is canceled or Stop is called.
15+
RunLoop(ctx context.Context)
16+
17+
// Stop gracefully shuts down the orchestrator with the given cause.
18+
// The cause is used for logging and error reporting.
19+
Stop(ctx context.Context, cause error)
20+
}
21+
22+
type Reconciler interface {
23+
// ReconcileAll orchestrates the reconciliation of all prebuilds across all templates.
24+
// It takes a global snapshot of the system state and then reconciles each preset
25+
// in parallel, creating or deleting prebuilds as needed to reach their desired states.
26+
ReconcileAll(ctx context.Context) error
27+
}

‎coderd/prebuilds/global_snapshot.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
package prebuilds
2+
3+
import (
4+
"github.com/google/uuid"
5+
"golang.org/x/xerrors"
6+
7+
"github.com/coder/coder/v2/coderd/database"
8+
"github.com/coder/coder/v2/coderd/util/slice"
9+
)
10+
11+
// GlobalSnapshot represents a full point-in-time snapshot of state relating to prebuilds across all templates.
12+
type GlobalSnapshot struct {
13+
Presets []database.GetTemplatePresetsWithPrebuildsRow
14+
RunningPrebuilds []database.GetRunningPrebuiltWorkspacesRow
15+
PrebuildsInProgress []database.CountInProgressPrebuildsRow
16+
Backoffs []database.GetPresetsBackoffRow
17+
}
18+
19+
func NewGlobalSnapshot(
20+
presets []database.GetTemplatePresetsWithPrebuildsRow,
21+
runningPrebuilds []database.GetRunningPrebuiltWorkspacesRow,
22+
prebuildsInProgress []database.CountInProgressPrebuildsRow,
23+
backoffs []database.GetPresetsBackoffRow,
24+
) GlobalSnapshot {
25+
return GlobalSnapshot{
26+
Presets: presets,
27+
RunningPrebuilds: runningPrebuilds,
28+
PrebuildsInProgress: prebuildsInProgress,
29+
Backoffs: backoffs,
30+
}
31+
}
32+
33+
func (s GlobalSnapshot) FilterByPreset(presetID uuid.UUID) (*PresetSnapshot, error) {
34+
preset, found := slice.Find(s.Presets, func(preset database.GetTemplatePresetsWithPrebuildsRow) bool {
35+
return preset.ID == presetID
36+
})
37+
if !found {
38+
return nil, xerrors.Errorf("no preset found with ID %q", presetID)
39+
}
40+
41+
running := slice.Filter(s.RunningPrebuilds, func(prebuild database.GetRunningPrebuiltWorkspacesRow) bool {
42+
if !prebuild.CurrentPresetID.Valid {
43+
return false
44+
}
45+
return prebuild.CurrentPresetID.UUID == preset.ID
46+
})
47+
48+
inProgress := slice.Filter(s.PrebuildsInProgress, func(prebuild database.CountInProgressPrebuildsRow) bool {
49+
return prebuild.PresetID.UUID == preset.ID
50+
})
51+
52+
var backoffPtr *database.GetPresetsBackoffRow
53+
backoff, found := slice.Find(s.Backoffs, func(row database.GetPresetsBackoffRow) bool {
54+
return row.PresetID == preset.ID
55+
})
56+
if found {
57+
backoffPtr = &backoff
58+
}
59+
60+
return &PresetSnapshot{
61+
Preset: preset,
62+
Running: running,
63+
InProgress: inProgress,
64+
Backoff: backoffPtr,
65+
}, nil
66+
}

‎coderd/prebuilds/noop.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
package prebuilds
2+
3+
import (
4+
"context"
5+
6+
"github.com/coder/coder/v2/coderd/database"
7+
)
8+
9+
type NoopReconciler struct{}
10+
11+
func NewNoopReconciler() *NoopReconciler {
12+
return &NoopReconciler{}
13+
}
14+
15+
func (NoopReconciler) RunLoop(context.Context) {}
16+
17+
func (NoopReconciler) Stop(context.Context, error) {}
18+
19+
func (NoopReconciler) ReconcileAll(context.Context) error {
20+
return nil
21+
}
22+
23+
func (NoopReconciler) SnapshotState(context.Context, database.Store) (*GlobalSnapshot, error) {
24+
return &GlobalSnapshot{}, nil
25+
}
26+
27+
func (NoopReconciler) ReconcilePreset(context.Context, PresetSnapshot) error {
28+
return nil
29+
}
30+
31+
func (NoopReconciler) CalculateActions(context.Context, PresetSnapshot) (*ReconciliationActions, error) {
32+
return &ReconciliationActions{}, nil
33+
}
34+
35+
var _ ReconciliationOrchestrator = NoopReconciler{}

0 commit comments

Comments
 (0)