Skip to content

Commit 990d25f

Browse files
johnstcnjohnstcn
committed
stop accessing db directly, only query workspaces with autostart enabled
1 parent edf8c1e commit 990d25f

File tree

8 files changed

+125
-48
lines changed

8 files changed

+125
-48
lines changed

‎coderd/autostart/lifecycle/lifecycle_executor.go

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,16 +53,12 @@ func (e *Executor) Run() error {
5353
func (e *Executor) runOnce(t time.Time) error {
5454
currentTick := t.Round(time.Minute)
5555
return e.db.InTx(func(db database.Store) error {
56-
allWorkspaces, err := db.GetWorkspaces(e.ctx)
56+
autostartWorkspaces, err := db.GetWorkspacesAutostart(e.ctx)
5757
if err != nil {
5858
return xerrors.Errorf("get all workspaces: %w", err)
5959
}
6060

61-
for _, ws := range allWorkspaces {
62-
// We only care about workspaces with autostart enabled.
63-
if ws.AutostartSchedule.String == "" {
64-
continue
65-
}
61+
for _, ws := range autostartWorkspaces {
6662
sched, err := schedule.Weekly(ws.AutostartSchedule.String)
6763
if err != nil {
6864
e.log.Warn(e.ctx, "workspace has invalid autostart schedule",
@@ -73,7 +69,6 @@ func (e *Executor) runOnce(t time.Time) error {
7369
}
7470

7571
// Determine the workspace state based on its latest build. We expect it to be stopped.
76-
// TODO(cian): is this **guaranteed** to be the latest build???
7772
latestBuild, err := db.GetWorkspaceBuildByWorkspaceIDWithoutAfter(e.ctx, ws.ID)
7873
if err != nil {
7974
return xerrors.Errorf("get latest build for workspace %q: %w", ws.ID, err)

‎coderd/autostart/lifecycle/lifecycle_executor_test.go

Lines changed: 48 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,9 @@ import (
55
"testing"
66
"time"
77

8-
"cdr.dev/slog"
9-
"cdr.dev/slog/sloggers/slogtest"
10-
11-
"github.com/coder/coder/coderd/autostart/lifecycle"
128
"github.com/coder/coder/coderd/autostart/schedule"
139
"github.com/coder/coder/coderd/coderdtest"
1410
"github.com/coder/coder/coderd/database"
15-
"github.com/coder/coder/coderd/database/databasefake"
1611
"github.com/coder/coder/codersdk"
1712

1813
"github.com/stretchr/testify/require"
@@ -25,15 +20,11 @@ func Test_Executor_Run(t *testing.T) {
2520
t.Parallel()
2621

2722
var (
28-
ctx = context.Background()
29-
cancelCtx, cancel = context.WithCancel(context.Background())
30-
log = slogtest.Make(t, nil).Named("lifecycle.executor").Leveled(slog.LevelDebug)
31-
err error
32-
tickCh = make(chan time.Time)
33-
db = databasefake.New()
34-
le = lifecycle.NewExecutor(cancelCtx, db, log, tickCh)
35-
client = coderdtest.New(t, &coderdtest.Options{
36-
Ticker: tickCh,
23+
ctx = context.Background()
24+
err error
25+
tickCh = make(chan time.Time)
26+
client = coderdtest.New(t, &coderdtest.Options{
27+
LifecycleTicker: tickCh,
3728
})
3829
// Given: we have a user with a workspace
3930
_ = coderdtest.NewProvisionerDaemon(t, client)
@@ -71,31 +62,25 @@ func Test_Executor_Run(t *testing.T) {
7162
// When: the lifecycle executor ticks
7263
go func() {
7364
tickCh <- time.Now().UTC().Add(time.Minute)
74-
cancel()
7565
}()
76-
require.NoError(t, le.Run())
7766

7867
// Then: the workspace should be started
7968
require.Eventually(t, func() bool {
8069
ws := coderdtest.MustWorkspace(t, client, workspace.ID)
8170
return ws.LatestBuild.Job.Status == codersdk.ProvisionerJobSucceeded &&
8271
ws.LatestBuild.Transition == database.WorkspaceTransitionStart
83-
}, 10*time.Second, 1000*time.Millisecond)
72+
}, 5*time.Second, 250*time.Millisecond)
8473
})
8574

8675
t.Run("AlreadyRunning", func(t *testing.T) {
8776
t.Parallel()
8877

8978
var (
90-
ctx = context.Background()
91-
cancelCtx, cancel = context.WithCancel(context.Background())
92-
log = slogtest.Make(t, nil).Named("lifecycle.executor").Leveled(slog.LevelDebug)
93-
err error
94-
tickCh = make(chan time.Time)
95-
db = databasefake.New()
96-
le = lifecycle.NewExecutor(cancelCtx, db, log, tickCh)
97-
client = coderdtest.New(t, &coderdtest.Options{
98-
Ticker: tickCh,
79+
ctx = context.Background()
80+
err error
81+
tickCh = make(chan time.Time)
82+
client = coderdtest.New(t, &coderdtest.Options{
83+
LifecycleTicker: tickCh,
9984
})
10085
// Given: we have a user with a workspace
10186
_ = coderdtest.NewProvisionerDaemon(t, client)
@@ -123,14 +108,48 @@ func Test_Executor_Run(t *testing.T) {
123108
// When: the lifecycle executor ticks
124109
go func() {
125110
tickCh <- time.Now().UTC().Add(time.Minute)
126-
cancel()
127111
}()
128-
require.NoError(t, le.Run())
129112

130113
// Then: the workspace should not be started.
131114
require.Never(t, func() bool {
132115
ws := coderdtest.MustWorkspace(t, client, workspace.ID)
133116
return ws.LatestBuild.ID != workspace.LatestBuild.ID
134-
}, 10*time.Second, 1000*time.Millisecond)
117+
}, 5*time.Second, 250*time.Millisecond)
118+
})
119+
120+
t.Run("NotEnabled", func(t *testing.T) {
121+
t.Parallel()
122+
123+
var (
124+
tickCh = make(chan time.Time)
125+
client = coderdtest.New(t, &coderdtest.Options{
126+
LifecycleTicker: tickCh,
127+
})
128+
// Given: we have a user with a workspace
129+
_ = coderdtest.NewProvisionerDaemon(t, client)
130+
user = coderdtest.CreateFirstUser(t, client)
131+
version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
132+
template = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
133+
_ = coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
134+
workspace = coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
135+
_ = coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
136+
)
137+
138+
// Given: we ensure the workspace is now in a stopped state
139+
require.Equal(t, database.WorkspaceTransitionStart, workspace.LatestBuild.Transition)
140+
141+
// Given: the workspace has autostart disabled
142+
require.Empty(t, workspace.AutostartSchedule)
143+
144+
// When: the lifecycle executor ticks
145+
go func() {
146+
tickCh <- time.Now().UTC().Add(time.Minute)
147+
}()
148+
149+
// Then: the workspace should not be started.
150+
require.Never(t, func() bool {
151+
ws := coderdtest.MustWorkspace(t, client, workspace.ID)
152+
return ws.LatestBuild.ID != workspace.LatestBuild.ID
153+
}, 5*time.Second, 250*time.Millisecond)
135154
})
136155
}

‎coderd/coderdtest/coderdtest.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ type Options struct {
5959
GoogleTokenValidator *idtoken.Validator
6060
SSHKeygenAlgorithm gitsshkey.Algorithm
6161
APIRateLimit int
62-
Ticker <-chan time.Time
62+
LifecycleTicker <-chan time.Time
6363
}
6464

6565
// New constructs an in-memory coderd instance and returns
@@ -75,10 +75,10 @@ func New(t *testing.T, options *Options) *codersdk.Client {
7575
options.GoogleTokenValidator, err = idtoken.NewValidator(ctx, option.WithoutAuthentication())
7676
require.NoError(t, err)
7777
}
78-
if options.Ticker == nil {
79-
ticker := time.NewTicker(time.Second)
80-
options.Ticker = ticker.C
81-
t.Cleanup(ticker.Stop)
78+
if options.LifecycleTicker == nil {
79+
ticker := make(chan time.Time)
80+
options.LifecycleTicker = ticker
81+
t.Cleanup(func() { close(ticker) })
8282
}
8383

8484
// This can be hotswapped for a live database instance.
@@ -109,7 +109,7 @@ func New(t *testing.T, options *Options) *codersdk.Client {
109109
ctx,
110110
db,
111111
slogtest.Make(t, nil).Named("lifecycle.executor").Leveled(slog.LevelDebug),
112-
options.Ticker,
112+
options.LifecycleTicker,
113113
)
114114
go lifecycleExecutor.Run()
115115

‎coderd/database/databasefake/databasefake.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,18 @@ func (q *fakeQuerier) GetWorkspaces(_ context.Context) ([]database.Workspace, er
290290
return workspaces, nil
291291
}
292292

293+
func (q *fakeQuerier) GetWorkspacesAutostart(_ context.Context) ([]database.Workspace, error) {
294+
q.mutex.RLock()
295+
defer q.mutex.RUnlock()
296+
workspaces := make([]database.Workspace, 0)
297+
for _, ws := range q.workspaces {
298+
if ws.AutostartSchedule.String != "" {
299+
workspaces = append(workspaces, ws)
300+
}
301+
}
302+
return workspaces, nil
303+
}
304+
293305
func (q *fakeQuerier) GetWorkspaceOwnerCountsByTemplateIDs(_ context.Context, templateIDs []uuid.UUID) ([]database.GetWorkspaceOwnerCountsByTemplateIDsRow, error) {
294306
q.mutex.RLock()
295307
defer q.mutex.RUnlock()

‎coderd/database/models.go

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

‎coderd/database/querier.go

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

‎coderd/database/queries/workspaces.sql

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,17 @@ FROM
1919
WHERE
2020
deleted = false;
2121

22+
-- name: GetWorkspacesAutostart :many
23+
SELECT
24+
*
25+
FROM
26+
workspaces
27+
WHERE
28+
deleted = false
29+
AND
30+
autostart_schedule <> ''
31+
;
32+
2233
-- name: GetWorkspacesByTemplateID :many
2334
SELECT
2435
*

0 commit comments

Comments
 (0)