Skip to content

Commit a5e4bf3

Browse files
mtojekmtojek
authored
feat: notify owner about failed autobuild (coder#13891)
1 parent 36454aa commit a5e4bf3

File tree

8 files changed

+204
-25
lines changed

8 files changed

+204
-25
lines changed

‎coderd/database/migrations/000226_notifications_autobuild_failed.down.sql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
DELETE FROM notification_templates WHERE id = '381df2a9-c0c0-4749-420f-80a9280c66f9';

‎coderd/database/migrations/000226_notifications_autobuild_failed.up.sql

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
INSERT INTO notification_templates (id, name, title_template, body_template, "group", actions)
2+
VALUES ('381df2a9-c0c0-4749-420f-80a9280c66f9', 'Workspace Autobuild Failed', E'Workspace "{{.Labels.name}}" autobuild failed',
3+
E'Hi {{.UserName}}\n\Automatic build of your workspace **{{.Labels.name}}** failed.\nThe specified reason was "**{{.Labels.reason}}**".',
4+
'Workspace Events', '[
5+
{
6+
"label": "View workspace",
7+
"url": "{{ base_url }}/@{{.UserName}}/{{.Labels.name}}"
8+
}
9+
]'::jsonb);

‎coderd/notifications/enqueuer.go

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ func (s *StoreEnqueuer) Enqueue(ctx context.Context, userID, templateID uuid.UUI
8080
// buildPayload creates the payload that the notification will for variable substitution and/or routing.
8181
// The payload contains information about the recipient, the event that triggered the notification, and any subsequent
8282
// actions which can be taken by the recipient.
83-
func (s *StoreEnqueuer) buildPayload(ctx context.Context, userID uuid.UUID, templateID uuid.UUID, labels map[string]string) (*types.MessagePayload, error) {
83+
func (s *StoreEnqueuer) buildPayload(ctx context.Context, userID, templateID uuid.UUID, labels map[string]string) (*types.MessagePayload, error) {
8484
metadata, err := s.store.FetchNewMessageMetadata(ctx, database.FetchNewMessageMetadataParams{
8585
UserID: userID,
8686
NotificationTemplateID: templateID,
@@ -89,8 +89,21 @@ func (s *StoreEnqueuer) buildPayload(ctx context.Context, userID uuid.UUID, temp
8989
return nil, xerrors.Errorf("new message metadata: %w", err)
9090
}
9191

92+
payload := types.MessagePayload{
93+
Version: "1.0",
94+
95+
NotificationName: metadata.NotificationName,
96+
97+
UserID: metadata.UserID.String(),
98+
UserEmail: metadata.UserEmail,
99+
UserName: metadata.UserName,
100+
101+
Labels: labels,
102+
// No actions yet
103+
}
104+
92105
// Execute any templates in actions.
93-
out, err := render.GoTemplate(string(metadata.Actions), types.MessagePayload{}, s.helpers)
106+
out, err := render.GoTemplate(string(metadata.Actions), payload, s.helpers)
94107
if err != nil {
95108
return nil, xerrors.Errorf("render actions: %w", err)
96109
}
@@ -100,19 +113,8 @@ func (s *StoreEnqueuer) buildPayload(ctx context.Context, userID uuid.UUID, temp
100113
if err = json.Unmarshal(metadata.Actions, &actions); err != nil {
101114
return nil, xerrors.Errorf("new message metadata: parse template actions: %w", err)
102115
}
103-
104-
return &types.MessagePayload{
105-
Version: "1.0",
106-
107-
NotificationName: metadata.NotificationName,
108-
109-
UserID: metadata.UserID.String(),
110-
UserEmail: metadata.UserEmail,
111-
UserName: metadata.UserName,
112-
113-
Actions: actions,
114-
Labels: labels,
115-
}, nil
116+
payload.Actions = actions
117+
return &payload, nil
116118
}
117119

118120
// NoopEnqueuer implements the Enqueuer interface but performs a noop.

‎coderd/notifications/events.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,7 @@ import "github.com/google/uuid"
66
// TODO: autogenerate these.
77

88
// Workspace-related events.
9-
var TemplateWorkspaceDeleted = uuid.MustParse("f517da0b-cdc9-410f-ab89-a86107c420ed")
9+
var (
10+
TemplateWorkspaceDeleted = uuid.MustParse("f517da0b-cdc9-410f-ab89-a86107c420ed")
11+
WorkspaceAutobuildFailed = uuid.MustParse("381df2a9-c0c0-4749-420f-80a9280c66f9")
12+
)

‎coderd/notifications/manager_test.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,11 @@ func TestBuildPayload(t *testing.T) {
9898

9999
// GIVEN: a set of helpers to be injected into the templates
100100
const label = "Click here!"
101-
const url = "http://xyz.com/"
101+
const baseURL = "http://xyz.com"
102+
const url = baseURL + "/@bobby/my-workspace"
102103
helpers := map[string]any{
103104
"my_label": func() string { return label },
104-
"my_url": func() string { return url },
105+
"my_url": func() string { return baseURL },
105106
}
106107

107108
// GIVEN: an enqueue interceptor which returns mock metadata
@@ -112,7 +113,7 @@ func TestBuildPayload(t *testing.T) {
112113
actions := []types.TemplateAction{
113114
{
114115
Label: "{{ my_label }}",
115-
URL: "{{ my_url }}",
116+
URL: "{{ my_url }}/@{{.UserName}}/{{.Labels.name}}",
116117
},
117118
}
118119
out, err := json.Marshal(actions)
@@ -131,7 +132,9 @@ func TestBuildPayload(t *testing.T) {
131132
require.NoError(t, err)
132133

133134
// WHEN: a notification is enqueued
134-
_, err = enq.Enqueue(ctx, uuid.New(), notifications.TemplateWorkspaceDeleted, nil, "test")
135+
_, err = enq.Enqueue(ctx, uuid.New(), notifications.TemplateWorkspaceDeleted, map[string]string{
136+
"name": "my-workspace",
137+
}, "test")
135138
require.NoError(t, err)
136139

137140
// THEN: expect that a payload will be constructed and have the expected values

‎coderd/notifications/render/gotmpl_test.go

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,23 @@ func TestGoTemplate(t *testing.T) {
3838
expectedOutput: userEmail,
3939
expectedErr: nil,
4040
},
41+
{
42+
name: "render workspace URL",
43+
in: `[{
44+
"label": "View workspace",
45+
"url": "{{ base_url }}/@{{.UserName}}/{{.Labels.name}}"
46+
}]`,
47+
payload: types.MessagePayload{
48+
UserName: "johndoe",
49+
Labels: map[string]string{
50+
"name": "my-workspace",
51+
},
52+
},
53+
expectedOutput: `[{
54+
"label": "View workspace",
55+
"url": "https://mocked-server-address/@johndoe/my-workspace"
56+
}]`,
57+
},
4158
}
4259

4360
for _, tc := range tests {
@@ -46,7 +63,9 @@ func TestGoTemplate(t *testing.T) {
4663
t.Run(tc.name, func(t *testing.T) {
4764
t.Parallel()
4865

49-
out, err := render.GoTemplate(tc.in, tc.payload, nil)
66+
out, err := render.GoTemplate(tc.in, tc.payload, map[string]any{
67+
"base_url": func() string { return "https://mocked-server-address" },
68+
})
5069
if tc.expectedErr == nil {
5170
require.NoError(t, err)
5271
} else {

‎coderd/provisionerdserver/provisionerdserver.go

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -982,12 +982,18 @@ func (s *server) FailJob(ctx context.Context, failJob *proto.FailedJob) (*proto.
982982
}
983983

984984
var build database.WorkspaceBuild
985+
var workspace database.Workspace
985986
err = s.Database.InTx(func(db database.Store) error {
986987
build, err = db.GetWorkspaceBuildByID(ctx, input.WorkspaceBuildID)
987988
if err != nil {
988989
return xerrors.Errorf("get workspace build: %w", err)
989990
}
990991

992+
workspace, err = db.GetWorkspaceByID(ctx, build.WorkspaceID)
993+
if err != nil {
994+
return xerrors.Errorf("get workspace: %w", err)
995+
}
996+
991997
if jobType.WorkspaceBuild.State != nil {
992998
err = db.UpdateWorkspaceBuildProvisionerStateByID(ctx, database.UpdateWorkspaceBuildProvisionerStateByIDParams{
993999
ID: input.WorkspaceBuildID,
@@ -1014,6 +1020,8 @@ func (s *server) FailJob(ctx context.Context, failJob *proto.FailedJob) (*proto.
10141020
return nil, err
10151021
}
10161022

1023+
s.notifyWorkspaceBuildFailed(ctx, workspace, build)
1024+
10171025
err = s.Pubsub.Publish(codersdk.WorkspaceNotifyChannel(build.WorkspaceID), []byte{})
10181026
if err != nil {
10191027
return nil, xerrors.Errorf("update workspace: %w", err)
@@ -1087,6 +1095,27 @@ func (s *server) FailJob(ctx context.Context, failJob *proto.FailedJob) (*proto.
10871095
return &proto.Empty{}, nil
10881096
}
10891097

1098+
func (s *server) notifyWorkspaceBuildFailed(ctx context.Context, workspace database.Workspace, build database.WorkspaceBuild) {
1099+
var reason string
1100+
if build.Reason.Valid() && build.Reason == database.BuildReasonInitiator {
1101+
return // failed workspace build initiated by a user should not notify
1102+
}
1103+
reason = string(build.Reason)
1104+
initiator := "autobuild"
1105+
1106+
if _, err := s.NotificationEnqueuer.Enqueue(ctx, workspace.OwnerID, notifications.WorkspaceAutobuildFailed,
1107+
map[string]string{
1108+
"name": workspace.Name,
1109+
"initiator": initiator,
1110+
"reason": reason,
1111+
}, "provisionerdserver",
1112+
// Associate this notification with all the related entities.
1113+
workspace.ID, workspace.OwnerID, workspace.TemplateID, workspace.OrganizationID,
1114+
); err != nil {
1115+
s.Logger.Warn(ctx, "failed to notify of failed workspace autobuild", slog.Error(err))
1116+
}
1117+
}
1118+
10901119
// CompleteJob is triggered by a provision daemon to mark a provisioner job as completed.
10911120
func (s *server) CompleteJob(ctx context.Context, completed *proto.CompletedJob) (*proto.Empty, error) {
10921121
ctx, span := s.startTrace(ctx, tracing.FuncName())
@@ -1523,6 +1552,7 @@ func (s *server) CompleteJob(ctx context.Context, completed *proto.CompletedJob)
15231552

15241553
func (s *server) notifyWorkspaceDeleted(ctx context.Context, workspace database.Workspace, build database.WorkspaceBuild) {
15251554
var reason string
1555+
initiator := build.InitiatorByUsername
15261556
if build.Reason.Valid() {
15271557
switch build.Reason {
15281558
case database.BuildReasonInitiator:
@@ -1534,6 +1564,7 @@ func (s *server) notifyWorkspaceDeleted(ctx context.Context, workspace database.
15341564
reason = "initiated by user"
15351565
case database.BuildReasonAutodelete:
15361566
reason = "autodeleted due to dormancy"
1567+
initiator = "autobuild"
15371568
default:
15381569
reason = string(build.Reason)
15391570
}
@@ -1545,9 +1576,9 @@ func (s *server) notifyWorkspaceDeleted(ctx context.Context, workspace database.
15451576

15461577
if _, err := s.NotificationEnqueuer.Enqueue(ctx, workspace.OwnerID, notifications.TemplateWorkspaceDeleted,
15471578
map[string]string{
1548-
"name": workspace.Name,
1549-
"initiatedBy": build.InitiatorByUsername,
1550-
"reason": reason,
1579+
"name": workspace.Name,
1580+
"reason": reason,
1581+
"initiator": initiator,
15511582
}, "provisionerdserver",
15521583
// Associate this notification with all the related entities.
15531584
workspace.ID, workspace.OwnerID, workspace.TemplateID, workspace.OrganizationID,

‎coderd/provisionerdserver/provisionerdserver_test.go

Lines changed: 112 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1687,14 +1687,125 @@ func TestNotifications(t *testing.T) {
16871687
require.Contains(t, notifEnq.sent[0].targets, workspace.OrganizationID)
16881688
require.Contains(t, notifEnq.sent[0].targets, user.ID)
16891689
if tc.deletionReason == database.BuildReasonInitiator {
1690-
require.Equal(t, notifEnq.sent[0].labels["initiatedBy"], initiator.Username)
1690+
require.Equal(t, initiator.Username, notifEnq.sent[0].labels["initiator"])
16911691
}
16921692
} else {
16931693
require.Len(t, notifEnq.sent, 0)
16941694
}
16951695
})
16961696
}
16971697
})
1698+
1699+
t.Run("Workspace build failed", func(t *testing.T) {
1700+
t.Parallel()
1701+
1702+
tests := []struct {
1703+
name string
1704+
1705+
buildReason database.BuildReason
1706+
shouldNotify bool
1707+
}{
1708+
{
1709+
name: "initiated by owner",
1710+
buildReason: database.BuildReasonInitiator,
1711+
shouldNotify: false,
1712+
},
1713+
{
1714+
name: "initiated by autostart",
1715+
buildReason: database.BuildReasonAutostart,
1716+
shouldNotify: true,
1717+
},
1718+
}
1719+
1720+
for _, tc := range tests {
1721+
t.Run(tc.name, func(t *testing.T) {
1722+
t.Parallel()
1723+
1724+
ctx := context.Background()
1725+
notifEnq := &fakeNotificationEnqueuer{}
1726+
1727+
// Otherwise `(*Server).FailJob` fails with:
1728+
// audit log - get build {"error": "sql: no rows in result set"}
1729+
ignoreLogErrors := true
1730+
srv, db, ps, pd := setup(t, ignoreLogErrors, &overrides{
1731+
notificationEnqueuer: notifEnq,
1732+
})
1733+
1734+
user := dbgen.User(t, db, database.User{})
1735+
initiator := user
1736+
1737+
template := dbgen.Template(t, db, database.Template{
1738+
Name: "template",
1739+
Provisioner: database.ProvisionerTypeEcho,
1740+
OrganizationID: pd.OrganizationID,
1741+
})
1742+
template, err := db.GetTemplateByID(ctx, template.ID)
1743+
require.NoError(t, err)
1744+
file := dbgen.File(t, db, database.File{CreatedBy: user.ID})
1745+
workspace := dbgen.Workspace(t, db, database.Workspace{
1746+
TemplateID: template.ID,
1747+
OwnerID: user.ID,
1748+
OrganizationID: pd.OrganizationID,
1749+
})
1750+
version := dbgen.TemplateVersion(t, db, database.TemplateVersion{
1751+
OrganizationID: pd.OrganizationID,
1752+
TemplateID: uuid.NullUUID{
1753+
UUID: template.ID,
1754+
Valid: true,
1755+
},
1756+
JobID: uuid.New(),
1757+
})
1758+
build := dbgen.WorkspaceBuild(t, db, database.WorkspaceBuild{
1759+
WorkspaceID: workspace.ID,
1760+
TemplateVersionID: version.ID,
1761+
InitiatorID: initiator.ID,
1762+
Transition: database.WorkspaceTransitionDelete,
1763+
Reason: tc.buildReason,
1764+
})
1765+
job := dbgen.ProvisionerJob(t, db, ps, database.ProvisionerJob{
1766+
FileID: file.ID,
1767+
Type: database.ProvisionerJobTypeWorkspaceBuild,
1768+
Input: must(json.Marshal(provisionerdserver.WorkspaceProvisionJob{
1769+
WorkspaceBuildID: build.ID,
1770+
})),
1771+
OrganizationID: pd.OrganizationID,
1772+
})
1773+
_, err = db.AcquireProvisionerJob(ctx, database.AcquireProvisionerJobParams{
1774+
OrganizationID: pd.OrganizationID,
1775+
WorkerID: uuid.NullUUID{
1776+
UUID: pd.ID,
1777+
Valid: true,
1778+
},
1779+
Types: []database.ProvisionerType{database.ProvisionerTypeEcho},
1780+
})
1781+
require.NoError(t, err)
1782+
1783+
_, err = srv.FailJob(ctx, &proto.FailedJob{
1784+
JobId: job.ID.String(),
1785+
Type: &proto.FailedJob_WorkspaceBuild_{
1786+
WorkspaceBuild: &proto.FailedJob_WorkspaceBuild{
1787+
State: []byte{},
1788+
},
1789+
},
1790+
})
1791+
require.NoError(t, err)
1792+
1793+
if tc.shouldNotify {
1794+
// Validate that the notification was sent and contained the expected values.
1795+
require.Len(t, notifEnq.sent, 1)
1796+
require.Equal(t, notifEnq.sent[0].userID, user.ID)
1797+
require.Contains(t, notifEnq.sent[0].targets, template.ID)
1798+
require.Contains(t, notifEnq.sent[0].targets, workspace.ID)
1799+
require.Contains(t, notifEnq.sent[0].targets, workspace.OrganizationID)
1800+
require.Contains(t, notifEnq.sent[0].targets, user.ID)
1801+
require.Equal(t, "autobuild", notifEnq.sent[0].labels["initiator"])
1802+
require.Equal(t, string(tc.buildReason), notifEnq.sent[0].labels["reason"])
1803+
} else {
1804+
require.Len(t, notifEnq.sent, 0)
1805+
}
1806+
})
1807+
}
1808+
})
16981809
}
16991810

17001811
type overrides struct {

0 commit comments

Comments
 (0)