Skip to content

Commit bca5c35

Browse files
evgeniy-scherbinaevgeniy-scherbina
authored
fix: remove notifications for hard-limited prebuilds (#18528)
Relates to coder/internal#674 Currently, we send notifications to **all template admins** for **every failed and hard-limited preset**. This can generate excessive noise—especially when someone is debugging a template and creates multiple broken versions in quick succession. For now, we've decided to remove hard-limited preset notifications to reduce excessive noise. In the long term, we plan to aggregate failure information and deliver it on a daily or weekly basis.
1 parent 7b152cd commit bca5c35

File tree

2 files changed

+0
-87
lines changed

2 files changed

+0
-87
lines changed

‎enterprise/coderd/prebuilds/reconcile.go

Lines changed: 0 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,6 @@ func (c *StoreReconciler) ReconcilePreset(ctx context.Context, ps prebuilds.Pres
427427

428428
// If the preset reached the hard failure limit for the first time during this iteration:
429429
// - Mark it as hard-limited in the database
430-
// - Send notifications to template admins
431430
// - Continue execution, we disallow only creation operation for hard-limited presets. Deletion is allowed.
432431
if ps.Preset.PrebuildStatus != database.PrebuildStatusHardLimited && ps.IsHardLimited {
433432
logger.Warn(ctx, "preset is hard limited, notifying template admins")
@@ -439,11 +438,6 @@ func (c *StoreReconciler) ReconcilePreset(ctx context.Context, ps prebuilds.Pres
439438
if err != nil {
440439
return xerrors.Errorf("failed to update preset prebuild status: %w", err)
441440
}
442-
443-
err = c.notifyPrebuildFailureLimitReached(ctx, ps)
444-
if err != nil {
445-
logger.Error(ctx, "failed to notify that number of prebuild failures reached the limit", slog.Error(err))
446-
}
447441
}
448442

449443
state := ps.CalculateState()
@@ -474,49 +468,6 @@ func (c *StoreReconciler) ReconcilePreset(ctx context.Context, ps prebuilds.Pres
474468
return multiErr.ErrorOrNil()
475469
}
476470

477-
func (c *StoreReconciler) notifyPrebuildFailureLimitReached(ctx context.Context, ps prebuilds.PresetSnapshot) error {
478-
// nolint:gocritic // Necessary to query all the required data.
479-
ctx = dbauthz.AsSystemRestricted(ctx)
480-
481-
// Send notification to template admins.
482-
if c.notifEnq == nil {
483-
c.logger.Warn(ctx, "notification enqueuer not set, cannot send prebuild is hard limited notification(s)")
484-
return nil
485-
}
486-
487-
templateAdmins, err := c.store.GetUsers(ctx, database.GetUsersParams{
488-
RbacRole: []string{codersdk.RoleTemplateAdmin},
489-
})
490-
if err != nil {
491-
return xerrors.Errorf("fetch template admins: %w", err)
492-
}
493-
494-
for _, templateAdmin := range templateAdmins {
495-
if _, err := c.notifEnq.EnqueueWithData(ctx, templateAdmin.ID, notifications.PrebuildFailureLimitReached,
496-
map[string]string{
497-
"org": ps.Preset.OrganizationName,
498-
"template": ps.Preset.TemplateName,
499-
"template_version": ps.Preset.TemplateVersionName,
500-
"preset": ps.Preset.Name,
501-
},
502-
map[string]any{},
503-
"prebuilds_reconciler",
504-
// Associate this notification with all the related entities.
505-
ps.Preset.TemplateID, ps.Preset.TemplateVersionID, ps.Preset.ID, ps.Preset.OrganizationID,
506-
); err != nil {
507-
c.logger.Error(ctx,
508-
"failed to send notification",
509-
slog.Error(err),
510-
slog.F("template_admin_id", templateAdmin.ID.String()),
511-
)
512-
513-
continue
514-
}
515-
}
516-
517-
return nil
518-
}
519-
520471
func (c *StoreReconciler) CalculateActions(ctx context.Context, snapshot prebuilds.PresetSnapshot) ([]*prebuilds.ReconciliationActions, error) {
521472
if ctx.Err() != nil {
522473
return nil, ctx.Err()

‎enterprise/coderd/prebuilds/reconcile_test.go

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -853,11 +853,6 @@ func TestSkippingHardLimitedPresets(t *testing.T) {
853853
cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{})
854854
controller := prebuilds.NewStoreReconciler(db, pubSub, cache, cfg, logger, clock, registry, fakeEnqueuer)
855855

856-
// Template admin to receive a notification.
857-
templateAdmin := dbgen.User(t, db, database.User{
858-
RBACRoles: []string{codersdk.RoleTemplateAdmin},
859-
})
860-
861856
// Set up test environment with a template, version, and preset.
862857
ownerID := uuid.New()
863858
dbgen.User(t, db, database.User{
@@ -938,20 +933,6 @@ func TestSkippingHardLimitedPresets(t *testing.T) {
938933
require.Equal(t, 1, len(workspaces))
939934
require.Equal(t, database.PrebuildStatusHardLimited, updatedPreset.PrebuildStatus)
940935

941-
// When hard limit is reached, a notification should be sent.
942-
matching := fakeEnqueuer.Sent(func(notification *notificationstest.FakeNotification) bool {
943-
if !assert.Equal(t, notifications.PrebuildFailureLimitReached, notification.TemplateID, "unexpected template") {
944-
return false
945-
}
946-
947-
if !assert.Equal(t, templateAdmin.ID, notification.UserID, "unexpected receiver") {
948-
return false
949-
}
950-
951-
return true
952-
})
953-
require.Len(t, matching, 1)
954-
955936
// When hard limit is reached, metric is set to 1.
956937
mf, err = registry.Gather()
957938
require.NoError(t, err)
@@ -1016,11 +997,6 @@ func TestHardLimitedPresetShouldNotBlockDeletion(t *testing.T) {
1016997
cache := files.New(prometheus.NewRegistry(), &coderdtest.FakeAuthorizer{})
1017998
controller := prebuilds.NewStoreReconciler(db, pubSub, cache, cfg, logger, clock, registry, fakeEnqueuer)
1018999

1019-
// Template admin to receive a notification.
1020-
templateAdmin := dbgen.User(t, db, database.User{
1021-
RBACRoles: []string{codersdk.RoleTemplateAdmin},
1022-
})
1023-
10241000
// Set up test environment with a template, version, and preset.
10251001
ownerID := uuid.New()
10261002
dbgen.User(t, db, database.User{
@@ -1125,20 +1101,6 @@ func TestHardLimitedPresetShouldNotBlockDeletion(t *testing.T) {
11251101
require.NoError(t, err)
11261102
require.Equal(t, database.PrebuildStatusHardLimited, updatedPreset.PrebuildStatus)
11271103

1128-
// When hard limit is reached, a notification should be sent.
1129-
matching := fakeEnqueuer.Sent(func(notification *notificationstest.FakeNotification) bool {
1130-
if !assert.Equal(t, notifications.PrebuildFailureLimitReached, notification.TemplateID, "unexpected template") {
1131-
return false
1132-
}
1133-
1134-
if !assert.Equal(t, templateAdmin.ID, notification.UserID, "unexpected receiver") {
1135-
return false
1136-
}
1137-
1138-
return true
1139-
})
1140-
require.Len(t, matching, 1)
1141-
11421104
// When hard limit is reached, metric is set to 1.
11431105
mf, err = registry.Gather()
11441106
require.NoError(t, err)

0 commit comments

Comments
 (0)