- Notifications
You must be signed in to change notification settings - Fork928
feat: update workspace deadline when workspace ttl updated#2165
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Merged
Uh oh!
There was an error while loading.Please reload this page.
Merged
Changes fromall commits
Commits
Show all changes
20 commits Select commitHold shift + click to select a range
5c443de RED: update unit test with new desired behaviour
johnstcnc2127c1 cli: warn user upon ttl-triggered workspace shutdown
johnstcn07320a4 GREEN: do the thing
johnstcn7846dd8 REFACTOR: coderd: putWorkspaceTTL: wrap in tx
johnstcn6d7187d update unit tests
johnstcndaa9a5f feat: WorkspaceScheduleForm: show shutdown time when editing workspac…
johnstcnfe79c0c make it prettier
johnstcn85da57b hopefully fix unit tests
johnstcn8c38a74 Merge remote-tracking branch 'origin/main' into cj/1783/updating-work…
johnstcna4c07df Address PR comments
johnstcn582873a Merge origin/main
greyscaled9a7c623 chore: WorkspaceScheduleForm: add unit tests
johnstcn0278a93 WorkspaceScheduleForm: refactor ttlShutdownAt
johnstcna372dce apply suggestions from code review
johnstcnac1d29a fixup! apply suggestions from code review
johnstcn23b474c yarn lint; more pr comments
johnstcnde28477 bump ci
johnstcna52d815 Merge remote-tracking branch 'origin/main' into cj/1783/updating-work…
johnstcnfd86474 mind your language
johnstcn2b9f93c fumpt
johnstcnFile filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
There are no files selected for viewing
41 changes: 41 additions & 0 deletionscli/ttl.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,14 @@ | ||
| package cli | ||
| import ( | ||
| "errors" | ||
| "fmt" | ||
| "time" | ||
| "github.com/spf13/cobra" | ||
| "golang.org/x/xerrors" | ||
| "github.com/coder/coder/cli/cliui" | ||
| "github.com/coder/coder/codersdk" | ||
| ) | ||
| @@ -89,6 +91,30 @@ func ttlset() *cobra.Command { | ||
| _, _ = fmt.Fprintf(cmd.OutOrStdout(), "warning: ttl rounded down to %s\n", truncated) | ||
| } | ||
| if changed, newDeadline := changedNewDeadline(workspace, truncated); changed { | ||
| // For the purposes of the user, "less than a minute" is essentially the same as "immediately". | ||
| timeRemaining := time.Until(newDeadline).Truncate(time.Minute) | ||
| humanRemaining := "in " + timeRemaining.String() | ||
| if timeRemaining <= 0 { | ||
| humanRemaining = "immediately" | ||
| } | ||
| _, err = cliui.Prompt(cmd, cliui.PromptOptions{ | ||
| Text: fmt.Sprintf( | ||
| "Workspace %q will be stopped %s. Are you sure?", | ||
| workspace.Name, | ||
| humanRemaining, | ||
| ), | ||
Comment on lines +97 to +106 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. praise: wonderful strings! | ||
| Default: "yes", | ||
| IsConfirm: true, | ||
Comment on lines +101 to +108 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. review: we can skip the prompt if | ||
| }) | ||
| if err != nil { | ||
| if errors.Is(err, cliui.Canceled) { | ||
| return nil | ||
| } | ||
| return err | ||
| } | ||
| } | ||
| millis := truncated.Milliseconds() | ||
| if err = client.UpdateWorkspaceTTL(cmd.Context(), workspace.ID, codersdk.UpdateWorkspaceTTLRequest{ | ||
| TTLMillis: &millis, | ||
| @@ -131,3 +157,18 @@ func ttlunset() *cobra.Command { | ||
| }, | ||
| } | ||
| } | ||
| func changedNewDeadline(ws codersdk.Workspace, newTTL time.Duration) (changed bool, newDeadline time.Time) { | ||
| if ws.LatestBuild.Transition != codersdk.WorkspaceTransitionStart { | ||
| // not running | ||
| return false, newDeadline | ||
| } | ||
| if ws.LatestBuild.Job.CompletedAt == nil { | ||
| // still building | ||
| return false, newDeadline | ||
| } | ||
| newDeadline = ws.LatestBuild.Job.CompletedAt.Add(newTTL) | ||
| return true, newDeadline | ||
| } | ||
77 changes: 53 additions & 24 deletionscli/ttl_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
33 changes: 28 additions & 5 deletionscoderd/autobuild/executor/lifecycle_executor_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
48 changes: 44 additions & 4 deletionscoderd/workspaces.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -566,17 +566,57 @@ func (api *API) putWorkspaceTTL(rw http.ResponseWriter, r *http.Request) { | ||||
| return | ||||
| } | ||||
| err = api.Database.InTx(func(s database.Store) error { | ||||
| if err := s.UpdateWorkspaceTTL(r.Context(), database.UpdateWorkspaceTTLParams{ | ||||
| ID: workspace.ID, | ||||
| Ttl: dbTTL, | ||||
| }); err != nil { | ||||
| return xerrors.Errorf("update workspace TTL: %w", err) | ||||
| } | ||||
| // Also extend the workspace deadline if the workspace is running | ||||
| latestBuild, err := s.GetLatestWorkspaceBuildByWorkspaceID(r.Context(), workspace.ID) | ||||
| if err != nil { | ||||
| return xerrors.Errorf("get latest workspace build: %w", err) | ||||
| } | ||||
| if latestBuild.Transition != database.WorkspaceTransitionStart { | ||||
| return nil // nothing to do | ||||
| } | ||||
| if latestBuild.UpdatedAt.IsZero() { | ||||
| // Build in progress; provisionerd should update with the new TTL. | ||||
| return nil | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Question: What happens if a user starts a workspace with one TTL, but tries to update the TTL before the build is complete? As a user, I'd expect the new TTL to be in-effect once the build has completed or receive an error that TTL cannot be updated (yet). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. See coder/coderd/provisionerdaemons.go Line 598 in567e4af
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others.Learn more. Ooh yeah that makes sense, thanks for clarifying! | ||||
| } | ||||
| var newDeadline time.Time | ||||
| if dbTTL.Valid { | ||||
| newDeadline = latestBuild.UpdatedAt.Add(time.Duration(dbTTL.Int64)) | ||||
| } | ||||
| if err := s.UpdateWorkspaceBuildByID( | ||||
| r.Context(), | ||||
| database.UpdateWorkspaceBuildByIDParams{ | ||||
| ID: latestBuild.ID, | ||||
| UpdatedAt: latestBuild.UpdatedAt, | ||||
| ProvisionerState: latestBuild.ProvisionerState, | ||||
| Deadline: newDeadline, | ||||
| }, | ||||
| ); err != nil { | ||||
| return xerrors.Errorf("update workspace deadline: %w", err) | ||||
| } | ||||
| return nil | ||||
| }) | ||||
johnstcn marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||||
| if err != nil { | ||||
| httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ | ||||
| Message: "Errorupdating workspacetime until shutdown!", | ||||
| Detail: err.Error(), | ||||
| }) | ||||
| return | ||||
| } | ||||
| httpapi.Write(rw, http.StatusOK, nil) | ||||
| } | ||||
| func (api *API) putExtendWorkspace(rw http.ResponseWriter, r *http.Request) { | ||||
Oops, something went wrong.
Uh oh!
There was an error while loading.Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.