Skip to content

Commit 9a3f534

Browse files
cstyancstyan
committed
more refactor and clean up
Signed-off-by: Callum Styan <callumstyan@gmail.com>
1 parent 0bc4cce commit 9a3f534

File tree

4 files changed

+43
-38
lines changed

4 files changed

+43
-38
lines changed

‎coderd/autobuild/lifecycle_executor.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,9 @@ func (e *Executor) Run() {
132132
}()
133133
}
134134

135-
func (e *Executor) hasAvailableProvisioners(ctx context.Context, tx database.Store, t time.Time, ws database.Workspace, templateVersionJob database.ProvisionerJob) (bool, error) {
135+
// hasNonStaleProvisioner checks whether there is at least one valid (non-stale, correct tags) provisioner
136+
// based on t and the tags on templateVersionJob.
137+
func (e *Executor) hasValidProvisioner(ctx context.Context, tx database.Store, t time.Time, ws database.Workspace, templateVersionJob database.ProvisionerJob) (bool, error) {
136138
queryParams := database.GetProvisionerDaemonsByOrganizationParams{
137139
OrganizationID: ws.OrganizationID,
138140
WantTags: templateVersionJob.Tags,
@@ -319,7 +321,7 @@ func (e *Executor) runOnce(t time.Time) Stats {
319321
}
320322

321323
// Before creating the workspace build, check for available provisioners
322-
hasProvisioners, err := e.hasAvailableProvisioners(e.ctx, tx, t, ws, templateVersionJob)
324+
hasProvisioners, err := e.hasNonStaleProvisioner(e.ctx, tx, t, ws, templateVersionJob)
323325
if err != nil {
324326
return xerrors.Errorf("check provisioner availability: %w", err)
325327
}

‎coderd/autobuild/lifecycle_executor_test.go

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func TestExecutorAutostartOK(t *testing.T) {
5656
)
5757
// Given: workspace is stopped
5858
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)
59-
p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{})
59+
p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, map[string]string{})
6060
require.NoError(t, err)
6161
// When: the autobuild executor ticks after the scheduled time
6262
go func() {
@@ -118,7 +118,7 @@ func TestMultipleLifecycleExecutors(t *testing.T) {
118118
// Have the workspace stopped so we can perform an autostart
119119
workspace = coderdtest.MustTransitionWorkspace(t, clientA, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)
120120

121-
p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{})
121+
p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil)
122122
require.NoError(t, err)
123123
// Get both clients to perform a lifecycle execution tick
124124
next := sched.Next(workspace.LatestBuild.CreatedAt)
@@ -254,7 +254,7 @@ func TestExecutorAutostartTemplateUpdated(t *testing.T) {
254254
},
255255
))
256256

257-
p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{})
257+
p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil)
258258
require.NoError(t, err)
259259

260260
t.Log("sending autobuild tick")
@@ -440,7 +440,7 @@ func TestExecutorAutostopOK(t *testing.T) {
440440
require.Equal(t, codersdk.WorkspaceTransitionStart, workspace.LatestBuild.Transition)
441441
require.NotZero(t, workspace.LatestBuild.Deadline)
442442

443-
p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{})
443+
p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil)
444444
require.NoError(t, err)
445445

446446
// When: the autobuild executor ticks *after* the deadline:
@@ -489,7 +489,7 @@ func TestExecutorAutostopExtend(t *testing.T) {
489489
})
490490
require.NoError(t, err, "extend workspace deadline")
491491

492-
p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{})
492+
p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil)
493493
require.NoError(t, err)
494494

495495
// When: the autobuild executor ticks *after* the original deadline:
@@ -802,7 +802,7 @@ func TestExecutorAutostartMultipleOK(t *testing.T) {
802802
// Given: workspace is stopped
803803
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)
804804

805-
p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{})
805+
p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil)
806806
require.NoError(t, err)
807807

808808
// When: the autobuild executor ticks past the scheduled time
@@ -872,7 +872,7 @@ func TestExecutorAutostartWithParameters(t *testing.T) {
872872
// Given: workspace is stopped
873873
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)
874874

