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

Commitc28b7ec

Browse files
authored
fix: coderd: decouple ttl and deadline (#2282)
This commit makes the following changes:- Partially reverts the changes of feat: update workspace deadline when workspace ttl updated#2165, making the deadline of a running workspace build independant of TTL, once started.- CLI: updating a workspace TTL no longer updates the deadline of the workspace.- UI: updating a workspace TTL no longer updates the deadline of the workspace.- Drive-by: API: When creating a workspace, default TTL to min(12 hours, template max_ttl) if not instructed otherwise.- Drive-by: CLI: list: measure workspace extension correctly (+X in last column) from the time the provisioner job was completed- Drive-by: WorkspaceSchedule: show timezone of schedule if it is set, defaulting to dayjs guess otherwise.- Drive-by: WorkspaceScheduleForm: fixed an issue where deleting the "TTL" value in the form would show the text "Your workspace will shut down a few seconds after start".
1 parent2513167 commitc28b7ec

File tree

14 files changed

+152
-336
lines changed

14 files changed

+152
-336
lines changed

‎cli/bump.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,9 @@ func bump() *cobra.Command {
5858

5959
_,_=fmt.Fprintf(
6060
cmd.OutOrStdout(),
61-
"Workspace %q will now stop at %s\n",workspace.Name,
62-
newDeadline.Format(time.RFC822),
61+
"Workspace %q will now stop at %s on %s\n",workspace.Name,
62+
newDeadline.Format(timeFormat),
63+
newDeadline.Format(dateFormat),
6364
)
6465

6566
returnnil

‎cli/constants.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
package cli
2+
3+
const (
4+
timeFormat="3:04:05 PM MST"
5+
dateFormat="Jan 2, 2006"
6+
)

‎cli/list.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,14 +127,17 @@ func hasExtension(ws codersdk.Workspace) (bool, time.Duration) {
127127
ifws.LatestBuild.Transition!=codersdk.WorkspaceTransitionStart {
128128
returnfalse,0
129129
}
130+
ifws.LatestBuild.Job.CompletedAt==nil {
131+
returnfalse,0
132+
}
130133
ifws.LatestBuild.Deadline.IsZero() {
131134
returnfalse,0
132135
}
133136
ifws.TTLMillis==nil {
134137
returnfalse,0
135138
}
136139
ttl:=time.Duration(*ws.TTLMillis)*time.Millisecond
137-
delta:=ws.LatestBuild.Deadline.Add(-ttl).Sub(ws.LatestBuild.CreatedAt)
140+
delta:=ws.LatestBuild.Deadline.Add(-ttl).Sub(*ws.LatestBuild.Job.CompletedAt)
138141
ifdelta<time.Minute {
139142
returnfalse,0
140143
}

‎cli/ttl.go

Lines changed: 21 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
package cli
22

33
import (
4-
"errors"
54
"fmt"
65
"time"
76

87
"github.com/spf13/cobra"
98
"golang.org/x/xerrors"
109

11-
"github.com/coder/coder/cli/cliui"
10+
"github.com/coder/coder/coderd/autobuild/schedule"
11+
"github.com/coder/coder/coderd/util/ptr"
1212
"github.com/coder/coder/codersdk"
1313
)
1414

@@ -91,37 +91,32 @@ func ttlset() *cobra.Command {
9191
_,_=fmt.Fprintf(cmd.OutOrStdout(),"warning: ttl rounded down to %s\n",truncated)
9292
}
9393

94-
ifchanged,newDeadline:=changedNewDeadline(workspace,truncated);changed {
95-
// For the purposes of the user, "less than a minute" is essentially the same as "immediately".
96-
timeRemaining:=time.Until(newDeadline).Truncate(time.Minute)
97-
humanRemaining:="in "+timeRemaining.String()
98-
iftimeRemaining<=0 {
99-
humanRemaining="immediately"
100-
}
101-
_,err=cliui.Prompt(cmd, cliui.PromptOptions{
102-
Text:fmt.Sprintf(
103-
"Workspace %q will be stopped %s. Are you sure?",
104-
workspace.Name,
105-
humanRemaining,
106-
),
107-
Default:"yes",
108-
IsConfirm:true,
109-
})
110-
iferr!=nil {
111-
iferrors.Is(err,cliui.Canceled) {
112-
returnnil
113-
}
114-
returnerr
115-
}
116-
}
117-
11894
millis:=truncated.Milliseconds()
11995
iferr=client.UpdateWorkspaceTTL(cmd.Context(),workspace.ID, codersdk.UpdateWorkspaceTTLRequest{
12096
TTLMillis:&millis,
12197
});err!=nil {
12298
returnxerrors.Errorf("update workspace ttl: %w",err)
12399
}
124100

101+
ifptr.NilOrEmpty(workspace.AutostartSchedule) {
102+
_,_=fmt.Fprintf(cmd.OutOrStdout(),"%q will shut down %s after start.\n",workspace.Name,truncated)
103+
returnnil
104+
}
105+
106+
sched,err:=schedule.Weekly(*workspace.AutostartSchedule)
107+
iferr!=nil {
108+
returnxerrors.Errorf("parse workspace schedule: %w",err)
109+
}
110+
111+
nextShutdown:=sched.Next(time.Now()).Add(truncated).In(sched.Location())
112+
_,_=fmt.Fprintf(cmd.OutOrStdout(),"%q will shut down at %s on %s (%s after start).\n",
113+
workspace.Name,
114+
nextShutdown.Format(timeFormat),
115+
nextShutdown.Format(dateFormat),
116+
truncated,
117+
)
118+
119+
_,_=fmt.Fprintf(cmd.OutOrStdout(),"NOTE: this will only take effect the next time the workspace is started.\n")
125120
returnnil
126121
},
127122
}
@@ -157,18 +152,3 @@ func ttlunset() *cobra.Command {
157152
},
158153
}
159154
}
160-
161-
funcchangedNewDeadline(ws codersdk.Workspace,newTTL time.Duration) (changedbool,newDeadline time.Time) {
162-
ifws.LatestBuild.Transition!=codersdk.WorkspaceTransitionStart {
163-
// not running
164-
returnfalse,newDeadline
165-
}
166-
167-
ifws.LatestBuild.Job.CompletedAt==nil {
168-
// still building
169-
returnfalse,newDeadline
170-
}
171-
172-
newDeadline=ws.LatestBuild.Job.CompletedAt.Add(newTTL)
173-
returntrue,newDeadline
174-
}

