Repository navigation
🤖 test: run the remaining CoderTemplateTest Kind phases - #211
Conversation
|
@codex security review |
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 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. |
The lifecycle driver now runs the plan 5.4 phases before the namespace deletion: fail=true ends Failed/AgentStartError with the workspace deleted; fail=notabool ends Failed/CreateRejected with no workspace; deleting the operator pod at WaitingForAgents still ends Succeeded; deleting a test at WaitingForAgents removes it and its workspace; a spec patch is refused as immutable; a server dry run stores no test and no workspace; and ttlSecondsAfterFinished=0 removes a test after a watched Succeeded. Workspaces rows prove one workspace per run (zero for the bad parameter and the dry run), and the receipt records each phase's duration. Offline stubs cover the success path and one failure per phase. Refs #152 Signed-off-by: Thomas Kosiewski <tk@coder.com> --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ Change-Id: I2625040736fc55f889c0c639e559ca81f7ab6630
Without spec.templateTests.allowRetain, the OwnershipUnknown message now says that only removing the finalizer releases the test. The controlPlaneGone doc comment is back above its function. Refs #152 Change-Id: Ic91a6816d0c08c397876f384defb10a52aaeb222 Signed-off-by: Thomas Kosiewski <tk@coder.com>
c024353 to
83b1eac
Compare
|
@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. Chef's kiss. 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". |
Summary
Plan PR 7b of the
CoderTemplateTeststack (Refs #152): the remaining Kind E2E phases from plan 5.4, split out of the activation PR (plan A16.5), plus two nits from the #210 assessment.Based on
mainafter #210 (activation) landed.New driver phases
They run after the three passing tests from #210 and before the key count. The namespace-deletion phase stays last.
fail=truemust endFailedAgentStartErrorwithWorkspaceDeleted=True(Deleted), and one workspace row.fail=notaboolmust endFailedCreateRejectedwithWorkspaceDeleted=True(NotCreated), and zero workspace rows.startup_delay=60, atRunning/WaitingForAgents, the driver deletes the operator pod and waits for a new Ready pod. The test must still endSucceeded, with one workspace row.startup_delay=60, atRunning/WaitingForAgents, the driver deletes the test. The object must disappear within 300 s, Coder must show a succeeded delete build for itsworkspaceID, and there must be one workspace row.spec.timeoutSecondsmust fail withspec is immutable.kubectl create --dry-run=servermust store no object. The driver derives the workspace name the test would get from the UID in the dry-run answer and requires zero rows with that name. It checks again after the TTL phase, so a late workspace is still caught.ttlSecondsAfterFinished: 0must disappear. A watch started before the create must show aMODIFIEDevent withSucceededandWorkspaceDeleted=True, then aDELETEDevent. This proves the test passed before it was removed.The receipt adds
phase_seconds. Workspace rows are counted in Coder'sworkspacestable, deleted rows included.#210 assessment nits
controlPlaneGonedoc comment is back above its function.retainAllowedhad ended up between them.OwnershipUnknownmessage no longer namesretainwhen the control plane does not setspec.templateTests.allowRetain. It then says that only removing the finalizer releases the test. With the opt-in, it names both. Test:TestTemplateTestCleanupRecheckschecks both messages, without Coder reads.The upgrade note (apply the new CRD before or together with the new image) goes into the docs PR 8b.
Offline tests
The stubs cover the success path and one failure scenario per phase:
fail-ignored,badparam-created,restart-failed,midrun-delete-failed,immutable-accepted,dryrun-persists, andttl-failed. Each one exits nonzero with a clear message.Validation
Local Kind replica of the whole
e2e-kindjob, including the step after the driver: the driver exited 0 with 28 passed cases in 306 s, and the job took 482 s.Offline driver tests,
make test-scripts, shellcheck, and actionlint pass.All on the pushed tree, with exit 0 each:
make verify-vendor,make test,make test-integration,make build,make lint,make codegen,make manifests,make docs-reference(no diff afterwards), andgo test -race ./internal/controller/....Known limits
startup_delay=60to outlast the operator restart, which took about 15 s locally. On a much slower runner, the test could finish before the restart. That would weaken the phase without failing it.Line count
Hand-written: 4 files changed, 155 insertions(+), 24 deletions(-) (no generated or vendored files).
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high