Namespace deletion leaves AI Gateway keys and AI providers in Coder #353

Activity

1. ThomasK33

   self-assigned this

   on Oct 14, 2026
2. ThomasK33 commented on Oct 14, 2026

   

   ThomasK33

   on Oct 14, 2026

   MemberAuthor

   More actions

   Maintainer decision: this must land before the next release tag (v0.1.5). It does not block merging the CoderAIGatewayKey activation (#346) to main, because no released version runs that controller yet, and the how-to documents the cleanup.

   CoderAIProvider already shipped in v0.1.4 with the same gap. The v0.1.4 notes say to delete CoderAIProvider objects before their control plane, and that a provider deleted after its control plane stays in Coder. So the provider half is a fix for released behavior, and should go first if the work is split.

   Prefer option 2 (hold the control plane's deletion while dependent objects exist) or a combination, and include items q and u and the metric or Event in the decision.

   ---

   Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: medium
3. ThomasK33

   added this to the v0.1.5 milestone on Oct 14, 2026
4. ThomasK33 commented on Oct 14, 2026

   

   ThomasK33

   on Oct 14, 2026

   Last edited by ThomasK33

   MemberAuthor

   More actions

   Follow-ups from the Codex review of #362 (part 1, the control-plane hold). I deferred both at first: each one only delays a control-plane deletion by at most the 10-minute hold, and neither leaks more than the code before #362 did. Owner: me, in the next #353 part.

   1. Operator access is not repaired during the hold (🤖 fix: hold a deleted CoderControlPlane while AI Gateway keys and providers still use it #362 (comment)). When OperatorAccessReady is false at deletion time, the deletion path never runs reconcileOperatorAccess again. Dependents wait with OperatorAccessNotReady for 10 minutes and then release without a delete. Before 🤖 fix: hold a deleted CoderControlPlane while AI Gateway keys and providers still use it #362 they released at once with the same leak. Fix: skip the hold when operator access is not ready, or keep repairing it during the hold.
   2. Fixed in 🤖 fix: hold a deleted CoderControlPlane while AI Gateway keys and providers still use it #362 (49e6fdc): A terminating dependent with another finalizer keeps the hold (🤖 fix: hold a deleted CoderControlPlane while AI Gateway keys and providers still use it #362 (comment)). After its cleanup finalizer goes, an object that has a second finalizer still counts as a dependent until the 10 minutes pass. Fix: a terminating dependent holds only while it still has its AI cleanup finalizer.
   3. Queue backlog at the hold deadline (🤖 fix: hold a deleted CoderControlPlane while AI Gateway keys and providers still use it #362 (comment)). The grace period counts only this control plane's dependents, but one worker per kind also serves unrelated objects. A backlog of slow reconciles can use up the grace, and the dependent then releases without a delete (the metric counts it). Fix: an explicit cleanup-done acknowledgement, or the cleanup records of the next part.
   4. Foreground deletion seen after its marker is gone (🤖 fix: hold a deleted CoderControlPlane while AI Gateway keys and providers still use it #362 (comment)). When the operator is down during a foreground delete, the foregroundDeletion finalizer can be gone before the first reconcile, so the hold starts although Coder is gone. Dependents then retry for up to the hold plus the grace. Fix: also check that the owned Deployment exists before holding.
   5. Provider delete path reads the wall clock (advisor note on 🤖 fix: hold a deleted CoderControlPlane while AI Gateway keys and providers still use it #362). coderaiprovider_delete.go uses time.Now() for the hold check, while the key path uses r.Clock.Now(). Fix: use the reconciler clock, so tests can drive the provider hold end.
   6. Deployment check and delete are two requests (🤖 feat: write a cleanup record before each CoderAIProvider create (#353 part 2a) #374 (comment)). During a rolling update that changes the control plane database, the /buildinfo check and the DELETE can reach pods on different databases, so a 404 from the new one can remove the record while the provider stays in the old one. Fix: recreate the Deployment on a database change, or check the deployment again after the delete. The same window applies to the sweep in 🤖 feat: sweep the cleanup records of CoderAIProvider objects that went without a delete (#353 part 2b) #375 (cleanup_sweep.go): its buildinfo match and its DELETE are also two requests.
   7. ID-less record dropped after a deployment switch (🤖 feat: write a cleanup record before each CoderAIProvider create (#353 part 2a) #374 (comment)). A create in deployment A loses its answer, the control plane then reaches deployment B, and B has a provider with the same name. ensureAIProviderRecord skips the deployment check because both IDs are empty, recordAIProvider clears the marker (NameConflict), and a later delete removes the record. The provider in A can leak. Fix: when CreateAttemptTime is set, check the record's deployment ID against buildinfo before the ID shortcut, and test this transition.
   8. Record lost when a namespace is reused with a fake Coder (advisor note on 🤖 feat: write a cleanup record before each CoderAIProvider create (#353 part 2a) #374/🤖 feat: sweep the cleanup records of CoderAIProvider objects that went without a delete (#353 part 2b) #375). The sweep trusts a control plane in the record's namespace that reports the recorded deployment ID. If someone recreates that namespace with a server that claims the ID and answers 404, the sweep drops the record and the provider stays in the real Coder. This is no worse than v0.1.4 (no wrong delete). Needs design: the normal recovery also recreates the namespace, so a control plane UID check would block it, and the Namespace deletion leaves AI Gateway keys and AI providers in Coder #353 decision rules out URL matching.
   9. Key record keeps an older create time after a status edit (advisor note on 🤖 feat: write a cleanup record before each CoderAIGatewayKey create (#353 part 2c) #385, low). When someone clears status.lastCreateAttemptTime by hand, the observe step stamps it with the current time (fail closed), but recordAIGatewayKeyID rewrites the record only when the key ID changed. So the sweep can count its settle window from the older time. Needs a manual status edit. Fix: also rewrite the record when its create time is older than lastCreateAttemptTime.

   ---

   Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: high
5. ThomasK33 commented on Oct 14, 2026

   

   ThomasK33

   on Oct 14, 2026

   MemberAuthor

   More actions

   Maintainer decision for part 2 (cleanup records): option B′, from an independent design review.

   One ConfigMap per record, named ck8s-cleanup-<object UID>, in the operator's own namespace (POD_NAMESPACE), matched by Coder's deployment ID (/api/v2/buildinfo deployment_id, which lives in the Coder database), never by URL or by user-chosen name.

   Why not the alternatives: a finalizer or hold cannot keep coderd running during a namespace delete; key expiry does not exist in Coder v2.37.3; a sweeper without records would guess from absence and could delete another cluster's keys; a cluster-scoped CRD (option A) gives little extra safety for a new public API type.

   Invariants the implementation must enforce:

   1. Write the record before the create call in Coder; if the write fails, do not create.
   2. A record holds no secrets: kind, object UID, namespace/name, deployment ID, key name prefix or provider nonce, Coder IDs once known, created-at.
   3. Read the deployment ID from Coder in the same reconcile as the create (or from control plane status written by the operator). Never from user input.
   4. The sweeper acts only through a control plane whose current deployment ID equals the record's.
   5. Delete only exact recorded IDs, a key with the record's ck8s-<uid>- prefix, or a provider whose display name is the record's nonce. Never by user-chosen name alone.
   6. Act only when the object with that UID no longer exists (lookup + UID compare) and the 5-minute settle window has passed.
   7. Remove the record only after Coder confirms the delete (success or Coder's own 404) in the matching deployment.
   8. A record no matching deployment reaches stays, with a metric and an Event. An admin can delete it after deleting the key or provider by hand.
   9. Only the operator service account can write these ConfigMaps (namespaced Role). No user-facing role gets write access.
   10. Ship the provider half first (fixes released v0.1.4 behavior).

   Keep the docs warning: it is the only fix when no control plane ever reconnects to that database.

   ---

   Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: medium
6. ThomasK33

   added a commit that references this issue on Oct 14, 2026

   feat: sweep the cleanup records of CoderAIProvider objects that went …

   ...

   0c7697d
7. ThomasK33 commented on Oct 14, 2026

   

   ThomasK33

   on Oct 14, 2026

   MemberAuthor

   More actions

   Follow-ups from #384 (control plane hold now covers CoderMCPServer, fixes #382). None of them blocks v0.1.5.

   1. docs/how-to/manage-ai-providers.md names only providers and keys in its hold sentence. Add MCP servers.
   2. coder_k8s_cleanup_released_without_delete_total does not count a CoderMCPServer that releases its finalizer without a delete in Coder.
   3. The cleanup records from 🤖 feat: write a cleanup record before each CoderAIProvider create (#353 part 2a) #374 and 🤖 feat: sweep the cleanup records of CoderAIProvider objects that went without a delete (#353 part 2b) #375 cover only CoderAIProvider.
   4. 🤖 fix: hold a deleted CoderControlPlane while CoderMCPServer objects still use it #384 bounds the MCP delete pass by the hold end and counts its retry from the end of the pass. The provider and key delete paths (coderaiprovider_delete.go near lines 84-88, coderaigatewaykey_delete.go near lines 56-60) still count the retry from the start of the pass and have no hold-end deadline. A slow pass there can run into the grace period.
   5. controlPlaneDependents has no unit test for the per-kind count when CoderMCPServer is the largest kind.

   ---

   Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: medium
8. ThomasK33

   removed this from the v0.1.5 milestone on Oct 14, 2026
9. ThomasK33

   added 10 commits that reference this issue on Oct 30, 2026

   test: a control plane holds without its Coder Deployment, also when i…

   ...

   89b6930

   fix: keep the control plane hold when its Coder Deployment is missing (…

   ...

   910867b

   test: a control plane that lost operator access before its deletion d…

   ...

   89ce952

   fix: keep the control plane hold without operator access (#353)

   ...

   383d0d4

   docs: drop the no-operator-access hold claim from the keys how-to (#353)

   ...

   822a8c1

   fix: report CoderDeploymentChanged when an MCP config looks deleted a…

   ...

   4c54383

   test: an MCP record without a config ID deletes nothing and stays whi…

   ...

   62da960

   fix: an MCP record without a config ID deletes nothing and stays for …

   ...

   6474666

   test: cover MCP records on delete after a slug conflict, for legacy c…

   ...

   5c4eadd

   fix: keep MCP records after a slug conflict, backfill them on delete,…

   ...

   22aa163
10. ThomasK33

    closed this as completedin #419on Oct 30, 2026