Skip to content

feat: add lifecycle.Executor to manage autostart and autostop #1183

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 28 commits into from
May 11, 2022
Merged

feat: add lifecycle.Executor to manage autostart and autostop #1183

Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
a145d6d
feat: add lifecycle.Executor to autostart workspaces.
johnstcn Apr 19, 2022
8f401ca
refactor: do not expose Store in coderdtest.Options
johnstcn Apr 26, 2022
6d8f5fe
fixup! refactor: do not expose Store in coderdtest.Options
johnstcn Apr 26, 2022
cfd0d1e
stop accessing db directly, only query workspaces with autostart enabled
johnstcn Apr 26, 2022
ce63810
refactor unit tests, add tests for autostop
johnstcn Apr 26, 2022
579f362
make the new tests pass with some refactoring
johnstcn Apr 30, 2022
6e88f67
gitignore *.swp
johnstcn Apr 30, 2022
2b1a383
remove unused methods
johnstcn Apr 30, 2022
f31588e
fixup! remove unused methods
johnstcn Apr 30, 2022
80e0581
fix: range over channel, add continue to default switch case
johnstcn May 9, 2022
d176478
add test for deleted workspace
johnstcn May 9, 2022
bd97c1a
workspaces.sql: remove unused methods
johnstcn May 10, 2022
0931d25
unexport test helper methods
johnstcn May 10, 2022
faebe2e
chore: rename package autostart/lifecycle to lifecycle/executor
johnstcn May 10, 2022
abc0854
add test to ensure workspaces are not autostarted before time
johnstcn May 10, 2022
e53946a
wire up executor to coderd
johnstcn May 10, 2022
364a27c
fix: executor: skip workspaces whose last build was not successful
johnstcn May 10, 2022
e96414f
address PR comments
johnstcn May 11, 2022
b5bf50e
add goleak TestMain
johnstcn May 11, 2022
d37cc2b
fmt
johnstcn May 11, 2022
d11f5d7
mustTransitionWorkspace should return the updated workspace
johnstcn May 11, 2022
f6388b4
remove usage of require.Eventually/Never which is flaky on Windows
johnstcn May 11, 2022
fd0f8a3
make lifecycle executor spawn a new goroutine automatically
johnstcn May 11, 2022
7b6f2e1
rename unit tests
johnstcn May 11, 2022
a7143bd
s/doBuild/build
johnstcn May 11, 2022
7d9b696
rename parent package lifecycle to autobuild
johnstcn May 11, 2022
5cba737
add unit test for behaviour with an updated template
johnstcn May 11, 2022
7627372
add ticket to reference TODO
johnstcn May 11, 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
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,4 @@ site/out/
.terraform/

.vscode/*.log
**/*.swp
2 changes: 1 addition & 1 deletion cli/autostart.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (

"github.com/spf13/cobra"

"github.com/coder/coder/coderd/autostart/schedule"
"github.com/coder/coder/coderd/lifecycle/schedule"
"github.com/coder/coder/codersdk"
)

Expand Down
2 changes: 1 addition & 1 deletion cli/autostop.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (

"github.com/spf13/cobra"

"github.com/coder/coder/coderd/autostart/schedule"
"github.com/coder/coder/coderd/lifecycle/schedule"
"github.com/coder/coder/codersdk"
)

