🌐 US-Proxy
class="logged-out env-production page-responsive" style="word-wrap: break-word;" >
Skip to content

Commit b080be4

Browse files
ibetitsmikeibetitsmike
authored
fix(coderd): show correct deletion time in dormancy notification (backport #26488) (#27895)
Code-only backport of #26488 to `release/2.34`. The dormancy notification's "will be automatically deleted in X" sentence rendered the dormancy threshold (`time_til_dormant`) instead of the auto-delete duration (`time_til_dormant_autodelete`). A 30-day threshold rendered as "4 weeks" even when auto-delete was configured to fire much sooner or later, so users were told the wrong deletion date. Unlike #26488, this backport contains **no migration**. Adding migration 000527 to the 2.34 line would break the migration ordering for deployments that later upgrade. Instead, the stored notification body is left untouched and the existing `timeTilDormant` label is populated with the correct value: - When the template has auto-delete configured, the label carries the countdown derived from the workspace's `deleting_at`, which `UpdateWorkspaceDormantDeletingAt` already computes atomically from `time_til_dormant_autodelete`. - When auto-delete is disabled, `deleting_at` is unset and the sentence cannot be omitted without a body change, so the label falls back to generic wording: "...will be automatically deleted in line with your template's auto-deletion policy if it remains inactive." Upgrading to >= 2.35.1 later applies migration 000527 and the `timeTilDelete` rename as usual; this patch is fully superseded at that point. <details> <summary>Implementation notes</summary> - `coderd/autobuild/lifecycle_executor.go`: propagate `wsNew.DeletingAt` onto `ws` after the dormancy UPDATE and humanize it into the `timeTilDormant` label. - `coderd/workspaces.go` (`putWorkspaceDormant`): use `newWorkspace.DeletingAt` for the label; the template fetch that fed the wrong duration is removed. - The label key intentionally stays `timeTilDormant` because the 2.34 notification body (migration 000311) references it; renaming would require a data migration, which this backport deliberately avoids. - No feature flag: the change is a pure correctness fix with no schema or API surface. - Tests adapted from #26488: `TestNotifications/DormancyAutoDelete` (lifecycle executor) and `TestWorkspaceNotifications/Dormant/InitiatorNotOwnerWithAutoDelete` (API path), plus fallback-wording assertions in the existing no-auto-delete tests. Both use a 35-day auto-delete so `humanize.Time` deterministically renders "1 month from now". </details> --- *This PR was generated by Coder Agents on behalf of @ibetitsmike.*
1 parent d547bea commit b080be4

5 files changed

Lines changed: 161 additions & 19 deletions

File tree

‎coderd/autobuild/lifecycle_executor.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"sync/atomic"
1212
"time"
1313

14-
"github.com/dustin/go-humanize"
1514
"github.com/google/uuid"
1615
"github.com/prometheus/client_golang/prometheus"
1716
"github.com/prometheus/client_golang/prometheus/promauto"
@@ -382,8 +381,10 @@ func (e *Executor) runOnce(t time.Time) Stats {
382381
Old: wsOld.WorkspaceTable(),
383382
New: wsNew,
384383
}
385-
// To keep the `ws` accurate without doing a sql fetch
384+
// Keep `ws` accurate without a sql fetch. The UPDATE derives
385+
// deleting_at from the template's time_til_dormant_autodelete.
386386
ws.DormantAt = wsNew.DormantAt
387+
ws.DeletingAt = wsNew.DeletingAt
387388

388389
shouldNotifyDormancy = true
389390

@@ -467,15 +468,14 @@ func (e *Executor) runOnce(t time.Time) Stats {
467468
}
468469
}
469470
if shouldNotifyDormancy {
470-
dormantTime := dbtime.Now().Add(time.Duration(tmpl.TimeTilDormant))
471471
_, err = e.notificationsEnqueuer.Enqueue(
472472
e.ctx,
473473
ws.OwnerID,
474474
notifications.TemplateWorkspaceDormant,
475475
map[string]string{
476476
"name": ws.Name,
477477
"reason": "inactivity exceeded the dormancy threshold",
478-
"timeTilDormant": humanize.Time(dormantTime),
478+
"timeTilDormant": notifications.DormantDeletionText(ws.DeletingAt),
479479
},
480480
"lifecycle_executor",
481481
ws.ID,

‎coderd/autobuild/lifecycle_executor_test.go‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1336,6 +1336,83 @@ func TestNotifications(t *testing.T) {
13361336
require.Contains(t, sent[0].Targets, workspace.ID)
13371337
require.Contains(t, sent[0].Targets, workspace.OrganizationID)
13381338
require.Contains(t, sent[0].Targets, workspace.OwnerID)
1339+
1340+
require.Equal(t, "line with your template's auto-deletion policy", sent[0].Labels["timeTilDormant"])
1341+
require.Equal(t, workspace.Name, sent[0].Labels["name"])
1342+
require.Equal(t, "inactivity exceeded the dormancy threshold", sent[0].Labels["reason"])
1343+
})
1344+
1345+
t.Run("DormancyAutoDelete", func(t *testing.T) {
1346+
t.Parallel()
1347+
1348+
var (
1349+
ticker = make(chan time.Time)
1350+
statCh = make(chan autobuild.Stats)
1351+
notifyEnq = notificationstest.FakeEnqueuer{}
1352+
// 35 days keeps humanize.Time deterministically in its "1 month" bucket.
1353+
timeTilDormant = time.Minute
1354+
timeTilDormantAutoDelete = 35 * 24 * time.Hour
1355+
client, db = coderdtest.NewWithDatabase(t, &coderdtest.Options{
1356+
AutobuildTicker: ticker,
1357+
AutobuildStats: statCh,
1358+
IncludeProvisionerDaemon: true,
1359+
NotificationsEnqueuer: &notifyEnq,
1360+
TemplateScheduleStore: schedule.MockTemplateScheduleStore{
1361+
SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions) (database.Template, error) {
1362+
template.TimeTilDormant = int64(options.TimeTilDormant)
1363+
template.TimeTilDormantAutoDelete = int64(options.TimeTilDormantAutoDelete)
1364+
return schedule.NewAGPLTemplateScheduleStore().Set(ctx, db, template, options)
1365+
},
1366+
GetFn: func(_ context.Context, _ database.Store, _ uuid.UUID) (schedule.TemplateScheduleOptions, error) {
1367+
return schedule.TemplateScheduleOptions{
1368+
UserAutostartEnabled: false,
1369+
UserAutostopEnabled: true,
1370+
DefaultTTL: 0,
1371+
AutostopRequirement: schedule.TemplateAutostopRequirement{},
1372+
TimeTilDormant: timeTilDormant,
1373+
TimeTilDormantAutoDelete: timeTilDormantAutoDelete,
1374+
}, nil
1375+
},
1376+
},
1377+
})
1378+
admin = coderdtest.CreateFirstUser(t, client)
1379+
version = coderdtest.CreateTemplateVersion(t, client, admin.OrganizationID, nil)
1380+
)
1381+
1382+
coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
1383+
template := coderdtest.CreateTemplate(t, client, admin.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) {
1384+
ctr.TimeTilDormantMillis = ptr.Ref(timeTilDormant.Milliseconds())
1385+
ctr.TimeTilDormantAutoDeleteMillis = ptr.Ref(timeTilDormantAutoDelete.Milliseconds())
1386+
})
1387+
userClient, _ := coderdtest.CreateAnotherUser(t, client, admin.OrganizationID)
1388+
workspace := coderdtest.CreateWorkspace(t, userClient, template.ID)
1389+
coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, workspace.LatestBuild.ID)
1390+
1391+
workspace = coderdtest.MustTransitionWorkspace(t, client, workspace.ID, codersdk.WorkspaceTransitionStart, codersdk.WorkspaceTransitionStop)
1392+
_ = coderdtest.AwaitWorkspaceBuildJobCompleted(t, userClient, workspace.LatestBuild.ID)
1393+
1394+
p, err := coderdtest.GetProvisionerForTags(db, time.Now(), workspace.OrganizationID, nil)
1395+
require.NoError(t, err)
1396+
1397+
notifyEnq.Clear()
1398+
tickTime := workspace.LastUsedAt.Add(timeTilDormant * 3)
1399+
coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime)
1400+
ticker <- tickTime
1401+
_ = testutil.TryReceive(testutil.Context(t, testutil.WaitShort), t, statCh)
1402+
1403+
workspace = coderdtest.MustWorkspace(t, client, workspace.ID)
1404+
require.NotNil(t, workspace.DormantAt)
1405+
1406+
sent := notifyEnq.Sent()
1407+
require.Len(t, sent, 1)
1408+
require.Equal(t, sent[0].TemplateID, notifications.TemplateWorkspaceDormant)
1409+
require.Contains(t, sent[0].Labels, "timeTilDormant")
1410+
require.Contains(t, sent[0].Labels["timeTilDormant"], "1 month",
1411+
"timeTilDormant must humanize TimeTilDormantAutoDelete, got %q",
1412+
sent[0].Labels["timeTilDormant"])
1413+
require.NotContains(t, sent[0].Labels["timeTilDormant"], "ago",
1414+
"timeTilDormant must be a future timestamp, got %q",
1415+
sent[0].Labels["timeTilDormant"])
13391416
})
13401417
}
13411418

