🤖 fix: hold a deleted CoderControlPlane while AI Gateway keys and providers still use it

.github/workflows/ci.yaml

@@ -91,6 +91,7 @@ jobs:
               - 'internal/controller/reserved_secret_names.go'
               # Shared Coder access that CoderTemplateTest and later pool controllers call.
               - 'internal/controller/controlplane_access.go'
+              - 'internal/controller/controlplane_hold.go'
               - 'internal/controller/coder_access.go'
               - 'internal/controller/coder_workspace_steps.go'
               # The URL that the aggregated API server and the controllers use for Coder API calls.

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

@@ -139,7 +139,11 @@ The finalizer stays while Coder cannot confirm the deletion:
 
 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`.
 
-While the control plane is being deleted 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.
+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.
+
+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.
+
+Each release without a delete counts in the controller metric `coder_k8s_cleanup_released_without_delete_total` (label `kind`), next to the log line.
 
 !!! warning "Delete the keys before the namespace"
     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`.
@@ -157,7 +161,7 @@ If the namespace is already gone, delete the logged keys with `coder ai-gateway
 A deletion that keeps failing never ends by itself. Leave it in one of these ways:
 
 1. **Fix Coder or the operator user.** Restore Coder access, the Premium license or the AI Gateway, the operator user's site-owner role, or the proxy that answers `404`. The next retry deletes the keys and releases the object.
-2. **Delete the control plane.** The operator then runs its one bounded pass and releases the object. Do this only when you remove that Coder deployment anyway.
+2. **Delete the control plane.** The control plane first waits up to 10 minutes for its objects, then the operator runs its one bounded pass and releases the object. Do this only when you remove that Coder deployment anyway.
 3. **Remove the finalizer by hand, then delete the keys in Coder.**
 
    Note the key names from `status.currentKey` and `status.retiringKeys`, and the object's UID (`kubectl -n coder get coderaigatewaykey gateway -o jsonpath='{.metadata.uid}'`). Find the index of the finalizer:

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

@@ -122,9 +122,10 @@ Deleting the object deletes the provider in Coder. With only an unconfirmed crea
 
 - 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).
 - 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.
-- While the control plane is being deleted 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.
+- 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.
+- 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.
 
-Delete `CoderAIProvider` objects before their control plane. Coder deletes a provider even while chat models use it. Those models stop working.
+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.
 
 ### Stuck finalizer
 

go.mod

@@ -10,6 +10,7 @@ require (
 	github.com/google/uuid v1.6.0
 	github.com/lib/pq v1.10.9
 	github.com/modelcontextprotocol/go-sdk v1.7.0
+	github.com/prometheus/client_golang v1.24.1
 	github.com/stretchr/testify v1.12.0
 	github.com/yuin/gopher-lua v1.1.2
 	golang.org/x/xerrors v0.0.0-20240903120638-7835f813f4da
@@ -293,7 +294,6 @@ require (
 	github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 // indirect
 	github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
 	github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55 // indirect
-	github.com/prometheus/client_golang v1.24.1 // indirect
 	github.com/prometheus/client_model v0.6.2 // indirect
 	github.com/prometheus/common v0.70.1 // indirect
 	github.com/prometheus/procfs v0.21.1 // indirect

internal/controller/coderaigatewaykey_controller.go

@@ -163,11 +163,16 @@ func (r *CoderAIGatewayKeyReconciler) Reconcile(ctx context.Context, req ctrl.Re
 		if !controllerutil.ContainsFinalizer(key, coderv1alpha1.CoderAIGatewayKeyCleanupFinalizer) {
 			return ctrl.Result{}, nil
 		}
+		forgetReleasedWithoutDelete(key.UID)
 		release, deleteStep, err := r.deleteStep(ctx, key)
 		if err != nil {
 			return ctrl.Result{}, err
 		} else if release {
-			return ctrl.Result{}, r.patchFinalizer(ctx, key, false)
+			if err := r.patchFinalizer(ctx, key, false); err != nil {
+				return ctrl.Result{}, err
+			}
+			releasedWithoutDeleteDone(key.UID)
+			return ctrl.Result{}, nil
 		}
 		step = deleteStep
 	} else {

internal/controller/coderaigatewaykey_delete.go

@@ -42,7 +42,25 @@ func (r *CoderAIGatewayKeyReconciler) deleteStep(ctx context.Context, key *coder
 		return false, nil, err
 	}
 	deleting := !controlPlane.DeletionTimestamp.IsZero()
-	sdk, _, wait, err := controlPlaneCoderClientFor(ctx, r.Client, ref.Namespace, ref.Name, deleting)
+	if deleting {
+		// While the control plane holds for this object, Coder keeps running:
+		// use the normal path, which retries, instead of the bounded pass
+		// (#353, item q).
+		now := r.Clock.Now()
+		holds, err := controlPlaneHolds(ctx, r.Client, controlPlane, now)
+		if err != nil {
+			return false, nil, err
+		}
+		deleting = !holds
+		if holds {
+			defer func() {
+				if step != nil {
+					step.requeue = controlPlaneHoldRequeue(controlPlane, now, step.requeue)
+				}
+			}()
+		}
+	}
+	sdk, _, wait, err := controlPlaneCoderClientFor(ctx, r.Client, ref.Namespace, ref.Name, !controlPlane.DeletionTimestamp.IsZero())
 	switch {
 	case err != nil && deleting:
 		return r.releaseWithoutDelete(ctx, key, "the Coder access of CoderControlPlane, which is being deleted, failed: "+err.Error()), nil, nil
@@ -146,6 +164,7 @@ func (r *CoderAIGatewayKeyReconciler) releaseWithoutDelete(ctx context.Context,
 	for _, retiring := range key.Status.RetiringKeys {
 		ids = append(ids, retiring.ID)
 	}
+	countReleasedWithoutDelete(key.UID, "CoderAIGatewayKey")
 	log.FromContext(ctx).Info("Releasing the finalizer without deleting the keys in Coder: the keys can stay in Coder",
 		"reason", "ControlPlaneGone", "why", why, "keyIDs", ids, "listedKeyIDs", left, "nextSerial", key.Status.NextSerial)
 	return true

internal/controller/coderaiprovider_controller.go

@@ -155,13 +155,18 @@ func (r *CoderAIProviderReconciler) Reconcile(ctx context.Context, req ctrl.Requ
 		if !controllerutil.ContainsFinalizer(provider, coderv1alpha1.CoderAIProviderCleanupFinalizer) {
 			return ctrl.Result{}, nil
 		}
+		forgetReleasedWithoutDelete(provider.UID)
 		release, step, err := r.deleteStep(ctx, provider)
 		if err != nil {
 			return ctrl.Result{}, err
 		}
 		if release {
 			r.sentKeys.Delete(provider.UID)
-			return ctrl.Result{}, r.patchFinalizer(ctx, provider, false)
+			if err := r.patchFinalizer(ctx, provider, false); err != nil {
+				return ctrl.Result{}, err
+			}
+			releasedWithoutDeleteDone(provider.UID)
+			return ctrl.Result{}, nil
 		}
 		applyAIProviderStep(provider, step)
 		return ctrl.Result{RequeueAfter: step.requeue}, r.writeStatus(ctx, provider, before)

internal/controller/coderaiprovider_delete.go

@@ -70,7 +70,25 @@ func (r *CoderAIProviderReconciler) deleteStep(ctx context.Context, provider *co
 		return false, nil, err
 	}
 	deleting := !controlPlane.DeletionTimestamp.IsZero()
-	sdk, _, wait, err := controlPlaneCoderClientFor(ctx, r.Client, ref.Namespace, ref.Name, deleting)
+	if deleting {
+		// While the control plane holds for this object, Coder keeps running:
+		// use the normal path, which retries, instead of the bounded pass
+		// (#353, item q).
+		now := time.Now()
+		holds, err := controlPlaneHolds(ctx, r.Client, controlPlane, now)
+		if err != nil {
+			return false, nil, err
+		}
+		deleting = !holds
+		if holds {
+			defer func() {
+				if step != nil {
+					step.requeue = controlPlaneHoldRequeue(controlPlane, now, step.requeue)
+				}
+			}()
+		}
+	}
+	sdk, _, wait, err := controlPlaneCoderClientFor(ctx, r.Client, ref.Namespace, ref.Name, !controlPlane.DeletionTimestamp.IsZero())
 	switch {
 	case err != nil && deleting:
 		// One attempt only, also when the access setup fails (plan D12).
@@ -155,6 +173,7 @@ func (r *CoderAIProviderReconciler) deleteInCoder(ctx context.Context, sdk *code
 // releaseWithoutDelete logs what can stay in Coder, because nothing can
 // delete it any more, and returns true (ControlPlaneGone).
 func (r *CoderAIProviderReconciler) releaseWithoutDelete(ctx context.Context, provider *coderv1alpha1.CoderAIProvider, why string) bool {
+	countReleasedWithoutDelete(provider.UID, "CoderAIProvider")
 	log.FromContext(ctx).Info("Releasing the finalizer without a delete in Coder: the provider can stay in the Coder database",
 		"reason", "ControlPlaneGone", "why", why, "provider", provider.Name, "providerID", provider.Status.ProviderID,
 		"createPending", provider.Status.CreateAttemptTime != nil)

internal/controller/codercontrolplane_controller.go

@@ -28,10 +28,13 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/client-go/util/retry"
+	"k8s.io/utils/clock"
 	ctrl "sigs.k8s.io/controller-runtime"
+	ctrlbuilder "sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
+	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
 
@@ -217,6 +220,16 @@ type CoderControlPlaneReconciler struct {
 	OperatorAccessProvisioner coderbootstrap.OperatorAccessProvisioner
 	LicenseUploader           LicenseUploader
 	EntitlementsInspector     EntitlementsInspector
+	// Clock is optional (nil: the real clock). Tests set it to move the
+	// dependent hold of a deleted control plane.
+	Clock clock.PassiveClock
+}
+
+func (r *CoderControlPlaneReconciler) now() time.Time {
+	if r.Clock == nil {
+		return time.Now()
+	}
+	return r.Clock.Now()
 }
 
 // +kubebuilder:rbac:groups=coder.com,resources=codercontrolplanes,verbs=get;list;watch;create;update;patch;delete
@@ -603,10 +616,32 @@ func (r *CoderControlPlaneReconciler) finalizeWorkspaceRBAC(
 	if !controllerutil.ContainsFinalizer(coderControlPlane, workspaceRBACFinalizer) {
 		return ctrl.Result{}, nil
 	}
-
+	// Revoke Coder's workspace RBAC first: the hold below keeps the Coder
+	// Deployment and its service account, but the AI cleanup calls Coder with
+	// the operator's token and does not need that Kubernetes access.
 	if err := r.cleanupManagedWorkspaceRBAC(ctx, coderControlPlane, nil, nil); err != nil {
 		return ctrl.Result{}, err
 	}
+	// The finalizer keeps the Coder Deployment (owned by this object)
+	// running while AI Gateway keys and AI providers that use it still
+	// delete their resources in Coder (#353). The grace period after the hold
+	// lets each of them run its bounded pass before Coder goes.
+	now := r.now()
+	dependents, perKind, err := controlPlaneDependents(ctx, r.Client, coderControlPlane, now)
+	if err != nil {
+		return ctrl.Result{}, err
+	}
+	if len(dependents) > 0 {
+		holds, err := controlPlaneHolds(ctx, r.Client, coderControlPlane, now.Add(-controlPlaneDependentGraceFor(perKind)))
+		if err != nil {
+			return ctrl.Result{}, err
+		}
+		if holds {
+			log.FromContext(ctx).Info("Waiting for the objects that use this control plane to delete their resources in Coder",
+				"dependents", dependents, "until", coderControlPlane.DeletionTimestamp.Add(controlPlaneDependentHold).UTC())
+			return ctrl.Result{RequeueAfter: controlPlaneDependentPoll}, nil
+		}
+	}
 
 	original := coderControlPlane.DeepCopy()
 	controllerutil.RemoveFinalizer(coderControlPlane, workspaceRBACFinalizer)
@@ -3269,7 +3304,12 @@ func (r *CoderControlPlaneReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Watches(
 			&corev1.ConfigMap{},
 			handler.EnqueueRequestsFromMapFunc(r.reconcileRequestsForEnvFromConfigMap),
-		)
+		).
+		// A control plane that is being deleted waits for these (#353).
+		Watches(&coderv1alpha1.CoderAIGatewayKey{}, handler.EnqueueRequestsFromMapFunc(requestForDependentControlPlane),
+			ctrlbuilder.WithPredicates(dependentRemoved)).
+		Watches(&coderv1alpha1.CoderAIProvider{}, handler.EnqueueRequestsFromMapFunc(requestForDependentControlPlane),
+			ctrlbuilder.WithPredicates(dependentRemoved))
 
 	// Gateway API is optional; only watch HTTPRoutes when the CRD is installed.
 	httpRouteGVK := schema.GroupVersionKind{Group: gatewayv1.GroupVersion.Group, Version: gatewayv1.GroupVersion.Version, Kind: "HTTPRoute"}