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

Changes from 1 commit
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
Prev Previous commit
Next Next commit
s/doBuild/build
  • Loading branch information
@johnstcn
johnstcn committed May 11, 2022
commit a7143bdbe7b74db42772ed96a4a0678b985b2a73
4 changes: 2 additions & 2 deletions coderd/lifecycle/executor/lifecycle_executor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ func (e *Executor) runOnce(t time.Time) error {
slog.F("transition", validTransition),
)

if err := doBuild(e.ctx, db, ws, validTransition, priorHistory, priorJob); err != nil {
if err := build(e.ctx, db, ws, validTransition, priorHistory, priorJob); err != nil {
e.log.Error(e.ctx, "unable to transition workspace",
slog.F("workspace_id", ws.ID),
slog.F("transition", validTransition),
Expand All @@ -145,7 +145,7 @@ func (e *Executor) runOnce(t time.Time) error {
}

// TODO(cian): this function duplicates most of api.postWorkspaceBuilds. Refactor.
Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't be opposed to exposing a function like: database.CreateWorkspaceBuild. I'm not a massive fan either, because it certainly conflicts with database.InsertWorkspaceBuild. 🤷

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm pausing extracting a function here because we only have two datapoints right now; I'd like to see at least one more before doing so.

Copy link
Member

Choose a reason for hiding this comment

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

I like that!

Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion(if-minor): Just one suggestion: ticket > todo. Entirely up to you, won't hold up on that.

Copy link
Member Author

Choose a reason for hiding this comment

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

@vapurrmaid good call: #1401

func doBuild(ctx context.Context, store database.Store, workspace database.Workspace, trans database.WorkspaceTransition, priorHistory database.WorkspaceBuild, priorJob database.ProvisionerJob) error {
func build(ctx context.Context, store database.Store, workspace database.Workspace, trans database.WorkspaceTransition, priorHistory database.WorkspaceBuild, priorJob database.ProvisionerJob) error {
template, err := store.GetTemplateByID(ctx, workspace.TemplateID)
if err != nil {
return xerrors.Errorf("get workspace template: %w", err)
Expand Down