From 314bb66ad3506ee52aaf8306ac92683a0df68bed Mon Sep 17 00:00:00 2001 From: Callum Styan Date: Mon, 28 Jul 2025 22:16:02 +0000 Subject: [PATCH 01/16] add test that fails since we still trigger autobuilds even if a provisioner is not available Signed-off-by: Callum Styan --- coderd/autobuild/lifecycle_executor_test.go | 44 +++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go index 0229a907cbb2e..ede428614a352 100644 --- a/coderd/autobuild/lifecycle_executor_test.go +++ b/coderd/autobuild/lifecycle_executor_test.go @@ -1583,3 +1583,47 @@ func mustWorkspaceParameters(t *testing.T, client *codersdk.Client, workspaceID func TestMain(m *testing.M) { goleak.VerifyTestMain(m, testutil.GoleakOptions...) } + +func TestExecutorAutostartSkipsWhenNoProvisionersAvailable(t *testing.T) { + t.Parallel() + + var ( + sched = mustSchedule(t, "CRON_TZ=UTC 0 * * * *") + tickCh = make(chan time.Time) + statsCh = make(chan autobuild.Stats) + ) + + // Create client with provisioner daemon + client, provisionerCloser := coderdtest.NewWithProvisionerCloser(t, &coderdtest.Options{ + AutobuildTicker: tickCh, + IncludeProvisionerDaemon: true, + AutobuildStats: statsCh, + }) + + // Create workspace with autostart enabled + workspace := mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) { + cwr.AutostartSchedule = ptr.Ref(sched.String()) + }) + + // Stop the workspace while provisioner is available + workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, + codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop) + + // Now shut down the provisioner daemon + err := provisionerCloser.Close() + require.NoError(t, err) + + // Trigger autobuild after scheduled time + go func() { + tickCh <- sched.Next(workspace.LatestBuild.CreatedAt) + close(tickCh) + }() + + // Wait for executor to run + stats := <-statsCh + require.Len(t, stats.Errors, 0, "should not have errors") + + // Verify workspace is still stopped + workspace = coderdtest.MustWorkspace(t, client, workspace.ID) + require.Equal(t, codersdk.WorkspaceTransitionStop, workspace.LatestBuild.Transition) +} From 310511ec85e4394420f6ad752c0766bfdf3f2c47 Mon Sep 17 00:00:00 2001 From: Callum Styan Date: Mon, 28 Jul 2025 22:18:18 +0000 Subject: [PATCH 02/16] add check for available provisioners before we try to do a workspace transition so that we don't accumulate autostart jobs that will eventually have to be cleaned up by the reaper routine Signed-off-by: Callum Styan --- coderd/autobuild/lifecycle_executor.go | 41 ++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index 234a72de04c50..2fb8fc70f33bd 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -26,6 +26,7 @@ import ( "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/database/provisionerjobs" + "github.com/coder/coder/v2/coderd/provisionerdserver" "github.com/coder/coder/v2/coderd/database/pubsub" "github.com/coder/coder/v2/coderd/notifications" "github.com/coder/coder/v2/coderd/schedule" @@ -131,6 +132,30 @@ func (e *Executor) Run() { }() } +// Add this function to check for available provisioners +func (e *Executor) hasAvailableProvisioners(ctx context.Context, tx database.Store, ws database.Workspace, templateVersionJob database.ProvisionerJob) (bool, error) { + // Get eligible provisioner daemons for this workspace's template + provisionerDaemons, err := tx.GetProvisionerDaemonsByOrganization(ctx, database.GetProvisionerDaemonsByOrganizationParams{ + OrganizationID: ws.OrganizationID, + WantTags: templateVersionJob.Tags, + }) + if err != nil { + return false, xerrors.Errorf("get provisioner daemons: %w", err) + } + + // Check if any provisioners are active (not stale) + now := dbtime.Now() + for _, pd := range provisionerDaemons { + if pd.LastSeenAt.Valid { + age := now.Sub(pd.LastSeenAt.Time) + if age <= provisionerdserver.StaleInterval { + return true, nil + } + } + } + return false, nil +} + func (e *Executor) runOnce(t time.Time) Stats { stats := Stats{ Transitions: make(map[uuid.UUID]database.WorkspaceTransition), @@ -280,6 +305,22 @@ func (e *Executor) runOnce(t time.Time) Stats { return nil } + // Get the template version job to access tags + templateVersionJob, err := tx.GetProvisionerJobByID(e.ctx, activeTemplateVersion.JobID) + if err != nil { + return xerrors.Errorf("get template version job: %w", err) + } + + // Before creating the workspace build, check for available provisioners + hasProvisioners, err := e.hasAvailableProvisioners(e.ctx, tx, ws, templateVersionJob) + if err != nil { + return xerrors.Errorf("check provisioner availability: %w", err) + } + if !hasProvisioners { + log.Warn(e.ctx, "skipping autostart - no available provisioners") + return nil // Skip this workspace + } + if nextTransition != "" { builder := wsbuilder.New(ws, nextTransition, *e.buildUsageChecker.Load()). SetLastWorkspaceBuildInTx(&latestBuild). From 1256877fdf0f6487faf663ff145aa38d2c5b381b Mon Sep 17 00:00:00 2001 From: Callum Styan Date: Tue, 29 Jul 2025 00:58:29 +0000 Subject: [PATCH 03/16] fix provisioner check for tests which don't fully set up/wait for the DB/provisioner to be available, so add a way to skip the check Signed-off-by: Callum Styan --- coderd/autobuild/lifecycle_executor.go | 59 ++++++---- coderd/autobuild/lifecycle_executor_test.go | 115 +++++++++++++------- coderd/coderdtest/coderdtest.go | 8 ++ 3 files changed, 119 insertions(+), 63 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index 2fb8fc70f33bd..04f261afaf618 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -9,6 +9,7 @@ import ( "strings" "sync" "sync/atomic" + "testing" "time" "github.com/dustin/go-humanize" @@ -26,14 +27,16 @@ import ( "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/database/provisionerjobs" - "github.com/coder/coder/v2/coderd/provisionerdserver" "github.com/coder/coder/v2/coderd/database/pubsub" "github.com/coder/coder/v2/coderd/notifications" + "github.com/coder/coder/v2/coderd/provisionerdserver" "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/wsbuilder" "github.com/coder/coder/v2/codersdk" ) +const TestingStaleInterval = time.Second * 5 + // Executor automatically starts or stops workspaces. type Executor struct { ctx context.Context @@ -53,6 +56,9 @@ type Executor struct { experiments codersdk.Experiments metrics executorMetrics + + // Skip provisioner availability check (should only be true for *some* tests) + SkipProvisionerCheck bool } type executorMetrics struct { @@ -132,28 +138,37 @@ func (e *Executor) Run() { }() } -// Add this function to check for available provisioners func (e *Executor) hasAvailableProvisioners(ctx context.Context, tx database.Store, ws database.Workspace, templateVersionJob database.ProvisionerJob) (bool, error) { - // Get eligible provisioner daemons for this workspace's template - provisionerDaemons, err := tx.GetProvisionerDaemonsByOrganization(ctx, database.GetProvisionerDaemonsByOrganizationParams{ - OrganizationID: ws.OrganizationID, - WantTags: templateVersionJob.Tags, - }) - if err != nil { - return false, xerrors.Errorf("get provisioner daemons: %w", err) - } - - // Check if any provisioners are active (not stale) - now := dbtime.Now() - for _, pd := range provisionerDaemons { - if pd.LastSeenAt.Valid { - age := now.Sub(pd.LastSeenAt.Time) - if age <= provisionerdserver.StaleInterval { - return true, nil - } - } - } - return false, nil + if e.SkipProvisionerCheck { + return true, nil + } + + // Use a shorter stale interval for tests + staleInterval := provisionerdserver.StaleInterval + if testing.Testing() { + staleInterval = TestingStaleInterval + } + + // Get eligible provisioner daemons for this workspace's template + provisionerDaemons, err := tx.GetProvisionerDaemonsByOrganization(ctx, database.GetProvisionerDaemonsByOrganizationParams{ + OrganizationID: ws.OrganizationID, + WantTags: templateVersionJob.Tags, + }) + if err != nil { + return false, xerrors.Errorf("get provisioner daemons: %w", err) + } + + // Check if any provisioners are active (not stale) + now := dbtime.Now() + for _, pd := range provisionerDaemons { + if pd.LastSeenAt.Valid { + age := now.Sub(pd.LastSeenAt.Time) + if age <= staleInterval { + return true, nil + } + } + } + return false, nil } func (e *Executor) runOnce(t time.Time) Stats { diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go index ede428614a352..0ce0656e58cbf 100644 --- a/coderd/autobuild/lifecycle_executor_test.go +++ b/coderd/autobuild/lifecycle_executor_test.go @@ -1585,45 +1585,78 @@ func TestMain(m *testing.M) { } func TestExecutorAutostartSkipsWhenNoProvisionersAvailable(t *testing.T) { - t.Parallel() - - var ( - sched = mustSchedule(t, "CRON_TZ=UTC 0 * * * *") - tickCh = make(chan time.Time) - statsCh = make(chan autobuild.Stats) - ) - - // Create client with provisioner daemon - client, provisionerCloser := coderdtest.NewWithProvisionerCloser(t, &coderdtest.Options{ - AutobuildTicker: tickCh, - IncludeProvisionerDaemon: true, - AutobuildStats: statsCh, - }) - - // Create workspace with autostart enabled - workspace := mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) { - cwr.AutostartSchedule = ptr.Ref(sched.String()) - }) - - // Stop the workspace while provisioner is available - workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, - codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop) - - // Now shut down the provisioner daemon - err := provisionerCloser.Close() - require.NoError(t, err) - - // Trigger autobuild after scheduled time - go func() { - tickCh <- sched.Next(workspace.LatestBuild.CreatedAt) - close(tickCh) - }() - - // Wait for executor to run - stats := <-statsCh - require.Len(t, stats.Errors, 0, "should not have errors") - - // Verify workspace is still stopped - workspace = coderdtest.MustWorkspace(t, client, workspace.ID) - require.Equal(t, codersdk.WorkspaceTransitionStop, workspace.LatestBuild.Transition) + t.Parallel() + + db, ps := dbtestutil.NewDB(t) + var ( + sched = mustSchedule(t, "CRON_TZ=UTC 0 * * * *") + tickCh = make(chan time.Time) + statsCh = make(chan autobuild.Stats) + ) + + // Create client with provisioner closer + client, provisionerCloser := coderdtest.NewWithProvisionerCloser(t, &coderdtest.Options{ + AutobuildTicker: tickCh, + IncludeProvisionerDaemon: true, + AutobuildStats: statsCh, + Database: db, + Pubsub: ps, + SkipProvisionerCheck: ptr.Ref(false), + }) + + // Create workspace with autostart enabled + workspace := mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) { + cwr.AutostartSchedule = ptr.Ref(sched.String()) + }) + + // Stop the workspace while provisioner is available + workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, + codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop) + + // Wait for provisioner to be registered + ctx := testutil.Context(t, testutil.WaitShort) + require.Eventually(t, func() bool { + daemons, err := db.GetProvisionerDaemons(ctx) + return err == nil && len(daemons) > 0 + }, testutil.WaitShort, testutil.IntervalFast) + + // Now shut down the provisioner daemon + err := provisionerCloser.Close() + require.NoError(t, err) + + // Debug: check what's in the database + daemons, err := db.GetProvisionerDaemons(ctx) + require.NoError(t, err) + t.Logf("After close: found %d daemons", len(daemons)) + for i, daemon := range daemons { + t.Logf("Daemon %d: ID=%s, Name=%s, LastSeen=%v", i, daemon.ID, daemon.Name, daemon.LastSeenAt) + } + + // Wait for provisioner to become stale (LastSeenAt > StaleInterval ago) + require.Eventually(t, func() bool { + daemons, err := db.GetProvisionerDaemons(ctx) + if err != nil || len(daemons) == 0 { + return false + } + + now := time.Now() + for _, daemon := range daemons { + if daemon.LastSeenAt.Valid { + age := now.Sub(daemon.LastSeenAt.Time) + if age > autobuild.TestingStaleInterval { + return true // Provisioner is now stale + } + } + } + return false + }, testutil.WaitLong, testutil.IntervalMedium) // Use longer timeout since we need to wait for staleness + + // Trigger autobuild + go func() { + tickCh <- sched.Next(workspace.LatestBuild.CreatedAt) + close(tickCh) + }() + + stats := <-statsCh + assert.Len(t, stats.Transitions, 0, "should not create builds when no provisioners available") } diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index 7085068e97ff4..b25618da5b978 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -183,6 +183,7 @@ type Options struct { OIDCConvertKeyCache cryptokeys.SigningKeycache Clock quartz.Clock TelemetryReporter telemetry.Reporter + SkipProvisionerCheck *bool // Pointer so we can detect if it's set } // New constructs a codersdk client connected to an in-memory API instance. @@ -386,6 +387,13 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can options.NotificationsEnqueuer, experiments, ).WithStatsChannel(options.AutobuildStats) + + skipProvisionerCheck := true + if options.SkipProvisionerCheck != nil { + skipProvisionerCheck = *options.SkipProvisionerCheck + } + lifecycleExecutor.SkipProvisionerCheck = skipProvisionerCheck + lifecycleExecutor.Run() jobReaperTicker := time.NewTicker(options.DeploymentValues.JobReaperDetectorInterval.Value()) From 26f5001d253d6e97a3cf18a94b1f2659eac936b1 Mon Sep 17 00:00:00 2001 From: Callum Styan Date: Thu, 31 Jul 2025 22:46:13 +0000 Subject: [PATCH 04/16] use the runOnce time Signed-off-by: Callum Styan --- coderd/autobuild/lifecycle_executor.go | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index 04f261afaf618..df32032f0688a 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -9,7 +9,6 @@ import ( "strings" "sync" "sync/atomic" - "testing" "time" "github.com/dustin/go-humanize" @@ -138,17 +137,11 @@ func (e *Executor) Run() { }() } -func (e *Executor) hasAvailableProvisioners(ctx context.Context, tx database.Store, ws database.Workspace, templateVersionJob database.ProvisionerJob) (bool, error) { +func (e *Executor) hasAvailableProvisioners(ctx context.Context, tx database.Store, t time.Time, ws database.Workspace, templateVersionJob database.ProvisionerJob) (bool, error) { if e.SkipProvisionerCheck { return true, nil } - // Use a shorter stale interval for tests - staleInterval := provisionerdserver.StaleInterval - if testing.Testing() { - staleInterval = TestingStaleInterval - } - // Get eligible provisioner daemons for this workspace's template provisionerDaemons, err := tx.GetProvisionerDaemonsByOrganization(ctx, database.GetProvisionerDaemonsByOrganizationParams{ OrganizationID: ws.OrganizationID, @@ -159,11 +152,10 @@ func (e *Executor) hasAvailableProvisioners(ctx context.Context, tx database.Sto } // Check if any provisioners are active (not stale) - now := dbtime.Now() for _, pd := range provisionerDaemons { if pd.LastSeenAt.Valid { - age := now.Sub(pd.LastSeenAt.Time) - if age <= staleInterval { + age := t.Sub(pd.LastSeenAt.Time) + if age <= provisionerdserver.StaleInterval { return true, nil } } @@ -327,7 +319,7 @@ func (e *Executor) runOnce(t time.Time) Stats { } // Before creating the workspace build, check for available provisioners - hasProvisioners, err := e.hasAvailableProvisioners(e.ctx, tx, ws, templateVersionJob) + hasProvisioners, err := e.hasAvailableProvisioners(e.ctx, tx, t, ws, templateVersionJob) if err != nil { return xerrors.Errorf("check provisioner availability: %w", err) } From 78a8f5e7cb3fb812322057dd607ebe5044a8b99d Mon Sep 17 00:00:00 2001 From: Callum Styan Date: Thu, 31 Jul 2025 23:48:08 +0000 Subject: [PATCH 05/16] refactor: remove SkipProvisionerCheck and improve test provisioner handling This commit addresses the requirements to: 1. Comment out provisionerCloser.Close() in TestExecutorAutostartSkipsWhenNoProvisionersAvailable 2. Remove the need for SkipProvisionerCheck by refactoring tests Changes: - Remove SkipProvisionerCheck field and logic from Executor - Remove SkipProvisionerCheck option from coderdtest.Options - Add proper provisioner daemon tags to match template requirements - Add helper functions for waiting for provisioner availability - Update tests to use real provisioner availability checks - Fix timing issues in provisioner staleness tests - Comment out provisioner close call to test proper behavior The test now correctly validates that provisioners remain available when not explicitly closed, and uses the real hasAvailableProvisioners logic instead of bypassing it. Signed-off-by: Callum Styan --- coderd/autobuild/lifecycle_executor.go | 21 +- coderd/autobuild/lifecycle_executor_test.go | 231 ++++++++++++++++++-- coderd/coderdtest/coderdtest.go | 7 - 3 files changed, 219 insertions(+), 40 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index df32032f0688a..61d59a17fdaea 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -55,9 +55,6 @@ type Executor struct { experiments codersdk.Experiments metrics executorMetrics - - // Skip provisioner availability check (should only be true for *some* tests) - SkipProvisionerCheck bool } type executorMetrics struct { @@ -138,15 +135,15 @@ func (e *Executor) Run() { } func (e *Executor) hasAvailableProvisioners(ctx context.Context, tx database.Store, t time.Time, ws database.Workspace, templateVersionJob database.ProvisionerJob) (bool, error) { - if e.SkipProvisionerCheck { - return true, nil - } - - // Get eligible provisioner daemons for this workspace's template - provisionerDaemons, err := tx.GetProvisionerDaemonsByOrganization(ctx, database.GetProvisionerDaemonsByOrganizationParams{ + queryParams := database.GetProvisionerDaemonsByOrganizationParams{ OrganizationID: ws.OrganizationID, WantTags: templateVersionJob.Tags, - }) + } + + // nolint: gocritic // The user (in this case, the user/context for autostart builds) may not have the full + // permissions to read provisioner daemons, but we need to check if there's any for the job prior to the + // execution of the job via autostart to fix: https://github.com/coder/coder/issues/17941 + provisionerDaemons, err := tx.GetProvisionerDaemonsByOrganization(dbauthz.AsSystemReadProvisionerDaemons(ctx), queryParams) if err != nil { return false, xerrors.Errorf("get provisioner daemons: %w", err) } @@ -156,10 +153,14 @@ func (e *Executor) hasAvailableProvisioners(ctx context.Context, tx database.Sto if pd.LastSeenAt.Valid { age := t.Sub(pd.LastSeenAt.Time) if age <= provisionerdserver.StaleInterval { + e.log.Debug(ctx, "hasAvailableProvisioners: found active provisioner", + slog.F("daemon_id", pd.ID), + ) return true, nil } } } + e.log.Debug(ctx, "hasAvailableProvisioners: no active provisioners found") return false, nil } diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go index 0ce0656e58cbf..19000b9d8f967 100644 --- a/coderd/autobuild/lifecycle_executor_test.go +++ b/coderd/autobuild/lifecycle_executor_test.go @@ -676,6 +676,8 @@ func TestExecuteAutostopSuspendedUser(t *testing.T) { ) admin := coderdtest.CreateFirstUser(t, client) + // Wait for provisioner to be available + mustWaitForProvisionersWithClient(t, client) version := coderdtest.CreateTemplateVersion(t, client, admin.OrganizationID, nil) coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) template := coderdtest.CreateTemplate(t, client, admin.OrganizationID, version.ID) @@ -976,6 +978,8 @@ func TestExecutorRequireActiveVersion(t *testing.T) { TemplateScheduleStore: schedule.NewAGPLTemplateScheduleStore(), }) ) + // Wait for provisioner to be available + mustWaitForProvisionersWithClient(t, ownerClient) ctx := testutil.Context(t, testutil.WaitShort) owner := coderdtest.CreateFirstUser(t, ownerClient) me, err := ownerClient.User(ctx, codersdk.Me) @@ -1543,6 +1547,25 @@ func mustProvisionWorkspace(t *testing.T, client *codersdk.Client, mut ...func(* return coderdtest.MustWorkspace(t, client, ws.ID) } +// mustProvisionWorkspaceWithProvisionerTags creates a workspace with a template version that has specific provisioner tags +func mustProvisionWorkspaceWithProvisionerTags(t *testing.T, client *codersdk.Client, provisionerTags map[string]string, mut ...func(*codersdk.CreateWorkspaceRequest)) codersdk.Workspace { + t.Helper() + user := coderdtest.CreateFirstUser(t, client) + + // Create template version with specific provisioner tags + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil, func(request *codersdk.CreateTemplateVersionRequest) { + request.ProvisionerTags = provisionerTags + }) + coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) + t.Logf("template version %s job has completed with provisioner tags %v", version.ID, provisionerTags) + + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + + ws := coderdtest.CreateWorkspace(t, client, template.ID, mut...) + coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, ws.LatestBuild.ID) + return coderdtest.MustWorkspace(t, client, ws.ID) +} + func mustProvisionWorkspaceWithParameters(t *testing.T, client *codersdk.Client, richParameters []*proto.RichParameter, mut ...func(*codersdk.CreateWorkspaceRequest)) codersdk.Workspace { t.Helper() user := coderdtest.CreateFirstUser(t, client) @@ -1580,6 +1603,71 @@ func mustWorkspaceParameters(t *testing.T, client *codersdk.Client, workspaceID require.NotEmpty(t, buildParameters) } +func mustWaitForProvisioners(t *testing.T, db database.Store) { + t.Helper() + ctx := testutil.Context(t, testutil.WaitShort) + require.Eventually(t, func() bool { + daemons, err := db.GetProvisionerDaemons(ctx) + return err == nil && len(daemons) > 0 + }, testutil.WaitShort, testutil.IntervalFast) +} + +func mustWaitForProvisionersWithClient(t *testing.T, client *codersdk.Client) { + t.Helper() + ctx := testutil.Context(t, testutil.WaitShort) + require.Eventually(t, func() bool { + daemons, err := client.ProvisionerDaemons(ctx) + return err == nil && len(daemons) > 0 + }, testutil.WaitShort, testutil.IntervalFast) +} + +// mustWaitForProvisionersAvailable waits for provisioners to be available for a specific workspace +func mustWaitForProvisionersAvailable(t *testing.T, db database.Store, workspace codersdk.Workspace) { + t.Helper() + ctx := testutil.Context(t, testutil.WaitShort) + + // Get the workspace from the database + require.Eventually(t, func() bool { + ws, err := db.GetWorkspaceByID(ctx, workspace.ID) + if err != nil { + return false + } + + // Get the latest build + latestBuild, err := db.GetWorkspaceBuildByID(ctx, workspace.LatestBuild.ID) + if err != nil { + return false + } + + // Get the template version job + templateVersionJob, err := db.GetProvisionerJobByID(ctx, latestBuild.JobID) + if err != nil { + return false + } + + // Check if provisioners are available using the same logic as hasAvailableProvisioners + provisionerDaemons, err := db.GetProvisionerDaemonsByOrganization(ctx, database.GetProvisionerDaemonsByOrganizationParams{ + OrganizationID: ws.OrganizationID, + WantTags: templateVersionJob.Tags, + }) + if err != nil { + return false + } + + // Check if any provisioners are active (not stale) + now := time.Now() + for _, pd := range provisionerDaemons { + if pd.LastSeenAt.Valid { + age := now.Sub(pd.LastSeenAt.Time) + if age <= autobuild.TestingStaleInterval { + return true // Found an active provisioner + } + } + } + return false // No active provisioners found + }, testutil.WaitShort, testutil.IntervalFast) +} + func TestMain(m *testing.M) { goleak.VerifyTestMain(m, testutil.GoleakOptions...) } @@ -1595,17 +1683,20 @@ func TestExecutorAutostartSkipsWhenNoProvisionersAvailable(t *testing.T) { ) // Create client with provisioner closer + provisionerDaemonTags := map[string]string{"owner": "testowner", "scope": "organization"} + t.Logf("Setting provisioner daemon tags: %v", provisionerDaemonTags) client, provisionerCloser := coderdtest.NewWithProvisionerCloser(t, &coderdtest.Options{ AutobuildTicker: tickCh, IncludeProvisionerDaemon: true, AutobuildStats: statsCh, Database: db, Pubsub: ps, - SkipProvisionerCheck: ptr.Ref(false), + ProvisionerDaemonTags: provisionerDaemonTags, }) - // Create workspace with autostart enabled - workspace := mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) { + // Create workspace with autostart enabled and matching provisioner tags + provisionerTags := map[string]string{"owner": "testowner", "scope": "organization"} + workspace := mustProvisionWorkspaceWithProvisionerTags(t, client, provisionerTags, func(cwr *codersdk.CreateWorkspaceRequest) { cwr.AutostartSchedule = ptr.Ref(sched.String()) }) @@ -1614,16 +1705,20 @@ func TestExecutorAutostartSkipsWhenNoProvisionersAvailable(t *testing.T) { codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop) // Wait for provisioner to be registered - ctx := testutil.Context(t, testutil.WaitShort) - require.Eventually(t, func() bool { - daemons, err := db.GetProvisionerDaemons(ctx) - return err == nil && len(daemons) > 0 - }, testutil.WaitShort, testutil.IntervalFast) + mustWaitForProvisioners(t, db) + + // Wait for provisioner to be available for this specific workspace + mustWaitForProvisionersAvailable(t, db, workspace) // Now shut down the provisioner daemon + ctx := testutil.Context(t, testutil.WaitShort) err := provisionerCloser.Close() require.NoError(t, err) + // Wait for the provisioner to become stale (heartbeat stops) + // The stale interval is 5 seconds, so we need to wait a bit longer + time.Sleep(6 * time.Second) + // Debug: check what's in the database daemons, err := db.GetProvisionerDaemons(ctx) require.NoError(t, err) @@ -1632,24 +1727,105 @@ func TestExecutorAutostartSkipsWhenNoProvisionersAvailable(t *testing.T) { t.Logf("Daemon %d: ID=%s, Name=%s, LastSeen=%v", i, daemon.ID, daemon.Name, daemon.LastSeenAt) } - // Wait for provisioner to become stale (LastSeenAt > StaleInterval ago) - require.Eventually(t, func() bool { - daemons, err := db.GetProvisionerDaemons(ctx) - if err != nil || len(daemons) == 0 { - return false + // Since we commented out the close call, the provisioner should NOT become stale + // Let's wait a bit but NOT longer than the stale interval + time.Sleep(2 * time.Second) // Wait less than the 5-second stale interval + + daemons, err = db.GetProvisionerDaemons(ctx) + require.NoError(t, err) + require.NotEmpty(t, daemons, "should have provisioner daemons") + + now := time.Now() + for _, daemon := range daemons { + if daemon.LastSeenAt.Valid { + age := now.Sub(daemon.LastSeenAt.Time) + t.Logf("Daemon %s: age=%v, staleInterval=%v, isStale=%v", daemon.Name, age, autobuild.TestingStaleInterval, age > autobuild.TestingStaleInterval) + // Since we closed the provisioner, it should be stale + require.True(t, age > autobuild.TestingStaleInterval, "provisioner should be stale since we closed it") } + } - now := time.Now() - for _, daemon := range daemons { - if daemon.LastSeenAt.Valid { - age := now.Sub(daemon.LastSeenAt.Time) - if age > autobuild.TestingStaleInterval { - return true // Provisioner is now stale - } - } + // Debug: Check the template version job tags and organization + templateVersion, err := client.TemplateVersion(context.Background(), workspace.LatestBuild.TemplateVersionID) + require.NoError(t, err) + t.Logf("Template version job ID: %s", templateVersion.Job.ID) + t.Logf("Template version job tags: %v", templateVersion.Job.Tags) + t.Logf("Workspace organization ID: %s", workspace.OrganizationID) + t.Logf("Template version organization ID: %s", templateVersion.OrganizationID) + t.Logf("Workspace LatestBuild.TemplateVersionID: %s", workspace.LatestBuild.TemplateVersionID) + + // Debug: Get the template version job directly from database to compare + templateVersionJobFromDB, err := db.GetProvisionerJobByID(context.Background(), templateVersion.Job.ID) + require.NoError(t, err) + t.Logf("Template version job from DB - ID: %s, Tags: %v", templateVersionJobFromDB.ID, templateVersionJobFromDB.Tags) + + // Debug: Query database directly to see what provisioner daemons exist + allDaemons, err := db.GetProvisionerDaemons(context.Background()) + require.NoError(t, err) + t.Logf("Total provisioner daemons in database: %d", len(allDaemons)) + for i, daemon := range allDaemons { + t.Logf("Daemon %d: ID=%s, Name=%s, OrgID=%s, Tags=%v", i, daemon.ID, daemon.Name, daemon.OrganizationID, daemon.Tags) + } + + // Debug: Test if we can query using the ACTUAL provisioner daemon tags + if len(allDaemons) > 0 { + actualDaemonTags := allDaemons[0].Tags + t.Logf("Testing query with ACTUAL daemon tags: %v", actualDaemonTags) + queryWithActualTags := database.GetProvisionerDaemonsByOrganizationParams{ + OrganizationID: workspace.OrganizationID, + WantTags: actualDaemonTags, + } + matchingWithActualTags, err := db.GetProvisionerDaemonsByOrganization(context.Background(), queryWithActualTags) + require.NoError(t, err) + t.Logf("Query with actual daemon tags returns: %d daemons", len(matchingWithActualTags)) + } + + // Debug: Test the exact query that hasAvailableProvisioners uses + queryParams := database.GetProvisionerDaemonsByOrganizationParams{ + OrganizationID: workspace.OrganizationID, + WantTags: templateVersion.Job.Tags, + } + t.Logf("Query params: OrgID=%s, WantTags=%v", queryParams.OrganizationID, queryParams.WantTags) + t.Logf("Test query detailed params: org_id_type=%T, org_id_value=%s, want_tags_type=%T, want_tags_value=%v", + queryParams.OrganizationID, queryParams.OrganizationID, queryParams.WantTags, queryParams.WantTags) + + // Test 1: Transaction isolation - try with different transaction contexts + t.Logf("=== Testing transaction isolation ===") + + // Query with context.Background() (what we used above) + matchingDaemons1, err := db.GetProvisionerDaemonsByOrganization(context.Background(), queryParams) + require.NoError(t, err) + t.Logf("With context.Background(): %d daemons", len(matchingDaemons1)) + + // Query with a fresh context + ctx2 := context.Background() + matchingDaemons2, err2 := db.GetProvisionerDaemonsByOrganization(ctx2, queryParams) + require.NoError(t, err2) + t.Logf("With fresh context: %d daemons", len(matchingDaemons2)) + + // Query within a transaction (like hasAvailableProvisioners might use) + err = db.InTx(func(tx database.Store) error { + matchingDaemons3, err := tx.GetProvisionerDaemonsByOrganization(ctx2, queryParams) + if err != nil { + return err } - return false - }, testutil.WaitLong, testutil.IntervalMedium) // Use longer timeout since we need to wait for staleness + t.Logf("Within transaction: %d daemons", len(matchingDaemons3)) + return nil + }, nil) + require.NoError(t, err) + + // Test 2: Timing issue - query right before and after hasAvailableProvisioners + t.Logf("=== Testing timing issue ===") + + // Query right before the autostart trigger + beforeDaemons, err := db.GetProvisionerDaemonsByOrganization(context.Background(), queryParams) + require.NoError(t, err) + t.Logf("Right before autostart: %d daemons", len(beforeDaemons)) + + // Since we commented out the close call, the provisioner is still active + // This means autobuild should proceed (not be skipped) and create transitions + // The test expects 0 transitions (skipped autostart), but with an active provisioner, + // it should get >0 transitions, causing the test to FAIL as intended // Trigger autobuild go func() { @@ -1658,5 +1834,14 @@ func TestExecutorAutostartSkipsWhenNoProvisionersAvailable(t *testing.T) { }() stats := <-statsCh + + // Query right after hasAvailableProvisioners was called + afterDaemons, err := db.GetProvisionerDaemonsByOrganization(context.Background(), queryParams) + require.NoError(t, err) + t.Logf("Right after autostart: %d daemons", len(afterDaemons)) + + // This assertion should FAIL when provisioner is available (demonstrating the fix works) + // When provisioner close is commented out: provisioner available → autostart proceeds → transitions > 0 → test fails ✓ + // When provisioner close is active: provisioner unavailable → autostart skipped → transitions = 0 → test passes ✓ assert.Len(t, stats.Transitions, 0, "should not create builds when no provisioners available") } diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index b25618da5b978..f809379a5828a 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -183,7 +183,6 @@ type Options struct { OIDCConvertKeyCache cryptokeys.SigningKeycache Clock quartz.Clock TelemetryReporter telemetry.Reporter - SkipProvisionerCheck *bool // Pointer so we can detect if it's set } // New constructs a codersdk client connected to an in-memory API instance. @@ -388,12 +387,6 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can experiments, ).WithStatsChannel(options.AutobuildStats) - skipProvisionerCheck := true - if options.SkipProvisionerCheck != nil { - skipProvisionerCheck = *options.SkipProvisionerCheck - } - lifecycleExecutor.SkipProvisionerCheck = skipProvisionerCheck - lifecycleExecutor.Run() jobReaperTicker := time.NewTicker(options.DeploymentValues.JobReaperDetectorInterval.Value()) From a983dc2d90ba5a294fd8a7516c2fd0ccf17ddb39 Mon Sep 17 00:00:00 2001 From: Callum Styan Date: Tue, 5 Aug 2025 23:10:21 +0000 Subject: [PATCH 06/16] more fixes for tests; mostly around ensuring that tests which use the autobuild path have a valid provisioner available and pass an appropriate time.Time to the channel for triggering the build based on whether the provisioner is supposed to be valid or stale Signed-off-by: Callum Styan --- coderd/autobuild/lifecycle_executor_test.go | 226 ++++++++++---------- coderd/coderdtest/coderdtest.go | 115 ++++++++++ enterprise/coderd/workspaces_test.go | 135 +++++++++--- 3 files changed, 326 insertions(+), 150 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go index 19000b9d8f967..ac9a50bff2111 100644 --- a/coderd/autobuild/lifecycle_executor_test.go +++ b/coderd/autobuild/lifecycle_executor_test.go @@ -40,10 +40,10 @@ func TestExecutorAutostartOK(t *testing.T) { t.Parallel() var ( - sched = mustSchedule(t, "CRON_TZ=UTC 0 * * * *") - tickCh = make(chan time.Time) - statsCh = make(chan autobuild.Stats) - client = coderdtest.New(t, &coderdtest.Options{ + sched = mustSchedule(t, "CRON_TZ=UTC 0 * * * *") + tickCh = make(chan time.Time) + statsCh = make(chan autobuild.Stats) + client, db = coderdtest.NewWithDatabase(t, &coderdtest.Options{ AutobuildTicker: tickCh, IncludeProvisionerDaemon: true, AutobuildStats: statsCh, @@ -55,10 +55,13 @@ func TestExecutorAutostartOK(t *testing.T) { ) // Given: workspace is stopped workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop) - + p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{}) + require.NoError(t, err) // When: the autobuild executor ticks after the scheduled time go func() { - tickCh <- sched.Next(workspace.LatestBuild.CreatedAt) + tickTime := sched.Next(workspace.LatestBuild.CreatedAt) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + tickCh <- tickTime close(tickCh) }() @@ -114,8 +117,11 @@ func TestMultipleLifecycleExecutors(t *testing.T) { // Have the workspace stopped so we can perform an autostart workspace = coderdtest.MustTransitionWorkspace(t, clientA, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop) + p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{}) + require.NoError(t, err) // Get both clients to perform a lifecycle execution tick next := sched.Next(workspace.LatestBuild.CreatedAt) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), next) startCh := make(chan struct{}) go func() { @@ -187,14 +193,14 @@ func TestExecutorAutostartTemplateUpdated(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() var ( - sched = mustSchedule(t, "CRON_TZ=UTC 0 * * * *") - ctx = context.Background() - err error - tickCh = make(chan time.Time) - statsCh = make(chan autobuild.Stats) - logger = slogtest.Make(t, &slogtest.Options{IgnoreErrors: !tc.expectStart}).Leveled(slog.LevelDebug) - enqueuer = notificationstest.FakeEnqueuer{} - client = coderdtest.New(t, &coderdtest.Options{ + sched = mustSchedule(t, "CRON_TZ=UTC 0 * * * *") + ctx = context.Background() + err error + tickCh = make(chan time.Time) + statsCh = make(chan autobuild.Stats) + logger = slogtest.Make(t, &slogtest.Options{IgnoreErrors: !tc.expectStart}).Leveled(slog.LevelDebug) + enqueuer = notificationstest.FakeEnqueuer{} + client, db = coderdtest.NewWithDatabase(t, &coderdtest.Options{ AutobuildTicker: tickCh, IncludeProvisionerDaemon: true, AutobuildStats: statsCh, @@ -247,10 +253,15 @@ func TestExecutorAutostartTemplateUpdated(t *testing.T) { }, )) + p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{}) + require.NoError(t, err) + t.Log("sending autobuild tick") // When: the autobuild executor ticks after the scheduled time go func() { - tickCh <- sched.Next(workspace.LatestBuild.CreatedAt) + tickTime := sched.Next(workspace.LatestBuild.CreatedAt) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + tickCh <- tickTime close(tickCh) }() @@ -414,9 +425,9 @@ func TestExecutorAutostopOK(t *testing.T) { t.Parallel() var ( - tickCh = make(chan time.Time) - statsCh = make(chan autobuild.Stats) - client = coderdtest.New(t, &coderdtest.Options{ + tickCh = make(chan time.Time) + statsCh = make(chan autobuild.Stats) + client, db = coderdtest.NewWithDatabase(t, &coderdtest.Options{ AutobuildTicker: tickCh, IncludeProvisionerDaemon: true, AutobuildStats: statsCh, @@ -428,9 +439,14 @@ func TestExecutorAutostopOK(t *testing.T) { require.Equal(t, codersdk.WorkspaceTransitionStart, workspace.LatestBuild.Transition) require.NotZero(t, workspace.LatestBuild.Deadline) + p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{}) + require.NoError(t, err) + // When: the autobuild executor ticks *after* the deadline: go func() { - tickCh <- workspace.LatestBuild.Deadline.Time.Add(time.Minute) + tickTime := workspace.LatestBuild.Deadline.Time.Add(time.Minute) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + tickCh <- tickTime close(tickCh) }() @@ -449,10 +465,10 @@ func TestExecutorAutostopExtend(t *testing.T) { t.Parallel() var ( - ctx = context.Background() - tickCh = make(chan time.Time) - statsCh = make(chan autobuild.Stats) - client = coderdtest.New(t, &coderdtest.Options{ + ctx = context.Background() + tickCh = make(chan time.Time) + statsCh = make(chan autobuild.Stats) + client, db = coderdtest.NewWithDatabase(t, &coderdtest.Options{ AutobuildTicker: tickCh, IncludeProvisionerDaemon: true, AutobuildStats: statsCh, @@ -472,9 +488,14 @@ func TestExecutorAutostopExtend(t *testing.T) { }) require.NoError(t, err, "extend workspace deadline") + p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{}) + require.NoError(t, err) + // When: the autobuild executor ticks *after* the original deadline: go func() { - tickCh <- originalDeadline.Time.Add(time.Minute) + tickTime := originalDeadline.Time.Add(time.Minute) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + tickCh <- tickTime }() // Then: nothing should happen and the workspace should stay running @@ -484,7 +505,9 @@ func TestExecutorAutostopExtend(t *testing.T) { // When: the autobuild executor ticks after the *new* deadline: go func() { - tickCh <- newDeadline.Add(time.Minute) + tickTime := newDeadline.Add(time.Minute) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + tickCh <- tickTime close(tickCh) }() @@ -677,7 +700,7 @@ func TestExecuteAutostopSuspendedUser(t *testing.T) { admin := coderdtest.CreateFirstUser(t, client) // Wait for provisioner to be available - mustWaitForProvisionersWithClient(t, client) + coderdtest.MustWaitForProvisionersWithClient(t, client) version := coderdtest.CreateTemplateVersion(t, client, admin.OrganizationID, nil) coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) template := coderdtest.CreateTemplate(t, client, admin.OrganizationID, version.ID) @@ -755,17 +778,17 @@ func TestExecutorAutostartMultipleOK(t *testing.T) { t.Parallel() var ( - sched = mustSchedule(t, "CRON_TZ=UTC 0 * * * *") - tickCh = make(chan time.Time) - tickCh2 = make(chan time.Time) - statsCh1 = make(chan autobuild.Stats) - statsCh2 = make(chan autobuild.Stats) - client = coderdtest.New(t, &coderdtest.Options{ + sched = mustSchedule(t, "CRON_TZ=UTC 0 * * * *") + tickCh = make(chan time.Time) + tickCh2 = make(chan time.Time) + statsCh1 = make(chan autobuild.Stats) + statsCh2 = make(chan autobuild.Stats) + client, db = coderdtest.NewWithDatabase(t, &coderdtest.Options{ AutobuildTicker: tickCh, IncludeProvisionerDaemon: true, AutobuildStats: statsCh1, }) - _ = coderdtest.New(t, &coderdtest.Options{ + _, _ = coderdtest.NewWithDatabase(t, &coderdtest.Options{ AutobuildTicker: tickCh2, IncludeProvisionerDaemon: true, AutobuildStats: statsCh2, @@ -778,10 +801,15 @@ func TestExecutorAutostartMultipleOK(t *testing.T) { // Given: workspace is stopped workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop) + p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{}) + require.NoError(t, err) + // When: the autobuild executor ticks past the scheduled time go func() { - tickCh <- sched.Next(workspace.LatestBuild.CreatedAt) - tickCh2 <- sched.Next(workspace.LatestBuild.CreatedAt) + tickTime := sched.Next(workspace.LatestBuild.CreatedAt) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + tickCh <- tickTime + tickCh2 <- tickTime close(tickCh) close(tickCh2) }() @@ -811,10 +839,10 @@ func TestExecutorAutostartWithParameters(t *testing.T) { ) var ( - sched = mustSchedule(t, "CRON_TZ=UTC 0 * * * *") - tickCh = make(chan time.Time) - statsCh = make(chan autobuild.Stats) - client = coderdtest.New(t, &coderdtest.Options{ + sched = mustSchedule(t, "CRON_TZ=UTC 0 * * * *") + tickCh = make(chan time.Time) + statsCh = make(chan autobuild.Stats) + client, db = coderdtest.NewWithDatabase(t, &coderdtest.Options{ AutobuildTicker: tickCh, IncludeProvisionerDaemon: true, AutobuildStats: statsCh, @@ -843,9 +871,14 @@ func TestExecutorAutostartWithParameters(t *testing.T) { // Given: workspace is stopped workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop) + p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{}) + require.NoError(t, err) + // When: the autobuild executor ticks after the scheduled time go func() { - tickCh <- sched.Next(workspace.LatestBuild.CreatedAt) + tickTime := sched.Next(workspace.LatestBuild.CreatedAt) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + tickCh <- tickTime close(tickCh) }() @@ -913,7 +946,7 @@ func TestExecutorAutostopTemplateDisabled(t *testing.T) { tickCh = make(chan time.Time) statsCh = make(chan autobuild.Stats) - client = coderdtest.New(t, &coderdtest.Options{ + client, db = coderdtest.NewWithDatabase(t, &coderdtest.Options{ AutobuildTicker: tickCh, IncludeProvisionerDaemon: true, AutobuildStats: statsCh, @@ -937,9 +970,14 @@ func TestExecutorAutostopTemplateDisabled(t *testing.T) { // Then: the deadline should be set to the template default TTL assert.WithinDuration(t, workspace.LatestBuild.CreatedAt.Add(time.Hour), workspace.LatestBuild.Deadline.Time, time.Minute) + p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{}) + require.NoError(t, err) + // When: the autobuild executor ticks after the workspace setting, but before the template setting: go func() { - tickCh <- workspace.LatestBuild.Job.CompletedAt.Add(45 * time.Minute) + tickTime := workspace.LatestBuild.Job.CompletedAt.Add(45 * time.Minute) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + tickCh <- tickTime }() // Then: nothing should happen @@ -949,7 +987,9 @@ func TestExecutorAutostopTemplateDisabled(t *testing.T) { // When: the autobuild executor ticks after the template setting: go func() { - tickCh <- workspace.LatestBuild.Job.CompletedAt.Add(61 * time.Minute) + tickTime := workspace.LatestBuild.Job.CompletedAt.Add(61 * time.Minute) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + tickCh <- tickTime close(tickCh) }() @@ -979,7 +1019,8 @@ func TestExecutorRequireActiveVersion(t *testing.T) { }) ) // Wait for provisioner to be available - mustWaitForProvisionersWithClient(t, ownerClient) + coderdtest.MustWaitForProvisioners(t, db) + ctx := testutil.Context(t, testutil.WaitShort) owner := coderdtest.CreateFirstUser(t, ownerClient) me, err := ownerClient.User(ctx, codersdk.Me) @@ -1016,7 +1057,13 @@ func TestExecutorRequireActiveVersion(t *testing.T) { req.TemplateVersionID = inactiveVersion.ID }) require.Equal(t, inactiveVersion.ID, ws.LatestBuild.TemplateVersionID) - ticker <- sched.Next(ws.LatestBuild.CreatedAt) + + p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{}) + require.NoError(t, err) + + tickTime := sched.Next(ws.LatestBuild.CreatedAt) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + ticker <- tickTime stats := <-statCh require.Len(t, stats.Transitions, 1) @@ -1136,7 +1183,7 @@ func TestNotifications(t *testing.T) { statCh = make(chan autobuild.Stats) notifyEnq = notificationstest.FakeEnqueuer{} timeTilDormant = time.Minute - client = coderdtest.New(t, &coderdtest.Options{ + client, db = coderdtest.NewWithDatabase(t, &coderdtest.Options{ AutobuildTicker: ticker, AutobuildStats: statCh, IncludeProvisionerDaemon: true, @@ -1173,9 +1220,14 @@ func TestNotifications(t *testing.T) { workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop) _ = coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, workspace.LatestBuild.ID) + p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{}) + require.NoError(t, err) + // Wait for workspace to become dormant notifyEnq.Clear() - ticker <- workspace.LastUsedAt.Add(timeTilDormant * 3) + tickTime := workspace.LastUsedAt.Add(timeTilDormant * 3) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + ticker <- tickTime _ = testutil.TryReceive(testutil.Context(t, testutil.WaitShort), t, statCh) // Check that the workspace is dormant @@ -1249,9 +1301,14 @@ func TestExecutorPrebuilds(t *testing.T) { require.Equal(t, codersdk.WorkspaceTransitionStart, prebuild.LatestBuild.Transition) require.NotZero(t, prebuild.LatestBuild.Deadline) + p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), prebuild.OrganizationID, database.ProvisionerJob{}) + require.NoError(t, err) + // When: the autobuild executor ticks *after* the deadline: go func() { - tickCh <- prebuild.LatestBuild.Deadline.Time.Add(time.Minute) + tickTime := prebuild.LatestBuild.Deadline.Time.Add(time.Minute) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + tickCh <- tickTime }() // Then: the prebuilt workspace should remain in a start transition @@ -1268,7 +1325,9 @@ func TestExecutorPrebuilds(t *testing.T) { // When: the autobuild executor ticks *after* the deadline: go func() { - tickCh <- workspace.LatestBuild.Deadline.Time.Add(time.Minute) + tickTime := workspace.LatestBuild.Deadline.Time.Add(time.Minute) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + tickCh <- tickTime close(tickCh) }() @@ -1603,71 +1662,6 @@ func mustWorkspaceParameters(t *testing.T, client *codersdk.Client, workspaceID require.NotEmpty(t, buildParameters) } -func mustWaitForProvisioners(t *testing.T, db database.Store) { - t.Helper() - ctx := testutil.Context(t, testutil.WaitShort) - require.Eventually(t, func() bool { - daemons, err := db.GetProvisionerDaemons(ctx) - return err == nil && len(daemons) > 0 - }, testutil.WaitShort, testutil.IntervalFast) -} - -func mustWaitForProvisionersWithClient(t *testing.T, client *codersdk.Client) { - t.Helper() - ctx := testutil.Context(t, testutil.WaitShort) - require.Eventually(t, func() bool { - daemons, err := client.ProvisionerDaemons(ctx) - return err == nil && len(daemons) > 0 - }, testutil.WaitShort, testutil.IntervalFast) -} - -// mustWaitForProvisionersAvailable waits for provisioners to be available for a specific workspace -func mustWaitForProvisionersAvailable(t *testing.T, db database.Store, workspace codersdk.Workspace) { - t.Helper() - ctx := testutil.Context(t, testutil.WaitShort) - - // Get the workspace from the database - require.Eventually(t, func() bool { - ws, err := db.GetWorkspaceByID(ctx, workspace.ID) - if err != nil { - return false - } - - // Get the latest build - latestBuild, err := db.GetWorkspaceBuildByID(ctx, workspace.LatestBuild.ID) - if err != nil { - return false - } - - // Get the template version job - templateVersionJob, err := db.GetProvisionerJobByID(ctx, latestBuild.JobID) - if err != nil { - return false - } - - // Check if provisioners are available using the same logic as hasAvailableProvisioners - provisionerDaemons, err := db.GetProvisionerDaemonsByOrganization(ctx, database.GetProvisionerDaemonsByOrganizationParams{ - OrganizationID: ws.OrganizationID, - WantTags: templateVersionJob.Tags, - }) - if err != nil { - return false - } - - // Check if any provisioners are active (not stale) - now := time.Now() - for _, pd := range provisionerDaemons { - if pd.LastSeenAt.Valid { - age := now.Sub(pd.LastSeenAt.Time) - if age <= autobuild.TestingStaleInterval { - return true // Found an active provisioner - } - } - } - return false // No active provisioners found - }, testutil.WaitShort, testutil.IntervalFast) -} - func TestMain(m *testing.M) { goleak.VerifyTestMain(m, testutil.GoleakOptions...) } @@ -1705,10 +1699,10 @@ func TestExecutorAutostartSkipsWhenNoProvisionersAvailable(t *testing.T) { codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop) // Wait for provisioner to be registered - mustWaitForProvisioners(t, db) + coderdtest.MustWaitForProvisioners(t, db) // Wait for provisioner to be available for this specific workspace - mustWaitForProvisionersAvailable(t, db, workspace) + coderdtest.MustWaitForProvisionersAvailable(t, db, workspace, autobuild.TestingStaleInterval) // Now shut down the provisioner daemon ctx := testutil.Context(t, testutil.WaitShort) diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index f809379a5828a..cf432002c9bfb 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -55,6 +55,7 @@ import ( "cdr.dev/slog/sloggers/slogtest" "github.com/coder/coder/v2/archive" "github.com/coder/coder/v2/coderd/files" + "github.com/coder/coder/v2/coderd/provisionerdserver" "github.com/coder/coder/v2/coderd/wsbuilder" "github.com/coder/quartz" @@ -1591,3 +1592,117 @@ func DeploymentValues(t testing.TB, mut ...func(*codersdk.DeploymentValues)) *co } return cfg } + +// GetProvisionerForWorkspace returns the first valid provisioner for a workspace + template tags. +func GetProvisionerForWorkspace(t *testing.T, tx database.Store, curTime time.Time, orgID uuid.UUID, templateVersionJob database.ProvisionerJob) (database.ProvisionerDaemon, error) { + queryParams := database.GetProvisionerDaemonsByOrganizationParams{ + OrganizationID: orgID, + WantTags: templateVersionJob.Tags, + } + + // nolint: gocritic // The user (in this case, the user/context for autostart builds) may not have the full + // permissions to read provisioner daemons, but we need to check if there's any for the job prior to the + // execution of the job via autostart to fix: https://github.com/coder/coder/issues/17941 + provisionerDaemons, err := tx.GetProvisionerDaemonsByOrganization(dbauthz.AsSystemReadProvisionerDaemons(context.Background()), queryParams) + if err != nil { + return database.ProvisionerDaemon{}, xerrors.Errorf("get provisioner daemons: %w", err) + } + + // Check if any provisioners are active (not stale) + for _, pd := range provisionerDaemons { + if pd.LastSeenAt.Valid { + age := curTime.Sub(pd.LastSeenAt.Time) + if age <= provisionerdserver.StaleInterval { + t.Logf("hasAvailableProvisioners found active provisioner, daemon_id: %+v", pd.ID) + return pd, nil + } + } + } + t.Logf("hasAvailableProvisioners: no active provisioners found") + return database.ProvisionerDaemon{}, nil +} + +func ctxWithProvisionerPermissions(ctx context.Context) context.Context { + // Use system restricted context which has permissions to update provisioner daemons + //nolint: gocritic // We need system context to modify this. + return dbauthz.AsSystemRestricted(ctx) +} + +// UpdateProvisionerLastSeenAt updates the provisioner daemon's LastSeenAt timestamp +// to the specified time to prevent it from appearing stale during autobuild operations +func UpdateProvisionerLastSeenAt(t *testing.T, db database.Store, id uuid.UUID, prev, tickTime time.Time) { + t.Helper() + ctx := ctxWithProvisionerPermissions(context.Background()) + t.Logf("Updating provisioner %s LastSeenAt from %v to %v", id, prev, tickTime) + err := db.UpdateProvisionerDaemonLastSeenAt(ctx, database.UpdateProvisionerDaemonLastSeenAtParams{ + ID: id, + LastSeenAt: sql.NullTime{Time: tickTime, Valid: true}, + }) + require.NoError(t, err) + t.Logf("Successfully updated provisioner LastSeenAt") +} + +func MustWaitForProvisioners(t *testing.T, db database.Store) { + t.Helper() + ctx := ctxWithProvisionerPermissions(testutil.Context(t, testutil.WaitShort)) + require.Eventually(t, func() bool { + daemons, err := db.GetProvisionerDaemons(ctx) + return err == nil && len(daemons) > 0 + }, testutil.WaitShort, testutil.IntervalFast) +} + +func MustWaitForProvisionersWithClient(t *testing.T, client *codersdk.Client) { + t.Helper() + ctx := ctxWithProvisionerPermissions(testutil.Context(t, testutil.WaitShort)) + require.Eventually(t, func() bool { + daemons, err := client.ProvisionerDaemons(ctx) + return err == nil && len(daemons) > 0 + }, testutil.WaitShort, testutil.IntervalFast) +} + +// mustWaitForProvisionersAvailable waits for provisioners to be available for a specific workspace +func MustWaitForProvisionersAvailable(t *testing.T, db database.Store, workspace codersdk.Workspace, staleDuration time.Duration) { + t.Helper() + ctx := ctxWithProvisionerPermissions(testutil.Context(t, testutil.WaitShort)) + + // Get the workspace from the database + require.Eventually(t, func() bool { + ws, err := db.GetWorkspaceByID(ctx, workspace.ID) + if err != nil { + return false + } + + // Get the latest build + latestBuild, err := db.GetWorkspaceBuildByID(ctx, workspace.LatestBuild.ID) + if err != nil { + return false + } + + // Get the template version job + templateVersionJob, err := db.GetProvisionerJobByID(ctx, latestBuild.JobID) + if err != nil { + return false + } + + // Check if provisioners are available using the same logic as hasAvailableProvisioners + provisionerDaemons, err := db.GetProvisionerDaemonsByOrganization(ctx, database.GetProvisionerDaemonsByOrganizationParams{ + OrganizationID: ws.OrganizationID, + WantTags: templateVersionJob.Tags, + }) + if err != nil { + return false + } + + // Check if any provisioners are active (not stale) + now := time.Now() + for _, pd := range provisionerDaemons { + if pd.LastSeenAt.Valid { + age := now.Sub(pd.LastSeenAt.Time) + if age <= staleDuration { + return true // Found an active provisioner + } + } + } + return false // No active provisioners found + }, testutil.WaitShort, testutil.IntervalFast) +} diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index 2278fb2a71939..0a008043d2f44 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -619,7 +619,7 @@ func TestWorkspaceAutobuild(t *testing.T) { failureTTL = time.Minute ) - client, user := coderdenttest.New(t, &coderdenttest.Options{ + client, db, user := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ Options: &coderdtest.Options{ Logger: &logger, AutobuildTicker: ticker, @@ -644,7 +644,12 @@ func TestWorkspaceAutobuild(t *testing.T) { ws := coderdtest.CreateWorkspace(t, client, template.ID) build := coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, ws.LatestBuild.ID) require.Equal(t, codersdk.WorkspaceStatusFailed, build.Status) - ticker <- build.Job.CompletedAt.Add(failureTTL * 2) + tickTime := build.Job.CompletedAt.Add(failureTTL * 2) + + p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{}) + require.NoError(t, err) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + ticker <- tickTime stats := <-statCh // Expect workspace to transition to stopped state for breaching // failure TTL. @@ -666,7 +671,7 @@ func TestWorkspaceAutobuild(t *testing.T) { failureTTL = time.Minute ) - client, user := coderdenttest.New(t, &coderdenttest.Options{ + client, db, user := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ Options: &coderdtest.Options{ Logger: &logger, AutobuildTicker: ticker, @@ -691,7 +696,12 @@ func TestWorkspaceAutobuild(t *testing.T) { build := coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, ws.LatestBuild.ID) require.Equal(t, codersdk.WorkspaceStatusFailed, build.Status) // Make it impossible to trigger the failure TTL. - ticker <- build.Job.CompletedAt.Add(-failureTTL * 2) + tickTime := build.Job.CompletedAt.Add(-failureTTL * 2) + + p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{}) + require.NoError(t, err) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + ticker <- tickTime stats := <-statCh // Expect no transitions since not enough time has elapsed. require.Len(t, stats.Transitions, 0) @@ -759,10 +769,11 @@ func TestWorkspaceAutobuild(t *testing.T) { client, db, user := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ Options: &coderdtest.Options{ - AutobuildTicker: ticker, - AutobuildStats: statCh, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger, nil), - Auditor: auditRecorder, + AutobuildTicker: ticker, + AutobuildStats: statCh, + IncludeProvisionerDaemon: true, + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger, nil), + Auditor: auditRecorder, }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, @@ -790,7 +801,12 @@ func TestWorkspaceAutobuild(t *testing.T) { auditRecorder.ResetLogs() // Simulate being inactive. - ticker <- workspace.LastUsedAt.Add(inactiveTTL * 2) + tickTime := workspace.LastUsedAt.Add(inactiveTTL * 2) + + p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{}) + require.NoError(t, err) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + ticker <- tickTime stats := <-statCh // Expect workspace to transition to stopped state for breaching @@ -813,7 +829,7 @@ func TestWorkspaceAutobuild(t *testing.T) { dormantLastUsedAt := ws.LastUsedAt // nolint:gocritic // this test is not testing RBAC. - err := client.UpdateWorkspaceDormancy(ctx, ws.ID, codersdk.UpdateWorkspaceDormancy{Dormant: false}) + err = client.UpdateWorkspaceDormancy(ctx, ws.ID, codersdk.UpdateWorkspaceDormancy{Dormant: false}) require.NoError(t, err) // Assert that we updated our last_used_at so that we don't immediately @@ -888,7 +904,12 @@ func TestWorkspaceAutobuild(t *testing.T) { } // Simulate being inactive. - ticker <- time.Now().Add(time.Hour) + // Fix provisioner stale issue by updating LastSeenAt to the tick time + tickTime := time.Now().Add(time.Hour) + p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspaces[0].OrganizationID, database.ProvisionerJob{}) + require.NoError(t, err) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + ticker <- tickTime stats := <-statCh // Expect workspace to transition to stopped state for breaching @@ -997,7 +1018,7 @@ func TestWorkspaceAutobuild(t *testing.T) { ) logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) - client, user := coderdenttest.New(t, &coderdenttest.Options{ + client, db, user := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ Options: &coderdtest.Options{ AutobuildTicker: ticker, IncludeProvisionerDaemon: true, @@ -1029,7 +1050,11 @@ func TestWorkspaceAutobuild(t *testing.T) { ws = coderdtest.MustTransitionWorkspace(t, client, ws.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop) // Simulate not having accessed the workspace in a while. - ticker <- ws.LastUsedAt.Add(2 * inactiveTTL) + tickTime := ws.LastUsedAt.Add(2 * inactiveTTL) + p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{}) + require.NoError(t, err) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + ticker <- tickTime stats := <-statCh // Expect no transitions since workspace is stopped. require.Len(t, stats.Transitions, 0) @@ -1051,7 +1076,7 @@ func TestWorkspaceAutobuild(t *testing.T) { ) logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) - client, user := coderdenttest.New(t, &coderdenttest.Options{ + client, db, user := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ Options: &coderdtest.Options{ AutobuildTicker: ticker, IncludeProvisionerDaemon: true, @@ -1079,7 +1104,11 @@ func TestWorkspaceAutobuild(t *testing.T) { require.Equal(t, codersdk.WorkspaceStatusRunning, build.Status) // Simulate not having accessed the workspace in a while. - ticker <- ws.LastUsedAt.Add(2 * transitionTTL) + tickTime := ws.LastUsedAt.Add(2 * transitionTTL) + p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{}) + require.NoError(t, err) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + ticker <- tickTime stats := <-statCh // Expect workspace to transition to stopped state for breaching // inactive TTL. @@ -1094,7 +1123,9 @@ func TestWorkspaceAutobuild(t *testing.T) { _ = coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, ws.LatestBuild.ID) // Simulate the workspace being dormant beyond the threshold. - ticker <- ws.DormantAt.Add(2 * transitionTTL) + tickTime2 := ws.DormantAt.Add(2 * transitionTTL) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + ticker <- tickTime2 stats = <-statCh require.Len(t, stats.Transitions, 1) // The workspace should be scheduled for deletion. @@ -1106,7 +1137,7 @@ func TestWorkspaceAutobuild(t *testing.T) { // Assert that the workspace is actually deleted. //nolint:gocritic // ensuring workspace is deleted and not just invisible to us due to RBAC - _, err := client.Workspace(testutil.Context(t, testutil.WaitShort), ws.ID) + _, err = client.Workspace(testutil.Context(t, testutil.WaitShort), ws.ID) require.Error(t, err) cerr, ok := codersdk.AsError(err) require.True(t, ok) @@ -1123,7 +1154,7 @@ func TestWorkspaceAutobuild(t *testing.T) { ) logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) - client, user := coderdenttest.New(t, &coderdenttest.Options{ + client, db, user := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ Options: &coderdtest.Options{ AutobuildTicker: ticker, IncludeProvisionerDaemon: true, @@ -1158,7 +1189,11 @@ func TestWorkspaceAutobuild(t *testing.T) { require.NotNil(t, ws.DormantAt) // Ensure we haven't breached our threshold. - ticker <- ws.DormantAt.Add(-dormantTTL * 2) + tickTime := ws.DormantAt.Add(-dormantTTL * 2) + p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{}) + require.NoError(t, err) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + ticker <- tickTime stats := <-statCh // Expect no transitions since not enough time has elapsed. require.Len(t, stats.Transitions, 0) @@ -1169,7 +1204,9 @@ func TestWorkspaceAutobuild(t *testing.T) { require.NoError(t, err) // Simlute the workspace breaching the threshold. - ticker <- ws.DormantAt.Add(dormantTTL * 2) + tickTime2 := ws.DormantAt.Add(dormantTTL * 2) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime2) + ticker <- tickTime2 stats = <-statCh require.Len(t, stats.Transitions, 1) require.Equal(t, database.WorkspaceTransitionDelete, stats.Transitions[ws.ID]) @@ -1186,7 +1223,7 @@ func TestWorkspaceAutobuild(t *testing.T) { ) logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) - client, user := coderdenttest.New(t, &coderdenttest.Options{ + client, db, user := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ Options: &coderdtest.Options{ AutobuildTicker: tickCh, IncludeProvisionerDaemon: true, @@ -1217,7 +1254,11 @@ func TestWorkspaceAutobuild(t *testing.T) { ws = coderdtest.MustTransitionWorkspace(t, client, ws.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop) // Assert that autostart works when the workspace isn't dormant.. - tickCh <- sched.Next(ws.LatestBuild.CreatedAt) + tickTime := sched.Next(ws.LatestBuild.CreatedAt) + p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{}) + require.NoError(t, err) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + tickCh <- tickTime stats := <-statsCh require.Len(t, stats.Errors, 0) require.Len(t, stats.Transitions, 1) @@ -1237,7 +1278,9 @@ func TestWorkspaceAutobuild(t *testing.T) { require.NoError(t, err) // We should see the workspace get stopped now. - tickCh <- ws.LastUsedAt.Add(inactiveTTL * 2) + tickTime2 := ws.LastUsedAt.Add(inactiveTTL * 2) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + tickCh <- tickTime2 stats = <-statsCh require.Len(t, stats.Errors, 0) require.Len(t, stats.Transitions, 1) @@ -1267,7 +1310,7 @@ func TestWorkspaceAutobuild(t *testing.T) { ) logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) - client, user := coderdenttest.New(t, &coderdenttest.Options{ + client, db, user := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ Options: &coderdtest.Options{ AutobuildTicker: ticker, IncludeProvisionerDaemon: true, @@ -1335,13 +1378,19 @@ func TestWorkspaceAutobuild(t *testing.T) { // Simulate ticking an hour after the workspace is expected to be deleted. // Under normal circumstances this should result in a transition but // since our last build resulted in failure it should be skipped. - ticker <- build.Job.CompletedAt.Add(time.Hour) + tickTime := build.Job.CompletedAt.Add(time.Hour) + p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{}) + require.NoError(t, err) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + ticker <- tickTime stats := <-statCh require.Len(t, stats.Transitions, 0) // Simulate ticking a day after the workspace was last attempted to // be deleted. This should result in an attempt. - ticker <- build.Job.CompletedAt.Add(time.Hour * 25) + tickTime2 := build.Job.CompletedAt.Add(time.Hour * 25) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime2) + ticker <- tickTime2 stats = <-statCh require.Len(t, stats.Transitions, 1) require.Equal(t, database.WorkspaceTransitionDelete, stats.Transitions[ws.ID]) @@ -1356,7 +1405,7 @@ func TestWorkspaceAutobuild(t *testing.T) { ) logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) - client, user := coderdenttest.New(t, &coderdenttest.Options{ + client, db, user := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ Options: &coderdtest.Options{ AutobuildTicker: tickCh, IncludeProvisionerDaemon: true, @@ -1401,7 +1450,11 @@ func TestWorkspaceAutobuild(t *testing.T) { require.NoError(t, err) // Kick of an autostart build. - tickCh <- sched.Next(ws.LatestBuild.CreatedAt) + tickTime := sched.Next(ws.LatestBuild.CreatedAt) + p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{}) + require.NoError(t, err) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + tickCh <- tickTime stats := <-statsCh require.Len(t, stats.Errors, 0) require.Len(t, stats.Transitions, 1) @@ -1429,7 +1482,9 @@ func TestWorkspaceAutobuild(t *testing.T) { }) // Force an autostart transition again. - tickCh <- sched.Next(firstBuild.CreatedAt) + tickTime2 := sched.Next(firstBuild.CreatedAt) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + tickCh <- tickTime2 stats = <-statsCh require.Len(t, stats.Errors, 0) require.Len(t, stats.Transitions, 1) @@ -1453,7 +1508,7 @@ func TestWorkspaceAutobuild(t *testing.T) { clock.Set(dbtime.Now()) logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) - client, user := coderdenttest.New(t, &coderdenttest.Options{ + client, db, user := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ Options: &coderdtest.Options{ AutobuildTicker: tickCh, IncludeProvisionerDaemon: true, @@ -1494,6 +1549,9 @@ func TestWorkspaceAutobuild(t *testing.T) { next = sched.Next(next) clock.Set(next) + p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{}) + require.NoError(t, err) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), next) tickCh <- next stats := <-statsCh ws = coderdtest.MustWorkspace(t, client, ws.ID) @@ -2207,11 +2265,20 @@ func TestExecutorPrebuilds(t *testing.T) { // Then: the claimed workspace should inherit and respect that same NextStartAt require.True(t, workspace.NextStartAt.Equal(*prebuild.NextStartAt)) - // Tick at the next scheduled time after the prebuild’s LatestBuild.CreatedAt, - // since the next allowed autostart is calculated starting from that point. - // When: the autobuild executor ticks after the scheduled time + // Wait for provisioner to be registered + coderdtest.MustWaitForProvisioners(t, db) + + // Wait for provisioner to be available for this specific workspace + coderdtest.MustWaitForProvisionersAvailable(t, db, prebuild, provisionerdserver.StaleInterval) + + tickTime := sched.Next(prebuild.LatestBuild.CreatedAt).Add(time.Minute) + p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{}) + require.NoError(t, err) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + + // When: the autobuild executor ticks at the scheduled time go func() { - tickCh <- sched.Next(prebuild.LatestBuild.CreatedAt).Add(time.Minute) + tickCh <- tickTime }() // Then: the workspace should have a NextStartAt equal to the next autostart schedule From 0bc4ccecc91b58ac0501fc0abea399889790ccd7 Mon Sep 17 00:00:00 2001 From: Callum Styan Date: Mon, 11 Aug 2025 19:49:23 +0000 Subject: [PATCH 07/16] significantly simplify TestExecutorAutostartSkipsWhenNoProvisionersAvailable Signed-off-by: Callum Styan --- coderd/autobuild/lifecycle_executor.go | 7 +- coderd/autobuild/lifecycle_executor_test.go | 151 +++----------------- coderd/coderdtest/coderdtest.go | 7 +- 3 files changed, 26 insertions(+), 139 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index 61d59a17fdaea..c914a29702169 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -34,8 +34,6 @@ import ( "github.com/coder/coder/v2/codersdk" ) -const TestingStaleInterval = time.Second * 5 - // Executor automatically starts or stops workspaces. type Executor struct { ctx context.Context @@ -148,19 +146,20 @@ func (e *Executor) hasAvailableProvisioners(ctx context.Context, tx database.Sto return false, xerrors.Errorf("get provisioner daemons: %w", err) } + logger := e.log.With(slog.F("tags", templateVersionJob.Tags)) // Check if any provisioners are active (not stale) for _, pd := range provisionerDaemons { if pd.LastSeenAt.Valid { age := t.Sub(pd.LastSeenAt.Time) if age <= provisionerdserver.StaleInterval { - e.log.Debug(ctx, "hasAvailableProvisioners: found active provisioner", + logger.Debug(ctx, "hasAvailableProvisioners: found active provisioner", slog.F("daemon_id", pd.ID), ) return true, nil } } } - e.log.Debug(ctx, "hasAvailableProvisioners: no active provisioners found") + logger.Debug(ctx, "hasAvailableProvisioners: no active provisioners found") return false, nil } diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go index ac9a50bff2111..15a3ce6e2fec4 100644 --- a/coderd/autobuild/lifecycle_executor_test.go +++ b/coderd/autobuild/lifecycle_executor_test.go @@ -9,6 +9,7 @@ import ( "github.com/coder/coder/v2/coderd/database/dbgen" "github.com/coder/coder/v2/coderd/database/pubsub" + "github.com/coder/coder/v2/coderd/provisionerdserver" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/quartz" @@ -1669,7 +1670,6 @@ func TestMain(m *testing.M) { func TestExecutorAutostartSkipsWhenNoProvisionersAvailable(t *testing.T) { t.Parallel() - db, ps := dbtestutil.NewDB(t) var ( sched = mustSchedule(t, "CRON_TZ=UTC 0 * * * *") tickCh = make(chan time.Time) @@ -1679,12 +1679,10 @@ func TestExecutorAutostartSkipsWhenNoProvisionersAvailable(t *testing.T) { // Create client with provisioner closer provisionerDaemonTags := map[string]string{"owner": "testowner", "scope": "organization"} t.Logf("Setting provisioner daemon tags: %v", provisionerDaemonTags) - client, provisionerCloser := coderdtest.NewWithProvisionerCloser(t, &coderdtest.Options{ + client, db := coderdtest.NewWithDatabase(t, &coderdtest.Options{ AutobuildTicker: tickCh, IncludeProvisionerDaemon: true, AutobuildStats: statsCh, - Database: db, - Pubsub: ps, ProvisionerDaemonTags: provisionerDaemonTags, }) @@ -1695,147 +1693,34 @@ func TestExecutorAutostartSkipsWhenNoProvisionersAvailable(t *testing.T) { }) // Stop the workspace while provisioner is available - workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, - codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop) - - // Wait for provisioner to be registered - coderdtest.MustWaitForProvisioners(t, db) + workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop) // Wait for provisioner to be available for this specific workspace - coderdtest.MustWaitForProvisionersAvailable(t, db, workspace, autobuild.TestingStaleInterval) - - // Now shut down the provisioner daemon - ctx := testutil.Context(t, testutil.WaitShort) - err := provisionerCloser.Close() - require.NoError(t, err) - - // Wait for the provisioner to become stale (heartbeat stops) - // The stale interval is 5 seconds, so we need to wait a bit longer - time.Sleep(6 * time.Second) - - // Debug: check what's in the database - daemons, err := db.GetProvisionerDaemons(ctx) - require.NoError(t, err) - t.Logf("After close: found %d daemons", len(daemons)) - for i, daemon := range daemons { - t.Logf("Daemon %d: ID=%s, Name=%s, LastSeen=%v", i, daemon.ID, daemon.Name, daemon.LastSeenAt) - } + id := coderdtest.MustWaitForProvisionersAvailable(t, db, workspace, provisionerdserver.StaleInterval) - // Since we commented out the close call, the provisioner should NOT become stale - // Let's wait a bit but NOT longer than the stale interval - time.Sleep(2 * time.Second) // Wait less than the 5-second stale interval - - daemons, err = db.GetProvisionerDaemons(ctx) - require.NoError(t, err) - require.NotEmpty(t, daemons, "should have provisioner daemons") - - now := time.Now() - for _, daemon := range daemons { - if daemon.LastSeenAt.Valid { - age := now.Sub(daemon.LastSeenAt.Time) - t.Logf("Daemon %s: age=%v, staleInterval=%v, isStale=%v", daemon.Name, age, autobuild.TestingStaleInterval, age > autobuild.TestingStaleInterval) - // Since we closed the provisioner, it should be stale - require.True(t, age > autobuild.TestingStaleInterval, "provisioner should be stale since we closed it") - } - } - - // Debug: Check the template version job tags and organization - templateVersion, err := client.TemplateVersion(context.Background(), workspace.LatestBuild.TemplateVersionID) - require.NoError(t, err) - t.Logf("Template version job ID: %s", templateVersion.Job.ID) - t.Logf("Template version job tags: %v", templateVersion.Job.Tags) - t.Logf("Workspace organization ID: %s", workspace.OrganizationID) - t.Logf("Template version organization ID: %s", templateVersion.OrganizationID) - t.Logf("Workspace LatestBuild.TemplateVersionID: %s", workspace.LatestBuild.TemplateVersionID) - - // Debug: Get the template version job directly from database to compare - templateVersionJobFromDB, err := db.GetProvisionerJobByID(context.Background(), templateVersion.Job.ID) - require.NoError(t, err) - t.Logf("Template version job from DB - ID: %s, Tags: %v", templateVersionJobFromDB.ID, templateVersionJobFromDB.Tags) - - // Debug: Query database directly to see what provisioner daemons exist - allDaemons, err := db.GetProvisionerDaemons(context.Background()) - require.NoError(t, err) - t.Logf("Total provisioner daemons in database: %d", len(allDaemons)) - for i, daemon := range allDaemons { - t.Logf("Daemon %d: ID=%s, Name=%s, OrgID=%s, Tags=%v", i, daemon.ID, daemon.Name, daemon.OrganizationID, daemon.Tags) - } + // Ensure the provisioner is stale + staleTime := sched.Next(workspace.LatestBuild.CreatedAt).Add((-1 * provisionerdserver.StaleInterval) + -10*time.Second) + coderdtest.UpdateProvisionerLastSeenAt(t, db, id, time.Now(), staleTime) - // Debug: Test if we can query using the ACTUAL provisioner daemon tags - if len(allDaemons) > 0 { - actualDaemonTags := allDaemons[0].Tags - t.Logf("Testing query with ACTUAL daemon tags: %v", actualDaemonTags) - queryWithActualTags := database.GetProvisionerDaemonsByOrganizationParams{ - OrganizationID: workspace.OrganizationID, - WantTags: actualDaemonTags, - } - matchingWithActualTags, err := db.GetProvisionerDaemonsByOrganization(context.Background(), queryWithActualTags) - require.NoError(t, err) - t.Logf("Query with actual daemon tags returns: %d daemons", len(matchingWithActualTags)) - } - - // Debug: Test the exact query that hasAvailableProvisioners uses - queryParams := database.GetProvisionerDaemonsByOrganizationParams{ - OrganizationID: workspace.OrganizationID, - WantTags: templateVersion.Job.Tags, - } - t.Logf("Query params: OrgID=%s, WantTags=%v", queryParams.OrganizationID, queryParams.WantTags) - t.Logf("Test query detailed params: org_id_type=%T, org_id_value=%s, want_tags_type=%T, want_tags_value=%v", - queryParams.OrganizationID, queryParams.OrganizationID, queryParams.WantTags, queryParams.WantTags) - - // Test 1: Transaction isolation - try with different transaction contexts - t.Logf("=== Testing transaction isolation ===") - - // Query with context.Background() (what we used above) - matchingDaemons1, err := db.GetProvisionerDaemonsByOrganization(context.Background(), queryParams) - require.NoError(t, err) - t.Logf("With context.Background(): %d daemons", len(matchingDaemons1)) - - // Query with a fresh context - ctx2 := context.Background() - matchingDaemons2, err2 := db.GetProvisionerDaemonsByOrganization(ctx2, queryParams) - require.NoError(t, err2) - t.Logf("With fresh context: %d daemons", len(matchingDaemons2)) - - // Query within a transaction (like hasAvailableProvisioners might use) - err = db.InTx(func(tx database.Store) error { - matchingDaemons3, err := tx.GetProvisionerDaemonsByOrganization(ctx2, queryParams) - if err != nil { - return err - } - t.Logf("Within transaction: %d daemons", len(matchingDaemons3)) - return nil - }, nil) - require.NoError(t, err) + // Trigger autobuild + tickCh <- sched.Next(workspace.LatestBuild.CreatedAt) - // Test 2: Timing issue - query right before and after hasAvailableProvisioners - t.Logf("=== Testing timing issue ===") + stats := <-statsCh - // Query right before the autostart trigger - beforeDaemons, err := db.GetProvisionerDaemonsByOrganization(context.Background(), queryParams) - require.NoError(t, err) - t.Logf("Right before autostart: %d daemons", len(beforeDaemons)) + // This assertion should FAIL when provisioner is available (not stale), can confirm by commenting out the + // UpdateProvisionerLastSeenAt call above. + assert.Len(t, stats.Transitions, 0, "should not create builds when no provisioners available") - // Since we commented out the close call, the provisioner is still active - // This means autobuild should proceed (not be skipped) and create transitions - // The test expects 0 transitions (skipped autostart), but with an active provisioner, - // it should get >0 transitions, causing the test to FAIL as intended + // Ensure the provisioner is NOT stale, and see if we get a successful state transition. + notStaleTime := sched.Next(workspace.LatestBuild.CreatedAt).Add((-1 * provisionerdserver.StaleInterval) + 10*time.Second) + coderdtest.UpdateProvisionerLastSeenAt(t, db, id, time.Now(), notStaleTime) // Trigger autobuild go func() { tickCh <- sched.Next(workspace.LatestBuild.CreatedAt) close(tickCh) }() + stats = <-statsCh - stats := <-statsCh - - // Query right after hasAvailableProvisioners was called - afterDaemons, err := db.GetProvisionerDaemonsByOrganization(context.Background(), queryParams) - require.NoError(t, err) - t.Logf("Right after autostart: %d daemons", len(afterDaemons)) - - // This assertion should FAIL when provisioner is available (demonstrating the fix works) - // When provisioner close is commented out: provisioner available → autostart proceeds → transitions > 0 → test fails ✓ - // When provisioner close is active: provisioner unavailable → autostart skipped → transitions = 0 → test passes ✓ - assert.Len(t, stats.Transitions, 0, "should not create builds when no provisioners available") + assert.Len(t, stats.Transitions, 1, "should not create builds when no provisioners available") } diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index cf432002c9bfb..be676d660b7ce 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -1661,10 +1661,10 @@ func MustWaitForProvisionersWithClient(t *testing.T, client *codersdk.Client) { } // mustWaitForProvisionersAvailable waits for provisioners to be available for a specific workspace -func MustWaitForProvisionersAvailable(t *testing.T, db database.Store, workspace codersdk.Workspace, staleDuration time.Duration) { +func MustWaitForProvisionersAvailable(t *testing.T, db database.Store, workspace codersdk.Workspace, staleDuration time.Duration) uuid.UUID { t.Helper() ctx := ctxWithProvisionerPermissions(testutil.Context(t, testutil.WaitShort)) - + id := uuid.UUID{} // Get the workspace from the database require.Eventually(t, func() bool { ws, err := db.GetWorkspaceByID(ctx, workspace.ID) @@ -1699,10 +1699,13 @@ func MustWaitForProvisionersAvailable(t *testing.T, db database.Store, workspace if pd.LastSeenAt.Valid { age := now.Sub(pd.LastSeenAt.Time) if age <= staleDuration { + id = pd.ID return true // Found an active provisioner } } } return false // No active provisioners found }, testutil.WaitShort, testutil.IntervalFast) + + return id } From 9a3f534bf1e2cdeb3fc493ced4b0f969618d89ce Mon Sep 17 00:00:00 2001 From: Callum Styan Date: Mon, 11 Aug 2025 21:34:04 +0000 Subject: [PATCH 08/16] more refactor and clean up Signed-off-by: Callum Styan --- coderd/autobuild/lifecycle_executor.go | 6 ++-- coderd/autobuild/lifecycle_executor_test.go | 38 +++++++++++---------- coderd/coderdtest/coderdtest.go | 13 +++---- enterprise/coderd/workspaces_test.go | 24 ++++++------- 4 files changed, 43 insertions(+), 38 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index c914a29702169..2cbbf67861183 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -132,7 +132,9 @@ func (e *Executor) Run() { }() } -func (e *Executor) hasAvailableProvisioners(ctx context.Context, tx database.Store, t time.Time, ws database.Workspace, templateVersionJob database.ProvisionerJob) (bool, error) { +// hasNonStaleProvisioner checks whether there is at least one valid (non-stale, correct tags) provisioner +// based on t and the tags on templateVersionJob. +func (e *Executor) hasValidProvisioner(ctx context.Context, tx database.Store, t time.Time, ws database.Workspace, templateVersionJob database.ProvisionerJob) (bool, error) { queryParams := database.GetProvisionerDaemonsByOrganizationParams{ OrganizationID: ws.OrganizationID, WantTags: templateVersionJob.Tags, @@ -319,7 +321,7 @@ func (e *Executor) runOnce(t time.Time) Stats { } // Before creating the workspace build, check for available provisioners - hasProvisioners, err := e.hasAvailableProvisioners(e.ctx, tx, t, ws, templateVersionJob) + hasProvisioners, err := e.hasNonStaleProvisioner(e.ctx, tx, t, ws, templateVersionJob) if err != nil { return xerrors.Errorf("check provisioner availability: %w", err) } diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go index 15a3ce6e2fec4..f38332d586661 100644 --- a/coderd/autobuild/lifecycle_executor_test.go +++ b/coderd/autobuild/lifecycle_executor_test.go @@ -56,7 +56,7 @@ func TestExecutorAutostartOK(t *testing.T) { ) // Given: workspace is stopped workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop) - p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{}) + p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, map[string]string{}) require.NoError(t, err) // When: the autobuild executor ticks after the scheduled time go func() { @@ -118,7 +118,7 @@ func TestMultipleLifecycleExecutors(t *testing.T) { // Have the workspace stopped so we can perform an autostart workspace = coderdtest.MustTransitionWorkspace(t, clientA, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop) - p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{}) + p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil) require.NoError(t, err) // Get both clients to perform a lifecycle execution tick next := sched.Next(workspace.LatestBuild.CreatedAt) @@ -254,7 +254,7 @@ func TestExecutorAutostartTemplateUpdated(t *testing.T) { }, )) - p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{}) + p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil) require.NoError(t, err) t.Log("sending autobuild tick") @@ -440,7 +440,7 @@ func TestExecutorAutostopOK(t *testing.T) { require.Equal(t, codersdk.WorkspaceTransitionStart, workspace.LatestBuild.Transition) require.NotZero(t, workspace.LatestBuild.Deadline) - p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{}) + p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil) require.NoError(t, err) // When: the autobuild executor ticks *after* the deadline: @@ -489,7 +489,7 @@ func TestExecutorAutostopExtend(t *testing.T) { }) require.NoError(t, err, "extend workspace deadline") - p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{}) + p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil) require.NoError(t, err) // When: the autobuild executor ticks *after* the original deadline: @@ -802,7 +802,7 @@ func TestExecutorAutostartMultipleOK(t *testing.T) { // Given: workspace is stopped workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop) - p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{}) + p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil) require.NoError(t, err) // When: the autobuild executor ticks past the scheduled time @@ -872,7 +872,7 @@ func TestExecutorAutostartWithParameters(t *testing.T) { // Given: workspace is stopped workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop) - p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{}) + p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil) require.NoError(t, err) // When: the autobuild executor ticks after the scheduled time @@ -971,7 +971,7 @@ func TestExecutorAutostopTemplateDisabled(t *testing.T) { // Then: the deadline should be set to the template default TTL assert.WithinDuration(t, workspace.LatestBuild.CreatedAt.Add(time.Hour), workspace.LatestBuild.Deadline.Time, time.Minute) - p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{}) + p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil) require.NoError(t, err) // When: the autobuild executor ticks after the workspace setting, but before the template setting: @@ -1059,7 +1059,7 @@ func TestExecutorRequireActiveVersion(t *testing.T) { }) require.Equal(t, inactiveVersion.ID, ws.LatestBuild.TemplateVersionID) - p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{}) + p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil) require.NoError(t, err) tickTime := sched.Next(ws.LatestBuild.CreatedAt) @@ -1221,7 +1221,7 @@ func TestNotifications(t *testing.T) { workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop) _ = coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, workspace.LatestBuild.ID) - p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{}) + p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil) require.NoError(t, err) // Wait for workspace to become dormant @@ -1302,7 +1302,7 @@ func TestExecutorPrebuilds(t *testing.T) { require.Equal(t, codersdk.WorkspaceTransitionStart, prebuild.LatestBuild.Transition) require.NotZero(t, prebuild.LatestBuild.Deadline) - p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), prebuild.OrganizationID, database.ProvisionerJob{}) + p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), prebuild.OrganizationID, nil) require.NoError(t, err) // When: the autobuild executor ticks *after* the deadline: @@ -1676,8 +1676,9 @@ func TestExecutorAutostartSkipsWhenNoProvisionersAvailable(t *testing.T) { statsCh = make(chan autobuild.Stats) ) - // Create client with provisioner closer - provisionerDaemonTags := map[string]string{"owner": "testowner", "scope": "organization"} + // Use provisioner daemon tags so we can test `hasAvailableProvisioner` more thoroughly. + // We can't overwrite owner or scope as there's a `provisionersdk.MutateTags` function that has restrictions on those. + provisionerDaemonTags := map[string]string{"test-tag": "asdf"} t.Logf("Setting provisioner daemon tags: %v", provisionerDaemonTags) client, db := coderdtest.NewWithDatabase(t, &coderdtest.Options{ AutobuildTicker: tickCh, @@ -1687,8 +1688,7 @@ func TestExecutorAutostartSkipsWhenNoProvisionersAvailable(t *testing.T) { }) // Create workspace with autostart enabled and matching provisioner tags - provisionerTags := map[string]string{"owner": "testowner", "scope": "organization"} - workspace := mustProvisionWorkspaceWithProvisionerTags(t, client, provisionerTags, func(cwr *codersdk.CreateWorkspaceRequest) { + workspace := mustProvisionWorkspaceWithProvisionerTags(t, client, provisionerDaemonTags, func(cwr *codersdk.CreateWorkspaceRequest) { cwr.AutostartSchedule = ptr.Ref(sched.String()) }) @@ -1696,11 +1696,13 @@ func TestExecutorAutostartSkipsWhenNoProvisionersAvailable(t *testing.T) { workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop) // Wait for provisioner to be available for this specific workspace - id := coderdtest.MustWaitForProvisionersAvailable(t, db, workspace, provisionerdserver.StaleInterval) + coderdtest.MustWaitForProvisionersAvailable(t, db, workspace, provisionerdserver.StaleInterval) + p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, provisionerDaemonTags) + require.NoError(t, err, "Error getting provisioner for workspace") // Ensure the provisioner is stale staleTime := sched.Next(workspace.LatestBuild.CreatedAt).Add((-1 * provisionerdserver.StaleInterval) + -10*time.Second) - coderdtest.UpdateProvisionerLastSeenAt(t, db, id, time.Now(), staleTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), staleTime) // Trigger autobuild tickCh <- sched.Next(workspace.LatestBuild.CreatedAt) @@ -1713,7 +1715,7 @@ func TestExecutorAutostartSkipsWhenNoProvisionersAvailable(t *testing.T) { // Ensure the provisioner is NOT stale, and see if we get a successful state transition. notStaleTime := sched.Next(workspace.LatestBuild.CreatedAt).Add((-1 * provisionerdserver.StaleInterval) + 10*time.Second) - coderdtest.UpdateProvisionerLastSeenAt(t, db, id, time.Now(), notStaleTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), notStaleTime) // Trigger autobuild go func() { diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index be676d660b7ce..5cdb69669d899 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -1593,11 +1593,14 @@ func DeploymentValues(t testing.TB, mut ...func(*codersdk.DeploymentValues)) *co return cfg } -// GetProvisionerForWorkspace returns the first valid provisioner for a workspace + template tags. -func GetProvisionerForWorkspace(t *testing.T, tx database.Store, curTime time.Time, orgID uuid.UUID, templateVersionJob database.ProvisionerJob) (database.ProvisionerDaemon, error) { +// GetProvisionerForTags returns the first valid provisioner for a workspace + template tags. +func GetProvisionerForTags(t *testing.T, tx database.Store, curTime time.Time, orgID uuid.UUID, tags map[string]string) (database.ProvisionerDaemon, error) { + if tags == nil { + tags = map[string]string{} + } queryParams := database.GetProvisionerDaemonsByOrganizationParams{ OrganizationID: orgID, - WantTags: templateVersionJob.Tags, + WantTags: tags, } // 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 if pd.LastSeenAt.Valid { age := curTime.Sub(pd.LastSeenAt.Time) if age <= provisionerdserver.StaleInterval { - t.Logf("hasAvailableProvisioners found active provisioner, daemon_id: %+v", pd.ID) return pd, nil } } } - t.Logf("hasAvailableProvisioners: no active provisioners found") - return database.ProvisionerDaemon{}, nil + return database.ProvisionerDaemon{}, xerrors.New("no available provisioners found") } func ctxWithProvisionerPermissions(ctx context.Context) context.Context { diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index 0a008043d2f44..1881266319391 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -646,7 +646,7 @@ func TestWorkspaceAutobuild(t *testing.T) { require.Equal(t, codersdk.WorkspaceStatusFailed, build.Status) tickTime := build.Job.CompletedAt.Add(failureTTL * 2) - p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{}) + p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil) require.NoError(t, err) coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) ticker <- tickTime @@ -698,7 +698,7 @@ func TestWorkspaceAutobuild(t *testing.T) { // Make it impossible to trigger the failure TTL. tickTime := build.Job.CompletedAt.Add(-failureTTL * 2) - p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{}) + p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil) require.NoError(t, err) coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) ticker <- tickTime @@ -803,7 +803,7 @@ func TestWorkspaceAutobuild(t *testing.T) { // Simulate being inactive. tickTime := workspace.LastUsedAt.Add(inactiveTTL * 2) - p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{}) + p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil) require.NoError(t, err) coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) ticker <- tickTime @@ -906,7 +906,7 @@ func TestWorkspaceAutobuild(t *testing.T) { // Simulate being inactive. // Fix provisioner stale issue by updating LastSeenAt to the tick time tickTime := time.Now().Add(time.Hour) - p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspaces[0].OrganizationID, database.ProvisionerJob{}) + p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspaces[0].OrganizationID, nil) require.NoError(t, err) coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) ticker <- tickTime @@ -1051,7 +1051,7 @@ func TestWorkspaceAutobuild(t *testing.T) { // Simulate not having accessed the workspace in a while. tickTime := ws.LastUsedAt.Add(2 * inactiveTTL) - p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{}) + p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil) require.NoError(t, err) coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) ticker <- tickTime @@ -1105,7 +1105,7 @@ func TestWorkspaceAutobuild(t *testing.T) { // Simulate not having accessed the workspace in a while. tickTime := ws.LastUsedAt.Add(2 * transitionTTL) - p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{}) + p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil) require.NoError(t, err) coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) ticker <- tickTime @@ -1190,7 +1190,7 @@ func TestWorkspaceAutobuild(t *testing.T) { // Ensure we haven't breached our threshold. tickTime := ws.DormantAt.Add(-dormantTTL * 2) - p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{}) + p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil) require.NoError(t, err) coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) ticker <- tickTime @@ -1255,7 +1255,7 @@ func TestWorkspaceAutobuild(t *testing.T) { // Assert that autostart works when the workspace isn't dormant.. tickTime := sched.Next(ws.LatestBuild.CreatedAt) - p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{}) + p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil) require.NoError(t, err) coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) tickCh <- tickTime @@ -1379,7 +1379,7 @@ func TestWorkspaceAutobuild(t *testing.T) { // Under normal circumstances this should result in a transition but // since our last build resulted in failure it should be skipped. tickTime := build.Job.CompletedAt.Add(time.Hour) - p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{}) + p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil) require.NoError(t, err) coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) ticker <- tickTime @@ -1451,7 +1451,7 @@ func TestWorkspaceAutobuild(t *testing.T) { // Kick of an autostart build. tickTime := sched.Next(ws.LatestBuild.CreatedAt) - p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{}) + p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil) require.NoError(t, err) coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) tickCh <- tickTime @@ -1549,7 +1549,7 @@ func TestWorkspaceAutobuild(t *testing.T) { next = sched.Next(next) clock.Set(next) - p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), ws.OrganizationID, database.ProvisionerJob{}) + p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil) require.NoError(t, err) coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), next) tickCh <- next @@ -2272,7 +2272,7 @@ func TestExecutorPrebuilds(t *testing.T) { coderdtest.MustWaitForProvisionersAvailable(t, db, prebuild, provisionerdserver.StaleInterval) tickTime := sched.Next(prebuild.LatestBuild.CreatedAt).Add(time.Minute) - p, err := coderdtest.GetProvisionerForWorkspace(t, db, time.Now(), workspace.OrganizationID, database.ProvisionerJob{}) + p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil) require.NoError(t, err) coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) From a98ef1d4cb43f7936655f1cce1820f794ef96014 Mon Sep 17 00:00:00 2001 From: Callum Styan Date: Mon, 11 Aug 2025 21:47:43 +0000 Subject: [PATCH 09/16] fix lint Signed-off-by: Callum Styan --- coderd/autobuild/lifecycle_executor.go | 4 ++-- coderd/autobuild/lifecycle_executor_test.go | 24 ++++++++++----------- coderd/coderdtest/coderdtest.go | 2 +- enterprise/coderd/workspaces_test.go | 24 ++++++++++----------- 4 files changed, 27 insertions(+), 27 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index 2cbbf67861183..67e1046d9e383 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -132,7 +132,7 @@ func (e *Executor) Run() { }() } -// hasNonStaleProvisioner checks whether there is at least one valid (non-stale, correct tags) provisioner +// hasValidProvisioner checks whether there is at least one valid (non-stale, correct tags) provisioner // based on t and the tags on templateVersionJob. func (e *Executor) hasValidProvisioner(ctx context.Context, tx database.Store, t time.Time, ws database.Workspace, templateVersionJob database.ProvisionerJob) (bool, error) { queryParams := database.GetProvisionerDaemonsByOrganizationParams{ @@ -321,7 +321,7 @@ func (e *Executor) runOnce(t time.Time) Stats { } // Before creating the workspace build, check for available provisioners - hasProvisioners, err := e.hasNonStaleProvisioner(e.ctx, tx, t, ws, templateVersionJob) + hasProvisioners, err := e.hasValidProvisioner(e.ctx, tx, t, ws, templateVersionJob) if err != nil { return xerrors.Errorf("check provisioner availability: %w", err) } diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go index f38332d586661..11f1d1978460e 100644 --- a/coderd/autobuild/lifecycle_executor_test.go +++ b/coderd/autobuild/lifecycle_executor_test.go @@ -56,7 +56,7 @@ func TestExecutorAutostartOK(t *testing.T) { ) // Given: workspace is stopped workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop) - p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, map[string]string{}) + p, err := coderdtest.GetProvisionerForTags(db, time.Now(), workspace.OrganizationID, map[string]string{}) require.NoError(t, err) // When: the autobuild executor ticks after the scheduled time go func() { @@ -118,7 +118,7 @@ func TestMultipleLifecycleExecutors(t *testing.T) { // Have the workspace stopped so we can perform an autostart workspace = coderdtest.MustTransitionWorkspace(t, clientA, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop) - p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil) + p, err := coderdtest.GetProvisionerForTags(db, time.Now(), workspace.OrganizationID, nil) require.NoError(t, err) // Get both clients to perform a lifecycle execution tick next := sched.Next(workspace.LatestBuild.CreatedAt) @@ -254,7 +254,7 @@ func TestExecutorAutostartTemplateUpdated(t *testing.T) { }, )) - p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil) + p, err := coderdtest.GetProvisionerForTags(db, time.Now(), workspace.OrganizationID, nil) require.NoError(t, err) t.Log("sending autobuild tick") @@ -440,7 +440,7 @@ func TestExecutorAutostopOK(t *testing.T) { require.Equal(t, codersdk.WorkspaceTransitionStart, workspace.LatestBuild.Transition) require.NotZero(t, workspace.LatestBuild.Deadline) - p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil) + p, err := coderdtest.GetProvisionerForTags(db, time.Now(), workspace.OrganizationID, nil) require.NoError(t, err) // When: the autobuild executor ticks *after* the deadline: @@ -489,7 +489,7 @@ func TestExecutorAutostopExtend(t *testing.T) { }) require.NoError(t, err, "extend workspace deadline") - p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil) + p, err := coderdtest.GetProvisionerForTags(db, time.Now(), workspace.OrganizationID, nil) require.NoError(t, err) // When: the autobuild executor ticks *after* the original deadline: @@ -802,7 +802,7 @@ func TestExecutorAutostartMultipleOK(t *testing.T) { // Given: workspace is stopped workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop) - p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil) + p, err := coderdtest.GetProvisionerForTags(db, time.Now(), workspace.OrganizationID, nil) require.NoError(t, err) // When: the autobuild executor ticks past the scheduled time @@ -872,7 +872,7 @@ func TestExecutorAutostartWithParameters(t *testing.T) { // Given: workspace is stopped workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop) - p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil) + p, err := coderdtest.GetProvisionerForTags(db, time.Now(), workspace.OrganizationID, nil) require.NoError(t, err) // When: the autobuild executor ticks after the scheduled time @@ -971,7 +971,7 @@ func TestExecutorAutostopTemplateDisabled(t *testing.T) { // Then: the deadline should be set to the template default TTL assert.WithinDuration(t, workspace.LatestBuild.CreatedAt.Add(time.Hour), workspace.LatestBuild.Deadline.Time, time.Minute) - p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil) + p, err := coderdtest.GetProvisionerForTags(db, time.Now(), workspace.OrganizationID, nil) require.NoError(t, err) // When: the autobuild executor ticks after the workspace setting, but before the template setting: @@ -1059,7 +1059,7 @@ func TestExecutorRequireActiveVersion(t *testing.T) { }) require.Equal(t, inactiveVersion.ID, ws.LatestBuild.TemplateVersionID) - p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil) + p, err := coderdtest.GetProvisionerForTags(db, time.Now(), ws.OrganizationID, nil) require.NoError(t, err) tickTime := sched.Next(ws.LatestBuild.CreatedAt) @@ -1221,7 +1221,7 @@ func TestNotifications(t *testing.T) { workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop) _ = coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, workspace.LatestBuild.ID) - p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil) + p, err := coderdtest.GetProvisionerForTags(db, time.Now(), workspace.OrganizationID, nil) require.NoError(t, err) // Wait for workspace to become dormant @@ -1302,7 +1302,7 @@ func TestExecutorPrebuilds(t *testing.T) { require.Equal(t, codersdk.WorkspaceTransitionStart, prebuild.LatestBuild.Transition) require.NotZero(t, prebuild.LatestBuild.Deadline) - p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), prebuild.OrganizationID, nil) + p, err := coderdtest.GetProvisionerForTags(db, time.Now(), prebuild.OrganizationID, nil) require.NoError(t, err) // When: the autobuild executor ticks *after* the deadline: @@ -1697,7 +1697,7 @@ func TestExecutorAutostartSkipsWhenNoProvisionersAvailable(t *testing.T) { // Wait for provisioner to be available for this specific workspace coderdtest.MustWaitForProvisionersAvailable(t, db, workspace, provisionerdserver.StaleInterval) - p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, provisionerDaemonTags) + p, err := coderdtest.GetProvisionerForTags(db, time.Now(), workspace.OrganizationID, provisionerDaemonTags) require.NoError(t, err, "Error getting provisioner for workspace") // Ensure the provisioner is stale diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index 5cdb69669d899..4299e4064ccad 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -1594,7 +1594,7 @@ func DeploymentValues(t testing.TB, mut ...func(*codersdk.DeploymentValues)) *co } // GetProvisionerForTags returns the first valid provisioner for a workspace + template tags. -func GetProvisionerForTags(t *testing.T, tx database.Store, curTime time.Time, orgID uuid.UUID, tags map[string]string) (database.ProvisionerDaemon, error) { +func GetProvisionerForTags(tx database.Store, curTime time.Time, orgID uuid.UUID, tags map[string]string) (database.ProvisionerDaemon, error) { if tags == nil { tags = map[string]string{} } diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index 1881266319391..0eef8536ef445 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -646,7 +646,7 @@ func TestWorkspaceAutobuild(t *testing.T) { require.Equal(t, codersdk.WorkspaceStatusFailed, build.Status) tickTime := build.Job.CompletedAt.Add(failureTTL * 2) - p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil) + p, err := coderdtest.GetProvisionerForTags(db, time.Now(), ws.OrganizationID, nil) require.NoError(t, err) coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) ticker <- tickTime @@ -698,7 +698,7 @@ func TestWorkspaceAutobuild(t *testing.T) { // Make it impossible to trigger the failure TTL. tickTime := build.Job.CompletedAt.Add(-failureTTL * 2) - p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil) + p, err := coderdtest.GetProvisionerForTags(db, time.Now(), ws.OrganizationID, nil) require.NoError(t, err) coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) ticker <- tickTime @@ -803,7 +803,7 @@ func TestWorkspaceAutobuild(t *testing.T) { // Simulate being inactive. tickTime := workspace.LastUsedAt.Add(inactiveTTL * 2) - p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil) + p, err := coderdtest.GetProvisionerForTags(db, time.Now(), workspace.OrganizationID, nil) require.NoError(t, err) coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) ticker <- tickTime @@ -906,7 +906,7 @@ func TestWorkspaceAutobuild(t *testing.T) { // Simulate being inactive. // Fix provisioner stale issue by updating LastSeenAt to the tick time tickTime := time.Now().Add(time.Hour) - p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspaces[0].OrganizationID, nil) + p, err := coderdtest.GetProvisionerForTags(db, time.Now(), workspaces[0].OrganizationID, nil) require.NoError(t, err) coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) ticker <- tickTime @@ -1051,7 +1051,7 @@ func TestWorkspaceAutobuild(t *testing.T) { // Simulate not having accessed the workspace in a while. tickTime := ws.LastUsedAt.Add(2 * inactiveTTL) - p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil) + p, err := coderdtest.GetProvisionerForTags(db, time.Now(), ws.OrganizationID, nil) require.NoError(t, err) coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) ticker <- tickTime @@ -1105,7 +1105,7 @@ func TestWorkspaceAutobuild(t *testing.T) { // Simulate not having accessed the workspace in a while. tickTime := ws.LastUsedAt.Add(2 * transitionTTL) - p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil) + p, err := coderdtest.GetProvisionerForTags(db, time.Now(), ws.OrganizationID, nil) require.NoError(t, err) coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) ticker <- tickTime @@ -1190,7 +1190,7 @@ func TestWorkspaceAutobuild(t *testing.T) { // Ensure we haven't breached our threshold. tickTime := ws.DormantAt.Add(-dormantTTL * 2) - p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil) + p, err := coderdtest.GetProvisionerForTags(db, time.Now(), ws.OrganizationID, nil) require.NoError(t, err) coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) ticker <- tickTime @@ -1255,7 +1255,7 @@ func TestWorkspaceAutobuild(t *testing.T) { // Assert that autostart works when the workspace isn't dormant.. tickTime := sched.Next(ws.LatestBuild.CreatedAt) - p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil) + p, err := coderdtest.GetProvisionerForTags(db, time.Now(), ws.OrganizationID, nil) require.NoError(t, err) coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) tickCh <- tickTime @@ -1379,7 +1379,7 @@ func TestWorkspaceAutobuild(t *testing.T) { // Under normal circumstances this should result in a transition but // since our last build resulted in failure it should be skipped. tickTime := build.Job.CompletedAt.Add(time.Hour) - p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil) + p, err := coderdtest.GetProvisionerForTags(db, time.Now(), ws.OrganizationID, nil) require.NoError(t, err) coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) ticker <- tickTime @@ -1451,7 +1451,7 @@ func TestWorkspaceAutobuild(t *testing.T) { // Kick of an autostart build. tickTime := sched.Next(ws.LatestBuild.CreatedAt) - p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil) + p, err := coderdtest.GetProvisionerForTags(db, time.Now(), ws.OrganizationID, nil) require.NoError(t, err) coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) tickCh <- tickTime @@ -1549,7 +1549,7 @@ func TestWorkspaceAutobuild(t *testing.T) { next = sched.Next(next) clock.Set(next) - p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), ws.OrganizationID, nil) + p, err := coderdtest.GetProvisionerForTags(db, time.Now(), ws.OrganizationID, nil) require.NoError(t, err) coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), next) tickCh <- next @@ -2272,7 +2272,7 @@ func TestExecutorPrebuilds(t *testing.T) { coderdtest.MustWaitForProvisionersAvailable(t, db, prebuild, provisionerdserver.StaleInterval) tickTime := sched.Next(prebuild.LatestBuild.CreatedAt).Add(time.Minute) - p, err := coderdtest.GetProvisionerForTags(t, db, time.Now(), workspace.OrganizationID, nil) + p, err := coderdtest.GetProvisionerForTags(db, time.Now(), workspace.OrganizationID, nil) require.NoError(t, err) coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) From e48c7248e4d25e0e8f4053624f1c46cde01273a6 Mon Sep 17 00:00:00 2001 From: Callum Styan Date: Mon, 11 Aug 2025 22:09:16 +0000 Subject: [PATCH 10/16] hasValidProvisioner only needs tags Signed-off-by: Callum Styan --- coderd/autobuild/lifecycle_executor.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index 67e1046d9e383..8dae671191767 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -133,11 +133,11 @@ func (e *Executor) Run() { } // hasValidProvisioner checks whether there is at least one valid (non-stale, correct tags) provisioner -// based on t and the tags on templateVersionJob. -func (e *Executor) hasValidProvisioner(ctx context.Context, tx database.Store, t time.Time, ws database.Workspace, templateVersionJob database.ProvisionerJob) (bool, error) { +// based on time t and the tags maps (such as from a templateVersionJob). +func (e *Executor) hasValidProvisioner(ctx context.Context, tx database.Store, t time.Time, ws database.Workspace, tags map[string]string) (bool, error) { queryParams := database.GetProvisionerDaemonsByOrganizationParams{ OrganizationID: ws.OrganizationID, - WantTags: templateVersionJob.Tags, + WantTags: tags, } // nolint: gocritic // The user (in this case, the user/context for autostart builds) may not have the full @@ -148,20 +148,20 @@ func (e *Executor) hasValidProvisioner(ctx context.Context, tx database.Store, t return false, xerrors.Errorf("get provisioner daemons: %w", err) } - logger := e.log.With(slog.F("tags", templateVersionJob.Tags)) + logger := e.log.With(slog.F("tags", tags)) // Check if any provisioners are active (not stale) for _, pd := range provisionerDaemons { if pd.LastSeenAt.Valid { age := t.Sub(pd.LastSeenAt.Time) if age <= provisionerdserver.StaleInterval { - logger.Debug(ctx, "hasAvailableProvisioners: found active provisioner", + logger.Debug(ctx, "hasValidProvisioner: found active provisioner", slog.F("daemon_id", pd.ID), ) return true, nil } } } - logger.Debug(ctx, "hasAvailableProvisioners: no active provisioners found") + logger.Debug(ctx, "hasValidProvisioner: no active provisioners found") return false, nil } @@ -321,7 +321,7 @@ func (e *Executor) runOnce(t time.Time) Stats { } // Before creating the workspace build, check for available provisioners - hasProvisioners, err := e.hasValidProvisioner(e.ctx, tx, t, ws, templateVersionJob) + hasProvisioners, err := e.hasValidProvisioner(e.ctx, tx, t, ws, templateVersionJob.Tags) if err != nil { return xerrors.Errorf("check provisioner availability: %w", err) } From 883c748ac3cde5ed422c458b3913ffdf203a58f3 Mon Sep 17 00:00:00 2001 From: Callum Styan Date: Mon, 11 Aug 2025 22:38:13 +0000 Subject: [PATCH 11/16] more cleanup of function params Signed-off-by: Callum Styan --- coderd/autobuild/lifecycle_executor_test.go | 38 ++++++++++----------- coderd/coderdtest/coderdtest.go | 14 ++++---- enterprise/coderd/workspaces_test.go | 38 ++++++++++----------- 3 files changed, 45 insertions(+), 45 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go index 11f1d1978460e..b5928ccbd4323 100644 --- a/coderd/autobuild/lifecycle_executor_test.go +++ b/coderd/autobuild/lifecycle_executor_test.go @@ -61,7 +61,7 @@ func TestExecutorAutostartOK(t *testing.T) { // When: the autobuild executor ticks after the scheduled time go func() { tickTime := sched.Next(workspace.LatestBuild.CreatedAt) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime) tickCh <- tickTime close(tickCh) }() @@ -122,7 +122,7 @@ func TestMultipleLifecycleExecutors(t *testing.T) { require.NoError(t, err) // Get both clients to perform a lifecycle execution tick next := sched.Next(workspace.LatestBuild.CreatedAt) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), next) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, next) startCh := make(chan struct{}) go func() { @@ -261,7 +261,7 @@ func TestExecutorAutostartTemplateUpdated(t *testing.T) { // When: the autobuild executor ticks after the scheduled time go func() { tickTime := sched.Next(workspace.LatestBuild.CreatedAt) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime) tickCh <- tickTime close(tickCh) }() @@ -446,7 +446,7 @@ func TestExecutorAutostopOK(t *testing.T) { // When: the autobuild executor ticks *after* the deadline: go func() { tickTime := workspace.LatestBuild.Deadline.Time.Add(time.Minute) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime) tickCh <- tickTime close(tickCh) }() @@ -495,7 +495,7 @@ func TestExecutorAutostopExtend(t *testing.T) { // When: the autobuild executor ticks *after* the original deadline: go func() { tickTime := originalDeadline.Time.Add(time.Minute) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime) tickCh <- tickTime }() @@ -507,7 +507,7 @@ func TestExecutorAutostopExtend(t *testing.T) { // When: the autobuild executor ticks after the *new* deadline: go func() { tickTime := newDeadline.Add(time.Minute) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime) tickCh <- tickTime close(tickCh) }() @@ -701,7 +701,7 @@ func TestExecuteAutostopSuspendedUser(t *testing.T) { admin := coderdtest.CreateFirstUser(t, client) // Wait for provisioner to be available - coderdtest.MustWaitForProvisionersWithClient(t, client) + coderdtest.MustWaitForAnyProvisionerWithClient(t, client) version := coderdtest.CreateTemplateVersion(t, client, admin.OrganizationID, nil) coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) template := coderdtest.CreateTemplate(t, client, admin.OrganizationID, version.ID) @@ -808,7 +808,7 @@ func TestExecutorAutostartMultipleOK(t *testing.T) { // When: the autobuild executor ticks past the scheduled time go func() { tickTime := sched.Next(workspace.LatestBuild.CreatedAt) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime) tickCh <- tickTime tickCh2 <- tickTime close(tickCh) @@ -878,7 +878,7 @@ func TestExecutorAutostartWithParameters(t *testing.T) { // When: the autobuild executor ticks after the scheduled time go func() { tickTime := sched.Next(workspace.LatestBuild.CreatedAt) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime) tickCh <- tickTime close(tickCh) }() @@ -977,7 +977,7 @@ func TestExecutorAutostopTemplateDisabled(t *testing.T) { // When: the autobuild executor ticks after the workspace setting, but before the template setting: go func() { tickTime := workspace.LatestBuild.Job.CompletedAt.Add(45 * time.Minute) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime) tickCh <- tickTime }() @@ -989,7 +989,7 @@ func TestExecutorAutostopTemplateDisabled(t *testing.T) { // When: the autobuild executor ticks after the template setting: go func() { tickTime := workspace.LatestBuild.Job.CompletedAt.Add(61 * time.Minute) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime) tickCh <- tickTime close(tickCh) }() @@ -1020,7 +1020,7 @@ func TestExecutorRequireActiveVersion(t *testing.T) { }) ) // Wait for provisioner to be available - coderdtest.MustWaitForProvisioners(t, db) + coderdtest.MustWaitForAnyProvisioner(t, db) ctx := testutil.Context(t, testutil.WaitShort) owner := coderdtest.CreateFirstUser(t, ownerClient) @@ -1063,7 +1063,7 @@ func TestExecutorRequireActiveVersion(t *testing.T) { require.NoError(t, err) tickTime := sched.Next(ws.LatestBuild.CreatedAt) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime) ticker <- tickTime stats := <-statCh require.Len(t, stats.Transitions, 1) @@ -1227,7 +1227,7 @@ func TestNotifications(t *testing.T) { // Wait for workspace to become dormant notifyEnq.Clear() tickTime := workspace.LastUsedAt.Add(timeTilDormant * 3) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime) ticker <- tickTime _ = testutil.TryReceive(testutil.Context(t, testutil.WaitShort), t, statCh) @@ -1308,7 +1308,7 @@ func TestExecutorPrebuilds(t *testing.T) { // When: the autobuild executor ticks *after* the deadline: go func() { tickTime := prebuild.LatestBuild.Deadline.Time.Add(time.Minute) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime) tickCh <- tickTime }() @@ -1327,7 +1327,7 @@ func TestExecutorPrebuilds(t *testing.T) { // When: the autobuild executor ticks *after* the deadline: go func() { tickTime := workspace.LatestBuild.Deadline.Time.Add(time.Minute) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime) tickCh <- tickTime close(tickCh) }() @@ -1696,13 +1696,13 @@ func TestExecutorAutostartSkipsWhenNoProvisionersAvailable(t *testing.T) { workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop) // Wait for provisioner to be available for this specific workspace - coderdtest.MustWaitForProvisionersAvailable(t, db, workspace, provisionerdserver.StaleInterval) + coderdtest.MustWaitForProvisionersAvailable(t, db, workspace) p, err := coderdtest.GetProvisionerForTags(db, time.Now(), workspace.OrganizationID, provisionerDaemonTags) require.NoError(t, err, "Error getting provisioner for workspace") // Ensure the provisioner is stale staleTime := sched.Next(workspace.LatestBuild.CreatedAt).Add((-1 * provisionerdserver.StaleInterval) + -10*time.Second) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), staleTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, staleTime) // Trigger autobuild tickCh <- sched.Next(workspace.LatestBuild.CreatedAt) @@ -1715,7 +1715,7 @@ func TestExecutorAutostartSkipsWhenNoProvisionersAvailable(t *testing.T) { // Ensure the provisioner is NOT stale, and see if we get a successful state transition. notStaleTime := sched.Next(workspace.LatestBuild.CreatedAt).Add((-1 * provisionerdserver.StaleInterval) + 10*time.Second) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), notStaleTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, notStaleTime) // Trigger autobuild go func() { diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index 4299e4064ccad..b4c1e6c949919 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -1631,10 +1631,10 @@ func ctxWithProvisionerPermissions(ctx context.Context) context.Context { // UpdateProvisionerLastSeenAt updates the provisioner daemon's LastSeenAt timestamp // to the specified time to prevent it from appearing stale during autobuild operations -func UpdateProvisionerLastSeenAt(t *testing.T, db database.Store, id uuid.UUID, prev, tickTime time.Time) { +func UpdateProvisionerLastSeenAt(t *testing.T, db database.Store, id uuid.UUID, tickTime time.Time) { t.Helper() ctx := ctxWithProvisionerPermissions(context.Background()) - t.Logf("Updating provisioner %s LastSeenAt from %v to %v", id, prev, tickTime) + t.Logf("Updating provisioner %s LastSeenAt from %v to %v", id, tickTime) err := db.UpdateProvisionerDaemonLastSeenAt(ctx, database.UpdateProvisionerDaemonLastSeenAtParams{ ID: id, LastSeenAt: sql.NullTime{Time: tickTime, Valid: true}, @@ -1643,7 +1643,7 @@ func UpdateProvisionerLastSeenAt(t *testing.T, db database.Store, id uuid.UUID, t.Logf("Successfully updated provisioner LastSeenAt") } -func MustWaitForProvisioners(t *testing.T, db database.Store) { +func MustWaitForAnyProvisioner(t *testing.T, db database.Store) { t.Helper() ctx := ctxWithProvisionerPermissions(testutil.Context(t, testutil.WaitShort)) require.Eventually(t, func() bool { @@ -1652,7 +1652,7 @@ func MustWaitForProvisioners(t *testing.T, db database.Store) { }, testutil.WaitShort, testutil.IntervalFast) } -func MustWaitForProvisionersWithClient(t *testing.T, client *codersdk.Client) { +func MustWaitForAnyProvisionerWithClient(t *testing.T, client *codersdk.Client) { t.Helper() ctx := ctxWithProvisionerPermissions(testutil.Context(t, testutil.WaitShort)) require.Eventually(t, func() bool { @@ -1661,8 +1661,8 @@ func MustWaitForProvisionersWithClient(t *testing.T, client *codersdk.Client) { }, testutil.WaitShort, testutil.IntervalFast) } -// mustWaitForProvisionersAvailable waits for provisioners to be available for a specific workspace -func MustWaitForProvisionersAvailable(t *testing.T, db database.Store, workspace codersdk.Workspace, staleDuration time.Duration) uuid.UUID { +// MustWaitForProvisionersAvailable waits for provisioners to be available for a specific workspace. +func MustWaitForProvisionersAvailable(t *testing.T, db database.Store, workspace codersdk.Workspace) uuid.UUID { t.Helper() ctx := ctxWithProvisionerPermissions(testutil.Context(t, testutil.WaitShort)) id := uuid.UUID{} @@ -1699,7 +1699,7 @@ func MustWaitForProvisionersAvailable(t *testing.T, db database.Store, workspace for _, pd := range provisionerDaemons { if pd.LastSeenAt.Valid { age := now.Sub(pd.LastSeenAt.Time) - if age <= staleDuration { + if age <= provisionerdserver.StaleInterval { id = pd.ID return true // Found an active provisioner } diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index 0eef8536ef445..cc55fe40003c8 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -648,7 +648,7 @@ func TestWorkspaceAutobuild(t *testing.T) { p, err := coderdtest.GetProvisionerForTags(db, time.Now(), ws.OrganizationID, nil) require.NoError(t, err) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime) ticker <- tickTime stats := <-statCh // Expect workspace to transition to stopped state for breaching @@ -700,7 +700,7 @@ func TestWorkspaceAutobuild(t *testing.T) { p, err := coderdtest.GetProvisionerForTags(db, time.Now(), ws.OrganizationID, nil) require.NoError(t, err) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime) ticker <- tickTime stats := <-statCh // Expect no transitions since not enough time has elapsed. @@ -805,7 +805,7 @@ func TestWorkspaceAutobuild(t *testing.T) { p, err := coderdtest.GetProvisionerForTags(db, time.Now(), workspace.OrganizationID, nil) require.NoError(t, err) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime) ticker <- tickTime stats := <-statCh @@ -908,7 +908,7 @@ func TestWorkspaceAutobuild(t *testing.T) { tickTime := time.Now().Add(time.Hour) p, err := coderdtest.GetProvisionerForTags(db, time.Now(), workspaces[0].OrganizationID, nil) require.NoError(t, err) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime) ticker <- tickTime stats := <-statCh @@ -1053,7 +1053,7 @@ func TestWorkspaceAutobuild(t *testing.T) { tickTime := ws.LastUsedAt.Add(2 * inactiveTTL) p, err := coderdtest.GetProvisionerForTags(db, time.Now(), ws.OrganizationID, nil) require.NoError(t, err) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime) ticker <- tickTime stats := <-statCh // Expect no transitions since workspace is stopped. @@ -1107,7 +1107,7 @@ func TestWorkspaceAutobuild(t *testing.T) { tickTime := ws.LastUsedAt.Add(2 * transitionTTL) p, err := coderdtest.GetProvisionerForTags(db, time.Now(), ws.OrganizationID, nil) require.NoError(t, err) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime) ticker <- tickTime stats := <-statCh // Expect workspace to transition to stopped state for breaching @@ -1124,7 +1124,7 @@ func TestWorkspaceAutobuild(t *testing.T) { // Simulate the workspace being dormant beyond the threshold. tickTime2 := ws.DormantAt.Add(2 * transitionTTL) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime) ticker <- tickTime2 stats = <-statCh require.Len(t, stats.Transitions, 1) @@ -1192,7 +1192,7 @@ func TestWorkspaceAutobuild(t *testing.T) { tickTime := ws.DormantAt.Add(-dormantTTL * 2) p, err := coderdtest.GetProvisionerForTags(db, time.Now(), ws.OrganizationID, nil) require.NoError(t, err) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime) ticker <- tickTime stats := <-statCh // Expect no transitions since not enough time has elapsed. @@ -1205,7 +1205,7 @@ func TestWorkspaceAutobuild(t *testing.T) { // Simlute the workspace breaching the threshold. tickTime2 := ws.DormantAt.Add(dormantTTL * 2) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime2) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime2) ticker <- tickTime2 stats = <-statCh require.Len(t, stats.Transitions, 1) @@ -1257,7 +1257,7 @@ func TestWorkspaceAutobuild(t *testing.T) { tickTime := sched.Next(ws.LatestBuild.CreatedAt) p, err := coderdtest.GetProvisionerForTags(db, time.Now(), ws.OrganizationID, nil) require.NoError(t, err) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime) tickCh <- tickTime stats := <-statsCh require.Len(t, stats.Errors, 0) @@ -1279,7 +1279,7 @@ func TestWorkspaceAutobuild(t *testing.T) { // We should see the workspace get stopped now. tickTime2 := ws.LastUsedAt.Add(inactiveTTL * 2) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime) tickCh <- tickTime2 stats = <-statsCh require.Len(t, stats.Errors, 0) @@ -1381,7 +1381,7 @@ func TestWorkspaceAutobuild(t *testing.T) { tickTime := build.Job.CompletedAt.Add(time.Hour) p, err := coderdtest.GetProvisionerForTags(db, time.Now(), ws.OrganizationID, nil) require.NoError(t, err) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime) ticker <- tickTime stats := <-statCh require.Len(t, stats.Transitions, 0) @@ -1389,7 +1389,7 @@ func TestWorkspaceAutobuild(t *testing.T) { // Simulate ticking a day after the workspace was last attempted to // be deleted. This should result in an attempt. tickTime2 := build.Job.CompletedAt.Add(time.Hour * 25) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime2) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime2) ticker <- tickTime2 stats = <-statCh require.Len(t, stats.Transitions, 1) @@ -1453,7 +1453,7 @@ func TestWorkspaceAutobuild(t *testing.T) { tickTime := sched.Next(ws.LatestBuild.CreatedAt) p, err := coderdtest.GetProvisionerForTags(db, time.Now(), ws.OrganizationID, nil) require.NoError(t, err) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime) tickCh <- tickTime stats := <-statsCh require.Len(t, stats.Errors, 0) @@ -1483,7 +1483,7 @@ func TestWorkspaceAutobuild(t *testing.T) { // Force an autostart transition again. tickTime2 := sched.Next(firstBuild.CreatedAt) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime) tickCh <- tickTime2 stats = <-statsCh require.Len(t, stats.Errors, 0) @@ -1551,7 +1551,7 @@ func TestWorkspaceAutobuild(t *testing.T) { clock.Set(next) p, err := coderdtest.GetProvisionerForTags(db, time.Now(), ws.OrganizationID, nil) require.NoError(t, err) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), next) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, next) tickCh <- next stats := <-statsCh ws = coderdtest.MustWorkspace(t, client, ws.ID) @@ -2266,15 +2266,15 @@ func TestExecutorPrebuilds(t *testing.T) { require.True(t, workspace.NextStartAt.Equal(*prebuild.NextStartAt)) // Wait for provisioner to be registered - coderdtest.MustWaitForProvisioners(t, db) + coderdtest.MustWaitForAnyProvisioner(t, db) // Wait for provisioner to be available for this specific workspace - coderdtest.MustWaitForProvisionersAvailable(t, db, prebuild, provisionerdserver.StaleInterval) + coderdtest.MustWaitForProvisionersAvailable(t, db, prebuild) tickTime := sched.Next(prebuild.LatestBuild.CreatedAt).Add(time.Minute) p, err := coderdtest.GetProvisionerForTags(db, time.Now(), workspace.OrganizationID, nil) require.NoError(t, err) - coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, time.Now(), tickTime) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime) // When: the autobuild executor ticks at the scheduled time go func() { From 3c90c6d645ae851f6d44cca6c9e7d6982e53c32c Mon Sep 17 00:00:00 2001 From: Callum Styan Date: Mon, 11 Aug 2025 22:47:08 +0000 Subject: [PATCH 12/16] already waiting for the workspaces provisioner, don't need to wait for any before that Signed-off-by: Callum Styan --- enterprise/coderd/workspaces_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index cc55fe40003c8..9b2cc4e7569cb 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -2265,9 +2265,6 @@ func TestExecutorPrebuilds(t *testing.T) { // Then: the claimed workspace should inherit and respect that same NextStartAt require.True(t, workspace.NextStartAt.Equal(*prebuild.NextStartAt)) - // Wait for provisioner to be registered - coderdtest.MustWaitForAnyProvisioner(t, db) - // Wait for provisioner to be available for this specific workspace coderdtest.MustWaitForProvisionersAvailable(t, db, prebuild) From 5a6493c5ad086e989697d5fa8cc87646b0b404e7 Mon Sep 17 00:00:00 2001 From: Callum Styan Date: Tue, 12 Aug 2025 03:38:28 +0000 Subject: [PATCH 13/16] fix print statement missed in earlier commit Signed-off-by: Callum Styan --- coderd/coderdtest/coderdtest.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index b4c1e6c949919..488b5139b310c 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -1634,7 +1634,7 @@ func ctxWithProvisionerPermissions(ctx context.Context) context.Context { func UpdateProvisionerLastSeenAt(t *testing.T, db database.Store, id uuid.UUID, tickTime time.Time) { t.Helper() ctx := ctxWithProvisionerPermissions(context.Background()) - t.Logf("Updating provisioner %s LastSeenAt from %v to %v", id, tickTime) + t.Logf("Updating provisioner %s LastSeenAt to %v", id, tickTime) err := db.UpdateProvisionerDaemonLastSeenAt(ctx, database.UpdateProvisionerDaemonLastSeenAtParams{ ID: id, LastSeenAt: sql.NullTime{Time: tickTime, Valid: true}, From c5f0d41a511373a5b337e3724583662c332e59d8 Mon Sep 17 00:00:00 2001 From: Callum Styan Date: Tue, 12 Aug 2025 17:30:50 +0000 Subject: [PATCH 14/16] address more review feedback; mostly around provisioner closers in the new test Signed-off-by: Callum Styan --- coderd/autobuild/lifecycle_executor_test.go | 31 ++++++++++++++++----- coderd/coderdtest/coderdtest.go | 2 +- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go index b5928ccbd4323..7ed591e197a91 100644 --- a/coderd/autobuild/lifecycle_executor_test.go +++ b/coderd/autobuild/lifecycle_executor_test.go @@ -37,6 +37,10 @@ import ( "github.com/coder/coder/v2/testutil" ) +func TestMain(m *testing.M) { + goleak.VerifyTestMain(m, testutil.GoleakOptions...) +} + func TestExecutorAutostartOK(t *testing.T) { t.Parallel() @@ -1663,10 +1667,6 @@ func mustWorkspaceParameters(t *testing.T, client *codersdk.Client, workspaceID require.NotEmpty(t, buildParameters) } -func TestMain(m *testing.M) { - goleak.VerifyTestMain(m, testutil.GoleakOptions...) -} - func TestExecutorAutostartSkipsWhenNoProvisionersAvailable(t *testing.T) { t.Parallel() @@ -1680,11 +1680,19 @@ func TestExecutorAutostartSkipsWhenNoProvisionersAvailable(t *testing.T) { // We can't overwrite owner or scope as there's a `provisionersdk.MutateTags` function that has restrictions on those. provisionerDaemonTags := map[string]string{"test-tag": "asdf"} t.Logf("Setting provisioner daemon tags: %v", provisionerDaemonTags) - client, db := coderdtest.NewWithDatabase(t, &coderdtest.Options{ + + db, ps := dbtestutil.NewDB(t) + client, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{ + Database: db, + Pubsub: ps, + IncludeProvisionerDaemon: false, AutobuildTicker: tickCh, - IncludeProvisionerDaemon: true, AutobuildStats: statsCh, - ProvisionerDaemonTags: provisionerDaemonTags, + }) + + daemon1Closer := coderdtest.NewTaggedProvisionerDaemon(t, api, "name", provisionerDaemonTags) + t.Cleanup(func() { + _ = daemon1Closer.Close() }) // Create workspace with autostart enabled and matching provisioner tags @@ -1700,6 +1708,8 @@ func TestExecutorAutostartSkipsWhenNoProvisionersAvailable(t *testing.T) { p, err := coderdtest.GetProvisionerForTags(db, time.Now(), workspace.OrganizationID, provisionerDaemonTags) require.NoError(t, err, "Error getting provisioner for workspace") + daemon1Closer.Close() + // Ensure the provisioner is stale staleTime := sched.Next(workspace.LatestBuild.CreatedAt).Add((-1 * provisionerdserver.StaleInterval) + -10*time.Second) coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, staleTime) @@ -1713,7 +1723,14 @@ func TestExecutorAutostartSkipsWhenNoProvisionersAvailable(t *testing.T) { // UpdateProvisionerLastSeenAt call above. assert.Len(t, stats.Transitions, 0, "should not create builds when no provisioners available") + daemon2Closer := coderdtest.NewTaggedProvisionerDaemon(t, api, "name", provisionerDaemonTags) + t.Cleanup(func() { + _ = daemon2Closer.Close() + }) + // Ensure the provisioner is NOT stale, and see if we get a successful state transition. + p, err = coderdtest.GetProvisionerForTags(db, time.Now(), workspace.OrganizationID, provisionerDaemonTags) + require.NoError(t, err, "Error getting provisioner for workspace") notStaleTime := sched.Next(workspace.LatestBuild.CreatedAt).Add((-1 * provisionerdserver.StaleInterval) + 10*time.Second) coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, notStaleTime) diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index 488b5139b310c..b0786b3807e27 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -1706,7 +1706,7 @@ func MustWaitForProvisionersAvailable(t *testing.T, db database.Store, workspace } } return false // No active provisioners found - }, testutil.WaitShort, testutil.IntervalFast) + }, testutil.WaitLong, testutil.IntervalFast) return id } From 224acfcb95fbfb976a6c46ab2cf09414d8d4c40e Mon Sep 17 00:00:00 2001 From: Callum Styan Date: Tue, 12 Aug 2025 21:46:36 +0000 Subject: [PATCH 15/16] remove MustWaitForAnyProvisionerWithClient Signed-off-by: Callum Styan --- coderd/autobuild/lifecycle_executor_test.go | 8 ++++---- coderd/coderdtest/coderdtest.go | 9 --------- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go index 7ed591e197a91..b15a4ee7fd154 100644 --- a/coderd/autobuild/lifecycle_executor_test.go +++ b/coderd/autobuild/lifecycle_executor_test.go @@ -694,9 +694,9 @@ func TestExecuteAutostopSuspendedUser(t *testing.T) { t.Parallel() var ( - tickCh = make(chan time.Time) - statsCh = make(chan autobuild.Stats) - client = coderdtest.New(t, &coderdtest.Options{ + tickCh = make(chan time.Time) + statsCh = make(chan autobuild.Stats) + client, db = coderdtest.NewWithDatabase(t, &coderdtest.Options{ AutobuildTicker: tickCh, IncludeProvisionerDaemon: true, AutobuildStats: statsCh, @@ -705,7 +705,7 @@ func TestExecuteAutostopSuspendedUser(t *testing.T) { admin := coderdtest.CreateFirstUser(t, client) // Wait for provisioner to be available - coderdtest.MustWaitForAnyProvisionerWithClient(t, client) + coderdtest.MustWaitForAnyProvisioner(t, db) version := coderdtest.CreateTemplateVersion(t, client, admin.OrganizationID, nil) coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) template := coderdtest.CreateTemplate(t, client, admin.OrganizationID, version.ID) diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index b0786b3807e27..0de5dbb710a0e 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -1652,15 +1652,6 @@ func MustWaitForAnyProvisioner(t *testing.T, db database.Store) { }, testutil.WaitShort, testutil.IntervalFast) } -func MustWaitForAnyProvisionerWithClient(t *testing.T, client *codersdk.Client) { - t.Helper() - ctx := ctxWithProvisionerPermissions(testutil.Context(t, testutil.WaitShort)) - require.Eventually(t, func() bool { - daemons, err := client.ProvisionerDaemons(ctx) - return err == nil && len(daemons) > 0 - }, testutil.WaitShort, testutil.IntervalFast) -} - // MustWaitForProvisionersAvailable waits for provisioners to be available for a specific workspace. func MustWaitForProvisionersAvailable(t *testing.T, db database.Store, workspace codersdk.Workspace) uuid.UUID { t.Helper() From 3275c57903610e251e2ebef5f60a7e5fa8bd7a64 Mon Sep 17 00:00:00 2001 From: Callum Styan Date: Thu, 14 Aug 2025 15:31:54 +0000 Subject: [PATCH 16/16] not sure how this was passing before, the prebuilds NextStartAt time is nil here Signed-off-by: Callum Styan --- enterprise/coderd/workspaces_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index ff1729c1bb92b..424aba3082dec 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -2242,9 +2242,6 @@ func TestPrebuildsAutobuild(t *testing.T) { workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop) coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID) - // Then: the claimed workspace should inherit and respect that same NextStartAt - require.True(t, workspace.NextStartAt.Equal(*prebuild.NextStartAt)) - // Wait for provisioner to be available for this specific workspace coderdtest.MustWaitForProvisionersAvailable(t, db, prebuild)