From a4409ed173685235e384db87af88d6db064dec0e Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 14 Oct 2026 19:17:09 +0000 Subject: [PATCH 1/4] =?UTF-8?q?=F0=9F=A4=96=20fix:=20hold=20a=20deleted=20?= =?UTF-8?q?CoderControlPlane=20while=20CoderMCPServer=20objects=20still=20?= =?UTF-8?q?use=20it?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #382 Refs #353 Refs #155 Change-Id: Ia046b07473f403eb059177a7a1c437002a60f041 Signed-off-by: Thomas Kosiewski --- docs/how-to/manage-mcp-servers.md | 5 ++- .../codercontrolplane_controller.go | 6 ++- internal/controller/codermcpserver_delete.go | 20 +++++++++- .../controller/codermcpserver_delete_test.go | 38 +++++++++++++++++++ internal/controller/controlplane_hold.go | 31 ++++++++++----- .../controlplane_hold_internal_test.go | 24 ++++++++++++ 6 files changed, 110 insertions(+), 14 deletions(-) diff --git a/docs/how-to/manage-mcp-servers.md b/docs/how-to/manage-mcp-servers.md index b216fc8a..6323abbd 100644 --- a/docs/how-to/manage-mcp-servers.md +++ b/docs/how-to/manage-mcp-servers.md @@ -137,7 +137,7 @@ When a group in `access.groups` does not exist, the sync fails closed: 1. It removes every entry that `access.groups` no longer names, also user entries. 2. It adds nothing. Access never widens while a group is missing. 3. If the same spec generation disables the server (`enabled: false`), it sends one update that holds only `enabled: false`. -4. It leaves the rest of the generation unapplied. The object stalls with `GroupNotFound` and checks again every 10 minutes. +4. It leaves the rest of the generation unapplied. The object stalls with `GroupNotFound` and checks again every 10 minutes. The API server refuses a name that is not a valid Coder group name. If such a name reaches the operator anyway, the object stalls with `GroupNotFound` and is not checked again until the spec changes: fix the name. 5. Once the missing groups exist, the sync adds them and applies the rest of the generation. A new server stays disabled in Coder while one of its groups is missing. @@ -207,7 +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. -- While the control plane is being deleted, 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. +- 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. - A withdrawn opt-in never blocks a delete. Delete `CoderMCPServer` objects before their control plane and before their namespace. diff --git a/internal/controller/codercontrolplane_controller.go b/internal/controller/codercontrolplane_controller.go index 804557c0..642458bd 100644 --- a/internal/controller/codercontrolplane_controller.go +++ b/internal/controller/codercontrolplane_controller.go @@ -623,8 +623,8 @@ func (r *CoderControlPlaneReconciler) finalizeWorkspaceRBAC( return ctrl.Result{}, err } // The finalizer keeps the Coder Deployment (owned by this object) - // running while AI Gateway keys and AI providers that use it still - // delete their resources in Coder (#353). The grace period after the hold + // running while AI Gateway keys, AI providers and MCP servers that use + // it still delete their resources in Coder (#353, #382). The grace period after the hold // lets each of them run its bounded pass before Coder goes. now := r.now() dependents, perKind, err := controlPlaneDependents(ctx, r.Client, coderControlPlane, now) @@ -3309,6 +3309,8 @@ func (r *CoderControlPlaneReconciler) SetupWithManager(mgr ctrl.Manager) error { Watches(&coderv1alpha1.CoderAIGatewayKey{}, handler.EnqueueRequestsFromMapFunc(requestForDependentControlPlane), ctrlbuilder.WithPredicates(dependentRemoved)). Watches(&coderv1alpha1.CoderAIProvider{}, handler.EnqueueRequestsFromMapFunc(requestForDependentControlPlane), + ctrlbuilder.WithPredicates(dependentRemoved)). + Watches(&coderv1alpha1.CoderMCPServer{}, handler.EnqueueRequestsFromMapFunc(requestForDependentControlPlane), ctrlbuilder.WithPredicates(dependentRemoved)) // Gateway API is optional; only watch HTTPRoutes when the CRD is installed. diff --git a/internal/controller/codermcpserver_delete.go b/internal/controller/codermcpserver_delete.go index 072364af..1c92dc36 100644 --- a/internal/controller/codermcpserver_delete.go +++ b/internal/controller/codermcpserver_delete.go @@ -49,7 +49,25 @@ func (r *CoderMCPServerReconciler) deleteStep(ctx context.Context, server *coder return false, nil, err } deleting := !controlPlane.DeletionTimestamp.IsZero() - sdk, _, wait, err := controlPlaneCoderClientFor(ctx, r.Client, ref.Namespace, ref.Name, deleting) + if deleting { + // While the control plane holds for this object, Coder keeps running: + // use the normal path, which retries, instead of the bounded pass + // (#382, #353). + now := time.Now() + holds, err := controlPlaneHolds(ctx, r.Client, controlPlane, now) + if err != nil { + return false, nil, err + } + deleting = !holds + if holds { + defer func() { + if step != nil { + step.requeue = controlPlaneHoldRequeue(controlPlane, now, step.requeue) + } + }() + } + } + sdk, _, wait, err := controlPlaneCoderClientFor(ctx, r.Client, ref.Namespace, ref.Name, !controlPlane.DeletionTimestamp.IsZero()) switch { case err != nil && deleting: // One attempt only, also when the access setup fails. diff --git a/internal/controller/codermcpserver_delete_test.go b/internal/controller/codermcpserver_delete_test.go index f6a3e76b..05a34dd9 100644 --- a/internal/controller/codermcpserver_delete_test.go +++ b/internal/controller/codermcpserver_delete_test.go @@ -218,6 +218,44 @@ func TestCoderMCPServerDeleteWhileControlPlaneDeletes(t *testing.T) { } } +// #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 +// finalizer and retries, instead of one bounded attempt and a release. Then +// the control plane goes. +func TestCoderMCPServerDeleteUnderControlPlaneHold(t *testing.T) { + t.Parallel() + ctx := context.Background() + env := newMCPServerEnv(ctx, t, true) + env.keySecret(ctx, t, testMCPKey) + got, _ := env.applied(ctx, t, nil) + deleteFinalizedControlPlane(ctx, t, env.namespace, "coder") + require.NoError(t, k8sClient.Delete(ctx, got)) + + controlPlaneGone := func() bool { + t.Helper() + r := &controller.CoderControlPlaneReconciler{Client: k8sClient, Scheme: scheme} + _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKey{Namespace: env.namespace, Name: "coder"}}) + require.NoError(t, err) + err = k8sClient.Get(ctx, client.ObjectKey{Namespace: env.namespace, Name: "coder"}, &coderv1alpha1.CoderControlPlane{}) + return apierrors.IsNotFound(err) + } + require.False(t, controlPlaneGone(), "the control plane waits for the MCP server") + + env.fake.failNext(routeDeleteMCPServer, fakeFault{Status: http.StatusBadGateway}) + kept, result := env.reconcileDeleting(ctx, t, got) + require.NotNil(t, kept, "a failed delete keeps the finalizer while the control plane waits") + requireMCPServerStep(t, kept, false, "CoderUnavailable") + require.Equal(t, 60*time.Second, result.RequeueAfter) + require.False(t, controlPlaneGone(), "the control plane still waits") + + gone, _ := env.reconcileDeleting(ctx, t, kept) + require.Nil(t, gone) + require.Empty(t, env.fake.mcpServers(), "the retry deleted the config") + require.Equal(t, 2, env.fake.requestCount(routeDeleteMCPServer)) + require.True(t, controlPlaneGone(), "the control plane goes after the MCP server") +} + // From the #329 final assessment: a released object's entry in the // reconciler's credential hash map goes, so the map does not grow without // bound. diff --git a/internal/controller/controlplane_hold.go b/internal/controller/controlplane_hold.go index fce505be..c2a93511 100644 --- a/internal/controller/controlplane_hold.go +++ b/internal/controller/controlplane_hold.go @@ -22,9 +22,10 @@ import ( ) // controlPlaneDependentHold is how long a CoderControlPlane that is being -// deleted waits for the CoderAIGatewayKey and CoderAIProvider objects that -// use it (#353). Its Coder Deployment keeps running while it waits, so they -// can delete their keys and providers in Coder. It covers the 5-minute +// deleted waits for the CoderAIGatewayKey, CoderAIProvider and CoderMCPServer +// objects that use it (#353, #382). Its Coder Deployment keeps running while +// it waits, so they can delete their keys, providers and MCP server configs +// in Coder. It covers the 5-minute // settle window of a key deletion plus retries. After it, the control plane // goes, and they release with ControlPlaneGone. const controlPlaneDependentHold = 10 * time.Minute @@ -93,7 +94,7 @@ func controlPlaneHoldEnd(controlPlane *coderv1alpha1.CoderControlPlane) time.Tim // controlPlaneDependentGraceFor is the grace period after the hold when // perKind dependents of one kind remain (controlPlaneDependentGrace). func controlPlaneDependentGraceFor(perKind int) time.Duration { - return min(controlPlaneDependentGrace+time.Duration(perKind)*max(aiGatewayKeyBoundedDeleteTimeout, aiProviderBoundedDeleteTimeout), controlPlaneDependentGraceMax) + return min(controlPlaneDependentGrace+time.Duration(perKind)*max(aiGatewayKeyBoundedDeleteTimeout, aiProviderBoundedDeleteTimeout, mcpServerBoundedDeleteTimeout), controlPlaneDependentGraceMax) } // controlPlaneHoldRequeue caps a dependent's requeue under the hold, so it @@ -126,8 +127,8 @@ func controlPlaneHolds(ctx context.Context, reader client.Reader, controlPlane * return namespace.DeletionTimestamp.IsZero() && namespace.Status.Phase != corev1.NamespaceTerminating, nil } -// controlPlaneDependents returns the CoderAIGatewayKey and CoderAIProvider -// objects in controlPlane's namespace that reference it and hold it at now, +// controlPlaneDependents returns the CoderAIGatewayKey, CoderAIProvider and +// CoderMCPServer objects in controlPlane's namespace that reference it and hold it at now, // as kind/name, and the larger count of one kind. A dependent holds it while // it is being deleted and still has its cleanup finalizer (another finalizer // does not hold it), and a live one only for controlPlaneLiveDependentWait @@ -163,7 +164,17 @@ func controlPlaneDependents(ctx context.Context, reader client.Reader, controlPl dependents = append(dependents, "CoderAIProvider/"+p.Name) } } - return dependents, max(keyCount, len(dependents)-keyCount), nil + providerCount := len(dependents) - keyCount + servers := &coderv1alpha1.CoderMCPServerList{} + if err := reader.List(ctx, servers, client.InNamespace(controlPlane.Namespace)); err != nil { + return nil, 0, fmt.Errorf("list MCP servers of namespace %s: %w", controlPlane.Namespace, err) + } + for i := range servers.Items { + if m := &servers.Items[i]; m.Spec.ControlPlaneRef.Name == controlPlane.Name && holds(m, coderv1alpha1.CoderMCPServerCleanupFinalizer) { + dependents = append(dependents, "CoderMCPServer/"+m.Name) + } + } + return dependents, max(keyCount, providerCount, len(dependents)-keyCount-providerCount), nil } // dependentRemoved passes only the removal of a dependent object, the event @@ -175,8 +186,8 @@ var dependentRemoved = predicate.Funcs{ GenericFunc: func(event.GenericEvent) bool { return false }, } -// requestForDependentControlPlane maps a CoderAIGatewayKey or CoderAIProvider -// to the control plane that it references. +// requestForDependentControlPlane maps a CoderAIGatewayKey, CoderAIProvider or +// CoderMCPServer to the control plane that it references. func requestForDependentControlPlane(_ context.Context, obj client.Object) []reconcile.Request { var name string switch o := obj.(type) { @@ -184,6 +195,8 @@ func requestForDependentControlPlane(_ context.Context, obj client.Object) []rec name = o.Spec.ControlPlaneRef.Name case *coderv1alpha1.CoderAIProvider: name = o.Spec.ControlPlaneRef.Name + case *coderv1alpha1.CoderMCPServer: + name = o.Spec.ControlPlaneRef.Name } if name == "" { return nil diff --git a/internal/controller/controlplane_hold_internal_test.go b/internal/controller/controlplane_hold_internal_test.go index 9b1bd8f7..495bf6b6 100644 --- a/internal/controller/controlplane_hold_internal_test.go +++ b/internal/controller/controlplane_hold_internal_test.go @@ -1,10 +1,17 @@ package controller import ( + "context" "testing" "time" "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + coderv1alpha1 "github.com/coder/coder-k8s/api/v1alpha1" ) // #353: the grace period after the hold grows by one bounded pass per @@ -16,3 +23,20 @@ func TestControlPlaneDependentGraceFor(t *testing.T) { require.Equal(t, time.Minute, controlPlaneDependentGraceFor(3)) require.Equal(t, 5*time.Minute, controlPlaneDependentGraceFor(1000), "the grace period has a cap") } + +// #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) { + t.Parallel() + meta := metav1.ObjectMeta{Name: "dependent", Namespace: "coder"} + ref := coderv1alpha1.CoderControlPlaneReference{Name: "coder"} + want := []reconcile.Request{{NamespacedName: types.NamespacedName{Namespace: "coder", Name: "coder"}}} + for _, obj := range []client.Object{ + &coderv1alpha1.CoderAIGatewayKey{ObjectMeta: meta, Spec: coderv1alpha1.CoderAIGatewayKeySpec{ControlPlaneRef: ref}}, + &coderv1alpha1.CoderAIProvider{ObjectMeta: meta, Spec: coderv1alpha1.CoderAIProviderSpec{ControlPlaneRef: ref}}, + &coderv1alpha1.CoderMCPServer{ObjectMeta: meta, Spec: coderv1alpha1.CoderMCPServerSpec{ControlPlaneRef: ref}}, + } { + require.Equal(t, want, requestForDependentControlPlane(context.Background(), obj), "%T", obj) + } + require.Nil(t, requestForDependentControlPlane(context.Background(), &coderv1alpha1.CoderTemplateTest{ObjectMeta: meta}), "not a dependent kind") +} From 05a47b6e2b85894fb339253623c53f60dc4492dd Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 14 Oct 2026 19:37:57 +0000 Subject: [PATCH 2/4] =?UTF-8?q?=F0=9F=A4=96=20docs:=20limit=20the=20ten-mi?= =?UTF-8?q?nute=20group=20recheck=20to=20valid=20group=20names?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: Id1e2e4a197291d2db02e0bea3e729702cc882a80 Signed-off-by: Thomas Kosiewski --- docs/how-to/manage-mcp-servers.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/how-to/manage-mcp-servers.md b/docs/how-to/manage-mcp-servers.md index 6323abbd..2380f158 100644 --- a/docs/how-to/manage-mcp-servers.md +++ b/docs/how-to/manage-mcp-servers.md @@ -137,7 +137,7 @@ When a group in `access.groups` does not exist, the sync fails closed: 1. It removes every entry that `access.groups` no longer names, also user entries. 2. It adds nothing. Access never widens while a group is missing. 3. If the same spec generation disables the server (`enabled: false`), it sends one update that holds only `enabled: false`. -4. It leaves the rest of the generation unapplied. The object stalls with `GroupNotFound` and checks again every 10 minutes. The API server refuses a name that is not a valid Coder group name. If such a name reaches the operator anyway, the object stalls with `GroupNotFound` and is not checked again until the spec changes: fix the name. +4. It leaves the rest of the generation unapplied. The object stalls with `GroupNotFound`. When the name is valid but no such group exists yet, the operator checks again every 10 minutes. The API server refuses a name that is not a valid Coder group name. If such a name reaches the operator anyway, the operator does not check again until the spec changes: fix the name. 5. Once the missing groups exist, the sync adds them and applies the rest of the generation. A new server stays disabled in Coder while one of its groups is missing. From cf8651d124cc92ad661d385914aeac0e971565b5 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 14 Oct 2026 19:57:45 +0000 Subject: [PATCH 3/4] =?UTF-8?q?=F0=9F=A4=96=20fix:=20end=20a=20CoderMCPSer?= =?UTF-8?q?ver=20delete=20pass=20under=20the=20control=20plane=20hold=20at?= =?UTF-8?q?=20the=20hold=20end?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: Ic019bddbfdcbe5c7e323f5dfe9b4409ac63d260e Signed-off-by: Thomas Kosiewski --- internal/controller/codermcpserver_delete.go | 6 ++ .../codermcpserver_delete_internal_test.go | 66 +++++++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 internal/controller/codermcpserver_delete_internal_test.go diff --git a/internal/controller/codermcpserver_delete.go b/internal/controller/codermcpserver_delete.go index 1c92dc36..3553b307 100644 --- a/internal/controller/codermcpserver_delete.go +++ b/internal/controller/codermcpserver_delete.go @@ -60,6 +60,12 @@ func (r *CoderMCPServerReconciler) deleteStep(ctx context.Context, server *coder } deleting = !holds if holds { + // A pass under the hold ends at the hold end, however slowly + // Coder answers. The bounded pass after it then still fits in + // the control plane's grace period. + 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) diff --git a/internal/controller/codermcpserver_delete_internal_test.go b/internal/controller/codermcpserver_delete_internal_test.go new file mode 100644 index 00000000..4b104749 --- /dev/null +++ b/internal/controller/codermcpserver_delete_internal_test.go @@ -0,0 +1,66 @@ +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 review: a delete pass under the control plane hold ends at the hold +// end, also when Coder does not answer, so the bounded pass after it fits in +// the grace period. Here the hold ends in 5 s and Coder hangs on the DELETE, +// whose own request timeout is 30 s. +func TestMCPServerDeleteUnderHoldEndsAtHoldEnd(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", 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"}, + 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} + 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") + + server := &coderv1alpha1.CoderMCPServer{ObjectMeta: metav1.ObjectMeta{Name: "github", Namespace: "ns"}} + server.Spec.ControlPlaneRef.Name = "coder" + server.Spec.Slug = "github" + server.Status.OrganizationID = uuid.NewString() + server.Status.ConfigID = uuid.NewString() + start := time.Now() + release, step, err := r.deleteStep(context.Background(), server) + 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 is the time to the hold end plus 1 s (controlPlaneHoldRequeue). + require.LessOrEqual(t, step.requeue, 6*time.Second, "the next pass runs right after the hold end") +} From 859d6bc714b56f394ac5ddd500f94cb9e21d24e6 Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Wed, 14 Oct 2026 20:19:36 +0000 Subject: [PATCH 4/4] =?UTF-8?q?=F0=9F=A4=96=20fix:=20count=20the=20hold=20?= =?UTF-8?q?requeue=20of=20a=20CoderMCPServer=20delete=20from=20the=20end?= =?UTF-8?q?=20of=20the=20pass?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change-Id: I8de8cb92e215f1879c2192913ffe49c83a84f4ee Signed-off-by: Thomas Kosiewski --- internal/controller/codermcpserver_delete.go | 4 +++- internal/controller/codermcpserver_delete_internal_test.go | 5 +++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/internal/controller/codermcpserver_delete.go b/internal/controller/codermcpserver_delete.go index 3553b307..aa47d74b 100644 --- a/internal/controller/codermcpserver_delete.go +++ b/internal/controller/codermcpserver_delete.go @@ -68,7 +68,9 @@ func (r *CoderMCPServerReconciler) deleteStep(ctx context.Context, server *coder defer cancel() defer func() { if step != nil { - step.requeue = controlPlaneHoldRequeue(controlPlane, now, step.requeue) + // From the time the pass ended: a pass that ran until the + // deadline retries right after the hold end. + step.requeue = controlPlaneHoldRequeue(controlPlane, time.Now(), step.requeue) } }() } diff --git a/internal/controller/codermcpserver_delete_internal_test.go b/internal/controller/codermcpserver_delete_internal_test.go index 4b104749..31e0672c 100644 --- a/internal/controller/codermcpserver_delete_internal_test.go +++ b/internal/controller/codermcpserver_delete_internal_test.go @@ -61,6 +61,7 @@ func TestMCPServerDeleteUnderHoldEndsAtHoldEnd(t *testing.T) { 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 is the time to the hold end plus 1 s (controlPlaneHoldRequeue). - require.LessOrEqual(t, step.requeue, 6*time.Second, "the next pass runs right after the hold end") + // 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") }