feat: Add `codersdk.NullTime`, change workspace build deadline (#3552)

Fixes #2015

Co-authored-by: Joe Previte <jjprevite@gmail.com>
This commit is contained in:
Mathias Fredriksson 2022-08-25 19:10:42 +03:00 committed by GitHub
parent a21a6d2f4a
commit 78a24941fe
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
18 changed files with 271 additions and 63 deletions

View File

@ -38,8 +38,8 @@ func workspaceListRowFromWorkspace(now time.Time, usersByID map[uuid.UUID]coders
if !ptr.NilOrZero(workspace.TTLMillis) {
dur := time.Duration(*workspace.TTLMillis) * time.Millisecond
autostopDisplay = durationDisplay(dur)
if !workspace.LatestBuild.Deadline.IsZero() && workspace.LatestBuild.Deadline.After(now) && status == "Running" {
remaining := time.Until(workspace.LatestBuild.Deadline)
if !workspace.LatestBuild.Deadline.IsZero() && workspace.LatestBuild.Deadline.Time.After(now) && status == "Running" {
remaining := time.Until(workspace.LatestBuild.Deadline.Time)
autostopDisplay = fmt.Sprintf("%s (%s)", autostopDisplay, relative(remaining))
}
}

View File

@ -280,8 +280,8 @@ func displaySchedule(workspace codersdk.Workspace, out io.Writer) error {
if workspace.LatestBuild.Transition != "start" {
schedNextStop = "-"
} else {
schedNextStop = workspace.LatestBuild.Deadline.In(loc).Format(timeFormat + " on " + dateFormat)
schedNextStop = fmt.Sprintf("%s (in %s)", schedNextStop, durationDisplay(time.Until(workspace.LatestBuild.Deadline)))
schedNextStop = workspace.LatestBuild.Deadline.Time.In(loc).Format(timeFormat + " on " + dateFormat)
schedNextStop = fmt.Sprintf("%s (in %s)", schedNextStop, durationDisplay(time.Until(workspace.LatestBuild.Deadline.Time)))
}
}

View File

@ -239,7 +239,7 @@ func TestScheduleOverride(t *testing.T) {
// Assert test invariant: workspace build has a deadline set equal to now plus ttl
initDeadline := time.Now().Add(time.Duration(*workspace.TTLMillis) * time.Millisecond)
require.WithinDuration(t, initDeadline, workspace.LatestBuild.Deadline, time.Minute)
require.WithinDuration(t, initDeadline, workspace.LatestBuild.Deadline.Time, time.Minute)
cmd, root := clitest.New(t, cmdArgs...)
clitest.SetupConfig(t, client, root)
@ -252,7 +252,7 @@ func TestScheduleOverride(t *testing.T) {
// Then: the deadline of the latest build is updated assuming the units are minutes
updated, err := client.Workspace(ctx, workspace.ID)
require.NoError(t, err)
require.WithinDuration(t, expectedDeadline, updated.LatestBuild.Deadline, time.Minute)
require.WithinDuration(t, expectedDeadline, updated.LatestBuild.Deadline.Time, time.Minute)
})
t.Run("InvalidDuration", func(t *testing.T) {
@ -279,7 +279,7 @@ func TestScheduleOverride(t *testing.T) {
// Assert test invariant: workspace build has a deadline set equal to now plus ttl
initDeadline := time.Now().Add(time.Duration(*workspace.TTLMillis) * time.Millisecond)
require.WithinDuration(t, initDeadline, workspace.LatestBuild.Deadline, time.Minute)
require.WithinDuration(t, initDeadline, workspace.LatestBuild.Deadline.Time, time.Minute)
cmd, root := clitest.New(t, cmdArgs...)
clitest.SetupConfig(t, client, root)

View File

@ -33,8 +33,10 @@ import (
"github.com/coder/coder/peer/peerwg"
)
var workspacePollInterval = time.Minute
var autostopNotifyCountdown = []time.Duration{30 * time.Minute}
var (
workspacePollInterval = time.Minute
autostopNotifyCountdown = []time.Duration{30 * time.Minute}
)
func ssh() *cobra.Command {
var (
@ -385,7 +387,7 @@ func notifyCondition(ctx context.Context, client *codersdk.Client, workspaceID u
return time.Time{}, nil
}
deadline = ws.LatestBuild.Deadline
deadline = ws.LatestBuild.Deadline.Time
callback = func() {
ttl := deadline.Sub(now)
var title, body string

View File

@ -193,7 +193,7 @@ func TestExecutorAutostopOK(t *testing.T) {
// When: the autobuild executor ticks *after* the deadline:
go func() {
tickCh <- workspace.LatestBuild.Deadline.Add(time.Minute)
tickCh <- workspace.LatestBuild.Deadline.Time.Add(time.Minute)
close(tickCh)
}()
@ -229,7 +229,7 @@ func TestExecutorAutostopExtend(t *testing.T) {
require.NotZero(t, originalDeadline)
// Given: we extend the workspace deadline
newDeadline := originalDeadline.Add(30 * time.Minute)
newDeadline := originalDeadline.Time.Add(30 * time.Minute)
err := client.PutExtendWorkspace(ctx, workspace.ID, codersdk.PutExtendWorkspaceRequest{
Deadline: newDeadline,
})
@ -237,7 +237,7 @@ func TestExecutorAutostopExtend(t *testing.T) {
// When: the autobuild executor ticks *after* the original deadline:
go func() {
tickCh <- originalDeadline.Add(time.Minute)
tickCh <- originalDeadline.Time.Add(time.Minute)
}()
// Then: nothing should happen and the workspace should stay running
@ -281,7 +281,7 @@ func TestExecutorAutostopAlreadyStopped(t *testing.T) {
// When: the autobuild executor ticks past the TTL
go func() {
tickCh <- workspace.LatestBuild.Deadline.Add(time.Minute)
tickCh <- workspace.LatestBuild.Deadline.Time.Add(time.Minute)
close(tickCh)
}()
@ -323,7 +323,7 @@ func TestExecutorAutostopNotEnabled(t *testing.T) {
// When: the autobuild executor ticks past the TTL
go func() {
tickCh <- workspace.LatestBuild.Deadline.Add(time.Minute)
tickCh <- workspace.LatestBuild.Deadline.Time.Add(time.Minute)
close(tickCh)
}()
@ -415,7 +415,7 @@ func TestExecutorWorkspaceAutostopBeforeDeadline(t *testing.T) {
// When: the autobuild executor ticks before the TTL
go func() {
tickCh <- workspace.LatestBuild.Deadline.Add(-1 * time.Minute)
tickCh <- workspace.LatestBuild.Deadline.Time.Add(-1 * time.Minute)
close(tickCh)
}()
@ -447,11 +447,11 @@ func TestExecutorWorkspaceAutostopNoWaitChangedMyMind(t *testing.T) {
// Then: the deadline should still be the original value
updated := coderdtest.MustWorkspace(t, client, workspace.ID)
assert.WithinDuration(t, workspace.LatestBuild.Deadline, updated.LatestBuild.Deadline, time.Minute)
assert.WithinDuration(t, workspace.LatestBuild.Deadline.Time, updated.LatestBuild.Deadline.Time, time.Minute)
// When: the autobuild executor ticks after the original deadline
go func() {
tickCh <- workspace.LatestBuild.Deadline.Add(time.Minute)
tickCh <- workspace.LatestBuild.Deadline.Time.Add(time.Minute)
}()
// Then: the workspace should stop
@ -478,7 +478,7 @@ func TestExecutorWorkspaceAutostopNoWaitChangedMyMind(t *testing.T) {
// When: the relentless onward march of time continues
go func() {
tickCh <- workspace.LatestBuild.Deadline.Add(newTTL + time.Minute)
tickCh <- workspace.LatestBuild.Deadline.Time.Add(newTTL + time.Minute)
close(tickCh)
}()

View File

@ -638,7 +638,8 @@ func convertWorkspaceBuild(
buildInitiator *database.User,
workspace database.Workspace,
workspaceBuild database.WorkspaceBuild,
job database.ProvisionerJob) codersdk.WorkspaceBuild {
job database.ProvisionerJob,
) codersdk.WorkspaceBuild {
//nolint:unconvert
if workspace.ID != workspaceBuild.WorkspaceID {
panic("workspace and build do not match")
@ -671,7 +672,7 @@ func convertWorkspaceBuild(
InitiatorID: workspaceBuild.InitiatorID,
InitiatorUsername: initiatorName,
Job: convertProvisionerJob(job),
Deadline: workspaceBuild.Deadline,
Deadline: codersdk.NewNullTime(workspaceBuild.Deadline, !workspaceBuild.Deadline.IsZero()),
Reason: codersdk.BuildReason(workspaceBuild.Reason),
}
}

View File

@ -1169,7 +1169,7 @@ func TestWorkspaceExtend(t *testing.T) {
workspace, err := client.Workspace(ctx, workspace.ID)
require.NoError(t, err, "fetch provisioned workspace")
oldDeadline := workspace.LatestBuild.Deadline
oldDeadline := workspace.LatestBuild.Deadline.Time
// Updating the deadline should succeed
req := codersdk.PutExtendWorkspaceRequest{
@ -1181,7 +1181,7 @@ func TestWorkspaceExtend(t *testing.T) {
// Ensure deadline set correctly
updated, err := client.Workspace(ctx, workspace.ID)
require.NoError(t, err, "failed to fetch updated workspace")
require.WithinDuration(t, newDeadline, updated.LatestBuild.Deadline, time.Minute)
require.WithinDuration(t, newDeadline, updated.LatestBuild.Deadline.Time, time.Minute)
// Zero time should fail
err = client.PutExtendWorkspace(ctx, workspace.ID, codersdk.PutExtendWorkspaceRequest{
@ -1220,7 +1220,7 @@ func TestWorkspaceExtend(t *testing.T) {
// Ensure deadline still set correctly
updated, err = client.Workspace(ctx, workspace.ID)
require.NoError(t, err, "failed to fetch updated workspace")
require.WithinDuration(t, oldDeadline.Add(-time.Hour), updated.LatestBuild.Deadline, time.Minute)
require.WithinDuration(t, oldDeadline.Add(-time.Hour), updated.LatestBuild.Deadline.Time, time.Minute)
}
func TestWorkspaceWatcher(t *testing.T) {

59
codersdk/time.go Normal file
View File

@ -0,0 +1,59 @@
package codersdk
import (
"bytes"
"database/sql"
"encoding/json"
"time"
"golang.org/x/xerrors"
)
var nullBytes = []byte("null")
// NullTime represents a nullable time.Time.
// @typescript-ignore NullTime
type NullTime struct {
sql.NullTime
}
// NewNullTime returns a new NullTime with the given time.Time.
func NewNullTime(t time.Time, valid bool) NullTime {
return NullTime{
NullTime: sql.NullTime{
Time: t,
Valid: valid,
},
}
}
// MarshalJSON implements json.Marshaler.
func (t NullTime) MarshalJSON() ([]byte, error) {
if !t.Valid {
return []byte("null"), nil
}
b, err := t.Time.MarshalJSON()
if err != nil {
return nil, xerrors.Errorf("codersdk.NullTime: json encode failed: %w", err)
}
return b, nil
}
// UnmarshalJSON implements json.Unmarshaler.
func (t *NullTime) UnmarshalJSON(data []byte) error {
t.Valid = false
if bytes.Equal(data, nullBytes) {
return nil
}
err := json.Unmarshal(data, &t.Time)
if err != nil {
return xerrors.Errorf("codersdk.NullTime: json decode failed: %w", err)
}
t.Valid = true
return nil
}
// IsZero return true if the time is null or zero.
func (t NullTime) IsZero() bool {
return !t.Valid || t.Time.IsZero()
}

156
codersdk/time_test.go Normal file
View File

@ -0,0 +1,156 @@
package codersdk_test
import (
"database/sql"
"encoding/json"
"fmt"
"testing"
"time"
"github.com/stretchr/testify/require"
"github.com/coder/coder/codersdk"
)
func TestNullTime_MarshalJSON(t *testing.T) {
t.Parallel()
t1, err := time.Parse(time.RFC3339, "2022-08-18T00:00:00Z")
require.NoError(t, err)
bt1, err := json.Marshal(t1)
require.NoError(t, err)
tests := []struct {
name string
input sql.NullTime
want string
}{
{
name: "valid zero",
input: sql.NullTime{Valid: true},
want: `"0001-01-01T00:00:00Z"`,
},
{
name: "invalid zero",
input: sql.NullTime{Valid: false},
want: "null",
},
{
name: "valid time",
input: sql.NullTime{Time: t1, Valid: true},
want: string(bt1),
},
{
name: "null time",
input: sql.NullTime{Time: t1, Valid: false},
want: "null",
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
tr := codersdk.NewNullTime(tt.input.Time, tt.input.Valid)
got, err := tr.MarshalJSON()
require.NoError(t, err)
require.Equal(t, tt.want, string(got))
})
}
}
func TestNullTime_UnmarshalJSON(t *testing.T) {
t.Parallel()
t1, err := time.Parse(time.RFC3339, "2022-08-18T00:00:00Z")
require.NoError(t, err)
bt1, err := json.Marshal(t1)
require.NoError(t, err)
type request struct {
Time codersdk.NullTime `json:"time"`
}
tests := []struct {
name string
data string
want codersdk.NullTime
wantErr bool
}{
{
name: "null",
data: `{"time": null}`,
want: codersdk.NullTime{},
},
{
name: "empty",
data: `{}`,
want: codersdk.NullTime{},
},
{
name: "empty string",
data: `{"time": ""}`,
wantErr: true,
},
{
name: "valid time",
data: fmt.Sprintf(`{"time": %s}`, bt1),
want: codersdk.NewNullTime(t1, true),
},
{
name: "invalid time",
data: fmt.Sprintf(`{"time": %q}`, `2022-08-18T00:00:00`),
wantErr: true,
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
var req request
err := json.Unmarshal([]byte(tt.data), &req)
if tt.wantErr {
require.Error(t, err)
return
}
require.NoError(t, err)
require.Equal(t, tt.want, req.Time)
})
}
}
func TestNullTime_IsZero(t *testing.T) {
t.Parallel()
tests := []struct {
name string
input sql.NullTime
want bool
}{
{
name: "zero",
input: sql.NullTime{},
want: true,
},
{
name: "not zero",
input: sql.NullTime{Time: time.Now(), Valid: true},
want: false,
},
{
name: "null is zero",
input: sql.NullTime{Time: time.Now(), Valid: false},
want: true,
},
}
for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
tr := codersdk.NullTime{NullTime: tt.input}
require.Equal(t, tt.want, tr.IsZero())
})
}
}

View File

@ -50,7 +50,7 @@ type WorkspaceBuild struct {
InitiatorID uuid.UUID `json:"initiator_id"`
InitiatorUsername string `json:"initiator_name"`
Job ProvisionerJob `json:"job"`
Deadline time.Time `json:"deadline"`
Deadline NullTime `json:"deadline,omitempty"`
Reason BuildReason `db:"reason" json:"reason"`
}

View File

@ -415,15 +415,6 @@ func (g *Generator) typescriptType(ty types.Type) (TypescriptType, error) {
}
case *types.Named:
n := ty
// First see if the type is defined elsewhere. If it is, we can just
// put the name as it will be defined in the typescript codeblock
// we generate.
name := n.Obj().Name()
if obj := g.pkg.Types.Scope().Lookup(name); obj != nil {
// Sweet! Using other typescript types as fields. This could be an
// enum or another struct
return TypescriptType{ValueType: name}, nil
}
// These are external named types that we handle uniquely.
switch n.String() {
@ -434,12 +425,24 @@ func (g *Generator) typescriptType(ty types.Type) (TypescriptType, error) {
return TypescriptType{ValueType: "string"}, nil
case "database/sql.NullTime":
return TypescriptType{ValueType: "string", Optional: true}, nil
case "github.com/coder/coder/codersdk.NullTime":
return TypescriptType{ValueType: "string", Optional: true}, nil
case "github.com/google/uuid.NullUUID":
return TypescriptType{ValueType: "string", Optional: true}, nil
case "github.com/google/uuid.UUID":
return TypescriptType{ValueType: "string"}, nil
}
// Then see if the type is defined elsewhere. If it is, we can just
// put the name as it will be defined in the typescript codeblock
// we generate.
name := n.Obj().Name()
if obj := g.pkg.Types.Scope().Lookup(name); obj != nil {
// Sweet! Using other typescript types as fields. This could be an
// enum or another struct
return TypescriptType{ValueType: name}, nil
}
// If it's a struct, just use the name of the struct type
if _, ok := n.Underlying().(*types.Struct); ok {
return TypescriptType{ValueType: "any", AboveTypeLine: fmt.Sprintf("%s\n%s",

View File

@ -527,7 +527,7 @@ export interface WorkspaceBuild {
readonly initiator_id: string
readonly initiator_name: string
readonly job: ProvisionerJob
readonly deadline: string
readonly deadline?: string
readonly reason: BuildReason
}

View File

@ -38,9 +38,7 @@ NoTTL.args = {
...Mocks.MockWorkspace,
latest_build: {
...Mocks.MockWorkspaceBuild,
// a manual shutdown has a deadline of '"0001-01-01T00:00:00Z"'
// SEE: #1834
deadline: "0001-01-01T00:00:00Z",
deadline: undefined,
},
ttl_ms: undefined,
},

View File

@ -10,12 +10,12 @@ describe("WorkspaceScheduleBanner", () => {
describe("shouldDisplay", () => {
// Manual TTL case
it("should not display if the build does not have a deadline", () => {
// Given: a workspace with deadline of '"0001-01-01T00:00:00Z"'
// Given: a workspace with deadline of undefined.
const workspace: TypesGen.Workspace = {
...Mocks.MockWorkspace,
latest_build: {
...Mocks.MockWorkspaceBuild,
deadline: "0001-01-01T00:00:00Z",
deadline: undefined,
transition: "start",
},
}

View File

@ -23,16 +23,12 @@ export interface WorkspaceScheduleBannerProps {
}
export const shouldDisplay = (workspace: TypesGen.Workspace): boolean => {
if (!isWorkspaceOn(workspace)) {
if (!isWorkspaceOn(workspace) || !workspace.latest_build.deadline) {
return false
} else {
// 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 thirtyMinutesFromNow = dayjs().add(30, "minutes").utc()
return hasDeadline && deadline.isSameOrBefore(thirtyMinutesFromNow)
}
const deadline = dayjs(workspace.latest_build.deadline).utc()
const thirtyMinutesFromNow = dayjs().add(30, "minutes").utc()
return deadline.isSameOrBefore(thirtyMinutesFromNow)
}
export const WorkspaceScheduleBanner: FC<React.PropsWithChildren<WorkspaceScheduleBannerProps>> = ({

View File

@ -40,9 +40,7 @@ NoTTL.args = {
...Mocks.MockWorkspace,
latest_build: {
...Mocks.MockWorkspaceBuild,
// a manual shutdown has a deadline of '"0001-01-01T00:00:00Z"'
// SEE: #1834
deadline: "0001-01-01T00:00:00Z",
deadline: undefined,
},
ttl_ms: undefined,
},

View File

@ -28,11 +28,7 @@ dayjs.extend(relativeTime)
dayjs.extend(timezone)
export const shouldDisplayPlusMinus = (workspace: Workspace): boolean => {
if (!isWorkspaceOn(workspace)) {
return false
}
const deadline = dayjs(workspace.latest_build.deadline).utc()
return deadline.year() > 1
return isWorkspaceOn(workspace) && Boolean(workspace.latest_build.deadline)
}
export interface WorkspaceScheduleButtonProps {

View File

@ -73,27 +73,26 @@ export const autoStartDisplay = (schedule: string | undefined): string => {
export const isShuttingDown = (workspace: Workspace, deadline?: Dayjs): boolean => {
if (!deadline) {
if (!workspace.latest_build.deadline) {
return false
}
deadline = dayjs(workspace.latest_build.deadline).utc()
}
const hasDeadline = deadline.year() > 1
const now = dayjs().utc()
return isWorkspaceOn(workspace) && hasDeadline && now.isAfter(deadline)
return isWorkspaceOn(workspace) && now.isAfter(deadline)
}
export const autoStopDisplay = (workspace: Workspace): string => {
const deadline = dayjs(workspace.latest_build.deadline).utc()
// a manual shutdown has a deadline of '"0001-01-01T00:00:00Z"'
// SEE: #1834
const hasDeadline = deadline.year() > 1
const ttl = workspace.ttl_ms
if (isWorkspaceOn(workspace) && hasDeadline) {
if (isWorkspaceOn(workspace) && workspace.latest_build.deadline) {
// Workspace is on --> derive from latest_build.deadline. Note that the
// user may modify their workspace object (ttl) while the workspace is
// running and depending on system semantics, the deadline may still
// represent the previously defined ttl. Thus, we always derive from the
// deadline as the source of truth.
const deadline = dayjs(workspace.latest_build.deadline).utc()
if (isShuttingDown(workspace, deadline)) {
return Language.workspaceShuttingDownLabel
} else {