From 9a398e75dbb4e545e511541631fdf55cbe20bb84 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Wed, 10 Jul 2024 18:48:00 +0000 Subject: [PATCH 01/35] feat(coderd): notify when workspace is marked as dormant --- coderd/coderdtest/coderdtest.go | 4 + .../migrations/000221_notifications.up.sql | 45 ++++-- coderd/dormancy/notifications.go | 43 ++++++ coderd/notifications/events.go | 1 + coderd/workspaces.go | 24 +++ coderd/workspaces_test.go | 139 ++++++++++++++++++ 6 files changed, 243 insertions(+), 13 deletions(-) create mode 100644 coderd/dormancy/notifications.go diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index 472c380926ec4..329b092058d12 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -64,6 +64,7 @@ import ( "github.com/coder/coder/v2/coderd/externalauth" "github.com/coder/coder/v2/coderd/gitsshkey" "github.com/coder/coder/v2/coderd/httpmw" + "github.com/coder/coder/v2/coderd/notifications" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/telemetry" @@ -154,6 +155,8 @@ type Options struct { DatabaseRolluper *dbrollup.Rolluper WorkspaceUsageTrackerFlush chan int WorkspaceUsageTrackerTick chan time.Time + + NotificationEnqueuer notifications.Enqueuer } // New constructs a codersdk client connected to an in-memory API instance. @@ -498,6 +501,7 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can NewTicker: options.NewTicker, DatabaseRolluper: options.DatabaseRolluper, WorkspaceUsageTracker: wuTracker, + NotificationsEnqueuer: options.NotificationEnqueuer, } } diff --git a/coderd/database/migrations/000221_notifications.up.sql b/coderd/database/migrations/000221_notifications.up.sql index 29a6b912d3e20..aa3ae9cf4aa60 100644 --- a/coderd/database/migrations/000221_notifications.up.sql +++ b/coderd/database/migrations/000221_notifications.up.sql @@ -50,16 +50,35 @@ CREATE TABLE notification_messages CREATE INDEX idx_notification_messages_status ON notification_messages (status); -- TODO: autogenerate constants which reference the UUIDs -INSERT INTO notification_templates (id, name, title_template, body_template, "group", actions) -VALUES ('f517da0b-cdc9-410f-ab89-a86107c420ed', 'Workspace Deleted', E'Workspace "{{.Labels.name}}" deleted', - E'Hi {{.UserName}}\n\nYour workspace **{{.Labels.name}}** was deleted.\nThe specified reason was "**{{.Labels.reason}}{{ if .Labels.initiator }} ({{ .Labels.initiator }}){{end}}**".', - 'Workspace Events', '[ - { - "label": "View workspaces", - "url": "{{ base_url }}/workspaces" - }, - { - "label": "View templates", - "url": "{{ base_url }}/templates" - } - ]'::jsonb); +INSERT INTO notification_templates + (id, name, title_template, body_template, "group", actions) +VALUES + -- Workspace Deleted + ('f517da0b-cdc9-410f-ab89-a86107c420ed', + 'Workspace Deleted', + E'Workspace "{{.Labels.name}}" deleted', + E'Hi {{.UserName}}\n\nYour workspace **{{.Labels.name}}** was deleted.\nThe specified reason was "**{{.Labels.reason}}{{ if .Labels.initiator }} ({{ .Labels.initiator }}){{end}}**".', + 'Workspace Events', + '[ + { + "label": "View workspaces", + "url": "{{ base_url }}/workspaces" + }, + { + "label": "View templates", + "url": "{{ base_url }}/templates" + } + ]'::jsonb), + -- Workspace Marked as Dormant + ('123e4567-e89b-12d3-a456-426614174000', + 'Workspace Marked as Dormant', + E'Workspace "{{.Labels.name}}" marked as dormant', + E'Hi {{.UserName}}\n\nYour workspace **{{.Labels.name}}** has been marked as dormant.\nThe specified reason was "**{{.Labels.reason}}{{ if .Labels.initiator }} ({{ .Labels.initiator }}){{end}}**', + 'Workspace Events', + '[ + { + "label": "View workspace", + "url": "{{ base_url }}/@{{.UserName}}/{{.Labels.name}}" + }, + ]'::jsonb); + diff --git a/coderd/dormancy/notifications.go b/coderd/dormancy/notifications.go new file mode 100644 index 0000000000000..25740bcfacef0 --- /dev/null +++ b/coderd/dormancy/notifications.go @@ -0,0 +1,43 @@ +package dormancy + +import ( + "context" + + "cdr.dev/slog" + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/notifications" +) + +type WorkspaceDormantNotification struct { + Workspace database.Workspace + InitiatedBy string + Reason string + CreatedBy string +} + +func NotifyWorkspaceDormant( + ctx context.Context, + logger slog.Logger, + enqueuer notifications.Enqueuer, + notification WorkspaceDormantNotification, +) { + labels := map[string]string{ + "name": notification.Workspace.Name, + "initiatedBy": notification.InitiatedBy, + "reason": notification.Reason, + } + if _, err := enqueuer.Enqueue( + ctx, + notification.Workspace.OwnerID, + notifications.TemplateWorkspaceDormant, + labels, + notification.CreatedBy, + // Associate this notification with all the related entities. + notification.Workspace.ID, + notification.Workspace.OwnerID, + notification.Workspace.TemplateID, + notification.Workspace.OrganizationID, + ); err != nil { + logger.Warn(ctx, "failed to notify of workspace marked as dormant", slog.Error(err)) + } +} diff --git a/coderd/notifications/events.go b/coderd/notifications/events.go index 6cb2870748b61..0068099700576 100644 --- a/coderd/notifications/events.go +++ b/coderd/notifications/events.go @@ -7,3 +7,4 @@ import "github.com/google/uuid" // Workspace-related events. var TemplateWorkspaceDeleted = uuid.MustParse("f517da0b-cdc9-410f-ab89-a86107c420ed") +var TemplateWorkspaceDormant = uuid.MustParse("123e4567-e89b-12d3-a456-426614174000") diff --git a/coderd/workspaces.go b/coderd/workspaces.go index bed982d5e2511..48c90748555e4 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -23,6 +23,7 @@ import ( "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/database/provisionerjobs" + "github.com/coder/coder/v2/coderd/dormancy" "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/coderd/httpmw" "github.com/coder/coder/v2/coderd/rbac" @@ -952,6 +953,29 @@ func (api *API) putWorkspaceDormant(rw http.ResponseWriter, r *http.Request) { return } + // We don't need to notify the owner if they are the one making the request. + if req.Dormant && apiKey.UserID != workspace.OwnerID { + initiator, err := api.Database.GetUserByID(ctx, apiKey.UserID) + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error fetching user.", + Detail: err.Error(), + }) + return + } + dormancy.NotifyWorkspaceDormant( + ctx, + api.Logger, + api.NotificationsEnqueuer, + dormancy.WorkspaceDormantNotification{ + Workspace: workspace, + InitiatedBy: initiator.Username, + Reason: "requested by user", + CreatedBy: "api", + }, + ) + } + data, err := api.workspaceData(ctx, []database.Workspace{workspace}) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index cef7f875fde46..a5ce17e261c7e 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -11,6 +11,7 @@ import ( "os" "slices" "strings" + "sync" "testing" "time" @@ -3297,6 +3298,7 @@ func TestWorkspaceDormant(t *testing.T) { require.NoError(t, err) coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStop, database.WorkspaceTransitionStart) }) + } func TestWorkspaceFavoriteUnfavorite(t *testing.T) { @@ -3504,3 +3506,140 @@ func TestWorkspaceUsageTracking(t *testing.T) { require.Greater(t, newWorkspace.LatestBuild.Deadline.Time, workspace.LatestBuild.Deadline.Time) }) } + +func TestNotifications(t *testing.T) { + t.Parallel() + + t.Run("Dormant", func(t *testing.T) { + t.Parallel() + + t.Run("InitiatorNotOwner", func(t *testing.T) { + t.Parallel() + + var ( + notifyEnq = &fakeNotificationEnqueuer{} + client = coderdtest.New(t, &coderdtest.Options{ + IncludeProvisionerDaemon: true, + NotificationEnqueuer: notifyEnq, + }) + user = coderdtest.CreateFirstUser(t, client) + memberClient, member = coderdtest.CreateAnotherUser(t, client, user.OrganizationID, rbac.RoleUserAdmin()) + version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + _ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) + template = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + workspace = coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + _ = coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID) + ) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + err := memberClient.UpdateWorkspaceDormancy(ctx, workspace.ID, codersdk.UpdateWorkspaceDormancy{ + Dormant: true, + }) + require.NoError(t, err, "mark workspace as dormant") + require.Len(t, notifyEnq.sent, 1) + require.Equal(t, notifyEnq.sent[0].userID, workspace.OwnerID) + require.Contains(t, notifyEnq.sent[0].targets, template.ID) + require.Contains(t, notifyEnq.sent[0].targets, workspace.ID) + require.Contains(t, notifyEnq.sent[0].targets, workspace.OrganizationID) + require.Contains(t, notifyEnq.sent[0].targets, workspace.OwnerID) + require.Equal(t, notifyEnq.sent[0].labels["initiatedBy"], member.Username) + }) + + t.Run("InitiatorIsOwner", func(t *testing.T) { + t.Parallel() + var ( + notifyEnq = &fakeNotificationEnqueuer{} + client = coderdtest.New(t, &coderdtest.Options{ + IncludeProvisionerDaemon: true, + NotificationEnqueuer: notifyEnq, + }) + user = coderdtest.CreateFirstUser(t, client) + version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + _ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) + template = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + workspace = coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + _ = coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID) + ) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + err := client.UpdateWorkspaceDormancy(ctx, workspace.ID, codersdk.UpdateWorkspaceDormancy{ + Dormant: true, + }) + require.NoError(t, err, "mark workspace as dormant") + require.Len(t, notifyEnq.sent, 0) + }) + + t.Run("ActivateDormantWorkspace", func(t *testing.T) { + t.Parallel() + + var ( + notifyEnq = &fakeNotificationEnqueuer{} + client = coderdtest.New(t, &coderdtest.Options{ + IncludeProvisionerDaemon: true, + NotificationEnqueuer: notifyEnq, + }) + user = coderdtest.CreateFirstUser(t, client) + version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + _ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) + template = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + workspace = coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + _ = coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID) + ) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + // Make workspace dormant before activate it + err := client.UpdateWorkspaceDormancy(ctx, workspace.ID, codersdk.UpdateWorkspaceDormancy{ + Dormant: true, + }) + require.NoError(t, err, "mark workspace as dormant") + // Clear notifications before activating the workspace + notifyEnq.Clear() + err = client.UpdateWorkspaceDormancy(ctx, workspace.ID, codersdk.UpdateWorkspaceDormancy{ + Dormant: false, + }) + require.NoError(t, err, "mark workspace as active") + require.Len(t, notifyEnq.sent, 0) + }) + }) +} + +type fakeNotificationEnqueuer struct { + mu sync.Mutex + sent []*notification +} + +type notification struct { + userID, templateID uuid.UUID + labels map[string]string + createdBy string + targets []uuid.UUID +} + +func (f *fakeNotificationEnqueuer) Enqueue(_ context.Context, userID, templateID uuid.UUID, labels map[string]string, createdBy string, targets ...uuid.UUID) (*uuid.UUID, error) { + f.mu.Lock() + defer f.mu.Unlock() + + f.sent = append(f.sent, ¬ification{ + userID: userID, + templateID: templateID, + labels: labels, + createdBy: createdBy, + targets: targets, + }) + + id := uuid.New() + return &id, nil +} + +func (f *fakeNotificationEnqueuer) Clear() { + f.mu.Lock() + defer f.mu.Unlock() + + f.sent = nil +} From fbf99be645ea98bad0db52cfd8ee4cadc6d240e9 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Wed, 10 Jul 2024 18:50:07 +0000 Subject: [PATCH 02/35] Fix role --- coderd/workspaces_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index a5ce17e261c7e..31da652bc7c39 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -3523,7 +3523,7 @@ func TestNotifications(t *testing.T) { NotificationEnqueuer: notifyEnq, }) user = coderdtest.CreateFirstUser(t, client) - memberClient, member = coderdtest.CreateAnotherUser(t, client, user.OrganizationID, rbac.RoleUserAdmin()) + memberClient, member = coderdtest.CreateAnotherUser(t, client, user.OrganizationID, rbac.RoleOwner()) version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) _ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) template = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) From 40e5801f8dbf793918fc133367fdb9e4bab513df Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Thu, 11 Jul 2024 13:19:38 +0000 Subject: [PATCH 03/35] Apply Danny suggestions --- .../migrations/000221_notifications.up.sql | 45 +++-------- ...23_dormancy_notification_template.down.sql | 1 + ...0223_dormancy_notification_template.up.sql | 16 ++++ .../provisionerdserver_test.go | 46 +++-------- coderd/workspaces.go | 15 ++-- coderd/workspaces_test.go | 79 +++++++------------ .../coderd}/dormancy/notifications.go | 14 ++-- testutil/notifications.go | 43 ++++++++++ 8 files changed, 123 insertions(+), 136 deletions(-) create mode 100644 coderd/database/migrations/000223_dormancy_notification_template.down.sql create mode 100644 coderd/database/migrations/000223_dormancy_notification_template.up.sql rename {coderd => enterprise/coderd}/dormancy/notifications.go (79%) create mode 100644 testutil/notifications.go diff --git a/coderd/database/migrations/000221_notifications.up.sql b/coderd/database/migrations/000221_notifications.up.sql index aa3ae9cf4aa60..29a6b912d3e20 100644 --- a/coderd/database/migrations/000221_notifications.up.sql +++ b/coderd/database/migrations/000221_notifications.up.sql @@ -50,35 +50,16 @@ CREATE TABLE notification_messages CREATE INDEX idx_notification_messages_status ON notification_messages (status); -- TODO: autogenerate constants which reference the UUIDs -INSERT INTO notification_templates - (id, name, title_template, body_template, "group", actions) -VALUES - -- Workspace Deleted - ('f517da0b-cdc9-410f-ab89-a86107c420ed', - 'Workspace Deleted', - E'Workspace "{{.Labels.name}}" deleted', - E'Hi {{.UserName}}\n\nYour workspace **{{.Labels.name}}** was deleted.\nThe specified reason was "**{{.Labels.reason}}{{ if .Labels.initiator }} ({{ .Labels.initiator }}){{end}}**".', - 'Workspace Events', - '[ - { - "label": "View workspaces", - "url": "{{ base_url }}/workspaces" - }, - { - "label": "View templates", - "url": "{{ base_url }}/templates" - } - ]'::jsonb), - -- Workspace Marked as Dormant - ('123e4567-e89b-12d3-a456-426614174000', - 'Workspace Marked as Dormant', - E'Workspace "{{.Labels.name}}" marked as dormant', - E'Hi {{.UserName}}\n\nYour workspace **{{.Labels.name}}** has been marked as dormant.\nThe specified reason was "**{{.Labels.reason}}{{ if .Labels.initiator }} ({{ .Labels.initiator }}){{end}}**', - 'Workspace Events', - '[ - { - "label": "View workspace", - "url": "{{ base_url }}/@{{.UserName}}/{{.Labels.name}}" - }, - ]'::jsonb); - +INSERT INTO notification_templates (id, name, title_template, body_template, "group", actions) +VALUES ('f517da0b-cdc9-410f-ab89-a86107c420ed', 'Workspace Deleted', E'Workspace "{{.Labels.name}}" deleted', + E'Hi {{.UserName}}\n\nYour workspace **{{.Labels.name}}** was deleted.\nThe specified reason was "**{{.Labels.reason}}{{ if .Labels.initiator }} ({{ .Labels.initiator }}){{end}}**".', + 'Workspace Events', '[ + { + "label": "View workspaces", + "url": "{{ base_url }}/workspaces" + }, + { + "label": "View templates", + "url": "{{ base_url }}/templates" + } + ]'::jsonb); diff --git a/coderd/database/migrations/000223_dormancy_notification_template.down.sql b/coderd/database/migrations/000223_dormancy_notification_template.down.sql new file mode 100644 index 0000000000000..6e63f38af7a6d --- /dev/null +++ b/coderd/database/migrations/000223_dormancy_notification_template.down.sql @@ -0,0 +1 @@ +DELETE notification_templates WHERE id = '123e4567-e89b-12d3-a456-426614174000'; diff --git a/coderd/database/migrations/000223_dormancy_notification_template.up.sql b/coderd/database/migrations/000223_dormancy_notification_template.up.sql new file mode 100644 index 0000000000000..a9fa9423e37f6 --- /dev/null +++ b/coderd/database/migrations/000223_dormancy_notification_template.up.sql @@ -0,0 +1,16 @@ +-- TODO: autogenerate constants which reference the UUIDs +INSERT INTO notification_templates + (id, name, title_template, body_template, "group", actions) +VALUES + -- Workspace Marked as Dormant + ('123e4567-e89b-12d3-a456-426614174000', + 'Workspace Marked as Dormant', + E'Workspace "{{.Labels.name}}" marked as dormant', + E'Hi {{.UserName}}\n\nYour workspace **{{.Labels.name}}** has been marked as dormant.\nThe specified reason was "**{{.Labels.reason}}{{ if .Labels.initiator }} ({{ .Labels.initiator }}){{end}}**', + 'Workspace Events', + '[ + { + "label": "View workspace", + "url": "{{ base_url }}/@{{.UserName}}/{{.Labels.name}}" + }, + ]'::jsonb); diff --git a/coderd/provisionerdserver/provisionerdserver_test.go b/coderd/provisionerdserver/provisionerdserver_test.go index 7049359be98a7..bb30349817fc7 100644 --- a/coderd/provisionerdserver/provisionerdserver_test.go +++ b/coderd/provisionerdserver/provisionerdserver_test.go @@ -1602,7 +1602,7 @@ func TestNotifications(t *testing.T) { t.Parallel() ctx := context.Background() - notifEnq := &fakeNotificationEnqueuer{} + notifEnq := &testutil.FakeNotificationEnqueuer{} srv, db, ps, pd := setup(t, false, &overrides{ notificationEnqueuer: notifEnq, @@ -1680,17 +1680,17 @@ func TestNotifications(t *testing.T) { if tc.shouldNotify { // Validate that the notification was sent and contained the expected values. - require.Len(t, notifEnq.sent, 1) - require.Equal(t, notifEnq.sent[0].userID, user.ID) - require.Contains(t, notifEnq.sent[0].targets, template.ID) - require.Contains(t, notifEnq.sent[0].targets, workspace.ID) - require.Contains(t, notifEnq.sent[0].targets, workspace.OrganizationID) - require.Contains(t, notifEnq.sent[0].targets, user.ID) + require.Len(t, notifEnq.Sent, 1) + require.Equal(t, notifEnq.Sent[0].UserID, user.ID) + require.Contains(t, notifEnq.Sent[0].Targets, template.ID) + require.Contains(t, notifEnq.Sent[0].Targets, workspace.ID) + require.Contains(t, notifEnq.Sent[0].Targets, workspace.OrganizationID) + require.Contains(t, notifEnq.Sent[0].Targets, user.ID) if tc.deletionReason == database.BuildReasonInitiator { - require.Equal(t, notifEnq.sent[0].labels["initiatedBy"], initiator.Username) + require.Equal(t, notifEnq.Sent[0].Labels["initiatedBy"], initiator.Username) } } else { - require.Len(t, notifEnq.sent, 0) + require.Len(t, notifEnq.Sent, 0) } }) } @@ -1919,31 +1919,3 @@ func (s *fakeStream) cancel() { s.canceled = true s.c.Broadcast() } - -type fakeNotificationEnqueuer struct { - mu sync.Mutex - sent []*notification -} - -type notification struct { - userID, templateID uuid.UUID - labels map[string]string - createdBy string - targets []uuid.UUID -} - -func (f *fakeNotificationEnqueuer) Enqueue(_ context.Context, userID, templateID uuid.UUID, labels map[string]string, createdBy string, targets ...uuid.UUID) (*uuid.UUID, error) { - f.mu.Lock() - defer f.mu.Unlock() - - f.sent = append(f.sent, ¬ification{ - userID: userID, - templateID: templateID, - labels: labels, - createdBy: createdBy, - targets: targets, - }) - - id := uuid.New() - return &id, nil -} diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 48c90748555e4..ee572e35f99a2 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -23,7 +23,6 @@ import ( "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/database/provisionerjobs" - "github.com/coder/coder/v2/coderd/dormancy" "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/coderd/httpmw" "github.com/coder/coder/v2/coderd/rbac" @@ -35,6 +34,7 @@ import ( "github.com/coder/coder/v2/coderd/wsbuilder" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/codersdk/agentsdk" + "github.com/coder/coder/v2/enterprise/coderd/dormancy" ) var ( @@ -957,10 +957,7 @@ func (api *API) putWorkspaceDormant(rw http.ResponseWriter, r *http.Request) { if req.Dormant && apiKey.UserID != workspace.OwnerID { initiator, err := api.Database.GetUserByID(ctx, apiKey.UserID) if err != nil { - httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Internal error fetching user.", - Detail: err.Error(), - }) + api.Logger.Warn(ctx, "failed to get the initiator by id", slog.Error(err)) return } dormancy.NotifyWorkspaceDormant( @@ -968,10 +965,10 @@ func (api *API) putWorkspaceDormant(rw http.ResponseWriter, r *http.Request) { api.Logger, api.NotificationsEnqueuer, dormancy.WorkspaceDormantNotification{ - Workspace: workspace, - InitiatedBy: initiator.Username, - Reason: "requested by user", - CreatedBy: "api", + Workspace: workspace, + Initiator: initiator.Username, + Reason: "requested by user", + CreatedBy: "api", }, ) } diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 31da652bc7c39..24d8fddff527f 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -11,7 +11,6 @@ import ( "os" "slices" "strings" - "sync" "testing" "time" @@ -3516,8 +3515,9 @@ func TestNotifications(t *testing.T) { t.Run("InitiatorNotOwner", func(t *testing.T) { t.Parallel() + // Given var ( - notifyEnq = &fakeNotificationEnqueuer{} + notifyEnq = &testutil.FakeNotificationEnqueuer{} client = coderdtest.New(t, &coderdtest.Options{ IncludeProvisionerDaemon: true, NotificationEnqueuer: notifyEnq, @@ -3532,25 +3532,30 @@ func TestNotifications(t *testing.T) { ) ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() + t.Cleanup(cancel) + // When err := memberClient.UpdateWorkspaceDormancy(ctx, workspace.ID, codersdk.UpdateWorkspaceDormancy{ Dormant: true, }) + + // Then require.NoError(t, err, "mark workspace as dormant") - require.Len(t, notifyEnq.sent, 1) - require.Equal(t, notifyEnq.sent[0].userID, workspace.OwnerID) - require.Contains(t, notifyEnq.sent[0].targets, template.ID) - require.Contains(t, notifyEnq.sent[0].targets, workspace.ID) - require.Contains(t, notifyEnq.sent[0].targets, workspace.OrganizationID) - require.Contains(t, notifyEnq.sent[0].targets, workspace.OwnerID) - require.Equal(t, notifyEnq.sent[0].labels["initiatedBy"], member.Username) + require.Len(t, notifyEnq.Sent, 1) + require.Equal(t, notifyEnq.Sent[0].UserID, workspace.OwnerID) + require.Contains(t, notifyEnq.Sent[0].Targets, template.ID) + require.Contains(t, notifyEnq.Sent[0].Targets, workspace.ID) + require.Contains(t, notifyEnq.Sent[0].Targets, workspace.OrganizationID) + require.Contains(t, notifyEnq.Sent[0].Targets, workspace.OwnerID) + require.Equal(t, notifyEnq.Sent[0].Labels["initiatedBy"], member.Username) }) t.Run("InitiatorIsOwner", func(t *testing.T) { t.Parallel() + + // Given var ( - notifyEnq = &fakeNotificationEnqueuer{} + notifyEnq = &testutil.FakeNotificationEnqueuer{} client = coderdtest.New(t, &coderdtest.Options{ IncludeProvisionerDaemon: true, NotificationEnqueuer: notifyEnq, @@ -3564,20 +3569,24 @@ func TestNotifications(t *testing.T) { ) ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() + t.Cleanup(cancel) + // When err := client.UpdateWorkspaceDormancy(ctx, workspace.ID, codersdk.UpdateWorkspaceDormancy{ Dormant: true, }) + + // Then require.NoError(t, err, "mark workspace as dormant") - require.Len(t, notifyEnq.sent, 0) + require.Len(t, notifyEnq.Sent, 0) }) t.Run("ActivateDormantWorkspace", func(t *testing.T) { t.Parallel() + // Given var ( - notifyEnq = &fakeNotificationEnqueuer{} + notifyEnq = &testutil.FakeNotificationEnqueuer{} client = coderdtest.New(t, &coderdtest.Options{ IncludeProvisionerDaemon: true, NotificationEnqueuer: notifyEnq, @@ -3590,8 +3599,9 @@ func TestNotifications(t *testing.T) { _ = coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID) ) + // When ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() + t.Cleanup(cancel) // Make workspace dormant before activate it err := client.UpdateWorkspaceDormancy(ctx, workspace.ID, codersdk.UpdateWorkspaceDormancy{ @@ -3600,46 +3610,13 @@ func TestNotifications(t *testing.T) { require.NoError(t, err, "mark workspace as dormant") // Clear notifications before activating the workspace notifyEnq.Clear() + + // Then err = client.UpdateWorkspaceDormancy(ctx, workspace.ID, codersdk.UpdateWorkspaceDormancy{ Dormant: false, }) require.NoError(t, err, "mark workspace as active") - require.Len(t, notifyEnq.sent, 0) + require.Len(t, notifyEnq.Sent, 0) }) }) } - -type fakeNotificationEnqueuer struct { - mu sync.Mutex - sent []*notification -} - -type notification struct { - userID, templateID uuid.UUID - labels map[string]string - createdBy string - targets []uuid.UUID -} - -func (f *fakeNotificationEnqueuer) Enqueue(_ context.Context, userID, templateID uuid.UUID, labels map[string]string, createdBy string, targets ...uuid.UUID) (*uuid.UUID, error) { - f.mu.Lock() - defer f.mu.Unlock() - - f.sent = append(f.sent, ¬ification{ - userID: userID, - templateID: templateID, - labels: labels, - createdBy: createdBy, - targets: targets, - }) - - id := uuid.New() - return &id, nil -} - -func (f *fakeNotificationEnqueuer) Clear() { - f.mu.Lock() - defer f.mu.Unlock() - - f.sent = nil -} diff --git a/coderd/dormancy/notifications.go b/enterprise/coderd/dormancy/notifications.go similarity index 79% rename from coderd/dormancy/notifications.go rename to enterprise/coderd/dormancy/notifications.go index 25740bcfacef0..e557ae1eb33c5 100644 --- a/coderd/dormancy/notifications.go +++ b/enterprise/coderd/dormancy/notifications.go @@ -9,10 +9,10 @@ import ( ) type WorkspaceDormantNotification struct { - Workspace database.Workspace - InitiatedBy string - Reason string - CreatedBy string + Workspace database.Workspace + Initiator string + Reason string + CreatedBy string } func NotifyWorkspaceDormant( @@ -22,9 +22,9 @@ func NotifyWorkspaceDormant( notification WorkspaceDormantNotification, ) { labels := map[string]string{ - "name": notification.Workspace.Name, - "initiatedBy": notification.InitiatedBy, - "reason": notification.Reason, + "name": notification.Workspace.Name, + "initiator": notification.Initiator, + "reason": notification.Reason, } if _, err := enqueuer.Enqueue( ctx, diff --git a/testutil/notifications.go b/testutil/notifications.go new file mode 100644 index 0000000000000..b777e2516f964 --- /dev/null +++ b/testutil/notifications.go @@ -0,0 +1,43 @@ +package testutil + +import ( + "context" + "sync" + + "github.com/google/uuid" +) + +type FakeNotificationEnqueuer struct { + mu sync.Mutex + Sent []*Notification +} + +type Notification struct { + UserID, TemplateID uuid.UUID + Labels map[string]string + CreatedBy string + Targets []uuid.UUID +} + +func (f *FakeNotificationEnqueuer) Enqueue(_ context.Context, userID, templateID uuid.UUID, labels map[string]string, createdBy string, targets ...uuid.UUID) (*uuid.UUID, error) { + f.mu.Lock() + defer f.mu.Unlock() + + f.Sent = append(f.Sent, &Notification{ + UserID: userID, + TemplateID: templateID, + Labels: labels, + CreatedBy: createdBy, + Targets: targets, + }) + + id := uuid.New() + return &id, nil +} + +func (f *FakeNotificationEnqueuer) Clear() { + f.mu.Lock() + defer f.mu.Unlock() + + f.Sent = nil +} From 9a35cbb9bc47c4e97322fb22db9ff5a98f52c3ab Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Thu, 11 Jul 2024 13:37:14 +0000 Subject: [PATCH 04/35] Notify dormant workspace on lifecycle executor --- cli/server.go | 2 +- coderd/autobuild/lifecycle_executor.go | 19 ++++++++++++++++++- coderd/coderdtest/coderdtest.go | 1 + 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/cli/server.go b/cli/server.go index 9c80ab1d9b8c7..92d801b64545c 100644 --- a/cli/server.go +++ b/cli/server.go @@ -1065,7 +1065,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd. autobuildTicker := time.NewTicker(vals.AutobuildPollInterval.Value()) defer autobuildTicker.Stop() autobuildExecutor := autobuild.NewExecutor( - ctx, options.Database, options.Pubsub, coderAPI.TemplateScheduleStore, &coderAPI.Auditor, coderAPI.AccessControlStore, logger, autobuildTicker.C) + ctx, options.Database, options.Pubsub, coderAPI.TemplateScheduleStore, &coderAPI.Auditor, coderAPI.AccessControlStore, logger, autobuildTicker.C, options.NotificationsEnqueuer) autobuildExecutor.Run() hangDetectorTicker := time.NewTicker(vals.JobHangDetectorInterval.Value()) diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index 4bbbaba667c7e..9c76932cb54e7 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -19,8 +19,10 @@ import ( "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/database/provisionerjobs" "github.com/coder/coder/v2/coderd/database/pubsub" + "github.com/coder/coder/v2/coderd/notifications" "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/wsbuilder" + "github.com/coder/coder/v2/enterprise/coderd/dormancy" ) // Executor automatically starts or stops workspaces. @@ -34,6 +36,8 @@ type Executor struct { log slog.Logger tick <-chan time.Time statsCh chan<- Stats + // NotificationsEnqueuer handles enqueueing notifications for delivery by SMTP, webhook, etc. + notificationsEnqueuer notifications.Enqueuer } // Stats contains information about one run of Executor. @@ -44,7 +48,7 @@ type Stats struct { } // New returns a new wsactions executor. -func NewExecutor(ctx context.Context, db database.Store, ps pubsub.Pubsub, tss *atomic.Pointer[schedule.TemplateScheduleStore], auditor *atomic.Pointer[audit.Auditor], acs *atomic.Pointer[dbauthz.AccessControlStore], log slog.Logger, tick <-chan time.Time) *Executor { +func NewExecutor(ctx context.Context, db database.Store, ps pubsub.Pubsub, tss *atomic.Pointer[schedule.TemplateScheduleStore], auditor *atomic.Pointer[audit.Auditor], acs *atomic.Pointer[dbauthz.AccessControlStore], log slog.Logger, tick <-chan time.Time, ntf notifications.Enqueuer) *Executor { le := &Executor{ //nolint:gocritic // Autostart has a limited set of permissions. ctx: dbauthz.AsAutostart(ctx), @@ -55,6 +59,7 @@ func NewExecutor(ctx context.Context, db database.Store, ps pubsub.Pubsub, tss * log: log.Named("autobuild"), auditor: auditor, accessControlStore: acs, + notificationsEnqueuer: ntf, } return le } @@ -215,6 +220,18 @@ func (e *Executor) runOnce(t time.Time) Stats { }, }) + dormancy.NotifyWorkspaceDormant( + e.ctx, + e.log, + e.notificationsEnqueuer, + dormancy.WorkspaceDormantNotification{ + Workspace: ws, + Initiator: "system", + Reason: "breached the template's threshold for inactivity", + CreatedBy: "lifecycleexecutor", + }, + ) + auditLog = &auditParams{ Old: wsOld, New: ws, diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index 329b092058d12..1275f588d710e 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -308,6 +308,7 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can accessControlStore, *options.Logger, options.AutobuildTicker, + options.NotificationEnqueuer, ).WithStatsChannel(options.AutobuildStats) lifecycleExecutor.Run() From 07015721e8ec68e6138a5946ca729cb126ffacd6 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Thu, 11 Jul 2024 14:53:40 +0000 Subject: [PATCH 05/35] Notify dormancy on template schedule --- coderd/database/dbauthz/dbauthz.go | 13 +++--- coderd/database/dbmem/dbmem.go | 6 +-- coderd/database/dbmetrics/dbmetrics.go | 6 +-- coderd/database/dbmock/dbmock.go | 7 +-- ...0223_dormancy_notification_template.up.sql | 25 ++++------- coderd/database/querier.go | 2 +- coderd/database/queries.sql.go | 43 +++++++++++++++++-- coderd/database/queries/workspaces.sql | 5 ++- coderd/schedule/template.go | 7 ++- enterprise/coderd/schedule/template.go | 24 ++++++++++- enterprise/coderd/schedule/template_test.go | 5 ++- 11 files changed, 100 insertions(+), 43 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 67dadd5d74e19..4845493f634cf 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -3526,12 +3526,15 @@ func (q *querier) UpdateWorkspaceTTL(ctx context.Context, arg database.UpdateWor return update(q.log, q.auth, fetch, q.db.UpdateWorkspaceTTL)(ctx, arg) } -func (q *querier) UpdateWorkspacesDormantDeletingAtByTemplateID(ctx context.Context, arg database.UpdateWorkspacesDormantDeletingAtByTemplateIDParams) error { - fetch := func(ctx context.Context, arg database.UpdateWorkspacesDormantDeletingAtByTemplateIDParams) (database.Template, error) { - return q.db.GetTemplateByID(ctx, arg.TemplateID) +func (q *querier) UpdateWorkspacesDormantDeletingAtByTemplateID(ctx context.Context, arg database.UpdateWorkspacesDormantDeletingAtByTemplateIDParams) ([]database.Workspace, error) { + template, err := q.db.GetTemplateByID(ctx, arg.TemplateID) + if err != nil { + return nil, xerrors.Errorf("get template by id: %w", err) } - - return fetchAndExec(q.log, q.auth, policy.ActionUpdate, fetch, q.db.UpdateWorkspacesDormantDeletingAtByTemplateID)(ctx, arg) + if err := q.authorizeContext(ctx, policy.ActionUpdate, template); err != nil { + return nil, err + } + return q.db.UpdateWorkspacesDormantDeletingAtByTemplateID(ctx, arg) } func (q *querier) UpsertAnnouncementBanners(ctx context.Context, value string) error { diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 3db958cb9a307..f9fee53e4f3b7 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -8552,13 +8552,13 @@ func (q *FakeQuerier) UpdateWorkspaceTTL(_ context.Context, arg database.UpdateW return sql.ErrNoRows } -func (q *FakeQuerier) UpdateWorkspacesDormantDeletingAtByTemplateID(_ context.Context, arg database.UpdateWorkspacesDormantDeletingAtByTemplateIDParams) error { +func (q *FakeQuerier) UpdateWorkspacesDormantDeletingAtByTemplateID(_ context.Context, arg database.UpdateWorkspacesDormantDeletingAtByTemplateIDParams) ([]database.Workspace, error) { q.mutex.Lock() defer q.mutex.Unlock() err := validateDatabaseType(arg) if err != nil { - return err + return nil, err } for i, ws := range q.workspaces { @@ -8587,7 +8587,7 @@ func (q *FakeQuerier) UpdateWorkspacesDormantDeletingAtByTemplateID(_ context.Co q.workspaces[i] = ws } - return nil + return q.workspaces, nil } func (q *FakeQuerier) UpsertAnnouncementBanners(_ context.Context, data string) error { diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go index 0a7ecd4fb5f10..b74d1ce8a43a8 100644 --- a/coderd/database/dbmetrics/dbmetrics.go +++ b/coderd/database/dbmetrics/dbmetrics.go @@ -2230,11 +2230,11 @@ func (m metricsStore) UpdateWorkspaceTTL(ctx context.Context, arg database.Updat return r0 } -func (m metricsStore) UpdateWorkspacesDormantDeletingAtByTemplateID(ctx context.Context, arg database.UpdateWorkspacesDormantDeletingAtByTemplateIDParams) error { +func (m metricsStore) UpdateWorkspacesDormantDeletingAtByTemplateID(ctx context.Context, arg database.UpdateWorkspacesDormantDeletingAtByTemplateIDParams) ([]database.Workspace, error) { start := time.Now() - r0 := m.s.UpdateWorkspacesDormantDeletingAtByTemplateID(ctx, arg) + r0, r1 := m.s.UpdateWorkspacesDormantDeletingAtByTemplateID(ctx, arg) m.queryLatencies.WithLabelValues("UpdateWorkspacesDormantDeletingAtByTemplateID").Observe(time.Since(start).Seconds()) - return r0 + return r0, r1 } func (m metricsStore) UpsertAnnouncementBanners(ctx context.Context, value string) error { diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 982a6472ec16c..00b49a3cd895e 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -4673,11 +4673,12 @@ func (mr *MockStoreMockRecorder) UpdateWorkspaceTTL(arg0, arg1 any) *gomock.Call } // UpdateWorkspacesDormantDeletingAtByTemplateID mocks base method. -func (m *MockStore) UpdateWorkspacesDormantDeletingAtByTemplateID(arg0 context.Context, arg1 database.UpdateWorkspacesDormantDeletingAtByTemplateIDParams) error { +func (m *MockStore) UpdateWorkspacesDormantDeletingAtByTemplateID(arg0 context.Context, arg1 database.UpdateWorkspacesDormantDeletingAtByTemplateIDParams) ([]database.Workspace, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "UpdateWorkspacesDormantDeletingAtByTemplateID", arg0, arg1) - ret0, _ := ret[0].(error) - return ret0 + ret0, _ := ret[0].([]database.Workspace) + ret1, _ := ret[1].(error) + return ret0, ret1 } // UpdateWorkspacesDormantDeletingAtByTemplateID indicates an expected call of UpdateWorkspacesDormantDeletingAtByTemplateID. diff --git a/coderd/database/migrations/000223_dormancy_notification_template.up.sql b/coderd/database/migrations/000223_dormancy_notification_template.up.sql index a9fa9423e37f6..e7241364451fe 100644 --- a/coderd/database/migrations/000223_dormancy_notification_template.up.sql +++ b/coderd/database/migrations/000223_dormancy_notification_template.up.sql @@ -1,16 +1,9 @@ --- TODO: autogenerate constants which reference the UUIDs -INSERT INTO notification_templates - (id, name, title_template, body_template, "group", actions) -VALUES - -- Workspace Marked as Dormant - ('123e4567-e89b-12d3-a456-426614174000', - 'Workspace Marked as Dormant', - E'Workspace "{{.Labels.name}}" marked as dormant', - E'Hi {{.UserName}}\n\nYour workspace **{{.Labels.name}}** has been marked as dormant.\nThe specified reason was "**{{.Labels.reason}}{{ if .Labels.initiator }} ({{ .Labels.initiator }}){{end}}**', - 'Workspace Events', - '[ - { - "label": "View workspace", - "url": "{{ base_url }}/@{{.UserName}}/{{.Labels.name}}" - }, - ]'::jsonb); +INSERT INTO notification_templates (id, name, title_template, body_template, "group", actions) +VALUES ('123e4567-e89b-12d3-a456-426614174000', 'Workspace Marked as Dormant', E'Workspace "{{.Labels.name}}" marked as dormant', + E'Hi {{.UserName}}\n\nYour workspace **{{.Labels.name}}** has been marked as dormant.\nThe specified reason was "**{{.Labels.reason}}{{ if .Labels.initiator }} ({{ .Labels.initiator }}){{end}}**', + 'Workspace Events', '[ + { + "label": "View workspace", + "url": "{{ base_url }}/@{{.UserName}}/{{.Labels.name}}" + } + ]'::jsonb); diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 75ade1dc12e5e..8d238667aad1b 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -441,7 +441,7 @@ type sqlcQuerier interface { UpdateWorkspaceProxy(ctx context.Context, arg UpdateWorkspaceProxyParams) (WorkspaceProxy, error) UpdateWorkspaceProxyDeleted(ctx context.Context, arg UpdateWorkspaceProxyDeletedParams) error UpdateWorkspaceTTL(ctx context.Context, arg UpdateWorkspaceTTLParams) error - UpdateWorkspacesDormantDeletingAtByTemplateID(ctx context.Context, arg UpdateWorkspacesDormantDeletingAtByTemplateIDParams) error + UpdateWorkspacesDormantDeletingAtByTemplateID(ctx context.Context, arg UpdateWorkspacesDormantDeletingAtByTemplateIDParams) ([]Workspace, error) UpsertAnnouncementBanners(ctx context.Context, value string) error UpsertAppSecurityKey(ctx context.Context, value string) error UpsertApplicationName(ctx context.Context, value string) error diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 95f25ee1dbd11..97c01eb96bc54 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -13860,7 +13860,7 @@ func (q *sqlQuerier) UpdateWorkspaceTTL(ctx context.Context, arg UpdateWorkspace return err } -const updateWorkspacesDormantDeletingAtByTemplateID = `-- name: UpdateWorkspacesDormantDeletingAtByTemplateID :exec +const updateWorkspacesDormantDeletingAtByTemplateID = `-- name: UpdateWorkspacesDormantDeletingAtByTemplateID :many UPDATE workspaces SET deleting_at = CASE @@ -13873,6 +13873,7 @@ WHERE template_id = $3 AND dormant_at IS NOT NULL +RETURNING id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, ttl, last_used_at, dormant_at, deleting_at, automatic_updates, favorite ` type UpdateWorkspacesDormantDeletingAtByTemplateIDParams struct { @@ -13881,9 +13882,43 @@ type UpdateWorkspacesDormantDeletingAtByTemplateIDParams struct { TemplateID uuid.UUID `db:"template_id" json:"template_id"` } -func (q *sqlQuerier) UpdateWorkspacesDormantDeletingAtByTemplateID(ctx context.Context, arg UpdateWorkspacesDormantDeletingAtByTemplateIDParams) error { - _, err := q.db.ExecContext(ctx, updateWorkspacesDormantDeletingAtByTemplateID, arg.TimeTilDormantAutodeleteMs, arg.DormantAt, arg.TemplateID) - return err +func (q *sqlQuerier) UpdateWorkspacesDormantDeletingAtByTemplateID(ctx context.Context, arg UpdateWorkspacesDormantDeletingAtByTemplateIDParams) ([]Workspace, error) { + rows, err := q.db.QueryContext(ctx, updateWorkspacesDormantDeletingAtByTemplateID, arg.TimeTilDormantAutodeleteMs, arg.DormantAt, arg.TemplateID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []Workspace + for rows.Next() { + var i Workspace + if err := rows.Scan( + &i.ID, + &i.CreatedAt, + &i.UpdatedAt, + &i.OwnerID, + &i.OrganizationID, + &i.TemplateID, + &i.Deleted, + &i.Name, + &i.AutostartSchedule, + &i.Ttl, + &i.LastUsedAt, + &i.DormantAt, + &i.DeletingAt, + &i.AutomaticUpdates, + &i.Favorite, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil } const getWorkspaceAgentScriptsByAgentIDs = `-- name: GetWorkspaceAgentScriptsByAgentIDs :many diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index ec8767e1f2be5..9b36a99b8c396 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -646,7 +646,7 @@ WHERE RETURNING workspaces.*; --- name: UpdateWorkspacesDormantDeletingAtByTemplateID :exec +-- name: UpdateWorkspacesDormantDeletingAtByTemplateID :many UPDATE workspaces SET deleting_at = CASE @@ -658,7 +658,8 @@ SET WHERE template_id = @template_id AND - dormant_at IS NOT NULL; + dormant_at IS NOT NULL +RETURNING *; -- name: UpdateTemplateWorkspacesLastUsedAt :exec UPDATE workspaces diff --git a/coderd/schedule/template.go b/coderd/schedule/template.go index a68cebd1fac93..a44ef15c31f38 100644 --- a/coderd/schedule/template.go +++ b/coderd/schedule/template.go @@ -4,11 +4,14 @@ import ( "context" "time" + "cdr.dev/slog" + "github.com/google/uuid" "golang.org/x/xerrors" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbtime" + "github.com/coder/coder/v2/coderd/notifications" "github.com/coder/coder/v2/coderd/tracing" ) @@ -152,7 +155,7 @@ type TemplateScheduleOptions struct { // scheduling options set by the template/site admin. type TemplateScheduleStore interface { Get(ctx context.Context, db database.Store, templateID uuid.UUID) (TemplateScheduleOptions, error) - Set(ctx context.Context, db database.Store, template database.Template, opts TemplateScheduleOptions) (database.Template, error) + Set(ctx context.Context, db database.Store, template database.Template, opts TemplateScheduleOptions, ntf notifications.Enqueuer, log slog.Logger) (database.Template, error) } type agplTemplateScheduleStore struct{} @@ -197,7 +200,7 @@ func (*agplTemplateScheduleStore) Get(ctx context.Context, db database.Store, te }, nil } -func (*agplTemplateScheduleStore) Set(ctx context.Context, db database.Store, tpl database.Template, opts TemplateScheduleOptions) (database.Template, error) { +func (*agplTemplateScheduleStore) Set(ctx context.Context, db database.Store, tpl database.Template, opts TemplateScheduleOptions, ntf notifications.Enqueuer, log slog.Logger) (database.Template, error) { ctx, span := tracing.StartSpan(ctx) defer span.End() diff --git a/enterprise/coderd/schedule/template.go b/enterprise/coderd/schedule/template.go index 5d5a786020241..8063cbe11c660 100644 --- a/enterprise/coderd/schedule/template.go +++ b/enterprise/coderd/schedule/template.go @@ -6,6 +6,8 @@ import ( "sync/atomic" "time" + "cdr.dev/slog" + "github.com/google/uuid" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" @@ -14,9 +16,11 @@ import ( "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbtime" + "github.com/coder/coder/v2/coderd/notifications" agpl "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/tracing" "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/enterprise/coderd/dormancy" ) // EnterpriseTemplateScheduleStore provides an agpl.TemplateScheduleStore that @@ -28,6 +32,9 @@ type EnterpriseTemplateScheduleStore struct { // Custom time.Now() function to use in tests. Defaults to dbtime.Now(). TimeNowFn func() time.Time + + // NotificationsEnqueuer handles enqueueing notifications for delivery by SMTP, webhook, etc. + NotificationsEnqueuer notifications.Enqueuer } var _ agpl.TemplateScheduleStore = &EnterpriseTemplateScheduleStore{} @@ -90,7 +97,7 @@ func (*EnterpriseTemplateScheduleStore) Get(ctx context.Context, db database.Sto } // Set implements agpl.TemplateScheduleStore. -func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.Store, tpl database.Template, opts agpl.TemplateScheduleOptions) (database.Template, error) { +func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.Store, tpl database.Template, opts agpl.TemplateScheduleOptions, ntf notifications.Enqueuer, logger slog.Logger) (database.Template, error) { ctx, span := tracing.StartSpan(ctx) defer span.End() @@ -159,7 +166,7 @@ func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.S // to ensure workspaces are being cleaned up correctly. Similarly if we are // disabling it (by passing 0), then we want to delete nullify the deleting_at // fields of all the template workspaces. - err = tx.UpdateWorkspacesDormantDeletingAtByTemplateID(ctx, database.UpdateWorkspacesDormantDeletingAtByTemplateIDParams{ + dormantWorspaces, err := tx.UpdateWorkspacesDormantDeletingAtByTemplateID(ctx, database.UpdateWorkspacesDormantDeletingAtByTemplateIDParams{ TemplateID: tpl.ID, TimeTilDormantAutodeleteMs: opts.TimeTilDormantAutoDelete.Milliseconds(), DormantAt: dormantAt, @@ -167,6 +174,19 @@ func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.S if err != nil { return xerrors.Errorf("update deleting_at of all workspaces for new time_til_dormant_autodelete %q: %w", opts.TimeTilDormantAutoDelete, err) } + for _, workspace := range dormantWorspaces { + dormancy.NotifyWorkspaceDormant( + ctx, + logger, + ntf, + dormancy.WorkspaceDormantNotification{ + Workspace: workspace, + Initiator: "system", + Reason: "template schedule update", + CreatedBy: "scheduletemplate", + }, + ) + } if opts.UpdateWorkspaceLastUsedAt != nil { err = opts.UpdateWorkspaceLastUsedAt(ctx, tx, tpl.ID, s.now()) diff --git a/enterprise/coderd/schedule/template_test.go b/enterprise/coderd/schedule/template_test.go index dd60805f00197..1df05d77d6cd9 100644 --- a/enterprise/coderd/schedule/template_test.go +++ b/enterprise/coderd/schedule/template_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + "cdr.dev/slog" "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -297,7 +298,7 @@ func TestTemplateUpdateBuildDeadlines(t *testing.T) { FailureTTL: 0, TimeTilDormant: 0, TimeTilDormantAutoDelete: 0, - }) + }, nil, slog.Logger{}) require.NoError(t, err) // Check that the workspace build has the expected deadlines. @@ -577,7 +578,7 @@ func TestTemplateUpdateBuildDeadlinesSkip(t *testing.T) { FailureTTL: 0, TimeTilDormant: 0, TimeTilDormantAutoDelete: 0, - }) + }, nil, slog.Logger{}) require.NoError(t, err) // Check each build. From 64cf76bbb7903366721ff6b24e004e4c31bf93c7 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 12 Jul 2024 13:31:14 +0000 Subject: [PATCH 06/35] Apply Danny review suggestions --- coderd/autobuild/lifecycle_executor.go | 24 +++++++-------- coderd/workspaces.go | 24 +++++++-------- enterprise/coderd/schedule/template.go | 34 ++++++++++++--------- enterprise/coderd/schedule/template_test.go | 5 +-- 4 files changed, 46 insertions(+), 41 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index 9c76932cb54e7..52d581427de2c 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -220,18 +220,6 @@ func (e *Executor) runOnce(t time.Time) Stats { }, }) - dormancy.NotifyWorkspaceDormant( - e.ctx, - e.log, - e.notificationsEnqueuer, - dormancy.WorkspaceDormantNotification{ - Workspace: ws, - Initiator: "system", - Reason: "breached the template's threshold for inactivity", - CreatedBy: "lifecycleexecutor", - }, - ) - auditLog = &auditParams{ Old: wsOld, New: ws, @@ -245,6 +233,18 @@ func (e *Executor) runOnce(t time.Time) Stats { slog.F("time_til_dormant", templateSchedule.TimeTilDormant), slog.F("since_last_used_at", time.Since(ws.LastUsedAt)), ) + + dormancy.NotifyWorkspaceDormant( + e.ctx, + e.log, + e.notificationsEnqueuer, + dormancy.WorkspaceDormantNotification{ + Workspace: ws, + Initiator: "system", + Reason: "breached the template's threshold for inactivity", + CreatedBy: "lifecycleexecutor", + }, + ) } if reason == database.BuildReasonAutodelete { diff --git a/coderd/workspaces.go b/coderd/workspaces.go index ee572e35f99a2..c0ebac132a56a 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -958,19 +958,19 @@ func (api *API) putWorkspaceDormant(rw http.ResponseWriter, r *http.Request) { initiator, err := api.Database.GetUserByID(ctx, apiKey.UserID) if err != nil { api.Logger.Warn(ctx, "failed to get the initiator by id", slog.Error(err)) - return + } else { + dormancy.NotifyWorkspaceDormant( + ctx, + api.Logger, + api.NotificationsEnqueuer, + dormancy.WorkspaceDormantNotification{ + Workspace: workspace, + Initiator: initiator.Username, + Reason: "requested by user", + CreatedBy: "api", + }, + ) } - dormancy.NotifyWorkspaceDormant( - ctx, - api.Logger, - api.NotificationsEnqueuer, - dormancy.WorkspaceDormantNotification{ - Workspace: workspace, - Initiator: initiator.Username, - Reason: "requested by user", - CreatedBy: "api", - }, - ) } data, err := api.workspaceData(ctx, []database.Workspace{workspace}) diff --git a/enterprise/coderd/schedule/template.go b/enterprise/coderd/schedule/template.go index 8063cbe11c660..37cb71625c8db 100644 --- a/enterprise/coderd/schedule/template.go +++ b/enterprise/coderd/schedule/template.go @@ -132,7 +132,10 @@ func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.S return database.Template{}, xerrors.Errorf("verify autostart requirement: %w", err) } - var template database.Template + var ( + template database.Template + dormantWorkspaces []database.Workspace + ) err = db.InTx(func(tx database.Store) error { ctx, span := tracing.StartSpanWithName(ctx, "(*schedule.EnterpriseTemplateScheduleStore).Set()-InTx()") defer span.End() @@ -166,7 +169,7 @@ func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.S // to ensure workspaces are being cleaned up correctly. Similarly if we are // disabling it (by passing 0), then we want to delete nullify the deleting_at // fields of all the template workspaces. - dormantWorspaces, err := tx.UpdateWorkspacesDormantDeletingAtByTemplateID(ctx, database.UpdateWorkspacesDormantDeletingAtByTemplateIDParams{ + dormantWorkspaces, err = tx.UpdateWorkspacesDormantDeletingAtByTemplateID(ctx, database.UpdateWorkspacesDormantDeletingAtByTemplateIDParams{ TemplateID: tpl.ID, TimeTilDormantAutodeleteMs: opts.TimeTilDormantAutoDelete.Milliseconds(), DormantAt: dormantAt, @@ -174,19 +177,6 @@ func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.S if err != nil { return xerrors.Errorf("update deleting_at of all workspaces for new time_til_dormant_autodelete %q: %w", opts.TimeTilDormantAutoDelete, err) } - for _, workspace := range dormantWorspaces { - dormancy.NotifyWorkspaceDormant( - ctx, - logger, - ntf, - dormancy.WorkspaceDormantNotification{ - Workspace: workspace, - Initiator: "system", - Reason: "template schedule update", - CreatedBy: "scheduletemplate", - }, - ) - } if opts.UpdateWorkspaceLastUsedAt != nil { err = opts.UpdateWorkspaceLastUsedAt(ctx, tx, tpl.ID, s.now()) @@ -213,6 +203,20 @@ func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.S return database.Template{}, err } + for _, workspace := range dormantWorkspaces { + dormancy.NotifyWorkspaceDormant( + ctx, + logger, + ntf, + dormancy.WorkspaceDormantNotification{ + Workspace: workspace, + Initiator: "system", + Reason: "template schedule update", + CreatedBy: "scheduletemplate", + }, + ) + } + return template, nil } diff --git a/enterprise/coderd/schedule/template_test.go b/enterprise/coderd/schedule/template_test.go index 1df05d77d6cd9..13e579c5a51d9 100644 --- a/enterprise/coderd/schedule/template_test.go +++ b/enterprise/coderd/schedule/template_test.go @@ -9,6 +9,7 @@ import ( "time" "cdr.dev/slog" + "cdr.dev/slog/sloggers/slogtest" "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -298,7 +299,7 @@ func TestTemplateUpdateBuildDeadlines(t *testing.T) { FailureTTL: 0, TimeTilDormant: 0, TimeTilDormantAutoDelete: 0, - }, nil, slog.Logger{}) + }, nil, slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug)) require.NoError(t, err) // Check that the workspace build has the expected deadlines. @@ -578,7 +579,7 @@ func TestTemplateUpdateBuildDeadlinesSkip(t *testing.T) { FailureTTL: 0, TimeTilDormant: 0, TimeTilDormantAutoDelete: 0, - }, nil, slog.Logger{}) + }, nil, slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug)) require.NoError(t, err) // Check each build. From b2f9180f4c20d50de0d3609872a9d7d5f771d8f0 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 12 Jul 2024 13:41:20 +0000 Subject: [PATCH 07/35] Fix mismatch usage --- coderd/autobuild/lifecycle_executor.go | 4 ++-- coderd/schedule/mock.go | 11 +++++++---- coderd/schedule/template.go | 4 ++-- coderd/templates.go | 4 ++-- coderd/templates_test.go | 16 +++++++++------- enterprise/coderd/schedule/template.go | 4 ++-- enterprise/coderd/schedule/template_test.go | 5 +++-- 7 files changed, 27 insertions(+), 21 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index 52d581427de2c..f06d4971eb918 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -48,7 +48,7 @@ type Stats struct { } // New returns a new wsactions executor. -func NewExecutor(ctx context.Context, db database.Store, ps pubsub.Pubsub, tss *atomic.Pointer[schedule.TemplateScheduleStore], auditor *atomic.Pointer[audit.Auditor], acs *atomic.Pointer[dbauthz.AccessControlStore], log slog.Logger, tick <-chan time.Time, ntf notifications.Enqueuer) *Executor { +func NewExecutor(ctx context.Context, db database.Store, ps pubsub.Pubsub, tss *atomic.Pointer[schedule.TemplateScheduleStore], auditor *atomic.Pointer[audit.Auditor], acs *atomic.Pointer[dbauthz.AccessControlStore], log slog.Logger, tick <-chan time.Time, enqueuer notifications.Enqueuer) *Executor { le := &Executor{ //nolint:gocritic // Autostart has a limited set of permissions. ctx: dbauthz.AsAutostart(ctx), @@ -59,7 +59,7 @@ func NewExecutor(ctx context.Context, db database.Store, ps pubsub.Pubsub, tss * log: log.Named("autobuild"), auditor: auditor, accessControlStore: acs, - notificationsEnqueuer: ntf, + notificationsEnqueuer: enqueuer, } return le } diff --git a/coderd/schedule/mock.go b/coderd/schedule/mock.go index 1fe33bb549e81..f4f6243112757 100644 --- a/coderd/schedule/mock.go +++ b/coderd/schedule/mock.go @@ -5,12 +5,15 @@ import ( "github.com/google/uuid" + "cdr.dev/slog" + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/notifications" ) type MockTemplateScheduleStore struct { GetFn func(ctx context.Context, db database.Store, templateID uuid.UUID) (TemplateScheduleOptions, error) - SetFn func(ctx context.Context, db database.Store, template database.Template, options TemplateScheduleOptions) (database.Template, error) + SetFn func(ctx context.Context, db database.Store, template database.Template, options TemplateScheduleOptions, enqueuer notifications.Enqueuer, logger slog.Logger) (database.Template, error) } var _ TemplateScheduleStore = MockTemplateScheduleStore{} @@ -23,12 +26,12 @@ func (m MockTemplateScheduleStore) Get(ctx context.Context, db database.Store, t return NewAGPLTemplateScheduleStore().Get(ctx, db, templateID) } -func (m MockTemplateScheduleStore) Set(ctx context.Context, db database.Store, template database.Template, options TemplateScheduleOptions) (database.Template, error) { +func (m MockTemplateScheduleStore) Set(ctx context.Context, db database.Store, template database.Template, options TemplateScheduleOptions, enqueuer notifications.Enqueuer, logger slog.Logger) (database.Template, error) { if m.SetFn != nil { - return m.SetFn(ctx, db, template, options) + return m.SetFn(ctx, db, template, options, enqueuer, logger) } - return NewAGPLTemplateScheduleStore().Set(ctx, db, template, options) + return NewAGPLTemplateScheduleStore().Set(ctx, db, template, options, enqueuer, logger) } type MockUserQuietHoursScheduleStore struct { diff --git a/coderd/schedule/template.go b/coderd/schedule/template.go index a44ef15c31f38..bc81d918989ec 100644 --- a/coderd/schedule/template.go +++ b/coderd/schedule/template.go @@ -155,7 +155,7 @@ type TemplateScheduleOptions struct { // scheduling options set by the template/site admin. type TemplateScheduleStore interface { Get(ctx context.Context, db database.Store, templateID uuid.UUID) (TemplateScheduleOptions, error) - Set(ctx context.Context, db database.Store, template database.Template, opts TemplateScheduleOptions, ntf notifications.Enqueuer, log slog.Logger) (database.Template, error) + Set(ctx context.Context, db database.Store, template database.Template, opts TemplateScheduleOptions, enqueuer notifications.Enqueuer, log slog.Logger) (database.Template, error) } type agplTemplateScheduleStore struct{} @@ -200,7 +200,7 @@ func (*agplTemplateScheduleStore) Get(ctx context.Context, db database.Store, te }, nil } -func (*agplTemplateScheduleStore) Set(ctx context.Context, db database.Store, tpl database.Template, opts TemplateScheduleOptions, ntf notifications.Enqueuer, log slog.Logger) (database.Template, error) { +func (*agplTemplateScheduleStore) Set(ctx context.Context, db database.Store, tpl database.Template, opts TemplateScheduleOptions, enqueuer notifications.Enqueuer, log slog.Logger) (database.Template, error) { ctx, span := tracing.StartSpan(ctx) defer span.End() diff --git a/coderd/templates.go b/coderd/templates.go index 00401c209b0a2..d15226253ca5a 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -383,7 +383,7 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque FailureTTL: failureTTL, TimeTilDormant: dormantTTL, TimeTilDormantAutoDelete: dormantAutoDeletionTTL, - }) + }, api.NotificationsEnqueuer, api.Logger) if err != nil { return xerrors.Errorf("set template schedule options: %s", err) } @@ -776,7 +776,7 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) { TimeTilDormantAutoDelete: timeTilDormantAutoDelete, UpdateWorkspaceLastUsedAt: updateWorkspaceLastUsedAt, UpdateWorkspaceDormantAt: req.UpdateWorkspaceDormantAt, - }) + }, api.NotificationsEnqueuer, api.Logger) if err != nil { return xerrors.Errorf("set template schedule options: %w", err) } diff --git a/coderd/templates_test.go b/coderd/templates_test.go index 612591120ec1a..abfd57508f11c 100644 --- a/coderd/templates_test.go +++ b/coderd/templates_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "cdr.dev/slog" "cdr.dev/slog/sloggers/slogtest" "github.com/coder/coder/v2/agent/agenttest" "github.com/coder/coder/v2/coderd/audit" @@ -18,6 +19,7 @@ import ( "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbtime" + "github.com/coder/coder/v2/coderd/notifications" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/util/ptr" @@ -190,7 +192,7 @@ func TestPostTemplateByOrganization(t *testing.T) { var setCalled int64 client := coderdtest.New(t, &coderdtest.Options{ TemplateScheduleStore: schedule.MockTemplateScheduleStore{ - SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions) (database.Template, error) { + SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions, enqueuer notifications.Enqueuer, logger slog.Logger) (database.Template, error) { atomic.AddInt64(&setCalled, 1) require.False(t, options.UserAutostartEnabled) require.False(t, options.UserAutostopEnabled) @@ -267,7 +269,7 @@ func TestPostTemplateByOrganization(t *testing.T) { var setCalled int64 client := coderdtest.New(t, &coderdtest.Options{ TemplateScheduleStore: schedule.MockTemplateScheduleStore{ - SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions) (database.Template, error) { + SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions, enqueuer notifications.Enqueuer, logger slog.Logger) (database.Template, error) { atomic.AddInt64(&setCalled, 1) assert.Zero(t, options.AutostopRequirement.DaysOfWeek) assert.Zero(t, options.AutostopRequirement.Weeks) @@ -317,7 +319,7 @@ func TestPostTemplateByOrganization(t *testing.T) { var setCalled int64 client := coderdtest.New(t, &coderdtest.Options{ TemplateScheduleStore: schedule.MockTemplateScheduleStore{ - SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions) (database.Template, error) { + SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions, enqueuer notifications.Enqueuer, logger slog.Logger) (database.Template, error) { atomic.AddInt64(&setCalled, 1) assert.EqualValues(t, 0b00110000, options.AutostopRequirement.DaysOfWeek) assert.EqualValues(t, 2, options.AutostopRequirement.Weeks) @@ -755,7 +757,7 @@ func TestPatchTemplateMeta(t *testing.T) { var setCalled int64 client := coderdtest.New(t, &coderdtest.Options{ TemplateScheduleStore: schedule.MockTemplateScheduleStore{ - SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions) (database.Template, error) { + SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions, enqueuer notifications.Enqueuer, logger slog.Logger) (database.Template, error) { if atomic.AddInt64(&setCalled, 1) == 2 { require.Equal(t, failureTTL, options.FailureTTL) require.Equal(t, inactivityTTL, options.TimeTilDormant) @@ -850,7 +852,7 @@ func TestPatchTemplateMeta(t *testing.T) { allowAutostop.Store(true) client := coderdtest.New(t, &coderdtest.Options{ TemplateScheduleStore: schedule.MockTemplateScheduleStore{ - SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions) (database.Template, error) { + SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions, enqueuer notifications.Enqueuer, logger slog.Logger) (database.Template, error) { atomic.AddInt64(&setCalled, 1) assert.Equal(t, allowAutostart.Load(), options.UserAutostartEnabled) assert.Equal(t, allowAutostop.Load(), options.UserAutostopEnabled) @@ -1020,7 +1022,7 @@ func TestPatchTemplateMeta(t *testing.T) { var setCalled int64 client := coderdtest.New(t, &coderdtest.Options{ TemplateScheduleStore: schedule.MockTemplateScheduleStore{ - SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions) (database.Template, error) { + SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions, enqueuer notifications.Enqueuer, logger slog.Logger) (database.Template, error) { if atomic.AddInt64(&setCalled, 1) == 2 { assert.EqualValues(t, 0b0110000, options.AutostopRequirement.DaysOfWeek) assert.EqualValues(t, 2, options.AutostopRequirement.Weeks) @@ -1091,7 +1093,7 @@ func TestPatchTemplateMeta(t *testing.T) { var setCalled int64 client := coderdtest.New(t, &coderdtest.Options{ TemplateScheduleStore: schedule.MockTemplateScheduleStore{ - SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions) (database.Template, error) { + SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions, enqueuer notifications.Enqueuer, logger slog.Logger) (database.Template, error) { if atomic.AddInt64(&setCalled, 1) == 2 { assert.EqualValues(t, 0, options.AutostopRequirement.DaysOfWeek) assert.EqualValues(t, 1, options.AutostopRequirement.Weeks) diff --git a/enterprise/coderd/schedule/template.go b/enterprise/coderd/schedule/template.go index 37cb71625c8db..b2076eec78e63 100644 --- a/enterprise/coderd/schedule/template.go +++ b/enterprise/coderd/schedule/template.go @@ -97,7 +97,7 @@ func (*EnterpriseTemplateScheduleStore) Get(ctx context.Context, db database.Sto } // Set implements agpl.TemplateScheduleStore. -func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.Store, tpl database.Template, opts agpl.TemplateScheduleOptions, ntf notifications.Enqueuer, logger slog.Logger) (database.Template, error) { +func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.Store, tpl database.Template, opts agpl.TemplateScheduleOptions, enqueuer notifications.Enqueuer, logger slog.Logger) (database.Template, error) { ctx, span := tracing.StartSpan(ctx) defer span.End() @@ -207,7 +207,7 @@ func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.S dormancy.NotifyWorkspaceDormant( ctx, logger, - ntf, + enqueuer, dormancy.WorkspaceDormantNotification{ Workspace: workspace, Initiator: "system", diff --git a/enterprise/coderd/schedule/template_test.go b/enterprise/coderd/schedule/template_test.go index 13e579c5a51d9..77e3ed8f08eed 100644 --- a/enterprise/coderd/schedule/template_test.go +++ b/enterprise/coderd/schedule/template_test.go @@ -8,12 +8,13 @@ import ( "testing" "time" - "cdr.dev/slog" - "cdr.dev/slog/sloggers/slogtest" "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "cdr.dev/slog" + "cdr.dev/slog/sloggers/slogtest" + "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbgen" "github.com/coder/coder/v2/coderd/database/dbtestutil" From a445c4c33760f38113cfe2acfe673eeaba16ae3b Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 12 Jul 2024 13:42:17 +0000 Subject: [PATCH 08/35] Resolve migration conflict --- ...te.down.sql => 000226_dormancy_notification_template.down.sql} | 0 ...mplate.up.sql => 000226_dormancy_notification_template.up.sql} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename coderd/database/migrations/{000223_dormancy_notification_template.down.sql => 000226_dormancy_notification_template.down.sql} (100%) rename coderd/database/migrations/{000223_dormancy_notification_template.up.sql => 000226_dormancy_notification_template.up.sql} (100%) diff --git a/coderd/database/migrations/000223_dormancy_notification_template.down.sql b/coderd/database/migrations/000226_dormancy_notification_template.down.sql similarity index 100% rename from coderd/database/migrations/000223_dormancy_notification_template.down.sql rename to coderd/database/migrations/000226_dormancy_notification_template.down.sql diff --git a/coderd/database/migrations/000223_dormancy_notification_template.up.sql b/coderd/database/migrations/000226_dormancy_notification_template.up.sql similarity index 100% rename from coderd/database/migrations/000223_dormancy_notification_template.up.sql rename to coderd/database/migrations/000226_dormancy_notification_template.up.sql From ee7d5429c5b44fb0947b96a929c61e1033c03577 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 12 Jul 2024 17:33:28 +0000 Subject: [PATCH 09/35] Return error instead of receiving the logger --- coderd/autobuild/lifecycle_executor.go | 7 +++++-- coderd/workspaces.go | 6 ++++-- enterprise/coderd/dormancy/notifications.go | 13 ++++++------- enterprise/coderd/schedule/template.go | 6 ++++-- 4 files changed, 19 insertions(+), 13 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index f06d4971eb918..99e540aca5c01 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -234,9 +234,8 @@ func (e *Executor) runOnce(t time.Time) Stats { slog.F("since_last_used_at", time.Since(ws.LastUsedAt)), ) - dormancy.NotifyWorkspaceDormant( + _, err = dormancy.NotifyWorkspaceDormant( e.ctx, - e.log, e.notificationsEnqueuer, dormancy.WorkspaceDormantNotification{ Workspace: ws, @@ -245,6 +244,10 @@ func (e *Executor) runOnce(t time.Time) Stats { CreatedBy: "lifecycleexecutor", }, ) + + if err != nil { + log.Warn(e.ctx, "failed to notify of workspace marked as dormant", slog.Error(err)) + } } if reason == database.BuildReasonAutodelete { diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 1fe77d7313616..05e7e12c7ab9b 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -957,9 +957,8 @@ func (api *API) putWorkspaceDormant(rw http.ResponseWriter, r *http.Request) { if err != nil { api.Logger.Warn(ctx, "failed to get the initiator by id", slog.Error(err)) } else { - dormancy.NotifyWorkspaceDormant( + _, err = dormancy.NotifyWorkspaceDormant( ctx, - api.Logger, api.NotificationsEnqueuer, dormancy.WorkspaceDormantNotification{ Workspace: workspace, @@ -968,6 +967,9 @@ func (api *API) putWorkspaceDormant(rw http.ResponseWriter, r *http.Request) { CreatedBy: "api", }, ) + if err != nil { + api.Logger.Warn(ctx, "failed to notify of workspace marked as dormant", slog.Error(err)) + } } } diff --git a/enterprise/coderd/dormancy/notifications.go b/enterprise/coderd/dormancy/notifications.go index e557ae1eb33c5..c5c446d2b54a6 100644 --- a/enterprise/coderd/dormancy/notifications.go +++ b/enterprise/coderd/dormancy/notifications.go @@ -3,7 +3,8 @@ package dormancy import ( "context" - "cdr.dev/slog" + "github.com/google/uuid" + "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/notifications" ) @@ -17,16 +18,16 @@ type WorkspaceDormantNotification struct { func NotifyWorkspaceDormant( ctx context.Context, - logger slog.Logger, + enqueuer notifications.Enqueuer, notification WorkspaceDormantNotification, -) { +) (id *uuid.UUID, err error) { labels := map[string]string{ "name": notification.Workspace.Name, "initiator": notification.Initiator, "reason": notification.Reason, } - if _, err := enqueuer.Enqueue( + return enqueuer.Enqueue( ctx, notification.Workspace.OwnerID, notifications.TemplateWorkspaceDormant, @@ -37,7 +38,5 @@ func NotifyWorkspaceDormant( notification.Workspace.OwnerID, notification.Workspace.TemplateID, notification.Workspace.OrganizationID, - ); err != nil { - logger.Warn(ctx, "failed to notify of workspace marked as dormant", slog.Error(err)) - } + ) } diff --git a/enterprise/coderd/schedule/template.go b/enterprise/coderd/schedule/template.go index b2076eec78e63..7a806b32a50cd 100644 --- a/enterprise/coderd/schedule/template.go +++ b/enterprise/coderd/schedule/template.go @@ -204,9 +204,8 @@ func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.S } for _, workspace := range dormantWorkspaces { - dormancy.NotifyWorkspaceDormant( + _, err = dormancy.NotifyWorkspaceDormant( ctx, - logger, enqueuer, dormancy.WorkspaceDormantNotification{ Workspace: workspace, @@ -215,6 +214,9 @@ func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.S CreatedBy: "scheduletemplate", }, ) + if err != nil { + logger.Warn(ctx, "failed to notify of workspace marked as dormant", slog.Error(err)) + } } return template, nil From f6db3c7cd5116365c2c861af508112727ee7dc99 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Fri, 12 Jul 2024 17:35:39 +0000 Subject: [PATCH 10/35] Improve verbiage --- coderd/workspaces.go | 2 +- enterprise/coderd/schedule/template.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 05e7e12c7ab9b..f4efbd79f440d 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -955,7 +955,7 @@ func (api *API) putWorkspaceDormant(rw http.ResponseWriter, r *http.Request) { if req.Dormant && apiKey.UserID != workspace.OwnerID { initiator, err := api.Database.GetUserByID(ctx, apiKey.UserID) if err != nil { - api.Logger.Warn(ctx, "failed to get the initiator by id", slog.Error(err)) + api.Logger.Warn(ctx, "failed to fetch the user that marked the workspace as dormant", slog.Error(err)) } else { _, err = dormancy.NotifyWorkspaceDormant( ctx, diff --git a/enterprise/coderd/schedule/template.go b/enterprise/coderd/schedule/template.go index 7a806b32a50cd..fb4f40392cdd3 100644 --- a/enterprise/coderd/schedule/template.go +++ b/enterprise/coderd/schedule/template.go @@ -210,7 +210,7 @@ func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.S dormancy.WorkspaceDormantNotification{ Workspace: workspace, Initiator: "system", - Reason: "template schedule update", + Reason: "template updated to new dormancy policy", CreatedBy: "scheduletemplate", }, ) From 467a797edcd927d66c47f0ef44b52f0bee22f05e Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Tue, 16 Jul 2024 17:33:54 +0200 Subject: [PATCH 11/35] Possible implementation simplification Signed-off-by: Danny Kopping --- ...0226_dormancy_notification_template.up.sql | 9 --- ...7_dormancy_notification_template.down.sql} | 0 ...0227_dormancy_notification_template.up.sql | 13 ++++ coderd/schedule/mock.go | 11 ++-- coderd/schedule/template.go | 7 +-- coderd/templates.go | 4 +- coderd/templates_test.go | 17 +++--- coderd/workspaces_test.go | 2 +- enterprise/coderd/coderd.go | 3 +- enterprise/coderd/schedule/template.go | 14 +++-- enterprise/coderd/schedule/template_test.go | 18 +++--- enterprise/coderd/templates_test.go | 9 ++- enterprise/coderd/workspaces_test.go | 59 ++++++++++++------- 13 files changed, 97 insertions(+), 69 deletions(-) delete mode 100644 coderd/database/migrations/000226_dormancy_notification_template.up.sql rename coderd/database/migrations/{000226_dormancy_notification_template.down.sql => 000227_dormancy_notification_template.down.sql} (100%) create mode 100644 coderd/database/migrations/000227_dormancy_notification_template.up.sql diff --git a/coderd/database/migrations/000226_dormancy_notification_template.up.sql b/coderd/database/migrations/000226_dormancy_notification_template.up.sql deleted file mode 100644 index e7241364451fe..0000000000000 --- a/coderd/database/migrations/000226_dormancy_notification_template.up.sql +++ /dev/null @@ -1,9 +0,0 @@ -INSERT INTO notification_templates (id, name, title_template, body_template, "group", actions) -VALUES ('123e4567-e89b-12d3-a456-426614174000', 'Workspace Marked as Dormant', E'Workspace "{{.Labels.name}}" marked as dormant', - E'Hi {{.UserName}}\n\nYour workspace **{{.Labels.name}}** has been marked as dormant.\nThe specified reason was "**{{.Labels.reason}}{{ if .Labels.initiator }} ({{ .Labels.initiator }}){{end}}**', - 'Workspace Events', '[ - { - "label": "View workspace", - "url": "{{ base_url }}/@{{.UserName}}/{{.Labels.name}}" - } - ]'::jsonb); diff --git a/coderd/database/migrations/000226_dormancy_notification_template.down.sql b/coderd/database/migrations/000227_dormancy_notification_template.down.sql similarity index 100% rename from coderd/database/migrations/000226_dormancy_notification_template.down.sql rename to coderd/database/migrations/000227_dormancy_notification_template.down.sql diff --git a/coderd/database/migrations/000227_dormancy_notification_template.up.sql b/coderd/database/migrations/000227_dormancy_notification_template.up.sql new file mode 100644 index 0000000000000..48d7e13b1c675 --- /dev/null +++ b/coderd/database/migrations/000227_dormancy_notification_template.up.sql @@ -0,0 +1,13 @@ +INSERT INTO notification_templates (id, name, title_template, body_template, "group", actions) +VALUES ('123e4567-e89b-12d3-a456-426614174000', 'Workspace Marked as Dormant', E'Workspace "{{.Labels.name}}" marked as dormant', + E'Hi {{.UserName}}\n\n' || + E'Your workspace **{{.Labels.name}}** has been marked as **dormant**.\n' || + E'The specified reason was "**{{.Labels.reason}}{{ if .Labels.initiator }} (initiated by: {{ .Labels.initiator }}){{end}}**\n\n' || + E'Dormancy refers to a workspace being unused for a defined length of time, and after it exceeds {{.Labels.dormancyHours}} hours of dormancy it will be deleted.\n' || + E'To prevent your workspace from being deleted, simply use it as normal.', + 'Workspace Events', '[ + { + "label": "View workspace", + "url": "{{ base_url }}/@{{.UserName}}/{{.Labels.name}}" + } + ]'::jsonb); diff --git a/coderd/schedule/mock.go b/coderd/schedule/mock.go index f4f6243112757..c38fddb8acedb 100644 --- a/coderd/schedule/mock.go +++ b/coderd/schedule/mock.go @@ -5,15 +5,12 @@ import ( "github.com/google/uuid" - "cdr.dev/slog" - "github.com/coder/coder/v2/coderd/database" - "github.com/coder/coder/v2/coderd/notifications" ) type MockTemplateScheduleStore struct { GetFn func(ctx context.Context, db database.Store, templateID uuid.UUID) (TemplateScheduleOptions, error) - SetFn func(ctx context.Context, db database.Store, template database.Template, options TemplateScheduleOptions, enqueuer notifications.Enqueuer, logger slog.Logger) (database.Template, error) + SetFn func(ctx context.Context, db database.Store, template database.Template, options TemplateScheduleOptions) (database.Template, error) } var _ TemplateScheduleStore = MockTemplateScheduleStore{} @@ -26,12 +23,12 @@ func (m MockTemplateScheduleStore) Get(ctx context.Context, db database.Store, t return NewAGPLTemplateScheduleStore().Get(ctx, db, templateID) } -func (m MockTemplateScheduleStore) Set(ctx context.Context, db database.Store, template database.Template, options TemplateScheduleOptions, enqueuer notifications.Enqueuer, logger slog.Logger) (database.Template, error) { +func (m MockTemplateScheduleStore) Set(ctx context.Context, db database.Store, template database.Template, opts TemplateScheduleOptions) (database.Template, error) { if m.SetFn != nil { - return m.SetFn(ctx, db, template, options, enqueuer, logger) + return m.SetFn(ctx, db, template, opts) } - return NewAGPLTemplateScheduleStore().Set(ctx, db, template, options, enqueuer, logger) + return NewAGPLTemplateScheduleStore().Set(ctx, db, template, opts) } type MockUserQuietHoursScheduleStore struct { diff --git a/coderd/schedule/template.go b/coderd/schedule/template.go index bc81d918989ec..a68cebd1fac93 100644 --- a/coderd/schedule/template.go +++ b/coderd/schedule/template.go @@ -4,14 +4,11 @@ import ( "context" "time" - "cdr.dev/slog" - "github.com/google/uuid" "golang.org/x/xerrors" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbtime" - "github.com/coder/coder/v2/coderd/notifications" "github.com/coder/coder/v2/coderd/tracing" ) @@ -155,7 +152,7 @@ type TemplateScheduleOptions struct { // scheduling options set by the template/site admin. type TemplateScheduleStore interface { Get(ctx context.Context, db database.Store, templateID uuid.UUID) (TemplateScheduleOptions, error) - Set(ctx context.Context, db database.Store, template database.Template, opts TemplateScheduleOptions, enqueuer notifications.Enqueuer, log slog.Logger) (database.Template, error) + Set(ctx context.Context, db database.Store, template database.Template, opts TemplateScheduleOptions) (database.Template, error) } type agplTemplateScheduleStore struct{} @@ -200,7 +197,7 @@ func (*agplTemplateScheduleStore) Get(ctx context.Context, db database.Store, te }, nil } -func (*agplTemplateScheduleStore) Set(ctx context.Context, db database.Store, tpl database.Template, opts TemplateScheduleOptions, enqueuer notifications.Enqueuer, log slog.Logger) (database.Template, error) { +func (*agplTemplateScheduleStore) Set(ctx context.Context, db database.Store, tpl database.Template, opts TemplateScheduleOptions) (database.Template, error) { ctx, span := tracing.StartSpan(ctx) defer span.End() diff --git a/coderd/templates.go b/coderd/templates.go index 7d8d02ac5abfa..5bf32871dcbc1 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -383,7 +383,7 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque FailureTTL: failureTTL, TimeTilDormant: dormantTTL, TimeTilDormantAutoDelete: dormantAutoDeletionTTL, - }, api.NotificationsEnqueuer, api.Logger) + }) if err != nil { return xerrors.Errorf("set template schedule options: %s", err) } @@ -776,7 +776,7 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) { TimeTilDormantAutoDelete: timeTilDormantAutoDelete, UpdateWorkspaceLastUsedAt: updateWorkspaceLastUsedAt, UpdateWorkspaceDormantAt: req.UpdateWorkspaceDormantAt, - }, api.NotificationsEnqueuer, api.Logger) + }) if err != nil { return xerrors.Errorf("set template schedule options: %w", err) } diff --git a/coderd/templates_test.go b/coderd/templates_test.go index a5b120f367cfe..ed4486c783c4b 100644 --- a/coderd/templates_test.go +++ b/coderd/templates_test.go @@ -11,15 +11,14 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "cdr.dev/slog" "cdr.dev/slog/sloggers/slogtest" + "github.com/coder/coder/v2/agent/agenttest" "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbtime" - "github.com/coder/coder/v2/coderd/notifications" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/util/ptr" @@ -192,7 +191,7 @@ func TestPostTemplateByOrganization(t *testing.T) { var setCalled int64 client := coderdtest.New(t, &coderdtest.Options{ TemplateScheduleStore: schedule.MockTemplateScheduleStore{ - SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions, enqueuer notifications.Enqueuer, logger slog.Logger) (database.Template, error) { + SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions) (database.Template, error) { atomic.AddInt64(&setCalled, 1) require.False(t, options.UserAutostartEnabled) require.False(t, options.UserAutostopEnabled) @@ -269,7 +268,7 @@ func TestPostTemplateByOrganization(t *testing.T) { var setCalled int64 client := coderdtest.New(t, &coderdtest.Options{ TemplateScheduleStore: schedule.MockTemplateScheduleStore{ - SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions, enqueuer notifications.Enqueuer, logger slog.Logger) (database.Template, error) { + SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions) (database.Template, error) { atomic.AddInt64(&setCalled, 1) assert.Zero(t, options.AutostopRequirement.DaysOfWeek) assert.Zero(t, options.AutostopRequirement.Weeks) @@ -319,7 +318,7 @@ func TestPostTemplateByOrganization(t *testing.T) { var setCalled int64 client := coderdtest.New(t, &coderdtest.Options{ TemplateScheduleStore: schedule.MockTemplateScheduleStore{ - SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions, enqueuer notifications.Enqueuer, logger slog.Logger) (database.Template, error) { + SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions) (database.Template, error) { atomic.AddInt64(&setCalled, 1) assert.EqualValues(t, 0b00110000, options.AutostopRequirement.DaysOfWeek) assert.EqualValues(t, 2, options.AutostopRequirement.Weeks) @@ -759,7 +758,7 @@ func TestPatchTemplateMeta(t *testing.T) { var setCalled int64 client := coderdtest.New(t, &coderdtest.Options{ TemplateScheduleStore: schedule.MockTemplateScheduleStore{ - SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions, enqueuer notifications.Enqueuer, logger slog.Logger) (database.Template, error) { + SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions) (database.Template, error) { if atomic.AddInt64(&setCalled, 1) == 2 { require.Equal(t, failureTTL, options.FailureTTL) require.Equal(t, inactivityTTL, options.TimeTilDormant) @@ -854,7 +853,7 @@ func TestPatchTemplateMeta(t *testing.T) { allowAutostop.Store(true) client := coderdtest.New(t, &coderdtest.Options{ TemplateScheduleStore: schedule.MockTemplateScheduleStore{ - SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions, enqueuer notifications.Enqueuer, logger slog.Logger) (database.Template, error) { + SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions) (database.Template, error) { atomic.AddInt64(&setCalled, 1) assert.Equal(t, allowAutostart.Load(), options.UserAutostartEnabled) assert.Equal(t, allowAutostop.Load(), options.UserAutostopEnabled) @@ -1024,7 +1023,7 @@ func TestPatchTemplateMeta(t *testing.T) { var setCalled int64 client := coderdtest.New(t, &coderdtest.Options{ TemplateScheduleStore: schedule.MockTemplateScheduleStore{ - SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions, enqueuer notifications.Enqueuer, logger slog.Logger) (database.Template, error) { + SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions) (database.Template, error) { if atomic.AddInt64(&setCalled, 1) == 2 { assert.EqualValues(t, 0b0110000, options.AutostopRequirement.DaysOfWeek) assert.EqualValues(t, 2, options.AutostopRequirement.Weeks) @@ -1095,7 +1094,7 @@ func TestPatchTemplateMeta(t *testing.T) { var setCalled int64 client := coderdtest.New(t, &coderdtest.Options{ TemplateScheduleStore: schedule.MockTemplateScheduleStore{ - SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions, enqueuer notifications.Enqueuer, logger slog.Logger) (database.Template, error) { + SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions) (database.Template, error) { if atomic.AddInt64(&setCalled, 1) == 2 { assert.EqualValues(t, 0, options.AutostopRequirement.DaysOfWeek) assert.EqualValues(t, 1, options.AutostopRequirement.Weeks) diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 24d8fddff527f..436780901fab7 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -3547,7 +3547,7 @@ func TestNotifications(t *testing.T) { require.Contains(t, notifyEnq.Sent[0].Targets, workspace.ID) require.Contains(t, notifyEnq.Sent[0].Targets, workspace.OrganizationID) require.Contains(t, notifyEnq.Sent[0].Targets, workspace.OwnerID) - require.Equal(t, notifyEnq.Sent[0].Labels["initiatedBy"], member.Username) + require.Equal(t, notifyEnq.Sent[0].Labels["initiator"], member.Username) }) t.Run("InitiatorIsOwner", func(t *testing.T) { diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index 89cc82b73f68e..8911a58fc3b4b 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -26,6 +26,7 @@ import ( "github.com/prometheus/client_golang/prometheus" "cdr.dev/slog" + "github.com/coder/coder/v2/coderd" agplaudit "github.com/coder/coder/v2/coderd/audit" agpldbauthz "github.com/coder/coder/v2/coderd/database/dbauthz" @@ -632,7 +633,7 @@ func (api *API) updateEntitlements(ctx context.Context) error { if initial, changed, enabled := featureChanged(codersdk.FeatureAdvancedTemplateScheduling); shouldUpdate(initial, changed, enabled) { if enabled { - templateStore := schedule.NewEnterpriseTemplateScheduleStore(api.AGPL.UserQuietHoursScheduleStore) + templateStore := schedule.NewEnterpriseTemplateScheduleStore(api.AGPL.UserQuietHoursScheduleStore, api.NotificationsEnqueuer, api.Logger.Named("template.schedule-store")) templateStoreInterface := agplschedule.TemplateScheduleStore(templateStore) api.AGPL.TemplateScheduleStore.Store(&templateStoreInterface) diff --git a/enterprise/coderd/schedule/template.go b/enterprise/coderd/schedule/template.go index fb4f40392cdd3..4cdf3a63cc206 100644 --- a/enterprise/coderd/schedule/template.go +++ b/enterprise/coderd/schedule/template.go @@ -33,15 +33,17 @@ type EnterpriseTemplateScheduleStore struct { // Custom time.Now() function to use in tests. Defaults to dbtime.Now(). TimeNowFn func() time.Time - // NotificationsEnqueuer handles enqueueing notifications for delivery by SMTP, webhook, etc. - NotificationsEnqueuer notifications.Enqueuer + enqueuer notifications.Enqueuer + logger slog.Logger } var _ agpl.TemplateScheduleStore = &EnterpriseTemplateScheduleStore{} -func NewEnterpriseTemplateScheduleStore(userQuietHoursStore *atomic.Pointer[agpl.UserQuietHoursScheduleStore]) *EnterpriseTemplateScheduleStore { +func NewEnterpriseTemplateScheduleStore(userQuietHoursStore *atomic.Pointer[agpl.UserQuietHoursScheduleStore], enqueuer notifications.Enqueuer, logger slog.Logger) *EnterpriseTemplateScheduleStore { return &EnterpriseTemplateScheduleStore{ UserQuietHoursScheduleStore: userQuietHoursStore, + enqueuer: enqueuer, + logger: logger, } } @@ -97,7 +99,7 @@ func (*EnterpriseTemplateScheduleStore) Get(ctx context.Context, db database.Sto } // Set implements agpl.TemplateScheduleStore. -func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.Store, tpl database.Template, opts agpl.TemplateScheduleOptions, enqueuer notifications.Enqueuer, logger slog.Logger) (database.Template, error) { +func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.Store, tpl database.Template, opts agpl.TemplateScheduleOptions) (database.Template, error) { ctx, span := tracing.StartSpan(ctx) defer span.End() @@ -206,7 +208,7 @@ func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.S for _, workspace := range dormantWorkspaces { _, err = dormancy.NotifyWorkspaceDormant( ctx, - enqueuer, + s.enqueuer, dormancy.WorkspaceDormantNotification{ Workspace: workspace, Initiator: "system", @@ -215,7 +217,7 @@ func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.S }, ) if err != nil { - logger.Warn(ctx, "failed to notify of workspace marked as dormant", slog.Error(err)) + s.logger.Warn(ctx, "failed to notify of workspace marked as dormant", slog.Error(err)) } } diff --git a/enterprise/coderd/schedule/template_test.go b/enterprise/coderd/schedule/template_test.go index 77e3ed8f08eed..7198d99a84581 100644 --- a/enterprise/coderd/schedule/template_test.go +++ b/enterprise/coderd/schedule/template_test.go @@ -8,16 +8,16 @@ import ( "testing" "time" + "cdr.dev/slog" + "cdr.dev/slog/sloggers/slogtest" "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "cdr.dev/slog" - "cdr.dev/slog/sloggers/slogtest" - "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/notifications" agplschedule "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/cryptorand" @@ -273,13 +273,15 @@ func TestTemplateUpdateBuildDeadlines(t *testing.T) { wsBuild, err = db.GetWorkspaceBuildByID(ctx, wsBuild.ID) require.NoError(t, err) + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) + userQuietHoursStore, err := schedule.NewEnterpriseUserQuietHoursScheduleStore(userQuietHoursSchedule, true) require.NoError(t, err) userQuietHoursStorePtr := &atomic.Pointer[agplschedule.UserQuietHoursScheduleStore]{} userQuietHoursStorePtr.Store(&userQuietHoursStore) // Set the template policy. - templateScheduleStore := schedule.NewEnterpriseTemplateScheduleStore(userQuietHoursStorePtr) + templateScheduleStore := schedule.NewEnterpriseTemplateScheduleStore(userQuietHoursStorePtr, notifications.NewNoopEnqueuer(), logger) templateScheduleStore.TimeNowFn = func() time.Time { return c.now } @@ -300,7 +302,7 @@ func TestTemplateUpdateBuildDeadlines(t *testing.T) { FailureTTL: 0, TimeTilDormant: 0, TimeTilDormantAutoDelete: 0, - }, nil, slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug)) + }) require.NoError(t, err) // Check that the workspace build has the expected deadlines. @@ -558,13 +560,15 @@ func TestTemplateUpdateBuildDeadlinesSkip(t *testing.T) { require.NoError(t, err) } + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) + userQuietHoursStore, err := schedule.NewEnterpriseUserQuietHoursScheduleStore(userQuietHoursSchedule, true) require.NoError(t, err) userQuietHoursStorePtr := &atomic.Pointer[agplschedule.UserQuietHoursScheduleStore]{} userQuietHoursStorePtr.Store(&userQuietHoursStore) // Set the template policy. - templateScheduleStore := schedule.NewEnterpriseTemplateScheduleStore(userQuietHoursStorePtr) + templateScheduleStore := schedule.NewEnterpriseTemplateScheduleStore(userQuietHoursStorePtr, notifications.NewNoopEnqueuer(), logger) templateScheduleStore.TimeNowFn = func() time.Time { return now } @@ -580,7 +584,7 @@ func TestTemplateUpdateBuildDeadlinesSkip(t *testing.T) { FailureTTL: 0, TimeTilDormant: 0, TimeTilDormantAutoDelete: 0, - }, nil, slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug)) + }) require.NoError(t, err) // Check each build. diff --git a/enterprise/coderd/templates_test.go b/enterprise/coderd/templates_test.go index 5d44023af86b9..2b603dfc6c163 100644 --- a/enterprise/coderd/templates_test.go +++ b/enterprise/coderd/templates_test.go @@ -7,6 +7,8 @@ import ( "testing" "time" + "cdr.dev/slog" + "cdr.dev/slog/sloggers/slogtest" "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -14,6 +16,7 @@ import ( "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/notifications" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/codersdk" @@ -29,6 +32,8 @@ import ( func TestTemplates(t *testing.T) { t.Parallel() + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) + t.Run("Deprecated", func(t *testing.T) { t.Parallel() @@ -637,7 +642,7 @@ func TestTemplates(t *testing.T) { client, user := coderdenttest.New(t, &coderdenttest.Options{ Options: &coderdtest.Options{ IncludeProvisionerDaemon: true, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore()), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{ @@ -687,7 +692,7 @@ func TestTemplates(t *testing.T) { owner, first := coderdenttest.New(t, &coderdenttest.Options{ Options: &coderdtest.Options{ IncludeProvisionerDaemon: true, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore()), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{ diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index 9cb86f55ba55f..6c8530e401062 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "cdr.dev/slog" "github.com/stretchr/testify/require" "cdr.dev/slog/sloggers/slogtest" @@ -17,6 +18,7 @@ import ( "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbfake" "github.com/coder/coder/v2/coderd/database/dbtestutil" + "github.com/coder/coder/v2/coderd/notifications" "github.com/coder/coder/v2/coderd/rbac" agplschedule "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/schedule/cron" @@ -118,7 +120,7 @@ func TestWorkspaceAutobuild(t *testing.T) { AutobuildTicker: ticker, IncludeProvisionerDaemon: true, AutobuildStats: statCh, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore()), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, @@ -165,7 +167,7 @@ func TestWorkspaceAutobuild(t *testing.T) { AutobuildTicker: ticker, IncludeProvisionerDaemon: true, AutobuildStats: statCh, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore()), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, @@ -211,7 +213,7 @@ func TestWorkspaceAutobuild(t *testing.T) { AutobuildTicker: ticker, IncludeProvisionerDaemon: true, AutobuildStats: statCh, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore()), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, @@ -249,11 +251,13 @@ func TestWorkspaceAutobuild(t *testing.T) { auditRecorder = audit.NewMock() ) + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) + client, db, user := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ Options: &coderdtest.Options{ AutobuildTicker: ticker, AutobuildStats: statCh, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore()), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), Auditor: auditRecorder, }, LicenseOptions: &coderdenttest.LicenseOptions{ @@ -342,12 +346,13 @@ func TestWorkspaceAutobuild(t *testing.T) { // another connection from within a transaction. sdb.SetMaxOpenConns(maxConns) auditor := entaudit.NewAuditor(db, entaudit.DefaultFilter, backends.NewPostgres(db, true)) + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) client, user := coderdenttest.New(t, &coderdenttest.Options{ Options: &coderdtest.Options{ AutobuildTicker: ticker, AutobuildStats: statCh, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore()), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), Database: db, Pubsub: pubsub, Auditor: auditor, @@ -399,12 +404,13 @@ func TestWorkspaceAutobuild(t *testing.T) { inactiveTTL = time.Minute ) + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) client, user := coderdenttest.New(t, &coderdenttest.Options{ Options: &coderdtest.Options{ AutobuildTicker: ticker, IncludeProvisionerDaemon: true, AutobuildStats: statCh, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore()), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, @@ -441,12 +447,13 @@ func TestWorkspaceAutobuild(t *testing.T) { autoDeleteTTL = time.Minute ) + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) client, user := coderdenttest.New(t, &coderdenttest.Options{ Options: &coderdtest.Options{ AutobuildTicker: ticker, IncludeProvisionerDaemon: true, AutobuildStats: statCh, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore()), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, @@ -483,12 +490,13 @@ func TestWorkspaceAutobuild(t *testing.T) { inactiveTTL = time.Minute ) + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) client, user := coderdenttest.New(t, &coderdenttest.Options{ Options: &coderdtest.Options{ AutobuildTicker: ticker, IncludeProvisionerDaemon: true, AutobuildStats: statCh, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore()), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, @@ -536,12 +544,13 @@ func TestWorkspaceAutobuild(t *testing.T) { transitionTTL = time.Minute ) + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) client, user := coderdenttest.New(t, &coderdenttest.Options{ Options: &coderdtest.Options{ AutobuildTicker: ticker, IncludeProvisionerDaemon: true, AutobuildStats: statCh, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore()), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, @@ -607,12 +616,13 @@ func TestWorkspaceAutobuild(t *testing.T) { dormantTTL = time.Minute ) + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) client, user := coderdenttest.New(t, &coderdenttest.Options{ Options: &coderdtest.Options{ AutobuildTicker: ticker, IncludeProvisionerDaemon: true, AutobuildStats: statCh, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore()), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, @@ -669,12 +679,14 @@ func TestWorkspaceAutobuild(t *testing.T) { statsCh = make(chan autobuild.Stats) inactiveTTL = time.Minute ) + + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) client, user := coderdenttest.New(t, &coderdenttest.Options{ Options: &coderdtest.Options{ AutobuildTicker: tickCh, IncludeProvisionerDaemon: true, AutobuildStats: statsCh, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore()), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, @@ -748,12 +760,13 @@ func TestWorkspaceAutobuild(t *testing.T) { ctx = testutil.Context(t, testutil.WaitMedium) ) + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) client, user := coderdenttest.New(t, &coderdenttest.Options{ Options: &coderdtest.Options{ AutobuildTicker: ticker, IncludeProvisionerDaemon: true, AutobuildStats: statCh, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore()), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, @@ -833,12 +846,13 @@ func TestWorkspaceAutobuild(t *testing.T) { ctx = testutil.Context(t, testutil.WaitMedium) ) + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) client, user := coderdenttest.New(t, &coderdenttest.Options{ Options: &coderdtest.Options{ AutobuildTicker: tickCh, IncludeProvisionerDaemon: true, AutobuildStats: statsCh, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore()), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAccessControl: 1}, @@ -920,9 +934,10 @@ func TestTemplateDoesNotAllowUserAutostop(t *testing.T) { t.Run("TTLSetByTemplate", func(t *testing.T) { t.Parallel() + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) client := coderdtest.New(t, &coderdtest.Options{ IncludeProvisionerDaemon: true, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore()), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), }) user := coderdtest.CreateFirstUser(t, client) version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) @@ -958,9 +973,10 @@ func TestTemplateDoesNotAllowUserAutostop(t *testing.T) { t.Run("ExtendIsNotEnabledByTemplate", func(t *testing.T) { t.Parallel() + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) client := coderdtest.New(t, &coderdtest.Options{ IncludeProvisionerDaemon: true, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore()), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), }) user := coderdtest.CreateFirstUser(t, client) version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) @@ -1002,15 +1018,17 @@ func TestExecutorAutostartBlocked(t *testing.T) { } var ( - sched = must(cron.Weekly("CRON_TZ=UTC 0 * * * *")) - tickCh = make(chan time.Time) - statsCh = make(chan autobuild.Stats) + sched = must(cron.Weekly("CRON_TZ=UTC 0 * * * *")) + tickCh = make(chan time.Time) + statsCh = make(chan autobuild.Stats) + + logger = slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) client, owner = coderdenttest.New(t, &coderdenttest.Options{ Options: &coderdtest.Options{ AutobuildTicker: tickCh, IncludeProvisionerDaemon: true, AutobuildStats: statsCh, - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore()), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, @@ -1051,9 +1069,10 @@ func TestWorkspacesFiltering(t *testing.T) { t.Parallel() ctx := testutil.Context(t, testutil.WaitMedium) + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) client, db, owner := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ Options: &coderdtest.Options{ - TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore()), + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger), }, LicenseOptions: &coderdenttest.LicenseOptions{ Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, From bbcb28eadbf0d8ec8aeb106b62d6a5090cba4dfc Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Wed, 17 Jul 2024 19:27:38 +0000 Subject: [PATCH 12/35] Apply fmt --- coderd/workspaces_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 436780901fab7..fc9f19dd735a8 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -3297,7 +3297,6 @@ func TestWorkspaceDormant(t *testing.T) { require.NoError(t, err) coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStop, database.WorkspaceTransitionStart) }) - } func TestWorkspaceFavoriteUnfavorite(t *testing.T) { From e3c6f499fb9005cf3374971742e8bb2734b2f70b Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Wed, 17 Jul 2024 20:08:26 +0000 Subject: [PATCH 13/35] Notify after executor is done --- coderd/autobuild/lifecycle_executor.go | 36 +++++++++++---------- enterprise/coderd/dormancy/notifications.go | 1 - 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index 99e540aca5c01..19ae744faeaad 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -142,8 +142,11 @@ func (e *Executor) runOnce(t time.Time) Stats { eg.Go(func() error { err := func() error { - var job *database.ProvisionerJob - var auditLog *auditParams + var ( + job *database.ProvisionerJob + auditLog *auditParams + dormantNotification *dormancy.WorkspaceDormantNotification + ) err := e.db.InTx(func(tx database.Store) error { // Re-check eligibility since the first check was outside the // transaction and the workspace settings may have changed. @@ -228,26 +231,18 @@ func (e *Executor) runOnce(t time.Time) Stats { return xerrors.Errorf("update workspace dormant deleting at: %w", err) } + dormantNotification = &dormancy.WorkspaceDormantNotification{ + Workspace: ws, + Initiator: "system", + Reason: "breached the template's threshold for inactivity", + CreatedBy: "lifecycleexecutor", + } + log.Info(e.ctx, "dormant workspace", slog.F("last_used_at", ws.LastUsedAt), slog.F("time_til_dormant", templateSchedule.TimeTilDormant), slog.F("since_last_used_at", time.Since(ws.LastUsedAt)), ) - - _, err = dormancy.NotifyWorkspaceDormant( - e.ctx, - e.notificationsEnqueuer, - dormancy.WorkspaceDormantNotification{ - Workspace: ws, - Initiator: "system", - Reason: "breached the template's threshold for inactivity", - CreatedBy: "lifecycleexecutor", - }, - ) - - if err != nil { - log.Warn(e.ctx, "failed to notify of workspace marked as dormant", slog.Error(err)) - } } if reason == database.BuildReasonAutodelete { @@ -294,6 +289,13 @@ func (e *Executor) runOnce(t time.Time) Stats { return xerrors.Errorf("post provisioner job to pubsub: %w", err) } } + if dormantNotification != nil { + dormancy.NotifyWorkspaceDormant( + e.ctx, + e.notificationsEnqueuer, + *dormantNotification, + ) + } return nil }() if err != nil { diff --git a/enterprise/coderd/dormancy/notifications.go b/enterprise/coderd/dormancy/notifications.go index c5c446d2b54a6..e95b7556f891b 100644 --- a/enterprise/coderd/dormancy/notifications.go +++ b/enterprise/coderd/dormancy/notifications.go @@ -18,7 +18,6 @@ type WorkspaceDormantNotification struct { func NotifyWorkspaceDormant( ctx context.Context, - enqueuer notifications.Enqueuer, notification WorkspaceDormantNotification, ) (id *uuid.UUID, err error) { From 20a8766366d83cd89fc46e2a6ad6d6b802121782 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Thu, 18 Jul 2024 09:34:50 +0200 Subject: [PATCH 14/35] Revert refactoring mistake Signed-off-by: Danny Kopping --- coderd/schedule/mock.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/coderd/schedule/mock.go b/coderd/schedule/mock.go index c38fddb8acedb..1fe33bb549e81 100644 --- a/coderd/schedule/mock.go +++ b/coderd/schedule/mock.go @@ -23,12 +23,12 @@ func (m MockTemplateScheduleStore) Get(ctx context.Context, db database.Store, t return NewAGPLTemplateScheduleStore().Get(ctx, db, templateID) } -func (m MockTemplateScheduleStore) Set(ctx context.Context, db database.Store, template database.Template, opts TemplateScheduleOptions) (database.Template, error) { +func (m MockTemplateScheduleStore) Set(ctx context.Context, db database.Store, template database.Template, options TemplateScheduleOptions) (database.Template, error) { if m.SetFn != nil { - return m.SetFn(ctx, db, template, opts) + return m.SetFn(ctx, db, template, options) } - return NewAGPLTemplateScheduleStore().Set(ctx, db, template, opts) + return NewAGPLTemplateScheduleStore().Set(ctx, db, template, options) } type MockUserQuietHoursScheduleStore struct { From e08717256f897a2a261adc6ba896861836ca6bef Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Thu, 18 Jul 2024 12:43:20 +0000 Subject: [PATCH 15/35] Add workspace name to the log --- coderd/workspaces.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index f4efbd79f440d..52ed00e4e080d 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -955,7 +955,7 @@ func (api *API) putWorkspaceDormant(rw http.ResponseWriter, r *http.Request) { if req.Dormant && apiKey.UserID != workspace.OwnerID { initiator, err := api.Database.GetUserByID(ctx, apiKey.UserID) if err != nil { - api.Logger.Warn(ctx, "failed to fetch the user that marked the workspace as dormant", slog.Error(err)) + api.Logger.Warn(ctx, "failed to fetch the user that marked the workspace "+workspace.Name+" as dormant", slog.Error(err)) } else { _, err = dormancy.NotifyWorkspaceDormant( ctx, From 4b99061393f9c89718e17b7b80d2b5a417dcd2be Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Thu, 18 Jul 2024 12:48:45 +0000 Subject: [PATCH 16/35] Set a fake enqueuer on coderdtest options --- coderd/coderdtest/coderdtest.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index 6c4cf94b586d7..0cb8a13c95e0c 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -285,6 +285,9 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can options.StatsBatcher = batcher t.Cleanup(closeBatcher) } + if options.NotificationEnqueuer == nil { + options.NotificationEnqueuer = &testutil.FakeNotificationEnqueuer{} + } var templateScheduleStore atomic.Pointer[schedule.TemplateScheduleStore] if options.TemplateScheduleStore == nil { From 36c043cf1b59e59ae3e25927e8e271dec221f6e9 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Thu, 18 Jul 2024 13:01:00 +0000 Subject: [PATCH 17/35] Fix migration --- ...te.down.sql => 000228_dormancy_notification_template.down.sql} | 0 ...mplate.up.sql => 000228_dormancy_notification_template.up.sql} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename coderd/database/migrations/{000227_dormancy_notification_template.down.sql => 000228_dormancy_notification_template.down.sql} (100%) rename coderd/database/migrations/{000227_dormancy_notification_template.up.sql => 000228_dormancy_notification_template.up.sql} (100%) diff --git a/coderd/database/migrations/000227_dormancy_notification_template.down.sql b/coderd/database/migrations/000228_dormancy_notification_template.down.sql similarity index 100% rename from coderd/database/migrations/000227_dormancy_notification_template.down.sql rename to coderd/database/migrations/000228_dormancy_notification_template.down.sql diff --git a/coderd/database/migrations/000227_dormancy_notification_template.up.sql b/coderd/database/migrations/000228_dormancy_notification_template.up.sql similarity index 100% rename from coderd/database/migrations/000227_dormancy_notification_template.up.sql rename to coderd/database/migrations/000228_dormancy_notification_template.up.sql From d07f9d835b2b26c9ef4ef52d14f1360592d58467 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Thu, 18 Jul 2024 13:16:07 +0000 Subject: [PATCH 18/35] Fix migration sql --- .../migrations/000228_dormancy_notification_template.down.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/database/migrations/000228_dormancy_notification_template.down.sql b/coderd/database/migrations/000228_dormancy_notification_template.down.sql index 6e63f38af7a6d..c0f318c44b0e1 100644 --- a/coderd/database/migrations/000228_dormancy_notification_template.down.sql +++ b/coderd/database/migrations/000228_dormancy_notification_template.down.sql @@ -1 +1 @@ -DELETE notification_templates WHERE id = '123e4567-e89b-12d3-a456-426614174000'; +DELETE FROM notification_templates WHERE id = '123e4567-e89b-12d3-a456-426614174000'; From 72da82dfa753a6a7232992359047f785eadc1448 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Thu, 18 Jul 2024 13:31:23 +0000 Subject: [PATCH 19/35] Fix typo --- coderd/autobuild/lifecycle_executor_test.go | 2 +- coderd/coderdtest/coderdtest.go | 4 ++-- coderd/notifications/notiffake/notiffake.go | 4 ++-- coderd/provisionerdserver/provisionerdserver.go | 8 ++++---- coderd/provisionerdserver/provisionerdserver_test.go | 4 ++-- coderd/workspaces_test.go | 12 ++++++------ testutil/notifications.go | 6 +++--- 7 files changed, 20 insertions(+), 20 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go index c5362ef13829d..ec005453296e9 100644 --- a/coderd/autobuild/lifecycle_executor_test.go +++ b/coderd/autobuild/lifecycle_executor_test.go @@ -116,7 +116,7 @@ func TestExecutorAutostartTemplateUpdated(t *testing.T) { tickCh = make(chan time.Time) statsCh = make(chan autobuild.Stats) logger = slogtest.Make(t, &slogtest.Options{IgnoreErrors: !tc.expectStart}).Leveled(slog.LevelDebug) - enqueuer = notiffake.FakeNotificationEnqueuer{} + enqueuer = notiffake.FakeNotificationsEnqueuer{} client = coderdtest.New(t, &coderdtest.Options{ AutobuildTicker: tickCh, IncludeProvisionerDaemon: true, diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index 579fc90309bc0..d98693df50356 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -243,7 +243,7 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can } if options.NotificationsEnqueuer == nil { - options.NotificationsEnqueuer = new(notiffake.FakeNotificationEnqueuer) + options.NotificationsEnqueuer = new(notiffake.FakeNotificationsEnqueuer) } accessControlStore := &atomic.Pointer[dbauthz.AccessControlStore]{} @@ -291,7 +291,7 @@ func NewOptions(t testing.TB, options *Options) (func(http.Handler), context.Can t.Cleanup(closeBatcher) } if options.NotificationsEnqueuer == nil { - options.NotificationsEnqueuer = &testutil.FakeNotificationEnqueuer{} + options.NotificationsEnqueuer = &testutil.FakeNotificationsEnqueuer{} } var templateScheduleStore atomic.Pointer[schedule.TemplateScheduleStore] diff --git a/coderd/notifications/notiffake/notiffake.go b/coderd/notifications/notiffake/notiffake.go index 2435d86162877..c94889a0caf34 100644 --- a/coderd/notifications/notiffake/notiffake.go +++ b/coderd/notifications/notiffake/notiffake.go @@ -7,7 +7,7 @@ import ( "github.com/google/uuid" ) -type FakeNotificationEnqueuer struct { +type FakeNotificationsEnqueuer struct { mu sync.Mutex Sent []*Notification @@ -20,7 +20,7 @@ type Notification struct { Targets []uuid.UUID } -func (f *FakeNotificationEnqueuer) Enqueue(_ context.Context, userID, templateID uuid.UUID, labels map[string]string, createdBy string, targets ...uuid.UUID) (*uuid.UUID, error) { +func (f *FakeNotificationsEnqueuer) Enqueue(_ context.Context, userID, templateID uuid.UUID, labels map[string]string, createdBy string, targets ...uuid.UUID) (*uuid.UUID, error) { f.mu.Lock() defer f.mu.Unlock() diff --git a/coderd/provisionerdserver/provisionerdserver.go b/coderd/provisionerdserver/provisionerdserver.go index e8ec371b1c354..209c1b8448f34 100644 --- a/coderd/provisionerdserver/provisionerdserver.go +++ b/coderd/provisionerdserver/provisionerdserver.go @@ -98,7 +98,7 @@ type server struct { TemplateScheduleStore *atomic.Pointer[schedule.TemplateScheduleStore] UserQuietHoursScheduleStore *atomic.Pointer[schedule.UserQuietHoursScheduleStore] DeploymentValues *codersdk.DeploymentValues - NotificationEnqueuer notifications.Enqueuer + NotificationsEnqueuer notifications.Enqueuer OIDCConfig promoauth.OAuth2Config @@ -202,7 +202,7 @@ func NewServer( Database: db, Pubsub: ps, Acquirer: acquirer, - NotificationEnqueuer: enqueuer, + NotificationsEnqueuer: enqueuer, Telemetry: tel, Tracer: tracer, QuotaCommitter: quotaCommitter, @@ -1103,7 +1103,7 @@ func (s *server) notifyWorkspaceBuildFailed(ctx context.Context, workspace datab reason = string(build.Reason) initiator := "autobuild" - if _, err := s.NotificationEnqueuer.Enqueue(ctx, workspace.OwnerID, notifications.WorkspaceAutobuildFailed, + if _, err := s.NotificationsEnqueuer.Enqueue(ctx, workspace.OwnerID, notifications.WorkspaceAutobuildFailed, map[string]string{ "name": workspace.Name, "initiator": initiator, @@ -1574,7 +1574,7 @@ func (s *server) notifyWorkspaceDeleted(ctx context.Context, workspace database. slog.F("reason", reason), slog.F("workspace_id", workspace.ID), slog.F("build_id", build.ID)) } - if _, err := s.NotificationEnqueuer.Enqueue(ctx, workspace.OwnerID, notifications.TemplateWorkspaceDeleted, + if _, err := s.NotificationsEnqueuer.Enqueue(ctx, workspace.OwnerID, notifications.TemplateWorkspaceDeleted, map[string]string{ "name": workspace.Name, "reason": reason, diff --git a/coderd/provisionerdserver/provisionerdserver_test.go b/coderd/provisionerdserver/provisionerdserver_test.go index 6eaf84524a559..2117d8e5f3df8 100644 --- a/coderd/provisionerdserver/provisionerdserver_test.go +++ b/coderd/provisionerdserver/provisionerdserver_test.go @@ -1601,7 +1601,7 @@ func TestNotifications(t *testing.T) { t.Parallel() ctx := context.Background() - notifEnq := &testutil.FakeNotificationEnqueuer{} + notifEnq := &testutil.FakeNotificationsEnqueuer{} srv, db, ps, pd := setup(t, false, &overrides{ notificationEnqueuer: notifEnq, @@ -1721,7 +1721,7 @@ func TestNotifications(t *testing.T) { t.Parallel() ctx := context.Background() - notifEnq := &testutil.FakeNotificationEnqueuer{} + notifEnq := &testutil.FakeNotificationsEnqueuer{} // Otherwise `(*Server).FailJob` fails with: // audit log - get build {"error": "sql: no rows in result set"} diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index fc9f19dd735a8..93e0e6793ddcb 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -3516,10 +3516,10 @@ func TestNotifications(t *testing.T) { // Given var ( - notifyEnq = &testutil.FakeNotificationEnqueuer{} + notifyEnq = &testutil.FakeNotificationsEnqueuer{} client = coderdtest.New(t, &coderdtest.Options{ IncludeProvisionerDaemon: true, - NotificationEnqueuer: notifyEnq, + NotificationsEnqueuer: notifyEnq, }) user = coderdtest.CreateFirstUser(t, client) memberClient, member = coderdtest.CreateAnotherUser(t, client, user.OrganizationID, rbac.RoleOwner()) @@ -3554,10 +3554,10 @@ func TestNotifications(t *testing.T) { // Given var ( - notifyEnq = &testutil.FakeNotificationEnqueuer{} + notifyEnq = &testutil.FakeNotificationsEnqueuer{} client = coderdtest.New(t, &coderdtest.Options{ IncludeProvisionerDaemon: true, - NotificationEnqueuer: notifyEnq, + NotificationsEnqueuer: notifyEnq, }) user = coderdtest.CreateFirstUser(t, client) version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) @@ -3585,10 +3585,10 @@ func TestNotifications(t *testing.T) { // Given var ( - notifyEnq = &testutil.FakeNotificationEnqueuer{} + notifyEnq = &testutil.FakeNotificationsEnqueuer{} client = coderdtest.New(t, &coderdtest.Options{ IncludeProvisionerDaemon: true, - NotificationEnqueuer: notifyEnq, + NotificationsEnqueuer: notifyEnq, }) user = coderdtest.CreateFirstUser(t, client) version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) diff --git a/testutil/notifications.go b/testutil/notifications.go index b777e2516f964..a8d6486209d2a 100644 --- a/testutil/notifications.go +++ b/testutil/notifications.go @@ -7,7 +7,7 @@ import ( "github.com/google/uuid" ) -type FakeNotificationEnqueuer struct { +type FakeNotificationsEnqueuer struct { mu sync.Mutex Sent []*Notification } @@ -19,7 +19,7 @@ type Notification struct { Targets []uuid.UUID } -func (f *FakeNotificationEnqueuer) Enqueue(_ context.Context, userID, templateID uuid.UUID, labels map[string]string, createdBy string, targets ...uuid.UUID) (*uuid.UUID, error) { +func (f *FakeNotificationsEnqueuer) Enqueue(_ context.Context, userID, templateID uuid.UUID, labels map[string]string, createdBy string, targets ...uuid.UUID) (*uuid.UUID, error) { f.mu.Lock() defer f.mu.Unlock() @@ -35,7 +35,7 @@ func (f *FakeNotificationEnqueuer) Enqueue(_ context.Context, userID, templateID return &id, nil } -func (f *FakeNotificationEnqueuer) Clear() { +func (f *FakeNotificationsEnqueuer) Clear() { f.mu.Lock() defer f.mu.Unlock() From b49cd08c1d52347d4ffaf712cfeb4039df57f8e4 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Thu, 18 Jul 2024 13:36:28 +0000 Subject: [PATCH 20/35] Fix migration number --- ...te.down.sql => 000229_dormancy_notification_template.down.sql} | 0 ...mplate.up.sql => 000229_dormancy_notification_template.up.sql} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename coderd/database/migrations/{000228_dormancy_notification_template.down.sql => 000229_dormancy_notification_template.down.sql} (100%) rename coderd/database/migrations/{000228_dormancy_notification_template.up.sql => 000229_dormancy_notification_template.up.sql} (100%) diff --git a/coderd/database/migrations/000228_dormancy_notification_template.down.sql b/coderd/database/migrations/000229_dormancy_notification_template.down.sql similarity index 100% rename from coderd/database/migrations/000228_dormancy_notification_template.down.sql rename to coderd/database/migrations/000229_dormancy_notification_template.down.sql diff --git a/coderd/database/migrations/000228_dormancy_notification_template.up.sql b/coderd/database/migrations/000229_dormancy_notification_template.up.sql similarity index 100% rename from coderd/database/migrations/000228_dormancy_notification_template.up.sql rename to coderd/database/migrations/000229_dormancy_notification_template.up.sql From 281b545a14586aefd8ab330b11ba0b7a693c1612 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Thu, 18 Jul 2024 13:45:17 +0000 Subject: [PATCH 21/35] Apply Marcin comments --- coderd/autobuild/lifecycle_executor.go | 2 +- .../000229_dormancy_notification_template.down.sql | 2 +- .../migrations/000229_dormancy_notification_template.up.sql | 2 +- coderd/notifications/events.go | 2 +- enterprise/coderd/schedule/template.go | 2 +- enterprise/coderd/schedule/template_test.go | 5 +++-- 6 files changed, 8 insertions(+), 7 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index 680c6b3df3e63..8de28b2723250 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -250,7 +250,7 @@ func (e *Executor) runOnce(t time.Time) Stats { dormantNotification = &dormancy.WorkspaceDormantNotification{ Workspace: ws, - Initiator: "system", + Initiator: "autobuild", Reason: "breached the template's threshold for inactivity", CreatedBy: "lifecycleexecutor", } diff --git a/coderd/database/migrations/000229_dormancy_notification_template.down.sql b/coderd/database/migrations/000229_dormancy_notification_template.down.sql index c0f318c44b0e1..ada780956875a 100644 --- a/coderd/database/migrations/000229_dormancy_notification_template.down.sql +++ b/coderd/database/migrations/000229_dormancy_notification_template.down.sql @@ -1 +1 @@ -DELETE FROM notification_templates WHERE id = '123e4567-e89b-12d3-a456-426614174000'; +DELETE FROM notification_templates WHERE id = '0ea69165-ec14-4314-91f1-69566ac3c5a0'; diff --git a/coderd/database/migrations/000229_dormancy_notification_template.up.sql b/coderd/database/migrations/000229_dormancy_notification_template.up.sql index 48d7e13b1c675..27c733892b99e 100644 --- a/coderd/database/migrations/000229_dormancy_notification_template.up.sql +++ b/coderd/database/migrations/000229_dormancy_notification_template.up.sql @@ -1,5 +1,5 @@ INSERT INTO notification_templates (id, name, title_template, body_template, "group", actions) -VALUES ('123e4567-e89b-12d3-a456-426614174000', 'Workspace Marked as Dormant', E'Workspace "{{.Labels.name}}" marked as dormant', +VALUES ('0ea69165-ec14-4314-91f1-69566ac3c5a0', 'Workspace Marked as Dormant', E'Workspace "{{.Labels.name}}" marked as dormant', E'Hi {{.UserName}}\n\n' || E'Your workspace **{{.Labels.name}}** has been marked as **dormant**.\n' || E'The specified reason was "**{{.Labels.reason}}{{ if .Labels.initiator }} (initiated by: {{ .Labels.initiator }}){{end}}**\n\n' || diff --git a/coderd/notifications/events.go b/coderd/notifications/events.go index 1a62224a395f1..a0a56a09b8259 100644 --- a/coderd/notifications/events.go +++ b/coderd/notifications/events.go @@ -9,6 +9,6 @@ import "github.com/google/uuid" var ( TemplateWorkspaceDeleted = uuid.MustParse("f517da0b-cdc9-410f-ab89-a86107c420ed") WorkspaceAutobuildFailed = uuid.MustParse("381df2a9-c0c0-4749-420f-80a9280c66f9") - TemplateWorkspaceDormant = uuid.MustParse("123e4567-e89b-12d3-a456-426614174000") + TemplateWorkspaceDormant = uuid.MustParse("0ea69165-ec14-4314-91f1-69566ac3c5a0") WorkspaceAutoUpdated = uuid.MustParse("c34a0c09-0704-4cac-bd1c-0c0146811c2b") ) diff --git a/enterprise/coderd/schedule/template.go b/enterprise/coderd/schedule/template.go index 4cdf3a63cc206..2960c06367ac8 100644 --- a/enterprise/coderd/schedule/template.go +++ b/enterprise/coderd/schedule/template.go @@ -211,7 +211,7 @@ func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.S s.enqueuer, dormancy.WorkspaceDormantNotification{ Workspace: workspace, - Initiator: "system", + Initiator: "autobuild", Reason: "template updated to new dormancy policy", CreatedBy: "scheduletemplate", }, diff --git a/enterprise/coderd/schedule/template_test.go b/enterprise/coderd/schedule/template_test.go index 7198d99a84581..8a73a54c5213c 100644 --- a/enterprise/coderd/schedule/template_test.go +++ b/enterprise/coderd/schedule/template_test.go @@ -8,12 +8,13 @@ import ( "testing" "time" - "cdr.dev/slog" - "cdr.dev/slog/sloggers/slogtest" "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "cdr.dev/slog" + "cdr.dev/slog/sloggers/slogtest" + "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbgen" "github.com/coder/coder/v2/coderd/database/dbtestutil" From 7f73d90070c719db883f340465eb2ae4398f8d33 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Thu, 18 Jul 2024 14:20:08 +0000 Subject: [PATCH 22/35] Fix lint --- coderd/autobuild/lifecycle_executor.go | 7 +++++-- {enterprise/coderd => coderd}/dormancy/notifications.go | 5 +++++ coderd/workspaces.go | 2 +- enterprise/coderd/schedule/template.go | 2 +- enterprise/coderd/templates_test.go | 5 +++-- enterprise/coderd/workspaces_test.go | 3 ++- 6 files changed, 17 insertions(+), 7 deletions(-) rename {enterprise/coderd => coderd}/dormancy/notifications.go (75%) diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index 8de28b2723250..cefd13eb0cb10 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -19,10 +19,10 @@ import ( "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/database/provisionerjobs" "github.com/coder/coder/v2/coderd/database/pubsub" + "github.com/coder/coder/v2/coderd/dormancy" "github.com/coder/coder/v2/coderd/notifications" "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/wsbuilder" - "github.com/coder/coder/v2/enterprise/coderd/dormancy" ) // Executor automatically starts or stops workspaces. @@ -326,11 +326,14 @@ func (e *Executor) runOnce(t time.Time) Stats { } } if dormantNotification != nil { - dormancy.NotifyWorkspaceDormant( + _, err = dormancy.NotifyWorkspaceDormant( e.ctx, e.notificationsEnqueuer, *dormantNotification, ) + if err != nil { + e.log.Warn(e.ctx, "failed to notify of workspace "+dormantNotification.Workspace.Name+" marked as dormant", slog.Error(err)) + } } return nil }() diff --git a/enterprise/coderd/dormancy/notifications.go b/coderd/dormancy/notifications.go similarity index 75% rename from enterprise/coderd/dormancy/notifications.go rename to coderd/dormancy/notifications.go index e95b7556f891b..e78f11f0bee71 100644 --- a/enterprise/coderd/dormancy/notifications.go +++ b/coderd/dormancy/notifications.go @@ -1,3 +1,8 @@ +// This package is located outside of the enterprise package to ensure +// accessibility in the putWorkspaceDormant function. This design choice allows +// workspaces to be taken out of dormancy even if the license has expired, +// ensuring critical functionality remains available without an active +// enterprise license. package dormancy import ( diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 52ed00e4e080d..18c3fc9662874 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -23,6 +23,7 @@ import ( "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbtime" "github.com/coder/coder/v2/coderd/database/provisionerjobs" + "github.com/coder/coder/v2/coderd/dormancy" "github.com/coder/coder/v2/coderd/httpapi" "github.com/coder/coder/v2/coderd/httpmw" "github.com/coder/coder/v2/coderd/rbac" @@ -34,7 +35,6 @@ import ( "github.com/coder/coder/v2/coderd/wsbuilder" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/codersdk/agentsdk" - "github.com/coder/coder/v2/enterprise/coderd/dormancy" ) var ( diff --git a/enterprise/coderd/schedule/template.go b/enterprise/coderd/schedule/template.go index 2960c06367ac8..6027fa5dd2551 100644 --- a/enterprise/coderd/schedule/template.go +++ b/enterprise/coderd/schedule/template.go @@ -16,11 +16,11 @@ import ( "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbtime" + "github.com/coder/coder/v2/coderd/dormancy" "github.com/coder/coder/v2/coderd/notifications" agpl "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/tracing" "github.com/coder/coder/v2/codersdk" - "github.com/coder/coder/v2/enterprise/coderd/dormancy" ) // EnterpriseTemplateScheduleStore provides an agpl.TemplateScheduleStore that diff --git a/enterprise/coderd/templates_test.go b/enterprise/coderd/templates_test.go index 2b603dfc6c163..d817698ef75f8 100644 --- a/enterprise/coderd/templates_test.go +++ b/enterprise/coderd/templates_test.go @@ -7,12 +7,13 @@ import ( "testing" "time" - "cdr.dev/slog" - "cdr.dev/slog/sloggers/slogtest" "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "cdr.dev/slog" + "cdr.dev/slog/sloggers/slogtest" + "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index 6c8530e401062..11923e6889cd0 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -7,9 +7,10 @@ import ( "testing" "time" - "cdr.dev/slog" "github.com/stretchr/testify/require" + "cdr.dev/slog" + "cdr.dev/slog/sloggers/slogtest" "github.com/coder/coder/v2/coderd/audit" From 7c5de97ce73910d259dbc7fc89c382782c624d16 Mon Sep 17 00:00:00 2001 From: Bruno Quaresma Date: Thu, 18 Jul 2024 12:54:13 -0300 Subject: [PATCH 23/35] Update coderd/autobuild/lifecycle_executor.go Co-authored-by: Danny Kopping --- coderd/autobuild/lifecycle_executor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index cefd13eb0cb10..b342f838726c5 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -332,7 +332,7 @@ func (e *Executor) runOnce(t time.Time) Stats { *dormantNotification, ) if err != nil { - e.log.Warn(e.ctx, "failed to notify of workspace "+dormantNotification.Workspace.Name+" marked as dormant", slog.Error(err)) + e.log.Warn(e.ctx, "failed to notify of workspace marked as dormant", slog.Error(err), slog.F("workspace", dormantNotification.Workspace.Id)) } } return nil From 299cd7ffcfc9431c293e274f834dcbc3203f1a03 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Thu, 18 Jul 2024 16:01:21 +0000 Subject: [PATCH 24/35] Apply dannys comment --- coderd/autobuild/lifecycle_executor.go | 2 +- coderd/workspaces.go | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index b342f838726c5..176ab0967d7b7 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -332,7 +332,7 @@ func (e *Executor) runOnce(t time.Time) Stats { *dormantNotification, ) if err != nil { - e.log.Warn(e.ctx, "failed to notify of workspace marked as dormant", slog.Error(err), slog.F("workspace", dormantNotification.Workspace.Id)) + e.log.Warn(e.ctx, "failed to notify of workspace marked as dormant", slog.Error(err), slog.F("workspace_id", dormantNotification.Workspace.ID)) } } return nil diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 18c3fc9662874..1f4c4f276a5b8 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -955,7 +955,13 @@ func (api *API) putWorkspaceDormant(rw http.ResponseWriter, r *http.Request) { if req.Dormant && apiKey.UserID != workspace.OwnerID { initiator, err := api.Database.GetUserByID(ctx, apiKey.UserID) if err != nil { - api.Logger.Warn(ctx, "failed to fetch the user that marked the workspace "+workspace.Name+" as dormant", slog.Error(err)) + api.Logger.Warn( + ctx, + "failed to fetch the user that marked the workspace", + slog.Error(err), + slog.F("workspace_id", workspace.ID), + slog.F("user_id", apiKey.UserID), + ) } else { _, err = dormancy.NotifyWorkspaceDormant( ctx, From 16a3c193a4d43a8d7a021a330a8ea858d15a3782 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Mon, 22 Jul 2024 13:48:22 +0000 Subject: [PATCH 25/35] Simplify dormancy template --- coderd/autobuild/lifecycle_executor.go | 4 ++- ...0229_dormancy_notification_template.up.sql | 27 ++++++++++++------- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index 176ab0967d7b7..6622a97726146 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -332,8 +332,10 @@ func (e *Executor) runOnce(t time.Time) Stats { *dormantNotification, ) if err != nil { - e.log.Warn(e.ctx, "failed to notify of workspace marked as dormant", slog.Error(err), slog.F("workspace_id", dormantNotification.Workspace.ID)) + log.Warn(e.ctx, "failed to notify of workspace marked as dormant", slog.Error(err), slog.F("workspace_id", dormantNotification.Workspace.ID)) } + } else { + log.Warn(e.ctx, "no dormant notification to send", slog.F("workspace_id", wsID)) } return nil }() diff --git a/coderd/database/migrations/000229_dormancy_notification_template.up.sql b/coderd/database/migrations/000229_dormancy_notification_template.up.sql index 27c733892b99e..4db8d29d71b25 100644 --- a/coderd/database/migrations/000229_dormancy_notification_template.up.sql +++ b/coderd/database/migrations/000229_dormancy_notification_template.up.sql @@ -1,13 +1,22 @@ -INSERT INTO notification_templates (id, name, title_template, body_template, "group", actions) -VALUES ('0ea69165-ec14-4314-91f1-69566ac3c5a0', 'Workspace Marked as Dormant', E'Workspace "{{.Labels.name}}" marked as dormant', - E'Hi {{.UserName}}\n\n' || - E'Your workspace **{{.Labels.name}}** has been marked as **dormant**.\n' || - E'The specified reason was "**{{.Labels.reason}}{{ if .Labels.initiator }} (initiated by: {{ .Labels.initiator }}){{end}}**\n\n' || - E'Dormancy refers to a workspace being unused for a defined length of time, and after it exceeds {{.Labels.dormancyHours}} hours of dormancy it will be deleted.\n' || - E'To prevent your workspace from being deleted, simply use it as normal.', - 'Workspace Events', '[ +INSERT INTO + notification_templates ( + id, + name, + title_template, + body_template, + "group", + actions + ) +VALUES ( + '0ea69165-ec14-4314-91f1-69566ac3c5a0', + 'Workspace Marked as Dormant', + E'Workspace "{{.Labels.name}}" marked as dormant', + E'Hi {{.UserName}}\n\n' || E'Your workspace **{{.Labels.name}}** has been marked as **dormant**.\n' || E'The specified reason was "**{{.Labels.reason}} (initiated by: {{ .Labels.initiator }}){{end}}**\n\n' || E'Dormancy refers to a workspace being unused for a defined length of time, and after it exceeds {{.Labels.dormancyHours}} hours of dormancy it will be deleted.\n' || E'To prevent your workspace from being deleted, simply use it as normal.', + 'Workspace Events', + '[ { "label": "View workspace", "url": "{{ base_url }}/@{{.UserName}}/{{.Labels.name}}" } - ]'::jsonb); + ]'::jsonb + ); From aea55aa01c26a409bc3373565f6dcc5fb2597547 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Mon, 22 Jul 2024 20:34:10 +0000 Subject: [PATCH 26/35] Add test to verify dormancy in lifecycle executor --- coderd/autobuild/lifecycle_executor_test.go | 56 +++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go index ce240aa94a6a5..96826eecb03b8 100644 --- a/coderd/autobuild/lifecycle_executor_test.go +++ b/coderd/autobuild/lifecycle_executor_test.go @@ -3,6 +3,7 @@ package autobuild_test import ( "context" "os" + "sync/atomic" "testing" "time" @@ -25,6 +26,8 @@ import ( "github.com/coder/coder/v2/provisioner/echo" "github.com/coder/coder/v2/provisionersdk/proto" "github.com/coder/coder/v2/testutil" + + enterpriseSchedule "github.com/coder/coder/v2/enterprise/coderd/schedule" ) func TestExecutorAutostartOK(t *testing.T) { @@ -1062,6 +1065,52 @@ func TestExecutorInactiveWorkspace(t *testing.T) { }) } +func TestNotifications(t *testing.T) { + t.Parallel() + + t.Run("Dormancy", func(t *testing.T) { + t.Parallel() + + // Setup template with dormancy and create a workspace with it + var ( + ticker = make(chan time.Time) + statCh = make(chan autobuild.Stats) + logger = slogtest.Make(t, &slogtest.Options{}) + notificationsEnqueuer = testutil.FakeNotificationsEnqueuer{} + client = coderdtest.New(t, &coderdtest.Options{ + AutobuildTicker: ticker, + AutobuildStats: statCh, + IncludeProvisionerDaemon: true, + NotificationsEnqueuer: ¬ificationsEnqueuer, + TemplateScheduleStore: enterpriseSchedule.NewEnterpriseTemplateScheduleStore(userQuietHoursScheduleStore(), ¬ificationsEnqueuer, logger), + }) + admin = coderdtest.CreateFirstUser(t, client) + version = coderdtest.CreateTemplateVersion(t, client, admin.OrganizationID, nil) + timeTilDormant = 1000 + ) + + coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) + template := coderdtest.CreateTemplate(t, client, admin.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) { + ctr.TimeTilDormantMillis = ptr.Ref(int64(timeTilDormant)) + }) + userClient, _ := coderdtest.CreateAnotherUser(t, client, admin.OrganizationID) + workspace := coderdtest.CreateWorkspace(t, userClient, admin.OrganizationID, template.ID) + coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, workspace.LatestBuild.ID) + + // Stop workspace + workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) + build := coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, workspace.LatestBuild.ID) + + // Wait for workspace to become dormant + ticker <- build.Job.CompletedAt.Add(time.Millisecond * time.Duration(timeTilDormant) * 2) + <-statCh + + // Check that the workspace is dormant + workspace = coderdtest.MustWorkspace(t, client, workspace.ID) + require.NotNil(t, workspace.DormantAt) + }) +} + func mustProvisionWorkspace(t *testing.T, client *codersdk.Client, mut ...func(*codersdk.CreateWorkspaceRequest)) codersdk.Workspace { t.Helper() user := coderdtest.CreateFirstUser(t, client) @@ -1113,3 +1162,10 @@ func mustWorkspaceParameters(t *testing.T, client *codersdk.Client, workspaceID func TestMain(m *testing.M) { goleak.VerifyTestMain(m) } + +func userQuietHoursScheduleStore() *atomic.Pointer[schedule.UserQuietHoursScheduleStore] { + store := schedule.NewAGPLUserQuietHoursScheduleStore() + p := &atomic.Pointer[schedule.UserQuietHoursScheduleStore]{} + p.Store(&store) + return p +} From 6d47c04cc7f066ed6edb4dbf290b3be2c7d42c2f Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Mon, 22 Jul 2024 20:44:33 +0000 Subject: [PATCH 27/35] Add placeholder --- coderd/autobuild/lifecycle_executor_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go index 96826eecb03b8..38b3acb4b5387 100644 --- a/coderd/autobuild/lifecycle_executor_test.go +++ b/coderd/autobuild/lifecycle_executor_test.go @@ -1108,6 +1108,8 @@ func TestNotifications(t *testing.T) { // Check that the workspace is dormant workspace = coderdtest.MustWorkspace(t, client, workspace.ID) require.NotNil(t, workspace.DormantAt) + + // TODO: Write test to check notification. }) } From 7b4ae29af701fd734032312da80d9e071e2f42ea Mon Sep 17 00:00:00 2001 From: Jon Ayers Date: Tue, 23 Jul 2024 01:40:08 +0000 Subject: [PATCH 28/35] make test pass --- coderd/autobuild/lifecycle_executor_test.go | 31 +++++++++++---------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go index 38b3acb4b5387..d2ed19a699c13 100644 --- a/coderd/autobuild/lifecycle_executor_test.go +++ b/coderd/autobuild/lifecycle_executor_test.go @@ -26,8 +26,6 @@ import ( "github.com/coder/coder/v2/provisioner/echo" "github.com/coder/coder/v2/provisionersdk/proto" "github.com/coder/coder/v2/testutil" - - enterpriseSchedule "github.com/coder/coder/v2/enterprise/coderd/schedule" ) func TestExecutorAutostartOK(t *testing.T) { @@ -1075,41 +1073,46 @@ func TestNotifications(t *testing.T) { var ( ticker = make(chan time.Time) statCh = make(chan autobuild.Stats) - logger = slogtest.Make(t, &slogtest.Options{}) notificationsEnqueuer = testutil.FakeNotificationsEnqueuer{} + timeTilDormant = time.Minute client = coderdtest.New(t, &coderdtest.Options{ AutobuildTicker: ticker, AutobuildStats: statCh, IncludeProvisionerDaemon: true, NotificationsEnqueuer: ¬ificationsEnqueuer, - TemplateScheduleStore: enterpriseSchedule.NewEnterpriseTemplateScheduleStore(userQuietHoursScheduleStore(), ¬ificationsEnqueuer, logger), + TemplateScheduleStore: schedule.MockTemplateScheduleStore{ + GetFn: func(_ context.Context, _ database.Store, _ uuid.UUID) (schedule.TemplateScheduleOptions, error) { + return schedule.TemplateScheduleOptions{ + UserAutostartEnabled: false, + UserAutostopEnabled: true, + DefaultTTL: 0, + AutostopRequirement: schedule.TemplateAutostopRequirement{}, + TimeTilDormant: timeTilDormant, + }, nil + }, + }, }) - admin = coderdtest.CreateFirstUser(t, client) - version = coderdtest.CreateTemplateVersion(t, client, admin.OrganizationID, nil) - timeTilDormant = 1000 + admin = coderdtest.CreateFirstUser(t, client) + version = coderdtest.CreateTemplateVersion(t, client, admin.OrganizationID, nil) ) coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) - template := coderdtest.CreateTemplate(t, client, admin.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) { - ctr.TimeTilDormantMillis = ptr.Ref(int64(timeTilDormant)) - }) + template := coderdtest.CreateTemplate(t, client, admin.OrganizationID, version.ID) userClient, _ := coderdtest.CreateAnotherUser(t, client, admin.OrganizationID) workspace := coderdtest.CreateWorkspace(t, userClient, admin.OrganizationID, template.ID) coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, workspace.LatestBuild.ID) // Stop workspace workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, database.WorkspaceTransitionStart, database.WorkspaceTransitionStop) - build := coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, workspace.LatestBuild.ID) + _ = coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, workspace.LatestBuild.ID) // Wait for workspace to become dormant - ticker <- build.Job.CompletedAt.Add(time.Millisecond * time.Duration(timeTilDormant) * 2) + ticker <- workspace.LastUsedAt.Add(timeTilDormant * 3) <-statCh // Check that the workspace is dormant workspace = coderdtest.MustWorkspace(t, client, workspace.ID) require.NotNil(t, workspace.DormantAt) - - // TODO: Write test to check notification. }) } From 308bd69505dc5fd4da80fb945f44d08638d59328 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 23 Jul 2024 12:53:00 +0200 Subject: [PATCH 29/35] fix: lint --- coderd/autobuild/lifecycle_executor_test.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go index d2ed19a699c13..407620de059da 100644 --- a/coderd/autobuild/lifecycle_executor_test.go +++ b/coderd/autobuild/lifecycle_executor_test.go @@ -3,7 +3,6 @@ package autobuild_test import ( "context" "os" - "sync/atomic" "testing" "time" @@ -1167,10 +1166,3 @@ func mustWorkspaceParameters(t *testing.T, client *codersdk.Client, workspaceID func TestMain(m *testing.M) { goleak.VerifyTestMain(m) } - -func userQuietHoursScheduleStore() *atomic.Pointer[schedule.UserQuietHoursScheduleStore] { - store := schedule.NewAGPLUserQuietHoursScheduleStore() - p := &atomic.Pointer[schedule.UserQuietHoursScheduleStore]{} - p.Store(&store) - return p -} From 48ad2697a75f753967a40eb90277b61c93f81d32 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Tue, 23 Jul 2024 15:32:28 +0000 Subject: [PATCH 30/35] Add test to verify notification in lifecycle executor --- coderd/autobuild/lifecycle_executor_test.go | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go index 407620de059da..463f89078fb32 100644 --- a/coderd/autobuild/lifecycle_executor_test.go +++ b/coderd/autobuild/lifecycle_executor_test.go @@ -1070,15 +1070,15 @@ func TestNotifications(t *testing.T) { // Setup template with dormancy and create a workspace with it var ( - ticker = make(chan time.Time) - statCh = make(chan autobuild.Stats) - notificationsEnqueuer = testutil.FakeNotificationsEnqueuer{} - timeTilDormant = time.Minute - client = coderdtest.New(t, &coderdtest.Options{ + ticker = make(chan time.Time) + statCh = make(chan autobuild.Stats) + notifyEnq = testutil.FakeNotificationsEnqueuer{} + timeTilDormant = time.Minute + client = coderdtest.New(t, &coderdtest.Options{ AutobuildTicker: ticker, AutobuildStats: statCh, IncludeProvisionerDaemon: true, - NotificationsEnqueuer: ¬ificationsEnqueuer, + NotificationsEnqueuer: ¬ifyEnq, TemplateScheduleStore: schedule.MockTemplateScheduleStore{ GetFn: func(_ context.Context, _ database.Store, _ uuid.UUID) (schedule.TemplateScheduleOptions, error) { return schedule.TemplateScheduleOptions{ @@ -1112,6 +1112,15 @@ func TestNotifications(t *testing.T) { // Check that the workspace is dormant workspace = coderdtest.MustWorkspace(t, client, workspace.ID) require.NotNil(t, workspace.DormantAt) + + // Check that a notification was enqueued + require.Len(t, notifyEnq.Sent, 1) + require.Equal(t, notifyEnq.Sent[0].UserID, workspace.OwnerID) + require.Contains(t, notifyEnq.Sent[0].Targets, template.ID) + require.Contains(t, notifyEnq.Sent[0].Targets, workspace.ID) + require.Contains(t, notifyEnq.Sent[0].Targets, workspace.OrganizationID) + require.Contains(t, notifyEnq.Sent[0].Targets, workspace.OwnerID) + require.Equal(t, notifyEnq.Sent[0].Labels["initiator"], "autobuild") }) } From 0226fdfc9b7749a29f71f819c1f924713bf163af Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Tue, 23 Jul 2024 16:43:51 +0000 Subject: [PATCH 31/35] Add notification for marked as deletion --- coderd/database/dbmem/dbmem.go | 4 +- ...or_deletion_notification_template.down.sql | 3 + ..._for_deletion_notification_template.up sql | 22 +++++ coderd/dormancy/notifications.go | 30 +++++++ coderd/notifications/events.go | 9 +- enterprise/coderd/schedule/template.go | 13 ++- enterprise/coderd/schedule/template_test.go | 89 +++++++++++++++++++ 7 files changed, 158 insertions(+), 12 deletions(-) create mode 100644 coderd/database/migrations/000230_marked_for_deletion_notification_template.down.sql create mode 100644 coderd/database/migrations/000230_marked_for_deletion_notification_template.up sql diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 5be77c4651c62..cf1773f637a02 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -8709,6 +8709,7 @@ func (q *FakeQuerier) UpdateWorkspacesDormantDeletingAtByTemplateID(_ context.Co return nil, err } + affectedRows := []database.Workspace{} for i, ws := range q.workspaces { if ws.TemplateID != arg.TemplateID { continue @@ -8733,9 +8734,10 @@ func (q *FakeQuerier) UpdateWorkspacesDormantDeletingAtByTemplateID(_ context.Co } ws.DeletingAt = deletingAt q.workspaces[i] = ws + affectedRows = append(affectedRows, ws) } - return q.workspaces, nil + return affectedRows, nil } func (q *FakeQuerier) UpsertAnnouncementBanners(_ context.Context, data string) error { diff --git a/coderd/database/migrations/000230_marked_for_deletion_notification_template.down.sql b/coderd/database/migrations/000230_marked_for_deletion_notification_template.down.sql new file mode 100644 index 0000000000000..6aad699850aca --- /dev/null +++ b/coderd/database/migrations/000230_marked_for_deletion_notification_template.down.sql @@ -0,0 +1,3 @@ +DELETE FROM notification_templates +WHERE + id = '51ce2fdf-c9ca-4be1-8d70-628674f9bc42'; diff --git a/coderd/database/migrations/000230_marked_for_deletion_notification_template.up sql b/coderd/database/migrations/000230_marked_for_deletion_notification_template.up sql new file mode 100644 index 0000000000000..3ca470be60c5b --- /dev/null +++ b/coderd/database/migrations/000230_marked_for_deletion_notification_template.up sql @@ -0,0 +1,22 @@ +INSERT INTO + notification_templates ( + id, + name, + title_template, + body_template, + "group", + actions + ) +VALUES ( + '51ce2fdf-c9ca-4be1-8d70-628674f9bc42', + 'Workspace Marked for Deletion', + E'Workspace "{{.Labels.name}}" marked for deletion', + E'Hi {{.UserName}}\n\n' || E'Your workspace **{{.Labels.name}}** has been marked for **deletion** after {{.Labels.dormancyHours}} hours of dormancy.\n' || E'The specified reason was "**{{.Labels.reason}} (initiated by: {{ .Labels.initiator }}){{end}}**\n\n' || E'Dormancy refers to a workspace being unused for a defined length of time, and after it exceeds {{.Labels.dormancyHours}} hours of dormancy it will be deleted.\n' || E'To prevent your workspace from being deleted, simply use it as normal.', + 'Workspace Events', + '[ + { + "label": "View workspace", + "url": "{{ base_url }}/@{{.UserName}}/{{.Labels.name}}" + } + ]'::jsonb + ); diff --git a/coderd/dormancy/notifications.go b/coderd/dormancy/notifications.go index e78f11f0bee71..003271a2714cb 100644 --- a/coderd/dormancy/notifications.go +++ b/coderd/dormancy/notifications.go @@ -44,3 +44,33 @@ func NotifyWorkspaceDormant( notification.Workspace.OrganizationID, ) } + +type WorkspaceMarkedForDeletionNotification struct { + Workspace database.Workspace + Reason string + CreatedBy string +} + +func NotifyWorkspaceMarkedForDeletion( + ctx context.Context, + enqueuer notifications.Enqueuer, + notification WorkspaceMarkedForDeletionNotification, +) (id *uuid.UUID, err error) { + labels := map[string]string{ + "name": notification.Workspace.Name, + "initiator": "autobuild", + "reason": notification.Reason, + } + return enqueuer.Enqueue( + ctx, + notification.Workspace.OwnerID, + notifications.TemplateWorkspaceDormant, + labels, + notification.CreatedBy, + // Associate this notification with all the related entities. + notification.Workspace.ID, + notification.Workspace.OwnerID, + notification.Workspace.TemplateID, + notification.Workspace.OrganizationID, + ) +} diff --git a/coderd/notifications/events.go b/coderd/notifications/events.go index a0a56a09b8259..097686eebb8eb 100644 --- a/coderd/notifications/events.go +++ b/coderd/notifications/events.go @@ -7,8 +7,9 @@ import "github.com/google/uuid" // Workspace-related events. var ( - TemplateWorkspaceDeleted = uuid.MustParse("f517da0b-cdc9-410f-ab89-a86107c420ed") - WorkspaceAutobuildFailed = uuid.MustParse("381df2a9-c0c0-4749-420f-80a9280c66f9") - TemplateWorkspaceDormant = uuid.MustParse("0ea69165-ec14-4314-91f1-69566ac3c5a0") - WorkspaceAutoUpdated = uuid.MustParse("c34a0c09-0704-4cac-bd1c-0c0146811c2b") + TemplateWorkspaceDeleted = uuid.MustParse("f517da0b-cdc9-410f-ab89-a86107c420ed") + WorkspaceAutobuildFailed = uuid.MustParse("381df2a9-c0c0-4749-420f-80a9280c66f9") + TemplateWorkspaceDormant = uuid.MustParse("0ea69165-ec14-4314-91f1-69566ac3c5a0") + WorkspaceAutoUpdated = uuid.MustParse("c34a0c09-0704-4cac-bd1c-0c0146811c2b") + TemplateWorkspaceMarkedForDeletion = uuid.MustParse("51ce2fdf-c9ca-4be1-8d70-628674f9bc42") ) diff --git a/enterprise/coderd/schedule/template.go b/enterprise/coderd/schedule/template.go index 6027fa5dd2551..0b6f55ceda9c9 100644 --- a/enterprise/coderd/schedule/template.go +++ b/enterprise/coderd/schedule/template.go @@ -136,7 +136,7 @@ func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.S var ( template database.Template - dormantWorkspaces []database.Workspace + markedForDeletion []database.Workspace ) err = db.InTx(func(tx database.Store) error { ctx, span := tracing.StartSpanWithName(ctx, "(*schedule.EnterpriseTemplateScheduleStore).Set()-InTx()") @@ -171,7 +171,7 @@ func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.S // to ensure workspaces are being cleaned up correctly. Similarly if we are // disabling it (by passing 0), then we want to delete nullify the deleting_at // fields of all the template workspaces. - dormantWorkspaces, err = tx.UpdateWorkspacesDormantDeletingAtByTemplateID(ctx, database.UpdateWorkspacesDormantDeletingAtByTemplateIDParams{ + markedForDeletion, err = tx.UpdateWorkspacesDormantDeletingAtByTemplateID(ctx, database.UpdateWorkspacesDormantDeletingAtByTemplateIDParams{ TemplateID: tpl.ID, TimeTilDormantAutodeleteMs: opts.TimeTilDormantAutoDelete.Milliseconds(), DormantAt: dormantAt, @@ -205,19 +205,18 @@ func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.S return database.Template{}, err } - for _, workspace := range dormantWorkspaces { - _, err = dormancy.NotifyWorkspaceDormant( + for _, workspace := range markedForDeletion { + _, err = dormancy.NotifyWorkspaceMarkedForDeletion( ctx, s.enqueuer, - dormancy.WorkspaceDormantNotification{ + dormancy.WorkspaceMarkedForDeletionNotification{ Workspace: workspace, - Initiator: "autobuild", Reason: "template updated to new dormancy policy", CreatedBy: "scheduletemplate", }, ) if err != nil { - s.logger.Warn(ctx, "failed to notify of workspace marked as dormant", slog.Error(err)) + s.logger.Warn(ctx, "failed to notify of workspace marked for deletion", slog.Error(err)) } } diff --git a/enterprise/coderd/schedule/template_test.go b/enterprise/coderd/schedule/template_test.go index 8a73a54c5213c..d2b6fa63f17a9 100644 --- a/enterprise/coderd/schedule/template_test.go +++ b/enterprise/coderd/schedule/template_test.go @@ -606,6 +606,95 @@ func TestTemplateUpdateBuildDeadlinesSkip(t *testing.T) { } } +func TestNotifications(t *testing.T) { + t.Parallel() + + t.Run("Dormancy", func(t *testing.T) { + t.Parallel() + + var ( + db, _ = dbtestutil.NewDB(t) + ctx = testutil.Context(t, testutil.WaitLong) + user = dbgen.User(t, db, database.User{}) + file = dbgen.File(t, db, database.File{ + CreatedBy: user.ID, + }) + templateJob = dbgen.ProvisionerJob(t, db, nil, database.ProvisionerJob{ + FileID: file.ID, + InitiatorID: user.ID, + Tags: database.StringMap{ + "foo": "bar", + }, + }) + timeTilDormant = time.Minute * 2 + templateVersion = dbgen.TemplateVersion(t, db, database.TemplateVersion{ + CreatedBy: user.ID, + JobID: templateJob.ID, + OrganizationID: templateJob.OrganizationID, + }) + template = dbgen.Template(t, db, database.Template{ + ActiveVersionID: templateVersion.ID, + CreatedBy: user.ID, + OrganizationID: templateJob.OrganizationID, + TimeTilDormant: int64(timeTilDormant), + TimeTilDormantAutoDelete: int64(timeTilDormant), + }) + ) + + // Add two dormant workspaces and one active workspace. + dormantWorkspaces := []database.Workspace{ + dbgen.Workspace(t, db, database.Workspace{ + OwnerID: user.ID, + TemplateID: template.ID, + OrganizationID: templateJob.OrganizationID, + LastUsedAt: time.Now().Add(-time.Hour), + }), + dbgen.Workspace(t, db, database.Workspace{ + OwnerID: user.ID, + TemplateID: template.ID, + OrganizationID: templateJob.OrganizationID, + LastUsedAt: time.Now().Add(-time.Hour), + }), + } + dbgen.Workspace(t, db, database.Workspace{ + OwnerID: user.ID, + TemplateID: template.ID, + OrganizationID: templateJob.OrganizationID, + LastUsedAt: time.Now(), + }) + for _, ws := range dormantWorkspaces { + db.UpdateWorkspaceDormantDeletingAt(ctx, database.UpdateWorkspaceDormantDeletingAtParams{ + ID: ws.ID, + DormantAt: sql.NullTime{ + Time: ws.LastUsedAt.Add(timeTilDormant), + Valid: true, + }, + }) + } + + // Setup dependencies + notifyEnq := testutil.FakeNotificationsEnqueuer{} + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) + const userQuietHoursSchedule = "CRON_TZ=UTC 0 0 * * *" // midnight UTC + userQuietHoursStore, err := schedule.NewEnterpriseUserQuietHoursScheduleStore(userQuietHoursSchedule, true) + require.NoError(t, err) + userQuietHoursStorePtr := &atomic.Pointer[agplschedule.UserQuietHoursScheduleStore]{} + userQuietHoursStorePtr.Store(&userQuietHoursStore) + templateScheduleStore := schedule.NewEnterpriseTemplateScheduleStore(userQuietHoursStorePtr, ¬ifyEnq, logger) + templateScheduleStore.TimeNowFn = time.Now + + // Update dormancy TTL for a lower value + _, err = templateScheduleStore.Set(ctx, db, template, agplschedule.TemplateScheduleOptions{ + TimeTilDormant: timeTilDormant / 2, + TimeTilDormantAutoDelete: timeTilDormant / 2, + }) + require.NoError(t, err) + + // We should expect two notifications. One for each dormant workspace. + require.Len(t, notifyEnq.Sent, len(dormantWorkspaces)) + }) +} + func must[V any](v V, err error) V { if err != nil { panic(err) From 4d71c946c8ba5c8e5e41d8508b0dad9a78cb45cc Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Wed, 24 Jul 2024 14:37:10 +0000 Subject: [PATCH 32/35] Apply Dannys review comments --- coderd/autobuild/lifecycle_executor.go | 4 +--- coderd/autobuild/lifecycle_executor_test.go | 6 +++-- ...29_dormancy_notification_template.down.sql | 8 ++++++- ...0229_dormancy_notification_template.up.sql | 13 +++++++++++ ...or_deletion_notification_template.down.sql | 3 --- ..._for_deletion_notification_template.up sql | 22 ------------------- coderd/dormancy/notifications.go | 7 +++--- coderd/notifications/events.go | 4 ++-- .../provisionerdserver/provisionerdserver.go | 2 +- coderd/workspaces_test.go | 2 ++ enterprise/coderd/schedule/template.go | 2 +- enterprise/coderd/schedule/template_test.go | 13 +++++++++-- 12 files changed, 45 insertions(+), 41 deletions(-) delete mode 100644 coderd/database/migrations/000230_marked_for_deletion_notification_template.down.sql delete mode 100644 coderd/database/migrations/000230_marked_for_deletion_notification_template.up sql diff --git a/coderd/autobuild/lifecycle_executor.go b/coderd/autobuild/lifecycle_executor.go index 6622a97726146..082ee0feedfcf 100644 --- a/coderd/autobuild/lifecycle_executor.go +++ b/coderd/autobuild/lifecycle_executor.go @@ -299,7 +299,7 @@ func (e *Executor) runOnce(t time.Time) Stats { nextBuildReason = string(nextBuild.Reason) } - if _, err := e.notificationsEnqueuer.Enqueue(e.ctx, ws.OwnerID, notifications.WorkspaceAutoUpdated, + if _, err := e.notificationsEnqueuer.Enqueue(e.ctx, ws.OwnerID, notifications.TemplateWorkspaceAutoUpdated, map[string]string{ "name": ws.Name, "initiator": "autobuild", @@ -334,8 +334,6 @@ func (e *Executor) runOnce(t time.Time) Stats { if err != nil { log.Warn(e.ctx, "failed to notify of workspace marked as dormant", slog.Error(err), slog.F("workspace_id", dormantNotification.Workspace.ID)) } - } else { - log.Warn(e.ctx, "no dormant notification to send", slog.F("workspace_id", wsID)) } return nil }() diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go index 463f89078fb32..888d6b1d6deca 100644 --- a/coderd/autobuild/lifecycle_executor_test.go +++ b/coderd/autobuild/lifecycle_executor_test.go @@ -18,6 +18,7 @@ import ( "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbauthz" + "github.com/coder/coder/v2/coderd/notifications" "github.com/coder/coder/v2/coderd/schedule" "github.com/coder/coder/v2/coderd/schedule/cron" "github.com/coder/coder/v2/coderd/util/ptr" @@ -1107,15 +1108,16 @@ func TestNotifications(t *testing.T) { // Wait for workspace to become dormant ticker <- workspace.LastUsedAt.Add(timeTilDormant * 3) - <-statCh + _ = testutil.RequireRecvCtx(testutil.Context(t, testutil.WaitShort), t, statCh) // Check that the workspace is dormant workspace = coderdtest.MustWorkspace(t, client, workspace.ID) - require.NotNil(t, workspace.DormantAt) + require.Equal(t, workspace.DormantAt, workspace.LastUsedAt.Add(timeTilDormant)) // Check that a notification was enqueued require.Len(t, notifyEnq.Sent, 1) require.Equal(t, notifyEnq.Sent[0].UserID, workspace.OwnerID) + require.Equal(t, notifyEnq.Sent[0].TemplateID, notifications.TemplateWorkspaceDormant) require.Contains(t, notifyEnq.Sent[0].Targets, template.ID) require.Contains(t, notifyEnq.Sent[0].Targets, workspace.ID) require.Contains(t, notifyEnq.Sent[0].Targets, workspace.OrganizationID) diff --git a/coderd/database/migrations/000229_dormancy_notification_template.down.sql b/coderd/database/migrations/000229_dormancy_notification_template.down.sql index ada780956875a..ca82cf912c53b 100644 --- a/coderd/database/migrations/000229_dormancy_notification_template.down.sql +++ b/coderd/database/migrations/000229_dormancy_notification_template.down.sql @@ -1 +1,7 @@ -DELETE FROM notification_templates WHERE id = '0ea69165-ec14-4314-91f1-69566ac3c5a0'; +DELETE FROM notification_templates +WHERE + id = '0ea69165-ec14-4314-91f1-69566ac3c5a0'; + +DELETE FROM notification_templates +WHERE + id = '51ce2fdf-c9ca-4be1-8d70-628674f9bc42'; diff --git a/coderd/database/migrations/000229_dormancy_notification_template.up.sql b/coderd/database/migrations/000229_dormancy_notification_template.up.sql index 4db8d29d71b25..f4403e75ed1cb 100644 --- a/coderd/database/migrations/000229_dormancy_notification_template.up.sql +++ b/coderd/database/migrations/000229_dormancy_notification_template.up.sql @@ -18,5 +18,18 @@ VALUES ( "label": "View workspace", "url": "{{ base_url }}/@{{.UserName}}/{{.Labels.name}}" } + ]'::jsonb, + ), + ( + '51ce2fdf-c9ca-4be1-8d70-628674f9bc42', + 'Workspace Marked for Deletion', + E'Workspace "{{.Labels.name}}" marked for deletion', + E'Hi {{.UserName}}\n\n' || E'Your workspace **{{.Labels.name}}** has been marked for **deletion** after {{.Labels.dormancyHours}} hours of dormancy.\n' || E'The specified reason was "**{{.Labels.reason}}{{end}}**\n\n' || E'Dormancy refers to a workspace being unused for a defined length of time, and after it exceeds {{.Labels.dormancyHours}} hours of dormancy it will be deleted.\n' || E'To prevent your workspace from being deleted, simply use it as normal.', + 'Workspace Events', + '[ + { + "label": "View workspace", + "url": "{{ base_url }}/@{{.UserName}}/{{.Labels.name}}" + } ]'::jsonb ); diff --git a/coderd/database/migrations/000230_marked_for_deletion_notification_template.down.sql b/coderd/database/migrations/000230_marked_for_deletion_notification_template.down.sql deleted file mode 100644 index 6aad699850aca..0000000000000 --- a/coderd/database/migrations/000230_marked_for_deletion_notification_template.down.sql +++ /dev/null @@ -1,3 +0,0 @@ -DELETE FROM notification_templates -WHERE - id = '51ce2fdf-c9ca-4be1-8d70-628674f9bc42'; diff --git a/coderd/database/migrations/000230_marked_for_deletion_notification_template.up sql b/coderd/database/migrations/000230_marked_for_deletion_notification_template.up sql deleted file mode 100644 index 3ca470be60c5b..0000000000000 --- a/coderd/database/migrations/000230_marked_for_deletion_notification_template.up sql +++ /dev/null @@ -1,22 +0,0 @@ -INSERT INTO - notification_templates ( - id, - name, - title_template, - body_template, - "group", - actions - ) -VALUES ( - '51ce2fdf-c9ca-4be1-8d70-628674f9bc42', - 'Workspace Marked for Deletion', - E'Workspace "{{.Labels.name}}" marked for deletion', - E'Hi {{.UserName}}\n\n' || E'Your workspace **{{.Labels.name}}** has been marked for **deletion** after {{.Labels.dormancyHours}} hours of dormancy.\n' || E'The specified reason was "**{{.Labels.reason}} (initiated by: {{ .Labels.initiator }}){{end}}**\n\n' || E'Dormancy refers to a workspace being unused for a defined length of time, and after it exceeds {{.Labels.dormancyHours}} hours of dormancy it will be deleted.\n' || E'To prevent your workspace from being deleted, simply use it as normal.', - 'Workspace Events', - '[ - { - "label": "View workspace", - "url": "{{ base_url }}/@{{.UserName}}/{{.Labels.name}}" - } - ]'::jsonb - ); diff --git a/coderd/dormancy/notifications.go b/coderd/dormancy/notifications.go index 003271a2714cb..162ca272db635 100644 --- a/coderd/dormancy/notifications.go +++ b/coderd/dormancy/notifications.go @@ -57,14 +57,13 @@ func NotifyWorkspaceMarkedForDeletion( notification WorkspaceMarkedForDeletionNotification, ) (id *uuid.UUID, err error) { labels := map[string]string{ - "name": notification.Workspace.Name, - "initiator": "autobuild", - "reason": notification.Reason, + "name": notification.Workspace.Name, + "reason": notification.Reason, } return enqueuer.Enqueue( ctx, notification.Workspace.OwnerID, - notifications.TemplateWorkspaceDormant, + notifications.TemplateWorkspaceMarkedForDeletion, labels, notification.CreatedBy, // Associate this notification with all the related entities. diff --git a/coderd/notifications/events.go b/coderd/notifications/events.go index 097686eebb8eb..97c5d19f57a19 100644 --- a/coderd/notifications/events.go +++ b/coderd/notifications/events.go @@ -8,8 +8,8 @@ import "github.com/google/uuid" // Workspace-related events. var ( TemplateWorkspaceDeleted = uuid.MustParse("f517da0b-cdc9-410f-ab89-a86107c420ed") - WorkspaceAutobuildFailed = uuid.MustParse("381df2a9-c0c0-4749-420f-80a9280c66f9") + TemplateWorkspaceAutobuildFailed = uuid.MustParse("381df2a9-c0c0-4749-420f-80a9280c66f9") TemplateWorkspaceDormant = uuid.MustParse("0ea69165-ec14-4314-91f1-69566ac3c5a0") - WorkspaceAutoUpdated = uuid.MustParse("c34a0c09-0704-4cac-bd1c-0c0146811c2b") + TemplateWorkspaceAutoUpdated = uuid.MustParse("c34a0c09-0704-4cac-bd1c-0c0146811c2b") TemplateWorkspaceMarkedForDeletion = uuid.MustParse("51ce2fdf-c9ca-4be1-8d70-628674f9bc42") ) diff --git a/coderd/provisionerdserver/provisionerdserver.go b/coderd/provisionerdserver/provisionerdserver.go index 209c1b8448f34..b71935e9a0436 100644 --- a/coderd/provisionerdserver/provisionerdserver.go +++ b/coderd/provisionerdserver/provisionerdserver.go @@ -1103,7 +1103,7 @@ func (s *server) notifyWorkspaceBuildFailed(ctx context.Context, workspace datab reason = string(build.Reason) initiator := "autobuild" - if _, err := s.NotificationsEnqueuer.Enqueue(ctx, workspace.OwnerID, notifications.WorkspaceAutobuildFailed, + if _, err := s.NotificationsEnqueuer.Enqueue(ctx, workspace.OwnerID, notifications.TemplateWorkspaceAutobuildFailed, map[string]string{ "name": workspace.Name, "initiator": initiator, diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 93e0e6793ddcb..bd158d3893c94 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -30,6 +30,7 @@ import ( "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/notifications" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/coderd/rbac/policy" "github.com/coder/coder/v2/coderd/render" @@ -3541,6 +3542,7 @@ func TestNotifications(t *testing.T) { // Then require.NoError(t, err, "mark workspace as dormant") require.Len(t, notifyEnq.Sent, 1) + require.Equal(t, notifyEnq.Sent[0].TemplateID, notifications.TemplateWorkspaceDormant) require.Equal(t, notifyEnq.Sent[0].UserID, workspace.OwnerID) require.Contains(t, notifyEnq.Sent[0].Targets, template.ID) require.Contains(t, notifyEnq.Sent[0].Targets, workspace.ID) diff --git a/enterprise/coderd/schedule/template.go b/enterprise/coderd/schedule/template.go index 0b6f55ceda9c9..c3cb5001e091c 100644 --- a/enterprise/coderd/schedule/template.go +++ b/enterprise/coderd/schedule/template.go @@ -216,7 +216,7 @@ func (s *EnterpriseTemplateScheduleStore) Set(ctx context.Context, db database.S }, ) if err != nil { - s.logger.Warn(ctx, "failed to notify of workspace marked for deletion", slog.Error(err)) + s.logger.Warn(ctx, "failed to notify of workspace marked for deletion", slog.Error(err), slog.F("workspace_id", workspace.ID)) } } diff --git a/enterprise/coderd/schedule/template_test.go b/enterprise/coderd/schedule/template_test.go index d2b6fa63f17a9..bce5ffbec930e 100644 --- a/enterprise/coderd/schedule/template_test.go +++ b/enterprise/coderd/schedule/template_test.go @@ -683,15 +683,24 @@ func TestNotifications(t *testing.T) { templateScheduleStore := schedule.NewEnterpriseTemplateScheduleStore(userQuietHoursStorePtr, ¬ifyEnq, logger) templateScheduleStore.TimeNowFn = time.Now - // Update dormancy TTL for a lower value + // Lower the dormancy TTL to ensure the schedule recalculates deadlines and + // triggers notifications. _, err = templateScheduleStore.Set(ctx, db, template, agplschedule.TemplateScheduleOptions{ TimeTilDormant: timeTilDormant / 2, TimeTilDormantAutoDelete: timeTilDormant / 2, }) require.NoError(t, err) - // We should expect two notifications. One for each dormant workspace. + // We should expect a notification for each dormant workspace. require.Len(t, notifyEnq.Sent, len(dormantWorkspaces)) + for i, dormantWs := range dormantWorkspaces { + require.Equal(t, notifyEnq.Sent[i].UserID, dormantWs.OwnerID) + require.Equal(t, notifyEnq.Sent[i].TemplateID, notifications.TemplateWorkspaceMarkedForDeletion) + require.Contains(t, notifyEnq.Sent[i].Targets, template.ID) + require.Contains(t, notifyEnq.Sent[i].Targets, dormantWs.ID) + require.Contains(t, notifyEnq.Sent[i].Targets, dormantWs.OrganizationID) + require.Contains(t, notifyEnq.Sent[i].Targets, dormantWs.OwnerID) + } }) } From e1d5fec213b5d05e94f0f85fd8dd688ea93a54f5 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Wed, 24 Jul 2024 14:42:10 +0000 Subject: [PATCH 33/35] Fix SQL --- .../migrations/000229_dormancy_notification_template.up.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/database/migrations/000229_dormancy_notification_template.up.sql b/coderd/database/migrations/000229_dormancy_notification_template.up.sql index f4403e75ed1cb..a80b8c9686b84 100644 --- a/coderd/database/migrations/000229_dormancy_notification_template.up.sql +++ b/coderd/database/migrations/000229_dormancy_notification_template.up.sql @@ -18,7 +18,7 @@ VALUES ( "label": "View workspace", "url": "{{ base_url }}/@{{.UserName}}/{{.Labels.name}}" } - ]'::jsonb, + ]'::jsonb ), ( '51ce2fdf-c9ca-4be1-8d70-628674f9bc42', From a2023a1ef57a81ebaa4a1b40bbca8bddce20e8df Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Wed, 24 Jul 2024 16:05:07 +0000 Subject: [PATCH 34/35] Rollback dormant at test --- coderd/autobuild/lifecycle_executor_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/autobuild/lifecycle_executor_test.go b/coderd/autobuild/lifecycle_executor_test.go index 888d6b1d6deca..243b2550ccf63 100644 --- a/coderd/autobuild/lifecycle_executor_test.go +++ b/coderd/autobuild/lifecycle_executor_test.go @@ -1112,7 +1112,7 @@ func TestNotifications(t *testing.T) { // Check that the workspace is dormant workspace = coderdtest.MustWorkspace(t, client, workspace.ID) - require.Equal(t, workspace.DormantAt, workspace.LastUsedAt.Add(timeTilDormant)) + require.NotNil(t, workspace.DormantAt) // Check that a notification was enqueued require.Len(t, notifyEnq.Sent, 1) From bae985c6aeb840ea4800a6a7daaaef837eff9851 Mon Sep 17 00:00:00 2001 From: BrunoQuaresma Date: Wed, 24 Jul 2024 16:06:04 +0000 Subject: [PATCH 35/35] Fix template --- .../migrations/000229_dormancy_notification_template.up.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/database/migrations/000229_dormancy_notification_template.up.sql b/coderd/database/migrations/000229_dormancy_notification_template.up.sql index a80b8c9686b84..8c8670f163870 100644 --- a/coderd/database/migrations/000229_dormancy_notification_template.up.sql +++ b/coderd/database/migrations/000229_dormancy_notification_template.up.sql @@ -11,7 +11,7 @@ VALUES ( '0ea69165-ec14-4314-91f1-69566ac3c5a0', 'Workspace Marked as Dormant', E'Workspace "{{.Labels.name}}" marked as dormant', - E'Hi {{.UserName}}\n\n' || E'Your workspace **{{.Labels.name}}** has been marked as **dormant**.\n' || E'The specified reason was "**{{.Labels.reason}} (initiated by: {{ .Labels.initiator }}){{end}}**\n\n' || E'Dormancy refers to a workspace being unused for a defined length of time, and after it exceeds {{.Labels.dormancyHours}} hours of dormancy it will be deleted.\n' || E'To prevent your workspace from being deleted, simply use it as normal.', + E'Hi {{.UserName}}\n\n' || E'Your workspace **{{.Labels.name}}** has been marked as **dormant**.\n' || E'The specified reason was "**{{.Labels.reason}} (initiated by: {{ .Labels.initiator }}){{end}}**\n\n' || E'Dormancy refers to a workspace being unused for a defined length of time, and after it exceeds {{.Labels.dormancyHours}} hours of dormancy might be deleted.\n' || E'To activate your workspace again, simply use it as normal.', 'Workspace Events', '[ {