Skip to content

Commit 62ac374

Browse files
authored
🤖 feat: create CoderWorkspacePool members on demand for waiting claims, keep claims of a missing pool, and mark a claim Lost when its workspace is deleted (#388)
## Summary This PR finishes the claim side of the dormant `CoderWorkspacePool` reconcilers (plan 6.2 step 11 rule 3, 6.3 steps 7 and 10, decision D4). Nothing registers the reconcilers yet, so no user sees a change. 1. **On-demand creation.** The pool now wants `spec.replicas` plus the waiting claims, for both creates and scale-down deletes. A waiting claim is one that no member holds, that is not deleting, and that is not `Lost`. So a pool at `replicas: 0` creates one member per waiting claim, binds it once it is ready, and then wants nothing more. 2. **`PoolNotFound`.** A reconcile for a pool that is gone or being deleted writes the status of its claims. A claim that a member holds stays as its member says, so a bound claim stays `Bound` after its pool is deleted: the claim, not the pool, controls the bound member. The other claims wait with `Pending` `PoolNotFound`. The same holds for a claim on a pool that never existed. 3. **Bound-member watch (`Lost`).** After the one extension at bind, a bound member keeps reading its workspace every 60 s, with the identity checks. Builds by the claimant, a stop included, change nothing. Once Coder answers 410 (the workspace is deleted), the member fails with `WorkspaceDeleted=True`, and its claim becomes `Lost` (`Stalled=True`, reason `WorkspaceDeleted`). `Lost` is final: the claim never binds again, also after its failed member is gone. Refs #153 ## Background This is PR 11b of the `CoderWorkspacePool` stack (#153). It stacks on PR 11a (#387). Before this PR, a bound member stopped reconciling after the bind extension (`RequeueAfter: 0`). It now polls every 60 s, as a ready unclaimed member does. `TestPoolMemberExtendsOnceAtBind` expects that now. Rule 2 (the latest build must be the start build) still runs for a bound member before its extension, because it is the provenance check before that write. After the extension, the member returns before rule 2, so a claimant's builds never fail it. ## Validation New tests (`coderworkspacepool_claim_lifecycle_test.go`). I wrote them first and ran them on PR 11a: all 3 failed, as did the changed `TestPoolMemberExtendsOnceAtBind`. With this PR they pass. - `TestPoolCreatesOnDemand`: `replicas: 0` and no claim give no member. One claim gives exactly one member. Once it is ready, the claim binds it, and the pool creates no more. - `TestPoolDeleteKeepsBoundClaims`: one member bound to the older of two claims, then the pool is deleted. The bound member keeps the claim as its controller, and its claim stays `Bound` with its workspace name and owner. The other claim waits with `PoolNotFound`, and so does a claim on a pool that never existed. - `TestPoolClaimLostWhenWorkspaceDeleted`: a real workspace, bound and extended. The member requeues after 60 s. A stop build by the claimant leaves the member `Ready`. After Coder deletes the workspace, the member fails with `WorkspaceDeleted=True`, and the claim becomes `Lost` with `WorkspaceDeleted`. After the failed member is gone, a new ready member is not bound to the `Lost` claim. - Changed: `TestPoolBindsOldestClaimFirst` now expects 3 members (the bound member, a replacement for `replicas: 1`, and one for the waiting claim). - `TestPoolOrphanClaimKeepsOwnerName` (round 1): the bind lands, the reconcile then fails, and the pool is deleted. The orphan reconcile shows `Bound` with owner `tester`. - `TestPoolClaimLostWhenWorkspaceDeleted` also runs a reconcile whose claim list shows the `Lost` claim without status, after its failed member is gone: no bind. Codex review round 1: 4277241744 (a `Lost` claim on a stale cache) and 4277241752 (the owner name of an orphaned claim after a crash) are fixed in #387 (`c4824691`: the uncached claim read before a bind, and the owner name stored before the bind) and tested here in `23e6281d`. Both tests failed on the previous head. Codex review round 2: 4277392817 (a stale `Lost` claim counted as demand when no member was free) is fixed in #387 (`c8c6095d`: every waiting claim is read again without the cache before `poolWant`) and tested here in `e6eb14ff`: `TestPoolClaimLostWhenWorkspaceDeleted` runs a stale-cache reconcile with no free member, and no member is created. It failed on the previous head. Mutation checks: I removed one control at a time, and each removal made a named test fail (10 of 10): the claims in `want`, counting only waiting claims in `want`, the missing-pool reconcile, keeping bound claims of a missing pool, setting `Lost`, never binding a `Lost` claim, the bound member's read after the extension, the early return before rule 2 after the extension, the owner name stored before the bind, and the uncached claim read for a `Lost` claim. Gates on the final commit: drift (codegen, manifests, installer, API reference: no diff), `make docs-check`, dormant-CRD guard, docs lint and spelling, `make verify-vendor`, `make build`, `make lint`, actionlint, `make test`, `make test-integration`, `make vuln`, race. All passed. ## Size 286 hand-written lines (114 production, 172 test, counting removals). No generated files. ## Risks None for users. The reconcilers are dormant. Recorded for PR 12: a workspace that the claimant renames fails the identity check of a bound member (`OwnershipUnknown`), so its claim shows `Bound` with `Ready=False`. --- _Generated with `mux` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$173.65`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=173.65 --> --------- Signed-off-by: Thomas Kosiewski <tk@coder.com>
1 parent 66902b2 commit 62ac374

7 files changed

Lines changed: 253 additions & 33 deletions

‎internal/controller/coderworkspacepool_claim.go‎

Lines changed: 52 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,12 @@ import (
2020
// poolClaims lists the claims of the pool in its namespace, oldest first.
2121
// The list can come from the cache: the member list, which decides the
2222
// binds, is uncached.
23-
func (r *CoderWorkspacePoolReconciler) poolClaims(ctx context.Context, pool *coderv1alpha1.CoderWorkspacePool) ([]coderv1alpha1.CoderWorkspaceClaim, error) {
23+
func (r *CoderWorkspacePoolReconciler) poolClaims(ctx context.Context, namespace, poolName string) ([]coderv1alpha1.CoderWorkspaceClaim, error) {
2424
list := &coderv1alpha1.CoderWorkspaceClaimList{}
25-
if err := r.List(ctx, list, client.InNamespace(pool.Namespace)); err != nil {
26-
return nil, fmt.Errorf("list workspace claims in namespace %s: %w", pool.Namespace, err)
25+
if err := r.List(ctx, list, client.InNamespace(namespace)); err != nil {
26+
return nil, fmt.Errorf("list workspace claims in namespace %s: %w", namespace, err)
2727
}
28-
claims := slices.DeleteFunc(list.Items, func(c coderv1alpha1.CoderWorkspaceClaim) bool { return c.Spec.PoolName != pool.Name })
28+
claims := slices.DeleteFunc(list.Items, func(c coderv1alpha1.CoderWorkspaceClaim) bool { return c.Spec.PoolName != poolName })
2929
for i := range claims {
3030
if claims[i].UID == "" {
3131
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
3737
return claims, nil
3838
}
3939

40+
// poolClaimWaits reports a claim that no member holds yet and that can
41+
// still bind.
42+
func poolClaimWaits(claim *coderv1alpha1.CoderWorkspaceClaim, all []coderv1alpha1.CoderWorkspacePoolMember) bool {
43+
return claim.DeletionTimestamp.IsZero() && !poolClaimEverBound(claim) && poolClaimHolder(claim, all) == nil
44+
}
45+
46+
// poolWant is the number of current unclaimed members the pool wants:
47+
// spec.replicas plus the waiting claims (decision D4), so a pool at
48+
// replicas 0 still creates a member for each claim.
49+
func poolWant(pool *coderv1alpha1.CoderWorkspacePool, claims []coderv1alpha1.CoderWorkspaceClaim, all []coderv1alpha1.CoderWorkspacePoolMember) int32 {
50+
want := *pool.Spec.Replicas
51+
for i := range claims {
52+
if poolClaimWaits(&claims[i], all) {
53+
want++
54+
}
55+
}
56+
return want
57+
}
58+
59+
// reconcileOrphanClaims writes the status of the claims of a pool that is
60+
// gone or being deleted: a claim that a member holds stays as its member
61+
// says, so a bound claim stays Bound, and the others wait with
62+
// PoolNotFound.
63+
func (r *CoderWorkspacePoolReconciler) reconcileOrphanClaims(ctx context.Context, namespace, poolName string) error {
64+
claims, err := r.poolClaims(ctx, namespace, poolName)
65+
if err != nil || len(claims) == 0 {
66+
return err
67+
}
68+
all := &coderv1alpha1.CoderWorkspacePoolMemberList{}
69+
if err := r.APIReader.List(ctx, all, client.InNamespace(namespace)); err != nil {
70+
return fmt.Errorf("list pool members in namespace %s: %w", namespace, err)
71+
}
72+
return r.writeClaimStatuses(ctx, poolName, true, claims, all.Items, "")
73+
}
74+
4075
// poolClaimHolder returns the member of the claim's pool that holds the
4176
// claim, or nil. The member is the source of truth: after a crash between
4277
// the bind and the claim status, the next reconcile finds the claim's
@@ -134,7 +169,7 @@ func (r *CoderWorkspacePoolReconciler) bindClaims(
134169
if len(free) == 0 {
135170
return nil
136171
}
137-
if !claim.DeletionTimestamp.IsZero() || poolClaimEverBound(claim) || poolClaimHolder(claim, all) != nil {
172+
if !poolClaimWaits(claim, all) {
138173
continue
139174
}
140175
if claim.Status.OwnerName != ownerName {
@@ -173,10 +208,12 @@ func (r *CoderWorkspacePoolReconciler) bindClaims(
173208
}
174209

175210
// writeClaimStatuses writes the status of every claim of the pool from the
176-
// member that holds it (plan 6.3 step 10). ownerName is the owner's Coder
211+
// member that holds it (plan 6.3 step 10). A claim whose member's workspace
212+
// is deleted becomes Lost, which is final. ownerName is the owner's Coder
177213
// username from this reconcile, or "" when the owner read did not run.
214+
// poolMissing marks a pool that is gone or being deleted.
178215
func (r *CoderWorkspacePoolReconciler) writeClaimStatuses(
179-
ctx context.Context, pool *coderv1alpha1.CoderWorkspacePool, claims []coderv1alpha1.CoderWorkspaceClaim,
216+
ctx context.Context, poolName string, poolMissing bool, claims []coderv1alpha1.CoderWorkspaceClaim,
180217
all []coderv1alpha1.CoderWorkspacePoolMember, ownerName string,
181218
) error {
182219
now := metav1.NewTime(r.Clock.Now())
@@ -187,7 +224,10 @@ func (r *CoderWorkspacePoolReconciler) writeClaimStatuses(
187224
}
188225
before := claim.Status.DeepCopy()
189226
st := &claim.Status
190-
ready, lost, reason, message := false, false, "WaitingForReadyWorkspace", fmt.Sprintf("CoderWorkspacePool %s has no ready workspace for this claim yet.", pool.Name)
227+
ready, lost, reason, message := false, false, "WaitingForReadyWorkspace", fmt.Sprintf("CoderWorkspacePool %s has no ready workspace for this claim yet.", poolName)
228+
if poolMissing {
229+
reason, message = "PoolNotFound", fmt.Sprintf("CoderWorkspacePool %s does not exist or is being deleted.", poolName)
230+
}
191231
m := poolClaimHolder(claim, all)
192232
if m == nil && poolClaimEverBound(claim) {
193233
// Lost is final, so check the holder on an uncached list first:
@@ -216,6 +256,10 @@ func (r *CoderWorkspacePoolReconciler) writeClaimStatuses(
216256
if !ready {
217257
reason, message = cmp.Or(m.Status.Reason, "WorkspaceNotReady"), truncateTemplateTestMessage(m.Status.Message)
218258
}
259+
if meta.IsStatusConditionTrue(m.Status.Conditions, coderv1alpha1.CoderWorkspacePoolMemberConditionWorkspaceDeleted) {
260+
st.Phase, lost = coderv1alpha1.CoderWorkspaceClaimPhaseLost, true
261+
reason, message = "WorkspaceDeleted", fmt.Sprintf("Workspace %s was deleted. The claim never binds again.", m.Status.WorkspaceName)
262+
}
219263
default:
220264
st.Phase = coderv1alpha1.CoderWorkspaceClaimPhasePending
221265
}
Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
package controller_test
2+
3+
import (
4+
"context"
5+
"errors"
6+
"testing"
7+
"time"
8+
9+
"github.com/coder/coder/v2/codersdk"
10+
"github.com/stretchr/testify/require"
11+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
12+
"k8s.io/apimachinery/pkg/types"
13+
ctrl "sigs.k8s.io/controller-runtime"
14+
"sigs.k8s.io/controller-runtime/pkg/client"
15+
16+
coderv1alpha1 "github.com/coder/coder-k8s/api/v1alpha1"
17+
"github.com/coder/coder-k8s/internal/controller"
18+
)
19+
20+
// TestPoolCreatesOnDemand: a pool at replicas 0 creates a member for each
21+
// waiting claim (decision D4: want is replicas plus the waiting claims),
22+
// binds it once it is ready, and creates no more after that.
23+
func TestPoolCreatesOnDemand(t *testing.T) {
24+
t.Parallel()
25+
e := newPoolMemberEnv(t)
26+
e.editPool(t, func(s *coderv1alpha1.CoderWorkspacePoolSpec) { s.Replicas = new(int32(0)) })
27+
e.reconcilePool(t, 1)
28+
require.Empty(t, e.poolMembers(t, e.pool.Name), "no claim, no member")
29+
30+
claimKey := e.createClaim(t, "build-1")
31+
e.reconcilePool(t, 2)
32+
members := e.poolMembers(t, e.pool.Name)
33+
require.Len(t, members, 1, "one member for the one waiting claim")
34+
requireClaim(t, e.claim(t, claimKey), coderv1alpha1.CoderWorkspaceClaimPhasePending, "WaitingForReadyWorkspace", false)
35+
36+
e.setMemberStatus(t, memberKey(members[0]), readyWithWorkspace("kwp-on-demand"))
37+
e.reconcilePool(t, 2)
38+
e.requireBound(t, memberKey(members[0]), claimKey)
39+
requireClaim(t, e.claim(t, claimKey), coderv1alpha1.CoderWorkspaceClaimPhaseBound, "WorkspaceReady", true)
40+
require.Len(t, e.poolMembers(t, e.pool.Name), 1, "no member after the claim is bound")
41+
}
42+
43+
// TestPoolDeleteKeepsBoundClaims: the claim controls a bound member, so
44+
// deleting the pool does not reach it, and its claim stays Bound. A claim
45+
// that no member holds waits with PoolNotFound, as does a claim on a pool
46+
// that never existed.
47+
func TestPoolDeleteKeepsBoundClaims(t *testing.T) {
48+
t.Parallel()
49+
e := newPoolMemberEnv(t)
50+
member := e.createMember(t, nil)
51+
e.setMemberStatus(t, member, readyWithWorkspace("kwp-kept"))
52+
bound := e.createClaim(t, "bound")
53+
time.Sleep(1100 * time.Millisecond) // creationTimestamp has second precision.
54+
waiting := e.createClaim(t, "waiting")
55+
e.reconcilePool(t, 1)
56+
e.requireBound(t, member, bound)
57+
58+
require.NoError(t, k8sClient.Delete(e.ctx, e.reconcilePool(t, 0))) // No finalizer: gone at once.
59+
r := &controller.CoderWorkspacePoolReconciler{Client: k8sClient, APIReader: k8sClient, Scheme: scheme, Clock: e.clock}
60+
_, err := r.Reconcile(e.ctx, ctrl.Request{NamespacedName: types.NamespacedName{Namespace: e.ns, Name: e.pool.Name}})
61+
require.NoError(t, err)
62+
e.requireBound(t, member, bound)
63+
claim := e.claim(t, bound)
64+
requireClaim(t, claim, coderv1alpha1.CoderWorkspaceClaimPhaseBound, "WorkspaceReady", true)
65+
require.Equal(t, "kwp-kept", claim.Status.WorkspaceName)
66+
require.Equal(t, "tester", claim.Status.OwnerName, "kept from the bind")
67+
requireClaim(t, e.claim(t, waiting), coderv1alpha1.CoderWorkspaceClaimPhasePending, "PoolNotFound", false)
68+
69+
stray := &coderv1alpha1.CoderWorkspaceClaim{}
70+
stray.Name, stray.Namespace, stray.Spec.PoolName = "stray", e.ns, "no-such-pool"
71+
require.NoError(t, k8sClient.Create(e.ctx, stray))
72+
_, err = r.Reconcile(e.ctx, ctrl.Request{NamespacedName: types.NamespacedName{Namespace: e.ns, Name: "no-such-pool"}})
73+
require.NoError(t, err)
74+
requireClaim(t, e.claim(t, types.NamespacedName{Namespace: e.ns, Name: "stray"}), coderv1alpha1.CoderWorkspaceClaimPhasePending, "PoolNotFound", false)
75+
}
76+
77+
// TestPoolClaimLostWhenWorkspaceDeleted: after the one extension at bind, a
78+
// bound member keeps reading its workspace every 60 s. Builds by the
79+
// claimant, a stop included, change nothing. A deleted workspace sets
80+
// WorkspaceDeleted=True, and the claim becomes Lost, which is final: it
81+
// never binds again (plan 6.2 step 11 rule 3).
82+
func TestPoolClaimLostWhenWorkspaceDeleted(t *testing.T) {
83+
t.Parallel()
84+
e := newPoolMemberEnv(t)
85+
e.editPool(t, func(s *coderv1alpha1.CoderWorkspacePoolSpec) { s.Replicas = new(int32(0)) })
86+
member, workspaceID := e.readyMember(t)
87+
claimKey := e.createClaim(t, "build-1")
88+
e.reconcilePool(t, 1)
89+
e.requireBound(t, member, claimKey)
90+
m := e.reconcileMember(t, member, 1)
91+
require.True(t, m.Status.BindExtended)
92+
require.Equal(t, time.Minute, e.lastStep.RequeueAfter, "a bound member keeps watching")
93+
94+
stop := e.fake.addExternalBuild(workspaceID, e.tester, codersdk.WorkspaceTransitionStop)
95+
e.fake.setBuildJob(stop.ID, codersdk.ProvisionerJobSucceeded)
96+
m = e.reconcileMember(t, member, 1)
97+
require.Equal(t, coderv1alpha1.CoderWorkspacePoolMemberPhaseReady, m.Status.Phase, "a stop by the claimant changes nothing")
98+
99+
e.fake.markDeleted(workspaceID)
100+
m = e.reconcileMember(t, member, 1)
101+
requireMemberFailed(t, m, "WorkspaceDeletedExternally", "deleted", metav1.ConditionTrue)
102+
e.reconcilePool(t, 1)
103+
claim := e.claim(t, claimKey)
104+
require.Equal(t, coderv1alpha1.CoderWorkspaceClaimPhaseLost, claim.Status.Phase)
105+
require.Equal(t, "WorkspaceDeleted", claim.Status.Reason)
106+
107+
// The failed member goes, and the cache still shows the claim without
108+
// its Lost status: a Lost claim is no demand, so no member is created
109+
// for it.
110+
e.deleteMember(t, member)
111+
require.Nil(t, e.reconcileMember(t, member, 2), "the workspace is already deleted")
112+
stale := &controller.CoderWorkspacePoolReconciler{Client: staleClaims{k8sClient}, APIReader: k8sClient, Scheme: scheme, Clock: e.clock}
113+
_, err := stale.Reconcile(e.ctx, ctrl.Request{NamespacedName: types.NamespacedName{Namespace: e.ns, Name: e.pool.Name}})
114+
require.NoError(t, err)
115+
require.Empty(t, e.poolMembers(t, e.pool.Name), "no member for a Lost claim")
116+
117+
// A ready member appears: the Lost claim never binds again.
118+
e.editPool(t, func(s *coderv1alpha1.CoderWorkspacePoolSpec) { s.Replicas = new(int32(1)) })
119+
spare := e.createMember(t, nil)
120+
e.setMemberStatus(t, spare, readyWithWorkspace("kwp-spare"))
121+
// Again on the stale cache, now with a ready member to bind.
122+
_, err = stale.Reconcile(e.ctx, ctrl.Request{NamespacedName: types.NamespacedName{Namespace: e.ns, Name: e.pool.Name}})
123+
require.NoError(t, err)
124+
require.Nil(t, e.reconcileMember(t, spare, 0).Spec.ClaimRef)
125+
e.reconcilePool(t, 1)
126+
require.Nil(t, e.reconcileMember(t, spare, 0).Spec.ClaimRef)
127+
require.Equal(t, coderv1alpha1.CoderWorkspaceClaimPhaseLost, e.claim(t, claimKey).Status.Phase)
128+
}
129+
130+
// crashAfterBind lets the first member update land, then reports an
131+
// error, as a crash right after the bind would end the reconcile.
132+
type crashAfterBind struct{ client.Client }
133+
134+
func (c crashAfterBind) Update(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error {
135+
if err := c.Client.Update(ctx, obj, opts...); err != nil {
136+
return err
137+
}
138+
if _, ok := obj.(*coderv1alpha1.CoderWorkspacePoolMember); ok {
139+
return errors.New("crash after the bind")
140+
}
141+
return nil
142+
}
143+
144+
// TestPoolOrphanClaimKeepsOwnerName: the pool crashes right after a bind,
145+
// before the claim status, and the pool is deleted before the next
146+
// reconcile. The claim still shows Bound with the owner's Coder username,
147+
// which the pool stored in the claim status before the bind.
148+
func TestPoolOrphanClaimKeepsOwnerName(t *testing.T) {
149+
t.Parallel()
150+
e := newPoolMemberEnv(t)
151+
member := e.createMember(t, nil)
152+
e.setMemberStatus(t, member, readyWithWorkspace("kwp-crash"))
153+
claimKey := e.createClaim(t, "build-1")
154+
key := types.NamespacedName{Namespace: e.ns, Name: e.pool.Name}
155+
r := &controller.CoderWorkspacePoolReconciler{Client: crashAfterBind{k8sClient}, APIReader: k8sClient, Scheme: scheme, Clock: e.clock}
156+
_, err := r.Reconcile(e.ctx, ctrl.Request{NamespacedName: key})
157+
require.ErrorContains(t, err, "crash after the bind")
158+
e.requireBound(t, member, claimKey)
159+
160+
require.NoError(t, k8sClient.Delete(e.ctx, e.reconcilePool(t, 0)))
161+
r = &controller.CoderWorkspacePoolReconciler{Client: k8sClient, APIReader: k8sClient, Scheme: scheme, Clock: e.clock}
162+
_, err = r.Reconcile(e.ctx, ctrl.Request{NamespacedName: key})
163+
require.NoError(t, err)
164+
claim := e.claim(t, claimKey)
165+
requireClaim(t, claim, coderv1alpha1.CoderWorkspaceClaimPhaseBound, "WorkspaceReady", true)
166+
require.Equal(t, "tester", claim.Status.OwnerName)
167+
require.Equal(t, "kwp-crash", claim.Status.WorkspaceName)
168+
}

‎internal/controller/coderworkspacepool_claim_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ func TestPoolBindsOldestClaimFirst(t *testing.T) {
9797
requireClaim(t, e.claim(t, newer), coderv1alpha1.CoderWorkspaceClaimPhasePending, "WaitingForReadyWorkspace", false)
9898
require.Equal(t, int32(1), pool.Status.BoundReplicas)
9999
require.Zero(t, pool.Status.ReadyReplicas, "a bound member is not ready to claim")
100-
require.Len(t, e.poolMembers(t, e.pool.Name), 2, "the pool replaces the bound member")
100+
require.Len(t, e.poolMembers(t, e.pool.Name), 3, "the pool replaces the bound member and adds one for the waiting claim")
101101
}
102102

103103
// TestPoolBindsCurrentBeforeOutdated: a claim takes a current member before

‎internal/controller/coderworkspacepool_controller.go‎

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/coder/coder/v2/codersdk"
1212
"github.com/google/uuid"
1313
"k8s.io/apimachinery/pkg/api/equality"
14+
apierrors "k8s.io/apimachinery/pkg/api/errors"
1415
"k8s.io/apimachinery/pkg/api/meta"
1516
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1617
"k8s.io/apimachinery/pkg/labels"
@@ -65,8 +66,10 @@ func (r *CoderWorkspacePoolReconciler) Reconcile(ctx context.Context, req ctrl.R
6566
// and a reconcile that a member delete starts must see the status
6667
// written right before that delete.
6768
pool := &coderv1alpha1.CoderWorkspacePool{}
68-
if err := r.APIReader.Get(ctx, req.NamespacedName, pool); err != nil {
69-
return ctrl.Result{}, client.IgnoreNotFound(err)
69+
if err := r.APIReader.Get(ctx, req.NamespacedName, pool); apierrors.IsNotFound(err) {
70+
return ctrl.Result{}, r.reconcileOrphanClaims(ctx, req.Namespace, req.Name)
71+
} else if err != nil {
72+
return ctrl.Result{}, fmt.Errorf("get pool %s: %w", req, err)
7073
}
7174
if pool.Name != req.Name || pool.Namespace != req.Namespace {
7275
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
7679
}
7780
if !pool.DeletionTimestamp.IsZero() {
7881
// Garbage collection deletes the unclaimed members. Their finalizers
79-
// run the last use check.
80-
return ctrl.Result{}, nil
82+
// run the last use check. Bound members belong to their claims.
83+
return ctrl.Result{}, r.reconcileOrphanClaims(ctx, pool.Namespace, pool.Name)
8184
}
8285

8386
before := pool.Status.DeepCopy()
@@ -114,7 +117,7 @@ func (r *CoderWorkspacePoolReconciler) Reconcile(ctx context.Context, req ctrl.R
114117
members = append(members, m)
115118
}
116119
}
117-
claims, err := r.poolClaims(ctx, pool)
120+
claims, err := r.poolClaims(ctx, pool.Namespace, pool.Name)
118121
if err != nil {
119122
return ctrl.Result{}, err
120123
}
@@ -152,16 +155,16 @@ func (r *CoderWorkspacePoolReconciler) Reconcile(ctx context.Context, req ctrl.R
152155
if err := r.bindClaims(ctx, pool, claims, all.Items, &counts, ownerName); err != nil {
153156
return ctrl.Result{}, err
154157
}
155-
step, requeue, err = r.createMembers(ctx, pool, maxWorkspaces, all.Items, &counts)
158+
step, requeue, err = r.createMembers(ctx, pool, poolWant(pool, claims, all.Items), maxWorkspaces, all.Items, &counts)
156159
if err != nil {
157160
return ctrl.Result{}, err
158161
}
159162
}
160163
// The namespace list, where trimToOwnerCap marked its deletes.
161-
if err := r.deleteUnclaimedMembers(ctx, pool, all.Items, step); err != nil {
164+
if err := r.deleteUnclaimedMembers(ctx, pool, poolWant(pool, claims, all.Items), all.Items, step); err != nil {
162165
return ctrl.Result{}, err
163166
}
164-
if err := r.writeClaimStatuses(ctx, pool, claims, all.Items, ownerName); err != nil {
167+
if err := r.writeClaimStatuses(ctx, pool.Name, false, claims, all.Items, ownerName); err != nil {
165168
return ctrl.Result{}, err
166169
}
167170
r.applyPoolStatus(pool, counts, step)
@@ -310,13 +313,12 @@ var poolConsentLostReasons = []string{"OwnerNotAllowed", "TemplateNotAllowed", "
310313
// go here. After a consent removal or an owner refusal, all other unclaimed
311314
// members go. Otherwise, new before old: outdated members that are not ready
312315
// go at once, and outdated ready members go oldest first, down to what the
313-
// current ready members do not cover yet. Current members above
314-
// spec.replicas go in this order: those without a create request, those not
316+
// current ready members do not cover yet. Current members above want
317+
// (spec.replicas plus the waiting claims) go in this order: those without a create request, those not
315318
// ready (newest first), then ready ones (oldest first).
316319
func (r *CoderWorkspacePoolReconciler) deleteUnclaimedMembers(
317-
ctx context.Context, pool *coderv1alpha1.CoderWorkspacePool, members []coderv1alpha1.CoderWorkspacePoolMember, step *coderStep,
320+
ctx context.Context, pool *coderv1alpha1.CoderWorkspacePool, want int32, members []coderv1alpha1.CoderWorkspacePoolMember, step *coderStep,
318321
) error {
319-
want := *pool.Spec.Replicas // Claims add their demand in PR 11.
320322
var current, outdatedReady, doomed []*coderv1alpha1.CoderWorkspacePoolMember
321323
var readyCurrent int32
322324
for i := range members {
@@ -419,17 +421,18 @@ func (r *CoderWorkspacePoolReconciler) freshOwnerCap(ctx context.Context, pool *
419421
}
420422

421423
// createMembers creates current members while all caps hold (plan 6.3 step
422-
// 7): current unclaimed members fewer than spec.replicas, building members
424+
// 7): current unclaimed members fewer than want (spec.replicas plus the
425+
// waiting claims, decision D4), building members
423426
// fewer than spec.maxInFlight, the owner's live members on this control
424427
// plane fewer than maxWorkspaces (from an uncached read), and the failure
425428
// backoff passed. It returns the wait that stops it, if any, and when to
426429
// read again.
427430
func (r *CoderWorkspacePoolReconciler) createMembers(
428-
ctx context.Context, pool *coderv1alpha1.CoderWorkspacePool, maxWorkspaces int32,
431+
ctx context.Context, pool *coderv1alpha1.CoderWorkspacePool, want, maxWorkspaces int32,
429432
all []coderv1alpha1.CoderWorkspacePoolMember, counts *poolMemberCounts,
430433
) (*coderStep, time.Duration, error) {
431434
st, now := &pool.Status, r.Clock.Now()
432-
need := *pool.Spec.Replicas - counts.current // Claims add their demand in PR 11.
435+
need := want - counts.current
433436
if need <= 0 {
434437
return nil, 0, nil
435438
}

0 commit comments

Comments
 (0)