diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 645ec140..b100bf66 100644 --- a/.github/workflows/ci.yaml +++ b/.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. diff --git a/docs/how-to/manage-ai-gateway-keys.md b/docs/how-to/manage-ai-gateway-keys.md index 80a03755..b9427ee6 100644 --- a/docs/how-to/manage-ai-gateway-keys.md +++ b/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: diff --git a/docs/how-to/manage-ai-providers.md b/docs/how-to/manage-ai-providers.md index 0bc9c157..e1879d0f 100644 --- a/docs/how-to/manage-ai-providers.md +++ b/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 diff --git a/go.mod b/go.mod index 0a877353..bf98ff62 100644 --- a/go.mod +++ b/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 diff --git a/internal/controller/coderaigatewaykey_controller.go b/internal/controller/coderaigatewaykey_controller.go index 8c5b12e7..0413f569 100644 --- a/internal/controller/coderaigatewaykey_controller.go +++ b/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 { diff --git a/internal/controller/coderaigatewaykey_delete.go b/internal/controller/coderaigatewaykey_delete.go index 9cb82b89..b76597c6 100644 --- a/internal/controller/coderaigatewaykey_delete.go +++ b/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 diff --git a/internal/controller/coderaiprovider_controller.go b/internal/controller/coderaiprovider_controller.go index 914fec42..b65d3808 100644 --- a/internal/controller/coderaiprovider_controller.go +++ b/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) diff --git a/internal/controller/coderaiprovider_delete.go b/internal/controller/coderaiprovider_delete.go index 9fee2b64..527b0d30 100644 --- a/internal/controller/coderaiprovider_delete.go +++ b/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) diff --git a/internal/controller/codercontrolplane_controller.go b/internal/controller/codercontrolplane_controller.go index 6009f0fa..804557c0 100644 --- a/internal/controller/codercontrolplane_controller.go +++ b/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"} diff --git a/internal/controller/controlplane_hold.go b/internal/controller/controlplane_hold.go new file mode 100644 index 00000000..fce505be --- /dev/null +++ b/internal/controller/controlplane_hold.go @@ -0,0 +1,192 @@ +package controller + +import ( + "context" + "errors" + "fmt" + "sync" + "time" + + "github.com/prometheus/client_golang/prometheus" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/event" + "sigs.k8s.io/controller-runtime/pkg/metrics" + "sigs.k8s.io/controller-runtime/pkg/predicate" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + coderv1alpha1 "github.com/coder/coder-k8s/api/v1alpha1" +) + +// controlPlaneDependentHold is how long a CoderControlPlane that is being +// deleted waits for the CoderAIGatewayKey and CoderAIProvider objects that +// use it (#353). Its Coder Deployment keeps running while it waits, so they +// can delete their keys and providers in Coder. It covers the 5-minute +// settle window of a key deletion plus retries. After it, the control plane +// goes, and they release with ControlPlaneGone. +const controlPlaneDependentHold = 10 * time.Minute + +// controlPlaneDependentGrace is the base of how long a control plane stays +// after its hold ends while dependents remain, so each of them sees the hold +// end and runs its bounded pass while Coder still runs. Dependents requeue +// before the hold ends (controlPlaneHoldRequeue). One worker runs the passes +// of one kind one after the other, so controlPlaneDependentGraceFor adds one +// bounded pass per dependent of the larger kind. +const controlPlaneDependentGrace = 30 * time.Second + +// controlPlaneDependentGraceMax caps the grace period, so dependents that +// someone keeps creating cannot keep the control plane forever. +const controlPlaneDependentGraceMax = 5 * time.Minute + +// controlPlaneLiveDependentWait is how long a deleted control plane waits for +// a dependent that is not being deleted itself. It covers one command that +// deletes the control plane before its dependents (kubectl delete -f). After +// it, only dependents that are being deleted hold the control plane: a live +// dependent does no cleanup, so a longer wait for it does nothing. +const controlPlaneLiveDependentWait = 30 * time.Second + +// controlPlaneDependentPoll is the requeue of a control plane that waits for +// its dependents. A dependent's removal also requeues it at once. +const controlPlaneDependentPoll = 10 * time.Second + +// releasedWithoutDelete counts finalizers that went while their keys or +// providers can stay in Coder (#353), by kind. Today the release log line is +// the only other signal. countReleasedWithoutDelete records a release, and +// releasedWithoutDeleteDone counts it once the finalizer is gone, so a +// failed finalizer patch and its retry count once. Each delete reconcile +// first drops an earlier record (forgetReleasedWithoutDelete), so a retry +// that then deletes in Coder does not count. +var releasedWithoutDelete = prometheus.NewCounterVec(prometheus.CounterOpts{ + Name: "coder_k8s_cleanup_released_without_delete_total", + Help: "Finalizers released without deleting the object's resources in Coder, because Coder was not reachable.", +}, []string{"kind"}) + +var pendingReleases sync.Map // types.UID -> kind label + +func init() { + metrics.Registry.MustRegister(releasedWithoutDelete) +} + +func countReleasedWithoutDelete(uid types.UID, kind string) { pendingReleases.Store(uid, kind) } + +// forgetReleasedWithoutDelete drops a release of uid that an earlier +// reconcile recorded but whose finalizer patch failed. +func forgetReleasedWithoutDelete(uid types.UID) { pendingReleases.Delete(uid) } + +// releasedWithoutDeleteDone counts the release of uid, if one was recorded, +// after its finalizer patch succeeded. +func releasedWithoutDeleteDone(uid types.UID) { + if kind, ok := pendingReleases.LoadAndDelete(uid); ok { + releasedWithoutDelete.WithLabelValues(kind.(string)).Inc() + } +} + +// controlPlaneHoldEnd is when the hold of a control plane that is being +// deleted ends. +func controlPlaneHoldEnd(controlPlane *coderv1alpha1.CoderControlPlane) time.Time { + return controlPlane.DeletionTimestamp.Add(controlPlaneDependentHold) +} + +// controlPlaneDependentGraceFor is the grace period after the hold when +// perKind dependents of one kind remain (controlPlaneDependentGrace). +func controlPlaneDependentGraceFor(perKind int) time.Duration { + return min(controlPlaneDependentGrace+time.Duration(perKind)*max(aiGatewayKeyBoundedDeleteTimeout, aiProviderBoundedDeleteTimeout), controlPlaneDependentGraceMax) +} + +// controlPlaneHoldRequeue caps a dependent's requeue under the hold, so it +// looks again right after the hold ends, inside the grace period. +func controlPlaneHoldRequeue(controlPlane *coderv1alpha1.CoderControlPlane, now time.Time, requeue time.Duration) time.Duration { + return min(requeue, max(controlPlaneHoldEnd(controlPlane).Sub(now)+time.Second, time.Second)) +} + +// controlPlaneHolds reports whether controlPlane, which is being deleted, +// waits for its dependents at now: it still has its finalizer, the hold time +// has not passed, it is not deleted in the foreground, and its namespace is +// not being deleted. A namespace deletion removes Coder's pods at once, and a +// foreground deletion removes the Coder Deployment (blockOwnerDeletion) +// before the control plane, so no hold can keep Coder reachable then. +func controlPlaneHolds(ctx context.Context, reader client.Reader, controlPlane *coderv1alpha1.CoderControlPlane, now time.Time) (bool, error) { + if reader == nil || controlPlane == nil { + return false, errors.New("assertion failed: the control plane hold check needs a reader and a control plane") + } + if controlPlane.DeletionTimestamp.IsZero() { + return false, errors.New("assertion failed: the control plane hold applies only while the control plane is being deleted") + } + if !controllerutil.ContainsFinalizer(controlPlane, workspaceRBACFinalizer) || controllerutil.ContainsFinalizer(controlPlane, metav1.FinalizerDeleteDependents) || + !now.Before(controlPlaneHoldEnd(controlPlane)) { + return false, nil + } + namespace := &corev1.Namespace{} + if err := reader.Get(ctx, types.NamespacedName{Name: controlPlane.Namespace}, namespace); err != nil { + return false, fmt.Errorf("get namespace %s: %w", controlPlane.Namespace, err) + } + return namespace.DeletionTimestamp.IsZero() && namespace.Status.Phase != corev1.NamespaceTerminating, nil +} + +// controlPlaneDependents returns the CoderAIGatewayKey and CoderAIProvider +// objects in controlPlane's namespace that reference it and hold it at now, +// as kind/name, and the larger count of one kind. A dependent holds it while +// it is being deleted and still has its cleanup finalizer (another finalizer +// does not hold it), and a live one only for controlPlaneLiveDependentWait +// after the control plane deletion. +func controlPlaneDependents(ctx context.Context, reader client.Reader, controlPlane *coderv1alpha1.CoderControlPlane, now time.Time) ([]string, int, error) { + if controlPlane.DeletionTimestamp.IsZero() { + return nil, 0, errors.New("assertion failed: dependents hold only a control plane that is being deleted") + } + liveHolds := now.Before(controlPlane.DeletionTimestamp.Add(controlPlaneLiveDependentWait)) + holds := func(o client.Object, finalizer string) bool { + if o.GetDeletionTimestamp().IsZero() { + return liveHolds + } + return controllerutil.ContainsFinalizer(o, finalizer) + } + var dependents []string + keys := &coderv1alpha1.CoderAIGatewayKeyList{} + if err := reader.List(ctx, keys, client.InNamespace(controlPlane.Namespace)); err != nil { + return nil, 0, fmt.Errorf("list AI Gateway keys of namespace %s: %w", controlPlane.Namespace, err) + } + for i := range keys.Items { + if k := &keys.Items[i]; k.Spec.ControlPlaneRef.Name == controlPlane.Name && holds(k, coderv1alpha1.CoderAIGatewayKeyCleanupFinalizer) { + dependents = append(dependents, "CoderAIGatewayKey/"+k.Name) + } + } + keyCount := len(dependents) + providers := &coderv1alpha1.CoderAIProviderList{} + if err := reader.List(ctx, providers, client.InNamespace(controlPlane.Namespace)); err != nil { + return nil, 0, fmt.Errorf("list AI providers of namespace %s: %w", controlPlane.Namespace, err) + } + for i := range providers.Items { + if p := &providers.Items[i]; p.Spec.ControlPlaneRef.Name == controlPlane.Name && holds(p, coderv1alpha1.CoderAIProviderCleanupFinalizer) { + dependents = append(dependents, "CoderAIProvider/"+p.Name) + } + } + return dependents, max(keyCount, len(dependents)-keyCount), nil +} + +// dependentRemoved passes only the removal of a dependent object, the event +// that can end a control plane's hold. +var dependentRemoved = predicate.Funcs{ + CreateFunc: func(event.CreateEvent) bool { return false }, + UpdateFunc: func(event.UpdateEvent) bool { return false }, + DeleteFunc: func(event.DeleteEvent) bool { return true }, + GenericFunc: func(event.GenericEvent) bool { return false }, +} + +// requestForDependentControlPlane maps a CoderAIGatewayKey or CoderAIProvider +// to the control plane that it references. +func requestForDependentControlPlane(_ context.Context, obj client.Object) []reconcile.Request { + var name string + switch o := obj.(type) { + case *coderv1alpha1.CoderAIGatewayKey: + name = o.Spec.ControlPlaneRef.Name + case *coderv1alpha1.CoderAIProvider: + name = o.Spec.ControlPlaneRef.Name + } + if name == "" { + return nil + } + return []reconcile.Request{{NamespacedName: types.NamespacedName{Namespace: obj.GetNamespace(), Name: name}}} +} diff --git a/internal/controller/controlplane_hold_internal_test.go b/internal/controller/controlplane_hold_internal_test.go new file mode 100644 index 00000000..9b1bd8f7 --- /dev/null +++ b/internal/controller/controlplane_hold_internal_test.go @@ -0,0 +1,18 @@ +package controller + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// #353: the grace period after the hold grows by one bounded pass per +// dependent of one kind, up to a cap, so dependents that someone keeps +// creating cannot keep a deleted control plane forever. +func TestControlPlaneDependentGraceFor(t *testing.T) { + t.Parallel() + require.Equal(t, 30*time.Second, controlPlaneDependentGraceFor(0)) + require.Equal(t, time.Minute, controlPlaneDependentGraceFor(3)) + require.Equal(t, 5*time.Minute, controlPlaneDependentGraceFor(1000), "the grace period has a cap") +} diff --git a/internal/controller/controlplane_hold_test.go b/internal/controller/controlplane_hold_test.go new file mode 100644 index 00000000..2fb44bcf --- /dev/null +++ b/internal/controller/controlplane_hold_test.go @@ -0,0 +1,388 @@ +package controller_test + +import ( + "context" + "net/http" + "testing" + "time" + + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/types" + clocktesting "k8s.io/utils/clock/testing" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + "sigs.k8s.io/controller-runtime/pkg/metrics" + + coderv1alpha1 "github.com/coder/coder-k8s/api/v1alpha1" + "github.com/coder/coder-k8s/internal/controller" +) + +const workspaceRBACFinalizer = "coder.com/workspace-rbac-cleanup" + +// deleteFinalizedControlPlane gives the control plane its own finalizer, as +// its reconciler does, and deletes it, so it stays Terminating. +func deleteFinalizedControlPlane(ctx context.Context, t *testing.T, namespace, name string) { + t.Helper() + key := client.ObjectKey{Namespace: namespace, Name: name} + controlPlane := &coderv1alpha1.CoderControlPlane{} + require.NoError(t, k8sClient.Get(ctx, key, controlPlane)) + controlPlane.Finalizers = append(controlPlane.Finalizers, workspaceRBACFinalizer) + require.NoError(t, k8sClient.Update(ctx, controlPlane)) + require.NoError(t, k8sClient.Delete(ctx, controlPlane)) + t.Cleanup(func() { + if k8sClient.Get(context.Background(), key, controlPlane) == nil { + controlPlane.Finalizers = nil + _ = k8sClient.Update(context.Background(), controlPlane) + } + }) +} + +func releasedWithoutDelete(t *testing.T, kind string) float64 { + t.Helper() + families, err := metrics.Registry.Gather() + require.NoError(t, err) + for _, f := range families { + if f.GetName() != "coder_k8s_cleanup_released_without_delete_total" { + continue + } + for _, m := range f.GetMetric() { + for _, l := range m.GetLabel() { + if l.GetName() == "kind" && l.GetValue() == kind { + return m.GetCounter().GetValue() + } + } + } + } + return 0 +} + +// #353: a control plane that is being deleted keeps its finalizer, and so its +// Coder Deployment, while a CoderAIGatewayKey or CoderAIProvider uses it. A +// dependent of another control plane does not hold it. A namespace deletion +// does not wait: it removes Coder's pods anyway. +func TestControlPlaneWaitsForDependents(t *testing.T) { + t.Parallel() + ctx := context.Background() + reconcileGone := func(t *testing.T, namespace string) (ctrl.Result, bool) { + t.Helper() + r := &controller.CoderControlPlaneReconciler{Client: k8sClient, Scheme: scheme} + result, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Namespace: namespace, Name: "coder"}}) + require.NoError(t, err) + err = k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: "coder"}, &coderv1alpha1.CoderControlPlane{}) + return result, apierrors.IsNotFound(err) + } + t.Run("dependents", func(t *testing.T) { + t.Parallel() + env := newAIGatewayKeyEnv(ctx, t, ptrTo(true)) + key := env.createKey(ctx, t, "coder", testAIGatewaySecret) + other := env.createNamedKey(ctx, t, "other", "another-control-plane", "other-secret") + provider := &coderv1alpha1.CoderAIProvider{ + ObjectMeta: metav1.ObjectMeta{Name: "openai", Namespace: env.namespace}, + Spec: coderv1alpha1.CoderAIProviderSpec{ + ControlPlaneRef: coderv1alpha1.CoderControlPlaneReference{Name: "coder"}, Type: "openai", BaseURL: "https://api.openai.com/v1", + APIKeySecretRef: coderv1alpha1.SecretKeySelector{Name: "openai-key", Key: "api-key"}, + }, + } + require.NoError(t, k8sClient.Create(ctx, provider)) + deleteFinalizedControlPlane(ctx, t, env.namespace, "coder") + + result, gone := reconcileGone(t, env.namespace) + require.False(t, gone, "the control plane waits for its key and provider") + require.Equal(t, 10*time.Second, result.RequeueAfter) + require.NoError(t, k8sClient.Delete(ctx, key)) + _, gone = reconcileGone(t, env.namespace) + require.False(t, gone, "the control plane waits for its provider") + require.NoError(t, k8sClient.Delete(ctx, provider)) + _, gone = reconcileGone(t, env.namespace) + require.True(t, gone, "a key of another control plane does not hold this one") + require.NoError(t, k8sClient.Get(ctx, client.ObjectKeyFromObject(other), other)) + }) + t.Run("namespace deletion", func(t *testing.T) { + t.Parallel() + env := newAIGatewayKeyEnv(ctx, t, ptrTo(true)) + env.createKey(ctx, t, "coder", testAIGatewaySecret) + deleteFinalizedControlPlane(ctx, t, env.namespace, "coder") + require.NoError(t, k8sClient.Delete(ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: env.namespace}})) + _, gone := reconcileGone(t, env.namespace) + require.True(t, gone, "a namespace deletion does not wait") + }) +} + +// #353: while the control plane holds for it, a deleted key takes the normal +// path: it waits for the settle window, then deletes every key, instead of +// one bounded pass. A control plane without operator access makes it wait +// and retry (item q), not release. +func TestCoderAIGatewayKeyDeleteUnderControlPlaneHold(t *testing.T) { + t.Parallel() + ctx := context.Background() + env := newAIGatewayKeyEnv(ctx, t, ptrTo(true)) + got := rotatedKey(ctx, t, &env) + deleteFinalizedControlPlane(ctx, t, env.namespace, "coder") + require.NoError(t, k8sClient.Delete(ctx, got)) + + got, result := env.reconcile(ctx, t, got) + requireAIGatewayKeyStep(t, got, false, "DeletingKeys") + require.Equal(t, 5*time.Minute, result.RequeueAfter, "the settle window, not the bounded pass") + require.Len(t, env.fake.aiGatewayKeys(), 2) + + controlPlane := &coderv1alpha1.CoderControlPlane{} + require.NoError(t, k8sClient.Get(ctx, client.ObjectKey{Namespace: env.namespace, Name: "coder"}, controlPlane)) + controlPlane.Status.OperatorAccessReady = false + require.NoError(t, k8sClient.Status().Update(ctx, controlPlane)) + env.clock.Step(5 * time.Minute) + got, result = env.reconcile(ctx, t, got) + requireAIGatewayKeyStep(t, got, false, "OperatorAccessNotReady") + require.Equal(t, time.Minute, result.RequeueAfter) + require.Len(t, env.fake.aiGatewayKeys(), 2, "no release without a delete") + + controlPlane.Status.OperatorAccessReady = true + require.NoError(t, k8sClient.Status().Update(ctx, controlPlane)) + gone, _ := env.reconcile(ctx, t, got) + require.Nil(t, gone) + require.Empty(t, env.fake.aiGatewayKeys()) +} + +// #353: while the control plane holds for it, a failed provider delete +// retries instead of releasing after one attempt. +func TestCoderAIProviderDeleteUnderControlPlaneHold(t *testing.T) { + t.Parallel() + ctx := context.Background() + env, _, provider := newReadyAIProvider(ctx, t) + deleteFinalizedControlPlane(ctx, t, env.namespace, "coder") + env.fake.failNext(routeDeleteAIProvider, fakeFault{Status: http.StatusBadGateway}) + got, result := env.deleteAIProvider(ctx, t, provider) + require.NotNil(t, got, "a failed delete keeps the finalizer while the control plane waits") + requireAIProviderStep(t, got, false, "CoderUnavailable") + require.Equal(t, 60*time.Second, result.RequeueAfter) + got, _ = env.reconcileDeleting(ctx, t, got) + require.Nil(t, got) + _, found := env.fake.aiProvider("openai") + require.False(t, found) + require.Equal(t, 2, env.fake.requestCount(routeDeleteAIProvider)) +} + +// #353: a release without a delete in Coder counts once in +// coder_k8s_cleanup_released_without_delete_total, also when the finalizer +// patch fails and the release runs again. Not parallel: other tests release +// keys too. +func TestReleasedWithoutDeleteMetric(t *testing.T) { + ctx := context.Background() + env := newAIGatewayKeyEnv(ctx, t, ptrTo(true)) + got := rotatedKey(ctx, t, &env) + before := releasedWithoutDelete(t, "CoderAIGatewayKey") + require.NoError(t, k8sClient.Delete(ctx, &coderv1alpha1.CoderControlPlane{ObjectMeta: metav1.ObjectMeta{Namespace: env.namespace, Name: "coder"}})) + require.NoError(t, k8sClient.Delete(ctx, got)) + base, err := client.NewWithWatch(cfg, client.Options{Scheme: scheme}) + require.NoError(t, err) + failed := false + env.client = interceptor.NewClient(base, interceptor.Funcs{ + Patch: func(ctx context.Context, c client.WithWatch, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + if _, ok := obj.(*coderv1alpha1.CoderAIGatewayKey); ok && !failed { + failed = true + return apierrors.NewConflict(schema.GroupResource{Resource: "coderaigatewaykeys"}, obj.GetName(), nil) + } + return c.Patch(ctx, obj, patch, opts...) + }, + }) + _, _, err = env.reconcileErr(ctx, t, got) + require.Error(t, err, "the finalizer patch fails once") + require.Equal(t, before, releasedWithoutDelete(t, "CoderAIGatewayKey"), "no count before the finalizer is gone") + gone, _ := env.reconcile(ctx, t, got) + require.Nil(t, gone) + require.Equal(t, before+1, releasedWithoutDelete(t, "CoderAIGatewayKey")) +} + +// #353: a foreground deletion removes the Coder Deployment before the +// control plane, so the control plane does not hold for its dependents. +func TestControlPlaneForegroundDeletionDoesNotHold(t *testing.T) { + t.Parallel() + ctx := context.Background() + env := newAIGatewayKeyEnv(ctx, t, ptrTo(true)) + env.createKey(ctx, t, "coder", testAIGatewaySecret) + key := client.ObjectKey{Namespace: env.namespace, Name: "coder"} + controlPlane := &coderv1alpha1.CoderControlPlane{} + require.NoError(t, k8sClient.Get(ctx, key, controlPlane)) + controlPlane.Finalizers = append(controlPlane.Finalizers, workspaceRBACFinalizer) + require.NoError(t, k8sClient.Update(ctx, controlPlane)) + require.NoError(t, k8sClient.Delete(ctx, controlPlane, client.PropagationPolicy(metav1.DeletePropagationForeground))) + t.Cleanup(func() { + if k8sClient.Get(context.Background(), key, controlPlane) == nil { + controlPlane.Finalizers = nil + _ = k8sClient.Update(context.Background(), controlPlane) + } + }) + r := &controller.CoderControlPlaneReconciler{Client: k8sClient, Scheme: scheme} + _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: key}) + require.NoError(t, err) + require.NoError(t, k8sClient.Get(ctx, key, controlPlane)) + require.NotContains(t, controlPlane.Finalizers, workspaceRBACFinalizer, "no hold under foreground deletion") +} + +// #353: when the hold ends, the control plane stays for a short grace period +// while dependents remain, and a dependent under the hold requeues before the +// hold ends, so it runs its bounded pass while Coder still runs. +func TestControlPlaneHoldEnd(t *testing.T) { + t.Parallel() + ctx := context.Background() + env := newAIGatewayKeyEnv(ctx, t, ptrTo(true)) + got := rotatedKey(ctx, t, &env) + deleteFinalizedControlPlane(ctx, t, env.namespace, "coder") + controlPlane := &coderv1alpha1.CoderControlPlane{} + require.NoError(t, k8sClient.Get(ctx, client.ObjectKey{Namespace: env.namespace, Name: "coder"}, controlPlane)) + end := controlPlane.DeletionTimestamp.Add(10 * time.Minute) + + controlPlane.Status.OperatorAccessReady = false + require.NoError(t, k8sClient.Status().Update(ctx, controlPlane)) + require.NoError(t, k8sClient.Delete(ctx, got)) + env.clock.SetTime(end.Add(-30 * time.Second)) + got, result := env.reconcile(ctx, t, got) + requireAIGatewayKeyStep(t, got, false, "OperatorAccessNotReady") + require.Equal(t, 31*time.Second, result.RequeueAfter, "the dependent looks again right after the hold ends") + + ccpClock := clocktesting.NewFakeClock(end.Add(10 * time.Second)) + r := &controller.CoderControlPlaneReconciler{Client: k8sClient, Scheme: scheme, Clock: ccpClock} + _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(controlPlane)}) + require.NoError(t, err) + require.NoError(t, k8sClient.Get(ctx, client.ObjectKeyFromObject(controlPlane), controlPlane), "the control plane stays in the grace period") + ccpClock.SetTime(end.Add(31 * time.Second)) + _, err = r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(controlPlane)}) + require.NoError(t, err) + require.NoError(t, k8sClient.Get(ctx, client.ObjectKeyFromObject(controlPlane), controlPlane), "each dependent adds one bounded pass to the grace period") + ccpClock.SetTime(end.Add(41 * time.Second)) + _, err = r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(controlPlane)}) + require.NoError(t, err) + require.True(t, apierrors.IsNotFound(k8sClient.Get(ctx, client.ObjectKeyFromObject(controlPlane), controlPlane)), "after the grace period it goes") +} + +// #353: a dependent that is not being deleted does no cleanup, so it holds a +// deleted control plane only for 30 s, which covers one command that deletes +// both. A dependent that is being deleted holds it for the whole hold. +func TestControlPlaneLiveDependentWait(t *testing.T) { + t.Parallel() + ctx := context.Background() + env := newAIGatewayKeyEnv(ctx, t, ptrTo(true)) + env.createKey(ctx, t, "coder", testAIGatewaySecret) + deleting := env.createNamedKey(ctx, t, "deleting", "coder", "deleting-secret") + deleting.Finalizers = []string{coderv1alpha1.CoderAIGatewayKeyCleanupFinalizer} + require.NoError(t, k8sClient.Update(ctx, deleting)) + removeFinalizer := func() { + if k8sClient.Get(context.Background(), client.ObjectKeyFromObject(deleting), deleting) == nil { + deleting.Finalizers = nil + require.NoError(t, k8sClient.Update(context.Background(), deleting)) + } + } + t.Cleanup(removeFinalizer) + // A deleted key whose cleanup finalizer is gone, held only by another + // finalizer, does not hold the control plane. + foreign := env.createNamedKey(ctx, t, "foreign", "coder", "foreign-secret") + foreign.Finalizers = []string{"example.com/keep"} + require.NoError(t, k8sClient.Update(ctx, foreign)) + t.Cleanup(func() { + if k8sClient.Get(context.Background(), client.ObjectKeyFromObject(foreign), foreign) == nil { + foreign.Finalizers = nil + _ = k8sClient.Update(context.Background(), foreign) + } + }) + require.NoError(t, k8sClient.Delete(ctx, foreign)) + deleteFinalizedControlPlane(ctx, t, env.namespace, "coder") + ref := client.ObjectKey{Namespace: env.namespace, Name: "coder"} + controlPlane := &coderv1alpha1.CoderControlPlane{} + require.NoError(t, k8sClient.Get(ctx, ref, controlPlane)) + ccpClock := clocktesting.NewFakeClock(controlPlane.DeletionTimestamp.Add(29 * time.Second)) + r := &controller.CoderControlPlaneReconciler{Client: k8sClient, Scheme: scheme, Clock: ccpClock} + gone := func() bool { + t.Helper() + _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: ref}) + require.NoError(t, err) + return apierrors.IsNotFound(k8sClient.Get(ctx, ref, &coderv1alpha1.CoderControlPlane{})) + } + require.False(t, gone(), "live keys hold the control plane for 30 s") + require.NoError(t, k8sClient.Delete(ctx, deleting)) + ccpClock.SetTime(controlPlane.DeletionTimestamp.Add(31 * time.Second)) + require.False(t, gone(), "a key that is being deleted still holds it") + removeFinalizer() + require.True(t, gone(), "after 30 s a live key, or a deleted key without its cleanup finalizer, does not hold it") +} + +// #353: a release that a failed finalizer patch left recorded does not count +// when the next reconcile deletes the keys in Coder. Not parallel: other +// tests release keys too. +func TestReleasedWithoutDeleteMetricRecovered(t *testing.T) { + ctx := context.Background() + env := newAIGatewayKeyEnv(ctx, t, ptrTo(true)) + got := rotatedKey(ctx, t, &env) + before := releasedWithoutDelete(t, "CoderAIGatewayKey") + deleteFinalizedControlPlane(ctx, t, env.namespace, "coder") + controlPlane := &coderv1alpha1.CoderControlPlane{} + require.NoError(t, k8sClient.Get(ctx, client.ObjectKey{Namespace: env.namespace, Name: "coder"}, controlPlane)) + controlPlane.Status.OperatorAccessReady = false + require.NoError(t, k8sClient.Status().Update(ctx, controlPlane)) + require.NoError(t, k8sClient.Delete(ctx, got)) + env.clock.SetTime(controlPlane.DeletionTimestamp.Add(10*time.Minute + time.Second)) // The hold has ended. + base, err := client.NewWithWatch(cfg, client.Options{Scheme: scheme}) + require.NoError(t, err) + failed := false + env.client = interceptor.NewClient(base, interceptor.Funcs{ + Patch: func(ctx context.Context, c client.WithWatch, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { + if _, ok := obj.(*coderv1alpha1.CoderAIGatewayKey); ok && !failed { + failed = true + return apierrors.NewConflict(schema.GroupResource{Resource: "coderaigatewaykeys"}, obj.GetName(), nil) + } + return c.Patch(ctx, obj, patch, opts...) + }, + }) + _, _, err = env.reconcileErr(ctx, t, got) + require.Error(t, err, "the release without a delete fails its finalizer patch") + require.Len(t, env.fake.aiGatewayKeys(), 2) + + controlPlane.Status.OperatorAccessReady = true + require.NoError(t, k8sClient.Status().Update(ctx, controlPlane)) + gone, _ := env.reconcile(ctx, t, got) + require.Nil(t, gone) + require.Empty(t, env.fake.aiGatewayKeys(), "the retry deletes the keys in Coder") + require.Equal(t, before, releasedWithoutDelete(t, "CoderAIGatewayKey"), "a release that deleted in Coder does not count") +} + +// #353: the hold keeps the Coder Deployment and its service account, so the +// control plane revokes the managed workspace RBAC before it waits. +func TestControlPlaneHoldRevokesWorkspaceRBAC(t *testing.T) { + t.Parallel() + ctx := context.Background() + namespace := createTestNamespace(ctx, t, "hold-rbac") + workspaceNamespace := createTestNamespace(ctx, t, "hold-rbac-ws") + cp := createCoderControlPlaneUnstructured(ctx, t, "coder", namespace, map[string]any{ + "image": "test-workspace-rbac:latest", + "serviceAccount": map[string]any{"name": "coder-hold-sa"}, + "rbac": map[string]any{"workspacePerms": true, "workspaceNamespaces": []any{workspaceNamespace}}, + }) + r := &controller.CoderControlPlaneReconciler{Client: k8sClient, Scheme: scheme} + ref := types.NamespacedName{Namespace: namespace, Name: cp.Name} + _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: ref}) + require.NoError(t, err) + bindingRef := types.NamespacedName{Namespace: workspaceNamespace, Name: expectedWorkspaceRoleBindingName(t, cp, "coder-hold-sa")} + require.NoError(t, k8sClient.Get(ctx, bindingRef, &rbacv1.RoleBinding{}), "the reconcile grants workspace RBAC") + + key := &coderv1alpha1.CoderAIGatewayKey{ + ObjectMeta: metav1.ObjectMeta{Name: "gateway", Namespace: namespace, Finalizers: []string{coderv1alpha1.CoderAIGatewayKeyCleanupFinalizer}}, + Spec: coderv1alpha1.CoderAIGatewayKeySpec{ControlPlaneRef: coderv1alpha1.CoderControlPlaneReference{Name: cp.Name}, SecretName: "gateway-key"}, + } + require.NoError(t, k8sClient.Create(ctx, key)) + t.Cleanup(func() { + if k8sClient.Get(context.Background(), client.ObjectKeyFromObject(key), key) == nil { + key.Finalizers = nil + _ = k8sClient.Update(context.Background(), key) + } + }) + require.NoError(t, k8sClient.Delete(ctx, key)) + require.NoError(t, k8sClient.Delete(ctx, cp)) + result, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: ref}) + require.NoError(t, err) + require.Equal(t, 10*time.Second, result.RequeueAfter, "the control plane holds for the key") + require.True(t, apierrors.IsNotFound(k8sClient.Get(ctx, bindingRef, &rbacv1.RoleBinding{})), "the hold does not keep workspace RBAC") +}