Conversation
Add golden tests for the identity the controller derives from a WorkerDeployment with one pod template: build IDs for tagged, digest, untagged and missing images, the deprecated template field, a cleaned unsafeCustomBuildID, the Temporal deployment name, versioned Deployment names (short and truncated), selector labels, and a fully rendered Deployment's labels, annotations and injected env. Worker pools will add a pool dimension to these functions. These goldens make sure CRs without pools keep the same build ID, Deployment name and selector, so upgrading the controller never renames live Deployments or starts a rollout. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A draining version whose Deployment is at 0 replicas has no pollers and can never finish draining, so the planner scales it back up. That path still read the deprecated top-level spec.replicas. A WorkerDeployment that sets replicas through spec.deployment was therefore scaled to 1 instead of its configured count. Read replicas from DeploymentSpec(), like every other scaling path. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Prepare the controller for versions that run more than one Kubernetes Deployment, one per worker pool. A Deployment's pool comes from the new temporal.io/worker-pool label; Deployments without it are the default pool, so every existing Deployment keeps its meaning. There is no API change yet, so the controller still only creates default pools. - DeploymentState gains PoolDeployments (build ID -> pool -> Deployment) plus VersionDeployments, VersionDeploymentList and BuildIDs helpers. Deployments and DeploymentRefs now hold only the default pool, so a named pool can never stand in for it. - Whether a version exists is decided from all of its pools. A version whose default Deployment is gone but whose named pools remain is still listed in status, scaled, and cleaned up. - Sunset scales every pool of a deprecated version and deletes them together, only once every pool meets the zero-replica preconditions. The default pool is deleted last. - deleteDeprecatedVersions calls DeleteVersion once per build ID, then deletes all of that build's Deployments, keeping the existing all-or-nothing behaviour per build. - A version is healthy once every pool's Deployment is Available, since the latest of them became Available. EligibleForDeletion requires every pool to have no pods. - Connection drift updates every pool of the target and current versions. Updates are tracked per Deployment rather than per build ID. - GetWorkerDeploymentState takes every build ID with a Deployment, so its fallback DescribeWorkerDeploymentVersion also covers builds that only have named pools left. Otherwise such a build would map to NotRegistered and be deleted. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Add spec.pools: a list of named worker pools, each with its own
appsv1.DeploymentSpec. The existing spec.deployment stays as the
implicit "default" pool. All pools will share the WorkerDeployment's
Temporal deployment name and build ID, so their task queues sit in one
Worker Deployment Version and roll out together while each pool keeps
its own pod template, replicas and placement. The controller does not
act on pools yet; that follows in the next change.
API:
- WorkerPool{name, deployment}. Names are DNS labels of at most 24
characters, "default" is reserved, and the list is a map keyed by name
with at most 10 entries.
- CEL requires spec.deployment when pools are set, so pools never mix
with the deprecated top-level template fields.
- Helpers: DefaultPoolName, HasPools, PoolNames (default first, then
named pools sorted) and PoolDeploymentSpec, which applies the default
rolling update strategy like DeploymentSpec does.
- Status: BaseWorkerDeploymentVersion gains pools[]{name, deployment,
healthySince}. The deprecated TemporalWorkerDeployment CRD shares these
status types, so its schema gains the field too (additive).
- The WorkerDeployment webhook warns when a pool sets a selector, since
the controller computes it.
CRD generation: hack/patch-workerdeployment-crd-schema.py now drops
selector from every embedded DeploymentSpec's required list with an
indentation-preserving regex, and fails unless it patches exactly the
two expected places. The old fixed markers would have matched only
spec.deployment and silently left pools[].deployment.selector required.
The WorkerDeployment CRD grows from 414 KB to 649 KB, still well under
etcd's 1.5 MB object limit.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Act on spec.pools. Every pool of a WorkerDeployment gets its own Kubernetes Deployment per version, and all of them share the Temporal deployment name and build ID, so they ramp, promote, roll back and sunset together while each pool scales on its own. Identity: - With pools, the build ID suffix is the first 10 hex characters of a sha256 over every pool's name and pod template (the default pool first, then named pools sorted). A change to any pool's template, or adding, removing or renaming a pool, starts one new version for all pools. Reordering pools or changing replicas does not. The suffix is wider than the 4-digit hash used for single templates because a named-pool-only change depends on it alone. The image prefix still comes from the default pool, and unsafeCustomBuildID still wins. - Named pool Deployments are named <wd>-<pool>-<build>, truncated to 47 characters and always ending in an 8-hex hash of the triple, so they never take another pool's or a default pool's name. - Every Deployment of a multi-pool version carries temporal.io/worker- pool in its labels and selector, the default pool included, so pool selectors never overlap. That keeps HPA pod metrics and PDBs scoped to one pool. Single-pool versions are unchanged (pinned by goldens). - Controller-injected env is identical across pools. Planner: - The target version gets a Deployment for each spec pool it lacks. The version cap applies only to a brand-new version. Whether the default pool is labelled follows the build's existing Deployments first, then the spec. - Adding pools under the same unsafeCustomBuildID to a version whose default Deployment predates pools is refused: its immutable selector would overlap the new pools. Only those creates are skipped; the rest of the plan runs, and the controller sets Progressing=False with reason InvalidSpec and a Warning event. - Pools removed from the spec under a stable build ID are deleted from the target version through DeletePoolDeployments, which never calls DeleteVersion. - Current and target versions scale each pool to that pool's replicas. A pool no longer in the spec is left alone. Sunset rules use each pool's own replicas. - Pod-template drift under a custom build ID rebuilds a pool from its own template, keeping its selector. Strategy sync runs per pool. - Gate workflows wait until a multi-pool target is healthy, so a gate's activities never reach a pool queue that is not in the version yet. Status: - Versions with pool labels report pools[] sorted by name, each with its Deployment and when it became Available. - A multi-pool target is healthy only when every spec pool's Deployment exists, is Available, and has at least one available replica, unless that pool is scaled to 0 on purpose. Available alone is true at zero replicas, which would pass a pool that never polled. - An Inactive multi-pool target that is waiting on pools reports WaitingForPollers and names the unavailable pools. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Add spec.pool to WorkerResourceTemplate so a per-version HPA, KEDA ScaledObject or PDB can target one pool's versioned Deployments. Omitting it targets the default pool, so existing WRTs behave exactly as before. A WRT still renders one resource per build ID, so its status stays keyed by build ID. - Rendering covers only builds that have a Deployment for the WRT's pool, and scaleTargetRef points at that pool's Deployment. - Injected selector labels add temporal.io/worker-pool when the target Deployment's selector has it, so a PDB selects only its pool's pods. Single-pool output is unchanged, so LastAppliedHash does not churn. - Rendered copies are deleted per pool. Removing one pool from a version, or sunsetting a version, deletes only the copies of WRTs that target those pools, and orphaned status entries are matched against the WRT's own pool. - A WRT whose pool no version has and the WorkerDeployment spec does not declare gets Ready=False with the new reason PoolNotFound. It gets no applies or deletes, so the controller writes that status itself, and only when the condition changes. - The WRT webhook warns when spec.pool is not declared by the referenced WorkerDeployment. It is a warning rather than an error because the pool may be added to the WorkerDeployment after the WRT. - spec.pool is mutable. Changing it re-renders onto the new pool's Deployments and prunes copies on builds that lack it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Add an integration scenario against a real Temporal dev server for a WorkerDeployment with a default pool that polls a workflow queue and an "activities" pool that polls a separate activity-only queue: - v1 gets one Deployment per pool, each with a pool selector. - v1 is not promoted while only the default pool is available, and the Progressing condition names the activities pool. It is promoted once both pools are available. - Temporal reports both pools' task queues, with their types, in the same Worker Deployment Version. - A WorkerResourceTemplate with pool: activities renders an HPA that targets only the activities pool's Deployment. - A new image rolls out as v2 across both pools. - A workflow pinned to v1 and one pinned to v2 each run their activity on the activities pool worker of their own build. With one CR per role the activity would run on the activities deployment's current build. - Once v1 drains, both of its Deployments, its Temporal version and its HPA copy are deleted, while v2's Deployments stay. Test workers gain an activity-only role (TEMPORAL_TEST_WORKER_ROLE) and a crossPoolWorkflow that runs an activity on a given queue and returns the build ID of the worker that ran it. Checked that the scenario fails when the multi-pool health rule is disabled: v1 is then promoted while its activities pool is down. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Add docs/worker-pools.md: - when to use pools instead of one WorkerDeployment per role - a worked example and how Build IDs, Deployment names, labels and status work with pools - how rollouts wait for every pool, what Temporal's missing-task-queue check does and doesn't cover, and why readiness probes should pass only once a worker polls - which pools run the gate workflow - per-pool scaling through WorkerResourceTemplate.spec.pool, with notes on backlog metrics, KEDA minReplicaCount and scale-to-zero pools - how pool changes interact with unsafeCustomBuildID - limits, and removing pools before downgrading to a controller release without them Point to it from concepts, architecture, configuration, limits, the WorkerResourceTemplate reference and the docs index. Add examples/worker-pools.yaml and examples/wrt-hpa-pool.yaml; both decode strictly into the API types. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
scaleDeprecatedDeployment moved out of getScaleDeployments when scaling became per pool. As new code it tripped the repo's lint gates: cognitive complexity 37 (limit 25) and a non-exhaustive switch over VersionStatus. Split the scale-to-spec and draining scale-up rules into their own helpers, and add an explicit default case for NotRegistered and Created versions, which are left alone. Behaviour is unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The full integration run showed the Go SDK (v1.46) starts its workflow poller even when a worker registers no workflows. The test's "activity-only" pool therefore also registered its queue as a workflow queue in the version. The assertion keyed queues by name only, so it passed or failed depending on the order Temporal returned them. This matters outside tests too. The gate starts a workflow on every workflow queue in the target version, so a real activity-only pool left at SDK defaults would get a gate workflow no worker can run, and the rollout would stall. - Test workers in the activities role set DisableWorkflowWorker. - The scenario checks exact (queue, type) pairs and fails if the activities queue is registered as a workflow queue. - docs/worker-pools.md tells activity-only pools to turn off workflow polling (worker.Options.DisableWorkflowWorker in Go). - The scenario is split into step methods on a poolScenario, with ctx first and no redundant +build tag, to satisfy the repo's lint. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Two issues from review of the worker pools series. A WorkerDeployment blocked by BlockedReason (pools added under the same unsafeCustomBuildID) rewrote its status and emitted events on every reconcile. syncConditions first set Progressing from the rollout state, then the blocked override flipped it back. Each flip reset lastTransitionTime, so the status never matched the stored one. When the target was current, the poller check also saw the stored InvalidSpec as a change and emitted an ActivePollers event each time. syncConditions now takes the blocked reason, sets Progressing once, and emits the InvalidSpec event only when the condition changes. A target pool that was Available with no available replicas showed a healthySince in status. Only the version as a whole was held back. So the Progressing condition found no pending pool and said the target was waiting for promotion, with every pool looking healthy. That is the KEDA scale-to-zero stall the docs warn about. The target's pools[] now use the same readiness rule as the version (Available plus at least one available replica, unless the pool is scaled to 0 on purpose), so the condition names the stuck pool. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- The parse pool had only a GPU limit, so the CPU utilization HPA in the docs and examples/wrt-hpa-pool.yaml could never compute utilization. Give it a CPU request. - The stricter health rule applies to the target version, and the controller only promotes under AllAtOnce and Progressive. - Pools are named in Progressing only once the target is registered. - concepts.md now gives the named pool Deployment name pattern. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
With unsafeCustomBuildID set, the planner detects pod-template drift by comparing the hash stored on the Deployment with the hash of the user's template. NewDeploymentWithOwnerRef stores the hash of the user's template. updateDeploymentWithPodTemplateSpec stored the hash after injecting the TEMPORAL_* env vars and TLS mounts. After one drift update the hashes could never match again, so every reconcile sent a no-op Update to the Deployment. With worker pools this repeats for each pool of the target version. Hash the template before applying controller modifications, matching creation. This bug predates worker pools. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
When adding pools under the same unsafeCustomBuildID was refused, only Progressing was set to InvalidSpec. Ready still followed the rollout state, so a current version showed Ready=True/RolloutComplete next to Progressing=False/InvalidSpec, and alerts on Ready never fired. An Inactive version also listed pools as pending that would never be created. ReasonInvalidSpec is documented as Ready=False and Progressing=False. syncConditions now sets both conditions and returns before the rollout switch when the plan is blocked. It emits the event only when either condition changes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The stricter multi-pool health rule (each pool needs an available replica) also applied when the target was already current. If an autoscaler scaled one of the current version's pools to zero, the target's healthySince went nil. getVersionConfigDiff returns early on that before it reaches the branch that clears a ramp left over from a rolled-back rollout, so the old ramping version kept receiving new workflows. The rule exists to gate promotion, so it now applies only while the target is not yet current. A current target keeps the plain all-pools-Available check. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
PoolNotFound was written in a separate pass after executeWRTOperations. That pass had three problems: - It never ran when any other WRT's apply failed, so the missing pool was never reported. - It could only set the condition, so a WRT whose pool was declared again, but whose Deployment didn't exist yet (version cap, or a blocked pool change), kept a stale "has no pool". - It wrote status again for a WRT executeWRTOperations had just updated. A conflict, or a WRT deleted in between, failed the whole reconcile with PlanExecutionFailed. The planner now reports WRTs with a missing pool and WRTs still marked PoolNotFound whose pool is known again. executeWRTOperations folds both into its single per-WRT status write. A missing pool sets Ready=False/PoolNotFound, and a recovered pool with nothing applied yet drops the stale condition. It writes only when the condition changes, whatever happened to other WRTs, and skips a WRT that no longer exists. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The scenario left its WorkerDeployment, WorkerResourceTemplate and Connection in the shared test namespace. The WorkerDeployment kept reconciling against the short-TTL Temporal server while the deletion tests ran next on the same server and namespace. It now deletes all three, after every worker is stopped, and waits until they are gone. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Cleanups from a reuse, simplification, efficiency and altitude review. No behaviour change: the identity goldens pass, and pool Deployment names, WRT resource names and pool build IDs match a snapshot taken before the change. - One construction path: NewDeploymentWithOwnerRef delegates to NewPoolDeploymentWithOwnerRef for the unlabelled default pool, the nine-argument helper folds into it, and NewDeploymentWithControllerRef is gone. - ComputePoolDeploymentName and ComputeWorkerResourceTemplateName share hashSuffixedName. Both build IDs share imagePrefixedBuildID and use HashString. - computePoolsBuildID reads pod templates directly instead of deep-copying every pool's spec. - k8s.NewDeploymentState indexes Deployments for GetDeploymentState and replaces two identical test helpers. - k8s.HasPoolLabel is the one definition of a multi-pool version, used by the planner's create/block decision and by status. - WorkerDeploymentSpec.HasPool answers existence checks without copying a pod template. - getCreateDeploymentPools returns three values instead of a single-use struct. - The current-version scale loop reuses scaleToSpecReplicas. - BuildIDs is computed once per WRT render pass, not once per WRT. - Target pool health is computed once per pool, and version health and target health share latestOrNil. - checkAndUpdateDeploymentConnectionSpec, now test-only, is removed; its tests call updateDeploymentConnectionIfStale. - SetTaskQueue and SetWorkerRole share one env upsert that returns a copy, so callers' templates are no longer changed through a shared slice. - Long comments are trimmed to two lines. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
|
Replace the implicit "default" pool with named pools only. A WorkerDeployment now sets exactly one of spec.template, spec.deployment or spec.pools. In a pooled WorkerDeployment every pool is named and equal, so nothing suggests that parent workflows belong in spec.deployment and everything else in pools. CRs without pools are unchanged. - CEL: one of deployment, template or pools must be set, and pools can't be combined with either. pools needs 1 to 10 entries. "default" stays reserved for WorkerDeployments without pools. - Helpers: in a pooled spec, PoolNames, HasPool and PoolDeploymentSpec cover only the named pools. A spec without pools has the single default pool, as before. - Build ID: hashes every pool's name and template in name order. The image prefix comes from the first pool by name, so reordering pools never changes the build ID. - Deployments: every pool's Deployment is <wd>-<pool>-<build>-<hash> and carries temporal.io/worker-pool in its selector. The labelDefault parameter is gone. - Under a fixed unsafeCustomBuildID, switching between spec.deployment and spec.pools is refused in either direction. Nothing is created or deleted, and the controller reports InvalidSpec until the build ID changes, since the old and new selectors would overlap. - WorkerResourceTemplate: a WRT on a pooled WorkerDeployment must set spec.pool. Without it, the WRT gets Ready=False/PoolNotFound with "set spec.pool", and the webhook warns. - Status: a pooled version reports pools[] and no singular deployment. - Docs, examples and the integration scenario now use only named pools (workflows and activities). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Run several worker pools in one WorkerDeployment. Each pool has its own Deployment spec, and all pools share one Temporal version.
Problem. A workflow on the
ordersqueue calls an activity on thepaymentsqueue. With one CR per role,paymentsis a separate version, so during a ramp a v2 workflow's activity can run on v1.Fix. Run both roles as pools of one
ordersCR. They share the build ID, so the activity runs on v2. Each pool keeps its own pods, replicas and autoscaler.flowchart LR wd["WorkerDeployment orders"] wd --> v2 wd --> v1 subgraph v2 ["Version v2 (current)"] w2["Deployment orders-workflows-v2<br/>workflows pool, orders queue"] p2["Deployment orders-payments-v2<br/>payments pool, payments queue"] end subgraph v1 ["Version v1 (draining)"] w1["orders-workflows-v1"] p1["orders-payments-v1"] end k["KEDA ScaledObject<br/>one per pool per version"] -.-> p2What the controller creates from this
For build
v2-2d5bea39fd. Owner references, hash annotations and the default strategy are left out.Temporal sees one version,
prod/orders:v2-2d5bea39fd, with theordersqueue (workflow) and thepaymentsqueue (activity). While v1 drains, the same set exists for v1.Notes:
spec.poolsorspec.deployment. CRs without pools are unchanged.DisableWorkflowWorker).Draft to make the shape discussed in #330 concrete. Unit tests,
make lintand the full integration suite pass.🤖 Generated with Claude Code