‎cli/ttl_test.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package cli_test
33
import (
44
"bytes"
55
"context"
6-
"fmt"
76
"strings"
87
"testing"
98
"time"
@@ -109,9 +108,6 @@ func TestTTL(t *testing.T) {
109108
assert.NoError(t,err,"unexpected error")
110109
}()
111110

112-
pty.ExpectMatch(fmt.Sprintf("warning: ttl rounded down to %s",ttl.Truncate(time.Minute)))
113-
pty.ExpectMatch(fmt.Sprintf("Workspace %q will be stopped in 8h29m0s. Are you sure?",workspace.Name))
114-
pty.WriteLine("yes")
115111
// Ensure ttl updated
116112
updated,err:=client.Workspace(ctx,workspace.ID)
117113
require.NoError(t,err,"fetch updated workspace")

‎coderd/autobuild/executor/lifecycle_executor.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,9 @@ func (e *Executor) runOnce(t time.Time) Stats {
8585
// is what we compare against when performing autostop operations, rounded down
8686
// to the minute.
8787
//
88-
// NOTE: Currently, if a workspace build is created with a given TTL and then
89-
// the user either changes or unsets the TTL, the deadline for the workspace
90-
// build will not have changed. So, autostop will still happen at the
91-
// original TTL value from when the workspace build was created.
92-
// Whether this is expected behavior from a user's perspective is not yet known.
88+
// NOTE: If a workspace build is created with a given TTL and then the user either
89+
// changes or unsets the TTL, the deadline for the workspace build will not
90+
// have changed. This behavior is as expected per #2229.
9391
eligibleWorkspaces,err:=db.GetWorkspacesAutostart(e.ctx)
9492
iferr!=nil {
9593
returnxerrors.Errorf("get eligible workspaces for autostart or autostop: %w",err)

‎coderd/autobuild/executor/lifecycle_executor_test.go

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -308,8 +308,7 @@ func TestExecutorAutostopNotEnabled(t *testing.T) {
308308
require.NoError(t,err)
309309
require.Nil(t,workspace.TTLMillis)
310310

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

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

443-
// Then: the deadline should be thezero value
442+
// Then: the deadline shouldstillbe theoriginal value
444443
updated:=coderdtest.MustWorkspace(t,client,workspace.ID)
445-
assert.Zero(t,updated.LatestBuild.Deadline)
444+
assert.WithinDuration(t,workspace.LatestBuild.Deadline,updated.LatestBuild.Deadline,time.Minute)
446445

447446
// When: the autobuild executor ticks after the original deadline
448447
gofunc() {
449448
tickCh<-workspace.LatestBuild.Deadline.Add(time.Minute)
450449
}()
451450

452-
// Then: the workspace shouldnotstop
451+
// Then: the workspace should stop
453452
stats:=<-statsCh
454453
assert.NoError(t,stats.Error)
455-
assert.Len(t,stats.Transitions,0)
454+
assert.Len(t,stats.Transitions,1)
455+
assert.Equal(t,stats.Transitions[workspace.ID],database.WorkspaceTransitionStop)
456+
457+
// Wait for stop to complete
458+
updated=coderdtest.MustWorkspace(t,client,workspace.ID)
459+
_=coderdtest.AwaitWorkspaceBuildJob(t,client,updated.LatestBuild.ID)
460+
461+
// Start the workspace again
462+
workspace=coderdtest.MustTransitionWorkspace(t,client,workspace.ID,database.WorkspaceTransitionStop,database.WorkspaceTransitionStart)
456463

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

463-
// Then: the deadline shouldbe updated based ontheTTL
469+
// Then: the deadline shouldremain atthezero value
464470
updated=coderdtest.MustWorkspace(t,client,workspace.ID)
465-
assert.WithinDuration(t,expectedDeadline,updated.LatestBuild.Deadline,time.Minute)
471+
assert.Zero(t,updated.LatestBuild.Deadline)
466472

467473
// When: the relentless onward march of time continues
468474
gofunc() {
469475
tickCh<-workspace.LatestBuild.Deadline.Add(newTTL+time.Minute)
470476
close(tickCh)
471477
}()
472478

473-
// Then: the workspace should stop
479+
// Then: the workspace shouldnotstop
474480
stats=<-statsCh
475481
assert.NoError(t,stats.Error)
476-
assert.Len(t,stats.Transitions,1)
477-
assert.Equal(t,stats.Transitions[workspace.ID],database.WorkspaceTransitionStop)
482+
assert.Len(t,stats.Transitions,0)
478483
}
479484

480485
funcTestExecutorAutostartMultipleOK(t*testing.T) {

‎coderd/workspaces.go

Lines changed: 33 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ import (
3131
"github.com/coder/coder/codersdk"
3232
)
3333

34+
constworkspaceDefaultTTL=12*time.Hour
35+
3436
func (api*API)workspace(rw http.ResponseWriter,r*http.Request) {
3537
workspace:=httpmw.WorkspaceParam(r)
3638
if!api.Authorize(r,rbac.ActionRead,workspace) {
@@ -291,8 +293,8 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req
291293
}
292294

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

298300
workspace,err:=api.Database.GetWorkspaceByOwnerIDAndName(r.Context(), database.GetWorkspaceByOwnerIDAndNameParams{
@@ -513,75 +515,41 @@ func (api *API) putWorkspaceTTL(rw http.ResponseWriter, r *http.Request) {
513515
return
514516
}
515517

516-
template,err:=api.Database.GetTemplateByID(r.Context(),workspace.TemplateID)
517-
iferr!=nil {
518-
httpapi.Write(rw,http.StatusInternalServerError, httpapi.Response{
519-
Message:"Error fetching workspace template!",
520-
})
521-
return
522-
}
518+
varvalidErrs []httpapi.Error
523519

524-
dbTTL,err:=validWorkspaceTTLMillis(req.TTLMillis,time.Duration(template.MaxTtl))
525-
iferr!=nil {
526-
httpapi.Write(rw,http.StatusBadRequest, httpapi.Response{
527-
Message:"Invalid workspace TTL.",
528-
Detail:err.Error(),
529-
Validations: []httpapi.Error{
530-
{
531-
Field:"ttl_ms",
532-
Detail:err.Error(),
533-
},
534-
},
535-
})
536-
return
537-
}
520+
err:=api.Database.InTx(func(s database.Store)error {
521+
template,err:=s.GetTemplateByID(r.Context(),workspace.TemplateID)
522+
iferr!=nil {
523+
httpapi.Write(rw,http.StatusInternalServerError, httpapi.Response{
524+
Message:"Error fetching workspace template!",
525+
})
526+
returnxerrors.Errorf("fetch workspace template: %w",err)
527+
}
538528

539-
err=api.Database.InTx(func(s database.Store)error {
529+
dbTTL,err:=validWorkspaceTTLMillis(req.TTLMillis,time.Duration(template.MaxTtl))
530+
iferr!=nil {
531+
validErrs=append(validErrs, httpapi.Error{Field:"ttl_ms",Detail:err.Error()})
532+
returnerr
533+
}
540534
iferr:=s.UpdateWorkspaceTTL(r.Context(), database.UpdateWorkspaceTTLParams{
541535
ID:workspace.ID,
542536
Ttl:dbTTL,
543537
});err!=nil {
544538
returnxerrors.Errorf("update workspace TTL: %w",err)
545539
}
546540

547-
// Also extend the workspace deadline if the workspace is running
548-
latestBuild,err:=s.GetLatestWorkspaceBuildByWorkspaceID(r.Context(),workspace.ID)
549-
iferr!=nil {
550-
returnxerrors.Errorf("get latest workspace build: %w",err)
551-
}
552-
553-
iflatestBuild.Transition!=database.WorkspaceTransitionStart {
554-
returnnil// nothing to do
555-
}
556-
557-
iflatestBuild.UpdatedAt.IsZero() {
558-
// Build in progress; provisionerd should update with the new TTL.
559-
returnnil
560-
}
561-
562-
varnewDeadline time.Time
563-
ifdbTTL.Valid {
564-
newDeadline=latestBuild.UpdatedAt.Add(time.Duration(dbTTL.Int64))
565-
}
566-
567-
iferr:=s.UpdateWorkspaceBuildByID(
568-
r.Context(),
569-
database.UpdateWorkspaceBuildByIDParams{
570-
ID:latestBuild.ID,
571-
UpdatedAt:latestBuild.UpdatedAt,
572-
ProvisionerState:latestBuild.ProvisionerState,
573-
Deadline:newDeadline,
574-
},
575-
);err!=nil {
576-
returnxerrors.Errorf("update workspace deadline: %w",err)
577-
}
578541
returnnil
579542
})
580543

581544
iferr!=nil {
582-
httpapi.Write(rw,http.StatusInternalServerError, httpapi.Response{
583-
Message:"Error updating workspace time until shutdown!",
584-
Detail:err.Error(),
545+
code:=http.StatusInternalServerError
546+
iflen(validErrs)>0 {
547+
code=http.StatusBadRequest
548+
}
549+
httpapi.Write(rw,code, httpapi.Response{
550+
Message:"Error updating workspace time until shutdown!",
551+
Validations:validErrs,
552+
Detail:err.Error(),
585553
})
586554
return
587555
}
@@ -1028,3 +996,10 @@ func splitQueryParameterByDelimiter(query string, delimiter rune, maintainQuotes
1028996

1029997
returnparts
1030998
}
999+
1000+
funcmin(x,yint64)int64 {
1001+
ifx<y {
1002+
returnx
1003+
}
1004+
returny
1005+
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp