Skip to content

Commit 0e4600c

Browse files
evgeniy-scherbinaevgeniy-scherbina
committed
fix: using prebuild status
1 parent 08981a4 commit 0e4600c

File tree

9 files changed

+127
-47
lines changed

9 files changed

+127
-47
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4178,6 +4178,10 @@ func (q *querier) UpdateOrganizationDeletedByID(ctx context.Context, arg databas
41784178
return deleteQ(q.log, q.auth, q.db.GetOrganizationByID, deleteF)(ctx, arg.ID)
41794179
}
41804180

4181+
func (q *querier) UpdatePrebuildStatus(ctx context.Context, arg database.UpdatePrebuildStatusParams) error {
4182+
panic("not implemented")
4183+
}
4184+
41814185
func (q *querier) UpdateProvisionerDaemonLastSeenAt(ctx context.Context, arg database.UpdateProvisionerDaemonLastSeenAtParams) error {
41824186
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceProvisionerDaemon); err != nil {
41834187
return err

‎coderd/database/dbmem/dbmem.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10867,6 +10867,15 @@ func (q *FakeQuerier) UpdateOrganizationDeletedByID(_ context.Context, arg datab
1086710867
return sql.ErrNoRows
1086810868
}
1086910869

10870+
func (q *FakeQuerier) UpdatePrebuildStatus(ctx context.Context, arg database.UpdatePrebuildStatusParams) error {
10871+
err := validateDatabaseType(arg)
10872+
if err != nil {
10873+
return err
10874+
}
10875+
10876+
panic("not implemented")
10877+
}
10878+
1087010879
func (q *FakeQuerier) UpdateProvisionerDaemonLastSeenAt(_ context.Context, arg database.UpdateProvisionerDaemonLastSeenAtParams) error {
1087110880
err := validateDatabaseType(arg)
1087210881
if err != nil {

‎coderd/database/dbmetrics/querymetrics.go

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/querier.go

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go

Lines changed: 30 additions & 11 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/prebuilds.sql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ SELECT
3333
tvp.id,
3434
tvp.name,
3535
tvp.desired_instances AS desired_instances,
36+
tvp.prebuild_status,
3637
t.deleted,
3738
t.deprecated != '' AS deprecated
3839
FROM templates t

‎coderd/database/queries/presets.sql

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,11 @@ SELECT
2525
unnest(@values :: TEXT[])
2626
RETURNING *;
2727

28+
-- name: UpdatePrebuildStatus :exec
29+
UPDATE template_version_presets
30+
SET prebuild_status = @status
31+
WHERE id = @preset_id;
32+
2833
-- name: GetPresetsByTemplateVersionID :many
2934
SELECT
3035
*

‎enterprise/coderd/prebuilds/reconcile.go

Lines changed: 65 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -361,49 +361,32 @@ func (c *StoreReconciler) ReconcilePreset(ctx context.Context, ps prebuilds.Pres
361361
slog.F("preset_name", ps.Preset.Name),
362362
)
363363

364-
if ps.IsHardLimited {
364+
// If the preset was previously hard-limited, log it and exit early.
365+
if ps.Preset.PrebuildStatus.PrebuildStatus == database.PrebuildStatusHardLimited {
365366
logger.Warn(ctx, "skipping hard limited preset", slog.F("preset_id", ps.Preset.ID), slog.F("name", ps.Preset.Name))
367+
return nil
368+
}
366369

367-
// TODO: rename ctx?
368-
// nolint:gocritic // Necessary to query all the required data.
369-
ctx := dbauthz.AsSystemRestricted(ctx)
370-
371-
// TODO(yevhenii): move into separate function
372-
// Send notification to template admins.
373-
if c.notifEnq == nil {
374-
c.logger.Warn(ctx, "notification enqueuer not set, cannot send resource replacement notification(s)")
375-
return nil
376-
}
370+
// If the preset reached the hard failure limit for the first time during this iteration:
371+
// - Mark it as hard-limited in the database
372+
// - Send notifications to template admins
373+
if ps.IsHardLimited {
374+
logger.Warn(ctx, "skipping hard limited preset", slog.F("preset_id", ps.Preset.ID), slog.F("name", ps.Preset.Name))
377375

378-
// TODO(yevhenii): remove owner from the list
379-
templateAdmins, err := c.store.GetUsers(ctx, database.GetUsersParams{
380-
RbacRole: []string{codersdk.RoleTemplateAdmin, codersdk.RoleOwner},
376+
err := c.store.UpdatePrebuildStatus(ctx, database.UpdatePrebuildStatusParams{
377+
Status: database.NullPrebuildStatus{
378+
PrebuildStatus: database.PrebuildStatusHardLimited,
379+
Valid: true,
380+
},
381+
PresetID: ps.Preset.ID,
381382
})
382383
if err != nil {
383-
return xerrors.Errorf("fetch template admins: %w", err)
384+
return err
384385
}
385386

386-
for _, templateAdmin := range templateAdmins {
387-
if _, err := c.notifEnq.EnqueueWithData(ctx, templateAdmin.ID, notifications.PrebuildFailureLimitReached,
388-
map[string]string{
389-
"org": ps.Preset.OrganizationName,
390-
"template": ps.Preset.TemplateName,
391-
"template_version": ps.Preset.TemplateVersionName,
392-
"preset": ps.Preset.Name,
393-
},
394-
map[string]any{},
395-
"prebuilds_reconciler",
396-
// Associate this notification with all the related entities.
397-
ps.Preset.TemplateID, ps.Preset.TemplateVersionID, ps.Preset.ID,
398-
); err != nil {
399-
c.logger.Error(ctx,
400-
"failed to send notification",
401-
slog.Error(err),
402-
slog.F("template_admin_id", templateAdmin.ID.String()),
403-
)
404-
405-
continue
406-
}
387+
err = c.notifyPrebuildFailureLimitReached(ctx, ps)
388+
if err != nil {
389+
return err
407390
}
408391

409392
return nil
@@ -502,6 +485,52 @@ func (c *StoreReconciler) ReconcilePreset(ctx context.Context, ps prebuilds.Pres
502485
}
503486
}
504487

488+
func (c *StoreReconciler) notifyPrebuildFailureLimitReached(ctx context.Context, ps prebuilds.PresetSnapshot) error {
489+
// TODO: rename ctx?
490+
// nolint:gocritic // Necessary to query all the required data.
491+
ctx = dbauthz.AsSystemRestricted(ctx)
492+
493+
// TODO(yevhenii): move into separate function
494+
// Send notification to template admins.
495+
if c.notifEnq == nil {
496+
c.logger.Warn(ctx, "notification enqueuer not set, cannot send resource replacement notification(s)")
497+
return nil
498+
}
499+
500+
// TODO(yevhenii): remove owner from the list
501+
templateAdmins, err := c.store.GetUsers(ctx, database.GetUsersParams{
502+
RbacRole: []string{codersdk.RoleTemplateAdmin, codersdk.RoleOwner},
503+
})
504+
if err != nil {
505+
return xerrors.Errorf("fetch template admins: %w", err)
506+
}
507+
508+
for _, templateAdmin := range templateAdmins {
509+
if _, err := c.notifEnq.EnqueueWithData(ctx, templateAdmin.ID, notifications.PrebuildFailureLimitReached,
510+
map[string]string{
511+
"org": ps.Preset.OrganizationName,
512+
"template": ps.Preset.TemplateName,
513+
"template_version": ps.Preset.TemplateVersionName,
514+
"preset": ps.Preset.Name,
515+
},
516+
map[string]any{},
517+
"prebuilds_reconciler",
518+
// Associate this notification with all the related entities.
519+
ps.Preset.TemplateID, ps.Preset.TemplateVersionID, ps.Preset.ID,
520+
); err != nil {
521+
c.logger.Error(ctx,
522+
"failed to send notification",
523+
slog.Error(err),
524+
slog.F("template_admin_id", templateAdmin.ID.String()),
525+
)
526+
527+
continue
528+
}
529+
}
530+
531+
return nil
532+
}
533+
505534
func (c *StoreReconciler) CalculateActions(ctx context.Context, snapshot prebuilds.PresetSnapshot) (*prebuilds.ReconciliationActions, error) {
506535
if ctx.Err() != nil {
507536
return nil, ctx.Err()

‎enterprise/coderd/prebuilds/reconcile_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -733,6 +733,11 @@ func TestSkippingHardLimitedPresets(t *testing.T) {
733733
// The outcome depends on whether the hard limit has been reached
734734
require.NoError(t, controller.ReconcileAll(ctx))
735735

736+
// These two additional calls to ReconcileAll should not trigger any notifications.
737+
// A notification is only sent once.
738+
require.NoError(t, controller.ReconcileAll(ctx))
739+
require.NoError(t, controller.ReconcileAll(ctx))
740+
736741
// Verify the final state after reconciliation
737742
workspaces, err = db.GetWorkspacesByTemplateID(ctx, template.ID)
738743
require.NoError(t, err)

0 commit comments

Comments
 (0)