Skip to content
9 changes: 7 additions & 2 deletions internal/controller/coderworkspacepoolmember_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,10 @@ func (r *CoderWorkspacePoolMemberReconciler) Reconcile(ctx context.Context, req
}
keepBackoff = step.rateLimited && !step.failed
switch {
case step.failed, member.Spec.ClaimRef != nil && member.Status.BindExtended:
// The watch rules of a bound member (plan 6.2 step 11 rule 3) come later.
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.
return ctrl.Result{}, nil
case member.Status.Phase == coderv1alpha1.CoderWorkspacePoolMemberPhaseReady:
return ctrl.Result{RequeueAfter: r.retryAfter(req.NamespacedName, member, step)}, nil
Expand Down Expand Up @@ -445,6 +447,9 @@ func applyPoolMemberStep(member *coderv1alpha1.CoderWorkspacePoolMember, now tim
switch {
case !step.failed && member.Status.ReadyTime != nil:
member.Status.Phase, ready = coderv1alpha1.CoderWorkspacePoolMemberPhaseReady, metav1.ConditionTrue
if step.reason == "AgentNotReady" {
ready = metav1.ConditionFalse // Watch rule 3: the phase stays.
}
case !step.failed && poolMemberMayHaveWorkspace(member):
member.Status.Phase = coderv1alpha1.CoderWorkspacePoolMemberPhaseCreating
case !step.failed:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,8 @@ func requireMember(t *testing.T, member *coderv1alpha1.CoderWorkspacePoolMember,
require.Equal(t, phase, member.Status.Phase, member.Status.Message)
require.Equal(t, reason, member.Status.Reason, member.Status.Message)
require.Contains(t, member.Status.Message, messagePart)
require.Equal(t, phase == coderv1alpha1.CoderWorkspacePoolMemberPhaseReady,
meta.IsStatusConditionTrue(member.Status.Conditions, coderv1alpha1.CoderWorkspacePoolMemberConditionReady), "Ready=True exactly in phase Ready")
require.Equal(t, phase == coderv1alpha1.CoderWorkspacePoolMemberPhaseReady && reason != "AgentNotReady",
meta.IsStatusConditionTrue(member.Status.Conditions, coderv1alpha1.CoderWorkspacePoolMemberConditionReady), "Ready=True exactly in phase Ready, unless an agent is not ready")
require.True(t, controllerutil.ContainsFinalizer(member, coderv1alpha1.CoderWorkspacePoolMemberCleanupFinalizer))
}

Expand Down
17 changes: 5 additions & 12 deletions internal/controller/coderworkspacepoolmember_extend.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,26 +24,19 @@ const (
)

// keepWarm runs the deadline steps of a ready member (plan 6.2
// step 10 rule 5 and step 11 rule 1). An unclaimed workspace gets a new
// step 10 rule 5 and step 11 rule 1) on workspace, which the caller just
// read and proved to be this member's. An unclaimed workspace gets a new
// deadline when its deadline is near. A bound workspace gets one extension at
// bind, so the claimant has a full TTL, and then never another. Each
// extension follows a fresh read that proves the workspace is this member's.
// bind, so the claimant has a full TTL, and then never another.
func (r *CoderWorkspacePoolMemberReconciler) keepWarm(
ctx context.Context, sdk *codersdk.Client, member *coderv1alpha1.CoderWorkspacePoolMember, before *coderv1alpha1.CoderWorkspacePoolMemberStatus, now time.Time,
ctx context.Context, sdk *codersdk.Client, member *coderv1alpha1.CoderWorkspacePoolMember, before *coderv1alpha1.CoderWorkspacePoolMemberStatus,
workspace codersdk.Workspace, now time.Time,
) (*coderStep, error) {
name, bound := member.Status.WorkspaceName, member.Spec.ClaimRef != nil
if bound && member.Status.BindExtended {
// The one extension at bind is done: Coder's own autostop applies.
return coderWait(member.Status.Reason, "%s", member.Status.Message), nil
}
reason := "WorkspaceReady" // A successful read replaces an earlier wait.
if bound {
reason = "Bound"
}
workspace, step, err := readOwnPoolMemberWorkspace(ctx, sdk, member)
if step != nil || err != nil {
return step, err
}
build := workspace.LatestBuild
deadline, target := build.Deadline, member.Status.BindExtendDeadline
switch {
Expand Down
63 changes: 41 additions & 22 deletions internal/controller/coderworkspacepoolmember_ready.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func (r *CoderWorkspacePoolMemberReconciler) readWorkspace(
case member.Status.ReadyTime == nil:
return checkPoolMemberReadiness(ctx, sdk, member, now)
default:
return r.keepWarm(ctx, sdk, member, before, now)
return r.watchReady(ctx, sdk, member, before, now)
}
}

Expand Down Expand Up @@ -142,29 +142,40 @@ func checkPoolMemberReadiness(
return coderWait("WorkspaceReady", "Every agent of workspace %s is ready.", name), nil
}

// readOwnPoolMemberWorkspace reads the member's workspace and checks its
// identity (plan 6.2, identity guard) against values that no status writer
// can change: the owner from the immutable spec, the kwp- name derived from
// the UID, and the template. It also requires that the latest build is the
// start build, and that the operator user started it with the pinned version:
// a status writer can name another build or another kwp- workspace, but not
// one that the operator built for this member. A nil step means the workspace
// is this member's own.
// readOwnPoolMemberWorkspace reads the member's workspace, proves it is the
// member's own, and requires that the latest build is the start build
// (readPoolMemberWorkspace and poolMemberStartBuildStep). A nil step means
// the workspace is this member's own and still runs its start build.
func readOwnPoolMemberWorkspace(
ctx context.Context, sdk *codersdk.Client, member *coderv1alpha1.CoderWorkspacePoolMember,
) (codersdk.Workspace, *coderStep, error) {
workspace, operatorID, step, err := readPoolMemberWorkspace(ctx, sdk, member)
if step != nil || err != nil {
return workspace, step, err
}
return workspace, poolMemberStartBuildStep(member, workspace, operatorID), nil
}

// readPoolMemberWorkspace reads the member's workspace and checks its
// identity (plan 6.2, identity guard) against values that no status writer
// can change: the owner from the immutable spec, the kwp- name derived from
// the UID, and the template. It also returns the operator user's ID. A nil
// step means the workspace is this member's own.
func readPoolMemberWorkspace(
ctx context.Context, sdk *codersdk.Client, member *coderv1alpha1.CoderWorkspacePoolMember,
) (codersdk.Workspace, uuid.UUID, *coderStep, error) {
spec, name := member.Spec, member.Status.WorkspaceName
workspaceID, err := uuid.Parse(member.Status.WorkspaceID)
if err != nil || workspaceID == uuid.Nil || member.Status.StartBuildID == "" {
return codersdk.Workspace{}, nil, fmt.Errorf("assertion failed: pool member %s/%s has workspace ID %q and start build ID %q",
return codersdk.Workspace{}, uuid.Nil, nil, fmt.Errorf("assertion failed: pool member %s/%s has workspace ID %q and start build ID %q",
member.Namespace, member.Name, member.Status.WorkspaceID, member.Status.StartBuildID)
}
operator, err := sdk.User(ctx, codersdk.Me)
if err != nil {
return codersdk.Workspace{}, coderStepUnavailable("get operator user", err), nil
return codersdk.Workspace{}, uuid.Nil, coderStepUnavailable("get operator user", err), nil
}
if operator.ID == uuid.Nil {
return codersdk.Workspace{}, nil, &coderAnswerError{msg: "assertion failed: Coder answered the operator user without an ID"}
return codersdk.Workspace{}, uuid.Nil, nil, &coderAnswerError{msg: "assertion failed: Coder answered the operator user without an ID"}
}
workspace, err := sdk.Workspace(ctx, workspaceID)
// Only 410 proves deletion: Coder also answers 404 when the caller may
Expand All @@ -173,27 +184,35 @@ func readOwnPoolMemberWorkspace(
case coderStatus(err) == http.StatusGone:
step := coderFail("WorkspaceDeletedExternally", "Workspace %s was deleted by someone else.", name)
step.deleted = &metav1.Condition{Status: metav1.ConditionTrue, Reason: "DeletedExternally", Message: step.message}
return workspace, step, nil
return workspace, operator.ID, step, nil
case err != nil:
return workspace, coderStepUnavailable("get workspace", err), nil
return workspace, operator.ID, coderStepUnavailable("get workspace", err), nil
}
build := workspace.LatestBuild
if err := errors.Join(coderAnswerFor("workspace", workspaceID.String(), workspace.ID.String()),
coderAnswerFor("latest build workspace", workspaceID.String(), build.WorkspaceID.String())); err != nil {
return workspace, nil, err
coderAnswerFor("latest build workspace", workspaceID.String(), workspace.LatestBuild.WorkspaceID.String())); err != nil {
return workspace, operator.ID, nil, err
}
if workspace.OwnerID.String() != spec.OwnerID || workspace.Name != name || workspace.TemplateID.String() != spec.TemplateID {
step := coderFail("OwnershipUnknown", "Workspace %s is not the workspace of this member.", workspaceID)
step.deleted = &metav1.Condition{Status: metav1.ConditionUnknown, Reason: "OwnershipUnknown", Message: step.message}
return workspace, step, nil
return workspace, operator.ID, step, nil
}
return workspace, operator.ID, nil, nil
}

// poolMemberStartBuildStep requires that the latest build of the member's
// workspace is its start build, and that the operator user started it with
// the pinned version: a status writer can name another build or another
// kwp- workspace, but not one that the operator built for this member.
func poolMemberStartBuildStep(member *coderv1alpha1.CoderWorkspacePoolMember, workspace codersdk.Workspace, operatorID uuid.UUID) *coderStep {
build, name := workspace.LatestBuild, member.Status.WorkspaceName
if build.ID.String() != member.Status.StartBuildID {
return workspace, coderFail("ChangedExternally", "Someone else started build %d of workspace %s.", build.BuildNumber, name), nil
return coderFail("ChangedExternally", "Someone else started build %d of workspace %s.", build.BuildNumber, name)
}
if build.InitiatorID != operator.ID || build.Transition != codersdk.WorkspaceTransitionStart || build.TemplateVersionID.String() != spec.TemplateVersionID {
if build.InitiatorID != operatorID || build.Transition != codersdk.WorkspaceTransitionStart || build.TemplateVersionID.String() != member.Spec.TemplateVersionID {
step := coderFail("OwnershipUnknown", "Nothing proves that the controller started build %d of workspace %s.", build.BuildNumber, name)
step.deleted = &metav1.Condition{Status: metav1.ConditionUnknown, Reason: "OwnershipUnknown", Message: step.message}
return workspace, step, nil
return step
}
return workspace, nil, nil
return nil
}
131 changes: 131 additions & 0 deletions internal/controller/coderworkspacepoolmember_watch.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
package controller

import (
"context"
"fmt"
"slices"
"time"

"github.com/coder/coder/v2/codersdk"
"github.com/google/uuid"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"

coderv1alpha1 "github.com/coder/coder-k8s/api/v1alpha1"
)

const (
// poolMemberAgentLostAfter is how long an agent of a ready unclaimed
// workspace can stay not ready before the member fails.
poolMemberAgentLostAfter = 5 * time.Minute
// poolMemberTemplateWriteWindow: a last_used_at this close to the
// template's updated_at comes from a template schedule update.
poolMemberTemplateWriteWindow = 5 * time.Second
)

// watchReady runs the rules of a ready member on one read of its workspace:
// the watch rules of an unclaimed member in plan order (6.2 step 10), and
// the deadline steps (step 10 rule 5, step 11 rule 1).
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.
return coderWait(member.Status.Reason, "%s", member.Status.Message), nil
}
workspace, operatorID, step, err := readPoolMemberWorkspace(ctx, sdk, member)
if step != nil || err != nil {
return step, err
}
name := member.Status.WorkspaceName
if !bound {
// Rule 1 runs first, so the later rules never fail a used member.
if step, err := r.checkPoolMemberUse(ctx, sdk, member, workspace, now); step != nil || err != nil {
return step, err
}
}
if step := poolMemberStartBuildStep(member, workspace, operatorID); step != nil {
return step, nil // Rule 2: a stop, an update, or an autostop.
}
if !bound {
if step := poolMemberAgentStep(member, workspace.LatestBuild, now); step != nil {
return step, nil // Rule 3.
}
if workspace.DormantAt != nil || workspace.DeletingAt != nil {
return coderFail("Dormant", "Workspace %s is dormant.", name), nil // Rule 4.
}
}
return r.keepWarm(ctx, sdk, member, before, workspace, now)
}

// checkPoolMemberUse applies watch rule 1: a last_used_at later than the
// baseline from the ready read means that someone used the workspace without
// a claim. A template-wide write is not use: it moves the baseline instead.
// A nil step means the member is still unused.
func (r *CoderWorkspacePoolMemberReconciler) checkPoolMemberUse(
ctx context.Context, sdk *codersdk.Client, member *coderv1alpha1.CoderWorkspacePoolMember, workspace codersdk.Workspace, now time.Time,
) (*coderStep, error) {
baseline := member.Status.ReadyLastUsedAt
if baseline == nil {
return nil, fmt.Errorf("assertion failed: ready pool member %s/%s has no readyLastUsedAt", member.Namespace, member.Name)
}
// Status stores whole seconds. Both times come from Coder's clock.
lastUsed := workspace.LastUsedAt.Truncate(time.Second)
if !lastUsed.After(baseline.Time) {
return nil, nil
}
template, err := sdk.Template(ctx, uuid.MustParse(member.Spec.TemplateID))
if err != nil {
return coderStepUnavailable("get template", err), nil
}
if err := coderAnswerFor("template", member.Spec.TemplateID, template.ID.String()); err != nil {
return nil, err
}
// A template schedule update sets one last_used_at on every workspace of
// the template, and moves the template's updated_at (plan 3.1 item 7).
templateWide := lastUsed.Sub(template.UpdatedAt).Abs() <= poolMemberTemplateWriteWindow
if !templateWide {
// The template can change again after the write. Another unclaimed
// member of the pool that moved its baseline to the same value
// shows the same write. This member's own baseline is earlier, so
// it never matches.
members := &coderv1alpha1.CoderWorkspacePoolMemberList{}
label := coderv1alpha1.CoderWorkspacePoolSelectorLabel
if err := r.List(ctx, members, client.InNamespace(member.Namespace), client.MatchingLabels{label: member.Labels[label]}); err != nil {
return nil, fmt.Errorf("list members of pool %s/%s: %w", member.Namespace, member.Spec.PoolName, err)
}
templateWide = slices.ContainsFunc(members.Items, func(m coderv1alpha1.CoderWorkspacePoolMember) bool {
return m.Spec.ClaimRef == nil && m.Status.ReadyLastUsedAt != nil && m.Status.ReadyLastUsedAt.Time.Equal(lastUsed)
})
Comment thread
ThomasK33 marked this conversation as resolved.
}
if templateWide {
member.Status.ReadyLastUsedAt = &metav1.Time{Time: lastUsed}
return nil, nil
}
step := coderWait("Used", "Someone used workspace %s at %s without a claim. The pool releases it.", member.Status.WorkspaceName, lastUsed.UTC().Format(time.RFC3339))
setPoolMemberCondition(member, now, coderv1alpha1.CoderWorkspacePoolMemberConditionUsed, metav1.ConditionTrue, step.reason, step.message)
return step, nil
}

// poolMemberAgentStep applies watch rule 3 to the start build: an agent that
// is no longer connected and ready makes the member not ready, and after
// poolMemberAgentLostAfter it fails. A nil step means every agent is ready.
func poolMemberAgentStep(member *coderv1alpha1.CoderWorkspacePoolMember, build codersdk.WorkspaceBuild, now time.Time) *coderStep {
step := workspaceBuildReadiness(build, member.Status.WorkspaceName)
if step == nil {
return nil
}
since := now
if c := meta.FindStatusCondition(member.Status.Conditions, coderv1alpha1.CoderWorkspacePoolMemberConditionReady); c != nil &&
c.Status == metav1.ConditionFalse && c.Reason == "AgentNotReady" {
since = c.LastTransitionTime.Time
}
if now.Sub(since) >= poolMemberAgentLostAfter {
return coderFail("AgentLost", "%s It has not been ready since %s.", step.message, since.UTC().Format(time.RFC3339))
}
return coderWait("AgentNotReady", "%s", step.message)
}
Loading
Loading