Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

fix: coderd: decouple ttl and deadline#2282

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
johnstcn merged 11 commits intomainfromcj/2229/decouple-ws-deadline-from-ttl-again
Jun 14, 2022
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
Show all changes
11 commits
Select commitHold shift + click to select a range
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
5 changes: 3 additions & 2 deletionscli/bump.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -58,8 +58,9 @@ func bump() *cobra.Command {

_, _ = fmt.Fprintf(
cmd.OutOrStdout(),
"Workspace %q will now stop at %s\n", workspace.Name,
newDeadline.Format(time.RFC822),
"Workspace %q will now stop at %s on %s\n", workspace.Name,
newDeadline.Format(timeFormat),
newDeadline.Format(dateFormat),
)

return nil
Expand Down
6 changes: 6 additions & 0 deletionscli/constants.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
package cli

const (
timeFormat = "3:04:05 PM MST"
dateFormat = "Jan 2, 2006"
)
5 changes: 4 additions & 1 deletioncli/list.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -127,14 +127,17 @@ func hasExtension(ws codersdk.Workspace) (bool, time.Duration) {
if ws.LatestBuild.Transition != codersdk.WorkspaceTransitionStart {
return false, 0
}
if ws.LatestBuild.Job.CompletedAt == nil {
return false, 0
}
if ws.LatestBuild.Deadline.IsZero() {
return false, 0
}
if ws.TTLMillis == nil {
return false, 0
}
ttl := time.Duration(*ws.TTLMillis) * time.Millisecond
delta := ws.LatestBuild.Deadline.Add(-ttl).Sub(ws.LatestBuild.CreatedAt)
delta := ws.LatestBuild.Deadline.Add(-ttl).Sub(*ws.LatestBuild.Job.CompletedAt)
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

review: this was causing us to measure deadline extensions incorrectly, resulting in things like8h (+3m) if your workspace took 3 minutes to buidl.

if delta < time.Minute {
return false, 0
}
Expand Down
62 changes: 21 additions & 41 deletionscli/ttl.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,14 +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/coderd/autobuild/schedule"
"github.com/coder/coder/coderd/util/ptr"
"github.com/coder/coder/codersdk"
)

Expand DownExpand Up@@ -91,37 +91,32 @@ 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,
),
Default: "yes",
IsConfirm: true,
})
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,
}); err != nil {
return xerrors.Errorf("update workspace ttl: %w", err)
}

if ptr.NilOrEmpty(workspace.AutostartSchedule) {
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "%q will shut down %s after start.\n", workspace.Name, truncated)
return nil
}

sched, err := schedule.Weekly(*workspace.AutostartSchedule)
if err != nil {
return xerrors.Errorf("parse workspace schedule: %w", err)
}

nextShutdown := sched.Next(time.Now()).Add(truncated).In(sched.Location())
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "%q will shut down at %s on %s (%s after start).\n",
workspace.Name,
nextShutdown.Format(timeFormat),
nextShutdown.Format(dateFormat),
truncated,
)