‎coderd/notifications/dormancy.go‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
package notifications
2+
3+
import (
4+
"database/sql"
5+
6+
"github.com/dustin/go-humanize"
7+
)
8+
9+
// DormantDeletionText supplies the timeTilDormant label embedded in the stored
10+
// TemplateWorkspaceDormant "will be automatically deleted in ..." sentence.
11+
func DormantDeletionText(deletingAt sql.NullTime) string {
12+
if deletingAt.Valid {
13+
return humanize.Time(deletingAt.Time)
14+
}
15+
return "line with your template's auto-deletion policy"
16+
}

‎coderd/workspaces.go‎

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"strings"
1313
"time"
1414

15-
"github.com/dustin/go-humanize"
1615
"github.com/go-chi/chi/v5"
1716
"github.com/google/uuid"
1817
"golang.org/x/sync/errgroup"
@@ -1514,19 +1513,7 @@ func (api *API) putWorkspaceDormant(rw http.ResponseWriter, r *http.Request) {
15141513
)
15151514
}
15161515

1517-
tmpl, tmplErr := api.Database.GetTemplateByID(ctx, newWorkspace.TemplateID)
1518-
if tmplErr != nil {
1519-
api.Logger.Warn(
1520-
ctx,
1521-
"failed to fetch the template of the workspace marked as dormant",
1522-
slog.Error(err),
1523-
slog.F("workspace_id", newWorkspace.ID),
1524-
slog.F("template_id", newWorkspace.TemplateID),
1525-
)
1526-
}
1527-
1528-
if initiatorErr == nil && tmplErr == nil {
1529-
dormantTime := dbtime.Time(now).Add(time.Duration(tmpl.TimeTilDormant))
1516+
if initiatorErr == nil {
15301517
_, err = api.NotificationsEnqueuer.Enqueue(
15311518
// nolint:gocritic // Need notifier actor to enqueue notifications
15321519
dbauthz.AsNotifier(ctx),
@@ -1535,7 +1522,7 @@ func (api *API) putWorkspaceDormant(rw http.ResponseWriter, r *http.Request) {
15351522
map[string]string{
15361523
"name": newWorkspace.Name,
15371524
"reason": "a " + initiator.Username + " request",
1538-
"timeTilDormant": humanize.Time(dormantTime),
1525+
"timeTilDormant": notifications.DormantDeletionText(newWorkspace.DeletingAt),
15391526
},
15401527
"api",
15411528
newWorkspace.ID,

‎coderd/workspaces_test.go‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5156,6 +5156,68 @@ func TestWorkspaceNotifications(t *testing.T) {
51565156
require.Contains(t, sent[0].Targets, workspace.ID)
51575157
require.Contains(t, sent[0].Targets, workspace.OrganizationID)
51585158
require.Contains(t, sent[0].Targets, workspace.OwnerID)
5159+
require.Equal(t, "line with your template's auto-deletion policy", sent[0].Labels["timeTilDormant"])
5160+
})
5161+
5162+
t.Run("InitiatorNotOwnerWithAutoDelete", func(t *testing.T) {
5163+
t.Parallel()
5164+
5165+
// Given
5166+
var (
5167+
notifyEnq = &notificationstest.FakeEnqueuer{}
5168+
// 35 days keeps humanize.Time deterministically in its "1 month" bucket.
5169+
timeTilDormantAutoDelete = 35 * 24 * time.Hour
5170+
client = coderdtest.New(t, &coderdtest.Options{
5171+
IncludeProvisionerDaemon: true,
5172+
NotificationsEnqueuer: notifyEnq,
5173+
// The AGPL store ignores TimeTilDormantAutoDelete, so the mock
5174+
// writes it to the template row for deleting_at computation.
5175+
TemplateScheduleStore: schedule.MockTemplateScheduleStore{
5176+
SetFn: func(ctx context.Context, db database.Store, template database.Template, options schedule.TemplateScheduleOptions) (database.Template, error) {
5177+
template.TimeTilDormantAutoDelete = int64(options.TimeTilDormantAutoDelete)
5178+
return schedule.NewAGPLTemplateScheduleStore().Set(ctx, db, template, options)
5179+
},
5180+
GetFn: func(_ context.Context, _ database.Store, _ uuid.UUID) (schedule.TemplateScheduleOptions, error) {
5181+
return schedule.TemplateScheduleOptions{
5182+
UserAutostartEnabled: false,
5183+
UserAutostopEnabled: true,
5184+
DefaultTTL: 0,
5185+
AutostopRequirement: schedule.TemplateAutostopRequirement{},
5186+
TimeTilDormantAutoDelete: timeTilDormantAutoDelete,
5187+
}, nil
5188+
},
5189+
},
5190+
})
5191+
user = coderdtest.CreateFirstUser(t, client)
5192+
memberClient, _ = coderdtest.CreateAnotherUser(t, client, user.OrganizationID, rbac.RoleOwner())
5193+
version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
5194+
_ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
5195+
template = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) {
5196+
ctr.TimeTilDormantAutoDeleteMillis = ptr.Ref[int64](timeTilDormantAutoDelete.Milliseconds())
5197+
})
5198+
workspace = coderdtest.CreateWorkspace(t, client, template.ID)
5199+
_ = coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID)
5200+
)
5201+
5202+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
5203+
t.Cleanup(cancel)
5204+
5205+
// When
5206+
err := memberClient.UpdateWorkspaceDormancy(ctx, workspace.ID, codersdk.UpdateWorkspaceDormancy{
5207+
Dormant: true,
5208+
})
5209+
5210+
// Then
5211+
require.NoError(t, err, "mark workspace as dormant")
5212+
sent := notifyEnq.Sent(notificationstest.WithTemplateID(notifications.TemplateWorkspaceDormant))
5213+
require.Len(t, sent, 1)
5214+
require.Contains(t, sent[0].Labels, "timeTilDormant")
5215+
require.Contains(t, sent[0].Labels["timeTilDormant"], "1 month",
5216+
"timeTilDormant must humanize the workspace's deleting_at, got %q",
5217+
sent[0].Labels["timeTilDormant"])
5218+
require.NotContains(t, sent[0].Labels["timeTilDormant"], "ago",
5219+
"timeTilDormant must be a future timestamp, got %q",
5220+
sent[0].Labels["timeTilDormant"])
51595221
})
51605222

51615223
t.Run("InitiatorIsOwner", func(t *testing.T) {

0 commit comments

Comments
 (0)