Skip to content

Commit 57ee69a

Browse files
authored
🤖 feat: sweep the cleanup records of CoderAIGatewayKey objects that went without a delete (#353 part 2d) (#386)
Refs #353 (part 2d: the `CoderAIGatewayKey` half of option B′, sweep). Refs #156. Stacked on #385 (records). Review only the top commit. I retarget this PR to `main` after #385 lands. ## Summary The cleanup record sweep from #375 now also handles `CoderAIGatewayKey` records. After a namespace deletion with an external database, the leftover keys are deleted in Coder once the namespace comes back with a control plane on the same Coder deployment. A different deployment at the same URL is never swept. Provider sweeping is unchanged. ## Design note (option B′, #353 (comment)) The sweep keeps every rule of #375. The only new part is how one record's resources are deleted. That part now goes through a per-kind table (`cleanupRecordKinds`): the object type to look up, the words for the Event, and the sweep function. | Invariant | Providers (#375, unchanged) | Keys (this PR) | | --- | --- | --- | | 4. Who sweeps | A ready control plane in the record's namespace whose live `/buildinfo` deployment ID equals the record's | Same | | 5. What is deleted | The recorded provider ID, or the provider whose display name is the nonce | Every listed key whose name is the record's prefix `ck8s-<UID without dashes>-` plus a serial. The prefix is checked against the record's UID first (assertion). No other key is touched. | | 6. When | Object with that UID gone (live read), 5 minutes after the last create | Same. The object is gone, so no serial bound applies: after the settle window no create of the object can still commit. | | 7. Record removed | Delete answered with success or Coder's own 404 | A site owner's LIST shows no key with the prefix. The pass deletes and lists again, at most twice. A failed LIST or DELETE, or a 404 that is not Coder's own, keeps the record (retry in 1 minute). | | 8. No match | Metric and `NoMatchingDeployment` Event | Same, with the kind label `CoderAIGatewayKey`. The Event says "the AI Gateway keys". | The sweeper also wakes a key record when its `CoderAIGatewayKey` is deleted, as it does for providers. ## Validation - New tests in `cleanup_sweep_key_test.go`: - `TestCleanupSweepAIGatewayKeys`: the settle window, no match, a different deployment at the same URL, and then the sweep. The sweep deletes the current, the retiring and an orphan key. It leaves a person's key and another object's `ck8s-` key, and the metrics change. - `TestCleanupSweepAIGatewayKeyCases`: a living object, the same name with a new UID, a proxy 404, a LIST that still shows a key, an operator user that is not a site owner, and a control plane in another namespace. - Red then green: 6 mutations, each caught by a test: keys not swept, "gone" without a confirming LIST, no prefix filter, no site-owner check, any 404 confirming a delete, and an object lookup by name only. - Local gates: build-installer, codegen, docs-reference, drift, verify-vendor, build, lint, govulncheck, test, test-integration, race, docs-check, actionlint. All passed. - Kind dogfood with a licensed Coder on 5bb2486 (same Go code as the head; the round-2 commit changes only docs): #386 (comment). A namespace deletion left 2 keys. A different deployment at the same URL and a control plane in another namespace swept nothing. The namespace came back on the same database, and then 0 keys and 0 records were left. ## Risks - The provider path changes only in its log wording (`Swept the Coder resources of a deleted object`). Its Event text is the same as before. - A sweep of keys lists every key in the deployment (Coder has no filter). That is one LIST per record per sweep, the same LIST the key controller already sends. --- _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 45dc78b commit 57ee69a

3 files changed

Lines changed: 312 additions & 21 deletions

File tree

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -151,10 +151,19 @@ Before each key create, the operator writes a cleanup record: a ConfigMap named
151151

152152
When the control plane later reaches another Coder deployment (for example after its database Secret changed), the record still locates the keys in the old one. The operator then stalls the object with `CoderDeploymentChanged` at its next key create, keeps the record and creates no key in the new deployment. A delete of the object then removes the finalizer without a delete in Coder. Delete the keys in the old deployment, then delete the ConfigMap.
153153

154-
The operator removes the record only after a list in the record's deployment shows no key of the object: at the end of a normal delete, or after the one bounded pass when the last create was more than 5 minutes before it. In every other release without a delete, the record stays and names where the keys are. Find the records with `kubectl -n coder-system get configmaps -l coder.com/cleanup-record=CoderAIGatewayKey`. The operator gets write access to ConfigMaps only in that namespace. Do not give other users write access to ConfigMaps there.
154+
The operator removes the record only after a list in the record's deployment shows no key of the object: at the end of a normal delete, or after the one bounded pass when the last create was more than 5 minutes before it. In every other release without a delete, for example in a namespace deletion, the record stays, and the operator sweeps it:
155+
156+
1. It waits until the object with that UID is gone and 5 minutes have passed since the last create, so that a create still in flight commits first.
157+
2. It looks for a ready `CoderControlPlane` in the object's namespace whose Coder answers the record's deployment ID now. It never matches by URL or name: a new Coder database behind the same URL has another deployment ID and is never swept. A control plane in another namespace never sweeps the record.
158+
3. Through that control plane, whose operator user must be a site owner, it lists the keys and deletes every key whose name is the record's prefix `ck8s-<UID without dashes>-` followed by a serial number (the names that the operator gives, such as `ck8s-<UID without dashes>-3`). It never deletes any other key.
159+
4. It removes the record when a list shows no such key. A failed list or delete, or a `404` that is not Coder's own, keeps the record, and the operator retries every minute.
160+
161+
So after a namespace deletion with an external database, the leftover keys go once you create the namespace again with a control plane that uses the same database. The metric `coder_k8s_cleanup_records_swept_total` (label `kind`) counts the swept records.
162+
163+
When no ready control plane reaches the record's deployment, the record stays. The gauge `coder_k8s_cleanup_records_unmatched` counts these records, and the operator writes a `NoMatchingDeployment` Warning Event on the ConfigMap every 10 minutes. When that Coder deployment never comes back, delete the keys with the record's prefix in Coder by hand, then delete the ConfigMap. Find the records with `kubectl -n coder-system get configmaps -l coder.com/cleanup-record=CoderAIGatewayKey`. The operator gets write access to ConfigMaps only in that namespace. Do not give other users write access to ConfigMaps there: a record names the keys that a sweep deletes.
155164

156165
!!! warning "Delete the keys before the namespace"
157-
When you delete the namespace, Kubernetes deletes the control plane and the `CoderAIGatewayKey` objects at the same time. The control plane is usually gone before the operator handles the objects, so the finalizers go without a delete, and the keys stay valid in Coder. The namespace deletion finishes, and the operator logs the key IDs with reason `ControlPlaneGone`.
166+
When you delete the namespace, Kubernetes deletes the control plane and the `CoderAIGatewayKey` objects at the same time. The control plane is usually gone before the operator handles the objects, so the finalizers go without a delete, and the keys stay valid in Coder. The namespace deletion finishes, and the operator logs the key IDs with reason `ControlPlaneGone`. A [sweep](#cleanup-records) deletes the keys only when the namespace comes back with a control plane on the same Coder database. Until then they stay valid.
158167

159168
Delete the `CoderAIGatewayKey` objects first and wait until they are gone, then delete the control plane or the namespace:
160169

@@ -191,7 +200,7 @@ A deletion that keeps failing never ends by itself. Leave it in one of these way
191200

192201
## Known limits
193202

194-
- **Leftover keys.** A key that commits more than 5 minutes after a lost create answer, or after the object is gone, stays in Coder. So do the keys of objects that go together with their control plane, for example in a namespace deletion. Find keys of deleted objects with `coder ai-gateway keys list`: their names start with `ck8s-` and a UID that no `CoderAIGatewayKey` has.
203+
- **Leftover keys.** A key that commits more than 5 minutes after a lost create answer, or after the object is gone, stays in Coder. So do the keys of objects that go together with their control plane, for example in a namespace deletion, until a [sweep](#cleanup-records) deletes them. Find keys of deleted objects with `coder ai-gateway keys list`: their names start with `ck8s-` and a UID that no `CoderAIGatewayKey` has.
195204
- **A short key list.** The operator reads all keys with one list call. When a list answer names the current key but leaves out a replaced key, the operator treats the replaced key as deleted. If the key appears again, the operator deletes it as an orphan, without the rest of its grace period. Coder lists all keys in one answer, so only a proxy between the operator and Coder that changes the answer can cause this.
196205

197206
## Grant access

‎internal/controller/cleanup_sweep.go‎

Lines changed: 76 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,8 @@ func init() {
6363
// CleanupRecordReconciler sweeps the cleanup records of objects that went
6464
// without a confirmed delete in Coder (#353). It acts only through a ready
6565
// control plane whose current Coder deployment ID equals the record's, and
66-
// it deletes only the recorded provider ID or the provider with the
67-
// recorded nonce.
66+
// it deletes only the recorded provider ID, the provider with the recorded
67+
// nonce, or the AI Gateway keys whose names start with the recorded prefix.
6868
type CleanupRecordReconciler struct {
6969
client.Client
7070
// APIReader reads the object of a record live, so a stale cache never
@@ -78,20 +78,23 @@ type CleanupRecordReconciler struct {
7878
}
7979

8080
// SetupWithManager watches the records in the operator namespace. It wakes a
81-
// record when its CoderAIProvider is deleted, and every record when a control
82-
// plane gets operator access, since that control plane can match them.
81+
// record when its CoderAIProvider or CoderAIGatewayKey is deleted, and every
82+
// record when a control plane gets operator access, since that control plane
83+
// can match them.
8384
func (r *CleanupRecordReconciler) SetupWithManager(mgr ctrl.Manager) error {
8485
if mgr == nil || r.Client == nil || r.APIReader == nil || r.OperatorNamespace == "" || r.Clock == nil || r.Recorder == nil {
8586
return errors.New("assertion failed: the cleanup record reconciler needs a manager, a client, an API reader, the operator namespace, a clock and an event recorder")
8687
}
8788
isRecord := predicate.NewPredicateFuncs(func(obj client.Object) bool {
8889
return obj.GetNamespace() == r.OperatorNamespace && obj.GetLabels()[cleanupRecordLabel] != ""
8990
})
91+
recordOf := handler.EnqueueRequestsFromMapFunc(func(_ context.Context, obj client.Object) []reconcile.Request {
92+
return []reconcile.Request{{NamespacedName: types.NamespacedName{Namespace: r.OperatorNamespace, Name: cleanupRecordObjectName(obj.GetUID())}}}
93+
})
9094
return ctrl.NewControllerManagedBy(mgr).
9195
For(&corev1.ConfigMap{}, builder.WithPredicates(isRecord)).
92-
Watches(&coderv1alpha1.CoderAIProvider{}, handler.EnqueueRequestsFromMapFunc(func(_ context.Context, obj client.Object) []reconcile.Request {
93-
return []reconcile.Request{{NamespacedName: types.NamespacedName{Namespace: r.OperatorNamespace, Name: cleanupRecordObjectName(obj.GetUID())}}}
94-
}), builder.WithPredicates(dependentRemoved)).
96+
Watches(&coderv1alpha1.CoderAIProvider{}, recordOf, builder.WithPredicates(dependentRemoved)).
97+
Watches(&coderv1alpha1.CoderAIGatewayKey{}, recordOf, builder.WithPredicates(dependentRemoved)).
9598
Watches(&coderv1alpha1.CoderControlPlane{}, handler.EnqueueRequestsFromMapFunc(r.allRecords), builder.WithPredicates(operatorAccessGained)).
9699
Named("cleanuprecord").
97100
Complete(r)
@@ -149,13 +152,14 @@ func (r *CleanupRecordReconciler) Reconcile(ctx context.Context, req ctrl.Reques
149152
log.FromContext(ctx).Error(err, "Ignoring a ConfigMap that is not a valid cleanup record")
150153
return ctrl.Result{}, nil
151154
}
152-
if record.Kind != cleanupRecordKindAIProvider {
153-
return ctrl.Result{}, nil // Only provider records are swept so far.
155+
kind, ok := cleanupRecordKinds[record.Kind]
156+
if !ok {
157+
return ctrl.Result{}, nil // A kind that this operator does not sweep.
154158
}
155159

156160
// Act only when the object with that UID no longer exists (invariant 6).
157-
provider := &coderv1alpha1.CoderAIProvider{}
158-
if err := r.APIReader.Get(ctx, types.NamespacedName{Namespace: record.Namespace, Name: record.Name}, provider); err == nil && provider.UID == record.UID {
161+
object := kind.object()
162+
if err := r.APIReader.Get(ctx, types.NamespacedName{Namespace: record.Namespace, Name: record.Name}, object); err == nil && object.GetUID() == record.UID {
159163
r.setUnmatched(record.UID, record.Kind, false)
160164
return ctrl.Result{RequeueAfter: cleanupObjectRecheck}, nil
161165
} else if err != nil && !apierrors.IsNotFound(err) {
@@ -181,23 +185,23 @@ func (r *CleanupRecordReconciler) Reconcile(ctx context.Context, req ctrl.Reques
181185
if len(matches) == 0 {
182186
r.setUnmatched(record.UID, record.Kind, true)
183187
r.Recorder.Eventf(configMap, nil, corev1.EventTypeWarning, "NoMatchingDeployment", "Sweep",
184-
"No ready CoderControlPlane in namespace %s reaches Coder deployment %s, which holds the provider of deleted %s %s/%s. Delete the provider in Coder by hand, then delete this ConfigMap.",
185-
record.Namespace, record.DeploymentID, record.Kind, record.Namespace, record.Name)
188+
"No ready CoderControlPlane in namespace %s reaches Coder deployment %s, which holds %s of deleted %s %s/%s. Delete %s in Coder by hand, then delete this ConfigMap.",
189+
record.Namespace, record.DeploymentID, kind.what, record.Kind, record.Namespace, record.Name, kind.what)
186190
return ctrl.Result{RequeueAfter: cleanupUnmatchedRecheck}, nil
187191
}
188192
r.setUnmatched(record.UID, record.Kind, false)
189-
logger := log.FromContext(ctx).WithValues("record", configMap.Name, "deploymentID", record.DeploymentID, "provider", record.Namespace+"/"+record.Name)
193+
logger := log.FromContext(ctx).WithValues("record", configMap.Name, "deploymentID", record.DeploymentID, "object", record.Kind+" "+record.Namespace+"/"+record.Name)
190194
// Each matching control plane has its own operator token, so a failure
191195
// through one tries the next.
192196
gone, why, through := false, "", logger
193197
for _, m := range matches {
194198
through = logger.WithValues("controlPlane", types.NamespacedName{Namespace: m.controlPlane.Namespace, Name: m.controlPlane.Name})
195-
if gone, why, err = sweepAIProvider(ctx, m.sdk, record); err != nil {
199+
if gone, why, err = kind.sweep(ctx, m.sdk, record); err != nil {
196200
return ctrl.Result{}, err
197201
} else if gone {
198202
break
199203
}
200-
through.Info("The cleanup of a deleted provider in Coder failed", "why", why)
204+
through.Info("The cleanup of a deleted object in Coder failed", "why", why)
201205
}
202206
if !gone {
203207
return ctrl.Result{RequeueAfter: cleanupRetry}, nil
@@ -207,10 +211,26 @@ func (r *CleanupRecordReconciler) Reconcile(ctx context.Context, req ctrl.Reques
207211
return ctrl.Result{}, err
208212
}
209213
cleanupRecordsSwept.WithLabelValues(record.Kind).Inc()
210-
through.Info("Swept the provider of a deleted CoderAIProvider in Coder", "result", why, "providerID", record.CoderID)
214+
through.Info("Swept the Coder resources of a deleted object", "result", why, "coderID", record.CoderID)
211215
return ctrl.Result{}, nil
212216
}
213217

218+
// cleanupSweepKind is how a sweep handles the records of one object kind.
219+
type cleanupSweepKind struct {
220+
object func() client.Object
221+
what string // What the object holds in Coder, for the Event.
222+
sweep func(context.Context, *codersdk.Client, *cleanupRecord) (gone bool, why string, err error)
223+
}
224+
225+
var cleanupRecordKinds = map[string]cleanupSweepKind{
226+
cleanupRecordKindAIProvider: {
227+
object: func() client.Object { return &coderv1alpha1.CoderAIProvider{} }, what: "the provider", sweep: sweepAIProvider,
228+
},
229+
cleanupRecordKindAIGatewayKey: {
230+
object: func() client.Object { return &coderv1alpha1.CoderAIGatewayKey{} }, what: "the AI Gateway keys", sweep: sweepAIGatewayKeys,
231+
},
232+
}
233+
214234
// controlPlaneMatch is a control plane whose Coder deployment is a record's.
215235
type controlPlaneMatch struct {
216236
sdk *codersdk.Client
@@ -309,6 +329,44 @@ func sweepAIProvider(ctx context.Context, sdk *codersdk.Client, record *cleanupR
309329
return true, "deleted provider " + id, nil
310330
}
311331

332+
// sweepAIGatewayKeys deletes the keys of record in Coder: every key whose
333+
// name is the record's prefix ck8s-<uid>- and a serial (invariant 5). The
334+
// object is gone, so no serial bound applies: the settle window has passed,
335+
// and no create of the object can still commit. It returns gone only when a
336+
// LIST, read by a site owner, shows no such key.
337+
func sweepAIGatewayKeys(ctx context.Context, sdk *codersdk.Client, record *cleanupRecord) (gone bool, why string, err error) {
338+
prefix, err := aiGatewayKeyNamePrefix(record.UID)
339+
if err != nil {
340+
return false, "", err
341+
}
342+
if record.Nonce != prefix {
343+
return false, "", fmt.Errorf("assertion failed: cleanup record of %s has key prefix %q, not the prefix of its UID", record.UID, record.Nonce)
344+
}
345+
if step := requireSiteOwner(ctx, sdk); step != nil {
346+
// Only a site owner's LIST shows every key.
347+
return false, step.message, nil
348+
}
349+
deleted := 0
350+
for range 2 {
351+
listed, err := sdk.ListAIGatewayKeys(ctx)
352+
if err != nil {
353+
return false, "list AI Gateway keys: " + coderErrorSummary(err), nil
354+
}
355+
own := ownAIGatewayKeys(prefix, listed)
356+
if len(own) == 0 {
357+
return true, fmt.Sprintf("deleted %d keys, and a list shows none left", deleted), nil
358+
}
359+
for _, k := range own {
360+
// Only Coder's own 404 proves that the key is gone (#325).
361+
if err := sdk.DeleteAIGatewayKey(ctx, k.ID); err != nil && !isCoderResourceNotFound(err) {
362+
return false, "delete key " + k.Name + ": " + coderErrorSummary(err), nil
363+
}
364+
deleted++
365+
}
366+
}
367+
return false, "Coder still lists keys of the object after their delete", nil
368+
}
369+
312370
// setUnmatched keeps the unmatched gauge equal to the records that no
313371
// deployment reaches.
314372
func (r *CleanupRecordReconciler) setUnmatched(uid types.UID, kind string, unmatched bool) {
@@ -317,7 +375,7 @@ func (r *CleanupRecordReconciler) setUnmatched(uid types.UID, kind string, unmat
317375
} else {
318376
r.unmatched.Delete(uid)
319377
}
320-
counts := map[string]float64{cleanupRecordKindAIProvider: 0}
378+
counts := map[string]float64{cleanupRecordKindAIProvider: 0, cleanupRecordKindAIGatewayKey: 0}
321379
r.unmatched.Range(func(_, v any) bool {
322380
counts[v.(string)]++
323381
return true

0 commit comments

Comments
 (0)