Skip to content

Commit 3f1bee7

Browse files
authored
🤖 feat: write a cleanup record before each CoderAIProvider create (#353 part 2a) (#374)
Part 2a of #353: the operator now writes a cleanup record before every `CoderAIProvider` create in Coder. When a namespace deletion (or a gone control plane) releases the provider without a delete, the record stays and names the Coder deployment and the provider ID. The sweep that uses these records is the stacked PR (part 2b). The `CoderAIGatewayKey` half follows after both. Refs #353 ## Design note (option B′, maintainer decision in #353 (comment)) **Why records.** A namespace deletion removes Coder's pods at once, so no finalizer or hold can delete the provider in Coder (#362 covers only a direct control-plane delete). A sweeper without records would have to guess from absence and could delete another cluster's providers. A record proves which deployment holds which provider. **Record format.** One ConfigMap per object, named `ck8s-cleanup-<object UID>`, label `coder.com/cleanup-record=CoderAIProvider`, in the operator's namespace: `POD_NAMESPACE`, else the service account namespace, else `coder-system` (the namespace of the shipped Role). Data: `kind`, `uid`, `namespace`, `name`, `deploymentID`, `nonce` (the create display name `coder-k8s:<uid>`), `type` (the provider type), `coderID` (once known) and `createAttemptTime`. The ConfigMap creation time is the created-at. How this PR meets the invariants of the decision (2b covers 4–8): | # | Invariant | Where | |---|---|---| | 1 | Record before the create. A failed write sends no create. | `createAIProvider` calls `writeAIProviderRecord` before the create marker. A failed write or a failed deployment-ID read returns before any POST. | | 2 | No secrets in the record. | Only the fields above. A test checks the key value is in no record field. | | 3 | Deployment ID from Coder in the same reconcile as the create. | `coderDeploymentID` reads `/api/v2/buildinfo` `deployment_id` right before the create, and again when the provider ID is first recorded. Only a canonical UUID is accepted. | | 5 | Exact IDs, nonce. | The record keeps the provider ID before the sync step replaces the nonce (`ensureAIProviderRecord` runs before the status write and before every sync). A forged `status.providerID` fails `checkAIProviderID` first, so it never reaches a record. | | 7 | Remove only after Coder confirms. | `deleteStep` removes the record only when `deleteInCoder` confirms (success or Coder's own 404, or a read that proves nothing of this object exists), and when the status proves no create was ever confirmed. `releaseWithoutDelete` keeps it. | | 9 | Only the operator writes the records. | A namespaced Role `manager-role` in `coder-system` (from a kubebuilder marker) with ConfigMap create/update/delete, bound only to the operator service account by the new RoleBinding `coder-k8s-cleanup-records`. The ClusterRole keeps ConfigMap read only. | | 10 | Provider half first. | This PR and 2b touch only `CoderAIProvider`. | **Upgrade.** A provider that exists from before this release has no record. The first reconcile that reaches Coder writes one, with the deployment ID that Coder answers then. That includes the disable path for a failing key Secret: the record write there never delays the disable. A malformed `spec.baseURL` stalls before any Coder access, so such an object gets its record only after the URL is fixed. **Deployment check on delete (review round 2).** Before a delete, the controller compares Coder's current deployment ID with the record's. When the control plane now reaches another deployment (for example after its database Secret changed), it sends no delete there and releases the finalizer without a delete. The record stays for a sweep. The record backfill also runs first in every reconcile now, before the base URL and key Secret checks. **Deployment change under a live object (review round 3).** When the create path finds a record of another deployment, the object stalls with `CoderDeploymentChanged`. The record keeps the old locator, and the controller creates nothing in the new deployment. **Reads and deletes (review round 1).** Every record read goes through a live API reader (`APIReader`, wired to `mgr.GetAPIReader()`), because the cache can miss a record written moments ago. A record delete reads the ConfigMap first, deletes only a valid record of that UID, and uses a UID precondition. When `status.providerID` is empty, the delete path uses the record's `coderID`, so a lost status write after a create cannot make the controller treat its own provider as someone else's. A ConfigMap of the record's name that is not a valid record of that UID stops the reconcile with an error. The operator never writes over it. ## Other changes - Docs: both how-tos state the control-plane delete worst case as one number, "up to about 15 minutes" (from the #362 assessor). The provider how-to gets a "Cleanup records" section. - CI: the full Kind E2E path filter includes `internal/controller/cleanup_record*`. - Test fix for #362: `TestControlPlaneHoldRevokesWorkspaceRBAC` now registers the Gateway API scheme itself. It failed when run alone (it passed in the full suite only because another test registered the scheme first). ## Validation Local gates on the tree of this commit before a docs-only edit and a rebase onto main (`1c0df24c`, #373: CI and E2E script files only; no Go change came with it): - Passed: `make build-installer`, `make codegen`, `make docs-reference`, drift (`git diff --exit-code`), `make verify-vendor`, `make build`, `make lint`, govulncheck, `make test`, `make test-integration`, `go test -race ./internal/controller/...`, `make docs-check`, actionlint v1.7.10. - After the docs edit (one line in `docs/explanation/architecture.md`) and the rebase: `make docs-check`, actionlint and `go build ./...` again. Mutation checks (each one made a test fail): no record before the create, no provider ID before the sync, record kept after a confirmed delete, record removed on a release without delete, no backfill, no canonical-UUID check of the deployment ID, no name check of the record. The dogfood of the whole provider half (records plus sweep) is on the stacked PR, because a record alone changes nothing a user can see except the ConfigMap. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-opus-5-5` • Thinking: `high`_ --------- Signed-off-by: Thomas Kosiewski <tk@coder.com>
1 parent 5cfffaa commit 3f1bee7

21 files changed

Lines changed: 803 additions & 31 deletions

‎.cspell.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
"version": "0.2",
33
"language": "en",
44
"words": [
5+
"buildinfo",
56
"allapp",
67
"automount",
78
"prebuild",

‎.github/workflows/ci.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ jobs:
9595
# Shared Coder access that CoderTemplateTest and later pool controllers call.
9696
- 'internal/controller/controlplane_access.go'
9797
- 'internal/controller/controlplane_hold.go'
98+
- 'internal/controller/cleanup_*'
9899
- 'internal/controller/coder_access.go'
99100
- 'internal/controller/coder_workspace_steps.go'
100101
# The URL that the aggregated API server and the controllers use for Coder API calls.

‎config/default/kustomization.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ resources:
1010
- ../crd/bases/coder.com_coderprovisioners.yaml
1111
- ../crd/bases/coder.com_codertemplatetests.yaml
1212
- ../crd/bases/coder.com_coderworkspaceproxies.yaml
13+
- ../rbac/cleanup-records-rolebinding.yaml
1314
- ../rbac/clusterrolebinding.yaml
1415
- ../rbac/role.yaml
1516
- ../rbac/serviceaccount.yaml
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
# Lets the operator write its cleanup records (#353): ConfigMaps named ck8s-cleanup-<object UID> in its own
2+
# namespace. The namespaced Role manager-role comes from the kubebuilder RBAC markers in role.yaml. No other
3+
# subject gets write access to these ConfigMaps.
4+
apiVersion: rbac.authorization.k8s.io/v1
5+
kind: RoleBinding
6+
metadata:
7+
name: coder-k8s-cleanup-records
8+
namespace: coder-system
9+
roleRef:
10+
apiGroup: rbac.authorization.k8s.io
11+
kind: Role
12+
name: manager-role
13+
subjects:
14+
- kind: ServiceAccount
15+
name: coder-k8s
16+
namespace: coder-system

‎config/rbac/role.yaml‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,3 +198,21 @@ rules:
198198
- patch
199199
- update
200200
- watch
201+
---
202+
apiVersion: rbac.authorization.k8s.io/v1
203+
kind: Role
204+
metadata:
205+
name: manager-role
206+
namespace: coder-system
207+
rules:
208+
- apiGroups:
209+
- ""
210+
resources:
211+
- configmaps
212+
verbs:
213+
- create
214+
- delete
215+
- get
216+
- list
217+
- update
218+
- watch

‎dist/install.yaml‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6960,6 +6960,24 @@ metadata:
69606960
namespace: coder-system
69616961
---
69626962
apiVersion: rbac.authorization.k8s.io/v1
6963+
kind: Role
6964+
metadata:
6965+
name: manager-role
6966+
namespace: coder-system
6967+
rules:
6968+
- apiGroups:
6969+
- ""
6970+
resources:
6971+
- configmaps
6972+
verbs:
6973+
- create
6974+
- delete
6975+
- get
6976+
- list
6977+
- update
6978+
- watch
6979+
---
6980+
apiVersion: rbac.authorization.k8s.io/v1
69636981
kind: ClusterRole
69646982
metadata:
69656983
name: manager-role
@@ -7160,6 +7178,20 @@ rules:
71607178
- watch
71617179
---
71627180
apiVersion: rbac.authorization.k8s.io/v1
7181+
kind: RoleBinding
7182+
metadata:
7183+
name: coder-k8s-cleanup-records
7184+
namespace: coder-system
7185+
roleRef:
7186+
apiGroup: rbac.authorization.k8s.io
7187+
kind: Role
7188+
name: manager-role
7189+
subjects:
7190+
- kind: ServiceAccount
7191+
name: coder-k8s
7192+
namespace: coder-system
7193+
---
7194+
apiVersion: rbac.authorization.k8s.io/v1
71637195
kind: ClusterRoleBinding
71647196
metadata:
71657197
name: coder-k8s

‎docs/explanation/architecture.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ graph TD
7676
| Path | Contents |
7777
| --- | --- |
7878
| `config/crd/bases/` | The generated CRDs for `CoderControlPlane`, `CoderProvisioner`, `CoderWorkspaceProxy`, `CoderTemplateTest`, `CoderAIProvider`, `CoderAIGatewayKey`, and `CoderMCPServer` |
79-
| `config/rbac/` | The `coder-k8s` ServiceAccount, the `manager-role` ClusterRole, and the bindings (including auth-delegator) |
79+
| `config/rbac/` | The `coder-k8s` ServiceAccount, the `manager-role` ClusterRole, the `manager-role` Role in `coder-system` (write access to the cleanup-record ConfigMaps there), and the bindings (including auth-delegator) |
8080
| `config/admission/` | Opt-in admission policies that the install bundle does not apply, for example the operator-only status write policy for `CoderMCPServer` |
8181
| `config/apiserver-standalone/` | For standalone `--app=aggregated-apiserver`: the `coder-k8s-apiserver` ServiceAccount, its RBAC, and the placeholder Secret for the serving CA |
8282
| `deploy/deployment.yaml` | The `coder-k8s` Deployment (default `--app=all`) |

‎docs/how-to/manage-ai-gateway-keys.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ The finalizer stays while Coder cannot confirm the deletion:
139139

140140
When the control plane is gone, nothing can delete the keys. The finalizer goes at once. The operator logs `Releasing the finalizer without deleting the keys in Coder` with reason `ControlPlaneGone` and the key IDs: delete those keys with `coder ai-gateway keys delete`.
141141

142-
When you delete the control plane while `CoderAIGatewayKey` or `CoderAIProvider` objects that use it are being deleted, the control plane waits for them for up to 10 minutes: it keeps its finalizer, so Coder keeps running, and the objects delete their keys and providers as usual, with retries. The operator log shows `Waiting for the objects that use this control plane` with their names. The operator removes the workspace RBAC of the control plane before it waits, so Coder loses its Kubernetes access at once. An object that you do not delete holds the control plane for only 30 seconds, so delete the objects together with the control plane (or within 30 seconds). After 10 minutes, the objects that remain get 30 more seconds plus 10 seconds per object of one kind (at most 5 minutes) for their bounded pass below, and then the control plane goes anyway. A foreground deletion (`kubectl delete --cascade=foreground`) does not wait, because Kubernetes then deletes the Coder Deployment first.
142+
When you delete the control plane while `CoderAIGatewayKey` or `CoderAIProvider` objects that use it are being deleted, the control plane delete can take up to about 15 minutes. The control plane waits for the objects for up to 10 minutes: it keeps its finalizer, so Coder keeps running, and the objects delete their keys and providers as usual, with retries. The operator log shows `Waiting for the objects that use this control plane` with their names. The operator removes the workspace RBAC of the control plane before it waits, so Coder loses its Kubernetes access at once. An object that you do not delete holds the control plane for only 30 seconds, so delete the objects together with the control plane (or within 30 seconds). After 10 minutes, the objects that remain get at most 5 more minutes (30 seconds plus 10 seconds per object of one kind) for their bounded pass below, and then the control plane goes anyway. A foreground deletion (`kubectl delete --cascade=foreground`) does not wait, because Kubernetes then deletes the Coder Deployment first.
143143

144144
When the control plane does not wait (its namespace is being deleted, it is deleted in the foreground, or the 10 minutes have passed) and its operator access still works, the operator runs one pass of at most 10 seconds without the settle window, then removes the finalizer whatever the answer. A key create that was still in flight at that moment can commit later, and its key stays in Coder. When the pass fails, the log lists the IDs that it did not delete.
145145

‎docs/how-to/manage-ai-providers.md‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,11 +122,19 @@ Deleting the object deletes the provider in Coder. With only an unconfirmed crea
122122

123123
- When Coder is unreachable, or the control plane has no operator access, the finalizer `coder.com/ai-provider-cleanup` stays and the operator retries every 60 seconds. See [Stuck finalizer](#stuck-finalizer).
124124
- When the control plane is gone, nothing can delete the provider. The finalizer goes at once, and the operator logs the provider ID. Delete the provider in the Coder dashboard.
125-
- When you delete the control plane while `CoderAIProvider` or `CoderAIGatewayKey` objects that use it are being deleted, the control plane waits for them for up to 10 minutes, plus 30 seconds and 10 seconds per object of one kind (at most 5 minutes) for a last bounded attempt. It keeps its finalizer, so Coder keeps running, and the delete above runs with its retries. The operator removes the workspace RBAC of the control plane before it waits, so Coder loses its Kubernetes access at once. 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.
125+
- When you delete the control plane while `CoderAIProvider` or `CoderAIGatewayKey` objects that use it are being deleted, the control plane delete can take up to about 15 minutes. The control plane waits for the objects for up to 10 minutes, then for at most 5 more minutes (30 seconds plus 10 seconds per object of one kind) for a last bounded attempt. It keeps its finalizer, so Coder keeps running, and the delete above runs with its retries. The operator removes the workspace RBAC of the control plane before it waits, so Coder loses its Kubernetes access at once. 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.
126126
- When the control plane does not wait (its namespace is being deleted, it is deleted in the foreground, or the 10 minutes have passed) and its operator access still works, the operator sends one delete with a 10-second limit, then removes the finalizer. Without working access, the finalizer goes without a delete. Either way a namespace deletion finishes. When the delete did not succeed, the operator logs the provider ID: delete the provider in the Coder dashboard. The metric `coder_k8s_cleanup_released_without_delete_total` (label `kind`) counts these releases.
127127

128128
Delete `CoderAIProvider` objects before their namespace: a namespace deletion removes Coder's pods at once, so the control plane cannot wait for them. Coder deletes a provider even while chat models use it. Those models stop working.
129129

130+
### Cleanup records
131+
132+
Before the operator creates a provider in Coder, it writes a cleanup record: a ConfigMap named `ck8s-cleanup-<object UID>` with the label `coder.com/cleanup-record=CoderAIProvider` in the operator's namespace (`coder-system`). If it cannot write the record, it does not create the provider. The record holds no key: it holds the object's namespace, name and UID, Coder's deployment ID from `/api/v2/buildinfo`, the provider's display-name nonce, and the provider ID once the operator knows it. The deployment ID lives in the Coder database, so it names the database that holds the provider.
133+
134+
When the control plane later reaches another Coder deployment (for example after its database Secret changed), the record still locates the provider in the old one. The operator then stalls the object with `CoderDeploymentChanged`, keeps the record and creates nothing in the new deployment. Delete the provider in the old deployment, then delete the ConfigMap.
135+
136+
The operator removes the record when Coder confirms that the provider is deleted. When the finalizer goes without a delete, for example in a namespace deletion, the record stays. Find these records with `kubectl -n coder-system get configmaps -l coder.com/cleanup-record`. The operator gets write access to ConfigMaps only in that namespace (the Role `manager-role` and the RoleBinding `coder-k8s-cleanup-records`). Do not give other users write access to ConfigMaps there: a record names a Coder provider ID that a cleanup deletes. If you install the operator in another namespace, change the namespace of both.
137+
130138
### Stuck finalizer
131139

132140
A deleted `CoderAIProvider` stays `Terminating` while the operator cannot prove that Coder no longer holds its provider. Two cases never end by themselves:

‎internal/app/controllerapp/controllerapp.go‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,9 @@ func SetupControllers(mgr manager.Manager) error {
129129
aiProviderReconciler := &controller.CoderAIProviderReconciler{
130130
Client: client,
131131
Scheme: managerScheme,
132+
// The operator's own namespace holds the cleanup records (#353).
133+
OperatorNamespace: detectOperatorNamespace(),
134+
APIReader: mgr.GetAPIReader(),
132135
}
133136
if err := aiProviderReconciler.SetupWithManager(mgr); err != nil {
134137
return fmt.Errorf("unable to create AI provider controller: %w", err)
@@ -211,6 +214,26 @@ func Run(ctx context.Context) error {
211214
return nil
212215
}
213216

217+
// defaultOperatorNamespace is the namespace of the installed operator, where
218+
// the Role for the cleanup records lives (config/rbac).
219+
const defaultOperatorNamespace = "coder-system"
220+
221+
// detectOperatorNamespace returns the namespace that holds the cleanup
222+
// records: POD_NAMESPACE, else the in-cluster namespace file, else
223+
// coder-system, which the shipped Role names. Only that fallback differs
224+
// from detectLeaderElectionNamespace.
225+
func detectOperatorNamespace() string {
226+
if ns := strings.TrimSpace(os.Getenv("POD_NAMESPACE")); ns != "" {
227+
return ns
228+
}
229+
if data, err := os.ReadFile(inClusterNamespacePath); err == nil {
230+
if ns := strings.TrimSpace(string(data)); ns != "" {
231+
return ns
232+
}
233+
}
234+
return defaultOperatorNamespace
235+
}
236+
214237
// detectLeaderElectionNamespace returns the namespace to use for leader-election
215238
// lease objects. Resolution order:
216239
// 1. POD_NAMESPACE env var (allows explicit override for any environment).

0 commit comments

Comments
 (0)