Skip to content

Commit 58b810f

Browse files
BrunoQuaresmaBrunoQuaresma
authored
fix: fix dormancy notifications (coder#14029)
1 parent 22143d3 commit 58b810f

12 files changed

+194
-128
lines changed

‎coderd/autobuild/lifecycle_executor.go

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import (
1919
"github.com/coder/coder/v2/coderd/database/dbtime"
2020
"github.com/coder/coder/v2/coderd/database/provisionerjobs"
2121
"github.com/coder/coder/v2/coderd/database/pubsub"
22-
"github.com/coder/coder/v2/coderd/dormancy"
2322
"github.com/coder/coder/v2/coderd/notifications"
2423
"github.com/coder/coder/v2/coderd/schedule"
2524
"github.com/coder/coder/v2/coderd/wsbuilder"
@@ -145,10 +144,11 @@ func (e *Executor) runOnce(t time.Time) Stats {
145144
var (
146145
job *database.ProvisionerJob
147146
auditLog *auditParams
148-
dormantNotification *dormancy.WorkspaceDormantNotification
147+
shouldNotifyDormancy bool
149148
nextBuild *database.WorkspaceBuild
150149
activeTemplateVersion database.TemplateVersion
151150
ws database.Workspace
151+
tmpl database.Template
152152
didAutoUpdate bool
153153
)
154154
err := e.db.InTx(func(tx database.Store) error {
@@ -182,17 +182,17 @@ func (e *Executor) runOnce(t time.Time) Stats {
182182
return xerrors.Errorf("get template scheduling options: %w", err)
183183
}
184184

185-
template, err := tx.GetTemplateByID(e.ctx, ws.TemplateID)
185+
tmpl, err = tx.GetTemplateByID(e.ctx, ws.TemplateID)
186186
if err != nil {
187187
return xerrors.Errorf("get template by ID: %w", err)
188188
}
189189

190-
activeTemplateVersion, err = tx.GetTemplateVersionByID(e.ctx, template.ActiveVersionID)
190+
activeTemplateVersion, err = tx.GetTemplateVersionByID(e.ctx, tmpl.ActiveVersionID)
191191
if err != nil {
192192
return xerrors.Errorf("get active template version by ID: %w", err)
193193
}
194194

195-
accessControl := (*(e.accessControlStore.Load())).GetTemplateAccessControl(template)
195+
accessControl := (*(e.accessControlStore.Load())).GetTemplateAccessControl(tmpl)
196196

197197
nextTransition, reason, err := getNextTransition(user, ws, latestBuild, latestJob, templateSchedule, currentTick)
198198
if err != nil {
@@ -215,7 +215,7 @@ func (e *Executor) runOnce(t time.Time) Stats {
215215
log.Debug(e.ctx, "autostarting with active version")
216216
builder = builder.ActiveVersion()
217217

218-
if latestBuild.TemplateVersionID != template.ActiveVersionID {
218+
if latestBuild.TemplateVersionID != tmpl.ActiveVersionID {
219219
// control flag to know if the workspace was auto-updated,
220220
// so the lifecycle executor can notify the user
221221
didAutoUpdate = true
@@ -248,12 +248,7 @@ func (e *Executor) runOnce(t time.Time) Stats {
248248
return xerrors.Errorf("update workspace dormant deleting at: %w", err)
249249
}
250250

251-
dormantNotification = &dormancy.WorkspaceDormantNotification{
252-
Workspace: ws,
253-
Initiator: "autobuild",
254-
Reason: "breached the template's threshold for inactivity",
255-
CreatedBy: "lifecycleexecutor",
256-
}
251+
shouldNotifyDormancy = true
257252

258253
log.Info(e.ctx, "dormant workspace",
259254
slog.F("last_used_at", ws.LastUsedAt),
@@ -325,14 +320,24 @@ func (e *Executor) runOnce(t time.Time) Stats {
325320
return xerrors.Errorf("post provisioner job to pubsub: %w", err)
326321
}
327322
}
328-
if dormantNotification != nil {
329-
_, err = dormancy.NotifyWorkspaceDormant(
323+
if shouldNotifyDormancy {
324+
_, err = e.notificationsEnqueuer.Enqueue(
330325
e.ctx,
331-
e.notificationsEnqueuer,
332-
*dormantNotification,
326+
ws.OwnerID,
327+
notifications.TemplateWorkspaceDormant,
328+
map[string]string{
329+
"name": ws.Name,
330+
"reason": "inactivity exceeded the dormancy threshold",
331+
"timeTilDormant": time.Duration(tmpl.TimeTilDormant).String(),
332+
},
333+
"lifecycle_executor",
334+
ws.ID,
335+
ws.OwnerID,
336+
ws.TemplateID,
337+
ws.OrganizationID,
333338
)
334339
if err != nil {
335-
log.Warn(e.ctx, "failed to notify of workspace marked as dormant", slog.Error(err), slog.F("workspace_id", dormantNotification.Workspace.ID))
340+
log.Warn(e.ctx, "failed to notify of workspace marked as dormant", slog.Error(err), slog.F("workspace_id", ws.ID))
336341
}
337342
}
338343
return nil

‎coderd/autobuild/lifecycle_executor_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1122,7 +1122,6 @@ func TestNotifications(t *testing.T) {
11221122
require.Contains(t, notifyEnq.Sent[0].Targets, workspace.ID)
11231123
require.Contains(t, notifyEnq.Sent[0].Targets, workspace.OrganizationID)
11241124
require.Contains(t, notifyEnq.Sent[0].Targets, workspace.OwnerID)
1125-
require.Equal(t, notifyEnq.Sent[0].Labels["initiator"], "autobuild")
11261125
})
11271126
}
11281127

‎coderd/database/migrations/000232_update_dormancy_notification_template.down.sql

Whitespace-only changes.

‎coderd/database/migrations/000232_update_dormancy_notification_template.up.sql

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
UPDATE notification_templates
2+
SET
3+
body_template = E'Hi {{.UserName}}\n\n' ||
4+
E'Your workspace **{{.Labels.name}}** has been marked as [**dormant**](https://coder.com/docs/templates/schedule#dormancy-threshold-enterprise) because of {{.Labels.reason}}.\n' ||
5+
E'Dormant workspaces are [automatically deleted](https://coder.com/docs/templates/schedule#dormancy-auto-deletion-enterprise) after {{.Labels.timeTilDormant}} of inactivity.\n' ||
6+
E'To prevent deletion, use your workspace with the link below.'
7+
WHERE
8+
id = '0ea69165-ec14-4314-91f1-69566ac3c5a0';
9+
10+
UPDATE notification_templates
11+
SET
12+
body_template = E'Hi {{.UserName}}\n\n' ||
13+
E'Your workspace **{{.Labels.name}}** has been marked for **deletion** after {{.Labels.timeTilDormant}} of [dormancy](https://coder.com/docs/templates/schedule#dormancy-auto-deletion-enterprise) because of {{.Labels.reason}}.\n' ||
14+
E'To prevent deletion, use your workspace with the link below.'
15+
WHERE
16+
id = '51ce2fdf-c9ca-4be1-8d70-628674f9bc42';

‎coderd/database/queries/notifications.sql

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,4 +132,3 @@ WHERE id IN
132132

133133
-- name: GetNotificationMessagesByStatus :many
134134
SELECT * FROM notification_messages WHERE status = @status LIMIT sqlc.arg('limit')::int;
135-

‎coderd/dormancy/notifications.go

Lines changed: 0 additions & 75 deletions
This file was deleted.

‎coderd/notifications/notifications_test.go

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929
"github.com/coder/coder/v2/coderd/database/dbtestutil"
3030
"github.com/coder/coder/v2/coderd/notifications"
3131
"github.com/coder/coder/v2/coderd/notifications/dispatch"
32+
"github.com/coder/coder/v2/coderd/notifications/render"
3233
"github.com/coder/coder/v2/coderd/notifications/types"
3334
"github.com/coder/coder/v2/coderd/util/syncmap"
3435
"github.com/coder/coder/v2/codersdk"
@@ -603,6 +604,107 @@ func TestNotifierPaused(t *testing.T) {
603604
}, testutil.WaitShort, testutil.IntervalFast)
604605
}
605606

607+
func TestNotifcationTemplatesBody(t *testing.T) {
608+
t.Parallel()
609+
610+
if !dbtestutil.WillUsePostgres() {
611+
t.Skip("This test requires postgres; it relies on the notification templates added by migrations in the database")
612+
}
613+
614+
tests := []struct {
615+
name string
616+
id uuid.UUID
617+
payload types.MessagePayload
618+
}{
619+
{
620+
name: "TemplateWorkspaceDeleted",
621+
id: notifications.TemplateWorkspaceDeleted,
622+
payload: types.MessagePayload{
623+
UserName: "bobby",
624+
Labels: map[string]string{
625+
"name": "bobby-workspace",
626+
"reason": "autodeleted due to dormancy",
627+
"initiator": "autobuild",
628+
},
629+
},
630+
},
631+
{
632+
name: "TemplateWorkspaceAutobuildFailed",
633+
id: notifications.TemplateWorkspaceAutobuildFailed,
634+
payload: types.MessagePayload{
635+
UserName: "bobby",
636+
Labels: map[string]string{
637+
"name": "bobby-workspace",
638+
"reason": "autostart",
639+
},
640+
},
641+
},
642+
{
643+
name: "TemplateWorkspaceDormant",
644+
id: notifications.TemplateWorkspaceDormant,
645+
payload: types.MessagePayload{
646+
UserName: "bobby",
647+
Labels: map[string]string{
648+
"name": "bobby-workspace",
649+
"reason": "breached the template's threshold for inactivity",
650+
"initiator": "autobuild",
651+
"dormancyHours": "24",
652+
},
653+
},
654+
},
655+
{
656+
name: "TemplateWorkspaceAutoUpdated",
657+
id: notifications.TemplateWorkspaceAutoUpdated,
658+
payload: types.MessagePayload{
659+
UserName: "bobby",
660+
Labels: map[string]string{
661+
"name": "bobby-workspace",
662+
"template_version_name": "1.0",
663+
},
664+
},
665+
},
666+
{
667+
name: "TemplateWorkspaceMarkedForDeletion",
668+
id: notifications.TemplateWorkspaceMarkedForDeletion,
669+
payload: types.MessagePayload{
670+
UserName: "bobby",
671+
Labels: map[string]string{
672+
"name": "bobby-workspace",
673+
"reason": "template updated to new dormancy policy",
674+
"dormancyHours": "24",
675+
},
676+
},
677+
},
678+
}
679+
680+
for _, tc := range tests {
681+
tc := tc
682+
683+
t.Run(tc.name, func(t *testing.T) {
684+
t.Parallel()
685+
686+
_, _, sql := dbtestutil.NewDBWithSQLDB(t)
687+
688+
var (
689+
titleTmpl string
690+
bodyTmpl string
691+
)
692+
err := sql.
693+
QueryRow("SELECT title_template, body_template FROM notification_templates WHERE id = $1 LIMIT 1", tc.id).
694+
Scan(&titleTmpl, &bodyTmpl)
695+
require.NoError(t, err, "failed to query body template for template:", tc.id)
696+
697+
title, err := render.GoTemplate(titleTmpl, tc.payload, nil)
698+
require.NoError(t, err, "failed to render notification title template")
699+
require.NotEmpty(t, title, "title should not be empty")
700+
701+
body, err := render.GoTemplate(bodyTmpl, tc.payload, nil)
702+
require.NoError(t, err, "failed to render notification body template")
703+
require.NotEmpty(t, body, "body should not be empty")
704+
})
705+
}
706+
}
707+
606708
type fakeHandler struct {
607709
mu sync.RWMutex
608710
succeeded, failed []string

‎coderd/provisionerdserver/provisionerdserver.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1101,13 +1101,11 @@ func (s *server) notifyWorkspaceBuildFailed(ctx context.Context, workspace datab
11011101
return // failed workspace build initiated by a user should not notify
11021102
}
11031103
reason = string(build.Reason)
1104-
initiator := "autobuild"
11051104

11061105
if _, err := s.NotificationsEnqueuer.Enqueue(ctx, workspace.OwnerID, notifications.TemplateWorkspaceAutobuildFailed,
11071106
map[string]string{
1108-
"name": workspace.Name,
1109-
"initiator": initiator,
1110-
"reason": reason,
1107+
"name": workspace.Name,
1108+
"reason": reason,
11111109
}, "provisionerdserver",
11121110
// Associate this notification with all the related entities.
11131111
workspace.ID, workspace.OwnerID, workspace.TemplateID, workspace.OrganizationID,

‎coderd/provisionerdserver/provisionerdserver_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1797,7 +1797,6 @@ func TestNotifications(t *testing.T) {
17971797
require.Contains(t, notifEnq.Sent[0].Targets, workspace.ID)
17981798
require.Contains(t, notifEnq.Sent[0].Targets, workspace.OrganizationID)
17991799
require.Contains(t, notifEnq.Sent[0].Targets, user.ID)
1800-
require.Equal(t, "autobuild", notifEnq.Sent[0].Labels["initiator"])
18011800
require.Equal(t, string(tc.buildReason), notifEnq.Sent[0].Labels["reason"])
18021801
} else {
18031802
require.Len(t, notifEnq.Sent, 0)

0 commit comments

Comments
 (0)