Skip to content

Commit

Permalink
chore: refactor time.Duration -> int64 milliseconds for FE consumption (
Browse files Browse the repository at this point in the history 
#1944)

* Changes all public-facing codersdk types to use a plain int64 (milliseconds) instead of time.Duration.
* Makes autostart_schedule a *string as it may not be present.
* Adds a utils/ptr package with some useful methods.
  • Loading branch information
@johnstcn
johnstcn committed Jun 2, 2022
1 parent 51c420c commit dcf03d8
Show file tree
Hide file tree
Showing 24 changed files with 287 additions and 148 deletions.
10 changes: 5 additions & 5 deletions cli/autostart.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,15 @@ func autostartShow() *cobra.Command {
return err
}

if workspace.AutostartSchedule == "" {
if workspace.AutostartSchedule == nil || *workspace.AutostartSchedule == "" {
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "not enabled\n")
return nil
}

validSchedule, err := schedule.Weekly(workspace.AutostartSchedule)
validSchedule, err := schedule.Weekly(*workspace.AutostartSchedule)
if err != nil {
// This should never happen.
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "invalid autostart schedule %q for workspace %s: %s\n", workspace.AutostartSchedule, workspace.Name, err.Error())
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "invalid autostart schedule %q for workspace %s: %s\n", *workspace.AutostartSchedule, workspace.Name, err.Error())
return nil
}

Expand Down Expand Up @@ -110,7 +110,7 @@ func autostartEnable() *cobra.Command {
}

