From a19a0e97eca045c94b9c59d4e6e240be51c05361 Mon Sep 17 00:00:00 2001 From: johnstcn Date: Mon, 13 Jun 2022 14:56:56 +0000 Subject: [PATCH 01/10] fix: coderd: decouple ttl and deadline --- cli/ttl.go | 65 +++++------ cli/ttl_test.go | 4 - .../autobuild/executor/lifecycle_executor.go | 8 +- .../executor/lifecycle_executor_test.go | 29 ++--- coderd/workspaces.go | 78 ++++---------- coderd/workspaces_test.go | 40 +++---- .../WorkspaceScheduleForm.test.ts | 101 +++--------------- .../WorkspaceScheduleForm.tsx | 39 ++----- 8 files changed, 106 insertions(+), 258 deletions(-) diff --git a/cli/ttl.go b/cli/ttl.go index 9bc19dc033bea..ef4e0324b2f2d 100644 --- a/cli/ttl.go +++ b/cli/ttl.go @@ -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" ) @@ -91,30 +91,6 @@ 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, @@ -122,6 +98,28 @@ func ttlset() *cobra.Command { 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) + } + + loc, err := time.LoadLocation(sched.Timezone()) + if err != nil { + return xerrors.Errorf("schedule has invalid timezone: %w", err) + } + + nextShutdown := sched.Next(time.Now()).Add(truncated).In(loc) + _, _ = fmt.Fprintf(cmd.OutOrStdout(), "%q will shut down at %s on %s (%s after start).\n", + workspace.Name, + nextShutdown.Format("15:04:05 MST"), + nextShutdown.Format("2006-02-01"), + truncated, + ) return nil }, } @@ -157,18 +155,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 -} diff --git a/cli/ttl_test.go b/cli/ttl_test.go index 92ca201c81a44..28097d27071e2 100644 --- a/cli/ttl_test.go +++ b/cli/ttl_test.go @@ -3,7 +3,6 @@ package cli_test import ( "bytes" "context" - "fmt" "strings" "testing" "time" @@ -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") diff --git a/coderd/autobuild/executor/lifecycle_executor.go b/coderd/autobuild/executor/lifecycle_executor.go index 55b8b3228f775..161b85cf0f53f 100644 --- a/coderd/autobuild/executor/lifecycle_executor.go +++ b/coderd/autobuild/executor/lifecycle_executor.go @@ -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. eligibleWorkspaces, err := db.GetWorkspacesAutostart(e.ctx) if err != nil { return xerrors.Errorf("get eligible workspaces for autostart or autostop: %w", err) diff --git a/coderd/autobuild/executor/lifecycle_executor_test.go b/coderd/autobuild/executor/lifecycle_executor_test.go index be435b7730ab8..1680fe0368e12 100644 --- a/coderd/autobuild/executor/lifecycle_executor_test.go +++ b/coderd/autobuild/executor/lifecycle_executor_test.go @@ -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) @@ -440,29 +439,36 @@ func TestExecutorWorkspaceAutostopNoWaitChangedMyMind(t *testing.T) { err := client.UpdateWorkspaceTTL(ctx, workspace.ID, codersdk.UpdateWorkspaceTTLRequest{TTLMillis: nil}) require.NoError(t, err) - // Then: the deadline should be the zero value + // Then: the deadline should still be the original 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 should not stop + // 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 should be updated based on the TTL + // Then: the deadline should remain at the zero 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() { @@ -470,11 +476,10 @@ func TestExecutorWorkspaceAutostopNoWaitChangedMyMind(t *testing.T) { close(tickCh) }() - // Then: the workspace should stop + // Then: the workspace should not stop 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) { diff --git a/coderd/workspaces.go b/coderd/workspaces.go index b9e09f50513b4..3960ad5691f7c 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -541,30 +541,22 @@ 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, @@ -572,44 +564,18 @@ func (api *API) putWorkspaceTTL(rw http.ResponseWriter, r *http.Request) { 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 } diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index cda6688f4d7af..b62f8c0678d99 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -668,23 +668,20 @@ func TestWorkspaceUpdateTTL(t *testing.T) { t.Parallel() testCases := []struct { - name string - ttlMillis *int64 - expectedError string - expectedDeadline *time.Time - modifyTemplate func(*codersdk.CreateTemplateRequest) + name string + ttlMillis *int64 + expectedError string + modifyTemplate func(*codersdk.CreateTemplateRequest) }{ { - name: "disable ttl", - ttlMillis: nil, - expectedError: "", - expectedDeadline: ptr.Ref(time.Time{}), + name: "disable ttl", + ttlMillis: nil, + expectedError: "", }, { - name: "update ttl", - ttlMillis: ptr.Ref(12 * time.Hour.Milliseconds()), - expectedError: "", - expectedDeadline: ptr.Ref(time.Now().Add(12*time.Hour + time.Minute)), + name: "update ttl", + ttlMillis: ptr.Ref(12 * time.Hour.Milliseconds()), + expectedError: "", }, { name: "below minimum ttl", @@ -692,16 +689,14 @@ func TestWorkspaceUpdateTTL(t *testing.T) { expectedError: "ttl must be at least one minute", }, { - name: "minimum ttl", - ttlMillis: ptr.Ref(time.Minute.Milliseconds()), - expectedError: "", - expectedDeadline: ptr.Ref(time.Now().Add(2 * time.Minute)), + name: "minimum ttl", + ttlMillis: ptr.Ref(time.Minute.Milliseconds()), + expectedError: "", }, { - name: "maximum ttl", - ttlMillis: ptr.Ref((24 * 7 * time.Hour).Milliseconds()), - expectedError: "", - expectedDeadline: ptr.Ref(time.Now().Add(24*7*time.Hour + time.Minute)), + name: "maximum ttl", + ttlMillis: ptr.Ref((24 * 7 * time.Hour).Milliseconds()), + expectedError: "", }, { name: "above maximum ttl", @@ -754,9 +749,6 @@ func TestWorkspaceUpdateTTL(t *testing.T) { require.NoError(t, err, "fetch updated workspace") require.Equal(t, testCase.ttlMillis, updated.TTLMillis, "expected autostop ttl to equal requested") - if testCase.expectedDeadline != nil { - require.WithinDuration(t, *testCase.expectedDeadline, updated.LatestBuild.Deadline, time.Minute, "expected autostop deadline to be equal expected") - } }) } diff --git a/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.test.ts b/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.test.ts index 3e37484b9c09b..2d4364543ec91 100644 --- a/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.test.ts +++ b/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.test.ts @@ -1,6 +1,3 @@ -import dayjs from "dayjs" -import { Workspace } from "../../api/typesGenerated" -import * as Mocks from "../../testHelpers/entities" import { Language, ttlShutdownAt, validationSchema, WorkspaceScheduleFormValues } from "./WorkspaceScheduleForm" import { zones } from "./zones" @@ -160,99 +157,29 @@ describe("validationSchema", () => { }) describe("ttlShutdownAt", () => { - it.each<[string, dayjs.Dayjs, Workspace, string, number, string]>([ + it.each<[string, number, string]>([ + ["Manual shutdown --> manual helper text", 0, Language.ttlCausesNoShutdownHelperText], [ - "Workspace is stopped --> helper text", - dayjs("2022-05-17T18:09:00Z"), - Mocks.MockStoppedWorkspace, - "America/Chicago", + "One hour --> helper text shows shutdown after an hour", 1, - Language.ttlHelperText, + `${Language.ttlCausesShutdownHelperText} an hour ${Language.ttlCausesShutdownAfterStart}.`, ], [ - "TTL is not modified --> helper text", - dayjs("2022-05-17T16:09:00Z"), - { - ...Mocks.MockWorkspace, - latest_build: { - ...Mocks.MockWorkspaceBuild, - deadline: "2022-05-17T18:09:00Z", - }, - ttl_ms: 2 * 60 * 60 * 1000, // 2 hours = shuts off at 18:09 - }, - "America/Chicago", + "Two hours --> helper text shows shutdown after 2 hours", 2, - Language.ttlHelperText, + `${Language.ttlCausesShutdownHelperText} 2 hours ${Language.ttlCausesShutdownAfterStart}.`, ], [ - "TTL becomes 0 --> manual helper text", - dayjs("2022-05-17T18:09:00Z"), - Mocks.MockWorkspace, - "America/Chicago", - 0, - Language.ttlCausesNoShutdownHelperText, + "24 hours --> helper text shows shutdown after a day", + 24, + `${Language.ttlCausesShutdownHelperText} a day ${Language.ttlCausesShutdownAfterStart}.`, ], [ - "Deadline of 18:09 becomes 17:09 at 17:09 --> immediate shutdown", - dayjs("2022-05-17T17:09:00Z"), - { - ...Mocks.MockWorkspace, - latest_build: { - ...Mocks.MockWorkspaceBuild, - deadline: "2022-05-17T18:09:00Z", - }, - ttl_ms: 2 * 60 * 60 * 1000, // 2 hours = shuts off at 18:09 - }, - "America/Chicago", - 1, - `⚠️ ${Language.ttlCausesShutdownHelperText} ${Language.ttlCausesShutdownImmediately} ⚠️`, - ], - [ - "Deadline of 18:09 becomes 17:09 at 16:39 --> display shutdown soon", - dayjs("2022-05-17T16:39:00Z"), - { - ...Mocks.MockWorkspace, - latest_build: { - ...Mocks.MockWorkspaceBuild, - deadline: "2022-05-17T18:09:00Z", - }, - ttl_ms: 2 * 60 * 60 * 1000, // 2 hours = shuts off at 18:09 - }, - "America/Chicago", - 1, - `⚠️ ${Language.ttlCausesShutdownHelperText} ${Language.ttlCausesShutdownSoon} ⚠️`, - ], - [ - "Deadline of 18:09 becomes 17:09 at 16:09 --> display 12:09 CDT", - dayjs("2022-05-17T16:09:00Z"), - { - ...Mocks.MockWorkspace, - latest_build: { - ...Mocks.MockWorkspaceBuild, - deadline: "2022-05-17T18:09:00Z", - }, - ttl_ms: 2 * 60 * 60 * 1000, // 2 hours = shuts off at 18:09 - }, - "America/Chicago", - 1, - `${Language.ttlCausesShutdownHelperText} ${Language.ttlCausesShutdownAt} May 17, 2022 12:09 PM.`, - ], - [ - "Manual workspace gets new deadline of 18:09 at 17:09 --> display 1:09 CDT", - dayjs("2022-05-17T17:09:00Z"), - { - ...Mocks.MockWorkspace, - latest_build: { - ...Mocks.MockWorkspaceBuild, - deadline: "0001-01-01T00:00:00Z", - }, - ttl_ms: 0, - }, - "America/Chicago", - 1, - `${Language.ttlCausesShutdownHelperText} ${Language.ttlCausesShutdownAt} May 17, 2022 1:09 PM.`, + "48 hours --> helper text shows shutdown after 2 days", + 48, + `${Language.ttlCausesShutdownHelperText} 2 days ${Language.ttlCausesShutdownAfterStart}.`, ], - ])("%p", (_, now, workspace, timezone, ttlHours, expected) => { - expect(ttlShutdownAt(now, workspace, timezone, ttlHours)).toEqual(expected) + ])("%p", (_, ttlHours, expected) => { + expect(ttlShutdownAt(ttlHours)).toEqual(expected) }) }) diff --git a/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.tsx b/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.tsx index efd2c78199e34..547e4a3622119 100644 --- a/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.tsx +++ b/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.tsx @@ -9,7 +9,8 @@ import makeStyles from "@material-ui/core/styles/makeStyles" import TextField from "@material-ui/core/TextField" import dayjs from "dayjs" import advancedFormat from "dayjs/plugin/advancedFormat" -import isSameOrBefore from "dayjs/plugin/isSameOrBefore" +import duration from "dayjs/plugin/duration" +import relativeTime from "dayjs/plugin/relativeTime" import timezone from "dayjs/plugin/timezone" import utc from "dayjs/plugin/utc" import { useFormik } from "formik" @@ -18,7 +19,6 @@ import * as Yup from "yup" import { FieldErrors } from "../../api/errors" import { Workspace } from "../../api/typesGenerated" import { getFormHelpers } from "../../util/formUtils" -import { isWorkspaceOn } from "../../util/workspace" import { FormFooter } from "../FormFooter/FormFooter" import { FullPageForm } from "../FullPageForm/FullPageForm" import { Stack } from "../Stack/Stack" @@ -28,7 +28,8 @@ import { zones } from "./zones" // sorted alphabetically. dayjs.extend(utc) dayjs.extend(advancedFormat) -dayjs.extend(isSameOrBefore) +dayjs.extend(duration) +dayjs.extend(relativeTime) dayjs.extend(timezone) export const Language = { @@ -48,11 +49,8 @@ export const Language = { startTimeHelperText: "Your workspace will automatically start at this time.", timezoneLabel: "Timezone", ttlLabel: "Time until shutdown (hours)", - ttlHelperText: "Your workspace will automatically shut down after this amount of time has elapsed.", ttlCausesShutdownHelperText: "Your workspace will shut down", - ttlCausesShutdownAt: "at", - ttlCausesShutdownImmediately: "immediately!", - ttlCausesShutdownSoon: "within 30 minutes.", + ttlCausesShutdownAfterStart: "after start", ttlCausesNoShutdownHelperText: "Your workspace will not automatically shut down.", } @@ -269,7 +267,7 @@ export const WorkspaceScheduleForm: FC = ({ = ({ ) } -export const ttlShutdownAt = (now: dayjs.Dayjs, workspace: Workspace, tz: string, formTTL: number): string => { - // a manual shutdown has a deadline of '"0001-01-01T00:00:00Z"' - // SEE: #1834 - const deadline = dayjs(workspace.latest_build.deadline).utc() - const hasDeadline = deadline.year() > 1 - const ttl = workspace.ttl_ms ? workspace.ttl_ms / (1000 * 60 * 60) : 0 - const delta = formTTL - ttl - - if (delta === 0 || !isWorkspaceOn(workspace)) { - return Language.ttlHelperText - } else if (formTTL === 0) { +export const ttlShutdownAt = (formTTL: number): string => { + if (formTTL === 0) { return Language.ttlCausesNoShutdownHelperText } else { - const newDeadline = dayjs(hasDeadline ? deadline : now).add(delta, "hours") - if (newDeadline.isSameOrBefore(now)) { - return `⚠️ ${Language.ttlCausesShutdownHelperText} ${Language.ttlCausesShutdownImmediately} ⚠️` - } else if (newDeadline.isSameOrBefore(now.add(30, "minutes"))) { - return `⚠️ ${Language.ttlCausesShutdownHelperText} ${Language.ttlCausesShutdownSoon} ⚠️` - } else { - return `${Language.ttlCausesShutdownHelperText} ${Language.ttlCausesShutdownAt} ${newDeadline - .tz(tz) - .format("MMM D, YYYY h:mm A")}.` - } + return `${Language.ttlCausesShutdownHelperText} ${dayjs.duration(formTTL, "hours").humanize()} ${Language.ttlCausesShutdownAfterStart + }.` } } From 3999661ed654cd10a972ba3b36aadfa36aaf5250 Mon Sep 17 00:00:00 2001 From: johnstcn Date: Mon, 13 Jun 2022 15:05:00 +0000 Subject: [PATCH 02/10] yarn lint --- .../WorkspaceScheduleForm/WorkspaceScheduleForm.tsx | 10 +++------- .../WorkspaceSchedulePage/WorkspaceSchedulePage.tsx | 1 - 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.tsx b/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.tsx index 547e4a3622119..640b13310ba4d 100644 --- a/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.tsx +++ b/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.tsx @@ -17,7 +17,6 @@ import { useFormik } from "formik" import { FC } from "react" import * as Yup from "yup" import { FieldErrors } from "../../api/errors" -import { Workspace } from "../../api/typesGenerated" import { getFormHelpers } from "../../util/formUtils" import { FormFooter } from "../FormFooter/FormFooter" import { FullPageForm } from "../FullPageForm/FullPageForm" @@ -58,10 +57,8 @@ export interface WorkspaceScheduleFormProps { fieldErrors?: FieldErrors initialValues?: WorkspaceScheduleFormValues isLoading: boolean - now?: dayjs.Dayjs onCancel: () => void onSubmit: (values: WorkspaceScheduleFormValues) => void - workspace: Workspace } export interface WorkspaceScheduleFormValues { @@ -184,10 +181,8 @@ export const WorkspaceScheduleForm: FC = ({ fieldErrors, initialValues = defaultWorkspaceSchedule(), isLoading, - now = dayjs(), onCancel, onSubmit, - workspace, }) => { const styles = useStyles() @@ -285,8 +280,9 @@ export const ttlShutdownAt = (formTTL: number): string => { if (formTTL === 0) { return Language.ttlCausesNoShutdownHelperText } else { - return `${Language.ttlCausesShutdownHelperText} ${dayjs.duration(formTTL, "hours").humanize()} ${Language.ttlCausesShutdownAfterStart - }.` + return `${Language.ttlCausesShutdownHelperText} ${dayjs.duration(formTTL, "hours").humanize()} ${ + Language.ttlCausesShutdownAfterStart + }.` } } diff --git a/site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx b/site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx index df5f511e5bfe4..dbed18ea91927 100644 --- a/site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx +++ b/site/src/pages/WorkspaceSchedulePage/WorkspaceSchedulePage.tsx @@ -161,7 +161,6 @@ export const WorkspaceSchedulePage: React.FC = () => { } else if (scheduleState.matches("presentForm") || scheduleState.matches("submittingSchedule")) { return ( Date: Mon, 13 Jun 2022 15:19:22 +0000 Subject: [PATCH 03/10] update storybook --- .../WorkspaceScheduleForm.stories.tsx | 80 ++++--------------- 1 file changed, 17 insertions(+), 63 deletions(-) diff --git a/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.stories.tsx b/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.stories.tsx index 5c3126fc74248..4fccbfd90ec4d 100644 --- a/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.stories.tsx +++ b/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.stories.tsx @@ -18,97 +18,51 @@ export default { const Template: Story = (args) => -export const WorkspaceNotRunning = Template.bind({}) -WorkspaceNotRunning.args = { - now: dayjs("2022-05-17T17:40:00Z"), +export const WorkspaceWillNotShutDown = Template.bind({}) +WorkspaceWillNotShutDown.args = { initialValues: { ...defaultWorkspaceSchedule(5), - timezone: "UTC", - }, - workspace: { - ...Mocks.MockWorkspace, - latest_build: { - ...Mocks.MockWorkspaceBuild, - transition: "stop", - updated_at: "2022-05-17T17:39:00Z", - }, + ttl: 0, }, onCancel: () => action("onCancel"), onSubmit: () => action("onSubmit"), } -export const WorkspaceWillNotShutDown = Template.bind({}) -WorkspaceWillNotShutDown.args = { - now: dayjs("2022-05-17T17:40:00Z"), +export const WorkspaceWillShutdownInAnHour = Template.bind({}) +WorkspaceWillShutdownInAnHour.args = { initialValues: { ...defaultWorkspaceSchedule(5), - timezone: "UTC", - ttl: 0, - }, - workspace: { - ...Mocks.MockWorkspace, - latest_build: { - ...Mocks.MockWorkspaceBuild, - updated_at: "2022-05-17T17:39:00Z", - }, + ttl: 1, }, onCancel: () => action("onCancel"), onSubmit: () => action("onSubmit"), } -export const WorkspaceWillShutdown = Template.bind({}) -WorkspaceWillShutdown.args = { - now: dayjs("2022-05-17T17:40:00Z"), +export const WorkspaceWillShutdownInTwoHours = Template.bind({}) +WorkspaceWillShutdownInTwoHours.args = { initialValues: { - ...defaultWorkspaceSchedule(5), - timezone: "UTC", - }, - workspace: { - ...Mocks.MockWorkspace, - latest_build: { - ...Mocks.MockWorkspaceBuild, - updated_at: "2022-05-17T17:39:00Z", - }, + ...defaultWorkspaceSchedule(2), + ttl: 2, }, onCancel: () => action("onCancel"), onSubmit: () => action("onSubmit"), } -export const WorkspaceWillShutdownSoon = Template.bind({}) -WorkspaceWillShutdownSoon.args = { - now: dayjs("2022-05-17T16:39:00Z"), +export const WorkspaceWillShutdownInADay = Template.bind({}) +WorkspaceWillShutdownInADay.args = { initialValues: { ...defaultWorkspaceSchedule(2), - timezone: "UTC", - ttl: 1, - }, - workspace: { - ...Mocks.MockWorkspace, - latest_build: { - ...Mocks.MockWorkspaceBuild, - deadline: "2022-05-17T18:09:00Z", - }, - ttl_ms: 2 * 60 * 60 * 1000, // 2 hours = shuts off at 18:09 + ttl: 24, }, onCancel: () => action("onCancel"), onSubmit: () => action("onSubmit"), } -export const WorkspaceWillShutdownImmediately = Template.bind({}) -WorkspaceWillShutdownImmediately.args = { - now: dayjs("2022-05-17T17:09:00Z"), +export const WorkspaceWillShutdownInTwoDays = Template.bind({}) +WorkspaceWillShutdownInTwoDays.args = { initialValues: { - ...defaultWorkspaceSchedule(1), - timezone: "UTC", - ttl: 1, - }, - workspace: { - ...Mocks.MockWorkspace, - latest_build: { - ...Mocks.MockWorkspaceBuild, - deadline: "2022-05-17T18:09:00Z", - }, - ttl_ms: 2 * 60 * 60 * 1000, // 2 hours = shuts off at 18:09 + ...defaultWorkspaceSchedule(2), + ttl: 48, }, onCancel: () => action("onCancel"), onSubmit: () => action("onSubmit"), From 987f78593ea3e680711e12e90657cefa412be77a Mon Sep 17 00:00:00 2001 From: johnstcn Date: Mon, 13 Jun 2022 15:24:51 +0000 Subject: [PATCH 04/10] fmt --- .../WorkspaceScheduleForm/WorkspaceScheduleForm.stories.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.stories.tsx b/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.stories.tsx index 4fccbfd90ec4d..611d75458ece7 100644 --- a/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.stories.tsx +++ b/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.stories.tsx @@ -4,7 +4,6 @@ import dayjs from "dayjs" import advancedFormat from "dayjs/plugin/advancedFormat" import timezone from "dayjs/plugin/timezone" import utc from "dayjs/plugin/utc" -import * as Mocks from "../../testHelpers/entities" import { defaultWorkspaceSchedule, WorkspaceScheduleForm, WorkspaceScheduleFormProps } from "./WorkspaceScheduleForm" dayjs.extend(advancedFormat) From cd4dce336ce1b8f15319f519c9afec5f43cfb8c3 Mon Sep 17 00:00:00 2001 From: johnstcn Date: Tue, 14 Jun 2022 08:32:14 +0000 Subject: [PATCH 05/10] fix ttl command --- cli/ttl.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/cli/ttl.go b/cli/ttl.go index ef4e0324b2f2d..86422710a5ba7 100644 --- a/cli/ttl.go +++ b/cli/ttl.go @@ -108,12 +108,7 @@ func ttlset() *cobra.Command { return xerrors.Errorf("parse workspace schedule: %w", err) } - loc, err := time.LoadLocation(sched.Timezone()) - if err != nil { - return xerrors.Errorf("schedule has invalid timezone: %w", err) - } - - nextShutdown := sched.Next(time.Now()).Add(truncated).In(loc) + 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("15:04:05 MST"), From 91e3171d1bdec966845f5345aa50ec302027ef81 Mon Sep 17 00:00:00 2001 From: johnstcn Date: Tue, 14 Jun 2022 09:15:49 +0000 Subject: [PATCH 06/10] drive-by: default ttl to min(12 hours, template max) for new workspaces --- coderd/workspaces.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 3960ad5691f7c..56ec7f4105382 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -319,8 +319,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 to min(12 hours, template maximum). Just defaulting to template maximum can be surprising. + dbTTL = sql.NullInt64{Valid: true, Int64: min(template.MaxTtl, int64(12*time.Hour))} } workspace, err := api.Database.GetWorkspaceByOwnerIDAndName(r.Context(), database.GetWorkspaceByOwnerIDAndNameParams{ @@ -940,3 +940,10 @@ func validWorkspaceSchedule(s *string, min time.Duration) (sql.NullString, error String: *s, }, nil } + +func min(x, y int64) int64 { + if x < y { + return x + } + return y +} From 00ac2a226f7a89cca5148a8b3bb45bdbe1a5009e Mon Sep 17 00:00:00 2001 From: johnstcn Date: Tue, 14 Jun 2022 09:37:28 +0000 Subject: [PATCH 07/10] fix: WorkspaceSchedule: show tz of schedule if available, default to dayjs guess --- .../components/WorkspaceSchedule/WorkspaceSchedule.tsx | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/site/src/components/WorkspaceSchedule/WorkspaceSchedule.tsx b/site/src/components/WorkspaceSchedule/WorkspaceSchedule.tsx index 5d5d1db960c5c..2a60dd5e4ce71 100644 --- a/site/src/components/WorkspaceSchedule/WorkspaceSchedule.tsx +++ b/site/src/components/WorkspaceSchedule/WorkspaceSchedule.tsx @@ -13,7 +13,7 @@ import { FC } from "react" import { Link as RouterLink } from "react-router-dom" import { Workspace } from "../../api/typesGenerated" import { MONOSPACE_FONT_FAMILY } from "../../theme/constants" -import { stripTimezone } from "../../util/schedule" +import { extractTimezone, stripTimezone } from "../../util/schedule" import { isWorkspaceOn } from "../../util/workspace" import { Stack } from "../Stack/Stack" @@ -66,7 +66,10 @@ export const Language = { } }, editScheduleLink: "Edit schedule", - schedule: `Schedule (${dayjs.tz.guess()})`, + scheduleHeader: (workspace: Workspace): string => { + const tz = workspace.autostart_schedule ? extractTimezone(workspace.autostart_schedule) : dayjs.tz.guess() + return `Schedule (${tz})` + }, } export interface WorkspaceScheduleProps { @@ -81,7 +84,7 @@ export const WorkspaceSchedule: FC = ({ workspace }) => - {Language.schedule} + {Language.scheduleHeader(workspace)}
{Language.autoStartLabel} From 3faf3c82af9ae9806377186c05e2749dc66e9aa4 Mon Sep 17 00:00:00 2001 From: johnstcn Date: Tue, 14 Jun 2022 09:38:12 +0000 Subject: [PATCH 08/10] cli: list: measure extension from provisioner job completed_at --- cli/list.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cli/list.go b/cli/list.go index 47eae30d23a7b..c9400c984a202 100644 --- a/cli/list.go +++ b/cli/list.go @@ -127,6 +127,9 @@ 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 } @@ -134,7 +137,7 @@ func hasExtension(ws codersdk.Workspace) (bool, time.Duration) { 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) if delta < time.Minute { return false, 0 } From a763d2f05585eb64a1e523d5e1ed4de25049c247 Mon Sep 17 00:00:00 2001 From: johnstcn Date: Tue, 14 Jun 2022 09:50:30 +0000 Subject: [PATCH 09/10] WorkspaceScheduleForm: fix "a few seconds after start" --- .../components/WorkspaceScheduleForm/WorkspaceScheduleForm.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.tsx b/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.tsx index 640b13310ba4d..cab7356e168ac 100644 --- a/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.tsx +++ b/site/src/components/WorkspaceScheduleForm/WorkspaceScheduleForm.tsx @@ -277,7 +277,8 @@ export const WorkspaceScheduleForm: FC = ({ } export const ttlShutdownAt = (formTTL: number): string => { - if (formTTL === 0) { + if (formTTL < 1) { + // Passing an empty value for TTL in the form results in a number that is not zero but less than 1. return Language.ttlCausesNoShutdownHelperText } else { return `${Language.ttlCausesShutdownHelperText} ${dayjs.duration(formTTL, "hours").humanize()} ${ From bbd8288a6fcd5d18803d5e044bf10f8a686d31fc Mon Sep 17 00:00:00 2001 From: johnstcn Date: Tue, 14 Jun 2022 12:09:58 +0000 Subject: [PATCH 10/10] address PR comments --- cli/bump.go | 5 +++-- cli/constants.go | 6 ++++++ cli/ttl.go | 6 ++++-- coderd/workspaces.go | 4 +++- 4 files changed, 16 insertions(+), 5 deletions(-) create mode 100644 cli/constants.go diff --git a/cli/bump.go b/cli/bump.go index 220f6c406b816..e49f922495837 100644 --- a/cli/bump.go +++ b/cli/bump.go @@ -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 diff --git a/cli/constants.go b/cli/constants.go new file mode 100644 index 0000000000000..af9dd5260ee39 --- /dev/null +++ b/cli/constants.go @@ -0,0 +1,6 @@ +package cli + +const ( + timeFormat = "3:04:05 PM MST" + dateFormat = "Jan 2, 2006" +) diff --git a/cli/ttl.go b/cli/ttl.go index 86422710a5ba7..1b293c43044a1 100644 --- a/cli/ttl.go +++ b/cli/ttl.go @@ -111,10 +111,12 @@ func ttlset() *cobra.Command { 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("15:04:05 MST"), - nextShutdown.Format("2006-02-01"), + 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 }, } diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 56ec7f4105382..a45d749884831 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -29,6 +29,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) { @@ -320,7 +322,7 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req if !dbTTL.Valid { // Default to min(12 hours, template maximum). Just defaulting to template maximum can be surprising. - dbTTL = sql.NullInt64{Valid: true, Int64: min(template.MaxTtl, int64(12*time.Hour))} + dbTTL = sql.NullInt64{Valid: true, Int64: min(template.MaxTtl, int64(workspaceDefaultTTL))} } workspace, err := api.Database.GetWorkspaceByOwnerIDAndName(r.Context(), database.GetWorkspaceByOwnerIDAndNameParams{