Expand Down
6 changes: 6 additions & 0 deletions cli/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import (
"github.com/coder/coder/coderd/database/databasefake"
"github.com/coder/coder/coderd/devtunnel"
"github.com/coder/coder/coderd/gitsshkey"
"github.com/coder/coder/coderd/lifecycle/executor"
"github.com/coder/coder/coderd/turnconn"
"github.com/coder/coder/codersdk"
"github.com/coder/coder/cryptorand"
Expand Down Expand Up @@ -343,6 +344,11 @@ func server() *cobra.Command {
return xerrors.Errorf("notify systemd: %w", err)
}

lifecyclePoller := time.NewTicker(30 * time.Second)
defer lifecyclePoller.Stop()
lifecycleExecutor := executor.New(cmd.Context(), options.Database, logger, lifecyclePoller.C)
go lifecycleExecutor.Run()
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment(general): Your code looks great. I'm feeling a bit lost in understanding all that's happening in this "Start a Coder server" RunE because it's growing quite large with some branching and Goroutines. I'm wondering if that is just a me thing, or if maybe the complexity here is growing large. To understand what I mean, this diff in isolation looks harmless. In the grand scheme of initializing everything within this RunE, though, it's harder to really evaluate.

I'm under the impression we could extract out some initialization steps to named functions and call them in a pipeline, but maybe that's just a me thing and not the general preferred approach for Go + Cobra. Do you have any comments to that effect?

Regardless, this comment is not meant for your code/this PR, just a general observation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed, I'd be in favour of extracting some initialization logic to make things slightly more concise, just would want to be careful to avoid too many layers of indirection.


// Because the graceful shutdown includes cleaning up workspaces in dev mode, we're
// going to make it harder to accidentally skip the graceful shutdown by hitting ctrl+c
// two or more times. So the stopChan is unlimited in size and we don't call
Expand Down
17 changes: 16 additions & 1 deletion coderd/coderdtest/coderdtest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ import (
"github.com/coder/coder/coderd/database/databasefake"
"github.com/coder/coder/coderd/database/postgres"
"github.com/coder/coder/coderd/gitsshkey"
"github.com/coder/coder/coderd/lifecycle/executor"
"github.com/coder/coder/coderd/turnconn"
"github.com/coder/coder/codersdk"
"github.com/coder/coder/cryptorand"
Expand All @@ -57,6 +58,7 @@ type Options struct {
GoogleTokenValidator *idtoken.Validator
SSHKeygenAlgorithm gitsshkey.Algorithm
APIRateLimit int
LifecycleTicker <-chan time.Time
}

// New constructs an in-memory coderd instance and returns
Expand All @@ -72,6 +74,11 @@ func New(t *testing.T, options *Options) *codersdk.Client {
options.GoogleTokenValidator, err = idtoken.NewValidator(ctx, option.WithoutAuthentication())
require.NoError(t, err)
}
if options.LifecycleTicker == nil {
ticker := make(chan time.Time)
options.LifecycleTicker = ticker
t.Cleanup(func() { close(ticker) })
}

// This can be hotswapped for a live database instance.
db := databasefake.New()
Expand All @@ -96,8 +103,16 @@ func New(t *testing.T, options *Options) *codersdk.Client {
})
}

srv := httptest.NewUnstartedServer(nil)
ctx, cancelFunc := context.WithCancel(context.Background())
lifecycleExecutor := executor.New(
ctx,
db,
slogtest.Make(t, nil).Named("lifecycle.executor").Leveled(slog.LevelDebug),
options.LifecycleTicker,
)
go lifecycleExecutor.Run()
Copy link
Member

Choose a reason for hiding this comment

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

Since this Run function is always called in a goroutine, could we do that automatically for the caller in New()?


srv := httptest.NewUnstartedServer(nil)
srv.Config.BaseContext = func(_ net.Listener) context.Context {
return ctx
}
Expand Down
14 changes: 14 additions & 0 deletions coderd/database/databasefake/databasefake.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,20 @@ func (q *fakeQuerier) GetWorkspaceByOwnerIDAndName(_ context.Context, arg databa
return database.Workspace{}, sql.ErrNoRows
}

func (q *fakeQuerier) GetWorkspacesAutostartAutostop(_ context.Context) ([]database.Workspace, error) {
q.mutex.RLock()
defer q.mutex.RUnlock()
workspaces := make([]database.Workspace, 0)
for _, ws := range q.workspaces {
if ws.AutostartSchedule.String != "" {
workspaces = append(workspaces, ws)
} else if ws.AutostopSchedule.String != "" {
workspaces = append(workspaces, ws)
}
}
return workspaces, nil
}

func (q *fakeQuerier) GetWorkspaceOwnerCountsByTemplateIDs(_ context.Context, templateIDs []uuid.UUID) ([]database.GetWorkspaceOwnerCountsByTemplateIDsRow, error) {
q.mutex.RLock()
defer q.mutex.RUnlock()
Expand Down
1 change: 1 addition & 0 deletions coderd/database/querier.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

49 changes: 49 additions & 0 deletions coderd/database/queries.sql.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions coderd/database/queries/workspaces.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,20 @@ SELECT * FROM workspaces WHERE organization_id = $1 AND deleted = $2;
-- name: GetWorkspacesByOrganizationIDs :many
SELECT * FROM workspaces WHERE organization_id = ANY(@ids :: uuid [ ]) AND deleted = @deleted;

-- name: GetWorkspacesAutostartAutostop :many
SELECT
*
FROM
workspaces
WHERE
deleted = false
AND
(
autostart_schedule <> ''
OR
autostop_schedule <> ''
);

-- name: GetWorkspacesByTemplateID :many
SELECT
*
Expand Down
Loading