_, _ = fmt.Fprintf(cmd.OutOrStdout(), "NOTE: this will only take effect the next time the workspace is started.\n")
return nil
},
}
Expand DownExpand Up@@ -157,18 +152,3 @@ 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
}
4 changes: 0 additions & 4 deletionscli/ttl_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -3,7 +3,6 @@ package cli_test
import (
"bytes"
"context"
"fmt"
"strings"
"testing"
"time"
Expand DownExpand Up@@ -109,9 +108,6 @@ func TestTTL(t *testing.T) {
assert.NoError(t, err, "unexpected error")
}()

pty.ExpectMatch(fmt.Sprintf("warning: ttl rounded down to %s", ttl.Truncate(time.Minute)))
pty.ExpectMatch(fmt.Sprintf("Workspace %q will be stopped in 8h29m0s. Are you sure?", workspace.Name))
pty.WriteLine("yes")
// Ensure ttl updated
updated, err := client.Workspace(ctx, workspace.ID)
require.NoError(t, err, "fetch updated workspace")
Expand Down
8 changes: 3 additions & 5 deletionscoderd/autobuild/executor/lifecycle_executor.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -85,11 +85,9 @@ func (e *Executor) runOnce(t time.Time) Stats {
// is what we compare against when performing autostop operations, rounded down
// to the minute.
//
// NOTE: Currently, if a workspace build is created with a given TTL and then
// the user either changes or unsets the TTL, the deadline for the workspace
// build will not have changed. So, autostop will still happen at the
// original TTL value from when the workspace build was created.
// Whether this is expected behavior from a user's perspective is not yet known.
// NOTE: If a workspace build is created with a given TTL and then the user either
// changes or unsets the TTL, the deadline for the workspace build will not
// have changed. This behavior is as expected per #2229.
Comment on lines +88 to +90
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

review: this is now the expected behaviour until we're told otherwise!

eligibleWorkspaces, err := db.GetWorkspacesAutostart(e.ctx)
if err != nil {
return xerrors.Errorf("get eligible workspaces for autostart or autostop: %w", err)
Expand Down
29 changes: 17 additions & 12 deletionscoderd/autobuild/executor/lifecycle_executor_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -308,8 +308,7 @@ func TestExecutorAutostopNotEnabled(t *testing.T) {
require.NoError(t, err)
require.Nil(t, workspace.TTLMillis)

// TODO(cian): need to stop and start the workspace as we do not update the deadline yet
// see: https://github.com/coder/coder/issues/1783
// TODO(cian): need to stop and start the workspace as we do not update the deadline. See: #2229
coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop)
coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStop, database.WorkspaceTransitionStart)

Expand DownExpand Up@@ -440,41 +439,47 @@ func TestExecutorWorkspaceAutostopNoWaitChangedMyMind(t *testing.T) {
err := client.UpdateWorkspaceTTL(ctx, workspace.ID, codersdk.UpdateWorkspaceTTLRequest{TTLMillis: nil})
require.NoError(t, err)

// Then: the deadline should be thezero value
// Then: the deadline shouldstillbe theoriginal value
updated := coderdtest.MustWorkspace(t, client, workspace.ID)
assert.Zero(t, updated.LatestBuild.Deadline)
assert.WithinDuration(t,workspace.LatestBuild.Deadline,updated.LatestBuild.Deadline, time.Minute)

// When: the autobuild executor ticks after the original deadline
go func() {
tickCh <- workspace.LatestBuild.Deadline.Add(time.Minute)
}()

// Then: the workspace shouldnotstop
// Then: the workspace should stop
stats := <-statsCh
assert.NoError(t, stats.Error)
assert.Len(t, stats.Transitions, 0)
assert.Len(t, stats.Transitions, 1)
assert.Equal(t, stats.Transitions[workspace.ID], database.WorkspaceTransitionStop)

// Wait for stop to complete
updated = coderdtest.MustWorkspace(t, client, workspace.ID)
_ = coderdtest.AwaitWorkspaceBuildJob(t, client, updated.LatestBuild.ID)

// Start the workspace again
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStop, database.WorkspaceTransitionStart)

// Given: the user changes their mind again and wants to enable auto-stop
newTTL := 8 * time.Hour
expectedDeadline := workspace.LatestBuild.UpdatedAt.Add(newTTL)
err = client.UpdateWorkspaceTTL(ctx, workspace.ID, codersdk.UpdateWorkspaceTTLRequest{TTLMillis: ptr.Ref(newTTL.Milliseconds())})
require.NoError(t, err)

// Then: the deadline shouldbe updated based ontheTTL
// Then: the deadline shouldremain atthezero value
updated = coderdtest.MustWorkspace(t, client, workspace.ID)
assert.WithinDuration(t,expectedDeadline,updated.LatestBuild.Deadline, time.Minute)
assert.Zero(t, updated.LatestBuild.Deadline)

// When: the relentless onward march of time continues
go func() {
tickCh <- workspace.LatestBuild.Deadline.Add(newTTL + time.Minute)
close(tickCh)
}()

// Then: the workspace should stop
// Then: the workspace shouldnotstop
stats = <-statsCh
assert.NoError(t, stats.Error)
assert.Len(t, stats.Transitions, 1)
assert.Equal(t, stats.Transitions[workspace.ID], database.WorkspaceTransitionStop)
assert.Len(t, stats.Transitions, 0)
}

func TestExecutorAutostartMultipleOK(t *testing.T) {
Expand Down
91 changes: 33 additions & 58 deletionscoderd/workspaces.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -31,6 +31,8 @@ import (
"github.com/coder/coder/codersdk"
)

const workspaceDefaultTTL = 12 * time.Hour

func (api *API) workspace(rw http.ResponseWriter, r *http.Request) {
workspace := httpmw.WorkspaceParam(r)
if !api.Authorize(rw, r, rbac.ActionRead, workspace) {
Expand DownExpand Up@@ -289,8 +291,8 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req
}

if !dbTTL.Valid {
// Default to template maximum when creating a new workspace
dbTTL = sql.NullInt64{Valid: true, Int64: template.MaxTtl}
// Default tomin(12 hours,template maximum). Just defaulting to template maximum can be surprising.
dbTTL = sql.NullInt64{Valid: true, Int64:min(template.MaxTtl, int64(workspaceDefaultTTL))}
}

workspace, err := api.Database.GetWorkspaceByOwnerIDAndName(r.Context(), database.GetWorkspaceByOwnerIDAndNameParams{
Expand DownExpand Up@@ -511,75 +513,41 @@ func (api *API) putWorkspaceTTL(rw http.ResponseWriter, r *http.Request) {
return
}

template, err := api.Database.GetTemplateByID(r.Context(), workspace.TemplateID)
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: "Error fetching workspace template!",
})
return
}
var validErrs []httpapi.Error

dbTTL, err := validWorkspaceTTLMillis(req.TTLMillis, time.Duration(template.MaxTtl))
if err != nil {
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
Message: "Invalid workspace TTL.",
Detail: err.Error(),
Validations: []httpapi.Error{
{
Field: "ttl_ms",
Detail: err.Error(),
},
},
})
return
}
err := api.Database.InTx(func(s database.Store) error {
template, err := s.GetTemplateByID(r.Context(), workspace.TemplateID)
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: "Error fetching workspace template!",
})
return xerrors.Errorf("fetch workspace template: %w", err)
}

err = api.Database.InTx(func(s database.Store) error {
dbTTL, err := validWorkspaceTTLMillis(req.TTLMillis, time.Duration(template.MaxTtl))
if err != nil {
validErrs = append(validErrs, httpapi.Error{Field: "ttl_ms", Detail: err.Error()})
return err
}
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
}

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
})

if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: "Error updating workspace time until shutdown!",
Detail: err.Error(),
code := http.StatusInternalServerError
if len(validErrs) > 0 {
code = http.StatusBadRequest
}
httpapi.Write(rw, code, httpapi.Response{
Message: "Error updating workspace time until shutdown!",
Validations: validErrs,
Detail: err.Error(),
})
return
}
Expand DownExpand Up@@ -1024,3 +992,10 @@ func splitQueryParameterByDelimiter(query string, delimiter rune, maintainQuotes

return parts
}

func min(x, y int64) int64 {
if x < y {
return x
}
return y
}
Loading
[8]ページ先頭
©2009-2025 Movatter.jp