Skip to content

Commit 579f362

Browse files
johnstcnjohnstcn
committed
make the new tests pass with some refactoring
1 parent ce63810 commit 579f362

File tree

6 files changed

+211
-54
lines changed

6 files changed

+211
-54
lines changed

‎coderd/autostart/lifecycle/lifecycle_executor.go

Lines changed: 57 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,12 @@ import (
99

1010
"github.com/coder/coder/coderd/autostart/schedule"
1111
"github.com/coder/coder/coderd/database"
12-
"github.com/coder/coder/codersdk"
1312

1413
"github.com/google/uuid"
1514
"github.com/moby/moby/pkg/namesgenerator"
1615
"golang.org/x/xerrors"
1716
)
1817

19-
//var ExecutorUUID = uuid.MustParse("00000000-0000-0000-0000-000000000000")
20-
2118
// Executor executes automated workspace lifecycle operations.
2219
type Executor struct {
2320
ctx context.Context
@@ -26,6 +23,7 @@ type Executor struct {
2623
tick <-chan time.Time
2724
}
2825

26+
// NewExecutor returns a new instance of Executor.
2927
func NewExecutor(ctx context.Context, db database.Store, log slog.Logger, tick <-chan time.Time) *Executor {
3028
le := &Executor{
3129
ctx: ctx,
@@ -36,15 +34,18 @@ func NewExecutor(ctx context.Context, db database.Store, log slog.Logger, tick <
3634
return le
3735
}
3836

39-
func (e *Executor) Run() error {
37+
// Run will cause executor to run workspace lifecycle operations on every
38+
// tick from its channel. It will stop when its context is Done, or when
39+
// its channel is closed.
40+
func (e *Executor) Run() {
4041
for {
4142
select {
4243
case t := <-e.tick:
4344
if err := e.runOnce(t); err != nil {
4445
e.log.Error(e.ctx, "error running once", slog.Error(err))
4546
}
4647
case <-e.ctx.Done():
47-
return nil
48+
return
4849
default:
4950
}
5051
}
@@ -53,65 +54,89 @@ func (e *Executor) Run() error {
5354
func (e *Executor) runOnce(t time.Time) error {
5455
currentTick := t.Round(time.Minute)
5556
return e.db.InTx(func(db database.Store) error {
56-
autostartWorkspaces, err := db.GetWorkspacesAutostart(e.ctx)
57+
eligibleWorkspaces, err := db.GetWorkspacesAutostartAutostop(e.ctx)
5758
if err != nil {
58-
return xerrors.Errorf("get all workspaces: %w", err)
59+
return xerrors.Errorf("get eligible workspaces for autostart or autostop: %w", err)
5960
}
6061

61-
for _, ws := range autostartWorkspaces {
62-
sched, err := schedule.Weekly(ws.AutostartSchedule.String)
63-
if err != nil {
64-
e.log.Warn(e.ctx, "workspace has invalid autostart schedule",
65-
slog.F("workspace_id", ws.ID),
66-
slog.F("autostart_schedule", ws.AutostartSchedule.String),
67-
)
68-
continue
69-
}
70-
71-
// Determine the workspace state based on its latest build. We expect it to be stopped.
62+
for _, ws := range eligibleWorkspaces {
63+
// Determine the workspace state based on its latest build.
7264
latestBuild, err := db.GetWorkspaceBuildByWorkspaceIDWithoutAfter(e.ctx, ws.ID)
7365
if err != nil {
7466
return xerrors.Errorf("get latest build for workspace %q: %w", ws.ID, err)
7567
}
76-
if latestBuild.Transition != database.WorkspaceTransitionStop {
77-
e.log.Debug(e.ctx, "autostart: skipping workspace: wrong transition",
78-
slog.F("transition", latestBuild.Transition),
68+
69+
var validTransition database.WorkspaceTransition
70+
var sched *schedule.Schedule
71+
switch latestBuild.Transition {
72+
case database.WorkspaceTransitionStart:
73+
validTransition = database.WorkspaceTransitionStop
74+
sched, err = schedule.Weekly(ws.AutostopSchedule.String)
75+
if err != nil {
76+
e.log.Warn(e.ctx, "workspace has invalid autostop schedule, skipping",
77+
slog.F("workspace_id", ws.ID),
78+
slog.F("autostart_schedule", ws.AutostopSchedule.String),
79+
)
80+
continue
81+
}
82+
case database.WorkspaceTransitionStop:
83+
validTransition = database.WorkspaceTransitionStart
84+
sched, err = schedule.Weekly(ws.AutostartSchedule.String)
85+
if err != nil {
86+
e.log.Warn(e.ctx, "workspace has invalid autostart schedule, skipping",
87+
slog.F("workspace_id", ws.ID),
88+
slog.F("autostart_schedule", ws.AutostartSchedule.String),
89+
)
90+
continue
91+
}
92+
default:
93+
e.log.Debug(e.ctx, "last transition not valid for autostart or autostop",
7994
slog.F("workspace_id", ws.ID),
95+
slog.F("latest_build_transition", latestBuild.Transition),
8096
)
81-
continue
8297
}
8398

8499
// Round time to the nearest minute, as this is the finest granularity cron supports.
85-
earliestAutostart := sched.Next(latestBuild.CreatedAt).Round(time.Minute)
86-
if earliestAutostart.After(currentTick) {
87-
e.log.Debug(e.ctx, "autostart: skipping workspace: too early",
100+
nextTransitionAt := sched.Next(latestBuild.CreatedAt).Round(time.Minute)
101+
if nextTransitionAt.After(currentTick) {
102+
e.log.Debug(e.ctx, "skipping workspace: too early",
88103
slog.F("workspace_id", ws.ID),
89-
slog.F("earliest_autostart", earliestAutostart),
104+
slog.F("next_transition_at", nextTransitionAt),
105+
slog.F("transition", validTransition),
90106
slog.F("current_tick", currentTick),
91107
)
92108
continue
93109
}
94110

95-
e.log.Info(e.ctx, "autostart: scheduling workspace start",
111+
e.log.Info(e.ctx, "scheduling workspace transition",
96112
slog.F("workspace_id", ws.ID),
113+
slog.F("transition", validTransition),
97114
)
98115

99-
if err := doBuild(e.ctx, db, ws, currentTick); err != nil {
100-
e.log.Error(e.ctx, "autostart workspace", slog.F("workspace_id", ws.ID), slog.Error(err))
116+
if err := doBuild(e.ctx, db, ws, validTransition); err != nil {
117+
e.log.Error(e.ctx, "transition workspace",
118+
slog.F("workspace_id", ws.ID),
119+
slog.F("transition", validTransition),
120+
slog.Error(err),
121+
)
101122
}
102123
}
103124
return nil
104125
})
105126
}
106127

107-
// XXX: cian: this function shouldn't really exist. Refactor.
108-
func doBuild(ctx context.Context, store database.Store, workspace database.Workspace, now time.Time) error {
128+
// TODO(cian): this function duplicates most of api.postWorkspaceBuilds. Refactor.
129+
func doBuild(ctx context.Context, store database.Store, workspace database.Workspace, trans database.WorkspaceTransition) error {
109130
template, err := store.GetTemplateByID(ctx, workspace.TemplateID)
110131
if err != nil {
111132
return xerrors.Errorf("get template: %w", err)
112133
}
113134

114135
priorHistory, err := store.GetWorkspaceBuildByWorkspaceIDWithoutAfter(ctx, workspace.ID)
136+
if err != nil {
137+
return xerrors.Errorf("get prior history: %w", err)
138+
}
139+
115140
priorJob, err := store.GetProvisionerJobByID(ctx, priorHistory.JobID)
116141
if err == nil && !priorJob.CompletedAt.Valid {
117142
return xerrors.Errorf("workspace build already active")
@@ -160,7 +185,7 @@ func doBuild(ctx context.Context, store database.Store, workspace database.Works
160185
Name: namesgenerator.GetRandomName(1),
161186
ProvisionerState: priorHistory.ProvisionerState,
162187
InitiatorID: workspace.OwnerID,
163-
Transition: database.WorkspaceTransitionStart,
188+
Transition: trans,
164189
JobID: newProvisionerJob.ID,
165190
})
166191
if err != nil {
@@ -184,24 +209,3 @@ func doBuild(ctx context.Context, store database.Store, workspace database.Works
184209
}
185210
return nil
186211
}
187-
188-
func provisionerJobStatus(j database.ProvisionerJob, now time.Time) codersdk.ProvisionerJobStatus {
189-
switch {
190-
case j.CanceledAt.Valid:
191-
if j.CompletedAt.Valid {
192-
return codersdk.ProvisionerJobCanceled
193-
}
194-
return codersdk.ProvisionerJobCanceling
195-
case !j.StartedAt.Valid:
196-
return codersdk.ProvisionerJobPending
197-
case j.CompletedAt.Valid:
198-
if j.Error.String == "" {
199-
return codersdk.ProvisionerJobSucceeded
200-
}
201-
return codersdk.ProvisionerJobFailed
202-
case now.Sub(j.UpdatedAt) > 30*time.Second:
203-
return codersdk.ProvisionerJobFailed
204-
default:
205-
return codersdk.ProvisionerJobRunning
206-
}
207-
}

‎coderd/autostart/lifecycle/lifecycle_executor_test.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ func Test_Executor_Autostart_OK(t *testing.T) {
4343
// When: the lifecycle executor ticks
4444
go func() {
4545
tickCh <- time.Now().UTC().Add(time.Minute)
46+
close(tickCh)
4647
}()
4748

4849
// Then: the workspace should be started
@@ -83,6 +84,7 @@ func Test_Executor_Autostart_AlreadyRunning(t *testing.T) {
8384
// When: the lifecycle executor ticks
8485
go func() {
8586
tickCh <- time.Now().UTC().Add(time.Minute)
87+
close(tickCh)
8688
}()
8789

8890
// Then: the workspace should not be started.
@@ -113,6 +115,7 @@ func Test_Executor_Autostart_NotEnabled(t *testing.T) {
113115
// When: the lifecycle executor ticks
114116
go func() {
115117
tickCh <- time.Now().UTC().Add(time.Minute)
118+
close(tickCh)
116119
}()
117120

118121
// Then: the workspace should not be started.
@@ -151,12 +154,13 @@ func Test_Executor_Autostop_OK(t *testing.T) {
151154
// When: the lifecycle executor ticks
152155
go func() {
153156
tickCh <- time.Now().UTC().Add(time.Minute)
157+
close(tickCh)
154158
}()
155159

156160
// Then: the workspace should be started
157161
require.Eventually(t, func() bool {
158162
ws := coderdtest.MustWorkspace(t, client, workspace.ID)
159-
return ws.LatestBuild.ID != workspace.LatestBuild.ID && ws.LatestBuild.Transition == database.WorkspaceTransitionStart
163+
return ws.LatestBuild.ID != workspace.LatestBuild.ID && ws.LatestBuild.Transition == database.WorkspaceTransitionStop
160164
}, 5*time.Second, 250*time.Millisecond)
161165
}
162166
func Test_Executor_Autostop_AlreadyStopped(t *testing.T) {
@@ -189,6 +193,7 @@ func Test_Executor_Autostop_AlreadyStopped(t *testing.T) {
189193
// When: the lifecycle executor ticks
190194
go func() {
191195
tickCh <- time.Now().UTC().Add(time.Minute)
196+
close(tickCh)
192197
}()
193198

194199
// Then: the workspace should not be stopped.
@@ -219,6 +224,7 @@ func Test_Executor_Autostop_NotEnabled(t *testing.T) {
219224
// When: the lifecycle executor ticks
220225
go func() {
221226
tickCh <- time.Now().UTC().Add(time.Minute)
227+
close(tickCh)
222228
}()
223229

224230
// Then: the workspace should not be stopped.

‎coderd/database/databasefake/databasefake.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,32 @@ func (q *fakeQuerier) GetWorkspacesAutostart(_ context.Context) ([]database.Work
345345
return workspaces, nil
346346
}
347347

348+
func (q *fakeQuerier) GetWorkspacesAutostop(_ context.Context) ([]database.Workspace, error) {
349+
q.mutex.RLock()
350+
defer q.mutex.RUnlock()
351+
workspaces := make([]database.Workspace, 0)
352+
for _, ws := range q.workspaces {
353+
if ws.AutostopSchedule.String != "" {
354+
workspaces = append(workspaces, ws)
355+
}
356+
}
357+
return workspaces, nil
358+
}
359+
360+
func (q *fakeQuerier) GetWorkspacesAutostartAutostop(_ context.Context) ([]database.Workspace, error) {
361+
q.mutex.RLock()
362+
defer q.mutex.RUnlock()
363+
workspaces := make([]database.Workspace, 0)
364+
for _, ws := range q.workspaces {
365+
if ws.AutostartSchedule.String != "" {
366+
workspaces = append(workspaces, ws)
367+
} else if ws.AutostopSchedule.String != "" {
368+
workspaces = append(workspaces, ws)
369+
}
370+
}
371+
return workspaces, nil
372+
}
373+
348374
func (q *fakeQuerier) GetWorkspaceOwnerCountsByTemplateIDs(_ context.Context, templateIDs []uuid.UUID) ([]database.GetWorkspaceOwnerCountsByTemplateIDsRow, error) {
349375
q.mutex.RLock()
350376
defer q.mutex.RUnlock()

‎coderd/database/querier.go

Lines changed: 2 additions & 0 deletions
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: 94 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)