Skip to content

Commit 9971794

Browse files
cstyancstyan
committed
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 <callumstyan@gmail.com>
1 parent 858d44a commit 9971794

File tree

3 files changed

+119
-63
lines changed

3 files changed

+119
-63
lines changed

‎coderd/autobuild/lifecycle_executor.go

Lines changed: 37 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"strings"
1010
"sync"
1111
"sync/atomic"
12+
"testing"
1213
"time"
1314

1415
"github.com/dustin/go-humanize"
@@ -26,14 +27,16 @@ import (
2627
"github.com/coder/coder/v2/coderd/database/dbauthz"
2728
"github.com/coder/coder/v2/coderd/database/dbtime"
2829
"github.com/coder/coder/v2/coderd/database/provisionerjobs"
29-
"github.com/coder/coder/v2/coderd/provisionerdserver"
3030
"github.com/coder/coder/v2/coderd/database/pubsub"
3131
"github.com/coder/coder/v2/coderd/notifications"
32+
"github.com/coder/coder/v2/coderd/provisionerdserver"
3233
"github.com/coder/coder/v2/coderd/schedule"
3334
"github.com/coder/coder/v2/coderd/wsbuilder"
3435
"github.com/coder/coder/v2/codersdk"
3536
)
3637

38+
const TestingStaleInterval = time.Second * 5
39+
3740
// Executor automatically starts or stops workspaces.
3841
type Executor struct {
3942
ctx context.Context
@@ -53,6 +56,9 @@ type Executor struct {
5356
experiments codersdk.Experiments
5457

5558
metrics executorMetrics
59+
60+
// Skip provisioner availability check (should only be true for *some* tests)
61+
SkipProvisionerCheck bool
5662
}
5763

5864
type executorMetrics struct {
@@ -132,28 +138,37 @@ func (e *Executor) Run() {
132138
}()
133139
}
134140

135-
// Add this function to check for available provisioners
136141
func (e *Executor) hasAvailableProvisioners(ctx context.Context, tx database.Store, ws database.Workspace, templateVersionJob database.ProvisionerJob) (bool, error) {
137-
// Get eligible provisioner daemons for this workspace's template
138-
provisionerDaemons, err := tx.GetProvisionerDaemonsByOrganization(ctx, database.GetProvisionerDaemonsByOrganizationParams{
139-
OrganizationID: ws.OrganizationID,
140-
WantTags: templateVersionJob.Tags,
141-
})
142-
if err != nil {
143-
return false, xerrors.Errorf("get provisioner daemons: %w", err)
144-
}
145-
146-
// Check if any provisioners are active (not stale)
147-
now := dbtime.Now()
148-
for _, pd := range provisionerDaemons {
149-
if pd.LastSeenAt.Valid {
150-
age := now.Sub(pd.LastSeenAt.Time)
151-
if age <= provisionerdserver.StaleInterval {
152-
return true, nil
153-
}
154-
}
155-
}
156-
return false, nil
142+
if e.SkipProvisionerCheck {
143+
return true, nil
144+
}
145+
146+
// Use a shorter stale interval for tests
147+
staleInterval := provisionerdserver.StaleInterval
148+
if testing.Testing() {
149+
staleInterval = TestingStaleInterval
150+
}
151+
152+
// Get eligible provisioner daemons for this workspace's template
153+
provisionerDaemons, err := tx.GetProvisionerDaemonsByOrganization(ctx, database.GetProvisionerDaemonsByOrganizationParams{
154+
OrganizationID: ws.OrganizationID,
155+
WantTags: templateVersionJob.Tags,
156+
})
157+
if err != nil {
158+
return false, xerrors.Errorf("get provisioner daemons: %w", err)
159+
}
160+
161+
// Check if any provisioners are active (not stale)
162+
now := dbtime.Now()
163+
for _, pd := range provisionerDaemons {
164+
if pd.LastSeenAt.Valid {
165+
age := now.Sub(pd.LastSeenAt.Time)
166+
if age <= staleInterval {
167+
return true, nil
168+
}
169+
}
170+
}
171+
return false, nil
157172
}
158173

159174
func (e *Executor) runOnce(t time.Time) Stats {

‎coderd/autobuild/lifecycle_executor_test.go

Lines changed: 74 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1585,45 +1585,78 @@ func TestMain(m *testing.M) {
15851585
}
15861586

15871587
func TestExecutorAutostartSkipsWhenNoProvisionersAvailable(t *testing.T) {
1588-
t.Parallel()
1589-
1590-
var (
1591-
sched = mustSchedule(t, "CRON_TZ=UTC 0 * * * *")
1592-
tickCh = make(chan time.Time)
1593-
statsCh = make(chan autobuild.Stats)
1594-
)
1595-
1596-
// Create client with provisioner daemon
1597-
client, provisionerCloser := coderdtest.NewWithProvisionerCloser(t, &coderdtest.Options{
1598-
AutobuildTicker: tickCh,
1599-
IncludeProvisionerDaemon: true,
1600-
AutobuildStats: statsCh,
1601-
})
1602-
1603-
// Create workspace with autostart enabled
1604-
workspace := mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) {
1605-
cwr.AutostartSchedule = ptr.Ref(sched.String())
1606-
})
1607-
1608-
// Stop the workspace while provisioner is available
1609-
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID,
1610-
codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)
1611-
1612-
// Now shut down the provisioner daemon
1613-
err := provisionerCloser.Close()
1614-
require.NoError(t, err)
1615-
1616-
// Trigger autobuild after scheduled time
1617-
go func() {
1618-
tickCh <- sched.Next(workspace.LatestBuild.CreatedAt)
1619-
close(tickCh)
1620-
}()
1621-
1622-
// Wait for executor to run
1623-
stats := <-statsCh
1624-
require.Len(t, stats.Errors, 0, "should not have errors")
1625-
1626-
// Verify workspace is still stopped
1627-
workspace = coderdtest.MustWorkspace(t, client, workspace.ID)
1628-
require.Equal(t, codersdk.WorkspaceTransitionStop, workspace.LatestBuild.Transition)
1588+
t.Parallel()
1589+
1590+
db, ps := dbtestutil.NewDB(t)
1591+
var (
1592+
sched = mustSchedule(t, "CRON_TZ=UTC 0 * * * *")
1593+
tickCh = make(chan time.Time)
1594+
statsCh = make(chan autobuild.Stats)
1595+
)
1596+
1597+
// Create client with provisioner closer
1598+
client, provisionerCloser := coderdtest.NewWithProvisionerCloser(t, &coderdtest.Options{
1599+
AutobuildTicker: tickCh,
1600+
IncludeProvisionerDaemon: true,
1601+
AutobuildStats: statsCh,
1602+
Database: db,
1603+
Pubsub: ps,
1604+
SkipProvisionerCheck: ptr.Ref(false),
1605+
})
1606+
1607+
// Create workspace with autostart enabled
1608+
workspace := mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) {
1609+
cwr.AutostartSchedule = ptr.Ref(sched.String())
1610+
})
1611+
1612+
// Stop the workspace while provisioner is available
1613+
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID,
1614+
codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)
1615+
1616+
// Wait for provisioner to be registered
1617+
ctx := testutil.Context(t, testutil.WaitShort)
1618+
require.Eventually(t, func() bool {
1619+
daemons, err := db.GetProvisionerDaemons(ctx)
1620+
return err == nil && len(daemons) > 0
1621+
}, testutil.WaitShort, testutil.IntervalFast)
1622+
1623+
// Now shut down the provisioner daemon
1624+
err := provisionerCloser.Close()
1625+
require.NoError(t, err)
1626+
1627+
// Debug: check what's in the database
1628+
daemons, err := db.GetProvisionerDaemons(ctx)
1629+
require.NoError(t, err)
1630+
t.Logf("After close: found %d daemons", len(daemons))
1631+
for i, daemon := range daemons {
1632+
t.Logf("Daemon %d: ID=%s, Name=%s, LastSeen=%v", i, daemon.ID, daemon.Name, daemon.LastSeenAt)
1633+
}
1634+
1635+
// Wait for provisioner to become stale (LastSeenAt > StaleInterval ago)
1636+
require.Eventually(t, func() bool {
1637+
daemons, err := db.GetProvisionerDaemons(ctx)
1638+
if err != nil || len(daemons) == 0 {
1639+
return false
1640+
}
1641+
1642+
now := time.Now()
1643+
for _, daemon := range daemons {
1644+
if daemon.LastSeenAt.Valid {
1645+
age := now.Sub(daemon.LastSeenAt.Time)
1646+
if age > autobuild.TestingStaleInterval {
1647+
return true // Provisioner is now stale
1648+
}
1649+
}
1650+
}
1651+
return false
1652+
}, testutil.WaitLong, testutil.IntervalMedium) // Use longer timeout since we need to wait for staleness
1653+
1654+
// Trigger autobuild
1655+
go func() {
1656+
tickCh <- sched.Next(workspace.LatestBuild.CreatedAt)
1657+
close(tickCh)
1658+
}()
1659+
1660+
stats := <-statsCh
1661+
assert.Len(t, stats.Transitions, 0, "should not create builds when no provisioners available")
16291662
}

‎coderd/coderdtest/coderdtest.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,7 @@ type Options struct {
183183
OIDCConvertKeyCache cryptokeys.SigningKeycache
184184
Clock quartz.Clock
185185
TelemetryReporter telemetry.Reporter
186+
SkipProvisionerCheck *bool // Pointer so we can detect if it's set
186187
}
187188

188189
// 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
386387
options.NotificationsEnqueuer,
387388
experiments,
388389
).WithStatsChannel(options.AutobuildStats)
390+
391+
skipProvisionerCheck := true
392+
if options.SkipProvisionerCheck != nil {
393+
skipProvisionerCheck = *options.SkipProvisionerCheck
394+
}
395+
lifecycleExecutor.SkipProvisionerCheck = skipProvisionerCheck
396+
389397
lifecycleExecutor.Run()
390398

391399
jobReaperTicker := time.NewTicker(options.DeploymentValues.JobReaperDetectorInterval.Value())

0 commit comments

Comments
 (0)