Repository navigation
🤖 fix: confirm CoderWorkspacePoolMember bind extensions by Coder's deadline and check the start build initiator - #339
Conversation
…line and check the start build initiator Change-Id: I431d9dab372a21a885caef16659f5e9782a2fad3 Signed-off-by: Thomas Kosiewski <tk@coder.com>
…ne at bind Change-Id: I5b3d8d99f493c268d3b692baed5be0bec89e39a6 Signed-off-by: Thomas Kosiewski <tk@coder.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex security review |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 10dc11cb15
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Change-Id: I17d4226fdbc86196ea985c14a760c32b1917fa73 Signed-off-by: Thomas Kosiewski <tk@coder.com>
|
@codex review |
|
@codex security review |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
This PR fixes the deadline extension of a ready
CoderWorkspacePoolMemberand makes its identity read check who started the start build. The reconciler is still dormant (nothing registers it), so no user sees a change.status.bindExtendDeadline. Only a Coder deadline at or after that target setsbindExtended. So a crash, a lost status write, a 409, a 429, or an uncertain answer never leavesbindExtended=truewithout an extension. They also never cause a second extension after one that took effect (Codex 4267240452, 4267405687 on 🤖 feat: keep CoderWorkspacePoolMember workspaces warm and extend once at bind #324).workspaceIDandstartBuildIDcannot point the member at akwp-<uid>workspace or build that the operator did not start for it. This covers readiness and every extension. The delete step (PR 8c) uses the same check. (🤖 feat: confirm uncertain CoderWorkspacePoolMember creates and apply the readiness rules #322 and 🤖 feat: keep CoderWorkspacePoolMember workspaces warm and extend once at bind #324 final assessors.)max_deadline. Without room for at least 30 minutes, the member sends no request: an unclaimed member waits withMaxDeadline, a bound member counts the bind as done. Any other 400 (for example clock skew past Coder's 29-minute rule) is a wait withExtendRefused(advisor note on 🤖 feat: keep CoderWorkspacePoolMember workspaces warm and extend once at bind #324).Refs #153
Background
This is PR 8a of the
CoderWorkspacePoolstack (#153). The plan's PR 8 row lists these items with the trigger "fix before PR 12 registers the reconciler". PR 8b (watch rules) and PR 8c (cleanup) follow.createHadNoEffectis unchanged: #319 owns it. The extend path no longer uses it, because the next read decides every non-final answer.Implementation
Coder v2.37.3 answers
PUT /workspaces/{id}/extendwith 409 when the latest build is not a finished start build or the deadline is manual, and with 400 when the new deadline is less than 29 minutes ahead or aftermax_deadline(coderd/workspaces.go:1640-1700,3057-3061).keepWarm(coderworkspacepoolmember_extend.go):bindExtendedsetbindExtended(unchanged).bindExtendDeadlineset, Coder deadline at or after itbindExtended. No request.bindExtended. No request, so a bind never shortens the deadline.MaxDeadline. Bound: setsbindExtended.bindExtendDeadline, then reads the clock again.now + max(TTL, 30 min), at mostmax_deadline, in whole seconds (status stores seconds). A bound member never asks for less than its stored target.bindExtended. Unclaimed: setslastExtendTimeto the clock reading the new deadline counts from (Codex 4267760273).bindExtended, no retry. Unclaimed: waitExtendRefused.CoderUnavailable. The next read decides: a bound member retries only when Coder's deadline is still before the target.readOwnPoolMemberWorkspace(coderworkspacepoolmember_ready.go) now reads the operator user (GET /users/me) and fails withOwnershipUnknown(WorkspaceDeleted=Unknown) when the start build's initiator, transition, or template version is wrong. A claimed Coder prebuild still passes, because its claim build is the operator's start build.API: one new optional status field,
CoderWorkspacePoolMember.status.bindExtendDeadline. The CRD is still inconfig/crd/dormant/, and the member type has no API reference page yet, somake docs-referencechanges nothing.Validation
Tests (
coderworkspacepoolmember_extend_test.go,coderworkspacepoolmember_ready_test.go):TestPoolMemberKeepsUnclaimedWorkspaceWarm: a 90-minute max deadline caps the extension at it, then two polls inside the last 15 minutes send no request (MaxDeadline). A 400 is a waitExtendRefusedthat leaves the deadline.TestPoolMemberExtendsOnceAtBind(plan mutation 12): the 409 case now also checks the stored target. A slow target write (clock readingsstarted,started,late) storesstarted + 30mbut sendslate + 30m, which Coder accepts. A max deadline too near for an extension ends the bind extension without a request.TestPoolMemberBindExtensionConfirmed(replacesTestPoolMemberBindExtensionAtMostOnce): a lost status write after a successful request (no second request), a crash right after the target write (the next poll extends), a 409 followed by a conflicting status write (the next poll extends), 502 without effect (sent again), 502 after commit (not sent again), 429 (sent again), and a bound 400 (not tried again).TestPoolMemberGuardsEveryExtensionandTestPoolMemberReadinessFailures: a status that names a start build the owner started on the member's own workspace fails withOwnershipUnknownand no Coder write.Mutation checks: I removed one control at a time, and each removal made a named test fail (12 of 12): the confirm-by-read rule, the target write before the request, the fresh deadline after it, the max-deadline cap, the no-room rule (unclaimed and bound), the bound 400 rule, the
ExtendRefusedreason, the start build initiator check,bindExtendedafter success, and the earlier mutations 12 and no-shorten.Gates on the final commit: drift (codegen, manifests, installer, API reference: no diff),
make docs-check, dormant-CRD guard, docs lint and spelling,make verify-vendor,make build,make lint, actionlint,make test,make test-integration,make vuln, race. All passed (on the same tree before a rebase onto one unrelated main commit, #335).Size
319 hand-written lines (113 production, 206 test, counting removals), plus 12 generated lines (deepcopy and the dormant CRD).
Risks
None for users. The reconciler is dormant: nothing registers it.
Generated with
mux• Model:anthropic:claude-opus-5-5• Thinking:high• Cost:$121.59