875-
p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{})
875+
p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil)
876876
require.NoError(t, err)
877877

878878
// When: the autobuild executor ticks after the scheduled time
@@ -971,7 +971,7 @@ func TestExecutorAutostopTemplateDisabled(t *testing.T) {
971971
// Then: the deadline should be set to the template default TTL
972972
assert.WithinDuration(t, workspace.LatestBuild.CreatedAt.Add(time.Hour), workspace.LatestBuild.Deadline.Time, time.Minute)
973973

974-
p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{})
974+
p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil)
975975
require.NoError(t, err)
976976

977977
// When: the autobuild executor ticks after the workspace setting, but before the template setting:
@@ -1059,7 +1059,7 @@ func TestExecutorRequireActiveVersion(t *testing.T) {
10591059
})
10601060
require.Equal(t, inactiveVersion.ID, ws.LatestBuild.TemplateVersionID)
10611061

1062-
p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{})
1062+
p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil)
10631063
require.NoError(t, err)
10641064

10651065
tickTime := sched.Next(ws.LatestBuild.CreatedAt)
@@ -1221,7 +1221,7 @@ func TestNotifications(t *testing.T) {
12211221
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)
12221222
_ = coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, workspace.LatestBuild.ID)
12231223

1224-
p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{})
1224+
p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil)
12251225
require.NoError(t, err)
12261226

12271227
// Wait for workspace to become dormant
@@ -1302,7 +1302,7 @@ func TestExecutorPrebuilds(t *testing.T) {
13021302
require.Equal(t, codersdk.WorkspaceTransitionStart, prebuild.LatestBuild.Transition)
13031303
require.NotZero(t, prebuild.LatestBuild.Deadline)
13041304

1305-
p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), prebuild.OrganizationID, database.ProvisionerJob{})
1305+
p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), prebuild.OrganizationID, nil)
13061306
require.NoError(t, err)
13071307

13081308
// When: the autobuild executor ticks *after* the deadline:
@@ -1676,8 +1676,9 @@ func TestExecutorAutostartSkipsWhenNoProvisionersAvailable(t *testing.T) {
16761676
statsCh = make(chan autobuild.Stats)
16771677
)
16781678

1679-
// Create client with provisioner closer
1680-
provisionerDaemonTags := map[string]string{"owner": "testowner", "scope": "organization"}
1679+
// Use provisioner daemon tags so we can test `hasAvailableProvisioner` more thoroughly.
1680+
// We can't overwrite owner or scope as there's a `provisionersdk.MutateTags` function that has restrictions on those.
1681+
provisionerDaemonTags := map[string]string{"test-tag": "asdf"}
16811682
t.Logf("Setting provisioner daemon tags: %v", provisionerDaemonTags)
16821683
client, db := coderdtest.NewWithDatabase(t, &coderdtest.Options{
16831684
AutobuildTicker: tickCh,
@@ -1687,20 +1688,21 @@ func TestExecutorAutostartSkipsWhenNoProvisionersAvailable(t *testing.T) {
16871688
})
16881689

16891690
// Create workspace with autostart enabled and matching provisioner tags
1690-
provisionerTags := map[string]string{"owner": "testowner", "scope": "organization"}
1691-
workspace := mustProvisionWorkspaceWithProvisionerTags(t, client, provisionerTags, func(cwr *codersdk.CreateWorkspaceRequest) {
1691+
workspace := mustProvisionWorkspaceWithProvisionerTags(t, client, provisionerDaemonTags, func(cwr *codersdk.CreateWorkspaceRequest) {
16921692
cwr.AutostartSchedule = ptr.Ref(sched.String())
16931693
})
16941694

16951695
// Stop the workspace while provisioner is available
16961696
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)
16971697

16981698
// Wait for provisioner to be available for this specific workspace
1699-
id := coderdtest.MustWaitForProvisionersAvailable(t, db, workspace, provisionerdserver.StaleInterval)
1699+
coderdtest.MustWaitForProvisionersAvailable(t, db, workspace, provisionerdserver.StaleInterval)
1700+
p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, provisionerDaemonTags)
1701+
require.NoError(t, err, "Error getting provisioner for workspace")
17001702

17011703
// Ensure the provisioner is stale
17021704
staleTime := sched.Next(workspace.LatestBuild.CreatedAt).Add((-1 * provisionerdserver.StaleInterval) + -10*time.Second)
1703-
coderdtest.UpdateProvisionerLastSeenAt(t, db, id, time.Now(), staleTime)
1705+
coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), staleTime)
17041706

