From 779b5f2a97b9ee86d599fd4c9503e528633c6331 Mon Sep 17 00:00:00 2001 From: Garrett Delfosse Date: Wed, 15 May 2024 18:10:27 +0000 Subject: [PATCH 1/7] chore: add workspace_build_deadlines view --- .../000209_workspace_deadline_view.down.sql | 0 .../000209_workspace_deadline_view.up.sql | 13 +++++++++++++ 2 files changed, 13 insertions(+) create mode 100644 coderd/database/migrations/000209_workspace_deadline_view.down.sql create mode 100644 coderd/database/migrations/000209_workspace_deadline_view.up.sql diff --git a/coderd/database/migrations/000209_workspace_deadline_view.down.sql b/coderd/database/migrations/000209_workspace_deadline_view.down.sql new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/coderd/database/migrations/000209_workspace_deadline_view.up.sql b/coderd/database/migrations/000209_workspace_deadline_view.up.sql new file mode 100644 index 0000000000000..e0af6573f7698 --- /dev/null +++ b/coderd/database/migrations/000209_workspace_deadline_view.up.sql @@ -0,0 +1,13 @@ +CREATE VIEW workspace_build_deadlines AS + SELECT + workspace_builds.id, + LEAST( + workspaces.last_used_at + (workspaces.ttl / 1000 / 1000 / 1000 || ' seconds')::interval, + workspace_builds.max_deadline + )::timestamp with time zone AS deadline +FROM + workspace_builds +LEFT JOIN + workspaces +ON + workspace_builds.workspace_id = workspaces.id; From 6be0d4ec5ed029b80d020dedf009e6bce44b2468 Mon Sep 17 00:00:00 2001 From: Garrett Delfosse Date: Wed, 15 May 2024 19:38:29 +0000 Subject: [PATCH 2/7] make gen --- coderd/database/dump.sql | 66 ++++++++++--------- .../000209_workspace_deadline_view.up.sql | 11 ++-- coderd/database/models.go | 5 ++ coderd/database/sqlc.yaml | 3 + 4 files changed, 51 insertions(+), 34 deletions(-) diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index ed400cf82198f..b742f0abb1be1 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -1239,16 +1239,6 @@ CREATE TABLE workspace_apps ( COMMENT ON COLUMN workspace_apps.display_order IS 'Specifies the order in which to display agent app in user interfaces.'; -CREATE TABLE workspace_build_parameters ( - workspace_build_id uuid NOT NULL, - name text NOT NULL, - value text NOT NULL -); - -COMMENT ON COLUMN workspace_build_parameters.name IS 'Parameter name'; - -COMMENT ON COLUMN workspace_build_parameters.value IS 'Parameter value'; - CREATE TABLE workspace_builds ( id uuid NOT NULL, created_at timestamp with time zone NOT NULL, @@ -1266,6 +1256,42 @@ CREATE TABLE workspace_builds ( max_deadline timestamp with time zone DEFAULT '0001-01-01 00:00:00+00'::timestamp with time zone NOT NULL ); +CREATE TABLE workspaces ( + id uuid NOT NULL, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL, + owner_id uuid NOT NULL, + organization_id uuid NOT NULL, + template_id uuid NOT NULL, + deleted boolean DEFAULT false NOT NULL, + name character varying(64) NOT NULL, + autostart_schedule text, + ttl bigint, + last_used_at timestamp with time zone DEFAULT '0001-01-01 00:00:00+00'::timestamp with time zone NOT NULL, + dormant_at timestamp with time zone, + deleting_at timestamp with time zone, + automatic_updates automatic_updates DEFAULT 'never'::automatic_updates NOT NULL, + favorite boolean DEFAULT false NOT NULL +); + +COMMENT ON COLUMN workspaces.favorite IS 'Favorite is true if the workspace owner has favorited the workspace.'; + +CREATE VIEW workspace_build_deadlines AS + SELECT workspace_builds.id, + COALESCE(LEAST((workspaces.last_used_at + (((((workspaces.ttl / 1000) / 1000) / 1000) || ' seconds'::text))::interval), workspace_builds.max_deadline), '0001-01-01 00:00:00+00'::timestamp with time zone) AS deadline + FROM (workspace_builds + LEFT JOIN workspaces ON ((workspace_builds.workspace_id = workspaces.id))); + +CREATE TABLE workspace_build_parameters ( + workspace_build_id uuid NOT NULL, + name text NOT NULL, + value text NOT NULL +); + +COMMENT ON COLUMN workspace_build_parameters.name IS 'Parameter name'; + +COMMENT ON COLUMN workspace_build_parameters.value IS 'Parameter value'; + CREATE VIEW workspace_build_with_user AS SELECT workspace_builds.id, workspace_builds.created_at, @@ -1357,26 +1383,6 @@ CREATE TABLE workspace_resources ( daily_cost integer DEFAULT 0 NOT NULL ); -CREATE TABLE workspaces ( - id uuid NOT NULL, - created_at timestamp with time zone NOT NULL, - updated_at timestamp with time zone NOT NULL, - owner_id uuid NOT NULL, - organization_id uuid NOT NULL, - template_id uuid NOT NULL, - deleted boolean DEFAULT false NOT NULL, - name character varying(64) NOT NULL, - autostart_schedule text, - ttl bigint, - last_used_at timestamp with time zone DEFAULT '0001-01-01 00:00:00+00'::timestamp with time zone NOT NULL, - dormant_at timestamp with time zone, - deleting_at timestamp with time zone, - automatic_updates automatic_updates DEFAULT 'never'::automatic_updates NOT NULL, - favorite boolean DEFAULT false NOT NULL -); - -COMMENT ON COLUMN workspaces.favorite IS 'Favorite is true if the workspace owner has favorited the workspace.'; - ALTER TABLE ONLY licenses ALTER COLUMN id SET DEFAULT nextval('licenses_id_seq'::regclass); ALTER TABLE ONLY provisioner_job_logs ALTER COLUMN id SET DEFAULT nextval('provisioner_job_logs_id_seq'::regclass); diff --git a/coderd/database/migrations/000209_workspace_deadline_view.up.sql b/coderd/database/migrations/000209_workspace_deadline_view.up.sql index e0af6573f7698..6dfbc5696670c 100644 --- a/coderd/database/migrations/000209_workspace_deadline_view.up.sql +++ b/coderd/database/migrations/000209_workspace_deadline_view.up.sql @@ -1,10 +1,13 @@ CREATE VIEW workspace_build_deadlines AS SELECT workspace_builds.id, - LEAST( - workspaces.last_used_at + (workspaces.ttl / 1000 / 1000 / 1000 || ' seconds')::interval, - workspace_builds.max_deadline - )::timestamp with time zone AS deadline + COALESCE( + LEAST( + workspaces.last_used_at + (workspaces.ttl / 1000 / 1000 / 1000 || ' seconds')::interval, + workspace_builds.max_deadline + ), + '0001-01-01 00:00:00+00'::timestamp with time zone + ) AS deadline FROM workspace_builds LEFT JOIN diff --git a/coderd/database/models.go b/coderd/database/models.go index 18587b05ade1a..c6dd3020d5407 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -2499,6 +2499,11 @@ type WorkspaceBuild struct { InitiatorByUsername string `db:"initiator_by_username" json:"initiator_by_username"` } +type WorkspaceBuildDeadline struct { + ID uuid.UUID `db:"id" json:"id"` + Deadline time.Time `db:"deadline" json:"deadline"` +} + type WorkspaceBuildParameter struct { WorkspaceBuildID uuid.UUID `db:"workspace_build_id" json:"workspace_build_id"` // Parameter name diff --git a/coderd/database/sqlc.yaml b/coderd/database/sqlc.yaml index 7913a9acf1627..c2047a00d5587 100644 --- a/coderd/database/sqlc.yaml +++ b/coderd/database/sqlc.yaml @@ -51,6 +51,9 @@ sql: - column: "template_usage_stats.app_usage_mins" go_type: type: "StringMapOfInt" + - column: "workspace_build_deadlines.deadline" + go_type: + type: "time.Time" rename: template: TemplateTable template_with_user: Template From 0ad53399a6a4e8ee3cf45c530109ee279a62814e Mon Sep 17 00:00:00 2001 From: Garrett Delfosse Date: Wed, 15 May 2024 20:49:41 +0000 Subject: [PATCH 3/7] remove activity bump --- coderd/agentapi/activitybump.go | 62 ------ coderd/agentapi/activitybump_test.go | 319 --------------------------- coderd/agentapi/stats.go | 22 -- coderd/workspaceagents.go | 28 +-- 4 files changed, 2 insertions(+), 429 deletions(-) delete mode 100644 coderd/agentapi/activitybump.go delete mode 100644 coderd/agentapi/activitybump_test.go diff --git a/coderd/agentapi/activitybump.go b/coderd/agentapi/activitybump.go deleted file mode 100644 index a28ba695d018e..0000000000000 --- a/coderd/agentapi/activitybump.go +++ /dev/null @@ -1,62 +0,0 @@ -package agentapi - -import ( - "context" - "time" - - "github.com/google/uuid" - "golang.org/x/xerrors" - - "cdr.dev/slog" - "github.com/coder/coder/v2/coderd/database" -) - -// ActivityBumpWorkspace automatically bumps the workspace's auto-off timer -// if it is set to expire soon. The deadline will be bumped by 1 hour*. -// If the bump crosses over an autostart time, the workspace will be -// bumped by the workspace ttl instead. -// -// If nextAutostart is the zero value or in the past, the workspace -// will be bumped by 1 hour. -// It handles the edge case in the example: -// 1. Autostart is set to 9am. -// 2. User works all day, and leaves a terminal open to the workspace overnight. -// 3. The open terminal continually bumps the workspace deadline. -// 4. 9am the next day, the activity bump pushes to 10am. -// 5. If the user goes inactive for 1 hour during the day, the workspace will -// now stop, because it has been extended by 1 hour durations. Despite the TTL -// being set to 8hrs from the autostart time. -// -// So the issue is that when the workspace is bumped across an autostart -// deadline, we should treat the workspace as being "started" again and -// extend the deadline by the autostart time + workspace ttl instead. -// -// The issue still remains with build_max_deadline. We need to respect the original -// maximum deadline, so that will need to be handled separately. -// A way to avoid this is to configure the max deadline to something that will not -// span more than 1 day. This will force the workspace to restart and reset the deadline -// each morning when it autostarts. -func ActivityBumpWorkspace(ctx context.Context, log slog.Logger, db database.Store, workspaceID uuid.UUID, nextAutostart time.Time) { - // We set a short timeout so if the app is under load, these - // low priority operations fail first. - ctx, cancel := context.WithTimeout(ctx, time.Second*15) - defer cancel() - // nolint:gocritic // (#13146) Will be moved soon as part of refactor. - err := db.ActivityBumpWorkspace(ctx, database.ActivityBumpWorkspaceParams{ - NextAutostart: nextAutostart.UTC(), - WorkspaceID: workspaceID, - }) - if err != nil { - if !xerrors.Is(err, context.Canceled) && !database.IsQueryCanceledError(err) { - // Bump will fail if the context is canceled, but this is ok. - log.Error(ctx, "activity bump failed", slog.Error(err), - slog.F("workspace_id", workspaceID), - ) - } - return - } - - log.Debug(ctx, "bumped deadline from activity", - slog.F("workspace_id", workspaceID), - ) -} diff --git a/coderd/agentapi/activitybump_test.go b/coderd/agentapi/activitybump_test.go deleted file mode 100644 index 5c82454c97cef..0000000000000 --- a/coderd/agentapi/activitybump_test.go +++ /dev/null @@ -1,319 +0,0 @@ -package agentapi_test - -import ( - "database/sql" - "testing" - "time" - - "github.com/google/uuid" - - "cdr.dev/slog/sloggers/slogtest" - "github.com/coder/coder/v2/coderd/agentapi" - "github.com/coder/coder/v2/coderd/database" - "github.com/coder/coder/v2/coderd/database/dbgen" - "github.com/coder/coder/v2/coderd/database/dbtestutil" - "github.com/coder/coder/v2/coderd/database/dbtime" - "github.com/coder/coder/v2/coderd/util/ptr" - "github.com/coder/coder/v2/testutil" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func Test_ActivityBumpWorkspace(t *testing.T) { - t.Parallel() - - // We test the below in multiple timezones specifically - // chosen to trigger timezone-related bugs. - timezones := []string{ - "Asia/Kolkata", // No DST, positive fractional offset - "Canada/Newfoundland", // DST, negative fractional offset - "Europe/Paris", // DST, positive offset - "US/Arizona", // No DST, negative offset - "UTC", // Baseline - } - - for _, tt := range []struct { - name string - transition database.WorkspaceTransition - jobCompletedAt sql.NullTime - buildDeadlineOffset *time.Duration - maxDeadlineOffset *time.Duration - workspaceTTL time.Duration - templateTTL time.Duration - templateActivityBump time.Duration - templateDisallowsUserAutostop bool - expectedBump time.Duration - // If the tests get queued, we need to be able to set the next autostart - // based on the actual time the unit test is running. - nextAutostart func(now time.Time) time.Time - }{ - { - name: "NotFinishedYet", - transition: database.WorkspaceTransitionStart, - jobCompletedAt: sql.NullTime{}, - buildDeadlineOffset: ptr.Ref(8 * time.Hour), - workspaceTTL: 8 * time.Hour, - expectedBump: 0, - }, - { - name: "ManualShutdown", - transition: database.WorkspaceTransitionStart, - jobCompletedAt: sql.NullTime{Valid: true, Time: dbtime.Now()}, - buildDeadlineOffset: nil, - expectedBump: 0, - }, - { - name: "NotTimeToBumpYet", - transition: database.WorkspaceTransitionStart, - jobCompletedAt: sql.NullTime{Valid: true, Time: dbtime.Now()}, - buildDeadlineOffset: ptr.Ref(8 * time.Hour), - workspaceTTL: 8 * time.Hour, - expectedBump: 0, - }, - { - // Expected bump is 0 because the original deadline is more than 1 hour - // out, so a bump would decrease the deadline. - name: "BumpLessThanDeadline", - transition: database.WorkspaceTransitionStart, - jobCompletedAt: sql.NullTime{Valid: true, Time: dbtime.Now().Add(-30 * time.Minute)}, - buildDeadlineOffset: ptr.Ref(8*time.Hour - 30*time.Minute), - workspaceTTL: 8 * time.Hour, - expectedBump: 0, - }, - { - name: "TimeToBump", - transition: database.WorkspaceTransitionStart, - jobCompletedAt: sql.NullTime{Valid: true, Time: dbtime.Now().Add(-30 * time.Minute)}, - buildDeadlineOffset: ptr.Ref(-30 * time.Minute), - workspaceTTL: 8 * time.Hour, - expectedBump: time.Hour, - }, - { - name: "TimeToBumpNextAutostart", - transition: database.WorkspaceTransitionStart, - jobCompletedAt: sql.NullTime{Valid: true, Time: dbtime.Now().Add(-30 * time.Minute)}, - buildDeadlineOffset: ptr.Ref(-30 * time.Minute), - workspaceTTL: 8 * time.Hour, - expectedBump: 8*time.Hour + 30*time.Minute, - nextAutostart: func(now time.Time) time.Time { return now.Add(time.Minute * 30) }, - }, - { - name: "MaxDeadline", - transition: database.WorkspaceTransitionStart, - jobCompletedAt: sql.NullTime{Valid: true, Time: dbtime.Now().Add(-24 * time.Minute)}, - buildDeadlineOffset: ptr.Ref(time.Minute), // last chance to bump! - maxDeadlineOffset: ptr.Ref(time.Minute * 30), - workspaceTTL: 8 * time.Hour, - expectedBump: time.Minute * 30, - }, - { - // A workspace that is still running, has passed its deadline, but has not - // yet been auto-stopped should still bump the deadline. - name: "PastDeadlineStillBumps", - transition: database.WorkspaceTransitionStart, - jobCompletedAt: sql.NullTime{Valid: true, Time: dbtime.Now().Add(-24 * time.Minute)}, - buildDeadlineOffset: ptr.Ref(-time.Minute), - workspaceTTL: 8 * time.Hour, - expectedBump: time.Hour, - }, - { - // A stopped workspace should never bump. - name: "StoppedWorkspace", - transition: database.WorkspaceTransitionStop, - jobCompletedAt: sql.NullTime{Valid: true, Time: dbtime.Now().Add(-time.Minute)}, - buildDeadlineOffset: ptr.Ref(-time.Minute), - workspaceTTL: 8 * time.Hour, - }, - { - // A workspace built from a template that disallows user autostop should bump - // by the template TTL instead. - name: "TemplateDisallowsUserAutostop", - transition: database.WorkspaceTransitionStart, - jobCompletedAt: sql.NullTime{Valid: true, Time: dbtime.Now().Add(-3 * time.Hour)}, - buildDeadlineOffset: ptr.Ref(-30 * time.Minute), - workspaceTTL: 2 * time.Hour, - templateTTL: 10 * time.Hour, - templateDisallowsUserAutostop: true, - expectedBump: 10*time.Hour + (time.Minute * 30), - nextAutostart: func(now time.Time) time.Time { return now.Add(time.Minute * 30) }, - }, - { - // Custom activity bump duration specified on the template. - name: "TemplateCustomActivityBump", - transition: database.WorkspaceTransitionStart, - jobCompletedAt: sql.NullTime{Valid: true, Time: dbtime.Now().Add(-30 * time.Minute)}, - buildDeadlineOffset: ptr.Ref(-30 * time.Minute), - workspaceTTL: 8 * time.Hour, - templateActivityBump: 5 * time.Hour, // instead of default 1h - expectedBump: 5 * time.Hour, - }, - { - // Activity bump duration is 0. - name: "TemplateCustomActivityBumpZero", - transition: database.WorkspaceTransitionStart, - jobCompletedAt: sql.NullTime{Valid: true, Time: dbtime.Now().Add(-30 * time.Minute)}, - buildDeadlineOffset: ptr.Ref(-30 * time.Minute), - workspaceTTL: 8 * time.Hour, - templateActivityBump: -1, // negative values get changed to 0 in the test - expectedBump: 0, - }, - } { - tt := tt - for _, tz := range timezones { - tz := tz - t.Run(tt.name+"/"+tz, func(t *testing.T) { - t.Parallel() - nextAutostart := tt.nextAutostart - if tt.nextAutostart == nil { - nextAutostart = func(now time.Time) time.Time { return time.Time{} } - } - - var ( - now = dbtime.Now() - ctx = testutil.Context(t, testutil.WaitShort) - log = slogtest.Make(t, nil) - db, _ = dbtestutil.NewDB(t, dbtestutil.WithTimezone(tz)) - org = dbgen.Organization(t, db, database.Organization{}) - user = dbgen.User(t, db, database.User{ - Status: database.UserStatusActive, - }) - _ = dbgen.OrganizationMember(t, db, database.OrganizationMember{ - UserID: user.ID, - OrganizationID: org.ID, - }) - templateVersion = dbgen.TemplateVersion(t, db, database.TemplateVersion{ - OrganizationID: org.ID, - CreatedBy: user.ID, - }) - template = dbgen.Template(t, db, database.Template{ - OrganizationID: org.ID, - ActiveVersionID: templateVersion.ID, - CreatedBy: user.ID, - }) - ws = dbgen.Workspace(t, db, database.Workspace{ - OwnerID: user.ID, - OrganizationID: org.ID, - TemplateID: template.ID, - Ttl: sql.NullInt64{Valid: true, Int64: int64(tt.workspaceTTL)}, - }) - job = dbgen.ProvisionerJob(t, db, nil, database.ProvisionerJob{ - OrganizationID: org.ID, - CompletedAt: tt.jobCompletedAt, - }) - _ = dbgen.WorkspaceResource(t, db, database.WorkspaceResource{ - JobID: job.ID, - }) - buildID = uuid.New() - ) - - activityBump := 1 * time.Hour - if tt.templateActivityBump < 0 { - // less than 0 => 0 - activityBump = 0 - } else if tt.templateActivityBump != 0 { - activityBump = tt.templateActivityBump - } - require.NoError(t, db.UpdateTemplateScheduleByID(ctx, database.UpdateTemplateScheduleByIDParams{ - ID: template.ID, - UpdatedAt: dbtime.Now(), - AllowUserAutostop: !tt.templateDisallowsUserAutostop, - DefaultTTL: int64(tt.templateTTL), - ActivityBump: int64(activityBump), - }), "unexpected error updating template schedule") - - var buildNumber int32 = 1 - // Insert a number of previous workspace builds. - for i := 0; i < 5; i++ { - insertPrevWorkspaceBuild(t, db, org.ID, templateVersion.ID, ws.ID, database.WorkspaceTransitionStart, buildNumber) - buildNumber++ - insertPrevWorkspaceBuild(t, db, org.ID, templateVersion.ID, ws.ID, database.WorkspaceTransitionStop, buildNumber) - buildNumber++ - } - - // dbgen.WorkspaceBuild automatically sets deadline to now+1 hour if not set - var buildDeadline time.Time - if tt.buildDeadlineOffset != nil { - buildDeadline = now.Add(*tt.buildDeadlineOffset) - } - var maxDeadline time.Time - if tt.maxDeadlineOffset != nil { - maxDeadline = now.Add(*tt.maxDeadlineOffset) - } - err := db.InsertWorkspaceBuild(ctx, database.InsertWorkspaceBuildParams{ - ID: buildID, - CreatedAt: dbtime.Now(), - UpdatedAt: dbtime.Now(), - BuildNumber: buildNumber, - InitiatorID: user.ID, - Reason: database.BuildReasonInitiator, - WorkspaceID: ws.ID, - JobID: job.ID, - TemplateVersionID: templateVersion.ID, - Transition: tt.transition, - Deadline: buildDeadline, - MaxDeadline: maxDeadline, - }) - require.NoError(t, err, "unexpected error inserting workspace build") - bld, err := db.GetWorkspaceBuildByID(ctx, buildID) - require.NoError(t, err, "unexpected error fetching inserted workspace build") - - // Validate our initial state before bump - require.Equal(t, tt.transition, bld.Transition, "unexpected transition before bump") - require.Equal(t, tt.jobCompletedAt.Time.UTC(), job.CompletedAt.Time.UTC(), "unexpected job completed at before bump") - require.Equal(t, buildDeadline.UTC(), bld.Deadline.UTC(), "unexpected build deadline before bump") - require.Equal(t, maxDeadline.UTC(), bld.MaxDeadline.UTC(), "unexpected max deadline before bump") - require.Equal(t, tt.workspaceTTL, time.Duration(ws.Ttl.Int64), "unexpected workspace TTL before bump") - - // Wait a bit before bumping as dbtime is rounded to the nearest millisecond. - // This should also hopefully be enough for Windows time resolution to register - // a tick (win32 max timer resolution is apparently between 0.5 and 15.6ms) - <-time.After(testutil.IntervalFast) - - // Bump duration is measured from the time of the bump, so we measure from here. - start := dbtime.Now() - agentapi.ActivityBumpWorkspace(ctx, log, db, bld.WorkspaceID, nextAutostart(start)) - end := dbtime.Now() - - // Validate our state after bump - updatedBuild, err := db.GetLatestWorkspaceBuildByWorkspaceID(ctx, bld.WorkspaceID) - require.NoError(t, err, "unexpected error getting latest workspace build") - require.Equal(t, bld.MaxDeadline.UTC(), updatedBuild.MaxDeadline.UTC(), "max_deadline should not have changed") - if tt.expectedBump == 0 { - assert.Equal(t, bld.UpdatedAt.UTC(), updatedBuild.UpdatedAt.UTC(), "should not have bumped updated_at") - assert.Equal(t, bld.Deadline.UTC(), updatedBuild.Deadline.UTC(), "should not have bumped deadline") - return - } - assert.NotEqual(t, bld.UpdatedAt.UTC(), updatedBuild.UpdatedAt.UTC(), "should have bumped updated_at") - if tt.maxDeadlineOffset != nil { - assert.Equal(t, bld.MaxDeadline.UTC(), updatedBuild.MaxDeadline.UTC(), "new deadline must equal original max deadline") - return - } - - // Assert that the bump occurred between start and end. 1min buffer on either side. - expectedDeadlineStart := start.Add(tt.expectedBump).Add(time.Minute * -1) - expectedDeadlineEnd := end.Add(tt.expectedBump).Add(time.Minute) - require.GreaterOrEqual(t, updatedBuild.Deadline, expectedDeadlineStart, "new deadline should be greater than or equal to start") - require.LessOrEqual(t, updatedBuild.Deadline, expectedDeadlineEnd, "new deadline should be less than or equal to end") - }) - } - } -} - -func insertPrevWorkspaceBuild(t *testing.T, db database.Store, orgID, tvID, workspaceID uuid.UUID, transition database.WorkspaceTransition, buildNumber int32) { - t.Helper() - - job := dbgen.ProvisionerJob(t, db, nil, database.ProvisionerJob{ - OrganizationID: orgID, - }) - _ = dbgen.WorkspaceResource(t, db, database.WorkspaceResource{ - JobID: job.ID, - }) - _ = dbgen.WorkspaceBuild(t, db, database.WorkspaceBuild{ - BuildNumber: buildNumber, - WorkspaceID: workspaceID, - JobID: job.ID, - TemplateVersionID: tvID, - Transition: transition, - }) -} diff --git a/coderd/agentapi/stats.go b/coderd/agentapi/stats.go index e91a3624e915d..dc1bf19aec58f 100644 --- a/coderd/agentapi/stats.go +++ b/coderd/agentapi/stats.go @@ -70,28 +70,6 @@ func (a *StatsAPI) UpdateStats(ctx context.Context, req *agentproto.UpdateStatsR ) now := a.now() - if req.Stats.ConnectionCount > 0 { - var nextAutostart time.Time - if workspace.AutostartSchedule.String != "" { - templateSchedule, err := (*(a.TemplateScheduleStore.Load())).Get(ctx, a.Database, workspace.TemplateID) - // If the template schedule fails to load, just default to bumping - // without the next transition and log it. - if err != nil { - a.Log.Error(ctx, "failed to load template schedule bumping activity, defaulting to bumping by 60min", - slog.F("workspace_id", workspace.ID), - slog.F("template_id", workspace.TemplateID), - slog.Error(err), - ) - } else { - next, allowed := schedule.NextAutostart(now, workspace.AutostartSchedule.String, templateSchedule) - if allowed { - nextAutostart = next - } - } - } - ActivityBumpWorkspace(ctx, a.Log.Named("activity_bump"), a.Database, workspace.ID, nextAutostart) - } - var errGroup errgroup.Group errGroup.Go(func() error { err := a.StatsBatcher.Add(now, workspaceAgent.ID, workspace.TemplateID, workspace.OwnerID, workspace.ID, req.Stats) diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go index 9faae72f22ef7..68798d4144700 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -36,7 +36,6 @@ import ( "github.com/coder/coder/v2/coderd/httpmw" "github.com/coder/coder/v2/coderd/prometheusmetrics" "github.com/coder/coder/v2/coderd/rbac/policy" - "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/codersdk/agentsdk" "github.com/coder/coder/v2/codersdk/workspacesdk" @@ -1168,31 +1167,8 @@ func (api *API) workspaceAgentReportStats(rw http.ResponseWriter, r *http.Reques ) if req.ConnectionCount > 0 { - var nextAutostart time.Time - if workspace.AutostartSchedule.String != "" { - templateSchedule, err := (*(api.TemplateScheduleStore.Load())).Get(ctx, api.Database, workspace.TemplateID) - // If the template schedule fails to load, just default to bumping without the next transition and log it. - if err != nil { - // There's nothing we can do if the query was canceled, the - // client most likely went away so we just return an internal - // server error. - if database.IsQueryCanceledError(err) { - httpapi.InternalServerError(rw, err) - return - } - api.Logger.Error(ctx, "failed to load template schedule bumping activity, defaulting to bumping by 60min", - slog.F("workspace_id", workspace.ID), - slog.F("template_id", workspace.TemplateID), - slog.Error(err), - ) - } else { - next, allowed := schedule.NextAutostart(time.Now(), workspace.AutostartSchedule.String, templateSchedule) - if allowed { - nextAutostart = next - } - } - } - agentapi.ActivityBumpWorkspace(ctx, api.Logger.Named("activity_bump"), api.Database, workspace.ID, nextAutostart) + // do we still need to bump something here? + // or is it handled below with the req.SessionCount() > 0 check? } now := dbtime.Now() From 02c9991ed3b4e6708a677cdfde5d20b5ed09d388 Mon Sep 17 00:00:00 2001 From: Garrett Delfosse Date: Thu, 16 May 2024 15:20:57 +0000 Subject: [PATCH 4/7] change view to workspaces --- coderd/database/dump.sql | 75 ++++++++++--------- .../000209_workspace_deadline_view.up.sql | 28 ++++--- coderd/database/models.go | 10 +-- coderd/database/sqlc.yaml | 2 +- 4 files changed, 62 insertions(+), 53 deletions(-) diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index b742f0abb1be1..63143e78fadb1 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -1239,6 +1239,16 @@ CREATE TABLE workspace_apps ( COMMENT ON COLUMN workspace_apps.display_order IS 'Specifies the order in which to display agent app in user interfaces.'; +CREATE TABLE workspace_build_parameters ( + workspace_build_id uuid NOT NULL, + name text NOT NULL, + value text NOT NULL +); + +COMMENT ON COLUMN workspace_build_parameters.name IS 'Parameter name'; + +COMMENT ON COLUMN workspace_build_parameters.value IS 'Parameter value'; + CREATE TABLE workspace_builds ( id uuid NOT NULL, created_at timestamp with time zone NOT NULL, @@ -1256,42 +1266,6 @@ CREATE TABLE workspace_builds ( max_deadline timestamp with time zone DEFAULT '0001-01-01 00:00:00+00'::timestamp with time zone NOT NULL ); -CREATE TABLE workspaces ( - id uuid NOT NULL, - created_at timestamp with time zone NOT NULL, - updated_at timestamp with time zone NOT NULL, - owner_id uuid NOT NULL, - organization_id uuid NOT NULL, - template_id uuid NOT NULL, - deleted boolean DEFAULT false NOT NULL, - name character varying(64) NOT NULL, - autostart_schedule text, - ttl bigint, - last_used_at timestamp with time zone DEFAULT '0001-01-01 00:00:00+00'::timestamp with time zone NOT NULL, - dormant_at timestamp with time zone, - deleting_at timestamp with time zone, - automatic_updates automatic_updates DEFAULT 'never'::automatic_updates NOT NULL, - favorite boolean DEFAULT false NOT NULL -); - -COMMENT ON COLUMN workspaces.favorite IS 'Favorite is true if the workspace owner has favorited the workspace.'; - -CREATE VIEW workspace_build_deadlines AS - SELECT workspace_builds.id, - COALESCE(LEAST((workspaces.last_used_at + (((((workspaces.ttl / 1000) / 1000) / 1000) || ' seconds'::text))::interval), workspace_builds.max_deadline), '0001-01-01 00:00:00+00'::timestamp with time zone) AS deadline - FROM (workspace_builds - LEFT JOIN workspaces ON ((workspace_builds.workspace_id = workspaces.id))); - -CREATE TABLE workspace_build_parameters ( - workspace_build_id uuid NOT NULL, - name text NOT NULL, - value text NOT NULL -); - -COMMENT ON COLUMN workspace_build_parameters.name IS 'Parameter name'; - -COMMENT ON COLUMN workspace_build_parameters.value IS 'Parameter value'; - CREATE VIEW workspace_build_with_user AS SELECT workspace_builds.id, workspace_builds.created_at, @@ -1314,6 +1288,35 @@ CREATE VIEW workspace_build_with_user AS COMMENT ON VIEW workspace_build_with_user IS 'Joins in the username + avatar url of the initiated by user.'; +CREATE TABLE workspaces ( + id uuid NOT NULL, + created_at timestamp with time zone NOT NULL, + updated_at timestamp with time zone NOT NULL, + owner_id uuid NOT NULL, + organization_id uuid NOT NULL, + template_id uuid NOT NULL, + deleted boolean DEFAULT false NOT NULL, + name character varying(64) NOT NULL, + autostart_schedule text, + ttl bigint, + last_used_at timestamp with time zone DEFAULT '0001-01-01 00:00:00+00'::timestamp with time zone NOT NULL, + dormant_at timestamp with time zone, + deleting_at timestamp with time zone, + automatic_updates automatic_updates DEFAULT 'never'::automatic_updates NOT NULL, + favorite boolean DEFAULT false NOT NULL +); + +COMMENT ON COLUMN workspaces.favorite IS 'Favorite is true if the workspace owner has favorited the workspace.'; + +CREATE VIEW workspace_deadlines AS + SELECT workspaces.id, + LEAST((workspaces.last_used_at + (((((workspaces.ttl / 1000) / 1000) / 1000) || ' seconds'::text))::interval), workspace_builds.max_deadline) AS deadline + FROM (workspaces + LEFT JOIN workspace_builds ON ((workspace_builds.workspace_id = workspaces.id))) + WHERE (workspace_builds.build_number = ( SELECT max(workspace_builds_1.build_number) AS max + FROM workspace_builds workspace_builds_1 + WHERE (workspace_builds_1.workspace_id = workspaces.id))); + CREATE TABLE workspace_proxies ( id uuid NOT NULL, name text NOT NULL, diff --git a/coderd/database/migrations/000209_workspace_deadline_view.up.sql b/coderd/database/migrations/000209_workspace_deadline_view.up.sql index 6dfbc5696670c..4fa2aeff2cebb 100644 --- a/coderd/database/migrations/000209_workspace_deadline_view.up.sql +++ b/coderd/database/migrations/000209_workspace_deadline_view.up.sql @@ -1,16 +1,22 @@ -CREATE VIEW workspace_build_deadlines AS +CREATE VIEW workspace_deadlines AS SELECT - workspace_builds.id, - COALESCE( - LEAST( - workspaces.last_used_at + (workspaces.ttl / 1000 / 1000 / 1000 || ' seconds')::interval, - workspace_builds.max_deadline - ), - '0001-01-01 00:00:00+00'::timestamp with time zone + workspaces.id, + LEAST( + workspaces.last_used_at + (workspaces.ttl / 1000 / 1000 / 1000 || ' seconds')::interval, + workspace_builds.max_deadline ) AS deadline FROM - workspace_builds + workspaces LEFT JOIN - workspaces + workspace_builds ON - workspace_builds.workspace_id = workspaces.id; + workspace_builds.workspace_id = workspaces.id +WHERE + workspace_builds.build_number = ( + SELECT + MAX(build_number) + FROM + workspace_builds + WHERE + workspace_builds.workspace_id = workspaces.id + ); diff --git a/coderd/database/models.go b/coderd/database/models.go index c6dd3020d5407..775a44affc79f 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -2499,11 +2499,6 @@ type WorkspaceBuild struct { InitiatorByUsername string `db:"initiator_by_username" json:"initiator_by_username"` } -type WorkspaceBuildDeadline struct { - ID uuid.UUID `db:"id" json:"id"` - Deadline time.Time `db:"deadline" json:"deadline"` -} - type WorkspaceBuildParameter struct { WorkspaceBuildID uuid.UUID `db:"workspace_build_id" json:"workspace_build_id"` // Parameter name @@ -2529,6 +2524,11 @@ type WorkspaceBuildTable struct { MaxDeadline time.Time `db:"max_deadline" json:"max_deadline"` } +type WorkspaceDeadline struct { + ID uuid.UUID `db:"id" json:"id"` + Deadline time.Time `db:"deadline" json:"deadline"` +} + type WorkspaceProxy struct { ID uuid.UUID `db:"id" json:"id"` Name string `db:"name" json:"name"` diff --git a/coderd/database/sqlc.yaml b/coderd/database/sqlc.yaml index c2047a00d5587..1672ba0de7e6e 100644 --- a/coderd/database/sqlc.yaml +++ b/coderd/database/sqlc.yaml @@ -51,7 +51,7 @@ sql: - column: "template_usage_stats.app_usage_mins" go_type: type: "StringMapOfInt" - - column: "workspace_build_deadlines.deadline" + - column: "workspace_deadlines.deadline" go_type: type: "time.Time" rename: From c32f8111e9842836961d416395c50028d9fa9d61 Mon Sep 17 00:00:00 2001 From: Garrett Delfosse Date: Thu, 16 May 2024 18:06:45 +0000 Subject: [PATCH 5/7] remove query --- coderd/agentapi/stats_test.go | 14 ---- coderd/database/dbauthz/dbauthz.go | 7 -- coderd/database/dbauthz/dbauthz_test.go | 8 --- coderd/database/dbmem/dbmem.go | 88 ----------------------- coderd/database/dbmetrics/dbmetrics.go | 7 -- coderd/database/dbmock/dbmock.go | 14 ---- coderd/database/querier.go | 12 ---- coderd/database/queries.sql.go | 92 ------------------------ coderd/database/queries/activitybump.sql | 81 --------------------- coderd/workspaceagents.go | 5 -- 10 files changed, 328 deletions(-) delete mode 100644 coderd/database/queries/activitybump.sql diff --git a/coderd/agentapi/stats_test.go b/coderd/agentapi/stats_test.go index 943c8e7ac0e17..2797f25883ae6 100644 --- a/coderd/agentapi/stats_test.go +++ b/coderd/agentapi/stats_test.go @@ -155,12 +155,6 @@ func TestUpdateStates(t *testing.T) { TemplateName: template.Name, }, nil) - // We expect an activity bump because ConnectionCount > 0. - dbM.EXPECT().ActivityBumpWorkspace(gomock.Any(), database.ActivityBumpWorkspaceParams{ - WorkspaceID: workspace.ID, - NextAutostart: time.Time{}.UTC(), - }).Return(nil) - // Workspace last used at gets bumped. dbM.EXPECT().UpdateWorkspaceLastUsedAt(gomock.Any(), database.UpdateWorkspaceLastUsedAtParams{ ID: workspace.ID, @@ -307,7 +301,6 @@ func TestUpdateStates(t *testing.T) { now, err := time.Parse("2006-01-02 15:04:05 -0700 MST", "2023-12-19 07:30:00 +1100 AEDT") require.NoError(t, err) now = dbtime.Time(now) - nextAutostart := now.Add(30 * time.Minute).UTC() // always sent to DB as UTC var ( dbM = dbmock.NewMockStore(gomock.NewController(t)) @@ -369,13 +362,6 @@ func TestUpdateStates(t *testing.T) { TemplateName: template.Name, }, nil) - // We expect an activity bump because ConnectionCount > 0. However, the - // next autostart time will be set on the bump. - dbM.EXPECT().ActivityBumpWorkspace(gomock.Any(), database.ActivityBumpWorkspaceParams{ - WorkspaceID: workspace.ID, - NextAutostart: nextAutostart, - }).Return(nil) - // Workspace last used at gets bumped. dbM.EXPECT().UpdateWorkspaceLastUsedAt(gomock.Any(), database.UpdateWorkspaceLastUsedAtParams{ ID: workspace.ID, diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index a096346f57064..16e86755c6db0 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -716,13 +716,6 @@ func (q *querier) AcquireProvisionerJob(ctx context.Context, arg database.Acquir return q.db.AcquireProvisionerJob(ctx, arg) } -func (q *querier) ActivityBumpWorkspace(ctx context.Context, arg database.ActivityBumpWorkspaceParams) error { - fetch := func(ctx context.Context, arg database.ActivityBumpWorkspaceParams) (database.Workspace, error) { - return q.db.GetWorkspaceByID(ctx, arg.WorkspaceID) - } - return update(q.log, q.auth, fetch, q.db.ActivityBumpWorkspace)(ctx, arg) -} - func (q *querier) AllUserIDs(ctx context.Context) ([]uuid.UUID, error) { // Although this technically only reads users, only system-related functions should be // allowed to call this. diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index e8dcb2f8ee5bc..d2345cd503485 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -1599,14 +1599,6 @@ func (s *MethodTestSuite) TestWorkspace() { app := dbgen.WorkspaceApp(s.T(), db, database.WorkspaceApp{AgentID: agt.ID}) check.Args(app.ID).Asserts(ws, policy.ActionRead).Returns(ws) })) - s.Run("ActivityBumpWorkspace", s.Subtest(func(db database.Store, check *expects) { - ws := dbgen.Workspace(s.T(), db, database.Workspace{}) - build := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID, JobID: uuid.New()}) - dbgen.ProvisionerJob(s.T(), db, nil, database.ProvisionerJob{ID: build.JobID, Type: database.ProvisionerJobTypeWorkspaceBuild}) - check.Args(database.ActivityBumpWorkspaceParams{ - WorkspaceID: ws.ID, - }).Asserts(ws, policy.ActionUpdate).Returns() - })) s.Run("FavoriteWorkspace", s.Subtest(func(db database.Store, check *expects) { u := dbgen.User(s.T(), db, database.User{}) ws := dbgen.Workspace(s.T(), db, database.Workspace{OwnerID: u.ID}) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 8a2ce25b34367..265fdc59c01d8 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -957,94 +957,6 @@ func (q *FakeQuerier) AcquireProvisionerJob(_ context.Context, arg database.Acqu return database.ProvisionerJob{}, sql.ErrNoRows } -func (q *FakeQuerier) ActivityBumpWorkspace(ctx context.Context, arg database.ActivityBumpWorkspaceParams) error { - err := validateDatabaseType(arg) - if err != nil { - return err - } - - q.mutex.Lock() - defer q.mutex.Unlock() - - workspace, err := q.getWorkspaceByIDNoLock(ctx, arg.WorkspaceID) - if err != nil { - return err - } - latestBuild, err := q.getLatestWorkspaceBuildByWorkspaceIDNoLock(ctx, arg.WorkspaceID) - if err != nil { - return err - } - - now := dbtime.Now() - for i := range q.workspaceBuilds { - if q.workspaceBuilds[i].BuildNumber != latestBuild.BuildNumber { - continue - } - // If the build is not active, do not bump. - if q.workspaceBuilds[i].Transition != database.WorkspaceTransitionStart { - return nil - } - // If the provisioner job is not completed, do not bump. - pj, err := q.getProvisionerJobByIDNoLock(ctx, q.workspaceBuilds[i].JobID) - if err != nil { - return err - } - if !pj.CompletedAt.Valid { - return nil - } - // Do not bump if the deadline is not set. - if q.workspaceBuilds[i].Deadline.IsZero() { - return nil - } - - // Check the template default TTL. - template, err := q.getTemplateByIDNoLock(ctx, workspace.TemplateID) - if err != nil { - return err - } - if template.ActivityBump == 0 { - return nil - } - activityBump := time.Duration(template.ActivityBump) - - var ttlDur time.Duration - if now.Add(activityBump).After(arg.NextAutostart) && arg.NextAutostart.After(now) { - // Extend to TTL (NOT activity bump) - add := arg.NextAutostart.Sub(now) - if workspace.Ttl.Valid && template.AllowUserAutostop { - add += time.Duration(workspace.Ttl.Int64) - } else { - add += time.Duration(template.DefaultTTL) - } - ttlDur = add - } else { - // Otherwise, default to regular activity bump duration. - ttlDur = activityBump - } - - // Only bump if 5% of the deadline has passed. - ttlDur95 := ttlDur - (ttlDur / 20) - minBumpDeadline := q.workspaceBuilds[i].Deadline.Add(-ttlDur95) - if now.Before(minBumpDeadline) { - return nil - } - - // Bump. - newDeadline := now.Add(ttlDur) - // Never decrease deadlines from a bump - newDeadline = maxTime(newDeadline, q.workspaceBuilds[i].Deadline) - q.workspaceBuilds[i].UpdatedAt = now - if !q.workspaceBuilds[i].MaxDeadline.IsZero() { - q.workspaceBuilds[i].Deadline = minTime(newDeadline, q.workspaceBuilds[i].MaxDeadline) - } else { - q.workspaceBuilds[i].Deadline = newDeadline - } - return nil - } - - return sql.ErrNoRows -} - func (q *FakeQuerier) AllUserIDs(_ context.Context) ([]uuid.UUID, error) { q.mutex.RLock() defer q.mutex.RUnlock() diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go index 77ebfd6718757..3407510a45af7 100644 --- a/coderd/database/dbmetrics/dbmetrics.go +++ b/coderd/database/dbmetrics/dbmetrics.go @@ -95,13 +95,6 @@ func (m metricsStore) AcquireProvisionerJob(ctx context.Context, arg database.Ac return provisionerJob, err } -func (m metricsStore) ActivityBumpWorkspace(ctx context.Context, arg database.ActivityBumpWorkspaceParams) error { - start := time.Now() - r0 := m.s.ActivityBumpWorkspace(ctx, arg) - m.queryLatencies.WithLabelValues("ActivityBumpWorkspace").Observe(time.Since(start).Seconds()) - return r0 -} - func (m metricsStore) AllUserIDs(ctx context.Context) ([]uuid.UUID, error) { start := time.Now() r0, r1 := m.s.AllUserIDs(ctx) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index e651c8301c933..34a1415c92cae 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -73,20 +73,6 @@ func (mr *MockStoreMockRecorder) AcquireProvisionerJob(arg0, arg1 any) *gomock.C return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AcquireProvisionerJob", reflect.TypeOf((*MockStore)(nil).AcquireProvisionerJob), arg0, arg1) } -// ActivityBumpWorkspace mocks base method. -func (m *MockStore) ActivityBumpWorkspace(arg0 context.Context, arg1 database.ActivityBumpWorkspaceParams) error { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ActivityBumpWorkspace", arg0, arg1) - ret0, _ := ret[0].(error) - return ret0 -} - -// ActivityBumpWorkspace indicates an expected call of ActivityBumpWorkspace. -func (mr *MockStoreMockRecorder) ActivityBumpWorkspace(arg0, arg1 any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ActivityBumpWorkspace", reflect.TypeOf((*MockStore)(nil).ActivityBumpWorkspace), arg0, arg1) -} - // AllUserIDs mocks base method. func (m *MockStore) AllUserIDs(arg0 context.Context) ([]uuid.UUID, error) { m.ctrl.T.Helper() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 405f86bf47688..d459e5256cc29 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -24,18 +24,6 @@ type sqlcQuerier interface { // multiple provisioners from acquiring the same jobs. See: // https://www.postgresql.org/docs/9.5/sql-select.html#SQL-FOR-UPDATE-SHARE AcquireProvisionerJob(ctx context.Context, arg AcquireProvisionerJobParams) (ProvisionerJob, error) - // Bumps the workspace deadline by the template's configured "activity_bump" - // duration (default 1h). If the workspace bump will cross an autostart - // threshold, then the bump is autostart + TTL. This is the deadline behavior if - // the workspace was to autostart from a stopped state. - // - // Max deadline is respected, and the deadline will never be bumped past it. - // The deadline will never decrease. - // We only bump if the template has an activity bump duration set. - // We only bump if the raw interval is positive and non-zero. - // We only bump if workspace shutdown is manual. - // We only bump when 5% of the deadline has elapsed. - ActivityBumpWorkspace(ctx context.Context, arg ActivityBumpWorkspaceParams) error // AllUserIDs returns all UserIDs regardless of user status or deletion. AllUserIDs(ctx context.Context) ([]uuid.UUID, error) // Archiving templates is a soft delete action, so is reversible. diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index e0fba2dad35bd..a6ae225581e81 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -15,98 +15,6 @@ import ( "github.com/sqlc-dev/pqtype" ) -const activityBumpWorkspace = `-- name: ActivityBumpWorkspace :exec -WITH latest AS ( - SELECT - workspace_builds.id::uuid AS build_id, - workspace_builds.deadline::timestamp with time zone AS build_deadline, - workspace_builds.max_deadline::timestamp with time zone AS build_max_deadline, - workspace_builds.transition AS build_transition, - provisioner_jobs.completed_at::timestamp with time zone AS job_completed_at, - templates.activity_bump AS activity_bump, - ( - CASE - -- If the extension would push us over the next_autostart - -- interval, then extend the deadline by the full TTL (NOT - -- activity bump) from the autostart time. This will essentially - -- be as if the workspace auto started at the given time and the - -- original TTL was applied. - -- - -- Sadly we can't define ` + "`" + `activity_bump_interval` + "`" + ` above since - -- it won't be available for this CASE statement, so we have to - -- copy the cast twice. - WHEN NOW() + (templates.activity_bump / 1000 / 1000 / 1000 || ' seconds')::interval > $1 :: timestamptz - -- If the autostart is behind now(), then the - -- autostart schedule is either the 0 time and not provided, - -- or it was the autostart in the past, which is no longer - -- relevant. If autostart is > 0 and in the past, then - -- that is a mistake by the caller. - AND $1 > NOW() - THEN - -- Extend to the autostart, then add the activity bump - (($1 :: timestamptz) - NOW()) + CASE - WHEN templates.allow_user_autostop - THEN (workspaces.ttl / 1000 / 1000 / 1000 || ' seconds')::interval - ELSE (templates.default_ttl / 1000 / 1000 / 1000 || ' seconds')::interval - END - - -- Default to the activity bump duration. - ELSE - (templates.activity_bump / 1000 / 1000 / 1000 || ' seconds')::interval - END - ) AS ttl_interval - FROM workspace_builds - JOIN provisioner_jobs - ON provisioner_jobs.id = workspace_builds.job_id - JOIN workspaces - ON workspaces.id = workspace_builds.workspace_id - JOIN templates - ON templates.id = workspaces.template_id - WHERE workspace_builds.workspace_id = $2::uuid - ORDER BY workspace_builds.build_number DESC - LIMIT 1 -) -UPDATE - workspace_builds wb -SET - updated_at = NOW(), - deadline = CASE - WHEN l.build_max_deadline = '0001-01-01 00:00:00+00' - -- Never reduce the deadline from activity. - THEN GREATEST(wb.deadline, NOW() + l.ttl_interval) - ELSE LEAST(GREATEST(wb.deadline, NOW() + l.ttl_interval), l.build_max_deadline) - END -FROM latest l -WHERE wb.id = l.build_id -AND l.job_completed_at IS NOT NULL -AND l.activity_bump > 0 -AND l.build_transition = 'start' -AND l.ttl_interval > '0 seconds'::interval -AND l.build_deadline != '0001-01-01 00:00:00+00' -AND l.build_deadline - (l.ttl_interval * 0.95) < NOW() -` - -type ActivityBumpWorkspaceParams struct { - NextAutostart time.Time `db:"next_autostart" json:"next_autostart"` - WorkspaceID uuid.UUID `db:"workspace_id" json:"workspace_id"` -} - -// Bumps the workspace deadline by the template's configured "activity_bump" -// duration (default 1h). If the workspace bump will cross an autostart -// threshold, then the bump is autostart + TTL. This is the deadline behavior if -// the workspace was to autostart from a stopped state. -// -// Max deadline is respected, and the deadline will never be bumped past it. -// The deadline will never decrease. -// We only bump if the template has an activity bump duration set. -// We only bump if the raw interval is positive and non-zero. -// We only bump if workspace shutdown is manual. -// We only bump when 5% of the deadline has elapsed. -func (q *sqlQuerier) ActivityBumpWorkspace(ctx context.Context, arg ActivityBumpWorkspaceParams) error { - _, err := q.db.ExecContext(ctx, activityBumpWorkspace, arg.NextAutostart, arg.WorkspaceID) - return err -} - const deleteAPIKeyByID = `-- name: DeleteAPIKeyByID :exec DELETE FROM api_keys diff --git a/coderd/database/queries/activitybump.sql b/coderd/database/queries/activitybump.sql deleted file mode 100644 index 09349d29e5d06..0000000000000 --- a/coderd/database/queries/activitybump.sql +++ /dev/null @@ -1,81 +0,0 @@ --- Bumps the workspace deadline by the template's configured "activity_bump" --- duration (default 1h). If the workspace bump will cross an autostart --- threshold, then the bump is autostart + TTL. This is the deadline behavior if --- the workspace was to autostart from a stopped state. --- --- Max deadline is respected, and the deadline will never be bumped past it. --- The deadline will never decrease. --- name: ActivityBumpWorkspace :exec -WITH latest AS ( - SELECT - workspace_builds.id::uuid AS build_id, - workspace_builds.deadline::timestamp with time zone AS build_deadline, - workspace_builds.max_deadline::timestamp with time zone AS build_max_deadline, - workspace_builds.transition AS build_transition, - provisioner_jobs.completed_at::timestamp with time zone AS job_completed_at, - templates.activity_bump AS activity_bump, - ( - CASE - -- If the extension would push us over the next_autostart - -- interval, then extend the deadline by the full TTL (NOT - -- activity bump) from the autostart time. This will essentially - -- be as if the workspace auto started at the given time and the - -- original TTL was applied. - -- - -- Sadly we can't define `activity_bump_interval` above since - -- it won't be available for this CASE statement, so we have to - -- copy the cast twice. - WHEN NOW() + (templates.activity_bump / 1000 / 1000 / 1000 || ' seconds')::interval > @next_autostart :: timestamptz - -- If the autostart is behind now(), then the - -- autostart schedule is either the 0 time and not provided, - -- or it was the autostart in the past, which is no longer - -- relevant. If autostart is > 0 and in the past, then - -- that is a mistake by the caller. - AND @next_autostart > NOW() - THEN - -- Extend to the autostart, then add the activity bump - ((@next_autostart :: timestamptz) - NOW()) + CASE - WHEN templates.allow_user_autostop - THEN (workspaces.ttl / 1000 / 1000 / 1000 || ' seconds')::interval - ELSE (templates.default_ttl / 1000 / 1000 / 1000 || ' seconds')::interval - END - - -- Default to the activity bump duration. - ELSE - (templates.activity_bump / 1000 / 1000 / 1000 || ' seconds')::interval - END - ) AS ttl_interval - FROM workspace_builds - JOIN provisioner_jobs - ON provisioner_jobs.id = workspace_builds.job_id - JOIN workspaces - ON workspaces.id = workspace_builds.workspace_id - JOIN templates - ON templates.id = workspaces.template_id - WHERE workspace_builds.workspace_id = @workspace_id::uuid - ORDER BY workspace_builds.build_number DESC - LIMIT 1 -) -UPDATE - workspace_builds wb -SET - updated_at = NOW(), - deadline = CASE - WHEN l.build_max_deadline = '0001-01-01 00:00:00+00' - -- Never reduce the deadline from activity. - THEN GREATEST(wb.deadline, NOW() + l.ttl_interval) - ELSE LEAST(GREATEST(wb.deadline, NOW() + l.ttl_interval), l.build_max_deadline) - END -FROM latest l -WHERE wb.id = l.build_id -AND l.job_completed_at IS NOT NULL --- We only bump if the template has an activity bump duration set. -AND l.activity_bump > 0 -AND l.build_transition = 'start' --- We only bump if the raw interval is positive and non-zero. -AND l.ttl_interval > '0 seconds'::interval --- We only bump if workspace shutdown is manual. -AND l.build_deadline != '0001-01-01 00:00:00+00' --- We only bump when 5% of the deadline has elapsed. -AND l.build_deadline - (l.ttl_interval * 0.95) < NOW() -; diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go index 68798d4144700..21b2525a08f2e 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -1166,11 +1166,6 @@ func (api *API) workspaceAgentReportStats(rw http.ResponseWriter, r *http.Reques slog.F("payload", req), ) - if req.ConnectionCount > 0 { - // do we still need to bump something here? - // or is it handled below with the req.SessionCount() > 0 check? - } - now := dbtime.Now() protoStats := &agentproto.Stats{ ConnectionsByProto: req.ConnectionsByProto, From 6dc9508a2efe3ad325721955f75afb3324169c2c Mon Sep 17 00:00:00 2001 From: Garrett Delfosse Date: Thu, 16 May 2024 19:13:31 +0000 Subject: [PATCH 6/7] remove deadline usage from queries --- coderd/activitybump_test.go | 6 +- coderd/autobuild/lifecycle_executor.go | 4 +- coderd/database/dbauthz/dbauthz_test.go | 1 - coderd/database/dbgen/dbgen.go | 1 - coderd/database/dbmem/dbmem.go | 6 +- coderd/database/dump.sql | 4 +- .../000209_workspace_deadline_view.up.sql | 20 ++++++ coderd/database/models.go | 30 ++++----- coderd/database/queries.sql.go | 66 ++++++++----------- coderd/database/queries/workspacebuilds.sql | 4 +- coderd/database/queries/workspaces.sql | 6 +- .../provisionerdserver/provisionerdserver.go | 2 - .../provisionerdserver_test.go | 6 -- coderd/wsbuilder/wsbuilder.go | 1 - enterprise/coderd/schedule/template.go | 8 --- enterprise/coderd/schedule/template_test.go | 9 --- 16 files changed, 77 insertions(+), 97 deletions(-) diff --git a/coderd/activitybump_test.go b/coderd/activitybump_test.go index 20c17b8d27762..a602065e3e2ba 100644 --- a/coderd/activitybump_test.go +++ b/coderd/activitybump_test.go @@ -75,10 +75,8 @@ func TestWorkspaceActivityBump(t *testing.T) { } err := db.UpdateWorkspaceBuildDeadlineByID(ctx, database.UpdateWorkspaceBuildDeadlineByIDParams{ - ID: workspace.LatestBuild.ID, - UpdatedAt: dbtime.Now(), - // Make the deadline really close so it needs to be bumped immediately. - Deadline: dbtime.Now().Add(time.Minute), + ID: workspace.LatestBuild.ID, + UpdatedAt: dbtime.Now(), MaxDeadline: maxDeadline, }) require.NoError(t, err) diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index e0d804328b2d3..f7a8d260d5b2c 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -389,9 +389,9 @@ func isEligibleForAutostop(ws database.Workspace, build database.WorkspaceBuild, // A workspace must be started in order for it to be auto-stopped. return build.Transition == database.WorkspaceTransitionStart && - !build.Deadline.IsZero() && + !build.MaxDeadline.IsZero() && // We do not want to stop a workspace prior to it breaching its deadline. - !currentTick.Before(build.Deadline) + !currentTick.Before(build.MaxDeadline) } // isEligibleForDormantStop returns true if the workspace should be dormant diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index d2345cd503485..a287b450282d3 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -1557,7 +1557,6 @@ func (s *MethodTestSuite) TestWorkspace() { check.Args(database.UpdateWorkspaceBuildDeadlineByIDParams{ ID: build.ID, UpdatedAt: build.UpdatedAt, - Deadline: build.Deadline, }).Asserts(ws, policy.ActionUpdate) })) s.Run("SoftDeleteWorkspaceByID", s.Subtest(func(db database.Store, check *expects) { diff --git a/coderd/database/dbgen/dbgen.go b/coderd/database/dbgen/dbgen.go index fe660e7e8fa93..261cf55ae2190 100644 --- a/coderd/database/dbgen/dbgen.go +++ b/coderd/database/dbgen/dbgen.go @@ -233,7 +233,6 @@ func WorkspaceBuild(t testing.TB, db database.Store, orig database.WorkspaceBuil InitiatorID: takeFirst(orig.InitiatorID, uuid.New()), JobID: takeFirst(orig.JobID, uuid.New()), ProvisionerState: takeFirstSlice(orig.ProvisionerState, []byte{}), - Deadline: takeFirst(orig.Deadline, dbtime.Now().Add(time.Hour)), MaxDeadline: takeFirst(orig.MaxDeadline, time.Time{}), Reason: takeFirst(orig.Reason, database.BuildReasonInitiator), }) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 265fdc59c01d8..0be79d79ab71c 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -5500,8 +5500,8 @@ func (q *FakeQuerier) GetWorkspacesEligibleForTransition(ctx context.Context, no } if build.Transition == database.WorkspaceTransitionStart && - !build.Deadline.IsZero() && - build.Deadline.Before(now) && + !build.MaxDeadline.IsZero() && + build.MaxDeadline.Before(now) && !workspace.DormantAt.Valid { workspaces = append(workspaces, workspace) continue @@ -6580,7 +6580,6 @@ func (q *FakeQuerier) InsertWorkspaceBuild(_ context.Context, arg database.Inser InitiatorID: arg.InitiatorID, JobID: arg.JobID, ProvisionerState: arg.ProvisionerState, - Deadline: arg.Deadline, MaxDeadline: arg.MaxDeadline, Reason: arg.Reason, } @@ -7876,7 +7875,6 @@ func (q *FakeQuerier) UpdateWorkspaceBuildDeadlineByID(_ context.Context, arg da if build.ID != arg.ID { continue } - build.Deadline = arg.Deadline build.MaxDeadline = arg.MaxDeadline build.UpdatedAt = arg.UpdatedAt q.workspaceBuilds[idx] = build diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 63143e78fadb1..6225257bda308 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -1260,7 +1260,7 @@ CREATE TABLE workspace_builds ( initiator_id uuid NOT NULL, provisioner_state bytea, job_id uuid NOT NULL, - deadline timestamp with time zone DEFAULT '0001-01-01 00:00:00+00'::timestamp with time zone NOT NULL, + deadline_deprecated timestamp with time zone DEFAULT '0001-01-01 00:00:00+00'::timestamp with time zone NOT NULL, reason build_reason DEFAULT 'initiator'::build_reason NOT NULL, daily_cost integer DEFAULT 0 NOT NULL, max_deadline timestamp with time zone DEFAULT '0001-01-01 00:00:00+00'::timestamp with time zone NOT NULL @@ -1277,7 +1277,7 @@ CREATE VIEW workspace_build_with_user AS workspace_builds.initiator_id, workspace_builds.provisioner_state, workspace_builds.job_id, - workspace_builds.deadline, + workspace_builds.deadline_deprecated, workspace_builds.reason, workspace_builds.daily_cost, workspace_builds.max_deadline, diff --git a/coderd/database/migrations/000209_workspace_deadline_view.up.sql b/coderd/database/migrations/000209_workspace_deadline_view.up.sql index 4fa2aeff2cebb..f102759d51c54 100644 --- a/coderd/database/migrations/000209_workspace_deadline_view.up.sql +++ b/coderd/database/migrations/000209_workspace_deadline_view.up.sql @@ -20,3 +20,23 @@ WHERE WHERE workspace_builds.workspace_id = workspaces.id ); + +ALTER TABLE workspace_builds RENAME COLUMN deadline TO deadline_deprecated; + +DROP VIEW workspace_build_with_user; + +CREATE VIEW + workspace_build_with_user +AS +SELECT + workspace_builds.*, + coalesce(visible_users.avatar_url, '') AS initiator_by_avatar_url, + coalesce(visible_users.username, '') AS initiator_by_username +FROM + workspace_builds + LEFT JOIN + visible_users + ON + workspace_builds.initiator_id = visible_users.id; + +COMMENT ON VIEW workspace_build_with_user IS 'Joins in the username + avatar url of the initiated by user.'; diff --git a/coderd/database/models.go b/coderd/database/models.go index 775a44affc79f..17301717107bb 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -2491,7 +2491,7 @@ type WorkspaceBuild struct { InitiatorID uuid.UUID `db:"initiator_id" json:"initiator_id"` ProvisionerState []byte `db:"provisioner_state" json:"provisioner_state"` JobID uuid.UUID `db:"job_id" json:"job_id"` - Deadline time.Time `db:"deadline" json:"deadline"` + DeadlineDeprecated time.Time `db:"deadline_deprecated" json:"deadline_deprecated"` Reason BuildReason `db:"reason" json:"reason"` DailyCost int32 `db:"daily_cost" json:"daily_cost"` MaxDeadline time.Time `db:"max_deadline" json:"max_deadline"` @@ -2508,20 +2508,20 @@ type WorkspaceBuildParameter struct { } type WorkspaceBuildTable 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"` - WorkspaceID uuid.UUID `db:"workspace_id" json:"workspace_id"` - TemplateVersionID uuid.UUID `db:"template_version_id" json:"template_version_id"` - BuildNumber int32 `db:"build_number" json:"build_number"` - Transition WorkspaceTransition `db:"transition" json:"transition"` - InitiatorID uuid.UUID `db:"initiator_id" json:"initiator_id"` - ProvisionerState []byte `db:"provisioner_state" json:"provisioner_state"` - JobID uuid.UUID `db:"job_id" json:"job_id"` - Deadline time.Time `db:"deadline" json:"deadline"` - Reason BuildReason `db:"reason" json:"reason"` - DailyCost int32 `db:"daily_cost" json:"daily_cost"` - MaxDeadline time.Time `db:"max_deadline" json:"max_deadline"` + 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"` + WorkspaceID uuid.UUID `db:"workspace_id" json:"workspace_id"` + TemplateVersionID uuid.UUID `db:"template_version_id" json:"template_version_id"` + BuildNumber int32 `db:"build_number" json:"build_number"` + Transition WorkspaceTransition `db:"transition" json:"transition"` + InitiatorID uuid.UUID `db:"initiator_id" json:"initiator_id"` + ProvisionerState []byte `db:"provisioner_state" json:"provisioner_state"` + JobID uuid.UUID `db:"job_id" json:"job_id"` + DeadlineDeprecated time.Time `db:"deadline_deprecated" json:"deadline_deprecated"` + Reason BuildReason `db:"reason" json:"reason"` + DailyCost int32 `db:"daily_cost" json:"daily_cost"` + MaxDeadline time.Time `db:"max_deadline" json:"max_deadline"` } type WorkspaceDeadline struct { diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index a6ae225581e81..57734408bb50d 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -9048,7 +9048,7 @@ const getWorkspaceAgentAndLatestBuildByAuthToken = `-- name: GetWorkspaceAgentAn SELECT workspaces.id, workspaces.created_at, workspaces.updated_at, workspaces.owner_id, workspaces.organization_id, workspaces.template_id, workspaces.deleted, workspaces.name, workspaces.autostart_schedule, workspaces.ttl, workspaces.last_used_at, workspaces.dormant_at, workspaces.deleting_at, workspaces.automatic_updates, workspaces.favorite, workspace_agents.id, workspace_agents.created_at, workspace_agents.updated_at, workspace_agents.name, workspace_agents.first_connected_at, workspace_agents.last_connected_at, workspace_agents.disconnected_at, workspace_agents.resource_id, workspace_agents.auth_token, workspace_agents.auth_instance_id, workspace_agents.architecture, workspace_agents.environment_variables, workspace_agents.operating_system, workspace_agents.instance_metadata, workspace_agents.resource_metadata, workspace_agents.directory, workspace_agents.version, workspace_agents.last_connected_replica_id, workspace_agents.connection_timeout_seconds, workspace_agents.troubleshooting_url, workspace_agents.motd_file, workspace_agents.lifecycle_state, workspace_agents.expanded_directory, workspace_agents.logs_length, workspace_agents.logs_overflowed, workspace_agents.started_at, workspace_agents.ready_at, workspace_agents.subsystems, workspace_agents.display_apps, workspace_agents.api_version, workspace_agents.display_order, - workspace_build_with_user.id, workspace_build_with_user.created_at, workspace_build_with_user.updated_at, workspace_build_with_user.workspace_id, workspace_build_with_user.template_version_id, workspace_build_with_user.build_number, workspace_build_with_user.transition, workspace_build_with_user.initiator_id, workspace_build_with_user.provisioner_state, workspace_build_with_user.job_id, workspace_build_with_user.deadline, workspace_build_with_user.reason, workspace_build_with_user.daily_cost, workspace_build_with_user.max_deadline, workspace_build_with_user.initiator_by_avatar_url, workspace_build_with_user.initiator_by_username + workspace_build_with_user.id, workspace_build_with_user.created_at, workspace_build_with_user.updated_at, workspace_build_with_user.workspace_id, workspace_build_with_user.template_version_id, workspace_build_with_user.build_number, workspace_build_with_user.transition, workspace_build_with_user.initiator_id, workspace_build_with_user.provisioner_state, workspace_build_with_user.job_id, workspace_build_with_user.deadline_deprecated, workspace_build_with_user.reason, workspace_build_with_user.daily_cost, workspace_build_with_user.max_deadline, workspace_build_with_user.initiator_by_avatar_url, workspace_build_with_user.initiator_by_username FROM workspace_agents JOIN @@ -9146,7 +9146,7 @@ func (q *sqlQuerier) GetWorkspaceAgentAndLatestBuildByAuthToken(ctx context.Cont &i.WorkspaceBuild.InitiatorID, &i.WorkspaceBuild.ProvisionerState, &i.WorkspaceBuild.JobID, - &i.WorkspaceBuild.Deadline, + &i.WorkspaceBuild.DeadlineDeprecated, &i.WorkspaceBuild.Reason, &i.WorkspaceBuild.DailyCost, &i.WorkspaceBuild.MaxDeadline, @@ -10916,7 +10916,7 @@ func (q *sqlQuerier) InsertWorkspaceBuildParameters(ctx context.Context, arg Ins } const getActiveWorkspaceBuildsByTemplateID = `-- name: GetActiveWorkspaceBuildsByTemplateID :many -SELECT wb.id, wb.created_at, wb.updated_at, wb.workspace_id, wb.template_version_id, wb.build_number, wb.transition, wb.initiator_id, wb.provisioner_state, wb.job_id, wb.deadline, wb.reason, wb.daily_cost, wb.max_deadline, wb.initiator_by_avatar_url, wb.initiator_by_username +SELECT wb.id, wb.created_at, wb.updated_at, wb.workspace_id, wb.template_version_id, wb.build_number, wb.transition, wb.initiator_id, wb.provisioner_state, wb.job_id, wb.deadline_deprecated, wb.reason, wb.daily_cost, wb.max_deadline, wb.initiator_by_avatar_url, wb.initiator_by_username FROM ( SELECT workspace_id, MAX(build_number) as max_build_number @@ -10966,7 +10966,7 @@ func (q *sqlQuerier) GetActiveWorkspaceBuildsByTemplateID(ctx context.Context, t &i.InitiatorID, &i.ProvisionerState, &i.JobID, - &i.Deadline, + &i.DeadlineDeprecated, &i.Reason, &i.DailyCost, &i.MaxDeadline, @@ -10988,7 +10988,7 @@ func (q *sqlQuerier) GetActiveWorkspaceBuildsByTemplateID(ctx context.Context, t const getLatestWorkspaceBuildByWorkspaceID = `-- name: GetLatestWorkspaceBuildByWorkspaceID :one SELECT - id, created_at, updated_at, workspace_id, template_version_id, build_number, transition, initiator_id, provisioner_state, job_id, deadline, reason, daily_cost, max_deadline, initiator_by_avatar_url, initiator_by_username + id, created_at, updated_at, workspace_id, template_version_id, build_number, transition, initiator_id, provisioner_state, job_id, deadline_deprecated, reason, daily_cost, max_deadline, initiator_by_avatar_url, initiator_by_username FROM workspace_build_with_user AS workspace_builds WHERE @@ -11013,7 +11013,7 @@ func (q *sqlQuerier) GetLatestWorkspaceBuildByWorkspaceID(ctx context.Context, w &i.InitiatorID, &i.ProvisionerState, &i.JobID, - &i.Deadline, + &i.DeadlineDeprecated, &i.Reason, &i.DailyCost, &i.MaxDeadline, @@ -11024,7 +11024,7 @@ func (q *sqlQuerier) GetLatestWorkspaceBuildByWorkspaceID(ctx context.Context, w } const getLatestWorkspaceBuilds = `-- name: GetLatestWorkspaceBuilds :many -SELECT wb.id, wb.created_at, wb.updated_at, wb.workspace_id, wb.template_version_id, wb.build_number, wb.transition, wb.initiator_id, wb.provisioner_state, wb.job_id, wb.deadline, wb.reason, wb.daily_cost, wb.max_deadline, wb.initiator_by_avatar_url, wb.initiator_by_username +SELECT wb.id, wb.created_at, wb.updated_at, wb.workspace_id, wb.template_version_id, wb.build_number, wb.transition, wb.initiator_id, wb.provisioner_state, wb.job_id, wb.deadline_deprecated, wb.reason, wb.daily_cost, wb.max_deadline, wb.initiator_by_avatar_url, wb.initiator_by_username FROM ( SELECT workspace_id, MAX(build_number) as max_build_number @@ -11058,7 +11058,7 @@ func (q *sqlQuerier) GetLatestWorkspaceBuilds(ctx context.Context) ([]WorkspaceB &i.InitiatorID, &i.ProvisionerState, &i.JobID, - &i.Deadline, + &i.DeadlineDeprecated, &i.Reason, &i.DailyCost, &i.MaxDeadline, @@ -11079,7 +11079,7 @@ func (q *sqlQuerier) GetLatestWorkspaceBuilds(ctx context.Context) ([]WorkspaceB } const getLatestWorkspaceBuildsByWorkspaceIDs = `-- name: GetLatestWorkspaceBuildsByWorkspaceIDs :many -SELECT wb.id, wb.created_at, wb.updated_at, wb.workspace_id, wb.template_version_id, wb.build_number, wb.transition, wb.initiator_id, wb.provisioner_state, wb.job_id, wb.deadline, wb.reason, wb.daily_cost, wb.max_deadline, wb.initiator_by_avatar_url, wb.initiator_by_username +SELECT wb.id, wb.created_at, wb.updated_at, wb.workspace_id, wb.template_version_id, wb.build_number, wb.transition, wb.initiator_id, wb.provisioner_state, wb.job_id, wb.deadline_deprecated, wb.reason, wb.daily_cost, wb.max_deadline, wb.initiator_by_avatar_url, wb.initiator_by_username FROM ( SELECT workspace_id, MAX(build_number) as max_build_number @@ -11115,7 +11115,7 @@ func (q *sqlQuerier) GetLatestWorkspaceBuildsByWorkspaceIDs(ctx context.Context, &i.InitiatorID, &i.ProvisionerState, &i.JobID, - &i.Deadline, + &i.DeadlineDeprecated, &i.Reason, &i.DailyCost, &i.MaxDeadline, @@ -11137,7 +11137,7 @@ func (q *sqlQuerier) GetLatestWorkspaceBuildsByWorkspaceIDs(ctx context.Context, const getWorkspaceBuildByID = `-- name: GetWorkspaceBuildByID :one SELECT - id, created_at, updated_at, workspace_id, template_version_id, build_number, transition, initiator_id, provisioner_state, job_id, deadline, reason, daily_cost, max_deadline, initiator_by_avatar_url, initiator_by_username + id, created_at, updated_at, workspace_id, template_version_id, build_number, transition, initiator_id, provisioner_state, job_id, deadline_deprecated, reason, daily_cost, max_deadline, initiator_by_avatar_url, initiator_by_username FROM workspace_build_with_user AS workspace_builds WHERE @@ -11160,7 +11160,7 @@ func (q *sqlQuerier) GetWorkspaceBuildByID(ctx context.Context, id uuid.UUID) (W &i.InitiatorID, &i.ProvisionerState, &i.JobID, - &i.Deadline, + &i.DeadlineDeprecated, &i.Reason, &i.DailyCost, &i.MaxDeadline, @@ -11172,7 +11172,7 @@ func (q *sqlQuerier) GetWorkspaceBuildByID(ctx context.Context, id uuid.UUID) (W const getWorkspaceBuildByJobID = `-- name: GetWorkspaceBuildByJobID :one SELECT - id, created_at, updated_at, workspace_id, template_version_id, build_number, transition, initiator_id, provisioner_state, job_id, deadline, reason, daily_cost, max_deadline, initiator_by_avatar_url, initiator_by_username + id, created_at, updated_at, workspace_id, template_version_id, build_number, transition, initiator_id, provisioner_state, job_id, deadline_deprecated, reason, daily_cost, max_deadline, initiator_by_avatar_url, initiator_by_username FROM workspace_build_with_user AS workspace_builds WHERE @@ -11195,7 +11195,7 @@ func (q *sqlQuerier) GetWorkspaceBuildByJobID(ctx context.Context, jobID uuid.UU &i.InitiatorID, &i.ProvisionerState, &i.JobID, - &i.Deadline, + &i.DeadlineDeprecated, &i.Reason, &i.DailyCost, &i.MaxDeadline, @@ -11207,7 +11207,7 @@ func (q *sqlQuerier) GetWorkspaceBuildByJobID(ctx context.Context, jobID uuid.UU const getWorkspaceBuildByWorkspaceIDAndBuildNumber = `-- name: GetWorkspaceBuildByWorkspaceIDAndBuildNumber :one SELECT - id, created_at, updated_at, workspace_id, template_version_id, build_number, transition, initiator_id, provisioner_state, job_id, deadline, reason, daily_cost, max_deadline, initiator_by_avatar_url, initiator_by_username + id, created_at, updated_at, workspace_id, template_version_id, build_number, transition, initiator_id, provisioner_state, job_id, deadline_deprecated, reason, daily_cost, max_deadline, initiator_by_avatar_url, initiator_by_username FROM workspace_build_with_user AS workspace_builds WHERE @@ -11234,7 +11234,7 @@ func (q *sqlQuerier) GetWorkspaceBuildByWorkspaceIDAndBuildNumber(ctx context.Co &i.InitiatorID, &i.ProvisionerState, &i.JobID, - &i.Deadline, + &i.DeadlineDeprecated, &i.Reason, &i.DailyCost, &i.MaxDeadline, @@ -11246,7 +11246,7 @@ func (q *sqlQuerier) GetWorkspaceBuildByWorkspaceIDAndBuildNumber(ctx context.Co const getWorkspaceBuildsByWorkspaceID = `-- name: GetWorkspaceBuildsByWorkspaceID :many SELECT - id, created_at, updated_at, workspace_id, template_version_id, build_number, transition, initiator_id, provisioner_state, job_id, deadline, reason, daily_cost, max_deadline, initiator_by_avatar_url, initiator_by_username + id, created_at, updated_at, workspace_id, template_version_id, build_number, transition, initiator_id, provisioner_state, job_id, deadline_deprecated, reason, daily_cost, max_deadline, initiator_by_avatar_url, initiator_by_username FROM workspace_build_with_user AS workspace_builds WHERE @@ -11312,7 +11312,7 @@ func (q *sqlQuerier) GetWorkspaceBuildsByWorkspaceID(ctx context.Context, arg Ge &i.InitiatorID, &i.ProvisionerState, &i.JobID, - &i.Deadline, + &i.DeadlineDeprecated, &i.Reason, &i.DailyCost, &i.MaxDeadline, @@ -11333,7 +11333,7 @@ func (q *sqlQuerier) GetWorkspaceBuildsByWorkspaceID(ctx context.Context, arg Ge } const getWorkspaceBuildsCreatedAfter = `-- name: GetWorkspaceBuildsCreatedAfter :many -SELECT id, created_at, updated_at, workspace_id, template_version_id, build_number, transition, initiator_id, provisioner_state, job_id, deadline, reason, daily_cost, max_deadline, initiator_by_avatar_url, initiator_by_username FROM workspace_build_with_user WHERE created_at > $1 +SELECT id, created_at, updated_at, workspace_id, template_version_id, build_number, transition, initiator_id, provisioner_state, job_id, deadline_deprecated, reason, daily_cost, max_deadline, initiator_by_avatar_url, initiator_by_username FROM workspace_build_with_user WHERE created_at > $1 ` func (q *sqlQuerier) GetWorkspaceBuildsCreatedAfter(ctx context.Context, createdAt time.Time) ([]WorkspaceBuild, error) { @@ -11356,7 +11356,7 @@ func (q *sqlQuerier) GetWorkspaceBuildsCreatedAfter(ctx context.Context, created &i.InitiatorID, &i.ProvisionerState, &i.JobID, - &i.Deadline, + &i.DeadlineDeprecated, &i.Reason, &i.DailyCost, &i.MaxDeadline, @@ -11389,12 +11389,11 @@ INSERT INTO initiator_id, job_id, provisioner_state, - deadline, max_deadline, reason ) VALUES - ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13) + ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12) ` type InsertWorkspaceBuildParams struct { @@ -11408,7 +11407,6 @@ type InsertWorkspaceBuildParams struct { InitiatorID uuid.UUID `db:"initiator_id" json:"initiator_id"` JobID uuid.UUID `db:"job_id" json:"job_id"` ProvisionerState []byte `db:"provisioner_state" json:"provisioner_state"` - Deadline time.Time `db:"deadline" json:"deadline"` MaxDeadline time.Time `db:"max_deadline" json:"max_deadline"` Reason BuildReason `db:"reason" json:"reason"` } @@ -11425,7 +11423,6 @@ func (q *sqlQuerier) InsertWorkspaceBuild(ctx context.Context, arg InsertWorkspa arg.InitiatorID, arg.JobID, arg.ProvisionerState, - arg.Deadline, arg.MaxDeadline, arg.Reason, ) @@ -11455,26 +11452,19 @@ const updateWorkspaceBuildDeadlineByID = `-- name: UpdateWorkspaceBuildDeadlineB UPDATE workspace_builds SET - deadline = $1::timestamptz, - max_deadline = $2::timestamptz, - updated_at = $3::timestamptz -WHERE id = $4::uuid + max_deadline = $1::timestamptz, + updated_at = $2::timestamptz +WHERE id = $3::uuid ` type UpdateWorkspaceBuildDeadlineByIDParams struct { - Deadline time.Time `db:"deadline" json:"deadline"` MaxDeadline time.Time `db:"max_deadline" json:"max_deadline"` UpdatedAt time.Time `db:"updated_at" json:"updated_at"` ID uuid.UUID `db:"id" json:"id"` } func (q *sqlQuerier) UpdateWorkspaceBuildDeadlineByID(ctx context.Context, arg UpdateWorkspaceBuildDeadlineByIDParams) error { - _, err := q.db.ExecContext(ctx, updateWorkspaceBuildDeadlineByID, - arg.Deadline, - arg.MaxDeadline, - arg.UpdatedAt, - arg.ID, - ) + _, err := q.db.ExecContext(ctx, updateWorkspaceBuildDeadlineByID, arg.MaxDeadline, arg.UpdatedAt, arg.ID) return err } @@ -12642,8 +12632,10 @@ WHERE -- license here since that's done when the values are written to the build. ( workspace_builds.transition = 'start'::workspace_transition AND - workspace_builds.deadline IS NOT NULL AND - workspace_builds.deadline < $1 :: timestamptz + -- TODO: is this right? or should we check the last_used_at now? + -- workspace_builds.deadline IS NOT NULL AND + -- workspace_builds.deadline < @now :: timestamptz + workspace_builds.max_deadline < $1 :: timestamptz ) OR -- If the workspace build was a stop transition, the workspace is diff --git a/coderd/database/queries/workspacebuilds.sql b/coderd/database/queries/workspacebuilds.sql index 2a1107ef75c5c..da4ee387fb3e9 100644 --- a/coderd/database/queries/workspacebuilds.sql +++ b/coderd/database/queries/workspacebuilds.sql @@ -118,12 +118,11 @@ INSERT INTO initiator_id, job_id, provisioner_state, - deadline, max_deadline, reason ) VALUES - ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13); + ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12); -- name: UpdateWorkspaceBuildCostByID :exec UPDATE @@ -137,7 +136,6 @@ WHERE UPDATE workspace_builds SET - deadline = @deadline::timestamptz, max_deadline = @max_deadline::timestamptz, updated_at = @updated_at::timestamptz WHERE id = @id::uuid; diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 616e83a2bae16..e892864da0dfe 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -575,8 +575,10 @@ WHERE -- license here since that's done when the values are written to the build. ( workspace_builds.transition = 'start'::workspace_transition AND - workspace_builds.deadline IS NOT NULL AND - workspace_builds.deadline < @now :: timestamptz + -- TODO: is this right? or should we check the last_used_at now? + -- workspace_builds.deadline IS NOT NULL AND + -- workspace_builds.deadline < @now :: timestamptz + workspace_builds.max_deadline < @now :: timestamptz ) OR -- If the workspace build was a stop transition, the workspace is diff --git a/coderd/provisionerdserver/provisionerdserver.go b/coderd/provisionerdserver/provisionerdserver.go index f6a8d6abe3a5e..d6b5db82f9827 100644 --- a/coderd/provisionerdserver/provisionerdserver.go +++ b/coderd/provisionerdserver/provisionerdserver.go @@ -963,7 +963,6 @@ func (s *server) FailJob(ctx context.Context, failJob *proto.FailedJob) (*proto. err = db.UpdateWorkspaceBuildDeadlineByID(ctx, database.UpdateWorkspaceBuildDeadlineByIDParams{ ID: input.WorkspaceBuildID, UpdatedAt: dbtime.Now(), - Deadline: build.Deadline, MaxDeadline: build.MaxDeadline, }) if err != nil { @@ -1287,7 +1286,6 @@ func (s *server) CompleteJob(ctx context.Context, completed *proto.CompletedJob) } err = db.UpdateWorkspaceBuildDeadlineByID(ctx, database.UpdateWorkspaceBuildDeadlineByIDParams{ ID: workspaceBuild.ID, - Deadline: autoStop.Deadline, MaxDeadline: autoStop.MaxDeadline, UpdatedAt: now, }) diff --git a/coderd/provisionerdserver/provisionerdserver_test.go b/coderd/provisionerdserver/provisionerdserver_test.go index 6757bd2c6396d..5fcb96627d5d7 100644 --- a/coderd/provisionerdserver/provisionerdserver_test.go +++ b/coderd/provisionerdserver/provisionerdserver_test.go @@ -1304,16 +1304,10 @@ func TestCompleteJob(t *testing.T) { c.expectedDeadline = c.expectedMaxDeadline } - if c.expectedDeadline.IsZero() { - require.True(t, workspaceBuild.Deadline.IsZero()) - } else { - require.WithinDuration(t, c.expectedDeadline, workspaceBuild.Deadline, 15*time.Second, "deadline does not match expected") - } if c.expectedMaxDeadline.IsZero() { require.True(t, workspaceBuild.MaxDeadline.IsZero()) } else { require.WithinDuration(t, c.expectedMaxDeadline, workspaceBuild.MaxDeadline, 15*time.Second, "max deadline does not match expected") - require.GreaterOrEqual(t, workspaceBuild.MaxDeadline.Unix(), workspaceBuild.Deadline.Unix(), "max deadline is smaller than deadline") } require.Len(t, auditor.AuditLogs(), 1) diff --git a/coderd/wsbuilder/wsbuilder.go b/coderd/wsbuilder/wsbuilder.go index b34eb9ce3c858..ca1e6b98f46d8 100644 --- a/coderd/wsbuilder/wsbuilder.go +++ b/coderd/wsbuilder/wsbuilder.go @@ -348,7 +348,6 @@ func (b *Builder) buildTx(authFunc func(action policy.Action, object rbac.Object Transition: b.trans, JobID: provisionerJob.ID, Reason: b.reason, - Deadline: time.Time{}, // set by provisioner upon completion MaxDeadline: time.Time{}, // set by provisioner upon completion }) if err != nil { diff --git a/enterprise/coderd/schedule/template.go b/enterprise/coderd/schedule/template.go index 824bcca6a1bcc..cd6c261fdf2d8 100644 --- a/enterprise/coderd/schedule/template.go +++ b/enterprise/coderd/schedule/template.go @@ -281,13 +281,6 @@ func (s *EnterpriseTemplateScheduleStore) updateWorkspaceBuild(ctx context.Conte autostop.MaxDeadline = now.Add(time.Hour * 2) } - // If the current deadline on the build is after the new max_deadline, then - // set it to the max_deadline. - autostop.Deadline = build.Deadline - if !autostop.MaxDeadline.IsZero() && autostop.Deadline.After(autostop.MaxDeadline) { - autostop.Deadline = autostop.MaxDeadline - } - // If there's a max_deadline but the deadline is 0, then set the deadline to // the max_deadline. if !autostop.MaxDeadline.IsZero() && autostop.Deadline.IsZero() { @@ -298,7 +291,6 @@ func (s *EnterpriseTemplateScheduleStore) updateWorkspaceBuild(ctx context.Conte err = db.UpdateWorkspaceBuildDeadlineByID(ctx, database.UpdateWorkspaceBuildDeadlineByIDParams{ ID: build.ID, UpdatedAt: now, - Deadline: autostop.Deadline, MaxDeadline: autostop.MaxDeadline, }) if err != nil { diff --git a/enterprise/coderd/schedule/template_test.go b/enterprise/coderd/schedule/template_test.go index dd60805f00197..7dc72a7d5f690 100644 --- a/enterprise/coderd/schedule/template_test.go +++ b/enterprise/coderd/schedule/template_test.go @@ -262,7 +262,6 @@ func TestTemplateUpdateBuildDeadlines(t *testing.T) { err = db.UpdateWorkspaceBuildDeadlineByID(ctx, database.UpdateWorkspaceBuildDeadlineByIDParams{ ID: wsBuild.ID, UpdatedAt: buildTime, - Deadline: c.deadline, MaxDeadline: c.maxDeadline, }) require.NoError(t, err) @@ -303,11 +302,6 @@ func TestTemplateUpdateBuildDeadlines(t *testing.T) { // Check that the workspace build has the expected deadlines. newBuild, err := db.GetWorkspaceBuildByID(ctx, wsBuild.ID) require.NoError(t, err) - - if c.newDeadline == nil { - c.newDeadline = &wsBuild.Deadline - } - require.WithinDuration(t, *c.newDeadline, newBuild.Deadline, time.Second) require.WithinDuration(t, c.newMaxDeadline, newBuild.MaxDeadline, time.Second) // Check that the new build has the same state as before. @@ -499,7 +493,6 @@ func TestTemplateUpdateBuildDeadlinesSkip(t *testing.T) { err := db.UpdateWorkspaceBuildDeadlineByID(ctx, database.UpdateWorkspaceBuildDeadlineByIDParams{ ID: wsBuild.ID, UpdatedAt: buildTime, - Deadline: originalMaxDeadline, MaxDeadline: originalMaxDeadline, }) require.NoError(t, err) @@ -587,10 +580,8 @@ func TestTemplateUpdateBuildDeadlinesSkip(t *testing.T) { require.NoError(t, err) if b.shouldBeUpdated { - assert.WithinDuration(t, nextQuietHours, newBuild.Deadline, time.Second, msg) assert.WithinDuration(t, nextQuietHours, newBuild.MaxDeadline, time.Second, msg) } else { - assert.WithinDuration(t, originalMaxDeadline, newBuild.Deadline, time.Second, msg) assert.WithinDuration(t, originalMaxDeadline, newBuild.MaxDeadline, time.Second, msg) } From 523ab788be7b6c533db282f27e824253290fe3e7 Mon Sep 17 00:00:00 2001 From: Garrett Delfosse Date: Thu, 16 May 2024 19:27:23 +0000 Subject: [PATCH 7/7] remove usages --- coderd/workspacebuilds.go | 1 - coderd/workspaces.go | 1 - 2 files changed, 2 deletions(-) diff --git a/coderd/workspacebuilds.go b/coderd/workspacebuilds.go index ef5b63a1e5b19..2bbef1c4dab87 100644 --- a/coderd/workspacebuilds.go +++ b/coderd/workspacebuilds.go @@ -943,7 +943,6 @@ func (api *API) convertWorkspaceBuild( InitiatorID: build.InitiatorID, InitiatorUsername: build.InitiatorByUsername, Job: apiJob, - Deadline: codersdk.NewNullTime(build.Deadline, !build.Deadline.IsZero()), MaxDeadline: codersdk.NewNullTime(build.MaxDeadline, !build.MaxDeadline.IsZero()), Reason: codersdk.BuildReason(build.Reason), Resources: apiResources, diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 7d0344be4e321..aa20d3dbf31ea 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -1083,7 +1083,6 @@ func (api *API) putExtendWorkspace(rw http.ResponseWriter, r *http.Request) { if err := s.UpdateWorkspaceBuildDeadlineByID(ctx, database.UpdateWorkspaceBuildDeadlineByIDParams{ ID: build.ID, UpdatedAt: dbtime.Now(), - Deadline: newDeadline, MaxDeadline: build.MaxDeadline, }); err != nil { code = http.StatusInternalServerError