diff --git a/internal/controller/coderworkspacepool_claim.go b/internal/controller/coderworkspacepool_claim.go index 34c90724..2804722e 100644 --- a/internal/controller/coderworkspacepool_claim.go +++ b/internal/controller/coderworkspacepool_claim.go @@ -20,12 +20,12 @@ import ( // poolClaims lists the claims of the pool in its namespace, oldest first. // The list can come from the cache: the member list, which decides the // binds, is uncached. -func (r *CoderWorkspacePoolReconciler) poolClaims(ctx context.Context, pool *coderv1alpha1.CoderWorkspacePool) ([]coderv1alpha1.CoderWorkspaceClaim, error) { +func (r *CoderWorkspacePoolReconciler) poolClaims(ctx context.Context, namespace, poolName string) ([]coderv1alpha1.CoderWorkspaceClaim, error) { list := &coderv1alpha1.CoderWorkspaceClaimList{} - if err := r.List(ctx, list, client.InNamespace(pool.Namespace)); err != nil { - return nil, fmt.Errorf("list workspace claims in namespace %s: %w", pool.Namespace, err) + if err := r.List(ctx, list, client.InNamespace(namespace)); err != nil { + return nil, fmt.Errorf("list workspace claims in namespace %s: %w", namespace, err) } - claims := slices.DeleteFunc(list.Items, func(c coderv1alpha1.CoderWorkspaceClaim) bool { return c.Spec.PoolName != pool.Name }) + claims := slices.DeleteFunc(list.Items, func(c coderv1alpha1.CoderWorkspaceClaim) bool { return c.Spec.PoolName != poolName }) for i := range claims { if claims[i].UID == "" { return nil, fmt.Errorf("assertion failed: workspace claim %s/%s has no UID", claims[i].Namespace, claims[i].Name) @@ -37,6 +37,41 @@ func (r *CoderWorkspacePoolReconciler) poolClaims(ctx context.Context, pool *cod return claims, nil } +// poolClaimWaits reports a claim that no member holds yet and that can +// still bind. +func poolClaimWaits(claim *coderv1alpha1.CoderWorkspaceClaim, all []coderv1alpha1.CoderWorkspacePoolMember) bool { + return claim.DeletionTimestamp.IsZero() && !poolClaimEverBound(claim) && poolClaimHolder(claim, all) == nil +} + +// poolWant is the number of current unclaimed members the pool wants: +// spec.replicas plus the waiting claims (decision D4), so a pool at +// replicas 0 still creates a member for each claim. +func poolWant(pool *coderv1alpha1.CoderWorkspacePool, claims []coderv1alpha1.CoderWorkspaceClaim, all []coderv1alpha1.CoderWorkspacePoolMember) int32 { + want := *pool.Spec.Replicas + for i := range claims { + if poolClaimWaits(&claims[i], all) { + want++ + } + } + return want +} + +// reconcileOrphanClaims writes the status of the claims of a pool that is +// gone or being deleted: a claim that a member holds stays as its member +// says, so a bound claim stays Bound, and the others wait with +// PoolNotFound. +func (r *CoderWorkspacePoolReconciler) reconcileOrphanClaims(ctx context.Context, namespace, poolName string) error { + claims, err := r.poolClaims(ctx, namespace, poolName) + if err != nil || len(claims) == 0 { + return err + } + all := &coderv1alpha1.CoderWorkspacePoolMemberList{} + if err := r.APIReader.List(ctx, all, client.InNamespace(namespace)); err != nil { + return fmt.Errorf("list pool members in namespace %s: %w", namespace, err) + } + return r.writeClaimStatuses(ctx, poolName, true, claims, all.Items, "") +} + // poolClaimHolder returns the member of the claim's pool that holds the // claim, or nil. The member is the source of truth: after a crash between // the bind and the claim status, the next reconcile finds the claim's @@ -134,7 +169,7 @@ func (r *CoderWorkspacePoolReconciler) bindClaims( if len(free) == 0 { return nil } - if !claim.DeletionTimestamp.IsZero() || poolClaimEverBound(claim) || poolClaimHolder(claim, all) != nil { + if !poolClaimWaits(claim, all) { continue } if claim.Status.OwnerName != ownerName { @@ -173,10 +208,12 @@ func (r *CoderWorkspacePoolReconciler) bindClaims( } // writeClaimStatuses writes the status of every claim of the pool from the -// member that holds it (plan 6.3 step 10). ownerName is the owner's Coder +// member that holds it (plan 6.3 step 10). A claim whose member's workspace +// is deleted becomes Lost, which is final. ownerName is the owner's Coder // username from this reconcile, or "" when the owner read did not run. +// poolMissing marks a pool that is gone or being deleted. func (r *CoderWorkspacePoolReconciler) writeClaimStatuses( - ctx context.Context, pool *coderv1alpha1.CoderWorkspacePool, claims []coderv1alpha1.CoderWorkspaceClaim, + ctx context.Context, poolName string, poolMissing bool, claims []coderv1alpha1.CoderWorkspaceClaim, all []coderv1alpha1.CoderWorkspacePoolMember, ownerName string, ) error { now := metav1.NewTime(r.Clock.Now()) @@ -187,7 +224,10 @@ func (r *CoderWorkspacePoolReconciler) writeClaimStatuses( } before := claim.Status.DeepCopy() st := &claim.Status - ready, lost, reason, message := false, false, "WaitingForReadyWorkspace", fmt.Sprintf("CoderWorkspacePool %s has no ready workspace for this claim yet.", pool.Name) + ready, lost, reason, message := false, false, "WaitingForReadyWorkspace", fmt.Sprintf("CoderWorkspacePool %s has no ready workspace for this claim yet.", poolName) + if poolMissing { + reason, message = "PoolNotFound", fmt.Sprintf("CoderWorkspacePool %s does not exist or is being deleted.", poolName) + } m := poolClaimHolder(claim, all) if m == nil && poolClaimEverBound(claim) { // Lost is final, so check the holder on an uncached list first: @@ -216,6 +256,10 @@ func (r *CoderWorkspacePoolReconciler) writeClaimStatuses( if !ready { reason, message = cmp.Or(m.Status.Reason, "WorkspaceNotReady"), truncateTemplateTestMessage(m.Status.Message) } + if meta.IsStatusConditionTrue(m.Status.Conditions, coderv1alpha1.CoderWorkspacePoolMemberConditionWorkspaceDeleted) { + st.Phase, lost = coderv1alpha1.CoderWorkspaceClaimPhaseLost, true + reason, message = "WorkspaceDeleted", fmt.Sprintf("Workspace %s was deleted. The claim never binds again.", m.Status.WorkspaceName) + } default: st.Phase = coderv1alpha1.CoderWorkspaceClaimPhasePending } diff --git a/internal/controller/coderworkspacepool_claim_lifecycle_test.go b/internal/controller/coderworkspacepool_claim_lifecycle_test.go new file mode 100644 index 00000000..b2031064 --- /dev/null +++ b/internal/controller/coderworkspacepool_claim_lifecycle_test.go @@ -0,0 +1,168 @@ +package controller_test + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/coder/coder/v2/codersdk" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + + coderv1alpha1 "github.com/coder/coder-k8s/api/v1alpha1" + "github.com/coder/coder-k8s/internal/controller" +) + +// TestPoolCreatesOnDemand: a pool at replicas 0 creates a member for each +// waiting claim (decision D4: want is replicas plus the waiting claims), +// binds it once it is ready, and creates no more after that. +func TestPoolCreatesOnDemand(t *testing.T) { + t.Parallel() + e := newPoolMemberEnv(t) + e.editPool(t, func(s *coderv1alpha1.CoderWorkspacePoolSpec) { s.Replicas = new(int32(0)) }) + e.reconcilePool(t, 1) + require.Empty(t, e.poolMembers(t, e.pool.Name), "no claim, no member") + + claimKey := e.createClaim(t, "build-1") + e.reconcilePool(t, 2) + members := e.poolMembers(t, e.pool.Name) + require.Len(t, members, 1, "one member for the one waiting claim") + requireClaim(t, e.claim(t, claimKey), coderv1alpha1.CoderWorkspaceClaimPhasePending, "WaitingForReadyWorkspace", false) + + e.setMemberStatus(t, memberKey(members[0]), readyWithWorkspace("kwp-on-demand")) + e.reconcilePool(t, 2) + e.requireBound(t, memberKey(members[0]), claimKey) + requireClaim(t, e.claim(t, claimKey), coderv1alpha1.CoderWorkspaceClaimPhaseBound, "WorkspaceReady", true) + require.Len(t, e.poolMembers(t, e.pool.Name), 1, "no member after the claim is bound") +} + +// TestPoolDeleteKeepsBoundClaims: the claim controls a bound member, so +// deleting the pool does not reach it, and its claim stays Bound. A claim +// that no member holds waits with PoolNotFound, as does a claim on a pool +// that never existed. +func TestPoolDeleteKeepsBoundClaims(t *testing.T) { + t.Parallel() + e := newPoolMemberEnv(t) + member := e.createMember(t, nil) + e.setMemberStatus(t, member, readyWithWorkspace("kwp-kept")) + bound := e.createClaim(t, "bound") + time.Sleep(1100 * time.Millisecond) // creationTimestamp has second precision. + waiting := e.createClaim(t, "waiting") + e.reconcilePool(t, 1) + e.requireBound(t, member, bound) + + require.NoError(t, k8sClient.Delete(e.ctx, e.reconcilePool(t, 0))) // No finalizer: gone at once. + r := &controller.CoderWorkspacePoolReconciler{Client: k8sClient, APIReader: k8sClient, Scheme: scheme, Clock: e.clock} + _, err := r.Reconcile(e.ctx, ctrl.Request{NamespacedName: types.NamespacedName{Namespace: e.ns, Name: e.pool.Name}}) + require.NoError(t, err) + e.requireBound(t, member, bound) + claim := e.claim(t, bound) + requireClaim(t, claim, coderv1alpha1.CoderWorkspaceClaimPhaseBound, "WorkspaceReady", true) + require.Equal(t, "kwp-kept", claim.Status.WorkspaceName) + require.Equal(t, "tester", claim.Status.OwnerName, "kept from the bind") + requireClaim(t, e.claim(t, waiting), coderv1alpha1.CoderWorkspaceClaimPhasePending, "PoolNotFound", false) + + stray := &coderv1alpha1.CoderWorkspaceClaim{} + stray.Name, stray.Namespace, stray.Spec.PoolName = "stray", e.ns, "no-such-pool" + require.NoError(t, k8sClient.Create(e.ctx, stray)) + _, err = r.Reconcile(e.ctx, ctrl.Request{NamespacedName: types.NamespacedName{Namespace: e.ns, Name: "no-such-pool"}}) + require.NoError(t, err) + requireClaim(t, e.claim(t, types.NamespacedName{Namespace: e.ns, Name: "stray"}), coderv1alpha1.CoderWorkspaceClaimPhasePending, "PoolNotFound", false) +} + +// TestPoolClaimLostWhenWorkspaceDeleted: after the one extension at bind, a +// bound member keeps reading its workspace every 60 s. Builds by the +// claimant, a stop included, change nothing. A deleted workspace sets +// WorkspaceDeleted=True, and the claim becomes Lost, which is final: it +// never binds again (plan 6.2 step 11 rule 3). +func TestPoolClaimLostWhenWorkspaceDeleted(t *testing.T) { + t.Parallel() + e := newPoolMemberEnv(t) + e.editPool(t, func(s *coderv1alpha1.CoderWorkspacePoolSpec) { s.Replicas = new(int32(0)) }) + member, workspaceID := e.readyMember(t) + claimKey := e.createClaim(t, "build-1") + e.reconcilePool(t, 1) + e.requireBound(t, member, claimKey) + m := e.reconcileMember(t, member, 1) + require.True(t, m.Status.BindExtended) + require.Equal(t, time.Minute, e.lastStep.RequeueAfter, "a bound member keeps watching") + + stop := e.fake.addExternalBuild(workspaceID, e.tester, codersdk.WorkspaceTransitionStop) + e.fake.setBuildJob(stop.ID, codersdk.ProvisionerJobSucceeded) + m = e.reconcileMember(t, member, 1) + require.Equal(t, coderv1alpha1.CoderWorkspacePoolMemberPhaseReady, m.Status.Phase, "a stop by the claimant changes nothing") + + e.fake.markDeleted(workspaceID) + m = e.reconcileMember(t, member, 1) + requireMemberFailed(t, m, "WorkspaceDeletedExternally", "deleted", metav1.ConditionTrue) + e.reconcilePool(t, 1) + claim := e.claim(t, claimKey) + require.Equal(t, coderv1alpha1.CoderWorkspaceClaimPhaseLost, claim.Status.Phase) + require.Equal(t, "WorkspaceDeleted", claim.Status.Reason) + + // The failed member goes, and the cache still shows the claim without + // its Lost status: a Lost claim is no demand, so no member is created + // for it. + e.deleteMember(t, member) + require.Nil(t, e.reconcileMember(t, member, 2), "the workspace is already deleted") + stale := &controller.CoderWorkspacePoolReconciler{Client: staleClaims{k8sClient}, APIReader: k8sClient, Scheme: scheme, Clock: e.clock} + _, err := stale.Reconcile(e.ctx, ctrl.Request{NamespacedName: types.NamespacedName{Namespace: e.ns, Name: e.pool.Name}}) + require.NoError(t, err) + require.Empty(t, e.poolMembers(t, e.pool.Name), "no member for a Lost claim") + + // A ready member appears: the Lost claim never binds again. + e.editPool(t, func(s *coderv1alpha1.CoderWorkspacePoolSpec) { s.Replicas = new(int32(1)) }) + spare := e.createMember(t, nil) + e.setMemberStatus(t, spare, readyWithWorkspace("kwp-spare")) + // Again on the stale cache, now with a ready member to bind. + _, err = stale.Reconcile(e.ctx, ctrl.Request{NamespacedName: types.NamespacedName{Namespace: e.ns, Name: e.pool.Name}}) + require.NoError(t, err) + require.Nil(t, e.reconcileMember(t, spare, 0).Spec.ClaimRef) + e.reconcilePool(t, 1) + require.Nil(t, e.reconcileMember(t, spare, 0).Spec.ClaimRef) + require.Equal(t, coderv1alpha1.CoderWorkspaceClaimPhaseLost, e.claim(t, claimKey).Status.Phase) +} + +// crashAfterBind lets the first member update land, then reports an +// error, as a crash right after the bind would end the reconcile. +type crashAfterBind struct{ client.Client } + +func (c crashAfterBind) Update(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + if err := c.Client.Update(ctx, obj, opts...); err != nil { + return err + } + if _, ok := obj.(*coderv1alpha1.CoderWorkspacePoolMember); ok { + return errors.New("crash after the bind") + } + return nil +} + +// TestPoolOrphanClaimKeepsOwnerName: the pool crashes right after a bind, +// before the claim status, and the pool is deleted before the next +// reconcile. The claim still shows Bound with the owner's Coder username, +// which the pool stored in the claim status before the bind. +func TestPoolOrphanClaimKeepsOwnerName(t *testing.T) { + t.Parallel() + e := newPoolMemberEnv(t) + member := e.createMember(t, nil) + e.setMemberStatus(t, member, readyWithWorkspace("kwp-crash")) + claimKey := e.createClaim(t, "build-1") + key := types.NamespacedName{Namespace: e.ns, Name: e.pool.Name} + r := &controller.CoderWorkspacePoolReconciler{Client: crashAfterBind{k8sClient}, APIReader: k8sClient, Scheme: scheme, Clock: e.clock} + _, err := r.Reconcile(e.ctx, ctrl.Request{NamespacedName: key}) + require.ErrorContains(t, err, "crash after the bind") + e.requireBound(t, member, claimKey) + + require.NoError(t, k8sClient.Delete(e.ctx, e.reconcilePool(t, 0))) + r = &controller.CoderWorkspacePoolReconciler{Client: k8sClient, APIReader: k8sClient, Scheme: scheme, Clock: e.clock} + _, err = r.Reconcile(e.ctx, ctrl.Request{NamespacedName: key}) + require.NoError(t, err) + claim := e.claim(t, claimKey) + requireClaim(t, claim, coderv1alpha1.CoderWorkspaceClaimPhaseBound, "WorkspaceReady", true) + require.Equal(t, "tester", claim.Status.OwnerName) + require.Equal(t, "kwp-crash", claim.Status.WorkspaceName) +} diff --git a/internal/controller/coderworkspacepool_claim_test.go b/internal/controller/coderworkspacepool_claim_test.go index d45832d7..0aceabca 100644 --- a/internal/controller/coderworkspacepool_claim_test.go +++ b/internal/controller/coderworkspacepool_claim_test.go @@ -97,7 +97,7 @@ func TestPoolBindsOldestClaimFirst(t *testing.T) { requireClaim(t, e.claim(t, newer), coderv1alpha1.CoderWorkspaceClaimPhasePending, "WaitingForReadyWorkspace", false) require.Equal(t, int32(1), pool.Status.BoundReplicas) require.Zero(t, pool.Status.ReadyReplicas, "a bound member is not ready to claim") - require.Len(t, e.poolMembers(t, e.pool.Name), 2, "the pool replaces the bound member") + require.Len(t, e.poolMembers(t, e.pool.Name), 3, "the pool replaces the bound member and adds one for the waiting claim") } // TestPoolBindsCurrentBeforeOutdated: a claim takes a current member before diff --git a/internal/controller/coderworkspacepool_controller.go b/internal/controller/coderworkspacepool_controller.go index d0d99a7f..8a048f41 100644 --- a/internal/controller/coderworkspacepool_controller.go +++ b/internal/controller/coderworkspacepool_controller.go @@ -11,6 +11,7 @@ import ( "github.com/coder/coder/v2/codersdk" "github.com/google/uuid" "k8s.io/apimachinery/pkg/api/equality" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" @@ -65,8 +66,10 @@ func (r *CoderWorkspacePoolReconciler) Reconcile(ctx context.Context, req ctrl.R // and a reconcile that a member delete starts must see the status // written right before that delete. pool := &coderv1alpha1.CoderWorkspacePool{} - if err := r.APIReader.Get(ctx, req.NamespacedName, pool); err != nil { - return ctrl.Result{}, client.IgnoreNotFound(err) + if err := r.APIReader.Get(ctx, req.NamespacedName, pool); apierrors.IsNotFound(err) { + return ctrl.Result{}, r.reconcileOrphanClaims(ctx, req.Namespace, req.Name) + } else if err != nil { + return ctrl.Result{}, fmt.Errorf("get pool %s: %w", req, err) } if pool.Name != req.Name || pool.Namespace != req.Namespace { return ctrl.Result{}, fmt.Errorf("assertion failed: fetched pool %s/%s does not match request %s", pool.Namespace, pool.Name, req) @@ -76,8 +79,8 @@ func (r *CoderWorkspacePoolReconciler) Reconcile(ctx context.Context, req ctrl.R } if !pool.DeletionTimestamp.IsZero() { // Garbage collection deletes the unclaimed members. Their finalizers - // run the last use check. - return ctrl.Result{}, nil + // run the last use check. Bound members belong to their claims. + return ctrl.Result{}, r.reconcileOrphanClaims(ctx, pool.Namespace, pool.Name) } before := pool.Status.DeepCopy() @@ -114,7 +117,7 @@ func (r *CoderWorkspacePoolReconciler) Reconcile(ctx context.Context, req ctrl.R members = append(members, m) } } - claims, err := r.poolClaims(ctx, pool) + claims, err := r.poolClaims(ctx, pool.Namespace, pool.Name) if err != nil { return ctrl.Result{}, err } @@ -152,16 +155,16 @@ func (r *CoderWorkspacePoolReconciler) Reconcile(ctx context.Context, req ctrl.R if err := r.bindClaims(ctx, pool, claims, all.Items, &counts, ownerName); err != nil { return ctrl.Result{}, err } - step, requeue, err = r.createMembers(ctx, pool, maxWorkspaces, all.Items, &counts) + step, requeue, err = r.createMembers(ctx, pool, poolWant(pool, claims, all.Items), maxWorkspaces, all.Items, &counts) if err != nil { return ctrl.Result{}, err } } // The namespace list, where trimToOwnerCap marked its deletes. - if err := r.deleteUnclaimedMembers(ctx, pool, all.Items, step); err != nil { + if err := r.deleteUnclaimedMembers(ctx, pool, poolWant(pool, claims, all.Items), all.Items, step); err != nil { return ctrl.Result{}, err } - if err := r.writeClaimStatuses(ctx, pool, claims, all.Items, ownerName); err != nil { + if err := r.writeClaimStatuses(ctx, pool.Name, false, claims, all.Items, ownerName); err != nil { return ctrl.Result{}, err } r.applyPoolStatus(pool, counts, step) @@ -310,13 +313,12 @@ var poolConsentLostReasons = []string{"OwnerNotAllowed", "TemplateNotAllowed", " // go here. After a consent removal or an owner refusal, all other unclaimed // members go. Otherwise, new before old: outdated members that are not ready // go at once, and outdated ready members go oldest first, down to what the -// current ready members do not cover yet. Current members above -// spec.replicas go in this order: those without a create request, those not +// current ready members do not cover yet. Current members above want +// (spec.replicas plus the waiting claims) go in this order: those without a create request, those not // ready (newest first), then ready ones (oldest first). func (r *CoderWorkspacePoolReconciler) deleteUnclaimedMembers( - ctx context.Context, pool *coderv1alpha1.CoderWorkspacePool, members []coderv1alpha1.CoderWorkspacePoolMember, step *coderStep, + ctx context.Context, pool *coderv1alpha1.CoderWorkspacePool, want int32, members []coderv1alpha1.CoderWorkspacePoolMember, step *coderStep, ) error { - want := *pool.Spec.Replicas // Claims add their demand in PR 11. var current, outdatedReady, doomed []*coderv1alpha1.CoderWorkspacePoolMember var readyCurrent int32 for i := range members { @@ -419,17 +421,18 @@ func (r *CoderWorkspacePoolReconciler) freshOwnerCap(ctx context.Context, pool * } // createMembers creates current members while all caps hold (plan 6.3 step -// 7): current unclaimed members fewer than spec.replicas, building members +// 7): current unclaimed members fewer than want (spec.replicas plus the +// waiting claims, decision D4), building members // fewer than spec.maxInFlight, the owner's live members on this control // plane fewer than maxWorkspaces (from an uncached read), and the failure // backoff passed. It returns the wait that stops it, if any, and when to // read again. func (r *CoderWorkspacePoolReconciler) createMembers( - ctx context.Context, pool *coderv1alpha1.CoderWorkspacePool, maxWorkspaces int32, + ctx context.Context, pool *coderv1alpha1.CoderWorkspacePool, want, maxWorkspaces int32, all []coderv1alpha1.CoderWorkspacePoolMember, counts *poolMemberCounts, ) (*coderStep, time.Duration, error) { st, now := &pool.Status, r.Clock.Now() - need := *pool.Spec.Replicas - counts.current // Claims add their demand in PR 11. + need := want - counts.current if need <= 0 { return nil, 0, nil } diff --git a/internal/controller/coderworkspacepoolmember_controller.go b/internal/controller/coderworkspacepoolmember_controller.go index 91ef4033..75e53404 100644 --- a/internal/controller/coderworkspacepoolmember_controller.go +++ b/internal/controller/coderworkspacepoolmember_controller.go @@ -135,10 +135,8 @@ func (r *CoderWorkspacePoolMemberReconciler) Reconcile(ctx context.Context, req } keepBackoff = step.rateLimited && !step.failed switch { - case step.failed, member.Spec.ClaimRef != nil && member.Status.BindExtended, - meta.IsStatusConditionTrue(member.Status.Conditions, coderv1alpha1.CoderWorkspacePoolMemberConditionUsed): - // The watch rules of a bound member (plan 6.2 step 11 rule 3) come - // with claims. A used member waits for a person to delete it. + case step.failed, meta.IsStatusConditionTrue(member.Status.Conditions, coderv1alpha1.CoderWorkspacePoolMemberConditionUsed): + // A used member waits for a person to delete it. return ctrl.Result{}, nil case member.Status.Phase == coderv1alpha1.CoderWorkspacePoolMemberPhaseReady: return ctrl.Result{RequeueAfter: r.retryAfter(req.NamespacedName, member, step)}, nil diff --git a/internal/controller/coderworkspacepoolmember_extend_test.go b/internal/controller/coderworkspacepoolmember_extend_test.go index c7f433f7..90f6d6d6 100644 --- a/internal/controller/coderworkspacepoolmember_extend_test.go +++ b/internal/controller/coderworkspacepoolmember_extend_test.go @@ -146,7 +146,7 @@ func TestPoolMemberExtendsOnceAtBind(t *testing.T) { require.True(t, member.Status.BindExtended) require.Equal(t, 2, e.fake.requestCount(routeExtendWorkspace), "the bind extends even with 50 minutes left") require.True(t, e.deadline(t, workspaceID).Time.Equal(e.clock.Now().Add(time.Hour))) - require.Zero(t, e.lastStep.RequeueAfter) + require.Equal(t, time.Minute, e.lastStep.RequeueAfter, "a bound member keeps watching for a deleted workspace") e.clock.SetTime(e.clock.Now().Add(55 * time.Minute)) // 5 minutes left. requireMember(t, e.reconcileMember(t, key, 2), coderv1alpha1.CoderWorkspacePoolMemberPhaseReady, "Bound", "") diff --git a/internal/controller/coderworkspacepoolmember_watch.go b/internal/controller/coderworkspacepoolmember_watch.go index 2f55f67a..47c02e5c 100644 --- a/internal/controller/coderworkspacepoolmember_watch.go +++ b/internal/controller/coderworkspacepoolmember_watch.go @@ -31,16 +31,23 @@ func (r *CoderWorkspacePoolMemberReconciler) watchReady( ctx context.Context, sdk *codersdk.Client, member *coderv1alpha1.CoderWorkspacePoolMember, before *coderv1alpha1.CoderWorkspacePoolMemberStatus, now time.Time, ) (*coderStep, error) { bound := member.Spec.ClaimRef != nil - if (bound && member.Status.BindExtended) || meta.IsStatusConditionTrue(member.Status.Conditions, coderv1alpha1.CoderWorkspacePoolMemberConditionUsed) { - // The one extension at bind is done, or someone used the workspace - // without a claim: Coder's own autostop applies, and the controller - // never fails a used member. + if meta.IsStatusConditionTrue(member.Status.Conditions, coderv1alpha1.CoderWorkspacePoolMemberConditionUsed) { + // Someone used the workspace without a claim: Coder's own autostop + // applies, and the controller never fails a used member. return coderWait(member.Status.Reason, "%s", member.Status.Message), nil } + // A bound member only watches for the end (step 11 rule 3): the read + // answers 410 once someone deleted the workspace, which sets + // WorkspaceDeleted=True, and the claim becomes Lost. workspace, operatorID, step, err := readPoolMemberWorkspace(ctx, sdk, member) if step != nil || err != nil { return step, err } + if bound && member.Status.BindExtended { + // The one extension at bind is done. Builds by the claimant, a stop + // included, are normal and change nothing. + return coderWait(member.Status.Reason, "%s", member.Status.Message), nil + } name := member.Status.WorkspaceName if !bound { // Rule 1 runs first, so the later rules never fail a used member.