Skip to content

feat: add default autostart and ttl for new workspaces #1632

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
May 23, 2022
Merged

feat: add default autostart and ttl for new workspaces #1632

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
5c092be
Dockerfile: add tzdata
johnstcn May 20, 2022
391f527
database: add autostart_schedule and ttl to InsertWorkspace; make gen
johnstcn May 20, 2022
98561bf
coderd: workspaces: consume additional fields of CreateWorkspaceRequest
johnstcn May 20, 2022
0169315
fixup! coderd: workspaces: consume additional fields of CreateWorkspa…
johnstcn May 20, 2022
dea541f
fixup! fixup! coderd: workspaces: consume additional fields of Create…
johnstcn May 20, 2022
d621272
autobuild: fix unit tests
johnstcn May 20, 2022
2f7b939
cli: update: add support for TTL and autostart_schedule
johnstcn May 20, 2022
813a64f
cli: create: add unit tests
johnstcn May 20, 2022
1fbc0f2
lint
johnstcn May 20, 2022
142ad37
coder: use embedded tzdata instead
johnstcn May 20, 2022
0f348cd
autobuild: fix unit test that only runs with a real db
johnstcn May 20, 2022
32a86f1
address PR comments
johnstcn May 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
autobuild: fix unit tests
  • Loading branch information
@johnstcn
johnstcn committed May 20, 2022
commit d621272f1660bc84da69addab4a6ff8669aeb76f
101 changes: 31 additions & 70 deletions coderd/autobuild/executor/lifecycle_executor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,6 @@ func TestExecutorAutostartOK(t *testing.T) {
// Given: workspace is stopped
workspace = mustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)

// Given: the workspace initially has autostart disabled
require.Empty(t, workspace.AutostartSchedule)

// When: we enable workspace autostart
sched, err := schedule.Weekly("* * * * *")
require.NoError(t, err)
Expand Down Expand Up @@ -77,9 +74,6 @@ func TestExecutorAutostartTemplateUpdated(t *testing.T) {
// Given: workspace is stopped
workspace = mustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)

// Given: the workspace initially has autostart disabled
require.Empty(t, workspace.AutostartSchedule)

// Given: the workspace template has been updated
orgs, err := client.OrganizationsByUser(ctx, workspace.OwnerID.String())
require.NoError(t, err)
Expand Down Expand Up @@ -131,9 +125,6 @@ func TestExecutorAutostartAlreadyRunning(t *testing.T) {
// Given: we ensure the workspace is running
require.Equal(t, codersdk.WorkspaceTransitionStart, workspace.LatestBuild.Transition)

// Given: the workspace initially has autostart disabled
require.Empty(t, workspace.AutostartSchedule)

// When: we enable workspace autostart
sched, err := schedule.Weekly("* * * * *")
require.NoError(t, err)
Expand Down Expand Up @@ -164,15 +155,17 @@ func TestExecutorAutostartNotEnabled(t *testing.T) {
IncludeProvisionerD: true,
})
// Given: we have a user with a workspace
workspace = mustProvisionWorkspace(t, client)
workspace = mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) {
cwr.AutostartSchedule = nil
})
)

// Given: workspace does not have autostart enabled
require.Empty(t, workspace.AutostartSchedule)

// Given: workspace is stopped
workspace = mustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)

// Given: the workspace has autostart disabled
require.Empty(t, workspace.AutostartSchedule)