17051707
// Trigger autobuild
17061708
tickCh <- sched.Next(workspace.LatestBuild.CreatedAt)
@@ -1713,7 +1715,7 @@ func TestExecutorAutostartSkipsWhenNoProvisionersAvailable(t *testing.T) {
17131715

17141716
// Ensure the provisioner is NOT stale, and see if we get a successful state transition.
17151717
notStaleTime := sched.Next(workspace.LatestBuild.CreatedAt).Add((-1 * provisionerdserver.StaleInterval) + 10*time.Second)
1716-
coderdtest.UpdateProvisionerLastSeenAt(t, db, id, time.Now(), notStaleTime)
1718+
coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), notStaleTime)
17171719

17181720
// Trigger autobuild
17191721
go func() {

‎coderd/coderdtest/coderdtest.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1593,11 +1593,14 @@ func DeploymentValues(t testing.TB, mut ...func(*codersdk.DeploymentValues)) *co
15931593
return cfg
15941594
}
15951595

1596-
// GetProvisionerForWorkspace returns the first valid provisioner for a workspace + template tags.
1597-
func GetProvisionerForWorkspace(t *testing.T, tx database.Store, curTime time.Time, orgID uuid.UUID, templateVersionJob database.ProvisionerJob) (database.ProvisionerDaemon, error) {
1596+
// GetProvisionerForTags returns the first valid provisioner for a workspace + template tags.
1597+
func GetProvisionerForTags(t *testing.T, tx database.Store, curTime time.Time, orgID uuid.UUID, tags map[string]string) (database.ProvisionerDaemon, error) {
1598+
if tags == nil {
1599+
tags = map[string]string{}
1600+
}
15981601
queryParams := database.GetProvisionerDaemonsByOrganizationParams{
15991602
OrganizationID: orgID,
1600-
WantTags: templateVersionJob.Tags,
1603+
WantTags: tags,
16011604
}
16021605

16031606
// nolint: gocritic // The user (in this case, the user/context for autostart builds) may not have the full
@@ -1613,13 +1616,11 @@ func GetProvisionerForWorkspace(t *testing.T, tx database.Store, curTime time.Ti
16131616
if pd.LastSeenAt.Valid {
16141617
age := curTime.Sub(pd.LastSeenAt.Time)
16151618
if age <= provisionerdserver.StaleInterval {
1616-
t.Logf("hasAvailableProvisioners found active provisioner, daemon_id: %+v", pd.ID)
16171619
return pd, nil
16181620
}
16191621
}
16201622
}
1621-
t.Logf("hasAvailableProvisioners: no active provisioners found")
1622-
return database.ProvisionerDaemon{}, nil
1623+
return database.ProvisionerDaemon{}, xerrors.New("no available provisioners found")
16231624
}
16241625

16251626
func ctxWithProvisionerPermissions(ctx context.Context) context.Context {

‎enterprise/coderd/workspaces_test.go

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -646,7 +646,7 @@ func TestWorkspaceAutobuild(t *testing.T) {
646646
require.Equal(t, codersdk.WorkspaceStatusFailed, build.Status)
647647
tickTime := build.Job.CompletedAt.Add(failureTTL * 2)
648648

649-
p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{})
649+
p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil)
650650
require.NoError(t, err)
651651
coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime)
652652
ticker <- tickTime
@@ -698,7 +698,7 @@ func TestWorkspaceAutobuild(t *testing.T) {
698698
// Make it impossible to trigger the failure TTL.
699699
tickTime := build.Job.CompletedAt.Add(-failureTTL * 2)
700700

701-
p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{})
701+
p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil)
702702
require.NoError(t, err)
703703
coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime)
704704
ticker <- tickTime
@@ -803,7 +803,7 @@ func TestWorkspaceAutobuild(t *testing.T) {
803803
// Simulate being inactive.
804804
tickTime := workspace.LastUsedAt.Add(inactiveTTL * 2)
805805

806-
p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{})
806+
p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil)
807807
require.NoError(t, err)
808808
coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime)
809809
ticker <- tickTime
@@ -906,7 +906,7 @@ func TestWorkspaceAutobuild(t *testing.T) {
906906
// Simulate being inactive.
907907
// Fix provisioner stale issue by updating LastSeenAt to the tick time
908908
tickTime := time.Now().Add(time.Hour)
909-
p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspaces[0].OrganizationID, database.ProvisionerJob{})
909+
p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspaces[0].OrganizationID, nil)
910910
require.NoError(t, err)
911911
coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime)
912912
ticker <- tickTime
@@ -1051,7 +1051,7 @@ func TestWorkspaceAutobuild(t *testing.T) {
10511051

10521052
// Simulate not having accessed the workspace in a while.
10531053
tickTime := ws.LastUsedAt.Add(2 * inactiveTTL)
1054-
p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{})
1054+
p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil)
10551055
require.NoError(t, err)
10561056
coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime)
10571057
ticker <- tickTime
@@ -1105,7 +1105,7 @@ func TestWorkspaceAutobuild(t *testing.T) {
11051105

11061106
// Simulate not having accessed the workspace in a while.
11071107
tickTime := ws.LastUsedAt.Add(2 * transitionTTL)
1108-
p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{})
1108+
p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil)
11091109
require.NoError(t, err)
11101110
coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime)
11111111
ticker <- tickTime
@@ -1190,7 +1190,7 @@ func TestWorkspaceAutobuild(t *testing.T) {
11901190

11911191
// Ensure we haven't breached our threshold.
11921192
tickTime := ws.DormantAt.Add(-dormantTTL * 2)
1193-
p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{})
1193+
p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil)
11941194
require.NoError(t, err)
11951195
coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime)
11961196
ticker <- tickTime
@@ -1255,7 +1255,7 @@ func TestWorkspaceAutobuild(t *testing.T) {
12551255

12561256
// Assert that autostart works when the workspace isn't dormant..
12571257
tickTime := sched.Next(ws.LatestBuild.CreatedAt)
1258-
p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{})
1258+
p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil)
12591259
require.NoError(t, err)
12601260
coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime)
12611261
tickCh <- tickTime
@@ -1379,7 +1379,7 @@ func TestWorkspaceAutobuild(t *testing.T) {
13791379
// Under normal circumstances this should result in a transition but
13801380
// since our last build resulted in failure it should be skipped.
13811381
tickTime := build.Job.CompletedAt.Add(time.Hour)
1382-
p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{})
1382+
p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil)
13831383
require.NoError(t, err)
13841384
coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime)
13851385
ticker <- tickTime
@@ -1451,7 +1451,7 @@ func TestWorkspaceAutobuild(t *testing.T) {
14511451

14521452
// Kick of an autostart build.
14531453
tickTime := sched.Next(ws.LatestBuild.CreatedAt)
1454-
p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{})
1454+
p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil)
14551455
require.NoError(t, err)
14561456
coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime)
14571457
tickCh <- tickTime
@@ -1549,7 +1549,7 @@ func TestWorkspaceAutobuild(t *testing.T) {
15491549
next = sched.Next(next)
15501550

15511551
clock.Set(next)
1552-
p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{})
1552+
p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil)
15531553
require.NoError(t, err)
15541554
coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), next)
15551555
tickCh <- next
@@ -2272,7 +2272,7 @@ func TestExecutorPrebuilds(t *testing.T) {
22722272
coderdtest.MustWaitForProvisionersAvailable(t, db, prebuild, provisionerdserver.StaleInterval)
22732273

22742274
tickTime := sched.Next(prebuild.LatestBuild.CreatedAt).Add(time.Minute)
2275-
p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{})
2275+
p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil)
22762276
require.NoError(t, err)
22772277
coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime)
22782278

0 commit comments

Comments
 (0)