From 17134311a033d075a9b844536d3f8b482e41e8ea Mon Sep 17 00:00:00 2001 From: johnstcn Date: Thu, 2 Jun 2022 11:13:08 +0000 Subject: [PATCH 01/19] RED: add failing unit tests --- cli/autostart_test.go | 29 +++++++++++++++++ cli/create_test.go | 58 +++++++++++++++++++++++++++++++++ cli/ttl_test.go | 33 +++++++++++++++++++ coderd/coderdtest/coderdtest.go | 12 ++++--- coderd/workspaces_test.go | 20 +++++++++--- codersdk/organizations.go | 9 +++++ 6 files changed, 153 insertions(+), 8 deletions(-) diff --git a/cli/autostart_test.go b/cli/autostart_test.go index dd17105f3f9a9..e369200c83058 100644 --- a/cli/autostart_test.go +++ b/cli/autostart_test.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "testing" + "time" "github.com/stretchr/testify/require" @@ -158,4 +159,32 @@ func TestAutostart(t *testing.T) { require.NoError(t, err, "fetch updated workspace") require.Equal(t, expectedSchedule, *updated.AutostartSchedule, "expected default autostart schedule") }) + + t.Run("BelowTemplateConstraint", func(t *testing.T) { + t.Parallel() + + var ( + ctx = context.Background() + client = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) + user = coderdtest.CreateFirstUser(t, client) + version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + _ = coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + project = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) { + ctr.MinAutostartIntervalMillis = ptr.Ref(time.Hour.Milliseconds()) + }) + workspace = coderdtest.CreateWorkspace(t, client, user.OrganizationID, project.ID) + cmdArgs = []string{"autostart", "enable", workspace.Name, "--minute", "*", "--hour", "*"} + ) + + cmd, root := clitest.New(t, cmdArgs...) + clitest.SetupConfig(t, client, root) + + err := cmd.Execute() + require.NoError(t, err, "unexpected error") + + // Ensure nothing happened + updated, err := client.Workspace(ctx, workspace.ID) + require.NoError(t, err, "fetch updated workspace") + require.Equal(t, *workspace.AutostartSchedule, *updated.AutostartSchedule, "expected previous autostart schedule") + }) } diff --git a/cli/create_test.go b/cli/create_test.go index 9675a68fc3139..9172b08c1f402 100644 --- a/cli/create_test.go +++ b/cli/create_test.go @@ -14,6 +14,7 @@ import ( "github.com/coder/coder/cli/clitest" "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/coder/coder/provisioner/echo" "github.com/coder/coder/provisionersdk/proto" @@ -62,6 +63,63 @@ func TestCreate(t *testing.T) { <-doneChan }) + t.Run("AboveTemplateMaxTTL", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) + user := coderdtest.CreateFirstUser(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) { + ctr.MaxTTLMillis = ptr.Ref((12 * time.Hour).Milliseconds()) + }) + args := []string{ + "create", + "my-workspace", + "--template", template.Name, + "--ttl", "24h", + } + cmd, root := clitest.New(t, args...) + clitest.SetupConfig(t, client, root) + errCh := make(chan error) + pty := ptytest.New(t) + cmd.SetIn(pty.Input()) + cmd.SetOut(pty.Output()) + go func() { + defer close(errCh) + errCh <- cmd.Execute() + }() + require.EqualError(t, <-errCh, "TODO what is the error") + }) + + t.Run("BelowTemplateMinAutostartInterval", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) + user := coderdtest.CreateFirstUser(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) { + ctr.MinAutostartIntervalMillis = ptr.Ref(time.Hour.Milliseconds()) + }) + args := []string{ + "create", + "my-workspace", + "--template", template.Name, + "--autostart-minute", "*", // Every minute + "--autostart-hour", "*", // Every hour + } + cmd, root := clitest.New(t, args...) + clitest.SetupConfig(t, client, root) + errCh := make(chan error) + pty := ptytest.New(t) + cmd.SetIn(pty.Input()) + cmd.SetOut(pty.Output()) + go func() { + defer close(errCh) + errCh <- cmd.Execute() + }() + require.EqualError(t, <-errCh, "TODO what is the error") + }) + t.Run("CreateErrInvalidTz", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) diff --git a/cli/ttl_test.go b/cli/ttl_test.go index 7efe03ce27b7d..fab444200d541 100644 --- a/cli/ttl_test.go +++ b/cli/ttl_test.go @@ -168,4 +168,37 @@ func TestTTL(t *testing.T) { err := cmd.Execute() require.ErrorContains(t, err, "status code 403: forbidden", "unexpected error") }) + + t.Run("TemplateMaxTTL", func(t *testing.T) { + t.Parallel() + + var ( + ctx = context.Background() + client = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) + user = coderdtest.CreateFirstUser(t, client) + version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + _ = coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + project = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) { + ctr.MaxTTLMillis = ptr.Ref((8 * time.Hour).Milliseconds()) + }) + workspace = coderdtest.CreateWorkspace(t, client, user.OrganizationID, project.ID, func(cwr *codersdk.CreateWorkspaceRequest) { + cwr.TTLMillis = ptr.Ref((8 * time.Hour).Milliseconds()) + }) + cmdArgs = []string{"ttl", "set", workspace.Name, "24h"} + stdoutBuf = &bytes.Buffer{} + ) + + cmd, root := clitest.New(t, cmdArgs...) + clitest.SetupConfig(t, client, root) + cmd.SetOut(stdoutBuf) + + err := cmd.Execute() + require.EqualError(t, err, "TODO what is the error") + + // Ensure ttl not updated + updated, err := client.Workspace(ctx, workspace.ID) + require.NoError(t, err, "fetch updated workspace") + require.NotNil(t, updated.TTLMillis) + require.Equal(t, (8 * time.Hour).Milliseconds, *updated.TTLMillis) + }) } diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index c103dde98c13b..1be867af2c5f7 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -322,12 +322,16 @@ func CreateWorkspaceBuild( // CreateTemplate creates a template with the "echo" provisioner for // compatibility with testing. The name assigned is randomly generated. -func CreateTemplate(t *testing.T, client *codersdk.Client, organization uuid.UUID, version uuid.UUID) codersdk.Template { - template, err := client.CreateTemplate(context.Background(), organization, codersdk.CreateTemplateRequest{ +func CreateTemplate(t *testing.T, client *codersdk.Client, organization uuid.UUID, version uuid.UUID, mutators ...func(*codersdk.CreateTemplateRequest)) codersdk.Template { + req := codersdk.CreateTemplateRequest{ Name: randomUsername(), Description: randomUsername(), VersionID: version, - }) + } + for _, mut := range mutators { + mut(&req) + } + template, err := client.CreateTemplate(context.Background(), organization, req) require.NoError(t, err) return template } @@ -400,7 +404,7 @@ func CreateWorkspace(t *testing.T, client *codersdk.Client, organization uuid.UU req := codersdk.CreateWorkspaceRequest{ TemplateID: templateID, Name: randomUsername(), - AutostartSchedule: ptr.Ref("CRON_TZ=US/Central * * * * *"), + AutostartSchedule: ptr.Ref("CRON_TZ=US/Central 30 9 * * 1-5"), TTLMillis: ptr.Ref((8 * time.Hour).Milliseconds()), } for _, mutator := range mutators { diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 392007dcdf18a..0c923e415b892 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -588,9 +588,10 @@ func TestWorkspaceUpdateTTL(t *testing.T) { t.Parallel() testCases := []struct { - name string - ttlMillis *int64 - expectedError string + name string + ttlMillis *int64 + expectedError string + modifyTemplate func(*codersdk.CreateTemplateRequest) }{ { name: "disable ttl", @@ -617,19 +618,30 @@ func TestWorkspaceUpdateTTL(t *testing.T) { ttlMillis: ptr.Ref((24*7*time.Hour + time.Minute).Milliseconds()), expectedError: "ttl must be less than 7 days", }, + { + name: "above template maximum ttl", + ttlMillis: ptr.Ref((12*time.Hour + time.Minute).Milliseconds()), + expectedError: "TODO what is the error", + modifyTemplate: func(ctr *codersdk.CreateTemplateRequest) { ctr.MaxTTLMillis = ptr.Ref((8 * time.Hour).Milliseconds()) }, + }, } for _, testCase := range testCases { testCase := testCase t.Run(testCase.name, func(t *testing.T) { t.Parallel() + + mutators := make([]func(*codersdk.CreateTemplateRequest), 0) + if testCase.modifyTemplate != nil { + mutators = append(mutators, testCase.modifyTemplate) + } var ( ctx = context.Background() client = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) user = coderdtest.CreateFirstUser(t, client) version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) _ = coderdtest.AwaitTemplateVersionJob(t, client, version.ID) - project = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + project = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, mutators...) workspace = coderdtest.CreateWorkspace(t, client, user.OrganizationID, project.ID, func(cwr *codersdk.CreateWorkspaceRequest) { cwr.AutostartSchedule = nil cwr.TTLMillis = nil diff --git a/codersdk/organizations.go b/codersdk/organizations.go index c252acc524eff..6eebc4fbc100f 100644 --- a/codersdk/organizations.go +++ b/codersdk/organizations.go @@ -61,6 +61,15 @@ type CreateTemplateRequest struct { // templates, but it doesn't make sense for users. VersionID uuid.UUID `json:"template_version_id" validate:"required"` ParameterValues []CreateParameterRequest `json:"parameter_values,omitempty"` + + // MaxTTLMillis allows optionally specifying the maximum allowable TTL + // for all workspaces created from this template. + MaxTTLMillis *int64 `json:"max_ttl_ms,omitempty"` + + // MinAutostartIntervalMillis allows optionally specifying the minimum + // allowable duration between autostarts for all workspaces created from + // this template. + MinAutostartIntervalMillis *int64 `json:"min_autostart_interval_ms,omitempty"` } // CreateWorkspaceRequest provides options for creating a new workspace. From f11f942858f8b4f4d290b2f4676a8a31fa42add3 Mon Sep 17 00:00:00 2001 From: johnstcn Date: Thu, 2 Jun 2022 13:02:59 +0000 Subject: [PATCH 02/19] RED: add migrations and make gen --- coderd/database/dump.sql | 4 +++- ...19_template_autobuild_constraints.down.sql | 2 ++ ...0019_template_autobuild_constraints.up.sql | 2 ++ coderd/database/models.go | 20 ++++++++++--------- coderd/database/queries.sql.go | 20 ++++++++++++++----- site/src/api/typesGenerated.ts | 4 +++- 6 files changed, 36 insertions(+), 16 deletions(-) create mode 100644 coderd/database/migrations/000019_template_autobuild_constraints.down.sql create mode 100644 coderd/database/migrations/000019_template_autobuild_constraints.up.sql diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index acba309a9c4d4..1a85f094fd2fd 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -248,7 +248,9 @@ CREATE TABLE templates ( name character varying(64) NOT NULL, provisioner provisioner_type NOT NULL, active_version_id uuid NOT NULL, - description character varying(128) DEFAULT ''::character varying NOT NULL + description character varying(128) DEFAULT ''::character varying NOT NULL, + max_ttl bigint DEFAULT '604800000000000'::bigint NOT NULL, + min_autostart_interval bigint DEFAULT '3600000000000'::bigint NOT NULL ); CREATE TABLE users ( diff --git a/coderd/database/migrations/000019_template_autobuild_constraints.down.sql b/coderd/database/migrations/000019_template_autobuild_constraints.down.sql new file mode 100644 index 0000000000000..e03c22a7952c6 --- /dev/null +++ b/coderd/database/migrations/000019_template_autobuild_constraints.down.sql @@ -0,0 +1,2 @@ +ALTER TABLE ONLY templates DROP COLUMN IF EXISTS max_ttl; +ALTER TABLE ONLY templates DROP COLUMN IF EXISTS min_autostart_interval; diff --git a/coderd/database/migrations/000019_template_autobuild_constraints.up.sql b/coderd/database/migrations/000019_template_autobuild_constraints.up.sql new file mode 100644 index 0000000000000..e38eacf872cf5 --- /dev/null +++ b/coderd/database/migrations/000019_template_autobuild_constraints.up.sql @@ -0,0 +1,2 @@ +ALTER TABLE ONLY templates ADD COLUMN IF NOT EXISTS max_ttl BIGINT NOT NULL DEFAULT 604800000000000; -- 168 hours +ALTER TABLE ONLY templates ADD COLUMN IF NOT EXISTS min_autostart_interval BIGINT NOT NULL DEFAULT 3600000000000; -- 1 hour diff --git a/coderd/database/models.go b/coderd/database/models.go index 6b0f25d2ea554..880737ca101e1 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -429,15 +429,17 @@ type ProvisionerJobLog struct { } type Template struct { - ID uuid.UUID `db:"id" json:"id"` - CreatedAt time.Time `db:"created_at" json:"created_at"` - UpdatedAt time.Time `db:"updated_at" json:"updated_at"` - OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` - Deleted bool `db:"deleted" json:"deleted"` - Name string `db:"name" json:"name"` - Provisioner ProvisionerType `db:"provisioner" json:"provisioner"` - ActiveVersionID uuid.UUID `db:"active_version_id" json:"active_version_id"` - Description string `db:"description" json:"description"` + ID uuid.UUID `db:"id" json:"id"` + CreatedAt time.Time `db:"created_at" json:"created_at"` + UpdatedAt time.Time `db:"updated_at" json:"updated_at"` + OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` + Deleted bool `db:"deleted" json:"deleted"` + Name string `db:"name" json:"name"` + Provisioner ProvisionerType `db:"provisioner" json:"provisioner"` + ActiveVersionID uuid.UUID `db:"active_version_id" json:"active_version_id"` + Description string `db:"description" json:"description"` + MaxTtl int64 `db:"max_ttl" json:"max_ttl"` + MinAutostartInterval int64 `db:"min_autostart_interval" json:"min_autostart_interval"` } type TemplateVersion struct { diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 4d98d9ee2c4a8..fb77d1e0e657c 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -1603,7 +1603,7 @@ func (q *sqlQuerier) UpdateProvisionerJobWithCompleteByID(ctx context.Context, a const getTemplateByID = `-- name: GetTemplateByID :one SELECT - id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description + id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, max_ttl, min_autostart_interval FROM templates WHERE @@ -1625,13 +1625,15 @@ func (q *sqlQuerier) GetTemplateByID(ctx context.Context, id uuid.UUID) (Templat &i.Provisioner, &i.ActiveVersionID, &i.Description, + &i.MaxTtl, + &i.MinAutostartInterval, ) return i, err } const getTemplateByOrganizationAndName = `-- name: GetTemplateByOrganizationAndName :one SELECT - id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description + id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, max_ttl, min_autostart_interval FROM templates WHERE @@ -1661,13 +1663,15 @@ func (q *sqlQuerier) GetTemplateByOrganizationAndName(ctx context.Context, arg G &i.Provisioner, &i.ActiveVersionID, &i.Description, + &i.MaxTtl, + &i.MinAutostartInterval, ) return i, err } const getTemplatesByIDs = `-- name: GetTemplatesByIDs :many SELECT - id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description + id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, max_ttl, min_autostart_interval FROM templates WHERE @@ -1693,6 +1697,8 @@ func (q *sqlQuerier) GetTemplatesByIDs(ctx context.Context, ids []uuid.UUID) ([] &i.Provisioner, &i.ActiveVersionID, &i.Description, + &i.MaxTtl, + &i.MinAutostartInterval, ); err != nil { return nil, err } @@ -1709,7 +1715,7 @@ func (q *sqlQuerier) GetTemplatesByIDs(ctx context.Context, ids []uuid.UUID) ([] const getTemplatesByOrganization = `-- name: GetTemplatesByOrganization :many SELECT - id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description + id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, max_ttl, min_autostart_interval FROM templates WHERE @@ -1741,6 +1747,8 @@ func (q *sqlQuerier) GetTemplatesByOrganization(ctx context.Context, arg GetTemp &i.Provisioner, &i.ActiveVersionID, &i.Description, + &i.MaxTtl, + &i.MinAutostartInterval, ); err != nil { return nil, err } @@ -1768,7 +1776,7 @@ INSERT INTO description ) VALUES - ($1, $2, $3, $4, $5, $6, $7, $8) RETURNING id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description + ($1, $2, $3, $4, $5, $6, $7, $8) RETURNING id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, max_ttl, min_autostart_interval ` type InsertTemplateParams struct { @@ -1804,6 +1812,8 @@ func (q *sqlQuerier) InsertTemplate(ctx context.Context, arg InsertTemplateParam &i.Provisioner, &i.ActiveVersionID, &i.Description, + &i.MaxTtl, + &i.MinAutostartInterval, ) return i, err } diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index d0ce2c3fac371..1e0c08011711c 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -63,6 +63,8 @@ export interface CreateTemplateRequest { readonly description?: string readonly template_version_id: string readonly parameter_values?: CreateParameterRequest[] + readonly max_ttl_ms?: number + readonly min_autostart_interval_ms?: number } // From codersdk/templateversions.go:121:6 @@ -96,7 +98,7 @@ export interface CreateWorkspaceBuildRequest { readonly state?: string } -// From codersdk/organizations.go:67:6 +// From codersdk/organizations.go:76:6 export interface CreateWorkspaceRequest { readonly template_id: string readonly name: string From c5c9a7a3486ff8f59398774ebf99ac101726306d Mon Sep 17 00:00:00 2001 From: johnstcn Date: Thu, 2 Jun 2022 13:33:08 +0000 Subject: [PATCH 03/19] ORANGE: fix audit/diff tests --- coderd/audit/diff_test.go | 30 +++++++++++++++++------------- coderd/audit/table.go | 20 +++++++++++--------- 2 files changed, 28 insertions(+), 22 deletions(-) diff --git a/coderd/audit/diff_test.go b/coderd/audit/diff_test.go index 50bffa3b0d3a1..53f2110f07c26 100644 --- a/coderd/audit/diff_test.go +++ b/coderd/audit/diff_test.go @@ -78,21 +78,25 @@ func TestDiff(t *testing.T) { name: "Create", left: audit.Empty[database.Template](), right: database.Template{ - ID: uuid.UUID{1}, - CreatedAt: time.Now(), - UpdatedAt: time.Now(), - OrganizationID: uuid.UUID{2}, - Deleted: false, - Name: "rust", - Provisioner: database.ProvisionerTypeTerraform, - ActiveVersionID: uuid.UUID{3}, + ID: uuid.UUID{1}, + CreatedAt: time.Now(), + UpdatedAt: time.Now(), + OrganizationID: uuid.UUID{2}, + Deleted: false, + Name: "rust", + Provisioner: database.ProvisionerTypeTerraform, + ActiveVersionID: uuid.UUID{3}, + MaxTtl: int64(time.Hour), + MinAutostartInterval: int64(time.Minute), }, exp: audit.Map{ - "id": uuid.UUID{1}.String(), - "organization_id": uuid.UUID{2}.String(), - "name": "rust", - "provisioner": database.ProvisionerTypeTerraform, - "active_version_id": uuid.UUID{3}.String(), + "id": uuid.UUID{1}.String(), + "organization_id": uuid.UUID{2}.String(), + "name": "rust", + "provisioner": database.ProvisionerTypeTerraform, + "active_version_id": uuid.UUID{3}.String(), + "max_ttl": int64(3600000000000), + "min_autostart_interval": int64(60000000000), }, }, }) diff --git a/coderd/audit/table.go b/coderd/audit/table.go index 0a2f9c1795dda..efdf0de1d6431 100644 --- a/coderd/audit/table.go +++ b/coderd/audit/table.go @@ -61,15 +61,17 @@ var AuditableResources = auditMap(map[any]map[string]Action{ "updated_at": ActionIgnore, // Changes, but is implicit and not helpful in a diff. }, &database.Template{}: { - "id": ActionTrack, - "created_at": ActionIgnore, // Never changes, but is implicit and not helpful in a diff. - "updated_at": ActionIgnore, // Changes, but is implicit and not helpful in a diff. - "organization_id": ActionTrack, - "deleted": ActionIgnore, // Changes, but is implicit when a delete event is fired. - "name": ActionTrack, - "provisioner": ActionTrack, - "active_version_id": ActionTrack, - "description": ActionTrack, + "id": ActionTrack, + "created_at": ActionIgnore, // Never changes, but is implicit and not helpful in a diff. + "updated_at": ActionIgnore, // Changes, but is implicit and not helpful in a diff. + "organization_id": ActionTrack, + "deleted": ActionIgnore, // Changes, but is implicit when a delete event is fired. + "name": ActionTrack, + "provisioner": ActionTrack, + "active_version_id": ActionTrack, + "description": ActionTrack, + "max_ttl": ActionTrack, + "min_autostart_interval": ActionTrack, }, &database.TemplateVersion{}: { "id": ActionTrack, From 74f4dee47c2781e889e432e1b1f5f4324284b83a Mon Sep 17 00:00:00 2001 From: johnstcn Date: Thu, 2 Jun 2022 14:30:06 +0000 Subject: [PATCH 04/19] fixup! RED: add migrations and make gen --- coderd/database/queries.sql.go | 26 ++++++++++++++++---------- coderd/database/queries/templates.sql | 6 ++++-- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index fb77d1e0e657c..13d9cbc62eb7b 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -1773,21 +1773,25 @@ INSERT INTO "name", provisioner, active_version_id, - description + description, + max_ttl, + min_autostart_interval ) VALUES - ($1, $2, $3, $4, $5, $6, $7, $8) RETURNING id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, max_ttl, min_autostart_interval + ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) RETURNING id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, max_ttl, min_autostart_interval ` type InsertTemplateParams struct { - ID uuid.UUID `db:"id" json:"id"` - CreatedAt time.Time `db:"created_at" json:"created_at"` - UpdatedAt time.Time `db:"updated_at" json:"updated_at"` - OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` - Name string `db:"name" json:"name"` - Provisioner ProvisionerType `db:"provisioner" json:"provisioner"` - ActiveVersionID uuid.UUID `db:"active_version_id" json:"active_version_id"` - Description string `db:"description" json:"description"` + ID uuid.UUID `db:"id" json:"id"` + CreatedAt time.Time `db:"created_at" json:"created_at"` + UpdatedAt time.Time `db:"updated_at" json:"updated_at"` + OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` + Name string `db:"name" json:"name"` + Provisioner ProvisionerType `db:"provisioner" json:"provisioner"` + ActiveVersionID uuid.UUID `db:"active_version_id" json:"active_version_id"` + Description string `db:"description" json:"description"` + MaxTtl int64 `db:"max_ttl" json:"max_ttl"` + MinAutostartInterval int64 `db:"min_autostart_interval" json:"min_autostart_interval"` } func (q *sqlQuerier) InsertTemplate(ctx context.Context, arg InsertTemplateParams) (Template, error) { @@ -1800,6 +1804,8 @@ func (q *sqlQuerier) InsertTemplate(ctx context.Context, arg InsertTemplateParam arg.Provisioner, arg.ActiveVersionID, arg.Description, + arg.MaxTtl, + arg.MinAutostartInterval, ) var i Template err := row.Scan( diff --git a/coderd/database/queries/templates.sql b/coderd/database/queries/templates.sql index 133d3b4c47e5c..48bcd97722f09 100644 --- a/coderd/database/queries/templates.sql +++ b/coderd/database/queries/templates.sql @@ -47,10 +47,12 @@ INSERT INTO "name", provisioner, active_version_id, - description + description, + max_ttl, + min_autostart_interval ) VALUES - ($1, $2, $3, $4, $5, $6, $7, $8) RETURNING *; + ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) RETURNING *; -- name: UpdateTemplateActiveVersionByID :exec UPDATE From 7da27a0b6a373ea599820eb3ecaa285a293dc7b6 Mon Sep 17 00:00:00 2001 From: johnstcn Date: Thu, 2 Jun 2022 18:10:34 +0000 Subject: [PATCH 05/19] fixup! RED: add migrations and make gen --- coderd/database/databasefake/databasefake.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 88c17101f833c..225535f580ffa 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -1226,14 +1226,16 @@ func (q *fakeQuerier) InsertTemplate(_ context.Context, arg database.InsertTempl //nolint:gosimple template := database.Template{ - ID: arg.ID, - CreatedAt: arg.CreatedAt, - UpdatedAt: arg.UpdatedAt, - OrganizationID: arg.OrganizationID, - Name: arg.Name, - Provisioner: arg.Provisioner, - ActiveVersionID: arg.ActiveVersionID, - Description: arg.Description, + ID: arg.ID, + CreatedAt: arg.CreatedAt, + UpdatedAt: arg.UpdatedAt, + OrganizationID: arg.OrganizationID, + Name: arg.Name, + Provisioner: arg.Provisioner, + ActiveVersionID: arg.ActiveVersionID, + Description: arg.Description, + MaxTtl: arg.MaxTtl, + MinAutostartInterval: arg.MinAutostartInterval, } q.templates = append(q.templates, template) return template, nil From 5ad048b393275498bc932608a892e6ff727170d3 Mon Sep 17 00:00:00 2001 From: johnstcn Date: Thu, 2 Jun 2022 20:25:55 +0000 Subject: [PATCH 06/19] feat: schedule: add schedule.Min function to determine minimum interval --- coderd/autobuild/schedule/schedule.go | 30 ++++++++++++++++++++++ coderd/autobuild/schedule/schedule_test.go | 26 +++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/coderd/autobuild/schedule/schedule.go b/coderd/autobuild/schedule/schedule.go index 03981acac0489..81a5842898654 100644 --- a/coderd/autobuild/schedule/schedule.go +++ b/coderd/autobuild/schedule/schedule.go @@ -108,6 +108,36 @@ func (s Schedule) Next(t time.Time) time.Time { return s.sched.Next(t) } +var t0 = time.Date(1970, 1, 1, 1, 1, 1, 0, time.UTC) +var tMax = t0.Add(168 * time.Hour) + +// Min returns the minimum duration of the schedule. +// This is calculated as follows: +// - Let t(0) be a given point in time (1970-01-01T01:01:01Z00:00) +// - Let t(max) be 168 hours after t(0). +// - Let t(1) be the next scheduled time after t(0). +// - Let t(n) be the next scheduled time after t(n-1). +// - Then, the minimum duration of s d(min) +// = min( t(n) - t(n-1) ∀ n ∈ N, t(n) < t(max) ) +func (s Schedule) Min() time.Duration { + durMin := tMax.Sub(t0) + tPrev := s.Next(t0) + tCurr := s.Next(tPrev) + for { + dur := tCurr.Sub(tPrev) + if dur < durMin { + durMin = dur + } + tmp := tCurr + tCurr = s.Next(tmp) + tPrev = tmp + if tCurr.After(tMax) { + break + } + } + return durMin +} + // validateWeeklySpec ensures that the day-of-month and month options of // spec are both set to * func validateWeeklySpec(spec string) error { diff --git a/coderd/autobuild/schedule/schedule_test.go b/coderd/autobuild/schedule/schedule_test.go index 0c847a28e819c..c666b82f0850f 100644 --- a/coderd/autobuild/schedule/schedule_test.go +++ b/coderd/autobuild/schedule/schedule_test.go @@ -16,6 +16,7 @@ func Test_Weekly(t *testing.T) { spec string at time.Time expectedNext time.Time + expectedMin time.Duration expectedError string expectedCron string expectedTz string @@ -26,6 +27,7 @@ func Test_Weekly(t *testing.T) { spec: "CRON_TZ=US/Central 30 9 * * 1-5", at: time.Date(2022, 4, 1, 14, 29, 0, 0, time.UTC), expectedNext: time.Date(2022, 4, 1, 14, 30, 0, 0, time.UTC), + expectedMin: 24 * time.Hour, expectedError: "", expectedCron: "30 9 * * 1-5", expectedTz: "US/Central", @@ -36,11 +38,34 @@ func Test_Weekly(t *testing.T) { spec: "30 9 * * 1-5", at: time.Date(2022, 4, 1, 9, 29, 0, 0, time.UTC), expectedNext: time.Date(2022, 4, 1, 9, 30, 0, 0, time.UTC), + expectedMin: 24 * time.Hour, expectedError: "", expectedCron: "30 9 * * 1-5", expectedTz: "UTC", expectedString: "CRON_TZ=UTC 30 9 * * 1-5", }, + { + name: "convoluted with timezone", + spec: "CRON_TZ=US/Central */5 12-18 * * 1,3,6", + at: time.Date(2022, 4, 1, 14, 29, 0, 0, time.UTC), + expectedNext: time.Date(2022, 4, 2, 17, 0, 0, 0, time.UTC), // Apr 1 was a Friday in 2022 + expectedMin: 5 * time.Minute, + expectedError: "", + expectedCron: "*/5 12-18 * * 1,3,6", + expectedTz: "US/Central", + expectedString: "CRON_TZ=US/Central */5 12-18 * * 1,3,6", + }, + { + name: "another convoluted example", + spec: "CRON_TZ=US/Central 10,20,40-50 * * * *", + at: time.Date(2022, 4, 1, 14, 29, 0, 0, time.UTC), + expectedNext: time.Date(2022, 4, 1, 14, 40, 0, 0, time.UTC), + expectedMin: time.Minute, + expectedError: "", + expectedCron: "10,20,40-50 * * * *", + expectedTz: "US/Central", + expectedString: "CRON_TZ=US/Central 10,20,40-50 * * * *", + }, { name: "time.Local will bite you", spec: "CRON_TZ=Local 30 9 * * 1-5", @@ -104,6 +129,7 @@ func Test_Weekly(t *testing.T) { require.Equal(t, testCase.expectedCron, actual.Cron()) require.Equal(t, testCase.expectedTz, actual.Timezone()) require.Equal(t, testCase.expectedString, actual.String()) + require.Equal(t, testCase.expectedMin, actual.Min()) } else { require.EqualError(t, err, testCase.expectedError) require.Nil(t, actual) From 65b980ed4258d9335b27b34830ceabb441818eb1 Mon Sep 17 00:00:00 2001 From: johnstcn Date: Thu, 2 Jun 2022 20:29:16 +0000 Subject: [PATCH 07/19] ORANGE: coderd: fix a whole bunch of tests --- .../executor/lifecycle_executor_test.go | 108 +++++++----------- coderd/database/databasefake/databasefake.go | 8 ++ coderd/templates.go | 35 ++++-- coderd/workspaces.go | 70 ++++++++---- coderd/workspaces_test.go | 52 ++++++--- 5 files changed, 163 insertions(+), 110 deletions(-) diff --git a/coderd/autobuild/executor/lifecycle_executor_test.go b/coderd/autobuild/executor/lifecycle_executor_test.go index a97897e1a0bbc..034ba4a2ce126 100644 --- a/coderd/autobuild/executor/lifecycle_executor_test.go +++ b/coderd/autobuild/executor/lifecycle_executor_test.go @@ -2,7 +2,6 @@ package executor_test import ( "context" - "fmt" "os" "strings" "testing" @@ -26,8 +25,7 @@ func TestExecutorAutostartOK(t *testing.T) { t.Parallel() var ( - ctx = context.Background() - err error + sched = mustSchedule(t, "CRON_TZ=UTC 0 * * * *") tickCh = make(chan time.Time) statsCh = make(chan executor.Stats) client = coderdtest.New(t, &coderdtest.Options{ @@ -35,22 +33,17 @@ func TestExecutorAutostartOK(t *testing.T) { IncludeProvisionerD: true, AutobuildStats: statsCh, }) - // Given: we have a user with a workspace - workspace = mustProvisionWorkspace(t, client) + // Given: we have a user with a workspace that has autostart enabled + workspace = mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) { + cwr.AutostartSchedule = ptr.Ref(sched.String()) + }) ) // Given: workspace is stopped workspace = mustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) - // When: we enable workspace autostart - sched, err := schedule.Weekly("* * * * *") - require.NoError(t, err) - require.NoError(t, client.UpdateWorkspaceAutostart(ctx, workspace.ID, codersdk.UpdateWorkspaceAutostartRequest{ - Schedule: ptr.Ref(sched.String()), - })) - - // When: the autobuild executor ticks + // When: the autobuild executor ticks after the scheduled time go func() { - tickCh <- time.Now().UTC().Add(time.Minute) + tickCh <- sched.Next(workspace.LatestBuild.CreatedAt) close(tickCh) }() @@ -66,6 +59,7 @@ func TestExecutorAutostartTemplateUpdated(t *testing.T) { t.Parallel() var ( + sched = mustSchedule(t, "CRON_TZ=UTC 0 * * * *") ctx = context.Background() err error tickCh = make(chan time.Time) @@ -75,8 +69,10 @@ func TestExecutorAutostartTemplateUpdated(t *testing.T) { IncludeProvisionerD: true, AutobuildStats: statsCh, }) - // Given: we have a user with a workspace - workspace = mustProvisionWorkspace(t, client) + // Given: we have a user with a workspace that has autostart enabled + workspace = mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) { + cwr.AutostartSchedule = ptr.Ref(sched.String()) + }) ) // Given: workspace is stopped workspace = mustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) @@ -92,16 +88,9 @@ func TestExecutorAutostartTemplateUpdated(t *testing.T) { ID: newVersion.ID, })) - // When: we enable workspace autostart - sched, err := schedule.Weekly("* * * * *") - require.NoError(t, err) - require.NoError(t, client.UpdateWorkspaceAutostart(ctx, workspace.ID, codersdk.UpdateWorkspaceAutostartRequest{ - Schedule: ptr.Ref(sched.String()), - })) - - // When: the autobuild executor ticks + // When: the autobuild executor ticks after the scheduled time go func() { - tickCh <- time.Now().UTC().Add(time.Minute) + tickCh <- sched.Next(workspace.LatestBuild.CreatedAt) close(tickCh) }() @@ -119,8 +108,7 @@ func TestExecutorAutostartAlreadyRunning(t *testing.T) { t.Parallel() var ( - ctx = context.Background() - err error + sched = mustSchedule(t, "CRON_TZ=UTC 0 * * * *") tickCh = make(chan time.Time) statsCh = make(chan executor.Stats) client = coderdtest.New(t, &coderdtest.Options{ @@ -128,23 +116,18 @@ func TestExecutorAutostartAlreadyRunning(t *testing.T) { IncludeProvisionerD: true, AutobuildStats: statsCh, }) - // Given: we have a user with a workspace - workspace = mustProvisionWorkspace(t, client) + // Given: we have a user with a workspace that has autostart enabled + workspace = mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) { + cwr.AutostartSchedule = ptr.Ref(sched.String()) + }) ) // Given: we ensure the workspace is running require.Equal(t, codersdk.WorkspaceTransitionStart, workspace.LatestBuild.Transition) - // When: we enable workspace autostart - sched, err := schedule.Weekly("* * * * *") - require.NoError(t, err) - require.NoError(t, client.UpdateWorkspaceAutostart(ctx, workspace.ID, codersdk.UpdateWorkspaceAutostartRequest{ - Schedule: ptr.Ref(sched.String()), - })) - // When: the autobuild executor ticks go func() { - tickCh <- time.Now().UTC().Add(time.Minute) + tickCh <- sched.Next(workspace.LatestBuild.CreatedAt) close(tickCh) }() @@ -165,7 +148,7 @@ func TestExecutorAutostartNotEnabled(t *testing.T) { IncludeProvisionerD: true, AutobuildStats: statsCh, }) - // Given: we have a user with a workspace + // Given: we have a user with a workspace that does not have autostart enabled workspace = mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) { cwr.AutostartSchedule = nil }) @@ -177,9 +160,9 @@ func TestExecutorAutostartNotEnabled(t *testing.T) { // Given: workspace is stopped workspace = mustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) - // When: the autobuild executor ticks + // When: the autobuild executor ticks way into the future go func() { - tickCh <- time.Now().UTC().Add(time.Minute) + tickCh <- workspace.LatestBuild.CreatedAt.Add(24 * time.Hour) close(tickCh) }() @@ -343,8 +326,7 @@ func TestExecutorWorkspaceDeleted(t *testing.T) { t.Parallel() var ( - ctx = context.Background() - err error + sched = mustSchedule(t, "CRON_TZ=UTC 0 * * * *") tickCh = make(chan time.Time) statsCh = make(chan executor.Stats) client = coderdtest.New(t, &coderdtest.Options{ @@ -352,23 +334,18 @@ func TestExecutorWorkspaceDeleted(t *testing.T) { IncludeProvisionerD: true, AutobuildStats: statsCh, }) - // Given: we have a user with a workspace - workspace = mustProvisionWorkspace(t, client) + // Given: we have a user with a workspace that has autostart enabled + workspace = mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) { + cwr.AutostartSchedule = ptr.Ref(sched.String()) + }) ) - // When: we enable workspace autostart - sched, err := schedule.Weekly("* * * * *") - require.NoError(t, err) - require.NoError(t, client.UpdateWorkspaceAutostart(ctx, workspace.ID, codersdk.UpdateWorkspaceAutostartRequest{ - Schedule: ptr.Ref(sched.String()), - })) - // Given: workspace is deleted workspace = mustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionDelete) // When: the autobuild executor ticks go func() { - tickCh <- time.Now().UTC().Add(time.Minute) + tickCh <- sched.Next(workspace.LatestBuild.CreatedAt) close(tickCh) }() @@ -382,8 +359,7 @@ func TestExecutorWorkspaceAutostartTooEarly(t *testing.T) { t.Parallel() var ( - ctx = context.Background() - err error + sched = mustSchedule(t, "CRON_TZ=UTC 0 * * * *") tickCh = make(chan time.Time) statsCh = make(chan executor.Stats) client = coderdtest.New(t, &coderdtest.Options{ @@ -391,24 +367,17 @@ func TestExecutorWorkspaceAutostartTooEarly(t *testing.T) { IncludeProvisionerD: true, AutobuildStats: statsCh, }) - futureTime = time.Now().Add(time.Hour) - futureTimeCron = fmt.Sprintf("%d %d * * *", futureTime.Minute(), futureTime.Hour()) + // futureTime = time.Now().Add(time.Hour) + // futureTimeCron = fmt.Sprintf("%d %d * * *", futureTime.Minute(), futureTime.Hour()) // Given: we have a user with a workspace configured to autostart some time in the future workspace = mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) { - cwr.AutostartSchedule = &futureTimeCron + cwr.AutostartSchedule = ptr.Ref(sched.String()) }) ) - // When: we enable workspace autostart with some time in the future - sched, err := schedule.Weekly(futureTimeCron) - require.NoError(t, err) - require.NoError(t, client.UpdateWorkspaceAutostart(ctx, workspace.ID, codersdk.UpdateWorkspaceAutostartRequest{ - Schedule: ptr.Ref(sched.String()), - })) - - // When: the autobuild executor ticks + // When: the autobuild executor ticks before the next scheduled time go func() { - tickCh <- time.Now().UTC() + tickCh <- sched.Next(workspace.LatestBuild.CreatedAt).Add(-time.Minute) close(tickCh) }() @@ -573,6 +542,13 @@ func mustWorkspace(t *testing.T, client *codersdk.Client, workspaceID uuid.UUID) return ws } +func mustSchedule(t *testing.T, s string) *schedule.Schedule { + t.Helper() + sched, err := schedule.Weekly(s) + require.NoError(t, err) + return sched +} + func TestMain(m *testing.M) { goleak.VerifyTestMain(m) } diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 225535f580ffa..77885eaf15d63 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -1224,6 +1224,14 @@ func (q *fakeQuerier) InsertTemplate(_ context.Context, arg database.InsertTempl q.mutex.Lock() defer q.mutex.Unlock() + // default values + if arg.MaxTtl == 0 { + arg.MaxTtl = int64(168 * time.Hour) + } + if arg.MinAutostartInterval == 0 { + arg.MinAutostartInterval = int64(time.Hour) + } + //nolint:gosimple template := database.Template{ ID: arg.ID, diff --git a/coderd/templates.go b/coderd/templates.go index 185836e293335..69498626a7190 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "net/http" + "time" "github.com/go-chi/chi/v5" "github.com/google/uuid" @@ -14,9 +15,15 @@ import ( "github.com/coder/coder/coderd/httpapi" "github.com/coder/coder/coderd/httpmw" "github.com/coder/coder/coderd/rbac" + "github.com/coder/coder/coderd/util/ptr" "github.com/coder/coder/codersdk" ) +var ( + maxTTLDefault = 24 * 7 * time.Hour + minAutostartIntervalDefault = time.Hour +) + // Returns a single template. func (api *API) template(rw http.ResponseWriter, r *http.Request) { template := httpmw.TemplateParam(r) @@ -135,18 +142,30 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque return } + maxTTL := maxTTLDefault + if !ptr.NilOrZero(createTemplate.MaxTTLMillis) { + maxTTL = time.Duration(*createTemplate.MaxTTLMillis) * time.Millisecond + } + + minAutostartInterval := minAutostartIntervalDefault + if !ptr.NilOrZero(createTemplate.MinAutostartIntervalMillis) { + minAutostartInterval = time.Duration(*createTemplate.MinAutostartIntervalMillis) * time.Millisecond + } + var template codersdk.Template err = api.Database.InTx(func(db database.Store) error { now := database.Now() dbTemplate, err := db.InsertTemplate(r.Context(), database.InsertTemplateParams{ - ID: uuid.New(), - CreatedAt: now, - UpdatedAt: now, - OrganizationID: organization.ID, - Name: createTemplate.Name, - Provisioner: importJob.Provisioner, - ActiveVersionID: templateVersion.ID, - Description: createTemplate.Description, + ID: uuid.New(), + CreatedAt: now, + UpdatedAt: now, + OrganizationID: organization.ID, + Name: createTemplate.Name, + Provisioner: importJob.Provisioner, + ActiveVersionID: templateVersion.ID, + Description: createTemplate.Description, + MaxTtl: int64(maxTTL), + MinAutostartInterval: int64(minAutostartInterval), }) if err != nil { return xerrors.Errorf("insert template: %s", err) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 43c39140a7e53..a95794908f352 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -333,29 +333,20 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req return } - var dbAutostartSchedule sql.NullString - if createWorkspace.AutostartSchedule != nil { - _, err := schedule.Weekly(*createWorkspace.AutostartSchedule) - if err != nil { - httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ - Message: fmt.Sprintf("parse autostart schedule: %s", err.Error()), - }) - return - } - dbAutostartSchedule.Valid = true - dbAutostartSchedule.String = *createWorkspace.AutostartSchedule + dbAutostartSchedule, err := validWorkspaceSchedule(createWorkspace.AutostartSchedule, time.Duration(template.MinAutostartInterval)) + if err != nil { + httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ + Message: "Invalid Autostart Schedule", + Errors: []httpapi.Error{{Field: "schedule", Detail: err.Error()}}, + }) + return } dbTTL, err := validWorkspaceTTLMillis(createWorkspace.TTLMillis) if err != nil { httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ - Message: "validate workspace ttl", - Errors: []httpapi.Error{ - { - Field: "ttl", - Detail: err.Error(), - }, - }, + Message: "Invalid Workspace TTL", + Errors: []httpapi.Error{{Field: "ttl_ms", Detail: err.Error()}}, }) return } @@ -528,10 +519,19 @@ func (api *API) putWorkspaceAutostart(rw http.ResponseWriter, r *http.Request) { return } - dbSched, err := validWorkspaceSchedule(req.Schedule) + template, err := api.Database.GetTemplateByID(r.Context(), workspace.TemplateID) if err != nil { + api.Logger.Error(r.Context(), "fetch workspace template", slog.F("workspace_id", workspace.ID), slog.F("template_id", workspace.TemplateID), slog.Error(err)) httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ - Message: fmt.Sprintf("invalid autostart schedule: %s", err), + Message: "Error fetching workspace template", + }) + } + + dbSched, err := validWorkspaceSchedule(req.Schedule, time.Duration(template.MinAutostartInterval)) + if err != nil { + httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ + Message: "Invalid autostart schedule", + Errors: []httpapi.Error{{Field: "schedule", Detail: err.Error()}}, }) return } @@ -566,7 +566,7 @@ func (api *API) putWorkspaceTTL(rw http.ResponseWriter, r *http.Request) { Message: "validate workspace ttl", Errors: []httpapi.Error{ { - Field: "ttl", + Field: "ttl_ms", Detail: err.Error(), }, }, @@ -574,6 +574,26 @@ 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 + } + + if dbTTL.Valid && dbTTL.Int64 > template.MaxTtl { + httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ + Message: "Constrained by template", + Errors: []httpapi.Error{ + { + Field: "ttl_ms", + Detail: fmt.Sprintf("requested value is %s but template max is %s", time.Duration(dbTTL.Int64), time.Duration(template.MaxTtl)), + }, + }, + }) + } + err = api.Database.UpdateWorkspaceTTL(r.Context(), database.UpdateWorkspaceTTLParams{ ID: workspace.ID, Ttl: dbTTL, @@ -906,16 +926,20 @@ func validWorkspaceDeadline(old, new time.Time) error { return nil } -func validWorkspaceSchedule(s *string) (sql.NullString, error) { +func validWorkspaceSchedule(s *string, min time.Duration) (sql.NullString, error) { if ptr.NilOrEmpty(s) { return sql.NullString{}, nil } - _, err := schedule.Weekly(*s) + sched, err := schedule.Weekly(*s) if err != nil { return sql.NullString{}, err } + if schedMin := sched.Min(); schedMin < min { + return sql.NullString{}, xerrors.Errorf("Minimum autostart interval %s below template minimum %s", schedMin, min) + } + return sql.NullString{ Valid: true, String: *s, diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 0c923e415b892..e332cbf21b597 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -176,16 +176,18 @@ func TestPostWorkspacesByOrganization(t *testing.T) { template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) coderdtest.AwaitTemplateVersionJob(t, client, version.ID) req := codersdk.CreateWorkspaceRequest{ - TemplateID: template.ID, - Name: "testing", - AutostartSchedule: ptr.Ref("CRON_TZ=US/Central * * * * *"), - TTLMillis: ptr.Ref((59 * time.Second).Milliseconds()), + TemplateID: template.ID, + Name: "testing", + TTLMillis: ptr.Ref((59 * time.Second).Milliseconds()), } _, err := client.CreateWorkspace(context.Background(), template.OrganizationID, req) require.Error(t, err) var apiErr *codersdk.Error require.ErrorAs(t, err, &apiErr) require.Equal(t, http.StatusBadRequest, apiErr.StatusCode()) + require.Len(t, apiErr.Errors, 1) + require.Equal(t, apiErr.Errors[0].Field, "ttl_ms") + require.Equal(t, apiErr.Errors[0].Detail, "ttl must be at least one minute") }) t.Run("AboveMax", func(t *testing.T) { @@ -196,18 +198,42 @@ func TestPostWorkspacesByOrganization(t *testing.T) { template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) coderdtest.AwaitTemplateVersionJob(t, client, version.ID) req := codersdk.CreateWorkspaceRequest{ - TemplateID: template.ID, - Name: "testing", - AutostartSchedule: ptr.Ref("CRON_TZ=US/Central * * * * *"), - TTLMillis: ptr.Ref((24*7*time.Hour + time.Minute).Milliseconds()), + TemplateID: template.ID, + Name: "testing", + TTLMillis: ptr.Ref((24*7*time.Hour + time.Minute).Milliseconds()), } _, err := client.CreateWorkspace(context.Background(), template.OrganizationID, req) require.Error(t, err) var apiErr *codersdk.Error require.ErrorAs(t, err, &apiErr) require.Equal(t, http.StatusBadRequest, apiErr.StatusCode()) + require.Len(t, apiErr.Errors, 1) + require.Equal(t, apiErr.Errors[0].Field, "ttl_ms") + require.Equal(t, apiErr.Errors[0].Detail, "ttl must be less than 7 days") }) }) + + t.Run("InvalidAutostart", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) + user := coderdtest.CreateFirstUser(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + req := codersdk.CreateWorkspaceRequest{ + TemplateID: template.ID, + Name: "testing", + AutostartSchedule: ptr.Ref("CRON_TZ=US/Central * * * * *"), + } + _, err := client.CreateWorkspace(context.Background(), template.OrganizationID, req) + require.Error(t, err) + var apiErr *codersdk.Error + require.ErrorAs(t, err, &apiErr) + require.Equal(t, http.StatusBadRequest, apiErr.StatusCode()) + require.Len(t, apiErr.Errors, 1) + require.Equal(t, apiErr.Errors[0].Field, "schedule") + require.Equal(t, apiErr.Errors[0].Detail, "Minimum autostart interval 1m0s below template minimum 1h0m0s") + }) } func TestWorkspacesByOrganization(t *testing.T) { @@ -500,17 +526,17 @@ func TestWorkspaceUpdateAutostart(t *testing.T) { { name: "invalid location", schedule: ptr.Ref("CRON_TZ=Imaginary/Place 30 9 * * 1-5"), - expectedError: "status code 500: invalid autostart schedule: parse schedule: provided bad location Imaginary/Place: unknown time zone Imaginary/Place", + expectedError: "parse schedule: provided bad location Imaginary/Place: unknown time zone Imaginary/Place", }, { name: "invalid schedule", schedule: ptr.Ref("asdf asdf asdf "), - expectedError: `status code 500: invalid autostart schedule: validate weekly schedule: expected schedule to consist of 5 fields with an optional CRON_TZ= prefix`, + expectedError: `validate weekly schedule: expected schedule to consist of 5 fields with an optional CRON_TZ= prefix`, }, { name: "only 3 values", schedule: ptr.Ref("CRON_TZ=Europe/Dublin 30 9 *"), - expectedError: `status code 500: invalid autostart schedule: validate weekly schedule: expected schedule to consist of 5 fields with an optional CRON_TZ= prefix`, + expectedError: `validate weekly schedule: expected schedule to consist of 5 fields with an optional CRON_TZ= prefix`, }, } @@ -620,8 +646,8 @@ func TestWorkspaceUpdateTTL(t *testing.T) { }, { name: "above template maximum ttl", - ttlMillis: ptr.Ref((12*time.Hour + time.Minute).Milliseconds()), - expectedError: "TODO what is the error", + ttlMillis: ptr.Ref((12 * time.Hour).Milliseconds()), + expectedError: "requested value is 12h0m0s but template max is 8h0m0s", modifyTemplate: func(ctr *codersdk.CreateTemplateRequest) { ctr.MaxTTLMillis = ptr.Ref((8 * time.Hour).Milliseconds()) }, }, } From aa298a6f09db61038e6a3f14e648682d43e7db74 Mon Sep 17 00:00:00 2001 From: johnstcn Date: Thu, 2 Jun 2022 20:44:43 +0000 Subject: [PATCH 08/19] RED: fast-fail some slow-failing tests --- cli/create_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cli/create_test.go b/cli/create_test.go index 9172b08c1f402..c7b08e86b4f33 100644 --- a/cli/create_test.go +++ b/cli/create_test.go @@ -23,6 +23,7 @@ import ( func TestCreate(t *testing.T) { t.Parallel() + t.Fatal("CIAN FIX THIS") t.Run("Create", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) @@ -65,6 +66,7 @@ func TestCreate(t *testing.T) { t.Run("AboveTemplateMaxTTL", func(t *testing.T) { t.Parallel() + t.Fatal("CIAN FIX THIS") client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) user := coderdtest.CreateFirstUser(t, client) version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) @@ -93,6 +95,7 @@ func TestCreate(t *testing.T) { t.Run("BelowTemplateMinAutostartInterval", func(t *testing.T) { t.Parallel() + t.Fatal("CIAN FIX THIS") client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) user := coderdtest.CreateFirstUser(t, client) version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) @@ -149,6 +152,7 @@ func TestCreate(t *testing.T) { t.Run("CreateErrInvalidTTL", func(t *testing.T) { t.Parallel() + t.Fatal("CIAN FIX THIS") client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) user := coderdtest.CreateFirstUser(t, client) version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) From a5ec9bf90e4219fbfdd2f30e9cafb7a1c45e4d00 Mon Sep 17 00:00:00 2001 From: johnstcn Date: Fri, 3 Jun 2022 12:14:51 +0000 Subject: [PATCH 09/19] GREEN: fix even more tests --- cli/autostart_test.go | 8 +------ cli/create.go | 33 ++++++++++++++--------------- cli/create_test.go | 44 ++++++++++++--------------------------- cli/ttl_test.go | 4 ++-- coderd/workspaces.go | 30 ++++++++++---------------- coderd/workspaces_test.go | 2 +- 6 files changed, 44 insertions(+), 77 deletions(-) diff --git a/cli/autostart_test.go b/cli/autostart_test.go index e369200c83058..f2c894af869bd 100644 --- a/cli/autostart_test.go +++ b/cli/autostart_test.go @@ -164,7 +164,6 @@ func TestAutostart(t *testing.T) { t.Parallel() var ( - ctx = context.Background() client = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) user = coderdtest.CreateFirstUser(t, client) version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) @@ -180,11 +179,6 @@ func TestAutostart(t *testing.T) { clitest.SetupConfig(t, client, root) err := cmd.Execute() - require.NoError(t, err, "unexpected error") - - // Ensure nothing happened - updated, err := client.Workspace(ctx, workspace.ID) - require.NoError(t, err, "fetch updated workspace") - require.Equal(t, *workspace.AutostartSchedule, *updated.AutostartSchedule, "expected previous autostart schedule") + require.ErrorContains(t, err, "schedule: Minimum autostart interval 1m0s below template minimum 1h0m0s") }) } diff --git a/cli/create.go b/cli/create.go index aab94ed3a9f67..92e9bfa9bda21 100644 --- a/cli/create.go +++ b/cli/create.go @@ -10,7 +10,6 @@ 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" ) @@ -61,20 +60,6 @@ func create() *cobra.Command { } } - tz, err := time.LoadLocation(tzName) - if err != nil { - return xerrors.Errorf("Invalid workspace autostart timezone: %w", err) - } - schedSpec := fmt.Sprintf("CRON_TZ=%s %s %s * * %s", tz.String(), autostartMinute, autostartHour, autostartDow) - _, err = schedule.Weekly(schedSpec) - if err != nil { - return xerrors.Errorf("invalid workspace autostart schedule: %w", err) - } - - if ttl == 0 { - return xerrors.Errorf("TTL must be at least 1 minute") - } - _, err = client.WorkspaceByOwnerAndName(cmd.Context(), organization.ID, codersdk.Me, workspaceName) if err == nil { return xerrors.Errorf("A workspace already exists named %q!", workspaceName) @@ -129,6 +114,11 @@ func create() *cobra.Command { } } + schedSpec := buildSchedule(autostartMinute, autostartHour, autostartDow, tzName) + if ttl < time.Minute { + return xerrors.Errorf("TTL must be at least 1 minute") + } + templateVersion, err := client.TemplateVersion(cmd.Context(), template.ActiveVersionID) if err != nil { return err @@ -226,7 +216,7 @@ func create() *cobra.Command { workspace, err := client.CreateWorkspace(cmd.Context(), organization.ID, codersdk.CreateWorkspaceRequest{ TemplateID: template.ID, Name: workspaceName, - AutostartSchedule: &schedSpec, + AutostartSchedule: schedSpec, TTLMillis: ptr.Ref(ttl.Milliseconds()), ParameterValues: parameters, }) @@ -262,7 +252,16 @@ func create() *cobra.Command { cliflag.StringVarP(cmd.Flags(), &autostartMinute, "autostart-minute", "", "CODER_WORKSPACE_AUTOSTART_MINUTE", "0", "Specify the minute(s) at which the workspace should autostart (e.g. 0).") cliflag.StringVarP(cmd.Flags(), &autostartHour, "autostart-hour", "", "CODER_WORKSPACE_AUTOSTART_HOUR", "9", "Specify the hour(s) at which the workspace should autostart (e.g. 9).") cliflag.StringVarP(cmd.Flags(), &autostartDow, "autostart-day-of-week", "", "CODER_WORKSPACE_AUTOSTART_DOW", "MON-FRI", "Specify the days(s) on which the workspace should autostart (e.g. MON,TUE,WED,THU,FRI)") - cliflag.StringVarP(cmd.Flags(), &tzName, "tz", "", "TZ", "", "Specify your timezone location for workspace autostart (e.g. US/Central).") + cliflag.StringVarP(cmd.Flags(), &tzName, "tz", "", "TZ", "UTC", "Specify your timezone location for workspace autostart (e.g. US/Central).") cliflag.DurationVarP(cmd.Flags(), &ttl, "ttl", "", "CODER_WORKSPACE_TTL", 8*time.Hour, "Specify a time-to-live (TTL) for the workspace (e.g. 8h).") return cmd } + +func buildSchedule(minute, hour, dow, tzName string) *string { + if minute == "" || hour == "" || dow == "" { + return nil + } + + schedSpec := fmt.Sprintf("CRON_TZ=%s %s %s * * %s", tzName, minute, hour, dow) + return &schedSpec +} diff --git a/cli/create_test.go b/cli/create_test.go index c7b08e86b4f33..3c389008ceca1 100644 --- a/cli/create_test.go +++ b/cli/create_test.go @@ -23,7 +23,6 @@ import ( func TestCreate(t *testing.T) { t.Parallel() - t.Fatal("CIAN FIX THIS") t.Run("Create", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) @@ -66,7 +65,6 @@ func TestCreate(t *testing.T) { t.Run("AboveTemplateMaxTTL", func(t *testing.T) { t.Parallel() - t.Fatal("CIAN FIX THIS") client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) user := coderdtest.CreateFirstUser(t, client) version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) @@ -78,24 +76,20 @@ func TestCreate(t *testing.T) { "create", "my-workspace", "--template", template.Name, - "--ttl", "24h", + "--ttl", "12h1m", + "-y", // don't bother with waiting } cmd, root := clitest.New(t, args...) clitest.SetupConfig(t, client, root) - errCh := make(chan error) pty := ptytest.New(t) cmd.SetIn(pty.Input()) cmd.SetOut(pty.Output()) - go func() { - defer close(errCh) - errCh <- cmd.Execute() - }() - require.EqualError(t, <-errCh, "TODO what is the error") + err := cmd.Execute() + assert.ErrorContains(t, err, "ttl_ms: ttl must be below template maximum 12h0m0s") }) t.Run("BelowTemplateMinAutostartInterval", func(t *testing.T) { t.Parallel() - t.Fatal("CIAN FIX THIS") client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) user := coderdtest.CreateFirstUser(t, client) version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) @@ -109,18 +103,15 @@ func TestCreate(t *testing.T) { "--template", template.Name, "--autostart-minute", "*", // Every minute "--autostart-hour", "*", // Every hour + "-y", // don't bother with waiting } cmd, root := clitest.New(t, args...) clitest.SetupConfig(t, client, root) - errCh := make(chan error) pty := ptytest.New(t) cmd.SetIn(pty.Input()) cmd.SetOut(pty.Output()) - go func() { - defer close(errCh) - errCh <- cmd.Execute() - }() - require.EqualError(t, <-errCh, "TODO what is the error") + err := cmd.Execute() + assert.ErrorContains(t, err, "Minimum autostart interval 1m0s below template minimum 1h0m0s") }) t.Run("CreateErrInvalidTz", func(t *testing.T) { @@ -135,24 +126,19 @@ func TestCreate(t *testing.T) { "my-workspace", "--template", template.Name, "--tz", "invalid", + "-y", } cmd, root := clitest.New(t, args...) clitest.SetupConfig(t, client, root) - doneChan := make(chan struct{}) pty := ptytest.New(t) cmd.SetIn(pty.Input()) cmd.SetOut(pty.Output()) - go func() { - defer close(doneChan) - err := cmd.Execute() - assert.EqualError(t, err, "Invalid workspace autostart timezone: unknown time zone invalid") - }() - <-doneChan + err := cmd.Execute() + assert.ErrorContains(t, err, "schedule: parse schedule: provided bad location invalid: unknown time zone invalid") }) t.Run("CreateErrInvalidTTL", func(t *testing.T) { t.Parallel() - t.Fatal("CIAN FIX THIS") client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) user := coderdtest.CreateFirstUser(t, client) version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) @@ -163,19 +149,15 @@ func TestCreate(t *testing.T) { "my-workspace", "--template", template.Name, "--ttl", "0s", + "-y", } cmd, root := clitest.New(t, args...) clitest.SetupConfig(t, client, root) - doneChan := make(chan struct{}) pty := ptytest.New(t) cmd.SetIn(pty.Input()) cmd.SetOut(pty.Output()) - go func() { - defer close(doneChan) - err := cmd.Execute() - assert.EqualError(t, err, "TTL must be at least 1 minute") - }() - <-doneChan + err := cmd.Execute() + assert.EqualError(t, err, "TTL must be at least 1 minute") }) t.Run("CreateFromListWithSkip", func(t *testing.T) { diff --git a/cli/ttl_test.go b/cli/ttl_test.go index fab444200d541..181a6a7b33f2f 100644 --- a/cli/ttl_test.go +++ b/cli/ttl_test.go @@ -193,12 +193,12 @@ func TestTTL(t *testing.T) { cmd.SetOut(stdoutBuf) err := cmd.Execute() - require.EqualError(t, err, "TODO what is the error") + require.ErrorContains(t, err, "ttl_ms: ttl must be below template maximum 8h0m0s") // Ensure ttl not updated updated, err := client.Workspace(ctx, workspace.ID) require.NoError(t, err, "fetch updated workspace") require.NotNil(t, updated.TTLMillis) - require.Equal(t, (8 * time.Hour).Milliseconds, *updated.TTLMillis) + require.Equal(t, (8 * time.Hour).Milliseconds(), *updated.TTLMillis) }) } diff --git a/coderd/workspaces.go b/coderd/workspaces.go index a95794908f352..925bb771a0290 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -342,7 +342,7 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req return } - dbTTL, err := validWorkspaceTTLMillis(createWorkspace.TTLMillis) + dbTTL, err := validWorkspaceTTLMillis(createWorkspace.TTLMillis, time.Duration(template.MaxTtl)) if err != nil { httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ Message: "Invalid Workspace TTL", @@ -560,20 +560,6 @@ func (api *API) putWorkspaceTTL(rw http.ResponseWriter, r *http.Request) { return } - dbTTL, err := validWorkspaceTTLMillis(req.TTLMillis) - if err != nil { - httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ - Message: "validate workspace ttl", - Errors: []httpapi.Error{ - { - Field: "ttl_ms", - Detail: err.Error(), - }, - }, - }) - return - } - template, err := api.Database.GetTemplateByID(r.Context(), workspace.TemplateID) if err != nil { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ @@ -582,16 +568,18 @@ func (api *API) putWorkspaceTTL(rw http.ResponseWriter, r *http.Request) { return } - if dbTTL.Valid && dbTTL.Int64 > template.MaxTtl { + dbTTL, err := validWorkspaceTTLMillis(req.TTLMillis, time.Duration(template.MaxTtl)) + if err != nil { httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{ - Message: "Constrained by template", + Message: "Invalid workspace TTL", Errors: []httpapi.Error{ { Field: "ttl_ms", - Detail: fmt.Sprintf("requested value is %s but template max is %s", time.Duration(dbTTL.Int64), time.Duration(template.MaxTtl)), + Detail: err.Error(), }, }, }) + return } err = api.Database.UpdateWorkspaceTTL(r.Context(), database.UpdateWorkspaceTTLParams{ @@ -883,7 +871,7 @@ func convertWorkspaceTTLMillis(i sql.NullInt64) *int64 { return &millis } -func validWorkspaceTTLMillis(millis *int64) (sql.NullInt64, error) { +func validWorkspaceTTLMillis(millis *int64, max time.Duration) (sql.NullInt64, error) { if ptr.NilOrZero(millis) { return sql.NullInt64{}, nil } @@ -898,6 +886,10 @@ func validWorkspaceTTLMillis(millis *int64) (sql.NullInt64, error) { return sql.NullInt64{}, xerrors.New("ttl must be less than 7 days") } + if truncated > max { + return sql.NullInt64{}, xerrors.Errorf("ttl must be below template maximum %s", max.String()) + } + return sql.NullInt64{ Valid: true, Int64: int64(truncated), diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index e332cbf21b597..d09ade7d1b0b3 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -647,7 +647,7 @@ func TestWorkspaceUpdateTTL(t *testing.T) { { name: "above template maximum ttl", ttlMillis: ptr.Ref((12 * time.Hour).Milliseconds()), - expectedError: "requested value is 12h0m0s but template max is 8h0m0s", + expectedError: "ttl_ms: ttl must be below template maximum 8h0m0s", modifyTemplate: func(ctr *codersdk.CreateTemplateRequest) { ctr.MaxTTLMillis = ptr.Ref((8 * time.Hour).Milliseconds()) }, }, } From a6255d69fc93674cd223b1b74acbe03a55b7f67c Mon Sep 17 00:00:00 2001 From: johnstcn Date: Fri, 3 Jun 2022 13:01:52 +0000 Subject: [PATCH 10/19] GREEN: fix final unit test --- coderd/autobuild/executor/lifecycle_executor_test.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/coderd/autobuild/executor/lifecycle_executor_test.go b/coderd/autobuild/executor/lifecycle_executor_test.go index 034ba4a2ce126..86ddd23aa548b 100644 --- a/coderd/autobuild/executor/lifecycle_executor_test.go +++ b/coderd/autobuild/executor/lifecycle_executor_test.go @@ -456,6 +456,7 @@ func TestExecutorAutostartMultipleOK(t *testing.T) { t.Parallel() var ( + sched = mustSchedule(t, "CRON_TZ=UTC 0 * * * *") tickCh = make(chan time.Time) tickCh2 = make(chan time.Time) statsCh1 = make(chan executor.Stats) @@ -471,15 +472,17 @@ func TestExecutorAutostartMultipleOK(t *testing.T) { AutobuildStats: statsCh2, }) // Given: we have a user with a workspace that has autostart enabled (default) - workspace = mustProvisionWorkspace(t, client) + workspace = mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) { + cwr.AutostartSchedule = ptr.Ref(sched.String()) + }) ) // Given: workspace is stopped workspace = mustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) - // When: the autobuild executor ticks + // When: the autobuild executor ticks past the scheduled time go func() { - tickCh <- time.Now().UTC().Add(time.Minute) - tickCh2 <- time.Now().UTC().Add(time.Minute) + tickCh <- sched.Next(workspace.LatestBuild.CreatedAt) + tickCh2 <- sched.Next(workspace.LatestBuild.CreatedAt) close(tickCh) close(tickCh2) }() From 245795ec4401ca17d3945bb5e42170bd3eb7755e Mon Sep 17 00:00:00 2001 From: johnstcn Date: Fri, 3 Jun 2022 14:12:36 +0000 Subject: [PATCH 11/19] cli: templatecreate: add CLI flags --max-ttl --min-autostart-interval --- cli/templatecreate.go | 25 +++++++++++++++++-------- cli/templatecreate_test.go | 11 ++++++++++- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/cli/templatecreate.go b/cli/templatecreate.go index 06205aa008eb2..936c574b57396 100644 --- a/cli/templatecreate.go +++ b/cli/templatecreate.go @@ -14,6 +14,7 @@ import ( "github.com/coder/coder/cli/cliui" "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/util/ptr" "github.com/coder/coder/codersdk" "github.com/coder/coder/provisionerd" "github.com/coder/coder/provisionersdk" @@ -21,9 +22,11 @@ import ( func templateCreate() *cobra.Command { var ( - directory string - provisioner string - parameterFile string + directory string + provisioner string + parameterFile string + maxTTL time.Duration + minAutostartInterval time.Duration ) cmd := &cobra.Command{ Use: "create [name]", @@ -92,11 +95,15 @@ func templateCreate() *cobra.Command { return err } - _, err = client.CreateTemplate(cmd.Context(), organization.ID, codersdk.CreateTemplateRequest{ - Name: templateName, - VersionID: job.ID, - ParameterValues: parameters, - }) + createReq := codersdk.CreateTemplateRequest{ + Name: templateName, + VersionID: job.ID, + ParameterValues: parameters, + MaxTTLMillis: ptr.Ref(maxTTL.Milliseconds()), + MinAutostartIntervalMillis: ptr.Ref(minAutostartInterval.Milliseconds()), + } + + _, err = client.CreateTemplate(cmd.Context(), organization.ID, createReq) if err != nil { return err } @@ -115,6 +122,8 @@ func templateCreate() *cobra.Command { cmd.Flags().StringVarP(&directory, "directory", "d", currentDirectory, "Specify the directory to create from") cmd.Flags().StringVarP(&provisioner, "test.provisioner", "", "terraform", "Customize the provisioner backend") cmd.Flags().StringVarP(¶meterFile, "parameter-file", "", "", "Specify a file path with parameter values.") + cmd.Flags().DurationVarP(&maxTTL, "max-ttl", "", 168*time.Hour, "Specify a maximum TTL for worksapces created from this template.") + cmd.Flags().DurationVarP(&minAutostartInterval, "min-autostart-interval", "", time.Hour, "Specify a minimum autostart interval for workspaces created from this template.") // This is for testing! err := cmd.Flags().MarkHidden("test.provisioner") if err != nil { diff --git a/cli/templatecreate_test.go b/cli/templatecreate_test.go index 2dead6ee24b69..e3d619ccdf629 100644 --- a/cli/templatecreate_test.go +++ b/cli/templatecreate_test.go @@ -24,7 +24,16 @@ func TestTemplateCreate(t *testing.T) { Parse: echo.ParseComplete, Provision: echo.ProvisionComplete, }) - cmd, root := clitest.New(t, "templates", "create", "my-template", "--directory", source, "--test.provisioner", string(database.ProvisionerTypeEcho)) + args := []string{ + "templates", + "create", + "my-template", + "--directory", source, + "--test.provisioner", string(database.ProvisionerTypeEcho), + "--max-ttl", "24h", + "--min-autostart-interval", "2h", + } + cmd, root := clitest.New(t, args...) clitest.SetupConfig(t, client, root) pty := ptytest.New(t) cmd.SetIn(pty.Input()) From 709db5b22f36583c04ea2fdfab8fa3c62d323f1a Mon Sep 17 00:00:00 2001 From: johnstcn Date: Fri, 3 Jun 2022 15:57:14 +0000 Subject: [PATCH 12/19] fixup! cli: templatecreate: add CLI flags --max-ttl --min-autostart-interval --- cli/create.go | 31 +++++++++++++++++++++++++------ cli/create_test.go | 4 ++-- coderd/templates.go | 20 +++++++++++--------- codersdk/templates.go | 20 +++++++++++--------- 4 files changed, 49 insertions(+), 26 deletions(-) diff --git a/cli/create.go b/cli/create.go index 92e9bfa9bda21..973e0444537c4 100644 --- a/cli/create.go +++ b/cli/create.go @@ -10,6 +10,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" ) @@ -114,10 +115,22 @@ func create() *cobra.Command { } } - schedSpec := buildSchedule(autostartMinute, autostartHour, autostartDow, tzName) + schedSpec, err := validSchedule( + autostartMinute, + autostartHour, + autostartDow, + tzName, + time.Duration(template.MinAutostartIntervalMillis)*time.Millisecond, + ) + if err != nil { + return xerrors.Errorf("Invalid autostart schedule: %w", err) + } if ttl < time.Minute { return xerrors.Errorf("TTL must be at least 1 minute") } + if ttlMax := time.Duration(template.MaxTTLMillis) * time.Millisecond; ttl > ttlMax { + return xerrors.Errorf("TTL must be below template maximum %s", ttlMax) + } templateVersion, err := client.TemplateVersion(cmd.Context(), template.ActiveVersionID) if err != nil { @@ -257,11 +270,17 @@ func create() *cobra.Command { return cmd } -func buildSchedule(minute, hour, dow, tzName string) *string { - if minute == "" || hour == "" || dow == "" { - return nil +func validSchedule(minute, hour, dow, tzName string, min time.Duration) (*string, error) { + schedSpec := fmt.Sprintf("CRON_TZ=%s %s %s * * %s", tzName, minute, hour, dow) + + sched, err := schedule.Weekly(schedSpec) + if err != nil { + return nil, err } - schedSpec := fmt.Sprintf("CRON_TZ=%s %s %s * * %s", tzName, minute, hour, dow) - return &schedSpec + if schedMin := sched.Min(); schedMin < min { + return nil, xerrors.Errorf("minimum autostart interval %s is above template constraint %s", schedMin, min) + } + + return &schedSpec, nil } diff --git a/cli/create_test.go b/cli/create_test.go index 3c389008ceca1..ad43f22981ae5 100644 --- a/cli/create_test.go +++ b/cli/create_test.go @@ -85,7 +85,7 @@ func TestCreate(t *testing.T) { cmd.SetIn(pty.Input()) cmd.SetOut(pty.Output()) err := cmd.Execute() - assert.ErrorContains(t, err, "ttl_ms: ttl must be below template maximum 12h0m0s") + assert.ErrorContains(t, err, "TTL must be below template maximum 12h0m0s") }) t.Run("BelowTemplateMinAutostartInterval", func(t *testing.T) { @@ -111,7 +111,7 @@ func TestCreate(t *testing.T) { cmd.SetIn(pty.Input()) cmd.SetOut(pty.Output()) err := cmd.Execute() - assert.ErrorContains(t, err, "Minimum autostart interval 1m0s below template minimum 1h0m0s") + assert.ErrorContains(t, err, "minimum autostart interval 1m0s is above template constraint 1h0m0s") }) t.Run("CreateErrInvalidTz", func(t *testing.T) { diff --git a/coderd/templates.go b/coderd/templates.go index 69498626a7190..d220f37af411e 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -314,14 +314,16 @@ func convertTemplates(templates []database.Template, workspaceCounts []database. func convertTemplate(template database.Template, workspaceOwnerCount uint32) codersdk.Template { return codersdk.Template{ - ID: template.ID, - CreatedAt: template.CreatedAt, - UpdatedAt: template.UpdatedAt, - OrganizationID: template.OrganizationID, - Name: template.Name, - Provisioner: codersdk.ProvisionerType(template.Provisioner), - ActiveVersionID: template.ActiveVersionID, - WorkspaceOwnerCount: workspaceOwnerCount, - Description: template.Description, + ID: template.ID, + CreatedAt: template.CreatedAt, + UpdatedAt: template.UpdatedAt, + OrganizationID: template.OrganizationID, + Name: template.Name, + Provisioner: codersdk.ProvisionerType(template.Provisioner), + ActiveVersionID: template.ActiveVersionID, + WorkspaceOwnerCount: workspaceOwnerCount, + Description: template.Description, + MaxTTLMillis: time.Duration(template.MaxTtl).Milliseconds(), + MinAutostartIntervalMillis: time.Duration(template.MinAutostartInterval).Milliseconds(), } } diff --git a/codersdk/templates.go b/codersdk/templates.go index 6ca9361ad89df..62ec738511cfd 100644 --- a/codersdk/templates.go +++ b/codersdk/templates.go @@ -13,15 +13,17 @@ import ( // Template is the JSON representation of a Coder template. This type matches the // database object for now, but is abstracted for ease of change later on. type Template struct { - ID uuid.UUID `json:"id"` - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` - OrganizationID uuid.UUID `json:"organization_id"` - Name string `json:"name"` - Provisioner ProvisionerType `json:"provisioner"` - ActiveVersionID uuid.UUID `json:"active_version_id"` - WorkspaceOwnerCount uint32 `json:"workspace_owner_count"` - Description string `json:"description"` + ID uuid.UUID `json:"id"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` + OrganizationID uuid.UUID `json:"organization_id"` + Name string `json:"name"` + Provisioner ProvisionerType `json:"provisioner"` + ActiveVersionID uuid.UUID `json:"active_version_id"` + WorkspaceOwnerCount uint32 `json:"workspace_owner_count"` + Description string `json:"description"` + MaxTTLMillis int64 `json:"max_ttl_ms"` + MinAutostartIntervalMillis int64 `json:"min_autostart_interval_ms"` } type UpdateActiveTemplateVersion struct { From c4a09270b4d781142ed04bd20af386afe7623b74 Mon Sep 17 00:00:00 2001 From: johnstcn Date: Fri, 3 Jun 2022 16:45:54 +0000 Subject: [PATCH 13/19] make gen --- site/src/api/typesGenerated.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 1e0c08011711c..32e7cfe6a43a3 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -245,6 +245,8 @@ export interface Template { readonly active_version_id: string readonly workspace_owner_count: number readonly description: string + readonly max_ttl_ms: number + readonly min_autostart_interval_ms: number } // From codersdk/templateversions.go:14:6 @@ -274,12 +276,12 @@ export interface TemplateVersionParameter { readonly default_source_value: boolean } -// From codersdk/templates.go:73:6 +// From codersdk/templates.go:75:6 export interface TemplateVersionsByTemplateRequest extends Pagination { readonly template_id: string } -// From codersdk/templates.go:27:6 +// From codersdk/templates.go:29:6 export interface UpdateActiveTemplateVersion { readonly id: string } From 244e479dc837a4ea5cbf36047ce32c44fd9e08a1 Mon Sep 17 00:00:00 2001 From: johnstcn Date: Fri, 3 Jun 2022 17:21:23 +0000 Subject: [PATCH 14/19] update test entities --- site/src/testHelpers/entities.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index a73a1b14a9077..f42590d11c97b 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -112,6 +112,8 @@ export const MockTemplate: TypesGen.Template = { active_version_id: MockTemplateVersion.id, workspace_owner_count: 1, description: "This is a test description.", + max_ttl_ms: 604800000, + min_autostart_interval_ms: 3600000, } export const MockWorkspaceAutostartDisabled: TypesGen.UpdateWorkspaceAutostartRequest = { From c9aab8be8b2bbe17413b3119def0f6bc0b4a7ff1 Mon Sep 17 00:00:00 2001 From: johnstcn Date: Tue, 7 Jun 2022 10:54:13 +0000 Subject: [PATCH 15/19] fixup! Merge remote-tracking branch 'origin/main' into 1433-template-constraint-for-workspace-scheduling --- cli/create.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cli/create.go b/cli/create.go index a484f70e937a9..4a178bf1f5a2c 100644 --- a/cli/create.go +++ b/cli/create.go @@ -271,6 +271,11 @@ func create() *cobra.Command { } func validSchedule(minute, hour, dow, tzName string, min time.Duration) (*string, error) { + _, err := time.LoadLocation(tzName) + if err != nil { + return nil, xerrors.Errorf("Invalid workspace autostart timezone: %w", err) + } + schedSpec := fmt.Sprintf("CRON_TZ=%s %s %s * * %s", tzName, minute, hour, dow) sched, err := schedule.Weekly(schedSpec) From f997af26194ede8424a93ea6781930d6db8590f4 Mon Sep 17 00:00:00 2001 From: johnstcn Date: Tue, 7 Jun 2022 11:22:14 +0000 Subject: [PATCH 16/19] coderd: default newly-minted workspaces to template max_ttl --- coderd/workspaces.go | 5 +++++ coderd/workspaces_test.go | 21 ++++++++++++++++++--- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 4280224affac4..453229b3e3196 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -287,6 +287,11 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req return } + if !dbTTL.Valid { + // Default to template maximum when creating a new workspace + dbTTL = sql.NullInt64{Valid: true, Int64: template.MaxTtl} + } + workspace, err := api.Database.GetWorkspaceByOwnerIDAndName(r.Context(), database.GetWorkspaceByOwnerIDAndNameParams{ OwnerID: apiKey.UserID, Name: createWorkspace.Name, diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 72bc97fb33cfe..cb8a83e946039 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -165,6 +165,24 @@ func TestPostWorkspacesByOrganization(t *testing.T) { _ = coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) }) + t.Run("TemplateCustomTTL", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) + user := coderdtest.CreateFirstUser(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + templateTTL := 24 * time.Hour.Milliseconds() + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) { + ctr.MaxTTLMillis = ptr.Ref(templateTTL) + }) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID, func(cwr *codersdk.CreateWorkspaceRequest) { + cwr.TTLMillis = nil // ensure that no default TTL is set + }) + // TTL should be set by the template + require.Equal(t, template.MaxTTLMillis, templateTTL) + require.Equal(t, template.MaxTTLMillis, template.MaxTTLMillis, workspace.TTLMillis) + }) + t.Run("InvalidTTL", func(t *testing.T) { t.Parallel() t.Run("BelowMin", func(t *testing.T) { @@ -653,9 +671,6 @@ func TestWorkspaceUpdateTTL(t *testing.T) { }) ) - // ensure test invariant: new workspaces have no autostop schedule. - require.Nil(t, workspace.TTLMillis, "expected newly-minted workspace to have no TTL") - err := client.UpdateWorkspaceTTL(ctx, workspace.ID, codersdk.UpdateWorkspaceTTLRequest{ TTLMillis: testCase.ttlMillis, }) From d7f697b3405b8efda7846daca57a8ce6029f9679 Mon Sep 17 00:00:00 2001 From: johnstcn Date: Tue, 7 Jun 2022 11:54:16 +0000 Subject: [PATCH 17/19] address PR comment --- coderd/workspaces.go | 1 + 1 file changed, 1 insertion(+) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 453229b3e3196..eff07b5d1611d 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -472,6 +472,7 @@ func (api *API) putWorkspaceAutostart(rw http.ResponseWriter, r *http.Request) { httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{ Message: "Error fetching workspace template", }) + return } dbSched, err := validWorkspaceSchedule(req.Schedule, time.Duration(template.MinAutostartInterval)) From ebe4c15ef4e688ad96e71484bdb1fef26346bc78 Mon Sep 17 00:00:00 2001 From: johnstcn Date: Tue, 7 Jun 2022 11:54:26 +0000 Subject: [PATCH 18/19] fix broken unit test --- .../executor/lifecycle_executor_test.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/coderd/autobuild/executor/lifecycle_executor_test.go b/coderd/autobuild/executor/lifecycle_executor_test.go index 86ddd23aa548b..0984738f68659 100644 --- a/coderd/autobuild/executor/lifecycle_executor_test.go +++ b/coderd/autobuild/executor/lifecycle_executor_test.go @@ -291,6 +291,7 @@ func TestExecutorAutostopNotEnabled(t *testing.T) { t.Parallel() var ( + ctx = context.Background() tickCh = make(chan time.Time) statsCh = make(chan executor.Stats) client = coderdtest.New(t, &coderdtest.Options{ @@ -298,15 +299,22 @@ func TestExecutorAutostopNotEnabled(t *testing.T) { IncludeProvisionerD: true, AutobuildStats: statsCh, }) - // Given: we have a user with a workspace that has no TTL set - workspace = mustProvisionWorkspace(t, client, func(cwr *codersdk.CreateWorkspaceRequest) { - cwr.TTLMillis = nil - }) + // Given: we have a user with a workspace + workspace = mustProvisionWorkspace(t, client) ) // Given: workspace has no TTL set + err := client.UpdateWorkspaceTTL(ctx, workspace.ID, codersdk.UpdateWorkspaceTTLRequest{TTLMillis: nil}) + require.NoError(t, err) + workspace, err = client.Workspace(ctx, workspace.ID) + 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 + mustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) + mustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStop, database.WorkspaceTransitionStart) + // Given: workspace is running require.Equal(t, codersdk.WorkspaceTransitionStart, workspace.LatestBuild.Transition) From 08ef2ebed969c1537e36b865e06ab3c46f80d095 Mon Sep 17 00:00:00 2001 From: johnstcn Date: Tue, 7 Jun 2022 12:23:57 +0000 Subject: [PATCH 19/19] fix some more tests --- cli/bump_test.go | 12 ++++ cli/create_test.go | 2 +- .../executor/lifecycle_executor_test.go | 56 ++++--------------- coderd/coderdtest/coderdtest.go | 36 ++++++++++++ 4 files changed, 59 insertions(+), 47 deletions(-) diff --git a/cli/bump_test.go b/cli/bump_test.go index da00bb33b7fd3..041f8996919cf 100644 --- a/cli/bump_test.go +++ b/cli/bump_test.go @@ -10,6 +10,7 @@ import ( "github.com/coder/coder/cli/clitest" "github.com/coder/coder/coderd/coderdtest" + "github.com/coder/coder/coderd/database" "github.com/coder/coder/codersdk" ) @@ -152,12 +153,23 @@ func TestBump(t *testing.T) { cmdArgs = []string{"bump", workspace.Name} stdoutBuf = &bytes.Buffer{} ) + // Unset the workspace TTL + err = client.UpdateWorkspaceTTL(ctx, workspace.ID, codersdk.UpdateWorkspaceTTLRequest{TTLMillis: nil}) + require.NoError(t, err) + workspace, err = client.Workspace(ctx, workspace.ID) + require.NoError(t, err) + require.Nil(t, workspace.TTLMillis) // Given: we wait for the workspace to build coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) workspace, err = client.Workspace(ctx, workspace.ID) require.NoError(t, err) + // 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 + coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) + coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStop, database.WorkspaceTransitionStart) + // Assert test invariant: workspace has no TTL set require.Zero(t, workspace.LatestBuild.Deadline) require.NoError(t, err) diff --git a/cli/create_test.go b/cli/create_test.go index ad43f22981ae5..1352ee18b0291 100644 --- a/cli/create_test.go +++ b/cli/create_test.go @@ -134,7 +134,7 @@ func TestCreate(t *testing.T) { cmd.SetIn(pty.Input()) cmd.SetOut(pty.Output()) err := cmd.Execute() - assert.ErrorContains(t, err, "schedule: parse schedule: provided bad location invalid: unknown time zone invalid") + assert.ErrorContains(t, err, "Invalid autostart schedule: Invalid workspace autostart timezone: unknown time zone invalid") }) t.Run("CreateErrInvalidTTL", func(t *testing.T) { diff --git a/coderd/autobuild/executor/lifecycle_executor_test.go b/coderd/autobuild/executor/lifecycle_executor_test.go index 0984738f68659..5b1045aea2cab 100644 --- a/coderd/autobuild/executor/lifecycle_executor_test.go +++ b/coderd/autobuild/executor/lifecycle_executor_test.go @@ -3,7 +3,6 @@ package executor_test import ( "context" "os" - "strings" "testing" "time" @@ -16,7 +15,6 @@ import ( "github.com/coder/coder/coderd/util/ptr" "github.com/coder/coder/codersdk" - "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -39,7 +37,7 @@ func TestExecutorAutostartOK(t *testing.T) { }) ) // Given: workspace is stopped - workspace = mustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) + workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) // When: the autobuild executor ticks after the scheduled time go func() { @@ -75,7 +73,7 @@ func TestExecutorAutostartTemplateUpdated(t *testing.T) { }) ) // Given: workspace is stopped - workspace = mustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) + workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) // Given: the workspace template has been updated orgs, err := client.OrganizationsByUser(ctx, workspace.OwnerID.String()) @@ -100,7 +98,7 @@ func TestExecutorAutostartTemplateUpdated(t *testing.T) { assert.Len(t, stats.Transitions, 1) assert.Contains(t, stats.Transitions, workspace.ID) assert.Equal(t, database.WorkspaceTransitionStart, stats.Transitions[workspace.ID]) - ws := mustWorkspace(t, client, workspace.ID) + ws := coderdtest.MustWorkspace(t, client, workspace.ID) assert.Equal(t, workspace.LatestBuild.TemplateVersionID, ws.LatestBuild.TemplateVersionID, "expected workspace build to be using the old template version") } @@ -158,7 +156,7 @@ func TestExecutorAutostartNotEnabled(t *testing.T) { require.Empty(t, workspace.AutostartSchedule) // Given: workspace is stopped - workspace = mustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) + workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) // When: the autobuild executor ticks way into the future go func() { @@ -273,7 +271,7 @@ func TestExecutorAutostopAlreadyStopped(t *testing.T) { ) // Given: workspace is stopped - workspace = mustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) + workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) // When: the autobuild executor ticks past the TTL go func() { @@ -312,8 +310,8 @@ func TestExecutorAutostopNotEnabled(t *testing.T) { // 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 - mustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) - mustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStop, database.WorkspaceTransitionStart) + coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) + coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStop, database.WorkspaceTransitionStart) // Given: workspace is running require.Equal(t, codersdk.WorkspaceTransitionStart, workspace.LatestBuild.Transition) @@ -349,7 +347,7 @@ func TestExecutorWorkspaceDeleted(t *testing.T) { ) // Given: workspace is deleted - workspace = mustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionDelete) + workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionDelete) // When: the autobuild executor ticks go func() { @@ -485,7 +483,7 @@ func TestExecutorAutostartMultipleOK(t *testing.T) { }) ) // Given: workspace is stopped - workspace = mustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) + workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) // When: the autobuild executor ticks past the scheduled time go func() { @@ -516,41 +514,7 @@ func mustProvisionWorkspace(t *testing.T, client *codersdk.Client, mut ...func(* coderdtest.AwaitTemplateVersionJob(t, client, version.ID) ws := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID, mut...) coderdtest.AwaitWorkspaceBuildJob(t, client, ws.LatestBuild.ID) - return mustWorkspace(t, client, ws.ID) -} - -func mustTransitionWorkspace(t *testing.T, client *codersdk.Client, workspaceID uuid.UUID, from, to database.WorkspaceTransition) codersdk.Workspace { - t.Helper() - ctx := context.Background() - workspace, err := client.Workspace(ctx, workspaceID) - require.NoError(t, err, "unexpected error fetching workspace") - require.Equal(t, workspace.LatestBuild.Transition, codersdk.WorkspaceTransition(from), "expected workspace state: %s got: %s", from, workspace.LatestBuild.Transition) - - template, err := client.Template(ctx, workspace.TemplateID) - require.NoError(t, err, "fetch workspace template") - - build, err := client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{ - TemplateVersionID: template.ActiveVersionID, - Transition: codersdk.WorkspaceTransition(to), - }) - require.NoError(t, err, "unexpected error transitioning workspace to %s", to) - - _ = coderdtest.AwaitWorkspaceBuildJob(t, client, build.ID) - - updated := mustWorkspace(t, client, workspace.ID) - require.Equal(t, codersdk.WorkspaceTransition(to), updated.LatestBuild.Transition, "expected workspace to be in state %s but got %s", to, updated.LatestBuild.Transition) - return updated -} - -func mustWorkspace(t *testing.T, client *codersdk.Client, workspaceID uuid.UUID) codersdk.Workspace { - t.Helper() - ctx := context.Background() - ws, err := client.Workspace(ctx, workspaceID) - if err != nil && strings.Contains(err.Error(), "status code 410") { - ws, err = client.DeletedWorkspace(ctx, workspaceID) - } - require.NoError(t, err, "no workspace found with id %s", workspaceID) - return ws + return coderdtest.MustWorkspace(t, client, ws.ID) } func mustSchedule(t *testing.T, s string) *schedule.Schedule { diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index 5642d15bb3e7e..ea068927004df 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -415,6 +415,42 @@ func CreateWorkspace(t *testing.T, client *codersdk.Client, organization uuid.UU return workspace } +// TransitionWorkspace is a convenience method for transitioning a workspace from one state to another. +func MustTransitionWorkspace(t *testing.T, client *codersdk.Client, workspaceID uuid.UUID, from, to database.WorkspaceTransition) codersdk.Workspace { + t.Helper() + ctx := context.Background() + workspace, err := client.Workspace(ctx, workspaceID) + require.NoError(t, err, "unexpected error fetching workspace") + require.Equal(t, workspace.LatestBuild.Transition, codersdk.WorkspaceTransition(from), "expected workspace state: %s got: %s", from, workspace.LatestBuild.Transition) + + template, err := client.Template(ctx, workspace.TemplateID) + require.NoError(t, err, "fetch workspace template") + + build, err := client.CreateWorkspaceBuild(ctx, workspace.ID, codersdk.CreateWorkspaceBuildRequest{ + TemplateVersionID: template.ActiveVersionID, + Transition: codersdk.WorkspaceTransition(to), + }) + require.NoError(t, err, "unexpected error transitioning workspace to %s", to) + + _ = AwaitWorkspaceBuildJob(t, client, build.ID) + + updated := MustWorkspace(t, client, workspace.ID) + require.Equal(t, codersdk.WorkspaceTransition(to), updated.LatestBuild.Transition, "expected workspace to be in state %s but got %s", to, updated.LatestBuild.Transition) + return updated +} + +// MustWorkspace is a convenience method for fetching a workspace that should exist. +func MustWorkspace(t *testing.T, client *codersdk.Client, workspaceID uuid.UUID) codersdk.Workspace { + t.Helper() + ctx := context.Background() + ws, err := client.Workspace(ctx, workspaceID) + if err != nil && strings.Contains(err.Error(), "status code 410") { + ws, err = client.DeletedWorkspace(ctx, workspaceID) + } + require.NoError(t, err, "no workspace found with id %s", workspaceID) + return ws +} + // NewGoogleInstanceIdentity returns a metadata client and ID token validator for faking // instance authentication for Google Cloud. // nolint:revive