You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit e867e09
Browse filesBrowse the repository at this point in the historyBrowse files
🤖 fix: hold a deleted CoderControlPlane while CoderMCPServer objects still use it (#384)
## Summary
A deleted `CoderControlPlane` now waits for the `CoderMCPServer` objects
that use it, as #362 made it wait for `CoderAIGatewayKey` and
`CoderAIProvider` objects. Coder keeps running while it waits, so each
MCP server delete reaches Coder and retries on failure. Before this PR
the control plane went at once. The MCP server then made one 10-second
delete attempt or released with `ControlPlaneGone`, and its config could
stay in Coder, enabled and with its stored credential.
## What changes
1. `controlPlaneDependents` (`internal/controller/controlplane_hold.go`)
also lists `CoderMCPServer` objects that reference the control plane and
are being deleted with their cleanup finalizer, plus live ones for the
first 30 seconds, the same rule as the other two kinds. The per-kind
count for the grace period covers the new kind, and the grace period
uses the largest bounded delete timeout of the three kinds (all are 10
s).
2. `requestForDependentControlPlane` maps a `CoderMCPServer` to its
control plane, and the control plane controller watches `CoderMCPServer`
removals (`dependentRemoved`), so a waiting control plane goes at once.
3. The `CoderMCPServer` delete path checks `controlPlaneHolds`, as the
`CoderAIProvider` delete path does. While the control plane holds for
it, it takes the normal path (retry every 60 s, capped at the hold end)
instead of one bounded attempt and a release. Without this step, the
hold would keep Coder running but a failed attempt would still release
at once.
Under the hold, the pass runs with a context deadline at the hold end,
so a slow Coder cannot stretch it past the hold. The bounded pass after
the hold then fits in the grace period. The retry counts from the end of
the pass, so it lands right after the hold end (review rounds 2 and 3).
4. The how-to "Delete a server" section describes the hold and when the
control plane does not wait (namespace deletion, foreground deletion,
after 10 minutes).
5. How-to fix from the #381 assessment: a name in `access.groups` that
is not a valid Coder group name stalls with `GroupNotFound` and no
requeue (`codermcpserver_acl.go:147`). The doc said that it checks again
every 10 minutes. It now says that the object is not checked again until
the spec changes. The API server refuses such names, so this applies
only when admission was bypassed.
## Validation
- `TestCoderMCPServerDeleteUnderControlPlaneHold` (envtest): with a
control plane that is being deleted and an applied `CoderMCPServer` that
is being deleted, the control plane keeps its finalizer. A failed DELETE
(502) keeps the MCP finalizer with `CoderUnavailable` and a 60 s
requeue, and the control plane still waits. The retry deletes the
config, the MCP object goes, and then the control plane goes. Red before
the fix: "the control plane waits for the MCP server" failed.
- `TestMCPServerDeleteUnderHoldEndsAtHoldEnd` (unit, fake client): with
a hold that ends in 5 s and a Coder that hangs, the pass ends near the
hold end with `CoderUnavailable`, keeps the finalizer, and requeues
right after the hold end. Without the deadline it waits for the 30 s
request timeout (mutation checked).
- `TestRequestForDependentControlPlane` (unit): each of the three
dependent kinds maps to its control plane, and another kind maps to
nothing.
- Mutation checks: removing the `CoderMCPServer` case of the mapper
fails the unit test, forcing the bounded path in the MCP delete fails
the envtest test, and dropping the hold-end deadline fails the unit test
above.
- The existing `TestCoderMCPServerDeleteWhileControlPlaneDeletes`,
`TestControlPlane*`, `TestCoderAIProviderDelete*` and
`TestCoderAIGatewayKeyDelete*` tests pass unchanged.
- Local gates on the final head (`859d6bc7`): `make verify-vendor`,
`make build`, `make lint`, `GOFLAGS=-mod=vendor go tool govulncheck
./...`, `make test`, `make test-integration`, `go test -race
./internal/controller/`, and `make docs-check` pass. `make codegen`,
`make manifests`, `make build-installer` and `make docs-reference` leave
no diff. RBAC does not change: the manager role already grants `get`,
`list` and `watch` on `codermcpservers`.
- The full Kind E2E runs in CI on this head (the path filter matches
`internal/controller/codermcpserver*` and `controlplane_hold.go`).
## Not in this PR
- `docs/how-to/manage-ai-providers.md` names only `CoderAIProvider` and
`CoderAIGatewayKey` in its hold sentence. That page describes providers,
and #374 edits it, so I left it.
- MCP server releases without a delete do not count in
`coder_k8s_cleanup_released_without_delete_total`, and #353 part 2
(cleanup records and the sweep, #374 and #375) covers only
`CoderAIProvider`. Both are follow-ups for #353.
## Size
199 changed lines: 65 in the controllers, 129 in tests and 5 in the
how-to.
Fixes#382
Refs #353
Refs #155
---
_Generated with [`xum`](https://github.com/coder/xum) • Model:
`anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$224.47`_
<!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high
costs=224.47 -->
---------
Signed-off-by: Thomas Kosiewski <tk@coder.com>
Copy file name to clipboardExpand all lines: docs/how-to/manage-mcp-servers.md
+3-2Lines changed: 3 additions & 2 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -137,7 +137,7 @@ When a group in `access.groups` does not exist, the sync fails closed:
137
137
1. It removes every entry that `access.groups` no longer names, also user entries.
138
138
2. It adds nothing. Access never widens while a group is missing.
139
139
3. If the same spec generation disables the server (`enabled: false`), it sends one update that holds only `enabled: false`.
140
-
4. It leaves the rest of the generation unapplied. The object stalls with `GroupNotFound` and checks again every 10 minutes.
140
+
4. It leaves the rest of the generation unapplied. The object stalls with `GroupNotFound`. When the name is valid but no such group exists yet, the operator checks again every 10 minutes. The API server refuses a name that is not a valid Coder group name. If such a name reaches the operator anyway, the operator does not check again until the spec changes: fix the name.
141
141
5. Once the missing groups exist, the sync adds them and applies the rest of the generation.
142
142
143
143
A new server stays disabled in Coder while one of its groups is missing.
@@ -207,7 +207,8 @@ Deleting the object deletes the config in Coder. Coder removes the server from e
207
207
- The operator deletes only a config that this object created. With only an unconfirmed create, it reads Coder first. It deletes a config that carries this object's temporary display name, and it never touches another config with the slug. A create can still commit for a short time, so within 2 minutes of the create request the operator reads again before it trusts that the config is absent.
208
208
- When Coder is unreachable, or the control plane has no operator access, the finalizer stays and the operator retries every 60 seconds. The operator trusts only Coder's own 404 as proof that the config is gone. A 404 from a proxy keeps the finalizer.
209
209
- When the control plane is gone, nothing can delete the config. The finalizer goes at once. The operator logs the slug and the config ID with the reason `ControlPlaneGone`. Delete the config in the Coder dashboard.
210
-
- While the control plane is being deleted, the operator sends one delete with a 10-second limit, then removes the finalizer, also when that delete fails. So a namespace deletion finishes, but it can leave the config in Coder.
210
+
- When you delete the control plane while `CoderMCPServer` objects that use it are being deleted, the control plane waits for them for up to 10 minutes, plus a grace period for a last bounded attempt. It keeps its finalizer, so Coder keeps running, and the delete above runs with its retries. An object that you do not delete holds the control plane for only 30 seconds, so delete the objects together with the control plane. A foreground deletion (`kubectl delete --cascade=foreground`) does not wait.
211
+
- When the control plane does not wait (its namespace is being deleted, it is deleted in the foreground, or the 10 minutes have passed), the operator sends one delete with a 10-second limit, then removes the finalizer, also when that delete fails. So a namespace deletion finishes, but it can leave the config in Coder.
211
212
- A withdrawn opt-in never blocks a delete.
212
213
213
214
Delete `CoderMCPServer` objects before their control plane and before their namespace.
0 commit comments