// When: the autobuild executor ticks
go func() {
tickCh <- time.Now().UTC().Add(time.Minute)
Expand All @@ -190,29 +183,18 @@ func TestExecutorAutostopOK(t *testing.T) {
t.Parallel()

var (
ctx = context.Background()
err error
tickCh = make(chan time.Time)
client = coderdtest.New(t, &coderdtest.Options{
AutobuildTicker: tickCh,
IncludeProvisionerD: true,
})
// Given: we have a user with a workspace
workspace = mustProvisionWorkspace(t, client)
ttl = time.Minute
ttl = *workspace.TTL
)
// Given: workspace is running
require.Equal(t, codersdk.WorkspaceTransitionStart, workspace.LatestBuild.Transition)

// Given: the workspace initially has autostop disabled
require.Nil(t, workspace.TTL)

// When: we enable workspace autostop
require.NoError(t, err)
require.NoError(t, client.UpdateWorkspaceTTL(ctx, workspace.ID, codersdk.UpdateWorkspaceTTLRequest{
TTL: &ttl,
}))

// When: the autobuild executor ticks *after* the TTL:
go func() {
tickCh <- time.Now().UTC().Add(ttl + time.Minute)
Expand All @@ -231,41 +213,32 @@ func TestExecutorAutostopAlreadyStopped(t *testing.T) {
t.Parallel()

var (
ctx = context.Background()
err error
tickCh = make(chan time.Time)
client = coderdtest.New(t, &coderdtest.Options{
AutobuildTicker: tickCh,
IncludeProvisionerD: true,
})
// Given: we have a user with a workspace
workspace = mustProvisionWorkspace(t, client)
ttl = time.Minute
// Given: we have a user with a workspace (disabling autostart)
workspace = mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) {
cwr.AutostartSchedule = nil
})
ttl = *workspace.TTL
)

// Given: workspace is stopped
workspace = mustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)

// Given: the workspace initially has autostop disabled
require.Nil(t, workspace.TTL)

// When: we set the TTL on the workspace
require.NoError(t, err)
require.NoError(t, client.UpdateWorkspaceTTL(ctx, workspace.ID, codersdk.UpdateWorkspaceTTLRequest{
TTL: &ttl,
}))

// When: the autobuild executor ticks past the TTL
go func() {
tickCh <- time.Now().UTC().Add(ttl)
tickCh <- time.Now().UTC().Add(ttl + time.Minute)
close(tickCh)
}()

// Then: the workspace should not be stopped.
<-time.After(5 * time.Second)
ws := mustWorkspace(t, client, workspace.ID)
require.Equal(t, workspace.LatestBuild.ID, ws.LatestBuild.ID, "expected no further workspace builds to occur")
require.Equal(t, codersdk.WorkspaceTransitionStop, ws.LatestBuild.Transition, "expected workspace not to be running")
require.Equal(t, workspace.LatestBuild.ID, ws.LatestBuild.ID, "expected no further workspace builds to occur")
}

func TestExecutorAutostopNotEnabled(t *testing.T) {
Expand All @@ -277,17 +250,19 @@ func TestExecutorAutostopNotEnabled(t *testing.T) {
AutobuildTicker: tickCh,
IncludeProvisionerD: true,
})
// Given: we have a user with a workspace
workspace = mustProvisionWorkspace(t, client)
// Given: we have a user with a workspace that has no TTL set
workspace = mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) {
cwr.TTL = nil
})
)

// Given: workspace has no TTL set
require.Nil(t, workspace.TTL)

// Given: workspace is running
require.Equal(t, codersdk.WorkspaceTransitionStart, workspace.LatestBuild.Transition)

// Given: the workspace has autostop disabled
require.Empty(t, workspace.TTL)

// When: the autobuild executor ticks
// When: the autobuild executor ticks past the TTL
go func() {
tickCh <- time.Now().UTC().Add(time.Minute)
close(tickCh)
Expand Down Expand Up @@ -315,9 +290,6 @@ func TestExecutorWorkspaceDeleted(t *testing.T) {
workspace = mustProvisionWorkspace(t, client)
)

// Given: the workspace initially has autostart disabled
require.Empty(t, workspace.AutostartSchedule)

// When: we enable workspace autostart
sched, err := schedule.Weekly("* * * * *")
require.NoError(t, err)
Expand Down Expand Up @@ -352,16 +324,15 @@ func TestExecutorWorkspaceAutostartTooEarly(t *testing.T) {
AutobuildTicker: tickCh,
IncludeProvisionerD: true,
})
// Given: we have a user with a workspace
workspace = mustProvisionWorkspace(t, client)
futureTime = time.Now().Add(time.Hour)
futureTimeCron = fmt.Sprintf("%d %d * * *", futureTime.Minute(), futureTime.Hour())
// Given: we have a user with a workspace configured to autostart some time in the future
workspace = mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) {
cwr.AutostartSchedule = &futureTimeCron
})
)

