feat: implement expiration policy logic for prebuilds #17996

ssncferreira · 2025-05-22T18:30:36Z

Summary

This PR introduces support for expiration policies in prebuilds. The TTL (time-to-live) is retrieved from the Terraform configuration (terraform-provider-coder PR):

prebuilds = {
	  instances = 2
	  expiration_policy {
		  ttl = 86400
	  }
  }

Note: Since there is no need for precise TTL enforcement down to the second, in this implementation expired prebuilds are handled in a single reconciliation cycle: they are deleted, and new instances are created only if needed to match the desired count.

Changes

The outcome of a reconciliation cycle is now expressed as a slice of reconciliation actions, instead of a single aggregated action.
Adjusted reconciliation logic to delete expired prebuilds and guarantee that the number of desired instances is correct.
Updated relevant data structures and methods to support expiration policies parameters.
Added documentation to Prebuilt workspaces page
Update terraform-provider-coder to version 2.5.0: https://github.com/coder/terraform-provider-coder/releases/tag/v2.5.0

Depends on: coder/terraform-provider-coder#404
Fixes: #17916

ssncferreira · 2025-05-22T18:46:25Z

coderd/prebuilds/global_snapshot.go

+// filterExpiredWorkspaces splits running workspaces into expired and non-expired
+// based on the preset's InvalidateAfterSecs TTL. If TTL is missing or zero,
+// all workspaces are considered non-expired.
+func filterExpiredWorkspaces(preset database.GetTemplatePresetsWithPrebuildsRow, runningWorkspaces []database.GetRunningPrebuiltWorkspacesRow) (nonExpired []database.GetRunningPrebuiltWorkspacesRow, expired []database.GetRunningPrebuiltWorkspacesRow) {


There’s an alternative to perform this filtering directly in the database, but for now, opted for an in-memory approach to keep the implementation simpler and the PR smaller. The main trade-off here is performance at scale, while in-memory filtering is sufficient for the current scale, we may want to revisit a DB-based solution if we encounter issues at higher scale. Let me know if you think a database solution would be a better fit now.

If I understand this correctly, the un-filtered data still comes directly from the database but is filtered in-memory.

From what I can tell, the changes required to move this logic inside the database would involve passing in the preset TTL and doing the TTL calculation inside the database. Is that correct?

If so, I agree that it can be kept in-memory for the moment, but we should add a follow-up issue to track this.

If I understand this correctly, the un-filtered data still comes directly from the database but is filtered in-memory.

Yes, that's correct, the filtering currently happens in-memory after fetching all the running prebuilds (expired and non-expired) from the database.

From what I can tell, the changes required to move this logic inside the database would involve passing in the preset TTL and doing the TTL calculation inside the database. Is that correct?

Right, moving it into the database would require doing the TTL check within the query. We don’t need to pass the preset TTL manually since it's already stored in the database, so the query can join against the template_version_presets table and compute the expiration inline.
We’d also need to update the current GetRunningPrebuiltWorkspacesRow function to return only the non-expired running prebuilds.

If so, I agree that it can be kept in-memory for the moment, but we should add a follow-up issue to track this.

I’ll create a follow-up issue to track moving it into the database for better efficiency/scalability 👍

ssncferreira · 2025-05-22T18:50:26Z

coderd/prebuilds/preset_snapshot.go

+// - Running: prebuilds running and non-expired
+// - Expired: prebuilds running and expired due to the preset's TTL
+// - InProgress: prebuilds currently in progress
+// - Backoff: holds failure info to decide if prebuild creation should be backed off
 type PresetSnapshot struct {
 	Preset        database.GetTemplatePresetsWithPrebuildsRow
 	Running       []database.GetRunningPrebuiltWorkspacesRow


I’m not a fan of the Running field name here, it’s a bit misleading since now it only includes non-expired running prebuilds. Maybe something like RunningValid would make that clearer. Open to other naming ideas too if anyone has suggestions.
I can make this change in a follow-up PR to avoid adding more entropy here.

coderd/prebuilds/preset_snapshot.go

…builds-cache-invalidation

coderd/prebuilds/preset_snapshot.go

…spaces page

…cy.ttl

…red prebuilds

…builds-cache-invalidation

SasSwart · 2025-05-26T13:35:31Z

LGTM, but given my proximity to the project, I'll defer formal approval to an independent reviewer.

johnstcn · 2025-05-26T13:40:09Z

coderd/prebuilds/global_snapshot.go

+// filterExpiredWorkspaces splits running workspaces into expired and non-expired
+// based on the preset's InvalidateAfterSecs TTL. If TTL is missing or zero,
+// all workspaces are considered non-expired.
+func filterExpiredWorkspaces(preset database.GetTemplatePresetsWithPrebuildsRow, runningWorkspaces []database.GetRunningPrebuiltWorkspacesRow) (nonExpired []database.GetRunningPrebuiltWorkspacesRow, expired []database.GetRunningPrebuiltWorkspacesRow) {


If I understand this correctly, the un-filtered data still comes directly from the database but is filtered in-memory.

From what I can tell, the changes required to move this logic inside the database would involve passing in the preset TTL and doing the TTL calculation inside the database. Is that correct?

If so, I agree that it can be kept in-memory for the moment, but we should add a follow-up issue to track this.

provisionersdk/proto/provisioner.proto

coderd/prebuilds/preset_snapshot_test.go

…policy changes

…builds-cache-invalidation

evgeniy-scherbina · 2025-05-26T17:20:52Z

Despite TestExpiredPrebuilds contains good test coverage, I noticed that we don't have new tests in reconcile_test.go which tests ReconcileAll API? So we don't test how ReconcilePreset handles reconciliation based on multiple actions?

ssncferreira · 2025-05-26T19:30:49Z

Despite TestExpiredPrebuilds contains good test coverage, I noticed that we don't have new tests in reconcile_test.go which tests ReconcileAll API? So we don't test how ReconcilePreset handles reconciliation based on multiple actions?

You’re right that TestExpiredPrebuilds covers the lower-level logic, but we currently don’t have tests directly targeting ReconcileAll or ReconcilePreset to verify the full reconciliation flow with multiple actions.

I agree it would be valuable to add tests at the ReconcileAll level to ensure the end-to-end behavior. As discussed internally, I will address those in a follow-up PR.

github-actions bot assigned ssncferreira May 22, 2025

ssncferreira mentioned this pull request May 22, 2025

feat: add expiration_policy parameter to prebuild resource coder/terraform-provider-coder#404

Merged

ssncferreira force-pushed the ssncferreira/feat-prebuilds-cache-invalidation branch from 8977d49 to 5a27ee4 Compare May 22, 2025 18:40

ssncferreira commented May 22, 2025

View reviewed changes

ssncferreira force-pushed the ssncferreira/feat-prebuilds-cache-invalidation branch from 5a27ee4 to ef33f48 Compare May 22, 2025 18:54

ssncferreira commented May 22, 2025

View reviewed changes

coderd/prebuilds/preset_snapshot.go Outdated Show resolved Hide resolved

ssncferreira force-pushed the ssncferreira/feat-prebuilds-cache-invalidation branch from ef33f48 to 5538be5 Compare May 22, 2025 18:59

feat: implement cache invalidation logic for prebuilds

a38ee7c

ssncferreira force-pushed the ssncferreira/feat-prebuilds-cache-invalidation branch from 5538be5 to a38ee7c Compare May 22, 2025 19:20

ssncferreira added 2 commits May 23, 2025 09:27

chore: add preset_id to log messages in reconciliation cycle

a904d3f

Merge remote-tracking branch 'origin/main' into ssncferreira/feat-pre…

b0abd30

…builds-cache-invalidation

SasSwart reviewed May 23, 2025

View reviewed changes

coderd/prebuilds/preset_snapshot.go Show resolved Hide resolved

ssncferreira added 7 commits May 23, 2025 14:32

docs: add documentation regarding cache_invalidation to Prebuilt work…

f43ea2c

…spaces page

fix: update code according to latest terraform schema expiration_poli…

c7e442c

…cy.ttl

fix: proto comment

28a6274

fix: preset_snapshot_test to create Preset with TTL via options

7482bfb

chore: update Actual state count to include both expired and non-expi…

04c0e7c

…red prebuilds

chore: update terraform-provider-coder to version v2.5.0

8e5caae

fix: run terraform generate.sh to update preset files

4b1fbb5

ssncferreira changed the title ~~feat: implement cache invalidation logic for prebuilds~~ feat: implement expiration policy logic for prebuilds May 26, 2025

ssncferreira added 2 commits May 26, 2025 11:46

fix: update resources_test to include ApiKeyScope parameter

30771cf

Merge remote-tracking branch 'origin/main' into ssncferreira/feat-pre…

266c445

…builds-cache-invalidation

ssncferreira marked this pull request as ready for review May 26, 2025 11:58

ssncferreira requested review from spikecurtis and johnstcn as code owners May 26, 2025 11:58

ssncferreira requested review from SasSwart and evgeniy-scherbina May 26, 2025 11:58

johnstcn reviewed May 26, 2025

View reviewed changes

evgeniy-scherbina reviewed May 26, 2025

View reviewed changes

coderd/prebuilds/preset_snapshot_test.go Show resolved Hide resolved

ssncferreira added 2 commits May 26, 2025 16:30

chore: update Provisioner API version 1.6 with prebuilds' expiration …

14be03f

…policy changes

chore: update preset snapshot tests test data for consistency

7a24eea

johnstcn approved these changes May 26, 2025

View reviewed changes

Merge remote-tracking branch 'origin/main' into ssncferreira/feat-pre…

7666090

…builds-cache-invalidation

ssncferreira merged commit 6f6e73a into main May 26, 2025
38 of 39 checks passed

ssncferreira deleted the ssncferreira/feat-prebuilds-cache-invalidation branch May 26, 2025 19:31

github-actions bot locked and limited conversation to collaborators May 26, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: implement expiration policy logic for prebuilds #17996

feat: implement expiration policy logic for prebuilds #17996

ssncferreira commented May 22, 2025 •

edited

Loading

Uh oh!

ssncferreira May 22, 2025 •

edited

Loading

Uh oh!

johnstcn May 26, 2025

Uh oh!

ssncferreira May 26, 2025 •

edited

Loading

Uh oh!

ssncferreira May 22, 2025

Uh oh!

Uh oh!

Uh oh!

SasSwart commented May 26, 2025

Uh oh!

johnstcn May 26, 2025

Uh oh!

Uh oh!

Uh oh!

evgeniy-scherbina commented May 26, 2025

Uh oh!

ssncferreira commented May 26, 2025

Uh oh!

Uh oh!

Uh oh!

feat: implement expiration policy logic for prebuilds #17996

feat: implement expiration policy logic for prebuilds #17996

Conversation

ssncferreira commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Uh oh!

ssncferreira May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

johnstcn May 26, 2025

Choose a reason for hiding this comment

Uh oh!

ssncferreira May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ssncferreira May 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

SasSwart commented May 26, 2025

Uh oh!

johnstcn May 26, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

evgeniy-scherbina commented May 26, 2025

Uh oh!

ssncferreira commented May 26, 2025

Uh oh!

Uh oh!

Uh oh!

ssncferreira commented May 22, 2025 •

edited

Loading

ssncferreira May 22, 2025 •

edited

Loading

ssncferreira May 26, 2025 •

edited

Loading