From 7c554ccc5589da240bf2d454e626458b46382822 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 30 Oct 2026 14:14:49 +0000 Subject: [PATCH 01/18] test: a provider delete pass under the control plane hold ends at the hold end (#353) Change-Id: Ib45cb0e8ada72ddebca134b0c75dcb7262aa6e7c Signed-off-by: Thomas Kosiewski --- .../coderaiprovider_delete_internal_test.go | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 internal/controller/coderaiprovider_delete_internal_test.go diff --git a/internal/controller/coderaiprovider_delete_internal_test.go b/internal/controller/coderaiprovider_delete_internal_test.go new file mode 100644 index 00000000..9d292436 --- /dev/null +++ b/internal/controller/coderaiprovider_delete_internal_test.go @@ -0,0 +1,69 @@ +package controller + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/google/uuid" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + coderv1alpha1 "github.com/coder/coder-k8s/api/v1alpha1" +) + +// #384 follow-up 4 for providers: a delete pass under the control plane hold +// ends at the hold end, also when Coder does not answer, and its retry counts +// from the end of the pass. Here the hold ends in 5 s and Coder hangs on the +// DELETE, whose own request timeout is 30 s. +func TestAIProviderDeleteUnderHoldEndsAtHoldEnd(t *testing.T) { + t.Parallel() + coder := httptest.NewServer(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + <-r.Context().Done() + })) + t.Cleanup(coder.Close) + + scheme := runtime.NewScheme() + require.NoError(t, clientgoscheme.AddToScheme(scheme)) + require.NoError(t, coderv1alpha1.AddToScheme(scheme)) + deleted := metav1.NewTime(time.Now().Add(5*time.Second - controlPlaneDependentHold)) + controlPlane := &coderv1alpha1.CoderControlPlane{ObjectMeta: metav1.ObjectMeta{ + Name: "coder", Namespace: "ns", UID: "cp-uid", DeletionTimestamp: &deleted, Finalizers: []string{workspaceRBACFinalizer}, + }} + controlPlane.Status.URL = coder.URL + controlPlane.Status.OperatorAccessReady = true + controlPlane.Status.OperatorTokenSecretRef = &coderv1alpha1.SecretKeySelector{Name: "coder-operator-token"} + token := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "coder-operator-token", Namespace: "ns", + OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(controlPlane, coderv1alpha1.GroupVersion.WithKind("CoderControlPlane"))}, + }, + Data: map[string][]byte{coderv1alpha1.DefaultTokenSecretKey: []byte("token")}, + } + namespace := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "ns"}} + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(controlPlane, token, namespace).Build() + r := &CoderAIProviderReconciler{Client: c, APIReader: c, Scheme: scheme, OperatorNamespace: "coder-system"} + held, err := controlPlaneHolds(context.Background(), r.Client, controlPlane, time.Now()) + require.NoError(t, err) + require.True(t, held, "assertion failed: the test needs a control plane that holds") + + provider := &coderv1alpha1.CoderAIProvider{ObjectMeta: metav1.ObjectMeta{Name: "openai", Namespace: "ns", UID: "provider-uid"}} + provider.Spec.ControlPlaneRef.Name = "coder" + provider.Status.ProviderID = uuid.NewString() + start := time.Now() + release, step, err := r.deleteStep(context.Background(), provider) + require.NoError(t, err) + require.Less(t, time.Since(start), 15*time.Second, "the pass ends at the hold end, not at the 30 s request timeout") + require.False(t, release, "a pass under the hold never releases without a delete") + require.NotNil(t, step) + require.Equal(t, "CoderUnavailable", step.reason) + // The requeue counts from the end of the pass: the time left to the hold + // end, which is about 0 here, plus 1 s (controlPlaneHoldRequeue). + require.LessOrEqual(t, step.requeue, 2*time.Second, "the next pass runs right after the hold end") +} From a8a7d1541f6ec76159618ca2ef0efb215da254c9 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 30 Oct 2026 14:15:25 +0000 Subject: [PATCH 02/18] fix: end a provider delete pass under the control plane hold at the hold end (#353) Change-Id: Idd99f51f9b69f1d51228090a15b9d965082bbce8 Signed-off-by: Thomas Kosiewski --- internal/controller/coderaiprovider_delete.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/internal/controller/coderaiprovider_delete.go b/internal/controller/coderaiprovider_delete.go index 7a67c086..e7b636af 100644 --- a/internal/controller/coderaiprovider_delete.go +++ b/internal/controller/coderaiprovider_delete.go @@ -83,16 +83,23 @@ func (r *CoderAIProviderReconciler) deleteStep(ctx context.Context, provider *co // 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) + holds, err := controlPlaneHolds(ctx, r.Client, controlPlane, time.Now()) if err != nil { return false, nil, err } deleting = !holds if holds { + // A pass under the hold ends at the hold end, however slowly + // Coder answers, so the bounded pass after it still fits in the + // control plane's grace period (#384 follow-up 4). + var cancel context.CancelFunc + ctx, cancel = context.WithDeadline(ctx, controlPlaneHoldEnd(controlPlane)) + defer cancel() defer func() { if step != nil { - step.requeue = controlPlaneHoldRequeue(controlPlane, now, step.requeue) + // From the end of the pass: a pass that ran until the + // deadline retries right after the hold end. + step.requeue = controlPlaneHoldRequeue(controlPlane, time.Now(), step.requeue) } }() } From 6f66471766bd5cff29608d42805d666e1e0555d1 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 30 Oct 2026 14:17:26 +0000 Subject: [PATCH 03/18] test: count a CoderMCPServer released without a delete in Coder (#353) Change-Id: I12ec09f9374ff5c91cedf843b7d0be637891dda8 Signed-off-by: Thomas Kosiewski --- .../controller/codermcpserver_delete_test.go | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/internal/controller/codermcpserver_delete_test.go b/internal/controller/codermcpserver_delete_test.go index a4616d89..2366b060 100644 --- a/internal/controller/codermcpserver_delete_test.go +++ b/internal/controller/codermcpserver_delete_test.go @@ -11,8 +11,10 @@ import ( "github.com/stretchr/testify/require" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" coderv1alpha1 "github.com/coder/coder-k8s/api/v1alpha1" "github.com/coder/coder-k8s/internal/controller" @@ -306,6 +308,62 @@ func TestCoderMCPServerDeleteWhileControlPlaneDeletes(t *testing.T) { } } +// #384 follow-up 2 (#353): a release without a delete in Coder counts once in +// coder_k8s_cleanup_released_without_delete_total, after the finalizer patch +// succeeds. A release that a failed patch left recorded does not count when +// the retry deletes the config. Not parallel: other tests release MCP servers +// too. +func TestCoderMCPServerReleasedWithoutDeleteMetric(t *testing.T) { + ctx := context.Background() + failFirstPatch := func(t *testing.T, env *mcpServerEnv) { + t.Helper() + base, err := client.NewWithWatch(cfg, client.Options{Scheme: scheme}) + require.NoError(t, err) + failed := false + env.r = &controller.CoderMCPServerReconciler{Scheme: scheme, 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.CoderMCPServer); ok && !failed { + failed = true + return apierrors.NewConflict(schema.GroupResource{Resource: "codermcpservers"}, obj.GetName(), nil) + } + return c.Patch(ctx, obj, patch, opts...) + }, + })} + } + t.Run("control plane gone", func(t *testing.T) { + env := newMCPServerEnv(ctx, t, true) + env.keySecret(ctx, t, testMCPKey) + got, _ := env.applied(ctx, t, nil) + before := releasedWithoutDelete(t, "CoderMCPServer") + require.NoError(t, k8sClient.Delete(ctx, env.controlPlane)) + require.NoError(t, k8sClient.Delete(ctx, got)) + failFirstPatch(t, &env) + _, err := env.r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(got)}) + require.Error(t, err, "the finalizer patch fails once") + require.Equal(t, before, releasedWithoutDelete(t, "CoderMCPServer"), "no count before the finalizer is gone") + gone, _ := env.reconcileDeleting(ctx, t, got) + require.Nil(t, gone) + require.Len(t, env.fake.mcpServers(), 1, "the config stays in Coder") + require.Equal(t, before+1, releasedWithoutDelete(t, "CoderMCPServer")) + }) + t.Run("recovered", func(t *testing.T) { + env := newMCPServerEnv(ctx, t, true) + env.keySecret(ctx, t, testMCPKey) + got, _ := env.applied(ctx, t, nil) + before := releasedWithoutDelete(t, "CoderMCPServer") + holdControlPlane(ctx, t, env.namespace) + require.NoError(t, k8sClient.Delete(ctx, got)) + env.fake.failNext(routeDeleteMCPServer, fakeFault{Status: http.StatusBadGateway}) + failFirstPatch(t, &env) + _, err := env.r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(got)}) + require.Error(t, err, "the release after the failed bounded attempt fails its finalizer patch") + gone, _ := env.reconcileDeleting(ctx, t, got) + require.Nil(t, gone) + require.Empty(t, env.fake.mcpServers(), "the retry deleted the config") + require.Equal(t, before, releasedWithoutDelete(t, "CoderMCPServer"), "a release that deleted in Coder does not count") + }) +} + // #382 (#353): a control plane that is being deleted waits for a // CoderMCPServer that is being deleted, so Coder keeps running. Under that // hold the object takes the normal delete path: a failed DELETE keeps the From fb7b34865a09a21f91800eb8cf4073894458e777 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 30 Oct 2026 14:19:01 +0000 Subject: [PATCH 04/18] fix: count CoderMCPServer releases without a delete in coder_k8s_cleanup_released_without_delete_total (#353) Change-Id: Ia7fc446d421c2f95496d26d23065894dd96f6216 Signed-off-by: Thomas Kosiewski --- internal/controller/codermcpserver_controller.go | 7 ++++++- internal/controller/codermcpserver_delete.go | 1 + internal/controller/controlplane_hold.go | 4 ++-- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/internal/controller/codermcpserver_controller.go b/internal/controller/codermcpserver_controller.go index afee054b..df1e7ff7 100644 --- a/internal/controller/codermcpserver_controller.go +++ b/internal/controller/codermcpserver_controller.go @@ -175,13 +175,18 @@ func (r *CoderMCPServerReconciler) Reconcile(ctx context.Context, req ctrl.Reque if !controllerutil.ContainsFinalizer(server, coderv1alpha1.CoderMCPServerCleanupFinalizer) { return ctrl.Result{}, nil } + forgetReleasedWithoutDelete(server.UID) release, step, err := r.deleteStep(ctx, server) if err != nil { return ctrl.Result{}, err } if release { r.sentCredentials.Delete(req.NamespacedName) - return ctrl.Result{}, r.patchFinalizer(ctx, server, false) + if err := r.patchFinalizer(ctx, server, false); err != nil { + return ctrl.Result{}, err + } + releasedWithoutDeleteDone(server.UID) + return ctrl.Result{}, nil } applyMCPServerStep(server, step) return ctrl.Result{RequeueAfter: step.requeue}, r.writeStatus(ctx, server, before) diff --git a/internal/controller/codermcpserver_delete.go b/internal/controller/codermcpserver_delete.go index f1a3dae7..df3cd298 100644 --- a/internal/controller/codermcpserver_delete.go +++ b/internal/controller/codermcpserver_delete.go @@ -165,6 +165,7 @@ func deleteMCPServerInCoder(ctx context.Context, sdk *codersdk.Client, server *c // releaseWithoutDelete logs what can stay in Coder, because nothing can // delete it any more, and returns true (ControlPlaneGone). func (r *CoderMCPServerReconciler) releaseWithoutDelete(ctx context.Context, server *coderv1alpha1.CoderMCPServer, why string) bool { + countReleasedWithoutDelete(server.UID, "CoderMCPServer") log.FromContext(ctx).Info("Releasing the finalizer without a delete in Coder: the MCP server config can stay in the Coder database", "reason", "ControlPlaneGone", "why", why, "slug", server.Spec.Slug, "configID", server.Status.ConfigID, "createPending", server.Status.CreateAttemptTime != nil) diff --git a/internal/controller/controlplane_hold.go b/internal/controller/controlplane_hold.go index c2a93511..7b0f2cb2 100644 --- a/internal/controller/controlplane_hold.go +++ b/internal/controller/controlplane_hold.go @@ -53,8 +53,8 @@ const controlPlaneLiveDependentWait = 30 * time.Second // 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 +// releasedWithoutDelete counts finalizers that went while their keys, +// providers or MCP server configs 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 From 27eea7d5c91a7d5c2e43608ce655d0701eb35224 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 30 Oct 2026 14:20:28 +0000 Subject: [PATCH 05/18] test: pin the per-kind dependent count of a held control plane for each kind (#353) Change-Id: I0efae04d44d60e32c821b656636f24365e2703bf Signed-off-by: Thomas Kosiewski --- .../controlplane_hold_internal_test.go | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/internal/controller/controlplane_hold_internal_test.go b/internal/controller/controlplane_hold_internal_test.go index 495bf6b6..47b565b4 100644 --- a/internal/controller/controlplane_hold_internal_test.go +++ b/internal/controller/controlplane_hold_internal_test.go @@ -2,13 +2,16 @@ package controller import ( "context" + "fmt" "testing" "time" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/reconcile" coderv1alpha1 "github.com/coder/coder-k8s/api/v1alpha1" @@ -24,6 +27,45 @@ func TestControlPlaneDependentGraceFor(t *testing.T) { require.Equal(t, 5*time.Minute, controlPlaneDependentGraceFor(1000), "the grace period has a cap") } +// #384 follow-up 5: the grace period counts the dependents of the largest +// kind, whichever kind that is. A dependent of another control plane does not +// count. +func TestControlPlaneDependentsPerKind(t *testing.T) { + t.Parallel() + scheme := runtime.NewScheme() + require.NoError(t, coderv1alpha1.AddToScheme(scheme)) + deleted := metav1.NewTime(time.Now()) + controlPlane := &coderv1alpha1.CoderControlPlane{ObjectMeta: metav1.ObjectMeta{Name: "coder", Namespace: "ns", DeletionTimestamp: &deleted}} + kinds := []func(name, controlPlane string) client.Object{ + func(name, controlPlane string) client.Object { + return &coderv1alpha1.CoderAIGatewayKey{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "ns"}, Spec: coderv1alpha1.CoderAIGatewayKeySpec{ControlPlaneRef: coderv1alpha1.CoderControlPlaneReference{Name: controlPlane}}} + }, + func(name, controlPlane string) client.Object { + return &coderv1alpha1.CoderAIProvider{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "ns"}, Spec: coderv1alpha1.CoderAIProviderSpec{ControlPlaneRef: coderv1alpha1.CoderControlPlaneReference{Name: controlPlane}}} + }, + func(name, controlPlane string) client.Object { + return &coderv1alpha1.CoderMCPServer{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "ns"}, Spec: coderv1alpha1.CoderMCPServerSpec{ControlPlaneRef: coderv1alpha1.CoderControlPlaneReference{Name: controlPlane}}} + }, + } + for name, counts := range map[string][3]int{"keys": {3, 1, 2}, "providers": {1, 3, 2}, "MCP servers": {1, 2, 3}} { + t.Run(name+" largest", func(t *testing.T) { + t.Parallel() + var objects []client.Object + for kind, count := range counts { + for i := range count { + objects = append(objects, kinds[kind](fmt.Sprintf("d%d", i), "coder")) + } + objects = append(objects, kinds[kind]("other", "another-control-plane")) + } + reader := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objects...).Build() + dependents, perKind, err := controlPlaneDependents(context.Background(), reader, controlPlane, deleted.Time) + require.NoError(t, err) + require.Len(t, dependents, 6) + require.Equal(t, 3, perKind) + }) + } +} + // #353, #382: the removal of each dependent kind requeues the control plane // that it references, so a waiting control plane goes at once. func TestRequestForDependentControlPlane(t *testing.T) { From c481e35f8c1fb4e7e9a13678e48558d40ce76900 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 30 Oct 2026 14:21:24 +0000 Subject: [PATCH 06/18] test: a key record takes the create time that the controller stamps after a status edit (#353) Change-Id: Ia86a2e47096359ec5248b654b3da7e6afaf90eea Signed-off-by: Thomas Kosiewski --- .../coderaigatewaykey_record_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/internal/controller/coderaigatewaykey_record_test.go b/internal/controller/coderaigatewaykey_record_test.go index fad0c9fc..dcdcdfc3 100644 --- a/internal/controller/coderaigatewaykey_record_test.go +++ b/internal/controller/coderaigatewaykey_record_test.go @@ -227,3 +227,21 @@ func TestCoderAIGatewayKeyRecordBackfill(t *testing.T) { requireAIGatewayKeyReady(t, got) require.Equal(t, want, env.aiGatewayKeyRecord(ctx, t, got.UID)) } + +// #353 (#362 follow-up 9): when someone clears status.lastCreateAttemptTime +// by hand, the controller counts the settle window from now (fail closed), +// and the record gets that time too, so a sweep does not count from the +// older one. +func TestCoderAIGatewayKeyRecordAfterClearedCreateTime(t *testing.T) { + t.Parallel() + ctx := context.Background() + env := newAIGatewayKeyEnv(ctx, t, ptrTo(true)) + got, _ := env.reconcile(ctx, t, env.createKey(ctx, t, "coder", testAIGatewaySecret)) + requireAIGatewayKeyReady(t, got) + env.clock.Step(time.Hour) + got.Status.LastCreateAttemptTime = nil + require.NoError(t, k8sClient.Status().Update(ctx, got)) + got, _ = env.reconcile(ctx, t, got) + require.Equal(t, env.clock.Now().UTC(), got.Status.LastCreateAttemptTime.UTC()) + require.Equal(t, env.clock.Now().UTC().Format(time.RFC3339), env.aiGatewayKeyRecord(ctx, t, got.UID)["createAttemptTime"]) +} From 320e5eb6cc56eefe3bbd13095146f0b35c3a6831 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 30 Oct 2026 14:22:24 +0000 Subject: [PATCH 07/18] fix: rewrite a key cleanup record whose create time is older than status.lastCreateAttemptTime (#353) Change-Id: If34b70a7c627e0a500f6168129502ac060e3cf98 Signed-off-by: Thomas Kosiewski --- internal/controller/coderaigatewaykey_record.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/internal/controller/coderaigatewaykey_record.go b/internal/controller/coderaigatewaykey_record.go index 9a010b1d..27c5811a 100644 --- a/internal/controller/coderaigatewaykey_record.go +++ b/internal/controller/coderaigatewaykey_record.go @@ -74,12 +74,14 @@ func (r *CoderAIGatewayKeyReconciler) recordIssuedKeyID(ctx context.Context, key } } -// recordAIGatewayKeyID writes the ID of a current key that Coder lists into -// the record. It never stops the reconcile: the record already holds the -// prefix, which finds the key too, so a failure is logged and retried. +// recordAIGatewayKeyID writes the ID of a current key that Coder lists, and a +// create time that the observe step stamped later (#353), into the record. It +// never stops the reconcile: the record already holds the prefix, which +// finds the key too, so a failure is logged and retried. func (r *CoderAIGatewayKeyReconciler) recordAIGatewayKeyID(ctx context.Context, sdk *codersdk.Client, key *coderv1alpha1.CoderAIGatewayKey, prefix string) { record, _, err := readCleanupRecord(ctx, r.APIReader, r.OperatorNamespace, key.UID) - if err == nil && record != nil && record.CoderID == key.Status.CurrentKey.ID { + if err == nil && record != nil && record.CoderID == key.Status.CurrentKey.ID && + (key.Status.LastCreateAttemptTime == nil || !record.CreateAttemptTime.Before(key.Status.LastCreateAttemptTime.Time)) { return } if err == nil { From d03351911e02e9797c4c8ae50338d69649319ef3 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 30 Oct 2026 14:23:58 +0000 Subject: [PATCH 08/18] test: a pending provider create keeps its record after a deployment switch (#353) Change-Id: Ibde71ab3c6d21f32d016f7c963aae62cf2e7cde5 Signed-off-by: Thomas Kosiewski --- .../controller/coderaiprovider_record_test.go | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/internal/controller/coderaiprovider_record_test.go b/internal/controller/coderaiprovider_record_test.go index b30c3a76..0a21b382 100644 --- a/internal/controller/coderaiprovider_record_test.go +++ b/internal/controller/coderaiprovider_record_test.go @@ -288,3 +288,35 @@ func TestCoderAIProviderDeploymentSwitchKeepsRecord(t *testing.T) { require.Zero(t, env.fake.requestCount(routeDeleteAIProvider), "no delete in the new deployment") require.Equal(t, before, cleanupRecord(ctx, t, env.namespace, provider.UID), "the record stays for a sweep") } + +// #353 (#362 follow-up 7): a create whose answer was lost leaves a record +// without a provider ID. When the control plane then reaches another Coder +// deployment, where someone else's provider has the same name, that name +// conflict proves nothing about the old deployment. The controller keeps the +// marker and the record, so a delete keeps the record for a sweep. +func TestCoderAIProviderPendingCreateAfterDeploymentSwitch(t *testing.T) { + t.Parallel() + ctx := context.Background() + env, _, provider := newAIProviderCreateEnv(ctx, t) + env.fake.failNext(routeCreateAIProvider, fakeFault{AfterCommit: true, Reset: true}) + got, _ := env.reconcile(ctx, t, provider) + requireAIProviderStep(t, got, false, "ConfirmingCreate") + before := cleanupRecord(ctx, t, env.namespace, provider.UID) + require.Empty(t, before["coderID"], "the create answer was lost") + + env.fake.mu.Lock() + env.fake.deploymentID = uuid.NewString() // A new database behind the same control plane. + for _, p := range env.fake.aiProviders { + p.displayName = "Someone else's provider" + } + env.fake.mu.Unlock() + got, _ = env.reconcile(ctx, t, got) + require.Contains(t, requireAIProviderStep(t, got, true, "CoderDeploymentChanged"), before["deploymentID"]) + require.NotNil(t, got.Status.CreateAttemptTime, "the marker stays") + require.Equal(t, before, cleanupRecord(ctx, t, env.namespace, provider.UID)) + + gone, _ := env.deleteAIProvider(ctx, t, got) + require.Nil(t, gone, "the finalizer goes without a delete") + require.Zero(t, env.fake.requestCount(routeDeleteAIProvider), "no delete in the new deployment") + require.Equal(t, before, cleanupRecord(ctx, t, env.namespace, provider.UID), "the record stays for a sweep") +} From 38056f2bc83ff5d688c46e7f86d9ecd8eda749ac Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 30 Oct 2026 14:25:09 +0000 Subject: [PATCH 09/18] fix: check the deployment of a provider record without an ID before a pending create is confirmed (#353) Change-Id: I07403a11500125126e518ada28479b362443abcf Signed-off-by: Thomas Kosiewski --- internal/controller/coderaiprovider_record.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/controller/coderaiprovider_record.go b/internal/controller/coderaiprovider_record.go index cf4a9603..40b0a116 100644 --- a/internal/controller/coderaiprovider_record.go +++ b/internal/controller/coderaiprovider_record.go @@ -93,7 +93,10 @@ func (r *CoderAIProviderReconciler) ensureAIProviderRecord( if err != nil { return nil, err } - if record != nil && record.CoderID == provider.Status.ProviderID { + // A record without an ID names the deployment of a pending create, so + // the write checks the deployment ID before the create is confirmed in + // another deployment (#353). + if record != nil && record.CoderID != "" && record.CoderID == provider.Status.ProviderID { return nil, nil } return r.writeAIProviderRecord(ctx, sdk, provider, nil) From 6b91172ca4753b61becdea18c6479b328144333d Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 30 Oct 2026 14:28:54 +0000 Subject: [PATCH 10/18] test: a control plane without operator access or without its Coder Deployment does not hold (#353) Change-Id: Iada71ed3a9231a388eb8737ff927936f9d01bfc5 Signed-off-by: Thomas Kosiewski --- .../coderaigatewaykey_delete_internal_test.go | 2 +- .../coderaiprovider_delete_internal_test.go | 2 +- .../codermcpserver_delete_internal_test.go | 2 +- .../controlplane_hold_internal_test.go | 7 ++ internal/controller/controlplane_hold_test.go | 88 ++++++++++++++++--- 5 files changed, 86 insertions(+), 15 deletions(-) diff --git a/internal/controller/coderaigatewaykey_delete_internal_test.go b/internal/controller/coderaigatewaykey_delete_internal_test.go index d27432b9..61569629 100644 --- a/internal/controller/coderaigatewaykey_delete_internal_test.go +++ b/internal/controller/coderaigatewaykey_delete_internal_test.go @@ -44,7 +44,7 @@ func TestAIGatewayKeyDeleteUnderHoldEndsAtHoldEnd(t *testing.T) { Data: map[string][]byte{coderv1alpha1.DefaultTokenSecretKey: []byte("token")}, } namespace := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "ns"}} - c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(controlPlane, token, namespace).Build() + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(controlPlane, token, namespace, coderDeployment(controlPlane)).Build() r := &CoderAIGatewayKeyReconciler{Client: c, APIReader: c, Scheme: scheme, Clock: clock.RealClock{}, OperatorNamespace: "coder-system"} held, err := controlPlaneHolds(context.Background(), r.Client, controlPlane, time.Now()) require.NoError(t, err) diff --git a/internal/controller/coderaiprovider_delete_internal_test.go b/internal/controller/coderaiprovider_delete_internal_test.go index 9d292436..9ca8bab0 100644 --- a/internal/controller/coderaiprovider_delete_internal_test.go +++ b/internal/controller/coderaiprovider_delete_internal_test.go @@ -47,7 +47,7 @@ func TestAIProviderDeleteUnderHoldEndsAtHoldEnd(t *testing.T) { Data: map[string][]byte{coderv1alpha1.DefaultTokenSecretKey: []byte("token")}, } namespace := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "ns"}} - c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(controlPlane, token, namespace).Build() + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(controlPlane, token, namespace, coderDeployment(controlPlane)).Build() r := &CoderAIProviderReconciler{Client: c, APIReader: c, Scheme: scheme, OperatorNamespace: "coder-system"} held, err := controlPlaneHolds(context.Background(), r.Client, controlPlane, time.Now()) require.NoError(t, err) diff --git a/internal/controller/codermcpserver_delete_internal_test.go b/internal/controller/codermcpserver_delete_internal_test.go index 31e0672c..9092d4c7 100644 --- a/internal/controller/codermcpserver_delete_internal_test.go +++ b/internal/controller/codermcpserver_delete_internal_test.go @@ -44,7 +44,7 @@ func TestMCPServerDeleteUnderHoldEndsAtHoldEnd(t *testing.T) { Data: map[string][]byte{coderv1alpha1.DefaultTokenSecretKey: []byte("token")}, } namespace := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "ns"}} - r := &CoderMCPServerReconciler{Client: fake.NewClientBuilder().WithScheme(scheme).WithObjects(controlPlane, token, namespace).Build(), Scheme: scheme} + r := &CoderMCPServerReconciler{Client: fake.NewClientBuilder().WithScheme(scheme).WithObjects(controlPlane, token, namespace, coderDeployment(controlPlane)).Build(), Scheme: scheme} held, err := controlPlaneHolds(context.Background(), r.Client, controlPlane, time.Now()) require.NoError(t, err) require.True(t, held, "assertion failed: the test needs a control plane that holds") diff --git a/internal/controller/controlplane_hold_internal_test.go b/internal/controller/controlplane_hold_internal_test.go index 47b565b4..5f9bf485 100644 --- a/internal/controller/controlplane_hold_internal_test.go +++ b/internal/controller/controlplane_hold_internal_test.go @@ -7,6 +7,7 @@ import ( "time" "github.com/stretchr/testify/require" + appsv1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -17,6 +18,12 @@ import ( coderv1alpha1 "github.com/coder/coder-k8s/api/v1alpha1" ) +// coderDeployment is the Coder Deployment of controlPlane. A control plane +// holds only while it exists (#353). +func coderDeployment(controlPlane *coderv1alpha1.CoderControlPlane) *appsv1.Deployment { + return &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: controlPlane.Name, Namespace: controlPlane.Namespace}} +} + // #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. diff --git a/internal/controller/controlplane_hold_test.go b/internal/controller/controlplane_hold_test.go index 2ac02db0..0682d368 100644 --- a/internal/controller/controlplane_hold_test.go +++ b/internal/controller/controlplane_hold_test.go @@ -7,6 +7,7 @@ import ( "time" "github.com/stretchr/testify/require" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -25,11 +26,23 @@ import ( 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. +// deleteFinalizedControlPlane gives the control plane its own finalizer and +// its Coder Deployment, 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} + labels := map[string]string{"app": name} + require.NoError(t, k8sClient.Create(ctx, &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace}, + Spec: appsv1.DeploymentSpec{ + Selector: &metav1.LabelSelector{MatchLabels: labels}, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{Labels: labels}, + Spec: corev1.PodSpec{Containers: []corev1.Container{{Name: "coder", Image: "coder:test"}}}, + }, + }, + })) controlPlane := &coderv1alpha1.CoderControlPlane{} require.NoError(t, k8sClient.Get(ctx, key, controlPlane)) controlPlane.Finalizers = append(controlPlane.Finalizers, workspaceRBACFinalizer) @@ -43,6 +56,17 @@ func deleteFinalizedControlPlane(ctx context.Context, t *testing.T, namespace, n }) } +// setOperatorToken writes token into the operator token Secret of the +// control plane coder. Without a token, Coder calls wait with +// OperatorAccessNotReady. +func setOperatorToken(ctx context.Context, t *testing.T, namespace, token string) { + t.Helper() + secret := &corev1.Secret{} + require.NoError(t, k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: "coder-operator-token"}, secret)) + secret.Data = map[string][]byte{coderv1alpha1.DefaultTokenSecretKey: []byte(token)} + require.NoError(t, k8sClient.Update(ctx, secret)) +} + func releasedWithoutDelete(t *testing.T, kind string) float64 { t.Helper() families, err := metrics.Registry.Gather() @@ -116,8 +140,8 @@ func TestControlPlaneWaitsForDependents(t *testing.T) { // #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. +// one bounded pass. An operator token that cannot be read makes it wait and +// retry (item q), not release. func TestCoderAIGatewayKeyDeleteUnderControlPlaneHold(t *testing.T) { t.Parallel() ctx := context.Background() @@ -131,18 +155,14 @@ func TestCoderAIGatewayKeyDeleteUnderControlPlaneHold(t *testing.T) { 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)) + setOperatorToken(ctx, t, env.namespace, "") 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)) + setOperatorToken(ctx, t, env.namespace, "operator-session-token") gone, _ := env.reconcile(ctx, t, got) require.Nil(t, gone) require.Empty(t, env.fake.aiGatewayKeys()) @@ -224,6 +244,45 @@ func TestControlPlaneForegroundDeletionDoesNotHold(t *testing.T) { require.NotContains(t, controlPlane.Finalizers, workspaceRBACFinalizer, "no hold under foreground deletion") } +// #353 (#362 follow-ups 1 and 4): the hold keeps Coder running only for +// dependents that can reach it. Nothing repairs the operator access of a +// control plane that is being deleted, and a foreground deletion that the +// operator saw only after its marker went has removed the Coder Deployment. +// In both cases a deleted dependent and the control plane go at once. +func TestControlPlaneHoldNeedsCoder(t *testing.T) { + t.Parallel() + for name, breakCoder := range map[string]func(ctx context.Context, t *testing.T, namespace string){ + "no operator access": func(ctx context.Context, t *testing.T, namespace string) { + controlPlane := &coderv1alpha1.CoderControlPlane{} + require.NoError(t, k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: "coder"}, controlPlane)) + controlPlane.Status.OperatorAccessReady = false + require.NoError(t, k8sClient.Status().Update(ctx, controlPlane)) + }, + "no Coder Deployment": func(ctx context.Context, t *testing.T, namespace string) { + require.NoError(t, k8sClient.Delete(ctx, &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: "coder"}})) + }, + } { + t.Run(name, func(t *testing.T) { + t.Parallel() + ctx := context.Background() + env := newAIGatewayKeyEnv(ctx, t, ptrTo(true)) + got := rotatedKey(ctx, t, &env) + env.createNamedKey(ctx, t, "live", "coder", "live-secret") + deleteFinalizedControlPlane(ctx, t, env.namespace, "coder") + breakCoder(ctx, t, env.namespace) + require.NoError(t, k8sClient.Delete(ctx, got)) + gone, _ := env.reconcile(ctx, t, got) + require.Nil(t, gone, "the deleted key does not wait for a hold") + + r := &controller.CoderControlPlaneReconciler{Client: k8sClient, Scheme: scheme} + ref := client.ObjectKey{Namespace: env.namespace, Name: "coder"} + _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: ref}) + require.NoError(t, err) + require.True(t, apierrors.IsNotFound(k8sClient.Get(ctx, ref, &coderv1alpha1.CoderControlPlane{})), "the control plane does not wait for the live key") + }) + } +} + // #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. @@ -237,8 +296,7 @@ func TestControlPlaneHoldEnd(t *testing.T) { 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)) + setOperatorToken(ctx, t, env.namespace, "") require.NoError(t, k8sClient.Delete(ctx, got)) env.clock.SetTime(end.Add(-30 * time.Second)) got, result := env.reconcile(ctx, t, got) @@ -380,6 +438,12 @@ func TestControlPlaneHoldRevokesWorkspaceRBAC(t *testing.T) { _ = k8sClient.Update(context.Background(), key) } }) + // The control plane holds only with operator access, which this + // reconciler has no provisioner for. + require.NoError(t, k8sClient.Get(ctx, ref, cp)) + cp.Status.OperatorAccessReady = true + cp.Status.OperatorTokenSecretRef = &coderv1alpha1.SecretKeySelector{Name: "coder-operator-token"} + require.NoError(t, k8sClient.Status().Update(ctx, cp)) require.NoError(t, k8sClient.Delete(ctx, key)) require.NoError(t, k8sClient.Delete(ctx, cp)) result, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: ref}) From 2184060fdb501b6d04e1be3d8bf346a37b03a32e Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 30 Oct 2026 14:30:40 +0000 Subject: [PATCH 11/18] fix: hold a deleted control plane only while it has operator access and its Coder Deployment (#353) Change-Id: I96e990e46152796b2fee072da36c588a45ae53cf Signed-off-by: Thomas Kosiewski --- internal/controller/controlplane_hold.go | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/internal/controller/controlplane_hold.go b/internal/controller/controlplane_hold.go index 7b0f2cb2..b329361c 100644 --- a/internal/controller/controlplane_hold.go +++ b/internal/controller/controlplane_hold.go @@ -8,7 +8,9 @@ import ( "time" "github.com/prometheus/client_golang/prometheus" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -105,10 +107,13 @@ func controlPlaneHoldRequeue(controlPlane *coderv1alpha1.CoderControlPlane, now // 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 +// has not passed, it is not deleted in the foreground, it has operator +// access, its namespace is not being deleted, and its Coder Deployment +// exists. 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. +// Nothing repairs the operator access of a control plane that is being +// deleted, so without it no dependent can call Coder (#353). 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") @@ -117,14 +122,25 @@ func controlPlaneHolds(ctx context.Context, reader client.Reader, controlPlane * 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)) { + !now.Before(controlPlaneHoldEnd(controlPlane)) || !controlPlane.Status.OperatorAccessReady || controlPlane.Status.OperatorTokenSecretRef == nil { 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 + if !namespace.DeletionTimestamp.IsZero() || namespace.Status.Phase == corev1.NamespaceTerminating { + return false, nil + } + // The operator can see a foreground deletion only after its marker is + // gone, and then only the missing Deployment shows it. + deployment := types.NamespacedName{Namespace: controlPlane.Namespace, Name: controlPlane.Name} + if err := reader.Get(ctx, deployment, &appsv1.Deployment{}); apierrors.IsNotFound(err) { + return false, nil + } else if err != nil { + return false, fmt.Errorf("get deployment %s: %w", deployment, err) + } + return true, nil } // controlPlaneDependents returns the CoderAIGatewayKey, CoderAIProvider and From bbfe848fb7533c86a0029fbee1afd8abbbc776ef Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 30 Oct 2026 14:31:44 +0000 Subject: [PATCH 12/18] docs: name MCP servers in the control plane hold, and list the new cases without a hold (#353) Change-Id: I2a732a6a1531c2c558e4f5f4ac89c43adc09b553 Signed-off-by: Thomas Kosiewski --- docs/how-to/manage-ai-gateway-keys.md | 4 ++-- docs/how-to/manage-ai-providers.md | 4 ++-- docs/how-to/manage-mcp-servers.md | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/how-to/manage-ai-gateway-keys.md b/docs/how-to/manage-ai-gateway-keys.md index 2871b84f..eb647374 100644 --- a/docs/how-to/manage-ai-gateway-keys.md +++ b/docs/how-to/manage-ai-gateway-keys.md @@ -139,9 +139,9 @@ 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, and the [cleanup record](#cleanup-records) stays. 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`. -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. +When you delete the control plane while `CoderAIGatewayKey`, `CoderAIProvider` or `CoderMCPServer` 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, providers and MCP server configs 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. The control plane also does not wait when it has no operator access or its Coder Deployment is gone, because then no object can reach Coder. -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. +When the control plane does not wait (its namespace is being deleted, it is deleted in the foreground, its Coder Deployment is gone, 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. diff --git a/docs/how-to/manage-ai-providers.md b/docs/how-to/manage-ai-providers.md index 9271550e..158c751e 100644 --- a/docs/how-to/manage-ai-providers.md +++ b/docs/how-to/manage-ai-providers.md @@ -122,8 +122,8 @@ 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, the operator cannot delete the provider now. The finalizer goes at once, and the operator logs the provider ID. The [cleanup record](#cleanup-records) stays, so a later sweep can delete the provider. -- 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. -- 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. +- When you delete the control plane while `CoderAIProvider`, `CoderAIGatewayKey` or `CoderMCPServer` 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, and neither does a control plane without operator access or without its Coder Deployment. +- When the control plane does not wait (its namespace is being deleted, it is deleted in the foreground, its Coder Deployment is gone, 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 namespace: a namespace deletion removes Coder's pods at once, so the control plane cannot wait for them. A sweep deletes the leftover provider only when the namespace comes back with a control plane of the same Coder deployment. Coder deletes a provider even while chat models use it. Those models stop working. diff --git a/docs/how-to/manage-mcp-servers.md b/docs/how-to/manage-mcp-servers.md index 2888ac79..5564a002 100644 --- a/docs/how-to/manage-mcp-servers.md +++ b/docs/how-to/manage-mcp-servers.md @@ -207,8 +207,8 @@ Deleting the object deletes the config in Coder. Coder removes the server from e - The operator deletes only a config that this object created. With only an unconfirmed create, it reads Coder first. It deletes a config that carries this object's temporary display name, and it never touches another config with the slug. A create can still commit for a short time, so within 2 minutes of the create request the operator reads again before it trusts that the config is absent. - When Coder is unreachable, or the control plane has no operator access, the finalizer stays and the operator retries every 60 seconds. The operator trusts only Coder's own 404 as proof that the config is gone. A 404 from a proxy keeps the finalizer. - When the control plane is gone, nothing can delete the config. The finalizer goes at once. The operator logs the slug and the config ID with the reason `ControlPlaneGone`. Delete the config in the Coder dashboard. -- When you delete the control plane while `CoderMCPServer` objects that use it are being deleted, the control plane waits for them for up to 10 minutes, plus a grace period for a last bounded attempt. It keeps its finalizer, so Coder keeps running, and the delete above runs with its retries. 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), the operator sends one delete with a 10-second limit, then removes the finalizer, also when that delete fails. So a namespace deletion finishes, but it can leave the config in Coder. +- When you delete the control plane while `CoderMCPServer` objects that use it are being deleted, the control plane waits for them for up to 10 minutes, plus a grace period for a last bounded attempt. It keeps its finalizer, so Coder keeps running, and the delete above runs with its retries. 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, and neither does a control plane without operator access or without its Coder Deployment. +- When the control plane does not wait (its namespace is being deleted, it is deleted in the foreground, its Coder Deployment is gone, 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, also when that delete fails. Without working access, the finalizer goes without a delete. So a namespace deletion finishes, but it can leave the config in Coder. The metric `coder_k8s_cleanup_released_without_delete_total` (label `kind`) counts each release without a delete. - A withdrawn opt-in never blocks a delete. Delete `CoderMCPServer` objects before their control plane and before their namespace. From b92c573049d9f552e54fc6f88532fb741218bd08 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 30 Oct 2026 14:42:13 +0000 Subject: [PATCH 13/18] docs: scope the operator access retry to a control plane that is not being deleted (#353) Change-Id: I122bdcbb2d1529376d035e1bd35283ec99d275d1 Signed-off-by: Thomas Kosiewski --- docs/how-to/manage-ai-providers.md | 2 +- docs/how-to/manage-mcp-servers.md | 2 +- internal/controller/controlplane_hold.go | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/how-to/manage-ai-providers.md b/docs/how-to/manage-ai-providers.md index 158c751e..32273b9d 100644 --- a/docs/how-to/manage-ai-providers.md +++ b/docs/how-to/manage-ai-providers.md @@ -120,7 +120,7 @@ These limits remain: Deleting the object deletes the provider in Coder. With only an unconfirmed create, the operator reads Coder first and deletes only a provider that this object created. A create request can still commit for a short time, so within 2 minutes of that request the operator reads again before it trusts that the provider is absent. -- 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 Coder is unreachable, or a control plane that is not being deleted 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, the operator cannot delete the provider now. The finalizer goes at once, and the operator logs the provider ID. The [cleanup record](#cleanup-records) stays, so a later sweep can delete the provider. - When you delete the control plane while `CoderAIProvider`, `CoderAIGatewayKey` or `CoderMCPServer` 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, and neither does a control plane without operator access or without its Coder Deployment. - When the control plane does not wait (its namespace is being deleted, it is deleted in the foreground, its Coder Deployment is gone, 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. diff --git a/docs/how-to/manage-mcp-servers.md b/docs/how-to/manage-mcp-servers.md index 5564a002..0a44cf90 100644 --- a/docs/how-to/manage-mcp-servers.md +++ b/docs/how-to/manage-mcp-servers.md @@ -205,7 +205,7 @@ Disabling stops every chat from using the server, also chats that selected it ea Deleting the object deletes the config in Coder. Coder removes the server from every chat and deletes the users' OAuth2 tokens for it. The finalizer `coder.com/mcp-server-cleanup` keeps the object until then. - The operator deletes only a config that this object created. With only an unconfirmed create, it reads Coder first. It deletes a config that carries this object's temporary display name, and it never touches another config with the slug. A create can still commit for a short time, so within 2 minutes of the create request the operator reads again before it trusts that the config is absent. -- When Coder is unreachable, or the control plane has no operator access, the finalizer stays and the operator retries every 60 seconds. The operator trusts only Coder's own 404 as proof that the config is gone. A 404 from a proxy keeps the finalizer. +- When Coder is unreachable, or a control plane that is not being deleted has no operator access, the finalizer stays and the operator retries every 60 seconds. The operator trusts only Coder's own 404 as proof that the config is gone. A 404 from a proxy keeps the finalizer. - When the control plane is gone, nothing can delete the config. The finalizer goes at once. The operator logs the slug and the config ID with the reason `ControlPlaneGone`. Delete the config in the Coder dashboard. - When you delete the control plane while `CoderMCPServer` objects that use it are being deleted, the control plane waits for them for up to 10 minutes, plus a grace period for a last bounded attempt. It keeps its finalizer, so Coder keeps running, and the delete above runs with its retries. 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, and neither does a control plane without operator access or without its Coder Deployment. - When the control plane does not wait (its namespace is being deleted, it is deleted in the foreground, its Coder Deployment is gone, 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, also when that delete fails. Without working access, the finalizer goes without a delete. So a namespace deletion finishes, but it can leave the config in Coder. The metric `coder_k8s_cleanup_released_without_delete_total` (label `kind`) counts each release without a delete. diff --git a/internal/controller/controlplane_hold.go b/internal/controller/controlplane_hold.go index b329361c..2dd57614 100644 --- a/internal/controller/controlplane_hold.go +++ b/internal/controller/controlplane_hold.go @@ -55,8 +55,8 @@ const controlPlaneLiveDependentWait = 30 * time.Second // its dependents. A dependent's removal also requeues it at once. const controlPlaneDependentPoll = 10 * time.Second -// releasedWithoutDelete counts finalizers that went while their keys, -// providers or MCP server configs can stay in Coder (#353), by kind. Today the release log line is +// releasedWithoutDelete counts finalizers that went while the object's +// resources 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 From 89b6930f8dc193ec35ebed39176d58c36ae9ebc2 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 30 Oct 2026 15:25:56 +0000 Subject: [PATCH 14/18] test: a control plane holds without its Coder Deployment, also when it reconciles first (#353) Change-Id: Ie1b0cd061d176ec43a0b93e34f99b925629d6460 Signed-off-by: Thomas Kosiewski --- internal/controller/controlplane_hold_test.go | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/internal/controller/controlplane_hold_test.go b/internal/controller/controlplane_hold_test.go index 0682d368..e31d8aae 100644 --- a/internal/controller/controlplane_hold_test.go +++ b/internal/controller/controlplane_hold_test.go @@ -283,6 +283,92 @@ func TestControlPlaneHoldNeedsCoder(t *testing.T) { } } +// orphanCoderDeployment deletes the Coder Deployment as `kubectl delete +// --cascade=orphan` does, which leaves Coder's pods running. Envtest has no +// garbage collector, so the test removes the orphan finalizer itself. +func orphanCoderDeployment(ctx context.Context, t *testing.T, namespace string) { + t.Helper() + deployment := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: "coder"}} + require.NoError(t, k8sClient.Delete(ctx, deployment, client.PropagationPolicy(metav1.DeletePropagationOrphan))) + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(deployment), deployment); err == nil { + deployment.Finalizers = nil + require.NoError(t, k8sClient.Update(ctx, deployment)) + } + require.True(t, apierrors.IsNotFound(k8sClient.Get(ctx, client.ObjectKeyFromObject(deployment), &appsv1.Deployment{}))) +} + +// #353: a missing Coder Deployment does not end the hold. Coder's pods can +// still run (an orphan delete), so the control plane keeps its finalizer, +// also when it reconciles before its deleted dependents, and they delete +// their resources in Coder with retries. +func TestControlPlaneHoldsWithoutCoderDeployment(t *testing.T) { + t.Parallel() + controlPlaneHolds := func(ctx context.Context, t *testing.T, namespace string) { + t.Helper() + r := &controller.CoderControlPlaneReconciler{Client: k8sClient, Scheme: scheme} + ref := client.ObjectKey{Namespace: namespace, Name: "coder"} + result, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: ref}) + require.NoError(t, err) + require.NoError(t, k8sClient.Get(ctx, ref, &coderv1alpha1.CoderControlPlane{}), "the control plane holds") + require.Equal(t, 10*time.Second, result.RequeueAfter) + } + t.Run("keys", func(t *testing.T) { + t.Parallel() + ctx := context.Background() + env := newAIGatewayKeyEnv(ctx, t, ptrTo(true)) + key := rotatedKey(ctx, t, &env) + deleteFinalizedControlPlane(ctx, t, env.namespace, "coder") + orphanCoderDeployment(ctx, t, env.namespace) + require.NoError(t, k8sClient.Delete(ctx, key)) + controlPlaneHolds(ctx, t, env.namespace) + env.clock.Step(5 * time.Minute) + gone, _ := env.reconcile(ctx, t, key) + require.Nil(t, gone) + require.Empty(t, env.fake.aiGatewayKeys()) + }) + t.Run("provider", func(t *testing.T) { + t.Parallel() + ctx := context.Background() + env, _, provider := newReadyAIProvider(ctx, t) + deleteFinalizedControlPlane(ctx, t, env.namespace, "coder") + orphanCoderDeployment(ctx, t, env.namespace) + require.NoError(t, k8sClient.Delete(ctx, provider)) + controlPlaneHolds(ctx, t, env.namespace) + gone, _ := env.reconcileDeleting(ctx, t, provider) + require.Nil(t, gone) + _, found := env.fake.aiProvider("openai") + require.False(t, found) + }) + t.Run("MCP server", func(t *testing.T) { + t.Parallel() + ctx := context.Background() + env := newMCPServerEnv(ctx, t, true) + env.keySecret(ctx, t, testMCPKey) + server, _ := env.applied(ctx, t, nil) + deleteFinalizedControlPlane(ctx, t, env.namespace, "coder") + orphanCoderDeployment(ctx, t, env.namespace) + require.NoError(t, k8sClient.Delete(ctx, server)) + controlPlaneHolds(ctx, t, env.namespace) + gone, _ := env.reconcileDeleting(ctx, t, server) + require.Nil(t, gone) + require.Empty(t, env.fake.mcpServers()) + }) + t.Run("provider retry", func(t *testing.T) { + t.Parallel() + ctx := context.Background() + env, _, provider := newReadyAIProvider(ctx, t) + deleteFinalizedControlPlane(ctx, t, env.namespace, "coder") + orphanCoderDeployment(ctx, t, env.namespace) + env.fake.failNext(routeDeleteAIProvider, fakeFault{Status: http.StatusBadGateway}) + got, _ := env.deleteAIProvider(ctx, t, provider) + require.NotNil(t, got, "a failed delete keeps the finalizer while the control plane holds") + got, _ = env.reconcileDeleting(ctx, t, got) + require.Nil(t, got) + _, found := env.fake.aiProvider("openai") + require.False(t, found) + }) +} + // #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. From 910867b5195891f1fafc534c1e4f7d90319865d1 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 30 Oct 2026 15:27:32 +0000 Subject: [PATCH 15/18] fix: keep the control plane hold when its Coder Deployment is missing (#353) An orphan delete or a cache miss shows no Deployment while Coder's pods still run. Ending the hold then removed the control plane before its dependents deleted their resources in Coder. Change-Id: I967bbab03cbfafddc46a302fd11a4ed7ce830720 Signed-off-by: Thomas Kosiewski --- docs/how-to/manage-ai-gateway-keys.md | 4 +- docs/how-to/manage-ai-providers.md | 4 +- docs/how-to/manage-mcp-servers.md | 4 +- .../coderaigatewaykey_delete_internal_test.go | 2 +- .../coderaiprovider_delete_internal_test.go | 2 +- .../codermcpserver_delete_internal_test.go | 2 +- internal/controller/controlplane_hold.go | 28 +++------- .../controlplane_hold_internal_test.go | 7 --- internal/controller/controlplane_hold_test.go | 55 +++++++------------ 9 files changed, 38 insertions(+), 70 deletions(-) diff --git a/docs/how-to/manage-ai-gateway-keys.md b/docs/how-to/manage-ai-gateway-keys.md index eb647374..5abf6130 100644 --- a/docs/how-to/manage-ai-gateway-keys.md +++ b/docs/how-to/manage-ai-gateway-keys.md @@ -139,9 +139,9 @@ 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, and the [cleanup record](#cleanup-records) stays. 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`. -When you delete the control plane while `CoderAIGatewayKey`, `CoderAIProvider` or `CoderMCPServer` 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, providers and MCP server configs 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. The control plane also does not wait when it has no operator access or its Coder Deployment is gone, because then no object can reach Coder. +When you delete the control plane while `CoderAIGatewayKey`, `CoderAIProvider` or `CoderMCPServer` 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, providers and MCP server configs 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. The control plane also does not wait when it has no operator access, because then no object can call Coder. -When the control plane does not wait (its namespace is being deleted, it is deleted in the foreground, its Coder Deployment is gone, 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. +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. diff --git a/docs/how-to/manage-ai-providers.md b/docs/how-to/manage-ai-providers.md index 56cdbd63..c88b1960 100644 --- a/docs/how-to/manage-ai-providers.md +++ b/docs/how-to/manage-ai-providers.md @@ -124,8 +124,8 @@ Deleting the object deletes the provider in Coder. With only an unconfirmed crea - When Coder is unreachable, or a control plane that is not being deleted 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, the operator cannot delete the provider now. The finalizer goes at once, and the operator logs the provider ID. The [cleanup record](#cleanup-records) stays, so a later sweep can delete the provider. -- When you delete the control plane while `CoderAIProvider`, `CoderAIGatewayKey` or `CoderMCPServer` 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, and neither does a control plane without operator access or without its Coder Deployment. -- When the control plane does not wait (its namespace is being deleted, it is deleted in the foreground, its Coder Deployment is gone, 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. +- When you delete the control plane while `CoderAIProvider`, `CoderAIGatewayKey` or `CoderMCPServer` 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, and neither does a control plane without operator access. +- 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 namespace: a namespace deletion removes Coder's pods at once, so the control plane cannot wait for them. A sweep deletes the leftover provider only when the namespace comes back with a control plane of the same Coder deployment. Coder deletes a provider even while chat models use it. Those models stop working. diff --git a/docs/how-to/manage-mcp-servers.md b/docs/how-to/manage-mcp-servers.md index 0a44cf90..11a72016 100644 --- a/docs/how-to/manage-mcp-servers.md +++ b/docs/how-to/manage-mcp-servers.md @@ -207,8 +207,8 @@ Deleting the object deletes the config in Coder. Coder removes the server from e - The operator deletes only a config that this object created. With only an unconfirmed create, it reads Coder first. It deletes a config that carries this object's temporary display name, and it never touches another config with the slug. A create can still commit for a short time, so within 2 minutes of the create request the operator reads again before it trusts that the config is absent. - When Coder is unreachable, or a control plane that is not being deleted has no operator access, the finalizer stays and the operator retries every 60 seconds. The operator trusts only Coder's own 404 as proof that the config is gone. A 404 from a proxy keeps the finalizer. - When the control plane is gone, nothing can delete the config. The finalizer goes at once. The operator logs the slug and the config ID with the reason `ControlPlaneGone`. Delete the config in the Coder dashboard. -- When you delete the control plane while `CoderMCPServer` objects that use it are being deleted, the control plane waits for them for up to 10 minutes, plus a grace period for a last bounded attempt. It keeps its finalizer, so Coder keeps running, and the delete above runs with its retries. 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, and neither does a control plane without operator access or without its Coder Deployment. -- When the control plane does not wait (its namespace is being deleted, it is deleted in the foreground, its Coder Deployment is gone, 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, also when that delete fails. Without working access, the finalizer goes without a delete. So a namespace deletion finishes, but it can leave the config in Coder. The metric `coder_k8s_cleanup_released_without_delete_total` (label `kind`) counts each release without a delete. +- When you delete the control plane while `CoderMCPServer` objects that use it are being deleted, the control plane waits for them for up to 10 minutes, plus a grace period for a last bounded attempt. It keeps its finalizer, so Coder keeps running, and the delete above runs with its retries. 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, and neither does a control plane without operator access. +- 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, also when that delete fails. Without working access, the finalizer goes without a delete. So a namespace deletion finishes, but it can leave the config in Coder. The metric `coder_k8s_cleanup_released_without_delete_total` (label `kind`) counts each release without a delete. - A withdrawn opt-in never blocks a delete. Delete `CoderMCPServer` objects before their control plane and before their namespace. diff --git a/internal/controller/coderaigatewaykey_delete_internal_test.go b/internal/controller/coderaigatewaykey_delete_internal_test.go index fe0dff99..b20a1888 100644 --- a/internal/controller/coderaigatewaykey_delete_internal_test.go +++ b/internal/controller/coderaigatewaykey_delete_internal_test.go @@ -47,7 +47,7 @@ func TestAIGatewayKeyDeleteUnderHoldEndsAtHoldEnd(t *testing.T) { Data: map[string][]byte{coderv1alpha1.DefaultTokenSecretKey: []byte("token")}, } namespace := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "ns"}} - c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(controlPlane, token, namespace, coderDeployment(controlPlane)).Build() + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(controlPlane, token, namespace).Build() r := &CoderAIGatewayKeyReconciler{Client: c, APIReader: c, Scheme: scheme, Clock: clock.RealClock{}, OperatorNamespace: "coder-system"} held, err := controlPlaneHolds(context.Background(), r.Client, controlPlane, time.Now()) require.NoError(t, err) diff --git a/internal/controller/coderaiprovider_delete_internal_test.go b/internal/controller/coderaiprovider_delete_internal_test.go index 9ca8bab0..9d292436 100644 --- a/internal/controller/coderaiprovider_delete_internal_test.go +++ b/internal/controller/coderaiprovider_delete_internal_test.go @@ -47,7 +47,7 @@ func TestAIProviderDeleteUnderHoldEndsAtHoldEnd(t *testing.T) { Data: map[string][]byte{coderv1alpha1.DefaultTokenSecretKey: []byte("token")}, } namespace := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "ns"}} - c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(controlPlane, token, namespace, coderDeployment(controlPlane)).Build() + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(controlPlane, token, namespace).Build() r := &CoderAIProviderReconciler{Client: c, APIReader: c, Scheme: scheme, OperatorNamespace: "coder-system"} held, err := controlPlaneHolds(context.Background(), r.Client, controlPlane, time.Now()) require.NoError(t, err) diff --git a/internal/controller/codermcpserver_delete_internal_test.go b/internal/controller/codermcpserver_delete_internal_test.go index bc7f76a7..1de7efc4 100644 --- a/internal/controller/codermcpserver_delete_internal_test.go +++ b/internal/controller/codermcpserver_delete_internal_test.go @@ -47,7 +47,7 @@ func TestMCPServerDeleteUnderHoldEndsAtHoldEnd(t *testing.T) { Data: map[string][]byte{coderv1alpha1.DefaultTokenSecretKey: []byte("token")}, } namespace := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "ns"}} - r := &CoderMCPServerReconciler{Client: fake.NewClientBuilder().WithScheme(scheme).WithObjects(controlPlane, token, namespace, coderDeployment(controlPlane)).Build(), Scheme: scheme} + r := &CoderMCPServerReconciler{Client: fake.NewClientBuilder().WithScheme(scheme).WithObjects(controlPlane, token, namespace).Build(), Scheme: scheme} held, err := controlPlaneHolds(context.Background(), r.Client, controlPlane, time.Now()) require.NoError(t, err) require.True(t, held, "assertion failed: the test needs a control plane that holds") diff --git a/internal/controller/controlplane_hold.go b/internal/controller/controlplane_hold.go index 2dd57614..9cd351f9 100644 --- a/internal/controller/controlplane_hold.go +++ b/internal/controller/controlplane_hold.go @@ -8,9 +8,7 @@ import ( "time" "github.com/prometheus/client_golang/prometheus" - appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -108,12 +106,13 @@ func controlPlaneHoldRequeue(controlPlane *coderv1alpha1.CoderControlPlane, now // 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, it has operator -// access, its namespace is not being deleted, and its Coder Deployment -// exists. 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. -// Nothing repairs the operator access of a control plane that is being -// deleted, so without it no dependent can call Coder (#353). +// access, 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. Nothing repairs the operator access of a control +// plane that is being deleted: its reconcile only finalizes, and a status +// write still in flight was computed for an older generation and is dropped. +// So without operator access no dependent can call Coder (#353). 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") @@ -129,18 +128,7 @@ func controlPlaneHolds(ctx context.Context, reader client.Reader, controlPlane * if err := reader.Get(ctx, types.NamespacedName{Name: controlPlane.Namespace}, namespace); err != nil { return false, fmt.Errorf("get namespace %s: %w", controlPlane.Namespace, err) } - if !namespace.DeletionTimestamp.IsZero() || namespace.Status.Phase == corev1.NamespaceTerminating { - return false, nil - } - // The operator can see a foreground deletion only after its marker is - // gone, and then only the missing Deployment shows it. - deployment := types.NamespacedName{Namespace: controlPlane.Namespace, Name: controlPlane.Name} - if err := reader.Get(ctx, deployment, &appsv1.Deployment{}); apierrors.IsNotFound(err) { - return false, nil - } else if err != nil { - return false, fmt.Errorf("get deployment %s: %w", deployment, err) - } - return true, nil + return namespace.DeletionTimestamp.IsZero() && namespace.Status.Phase != corev1.NamespaceTerminating, nil } // controlPlaneDependents returns the CoderAIGatewayKey, CoderAIProvider and diff --git a/internal/controller/controlplane_hold_internal_test.go b/internal/controller/controlplane_hold_internal_test.go index 5f9bf485..47b565b4 100644 --- a/internal/controller/controlplane_hold_internal_test.go +++ b/internal/controller/controlplane_hold_internal_test.go @@ -7,7 +7,6 @@ import ( "time" "github.com/stretchr/testify/require" - appsv1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" @@ -18,12 +17,6 @@ import ( coderv1alpha1 "github.com/coder/coder-k8s/api/v1alpha1" ) -// coderDeployment is the Coder Deployment of controlPlane. A control plane -// holds only while it exists (#353). -func coderDeployment(controlPlane *coderv1alpha1.CoderControlPlane) *appsv1.Deployment { - return &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Name: controlPlane.Name, Namespace: controlPlane.Namespace}} -} - // #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. diff --git a/internal/controller/controlplane_hold_test.go b/internal/controller/controlplane_hold_test.go index e31d8aae..887f37c3 100644 --- a/internal/controller/controlplane_hold_test.go +++ b/internal/controller/controlplane_hold_test.go @@ -244,43 +244,30 @@ func TestControlPlaneForegroundDeletionDoesNotHold(t *testing.T) { require.NotContains(t, controlPlane.Finalizers, workspaceRBACFinalizer, "no hold under foreground deletion") } -// #353 (#362 follow-ups 1 and 4): the hold keeps Coder running only for -// dependents that can reach it. Nothing repairs the operator access of a -// control plane that is being deleted, and a foreground deletion that the -// operator saw only after its marker went has removed the Coder Deployment. -// In both cases a deleted dependent and the control plane go at once. +// #353 (#362 follow-up 1): the hold keeps Coder running only for dependents +// that can reach it. Nothing repairs the operator access of a control plane +// that is being deleted, so without it a deleted dependent and the control +// plane go at once. func TestControlPlaneHoldNeedsCoder(t *testing.T) { t.Parallel() - for name, breakCoder := range map[string]func(ctx context.Context, t *testing.T, namespace string){ - "no operator access": func(ctx context.Context, t *testing.T, namespace string) { - controlPlane := &coderv1alpha1.CoderControlPlane{} - require.NoError(t, k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: "coder"}, controlPlane)) - controlPlane.Status.OperatorAccessReady = false - require.NoError(t, k8sClient.Status().Update(ctx, controlPlane)) - }, - "no Coder Deployment": func(ctx context.Context, t *testing.T, namespace string) { - require.NoError(t, k8sClient.Delete(ctx, &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{Namespace: namespace, Name: "coder"}})) - }, - } { - t.Run(name, func(t *testing.T) { - t.Parallel() - ctx := context.Background() - env := newAIGatewayKeyEnv(ctx, t, ptrTo(true)) - got := rotatedKey(ctx, t, &env) - env.createNamedKey(ctx, t, "live", "coder", "live-secret") - deleteFinalizedControlPlane(ctx, t, env.namespace, "coder") - breakCoder(ctx, t, env.namespace) - require.NoError(t, k8sClient.Delete(ctx, got)) - gone, _ := env.reconcile(ctx, t, got) - require.Nil(t, gone, "the deleted key does not wait for a hold") + ctx := context.Background() + env := newAIGatewayKeyEnv(ctx, t, ptrTo(true)) + got := rotatedKey(ctx, t, &env) + env.createNamedKey(ctx, t, "live", "coder", "live-secret") + deleteFinalizedControlPlane(ctx, t, env.namespace, "coder") + controlPlane := &coderv1alpha1.CoderControlPlane{} + ref := client.ObjectKey{Namespace: env.namespace, Name: "coder"} + require.NoError(t, k8sClient.Get(ctx, ref, controlPlane)) + controlPlane.Status.OperatorAccessReady = false + require.NoError(t, k8sClient.Status().Update(ctx, controlPlane)) + require.NoError(t, k8sClient.Delete(ctx, got)) + gone, _ := env.reconcile(ctx, t, got) + require.Nil(t, gone, "the deleted key does not wait for a hold") - r := &controller.CoderControlPlaneReconciler{Client: k8sClient, Scheme: scheme} - ref := client.ObjectKey{Namespace: env.namespace, Name: "coder"} - _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: ref}) - require.NoError(t, err) - require.True(t, apierrors.IsNotFound(k8sClient.Get(ctx, ref, &coderv1alpha1.CoderControlPlane{})), "the control plane does not wait for the live key") - }) - } + r := &controller.CoderControlPlaneReconciler{Client: k8sClient, Scheme: scheme} + _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: ref}) + require.NoError(t, err) + require.True(t, apierrors.IsNotFound(k8sClient.Get(ctx, ref, &coderv1alpha1.CoderControlPlane{})), "the control plane does not wait for the live key") } // orphanCoderDeployment deletes the Coder Deployment as `kubectl delete From 89ce952dbac4134a46ff5593464439a68d34c848 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 30 Oct 2026 15:30:03 +0000 Subject: [PATCH 16/18] test: a control plane that lost operator access before its deletion does not get it back (#353) Change-Id: Ibbbb21ce939ed0c2483a9161d3ee165759a40dd7 Signed-off-by: Thomas Kosiewski --- internal/controller/controlplane_hold_test.go | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/internal/controller/controlplane_hold_test.go b/internal/controller/controlplane_hold_test.go index 887f37c3..3f4770b8 100644 --- a/internal/controller/controlplane_hold_test.go +++ b/internal/controller/controlplane_hold_test.go @@ -2,6 +2,7 @@ package controller_test import ( "context" + "errors" "net/http" "testing" "time" @@ -21,6 +22,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/metrics" coderv1alpha1 "github.com/coder/coder-k8s/api/v1alpha1" + "github.com/coder/coder-k8s/internal/coderbootstrap" "github.com/coder/coder-k8s/internal/controller" ) @@ -270,6 +272,69 @@ func TestControlPlaneHoldNeedsCoder(t *testing.T) { require.True(t, apierrors.IsNotFound(k8sClient.Get(ctx, ref, &coderv1alpha1.CoderControlPlane{})), "the control plane does not wait for the live key") } +// deletingProvisioner deletes the control plane while it provisions the +// operator token, like a deletion that comes during a reconcile. +type deletingProvisioner struct { + *fakeOperatorAccessProvisioner + deleteControlPlane func() +} + +func (p deletingProvisioner) EnsureOperatorToken(ctx context.Context, req coderbootstrap.EnsureOperatorTokenRequest) (string, error) { + p.deleteControlPlane() + return p.fakeOperatorAccessProvisioner.EnsureOperatorToken(ctx, req) +} + +// #353 (#362 follow-up 1): TestControlPlaneHoldNeedsCoder is safe only +// because a control plane that lost its operator access before its deletion, +// here in a Postgres failure, does not get it back while it is being +// deleted. A reconcile that was in flight when the deletion came does not +// write it back, and a reconcile after the deletion does not provision it. +func TestControlPlaneOperatorAccessStaysLostWhileDeleting(t *testing.T) { + ensureGatewaySchemeRegistered(t) + t.Parallel() + ctx := context.Background() + namespace := createTestNamespace(ctx, t, "cp-access") + ref := client.ObjectKey{Namespace: namespace, Name: "coder"} + const keep = "coder.com/test-keep" + require.NoError(t, k8sClient.Create(ctx, &coderv1alpha1.CoderControlPlane{ + ObjectMeta: metav1.ObjectMeta{Name: ref.Name, Namespace: namespace, Finalizers: []string{keep}}, + Spec: coderv1alpha1.CoderControlPlaneSpec{ExtraEnv: []corev1.EnvVar{{Name: "CODER_PG_CONNECTION_URL", Value: "postgres://example/coder"}}}, + })) + t.Cleanup(func() { + controlPlane := &coderv1alpha1.CoderControlPlane{} + if k8sClient.Get(context.Background(), ref, controlPlane) == nil { + controlPlane.Finalizers = nil + _ = k8sClient.Update(context.Background(), controlPlane) + } + }) + provisioner := &fakeOperatorAccessProvisioner{token: "operator-token"} + reconcile := func(p coderbootstrap.OperatorAccessProvisioner) *coderv1alpha1.CoderControlPlane { + t.Helper() + r := &controller.CoderControlPlaneReconciler{Client: k8sClient, Scheme: scheme, OperatorAccessProvisioner: p} + _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: ref}) + require.NoError(t, err) + got := &coderv1alpha1.CoderControlPlane{} + require.NoError(t, k8sClient.Get(ctx, ref, got)) + return got + } + require.True(t, reconcile(provisioner).Status.OperatorAccessReady) + provisioner.err = errors.New("dial tcp: connection refused") + require.False(t, reconcile(provisioner).Status.OperatorAccessReady, "a Postgres failure clears operator access") + + provisioner.err = nil + got := reconcile(deletingProvisioner{provisioner, func() { + require.NoError(t, k8sClient.Delete(ctx, &coderv1alpha1.CoderControlPlane{ObjectMeta: metav1.ObjectMeta{Name: ref.Name, Namespace: namespace}})) + }}) + require.False(t, got.DeletionTimestamp.IsZero()) + require.False(t, got.Status.OperatorAccessReady, "the reconcile in flight does not write operator access back") + + calls := provisioner.calls + got = reconcile(provisioner) + require.Equal(t, []string{keep}, got.Finalizers, "the control plane finalized") + require.False(t, got.Status.OperatorAccessReady) + require.Equal(t, calls, provisioner.calls, "no operator token is provisioned for a control plane that is being deleted") +} + // orphanCoderDeployment deletes the Coder Deployment as `kubectl delete // --cascade=orphan` does, which leaves Coder's pods running. Envtest has no // garbage collector, so the test removes the orphan finalizer itself. From 383d0d42789d7e96bcf1786f25a1b980fc9f4713 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 30 Oct 2026 16:30:10 +0000 Subject: [PATCH 17/18] fix: keep the control plane hold without operator access (#353) A provider cleanup that read a valid operator token before a Postgres failure cleared operator access still calls Coder. Releasing the control plane at once lets its Service and Deployment go and loses that cleanup, so the hold applies as on main. TestControlPlaneHoldsForCleanupInFlight covers the case. Change-Id: I315344184b059e1067b3ffb0e7ddd0d261fc6d11 Signed-off-by: Thomas Kosiewski --- docs/how-to/manage-ai-providers.md | 4 +- docs/how-to/manage-mcp-servers.md | 4 +- internal/controller/controlplane_hold.go | 14 +- internal/controller/controlplane_hold_test.go | 153 ++++++------------ 4 files changed, 60 insertions(+), 115 deletions(-) diff --git a/docs/how-to/manage-ai-providers.md b/docs/how-to/manage-ai-providers.md index c88b1960..ae933d10 100644 --- a/docs/how-to/manage-ai-providers.md +++ b/docs/how-to/manage-ai-providers.md @@ -122,9 +122,9 @@ These limits remain: Deleting the object deletes the provider in Coder. With only an unconfirmed create, the operator reads Coder first and deletes only a provider that this object created. A create request can still commit for a short time, so within 2 minutes of that request the operator reads again before it trusts that the provider is absent. -- When Coder is unreachable, or a control plane that is not being deleted 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 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, the operator cannot delete the provider now. The finalizer goes at once, and the operator logs the provider ID. The [cleanup record](#cleanup-records) stays, so a later sweep can delete the provider. -- When you delete the control plane while `CoderAIProvider`, `CoderAIGatewayKey` or `CoderMCPServer` 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, and neither does a control plane without operator access. +- When you delete the control plane while `CoderAIProvider`, `CoderAIGatewayKey` or `CoderMCPServer` 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. - 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 namespace: a namespace deletion removes Coder's pods at once, so the control plane cannot wait for them. A sweep deletes the leftover provider only when the namespace comes back with a control plane of the same Coder deployment. Coder deletes a provider even while chat models use it. Those models stop working. diff --git a/docs/how-to/manage-mcp-servers.md b/docs/how-to/manage-mcp-servers.md index 11a72016..5487e3bb 100644 --- a/docs/how-to/manage-mcp-servers.md +++ b/docs/how-to/manage-mcp-servers.md @@ -205,9 +205,9 @@ Disabling stops every chat from using the server, also chats that selected it ea Deleting the object deletes the config in Coder. Coder removes the server from every chat and deletes the users' OAuth2 tokens for it. The finalizer `coder.com/mcp-server-cleanup` keeps the object until then. - The operator deletes only a config that this object created. With only an unconfirmed create, it reads Coder first. It deletes a config that carries this object's temporary display name, and it never touches another config with the slug. A create can still commit for a short time, so within 2 minutes of the create request the operator reads again before it trusts that the config is absent. -- When Coder is unreachable, or a control plane that is not being deleted has no operator access, the finalizer stays and the operator retries every 60 seconds. The operator trusts only Coder's own 404 as proof that the config is gone. A 404 from a proxy keeps the finalizer. +- When Coder is unreachable, or the control plane has no operator access, the finalizer stays and the operator retries every 60 seconds. The operator trusts only Coder's own 404 as proof that the config is gone. A 404 from a proxy keeps the finalizer. - When the control plane is gone, nothing can delete the config. The finalizer goes at once. The operator logs the slug and the config ID with the reason `ControlPlaneGone`. Delete the config in the Coder dashboard. -- When you delete the control plane while `CoderMCPServer` objects that use it are being deleted, the control plane waits for them for up to 10 minutes, plus a grace period for a last bounded attempt. It keeps its finalizer, so Coder keeps running, and the delete above runs with its retries. 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, and neither does a control plane without operator access. +- When you delete the control plane while `CoderMCPServer` objects that use it are being deleted, the control plane waits for them for up to 10 minutes, plus a grace period for a last bounded attempt. It keeps its finalizer, so Coder keeps running, and the delete above runs with its retries. 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, also when that delete fails. Without working access, the finalizer goes without a delete. So a namespace deletion finishes, but it can leave the config in Coder. The metric `coder_k8s_cleanup_released_without_delete_total` (label `kind`) counts each release without a delete. - A withdrawn opt-in never blocks a delete. diff --git a/internal/controller/controlplane_hold.go b/internal/controller/controlplane_hold.go index 9cd351f9..75f7aade 100644 --- a/internal/controller/controlplane_hold.go +++ b/internal/controller/controlplane_hold.go @@ -105,14 +105,10 @@ func controlPlaneHoldRequeue(controlPlane *coderv1alpha1.CoderControlPlane, now // 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, it has operator -// access, 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. Nothing repairs the operator access of a control -// plane that is being deleted: its reconcile only finalizes, and a status -// write still in flight was computed for an older generation and is dropped. -// So without operator access no dependent can call Coder (#353). +// 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") @@ -121,7 +117,7 @@ func controlPlaneHolds(ctx context.Context, reader client.Reader, controlPlane * 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)) || !controlPlane.Status.OperatorAccessReady || controlPlane.Status.OperatorTokenSecretRef == nil { + !now.Before(controlPlaneHoldEnd(controlPlane)) { return false, nil } namespace := &corev1.Namespace{} diff --git a/internal/controller/controlplane_hold_test.go b/internal/controller/controlplane_hold_test.go index 3f4770b8..1e2ecb5c 100644 --- a/internal/controller/controlplane_hold_test.go +++ b/internal/controller/controlplane_hold_test.go @@ -22,7 +22,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/metrics" coderv1alpha1 "github.com/coder/coder-k8s/api/v1alpha1" - "github.com/coder/coder-k8s/internal/coderbootstrap" "github.com/coder/coder-k8s/internal/controller" ) @@ -58,17 +57,6 @@ func deleteFinalizedControlPlane(ctx context.Context, t *testing.T, namespace, n }) } -// setOperatorToken writes token into the operator token Secret of the -// control plane coder. Without a token, Coder calls wait with -// OperatorAccessNotReady. -func setOperatorToken(ctx context.Context, t *testing.T, namespace, token string) { - t.Helper() - secret := &corev1.Secret{} - require.NoError(t, k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: "coder-operator-token"}, secret)) - secret.Data = map[string][]byte{coderv1alpha1.DefaultTokenSecretKey: []byte(token)} - require.NoError(t, k8sClient.Update(ctx, secret)) -} - func releasedWithoutDelete(t *testing.T, kind string) float64 { t.Helper() families, err := metrics.Registry.Gather() @@ -142,8 +130,8 @@ func TestControlPlaneWaitsForDependents(t *testing.T) { // #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. An operator token that cannot be read makes it wait and -// retry (item q), not release. +// 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() @@ -157,14 +145,18 @@ func TestCoderAIGatewayKeyDeleteUnderControlPlaneHold(t *testing.T) { require.Equal(t, 5*time.Minute, result.RequeueAfter, "the settle window, not the bounded pass") require.Len(t, env.fake.aiGatewayKeys(), 2) - setOperatorToken(ctx, t, env.namespace, "") + 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") - setOperatorToken(ctx, t, env.namespace, "operator-session-token") + 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()) @@ -246,93 +238,55 @@ func TestControlPlaneForegroundDeletionDoesNotHold(t *testing.T) { require.NotContains(t, controlPlane.Finalizers, workspaceRBACFinalizer, "no hold under foreground deletion") } -// #353 (#362 follow-up 1): the hold keeps Coder running only for dependents -// that can reach it. Nothing repairs the operator access of a control plane -// that is being deleted, so without it a deleted dependent and the control -// plane go at once. -func TestControlPlaneHoldNeedsCoder(t *testing.T) { +// #353: a status without operator access does not end the hold. A provider +// cleanup that read the operator token before a Postgres failure cleared +// operator access still calls Coder, so the deleted control plane keeps +// Coder running for it. +func TestControlPlaneHoldsForCleanupInFlight(t *testing.T) { + ensureGatewaySchemeRegistered(t) t.Parallel() ctx := context.Background() - env := newAIGatewayKeyEnv(ctx, t, ptrTo(true)) - got := rotatedKey(ctx, t, &env) - env.createNamedKey(ctx, t, "live", "coder", "live-secret") - deleteFinalizedControlPlane(ctx, t, env.namespace, "coder") - controlPlane := &coderv1alpha1.CoderControlPlane{} + env, _, provider := newReadyAIProvider(ctx, t) ref := client.ObjectKey{Namespace: env.namespace, Name: "coder"} + controlPlane := &coderv1alpha1.CoderControlPlane{} require.NoError(t, k8sClient.Get(ctx, ref, controlPlane)) - controlPlane.Status.OperatorAccessReady = false - require.NoError(t, k8sClient.Status().Update(ctx, controlPlane)) - require.NoError(t, k8sClient.Delete(ctx, got)) - gone, _ := env.reconcile(ctx, t, got) - require.Nil(t, gone, "the deleted key does not wait for a hold") + controlPlane.Spec.ExtraEnv = []corev1.EnvVar{{Name: "CODER_PG_CONNECTION_URL", Value: "postgres://example/coder"}} + require.NoError(t, k8sClient.Update(ctx, controlPlane)) - r := &controller.CoderControlPlaneReconciler{Client: k8sClient, Scheme: scheme} - _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: ref}) + base, err := client.NewWithWatch(cfg, client.Options{Scheme: scheme}) require.NoError(t, err) - require.True(t, apierrors.IsNotFound(k8sClient.Get(ctx, ref, &coderv1alpha1.CoderControlPlane{})), "the control plane does not wait for the live key") -} - -// deletingProvisioner deletes the control plane while it provisions the -// operator token, like a deletion that comes during a reconcile. -type deletingProvisioner struct { - *fakeOperatorAccessProvisioner - deleteControlPlane func() -} - -func (p deletingProvisioner) EnsureOperatorToken(ctx context.Context, req coderbootstrap.EnsureOperatorTokenRequest) (string, error) { - p.deleteControlPlane() - return p.fakeOperatorAccessProvisioner.EnsureOperatorToken(ctx, req) -} - -// #353 (#362 follow-up 1): TestControlPlaneHoldNeedsCoder is safe only -// because a control plane that lost its operator access before its deletion, -// here in a Postgres failure, does not get it back while it is being -// deleted. A reconcile that was in flight when the deletion came does not -// write it back, and a reconcile after the deletion does not provision it. -func TestControlPlaneOperatorAccessStaysLostWhileDeleting(t *testing.T) { - ensureGatewaySchemeRegistered(t) - t.Parallel() - ctx := context.Background() - namespace := createTestNamespace(ctx, t, "cp-access") - ref := client.ObjectKey{Namespace: namespace, Name: "coder"} - const keep = "coder.com/test-keep" - require.NoError(t, k8sClient.Create(ctx, &coderv1alpha1.CoderControlPlane{ - ObjectMeta: metav1.ObjectMeta{Name: ref.Name, Namespace: namespace, Finalizers: []string{keep}}, - Spec: coderv1alpha1.CoderControlPlaneSpec{ExtraEnv: []corev1.EnvVar{{Name: "CODER_PG_CONNECTION_URL", Value: "postgres://example/coder"}}}, - })) - t.Cleanup(func() { - controlPlane := &coderv1alpha1.CoderControlPlane{} - if k8sClient.Get(context.Background(), ref, controlPlane) == nil { - controlPlane.Finalizers = nil - _ = k8sClient.Update(context.Background(), controlPlane) - } + tokenRead := false + env.r.Client = interceptor.NewClient(base, interceptor.Funcs{ + Get: func(ctx context.Context, c client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + if err := c.Get(ctx, key, obj, opts...); err != nil { + return err + } + if _, ok := obj.(*corev1.Secret); !ok || key.Name != "coder-operator-token" || tokenRead { + return nil + } + tokenRead = true + provisioner := &fakeOperatorAccessProvisioner{err: errors.New("dial tcp: connection refused")} + r := &controller.CoderControlPlaneReconciler{Client: k8sClient, APIReader: k8sClient, Scheme: scheme, OperatorAccessProvisioner: provisioner} + _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: ref}) + require.NoError(t, err) + require.NoError(t, k8sClient.Get(ctx, ref, controlPlane)) + require.False(t, controlPlane.Status.OperatorAccessReady, "a Postgres failure clears operator access") + + require.NoError(t, k8sClient.Delete(ctx, controlPlane)) + provisioner.err = nil + result, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: ref}) + require.NoError(t, err) + require.NoError(t, k8sClient.Get(ctx, ref, controlPlane), "the control plane holds for the cleanup in flight") + require.Equal(t, 10*time.Second, result.RequeueAfter) + return nil + }, }) - provisioner := &fakeOperatorAccessProvisioner{token: "operator-token"} - reconcile := func(p coderbootstrap.OperatorAccessProvisioner) *coderv1alpha1.CoderControlPlane { - t.Helper() - r := &controller.CoderControlPlaneReconciler{Client: k8sClient, Scheme: scheme, OperatorAccessProvisioner: p} - _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: ref}) - require.NoError(t, err) - got := &coderv1alpha1.CoderControlPlane{} - require.NoError(t, k8sClient.Get(ctx, ref, got)) - return got - } - require.True(t, reconcile(provisioner).Status.OperatorAccessReady) - provisioner.err = errors.New("dial tcp: connection refused") - require.False(t, reconcile(provisioner).Status.OperatorAccessReady, "a Postgres failure clears operator access") - - provisioner.err = nil - got := reconcile(deletingProvisioner{provisioner, func() { - require.NoError(t, k8sClient.Delete(ctx, &coderv1alpha1.CoderControlPlane{ObjectMeta: metav1.ObjectMeta{Name: ref.Name, Namespace: namespace}})) - }}) - require.False(t, got.DeletionTimestamp.IsZero()) - require.False(t, got.Status.OperatorAccessReady, "the reconcile in flight does not write operator access back") - calls := provisioner.calls - got = reconcile(provisioner) - require.Equal(t, []string{keep}, got.Finalizers, "the control plane finalized") - require.False(t, got.Status.OperatorAccessReady) - require.Equal(t, calls, provisioner.calls, "no operator token is provisioned for a control plane that is being deleted") + got, _ := env.deleteAIProvider(ctx, t, provider) + require.True(t, tokenRead) + require.Nil(t, got) + _, found := env.fake.aiProvider("openai") + require.False(t, found, "the cleanup deletes the provider in Coder") } // orphanCoderDeployment deletes the Coder Deployment as `kubectl delete @@ -434,7 +388,8 @@ func TestControlPlaneHoldEnd(t *testing.T) { require.NoError(t, k8sClient.Get(ctx, client.ObjectKey{Namespace: env.namespace, Name: "coder"}, controlPlane)) end := controlPlane.DeletionTimestamp.Add(10 * time.Minute) - setOperatorToken(ctx, t, env.namespace, "") + 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) @@ -576,12 +531,6 @@ func TestControlPlaneHoldRevokesWorkspaceRBAC(t *testing.T) { _ = k8sClient.Update(context.Background(), key) } }) - // The control plane holds only with operator access, which this - // reconciler has no provisioner for. - require.NoError(t, k8sClient.Get(ctx, ref, cp)) - cp.Status.OperatorAccessReady = true - cp.Status.OperatorTokenSecretRef = &coderv1alpha1.SecretKeySelector{Name: "coder-operator-token"} - require.NoError(t, k8sClient.Status().Update(ctx, cp)) require.NoError(t, k8sClient.Delete(ctx, key)) require.NoError(t, k8sClient.Delete(ctx, cp)) result, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: ref}) From 822a8c1e8126cc270b072ce8503fcb9965c3488f Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Fri, 30 Oct 2026 16:38:14 +0000 Subject: [PATCH 18/18] docs: drop the no-operator-access hold claim from the keys how-to (#353) Change-Id: I803385a71a62827cf1a28e6f0a95678c34b854e4 Signed-off-by: Thomas Kosiewski --- docs/how-to/manage-ai-gateway-keys.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/how-to/manage-ai-gateway-keys.md b/docs/how-to/manage-ai-gateway-keys.md index 5abf6130..b6bdf639 100644 --- a/docs/how-to/manage-ai-gateway-keys.md +++ b/docs/how-to/manage-ai-gateway-keys.md @@ -139,7 +139,7 @@ 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, and the [cleanup record](#cleanup-records) stays. 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`. -When you delete the control plane while `CoderAIGatewayKey`, `CoderAIProvider` or `CoderMCPServer` 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, providers and MCP server configs 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. The control plane also does not wait when it has no operator access, because then no object can call Coder. +When you delete the control plane while `CoderAIGatewayKey`, `CoderAIProvider` or `CoderMCPServer` 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, providers and MCP server configs 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. 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.