// Given: the workspace initially has autostart disabled
require.Empty(t, workspace.AutostartSchedule)

// When: we enable workspace autostart with some time in the future
futureTime := time.Now().Add(time.Hour)
futureTimeCron := fmt.Sprintf("%d %d * * *", futureTime.Minute(), futureTime.Hour())
sched, err := schedule.Weekly(futureTimeCron)
require.NoError(t, err)
require.NoError(t, client.UpdateWorkspaceAutostart(ctx, workspace.ID, codersdk.UpdateWorkspaceAutostartRequest{
Expand All @@ -385,26 +356,16 @@ func TestExecutorWorkspaceTTLTooEarly(t *testing.T) {
t.Parallel()

var (
ctx = context.Background()
tickCh = make(chan time.Time)
client = coderdtest.New(t, &coderdtest.Options{
AutobuildTicker: tickCh,
IncludeProvisionerD: true,
})
// Given: we have a user with a workspace
workspace = mustProvisionWorkspace(t, client)
ttl = time.Hour
)

// Given: the workspace initially has TTL unset
require.Nil(t, workspace.TTL)

// When: we set the TTL to some time in the distant future
require.NoError(t, client.UpdateWorkspaceTTL(ctx, workspace.ID, codersdk.UpdateWorkspaceTTLRequest{
TTL: &ttl,
}))

// When: the autobuild executor ticks
// When: the autobuild executor ticks before the TTL
go func() {
tickCh <- time.Now().UTC()
close(tickCh)
Expand Down Expand Up @@ -479,13 +440,13 @@ func TestExecutorAutostartMultipleOK(t *testing.T) {
require.True(t, builds[1].CreatedAt.After(builds[2].CreatedAt))
}

func mustProvisionWorkspace(t *testing.T, client *codersdk.Client) codersdk.Workspace {
func mustProvisionWorkspace(t *testing.T, client *codersdk.Client, mut ...func(*codersdk.CreateWorkspaceRequest)) codersdk.Workspace {
t.Helper()
user := coderdtest.CreateFirstUser(t, client)
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
ws := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
ws := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID, mut...)
coderdtest.AwaitWorkspaceBuildJob(t, client, ws.LatestBuild.ID)
return mustWorkspace(t, client, ws.ID)
}
Expand Down
4 changes: 4 additions & 0 deletions coderd/autobuild/schedule/schedule.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ func Weekly(raw string) (*Schedule, error) {
return nil, xerrors.Errorf("expected *cron.SpecSchedule but got %T", specSched)
}

if schedule.Location == time.Local {
return nil, xerrors.Errorf("schedules scoped to time.Local are not supported")
}
Comment on lines +58 to +60
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOTE: added an extra check here, we don't want these sneaking in.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL: time.Local != time.UTC even when TZ=UTC, cool!


// Strip the leading CRON_TZ prefix so we just store the cron string.
// The timezone info is available in SpecSchedule.
cronStr := raw
Expand Down
7 changes: 7 additions & 0 deletions coderd/autobuild/schedule/schedule_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@ func Test_Weekly(t *testing.T) {
expectedTz: "UTC",
expectedString: "CRON_TZ=UTC 30 9 * * 1-5",
},
{
name: "time.Local will bite you",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

spec: "CRON_TZ=Local 30 9 * * 1-5",
at: time.Time{},
expectedNext: time.Time{},
expectedError: "schedules scoped to time.Local are not supported",
},
{
name: "invalid schedule",
spec: "asdfasdfasdfsd",
Expand Down