🤖 fix: return an empty namespaced LIST where no Coder backend serves the namespace - #214
Conversation
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. |
|
Readiness record for head
Generated with |
|
@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. Keep it up! 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". |
…e namespace (#209) Change-Id: I2df6cd4bc4b1e5c1362a422beba4c663183f189c Signed-off-by: Thomas Kosiewski <tk@coder.com>
…t eligible Change-Id: Ia4dc887f37a8b6516dd035c24649d98aae485e37 Signed-off-by: Thomas Kosiewski <tk@coder.com>
Change-Id: I3515ac9d98be4a5416aff96cf74e7d2dc1193d1b Signed-off-by: Thomas Kosiewski <tk@coder.com>
cbf77c2 to
13405d0
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. |
![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: 13405d04a7
ℹ️ 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".
1 participant
Summary
Fixes #209. A namespaced LIST of
coderworkspaces,codertemplates, orcodertemplateversionsin a namespace that no Coder backend serves now returns200with an empty list instead of an error. With this change, a namespace without aCoderControlPlanefinishes deleting. GET, CREATE, UPDATE, DELETE and the subresources keep their current errors.Background
The namespace controller lists every deletable resource type in a namespace before it removes the namespace. The aggregated API answered that LIST with
503 ServiceUnavailable("no eligible CoderControlPlane instances found in namespace ...") since #53. The namespace controller treats the error as remaining content, so every namespace without an eligible control plane stayedTerminating. That included an empty namespace, and thecodernamespace after its control plane was gone.Implementation
internal/aggregated/coder:ClientForNamespacemarks the errors that mean "no backend serves this namespace" with an unexported wrapper type.coder.IsNamespaceNotServed(err)detects the mark. The wrapper unwraps to the originalStatusError, so every other verb returns the same status and message as before. Two errors get the mark:allmode: the named namespace contains noCoderControlPlaneat all (the503). A namespace whose control plane exists but is not eligible keeps the unmarked503.--app=aggregated-apiserver): a namespace other than--coder-namespace(the400). This mode had the same hang for every other namespace.internal/aggregated/storage:listsUnservedNamespace(ctx, err)is true only when the request names a namespace and the error carries the mark. The three LIST paths then return a typed list withitems: [], without a Coder request.operatorAccessReady=false), two eligible control planes in one namespace (400), token Secret or URL problems of an eligible control plane, and an unpinned standalone provider.Decisions on cluster-wide LIST and WATCH
503. The namespace controller never sends a cluster-wide LIST, so 🤖 Namespace deletion hangs: the aggregated API answers LIST with 503 when the namespace has no control plane #209 does not need it. Keeping the503keeps the clear "no eligible CoderControlPlane" message forkubectl get -Aon a new install. The garbage collector and quota controllers only log the failing informer.operatorAccessReady=falseon transient Postgres or bootstrap errors (reconcileOperatorAccess). If such a short outage emptied the LIST, watch clients would see every object as deleted. So a namespace that contains anyCoderControlPlanekeeps the503. Namespace deletion still finishes: the namespace controller continues through all resource types after a LIST error (deleteAllContentin kube-controller-manager), so it deletes the control plane in the same pass. The next pass gets empty lists. Thee2e-kindjob checks this order: it deletes thecodernamespace while its control plane still exists.E2E driver
The namespace-deletion phase of
hack/e2e-workspace-lifecycle.sh(added by #210) now waits until the namespace is gone (NotFound). It no longer acceptsNamespaceContentRemaining=FalseandNamespaceFinalizersRemaining=Falseas success. From the first poll after the delete, while the namespace exists, the driver logs its phase andTrueconditions each time they change. A run thus showsNamespaceDeletionContentFailure(the aggregated LIST503while the control plane still exists) and the diagnostics for a failure. The offline tests add anns-stays-terminatingscenario: a namespace that staysTerminating(the #209 symptom) now fails the phase with a timeout.Validation
TestNamespacedListInUnservedNamespaceReturnsEmptyListfailed onmainwith the 🤖 Namespace deletion hangs: the aggregated API answers LIST with 503 when the namespace has no control plane #209 error, then passed with the fix.operatorAccessReady=falsemust return503, and the provider must not mark that error. Both tests failed before the change.503as a top-levelStatusError, all-namespaces LIST keeps503, two eligible control planes keep400, an unreadable token Secret and an unpinned standalone provider stay errors, and WATCH succeeds.make verify-vendor,make test,make test-integration,make build,make lint,go test -raceoninternal/aggregated/coderandinternal/aggregated/storage,bash hack/e2e-workspace-lifecycle_test.sh(all offline tests pass),make docs-check, shellcheck, markdownlint and cspell on the changed docs.--app=allwith the APIService, noCoderControlPlane). With an image built frommain(dabb113), an empty namespace stayedTerminatingwithNamespaceDeletionContentFailure: Failed to delete all resource types, 2 remaining: no eligible CoderControlPlane .... After the switch to the image from this branch, the same stuck namespace disappeared within 5 s. A fresh empty namespace was deleted in 5 s. In that namespace, LIST returneditems: [], while GET, DELETE and CREATE still returnedServiceUnavailable, andkubectl get coderworkspaces -Astill returned503. The fulle2e-kindCI job runs the updated driver phase on this PR.Kind transcript, before the fix (image from main), condensed
Kind transcript, after the fix (image from this branch), condensed
Risks
Low to medium, limited to aggregated LIST responses. A client that relied on the LIST
503to detect a namespace without a control plane now gets an empty list. A namespace with a control plane that is not ready keeps the503. The reference docs describe both. GET and write paths are unchanged.Generated with
mux• Model:anthropic:claude-opus-5-5• Thinking:high