feat: add workspace build orchestration storage - #25757
Conversation
Add the workspace_build_orchestrations table, SQLC queries, generated store wrappers, and dbauthz coverage for durable follow-up workspace build intent. This includes insert, claim, completion, failure, cancellation, and retry update queries, plus authorization for creating orchestration rows and template version pin handling for child builds. Ref: https://linear.app/codercom/issue/PLAT-143/add-workspace-restart-functionality-to-api Ref: #5800
92bf471 to
17867b6
Compare
Emyrk
left a comment
There was a problem hiding this comment.
Note for my future self:
- Prometheus metrics?
There was a problem hiding this comment.
Review walkthrough on storage layer for the workspace build orchestration stack. Schema, queries, and dbauthz coverage all look solid. One blocker (lint) plus a handful of comments and callouts inline.
Reviewed by Coder Agents and @Emyrk.
Docs preview📖 View docs preview for |
|
@dannykopping, looping you in because you're the author of #14857, which is related to this. I've built this stack based on the requirements of #5800. It lays down the engine for 2-step, builds-only transitions (currently stop -> start): the start build is now issued server-side after the stop succeeds, so restart no longer needs the client to stick around. This stack is the foundation only - follow-up PRs will rewire the CLI/UI onto it and actually close #5800. @matifali pointed me at #14857, which proposes 3+ step transitions for more complex operations (transfers, snapshots, prebuild reassignments). I could refactor toward that, but without a concrete use case I'd be guessing at the schema. My best read of transfers, for example, is that it needs at least one step that isn't a workspace build (changing the workspace owner id), so the model would have to persist non-build step intent and the worker would have to execute it - that's a schema change plus new worker dispatch. Are there actual plans to implement any 3+ step operations soon? If not, I'm leaning toward shipping this as is and closing #5800 via the follow-ups, then adding 3+ support once there's a real feature driving the schema. The downside is having to do a data migration (backfilling any existing rows) and continue maintaining OnSuccess as legacy API surface (?) alongside whatever #14857 introduces. Let me know what you think. |
@geokat I'm all for the simplified approach using concrete requirements for now 👌 thanks for looking into this, it'll be a big UX win! Was there an RFC for this? |
|
@dannykopping that's good to hear, thanks!
No RFC. Didn't seem necessary at the moment since it's a minimal user-facing addition and the use case at hand is clear-cut (replace the current restart functionality). |
|
The technical implementation is the thing I'm interested in aligning on here. I wasn't expecting a whole new resource type to be introduced for this functionality; I'd be interested in hearing how we arrived at this design and how it might evolve. |
|
Thanks again for your input @dannykopping! The new resource type was added to avoid using The Design Note above provides a high level overview of the design arrived at after considering the discussion in #7070. How it might evolve: as mentioned above, it could be expanded to support 3+ step orchestrations; likely by adding another table ( |
To be more precise, I meant the |
The orchestrator was constructed with api.Clock and starts an always-on backup-poll clock.NewTicker on it. In tests that drive the mock clock deterministically (the autobuild lifecycle tests), that shared ticker makes TestWorkspaceAutobuild/NextStartAtIsValid fail date-dependently: NextStartAt fails to recompute past the current tick. The orchestrator is driven by pubsub wakes; the ticker is only a backup poll, so a real clock is correct and keeps its ticker off the mock clock. Empirically this resolves the failure; the wake-driven path is unaffected (it fires in real time).
This PR is part of a stack that adds durable server-side workspace restart support
to the API.
Add database storage for durable workspace build orchestration rows. These rows
represent follow-up workspace build intent, such as a child build that should be
created after a parent build reaches a terminal state.
This adds the
workspace_build_orchestrationstable, SQLC queries for insert,claim, completion, failure, cancellation, and retry updates, plus generated store
wrappers and dbauthz coverage. The dbauthz layer authorizes parent and child build
intent before inserting rows and prevents non-template-admin users from creating
durable child template version pins.
Ref: https://linear.app/codercom/issue/PLAT-143/add-workspace-restart-functionality-to-api
Ref: #5800
This PR is part of a stack that merges into
main:Created with stakk
Design note
Prior discussions: #7070.
This stack models restart as durable orchestration of existing stop and start
workspace builds instead of adding a new restart transition. Keeping restart as
two existing transitions preserves the current build/provisioner model.
A dedicated restart transition would also have unclear template semantics.
Existing transitions describe concrete lifecycle intent (
start,stop,delete), whilerestartis a compound operation whose implementation could varyby provisioner or template. Modeling restart as a durable stop followed by a start
keeps template behavior on the existing transition semantics.
The child start build is created only after the parent stop build succeeds, rather
than being inserted immediately in a pending state. That keeps
workspace_buildsaligned with actual provisioner-ready work and avoids introducing a second
pending-build lifecycle that the provisioner and build acquisition paths would
need to understand.