err = client.UpdateWorkspaceAutostart(cmd.Context(), workspace.ID, codersdk.UpdateWorkspaceAutostartRequest{
Schedule: validSchedule.String(),
Schedule: &spec,
})
if err != nil {
return err
Expand Down Expand Up @@ -153,7 +153,7 @@ func autostartDisable() *cobra.Command {
}

err = client.UpdateWorkspaceAutostart(cmd.Context(), workspace.ID, codersdk.UpdateWorkspaceAutostartRequest{
Schedule: "",
Schedule: nil,
})
if err != nil {
return err
Expand Down
9 changes: 5 additions & 4 deletions cli/autostart_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/coder/coder/cli/clitest"
"github.com/coder/coder/coderd/coderdtest"
"github.com/coder/coder/coderd/util/ptr"
"github.com/coder/coder/codersdk"
)

Expand All @@ -34,7 +35,7 @@ func TestAutostart(t *testing.T) {
)

err := client.UpdateWorkspaceAutostart(ctx, workspace.ID, codersdk.UpdateWorkspaceAutostartRequest{
Schedule: sched,
Schedule: ptr.Ref(sched),
})
require.NoError(t, err)

Expand Down Expand Up @@ -76,7 +77,7 @@ func TestAutostart(t *testing.T) {
// Ensure autostart schedule updated
updated, err := client.Workspace(ctx, workspace.ID)
require.NoError(t, err, "fetch updated workspace")
require.Equal(t, sched, updated.AutostartSchedule, "expected autostart schedule to be set")
require.Equal(t, sched, *updated.AutostartSchedule, "expected autostart schedule to be set")

// Disable schedule
cmd, root = clitest.New(t, "autostart", "disable", workspace.Name)
Expand All @@ -90,7 +91,7 @@ func TestAutostart(t *testing.T) {
// Ensure autostart schedule updated
updated, err = client.Workspace(ctx, workspace.ID)
require.NoError(t, err, "fetch updated workspace")
require.Empty(t, updated.AutostartSchedule, "expected autostart schedule to not be set")
require.Nil(t, updated.AutostartSchedule, "expected autostart schedule to not be set")
})

t.Run("Enable_NotFound", func(t *testing.T) {
Expand Down Expand Up @@ -155,6 +156,6 @@ func TestAutostart(t *testing.T) {
// Ensure nothing happened
updated, err := client.Workspace(ctx, workspace.ID)
require.NoError(t, err, "fetch updated workspace")
require.Equal(t, expectedSchedule, updated.AutostartSchedule, "expected default autostart schedule")
require.Equal(t, expectedSchedule, *updated.AutostartSchedule, "expected default autostart schedule")
})
}
18 changes: 9 additions & 9 deletions cli/bump_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ func TestBump(t *testing.T) {
expectedDeadline := workspace.LatestBuild.Deadline.Add(90 * time.Minute)

// Assert test invariant: workspace build has a deadline set equal to now plus ttl
require.WithinDuration(t, workspace.LatestBuild.Deadline, time.Now().Add(*workspace.TTL), time.Minute)
require.NoError(t, err)
initDeadline := time.Now().Add(time.Duration(*workspace.TTLMillis) * time.Millisecond)
require.WithinDuration(t, initDeadline, workspace.LatestBuild.Deadline, time.Minute)

cmd, root := clitest.New(t, cmdArgs...)
clitest.SetupConfig(t, client, root)
Expand Down Expand Up @@ -81,8 +81,8 @@ func TestBump(t *testing.T) {
expectedDeadline := workspace.LatestBuild.Deadline.Add(30 * time.Minute)

// Assert test invariant: workspace build has a deadline set equal to now plus ttl
require.WithinDuration(t, workspace.LatestBuild.Deadline, time.Now().Add(*workspace.TTL), time.Minute)
require.NoError(t, err)
initDeadline := time.Now().Add(time.Duration(*workspace.TTLMillis) * time.Millisecond)
require.WithinDuration(t, initDeadline, workspace.LatestBuild.Deadline, time.Minute)

cmd, root := clitest.New(t, cmdArgs...)
clitest.SetupConfig(t, client, root)
Expand Down Expand Up @@ -121,8 +121,8 @@ func TestBump(t *testing.T) {
require.NoError(t, err)

// Assert test invariant: workspace build has a deadline set equal to now plus ttl
require.WithinDuration(t, workspace.LatestBuild.Deadline, time.Now().Add(*workspace.TTL), time.Minute)
require.NoError(t, err)
initDeadline := time.Now().Add(time.Duration(*workspace.TTLMillis) * time.Millisecond)
require.WithinDuration(t, initDeadline, workspace.LatestBuild.Deadline, time.Minute)

cmd, root := clitest.New(t, cmdArgs...)
clitest.SetupConfig(t, client, root)
Expand All @@ -147,7 +147,7 @@ func TestBump(t *testing.T) {
_ = coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
project = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
workspace = coderdtest.CreateWorkspace(t, client, user.OrganizationID, project.ID, func(cwr *codersdk.CreateWorkspaceRequest) {
cwr.TTL = nil
cwr.TTLMillis = nil
})
cmdArgs = []string{"bump", workspace.Name}
stdoutBuf = &bytes.Buffer{}
Expand Down Expand Up @@ -199,8 +199,8 @@ func TestBump(t *testing.T) {
require.NoError(t, err)

// Assert test invariant: workspace build has a deadline set equal to now plus ttl
require.WithinDuration(t, workspace.LatestBuild.Deadline, time.Now().Add(*workspace.TTL), time.Minute)
require.NoError(t, err)
initDeadline := time.Now().Add(time.Duration(*workspace.TTLMillis) * time.Millisecond)
require.WithinDuration(t, initDeadline, workspace.LatestBuild.Deadline, time.Minute)

cmd, root := clitest.New(t, cmdArgs...)
clitest.SetupConfig(t, client, root)
Expand Down
3 changes: 2 additions & 1 deletion cli/create.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/coder/coder/cli/cliflag"
"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 Down Expand Up @@ -226,7 +227,7 @@ func create() *cobra.Command {
TemplateID: template.ID,
Name: workspaceName,
AutostartSchedule: &schedSpec,
TTL: &ttl,
TTLMillis: ptr.Ref(ttl.Milliseconds()),
ParameterValues: parameters,
})
if err != nil {
Expand Down
15 changes: 9 additions & 6 deletions cli/list.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"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 Down Expand Up @@ -87,15 +88,16 @@ func list() *cobra.Command {

duration := time.Now().UTC().Sub(workspace.LatestBuild.Job.CreatedAt).Truncate(time.Second)
autostartDisplay := "-"
if workspace.AutostartSchedule != "" {
if sched, err := schedule.Weekly(workspace.AutostartSchedule); err == nil {
if !ptr.NilOrEmpty(workspace.AutostartSchedule) {
if sched, err := schedule.Weekly(*workspace.AutostartSchedule); err == nil {
autostartDisplay = sched.Cron()
}
}

autostopDisplay := "-"
if workspace.TTL != nil {
autostopDisplay = durationDisplay(*workspace.TTL)
if !ptr.NilOrZero(workspace.TTLMillis) {
dur := time.Duration(*workspace.TTLMillis) * time.Millisecond
autostopDisplay = durationDisplay(dur)
if has, ext := hasExtension(workspace); has {
autostopDisplay += fmt.Sprintf(" (+%s)", durationDisplay(ext.Round(time.Minute)))
}
Expand Down Expand Up @@ -128,10 +130,11 @@ func hasExtension(ws codersdk.Workspace) (bool, time.Duration) {
if ws.LatestBuild.Deadline.IsZero() {
return false, 0
}
if ws.TTL == nil {
if ws.TTLMillis == nil {
return false, 0
}
delta := ws.LatestBuild.Deadline.Add(-*ws.TTL).Sub(ws.LatestBuild.CreatedAt)
ttl := time.Duration(*ws.TTLMillis) * time.Millisecond
delta := ws.LatestBuild.Deadline.Add(-ttl).Sub(ws.LatestBuild.CreatedAt)
if delta < time.Minute {
return false, 0
}
Expand Down
3 changes: 2 additions & 1 deletion cli/ssh.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/coder/coder/cli/cliflag"
"github.com/coder/coder/cli/cliui"
"github.com/coder/coder/coderd/autobuild/notify"
"github.com/coder/coder/coderd/util/ptr"
"github.com/coder/coder/codersdk"
"github.com/coder/coder/cryptorand"
)
Expand Down Expand Up @@ -290,7 +291,7 @@ func notifyCondition(ctx context.Context, client *codersdk.Client, workspaceID u
return time.Time{}, nil
}

if ws.TTL == nil || *ws.TTL == 0 {
if ptr.NilOrZero(ws.TTLMillis) {
return time.Time{}, nil
}

Expand Down
15 changes: 8 additions & 7 deletions cli/ttl.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,13 @@ func ttlShow() *cobra.Command {
return xerrors.Errorf("get workspace: %w", err)
}

if workspace.TTL == nil {
if workspace.TTLMillis == nil || *workspace.TTLMillis == 0 {
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "not set\n")
return nil
}

_, _ = fmt.Fprintf(cmd.OutOrStdout(), "%s\n", workspace.TTL)
dur := time.Duration(*workspace.TTLMillis) * time.Millisecond
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "%s\n", dur)

return nil
},
Expand Down Expand Up @@ -96,10 +97,10 @@ func ttlset() *cobra.Command {
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "warning: ttl rounded down to %s\n", truncated)
}

err = client.UpdateWorkspaceTTL(cmd.Context(), workspace.ID, codersdk.UpdateWorkspaceTTLRequest{
TTL: &truncated,
})
if err != nil {
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)
}

Expand Down Expand Up @@ -130,7 +131,7 @@ func ttlunset() *cobra.Command {
}

err = client.UpdateWorkspaceTTL(cmd.Context(), workspace.ID, codersdk.UpdateWorkspaceTTLRequest{
TTL: nil,
TTLMillis: nil,
})
if err != nil {
return xerrors.Errorf("update workspace ttl: %w", err)
Expand Down
11 changes: 6 additions & 5 deletions cli/ttl_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

"github.com/coder/coder/cli/clitest"
"github.com/coder/coder/coderd/coderdtest"
"github.com/coder/coder/coderd/util/ptr"
"github.com/coder/coder/codersdk"
)

Expand All @@ -34,7 +35,7 @@ func TestTTL(t *testing.T) {
)

err := client.UpdateWorkspaceTTL(ctx, workspace.ID, codersdk.UpdateWorkspaceTTLRequest{
TTL: &ttl,
TTLMillis: ptr.Ref(ttl.Milliseconds()),
})
require.NoError(t, err)

Expand Down Expand Up @@ -73,7 +74,7 @@ func TestTTL(t *testing.T) {
// Ensure ttl updated
updated, err := client.Workspace(ctx, workspace.ID)
require.NoError(t, err, "fetch updated workspace")
require.Equal(t, ttl.Truncate(time.Minute), *updated.TTL)
require.Equal(t, ttl.Truncate(time.Minute), time.Duration(*updated.TTLMillis)*time.Millisecond)
require.Contains(t, stdoutBuf.String(), "warning: ttl rounded down")

// unset schedule
Expand All @@ -87,7 +88,7 @@ func TestTTL(t *testing.T) {
// Ensure ttl updated
updated, err = client.Workspace(ctx, workspace.ID)
require.NoError(t, err, "fetch updated workspace")
require.Nil(t, updated.TTL, "expected ttl to not be set")
require.Nil(t, updated.TTLMillis, "expected ttl to not be set")
})

t.Run("ZeroInvalid", func(t *testing.T) {
Expand Down Expand Up @@ -116,7 +117,7 @@ func TestTTL(t *testing.T) {
// Ensure ttl updated
updated, err := client.Workspace(ctx, workspace.ID)
require.NoError(t, err, "fetch updated workspace")
require.Equal(t, ttl.Truncate(time.Minute), *updated.TTL)
require.Equal(t, ttl.Truncate(time.Minute), time.Duration(*updated.TTLMillis)*time.Millisecond)
require.Contains(t, stdoutBuf.String(), "warning: ttl rounded down")

// A TTL of zero is not considered valid.
Expand All @@ -131,7 +132,7 @@ func TestTTL(t *testing.T) {
// Ensure ttl remains as before
updated, err = client.Workspace(ctx, workspace.ID)
require.NoError(t, err, "fetch updated workspace")
require.Equal(t, ttl.Truncate(time.Minute), *updated.TTL)
require.Equal(t, ttl.Truncate(time.Minute), time.Duration(*updated.TTLMillis)*time.Millisecond)
})

t.Run("Set_NotFound", func(t *testing.T) {
Expand Down
17 changes: 9 additions & 8 deletions coderd/autobuild/executor/lifecycle_executor_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/coder/coder/coderd/autobuild/schedule"
"github.com/coder/coder/coderd/coderdtest"
"github.com/coder/coder/coderd/database"
"github.com/coder/coder/coderd/util/ptr"
"github.com/coder/coder/codersdk"

"github.com/google/uuid"
Expand Down Expand Up @@ -44,7 +45,7 @@ func TestExecutorAutostartOK(t *testing.T) {
sched, err := schedule.Weekly("* * * * *")
require.NoError(t, err)
require.NoError(t, client.UpdateWorkspaceAutostart(ctx, workspace.ID, codersdk.UpdateWorkspaceAutostartRequest{
Schedule: sched.String(),
Schedule: ptr.Ref(sched.String()),
}))

// When: the autobuild executor ticks
Expand Down Expand Up @@ -95,7 +96,7 @@ func TestExecutorAutostartTemplateUpdated(t *testing.T) {
sched, err := schedule.Weekly("* * * * *")
require.NoError(t, err)
require.NoError(t, client.UpdateWorkspaceAutostart(ctx, workspace.ID, codersdk.UpdateWorkspaceAutostartRequest{
Schedule: sched.String(),
Schedule: ptr.Ref(sched.String()),
}))

// When: the autobuild executor ticks
Expand Down Expand Up @@ -138,7 +139,7 @@ func TestExecutorAutostartAlreadyRunning(t *testing.T) {
sched, err := schedule.Weekly("* * * * *")
require.NoError(t, err)
require.NoError(t, client.UpdateWorkspaceAutostart(ctx, workspace.ID, codersdk.UpdateWorkspaceAutostartRequest{
Schedule: sched.String(),
Schedule: ptr.Ref(sched.String()),
}))

// When: the autobuild executor ticks
Expand Down Expand Up @@ -316,12 +317,12 @@ func TestExecutorAutostopNotEnabled(t *testing.T) {
})
// Given: we have a user with a workspace that has no TTL set
workspace = mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) {
cwr.TTL = nil
cwr.TTLMillis = nil
})
)

// Given: workspace has no TTL set
require.Nil(t, workspace.TTL)
require.Nil(t, workspace.TTLMillis)

// Given: workspace is running
require.Equal(t, codersdk.WorkspaceTransitionStart, workspace.LatestBuild.Transition)
Expand Down Expand Up @@ -359,7 +360,7 @@ func TestExecutorWorkspaceDeleted(t *testing.T) {
sched, err := schedule.Weekly("* * * * *")
require.NoError(t, err)
require.NoError(t, client.UpdateWorkspaceAutostart(ctx, workspace.ID, codersdk.UpdateWorkspaceAutostartRequest{
Schedule: sched.String(),
Schedule: ptr.Ref(sched.String()),
}))

// Given: workspace is deleted
Expand Down Expand Up @@ -402,7 +403,7 @@ func TestExecutorWorkspaceAutostartTooEarly(t *testing.T) {
sched, err := schedule.Weekly(futureTimeCron)
require.NoError(t, err)
require.NoError(t, client.UpdateWorkspaceAutostart(ctx, workspace.ID, codersdk.UpdateWorkspaceAutostartRequest{
Schedule: sched.String(),
Schedule: ptr.Ref(sched.String()),
}))

// When: the autobuild executor ticks
Expand Down Expand Up @@ -461,7 +462,7 @@ func TestExecutorWorkspaceAutostopNoWaitChangedMyMind(t *testing.T) {
)

// Given: the user changes their mind and decides their workspace should not auto-stop
err := client.UpdateWorkspaceTTL(ctx, workspace.ID, codersdk.UpdateWorkspaceTTLRequest{TTL: nil})
err := client.UpdateWorkspaceTTL(ctx, workspace.ID, codersdk.UpdateWorkspaceTTLRequest{TTLMillis: nil})
require.NoError(t, err)

// When: the autobuild executor ticks after the deadline
Expand Down

0 comments on commit dcf03d8

Please sign in to comment.