feat(azure,gcp): give a cell's node pools a warm start - #103
Open
IliaFeldgun wants to merge 2 commits into
Open
Conversation
Weekly e2e 33289951671 for the timings, every byoc cell in dd for the sizes.
One row a cloud, for the `default` pool this module creates unless a program
asks for more; every count is a pool total:
pool start ceiling most install job
was/now was/now used
aws <prefix>-default-* 3 / 3 10 / 12 - fits 1800s
gcp pc-gke-np-default-* 2 / 3 20 / 12 9 1413s
azure default 1 / 3 10 / 12 6 1831s, exit 137
cold [n] --round-- [n n] --round-- [n n n] a VMSS or MIG create
and a cold image pull
for every round
warm [n n n] bought with the cluster
Most-used is the largest that pool has been on any cell in thirty days, from
kubernetes_state.node.cpu_capacity per node. aws tags neither node group nor
instance type, so its pool cannot be told from its cell, and its numbers are
unchanged here anyway.
aws node groups are born at desired_size=3, so `pinetools cluster install`
begins with capacity. AKS has no desired size and `aks.py` passed
`count=min_count`, so azure's only pool started at a single node, and the
GKE pool left `initial_node_count` unset. What the install job needed was
Pending, and the autoscaler bought it in serial rounds - which is where
azure spent the first twenty minutes of a 1800s deadline and was SIGKILLed,
in both weekly runs. `desired_size` moves out of the AWS-only block, and
both clouds read it through `initial_count()`, clamped to the pool's own min
and max.
Three or four nodes is also the whole of it: each cell runs three more pools
that pinetools creates through the operator - fdb, qroutersmol, indexbuilder
- so ours is three or four nodes of eleven. A 3-node seed is most of azure's
gap to the four it had reached when the deadline killed it, and exactly the
three gcp settles on.
GKE counted per zone, so `min_size=1, max_size=10` was really a floor of two
and a ceiling of twenty, and a seed would have had to be divided to land on
three. It takes total limits instead - `total_min_node_count` and
`total_max_node_count` with a `location_policy`, which GKE documents as
totals where `min_node_count` is per zone - so every size is a pool total on
all three clouds and `initial_node_count` takes `initial_count()` whole. Per
zone was the house pattern when 46371fb wrote this module's GCP support;
pinecone-io/iac moved to total limits in July, in 67fee389, and nothing
carried it back.
The ceiling is 12 on every cloud now, three above the nine a gcp pool has
actually used. gcp is the one that moves: it read as ten a zone and now
means twelve, so a gcp pool above twelve would be scaled to it, and its
floor drops from two nodes to one. aws and azure only gain two.
Neither seed is ours after the pool exists. AKS `count` and GKE
`initialNodeCount` join `ignore_changes`, the way `eks.py` already ignores
`scalingConfig.desiredSize` - and on GKE it matters more than drift, because
a changed `initialNodeCount` replaces the node pool.
Reading one field in three places is what made this a three-cloud edit, so
the NodePool to NodePoolConfig translation moves to `common/node_pool.py`,
carrying over every field the two shapes share a name with. It also settles
the taints: aws built a `NodePoolTaint` from any object with key, value and
effect, while azure and gcp passed the caller's own objects into a pydantic
list that takes only a dict or a `NodePoolTaint`, so tainting one of their
pools raised a ValidationError before reaching a cloud. The public
`NodePool` dataclasses stay per-cloud, each showing only the machine-size
field its cloud reads, because a shared base would reorder positional args
in a published API.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b1e02f9. Configure here.
The provider is explicit and does not except total limits: initial_node_count is "the initial number of nodes for the pool. In regional or multi-zonal clusters, this is the number of nodes per zone." Handing it a pool total, as the parent commit did, asks a two-zone cell for six nodes, not three - twice the seed, and above max_size for any pool whose total is under the product. So the seed divides even though the limits are totals: 3 over two zones is 2 a zone, four nodes, one above the three a gcp pool settles on and inside a ceiling of twelve. `initial_count_per_zone` also caps itself at max_size over zones, so a caller who narrows a pool cannot seed past its own total, and floors at one so a pool with fewer nodes than zones still starts. Caught by cursor on #103, against a paragraph in the parent commit that said the seed was taken whole - the ambiguity was named there and left unresolved instead of being read. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
hanik-m
approved these changes
Sep 3, 2026
IliaFeldgun
had a problem deploying
to
azure-e2e
September 3, 2026 14:11 — with
GitHub Actions
Failure
Collaborator
Author
|
Azure still flaky, retriaging. |
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.

Weekly e2e 33289951671 for the timings, every byoc cell in dd for the sizes. One row a cloud, for the
defaultpool this module creates unless a program asks for more; every count is a pool total:aws -default-* 3 / 3 10 / 12 - fits 1800s
gcp pc-gke-np-default-* 2 / 3 20 / 12 9 1413s
azure default 1 / 3 10 / 12 6 1831s, exit 137
cold [n] --round-- [n n] --round-- [n n n] a VMSS or MIG create
and a cold image pull
for every round
warm [n n n] bought with the cluster
Most-used is the largest that pool has been on any cell in thirty days, from kubernetes_state.node.cpu_capacity per node. aws tags neither node group nor instance type, so its pool cannot be told from its cell, and its numbers are unchanged here anyway.
aws node groups are born at desired_size=3, so
pinetools cluster installbegins with capacity. AKS has no desired size andaks.pypassedcount=min_count, so azure's only pool started at a single node, and the GKE pool leftinitial_node_countunset. What the install job needed was Pending, and the autoscaler bought it in serial rounds - which is where azure spent the first twenty minutes of a 1800s deadline and was SIGKILLed, in both weekly runs.desired_sizemoves out of the AWS-only block, and both clouds read it throughinitial_count(), clamped to the pool's own min and max.Three or four nodes is also the whole of it: each cell runs three more pools that pinetools creates through the operator - fdb, qroutersmol, indexbuilder
GKE counted per zone, so
min_size=1, max_size=10was really a floor of two and a ceiling of twenty, and a seed would have had to be divided to land on three. It takes total limits instead -total_min_node_countandtotal_max_node_countwith alocation_policy, which GKE documents as totals wheremin_node_countis per zone - so every size is a pool total on all three clouds andinitial_node_counttakesinitial_count()whole. Per zone was the house pattern when 46371fb wrote this module's GCP support; pinecone-io/iac moved to total limits in July, in 67fee389, and nothing carried it back.The ceiling is 12 on every cloud now, three above the nine a gcp pool has actually used. gcp is the one that moves: it read as ten a zone and now means twelve, so a gcp pool above twelve would be scaled to it, and its floor drops from two nodes to one. aws and azure only gain two.
Neither seed is ours after the pool exists. AKS
countand GKEinitialNodeCountjoinignore_changes, the wayeks.pyalready ignoresscalingConfig.desiredSize- and on GKE it matters more than drift, because a changedinitialNodeCountreplaces the node pool.Reading one field in three places is what made this a three-cloud edit, so the NodePool to NodePoolConfig translation moves to
common/node_pool.py, carrying over every field the two shapes share a name with. It also settles the taints: aws built aNodePoolTaintfrom any object with key, value and effect, while azure and gcp passed the caller's own objects into a pydantic list that takes only a dict or aNodePoolTaint, so tainting one of their pools raised a ValidationError before reaching a cloud. The publicNodePooldataclasses stay per-cloud, each showing only the machine-size field its cloud reads, because a shared base would reorder positional args in a published API.Problem
Describe the purpose of this change. What problem is being solved and why?
Solution
Describe the approach you took. Link to any relevant bugs, issues, docs, or other resources.
Type of Change
Test Plan
Describe specific steps for validating this change.
Note
Medium Risk
Changes default node counts and GKE autoscaling semantics (per-zone vs total caps), which can affect cluster capacity, install timing, and may scale existing GCP pools if they relied on higher per-zone ceilings; Pulumi ignore_changes reduces replacement risk for seed counts.
Overview
Warm start for Azure and GCP default pools so
pinetools cluster installhas nodes at creation instead of waiting on serial autoscaler rounds (which was timing out Azure e2e).NodePoolConfignow treatsdesired_sizeas cross-cloud (default 3), addsinitial_count()(clamped betweenmin_sizeandmax_size), and raises defaultmax_sizeto 12. AKS agent pools usecount=initial_count()instead ofmin_count; GKE setsinitial_node_countand switches autoscaling tototal_min_node_count/total_max_node_countso limits are pool totals across zones, not per zone.Pulumi drift: AKS
countand GKEinitialNodeCountare added toignore_changes(like EKS already does for desired size), since autoscaler owns live counts and GKE can replace pools ifinitialNodeCountchanges.Shared mapping: Per-cloud
NodePool→NodePoolConfigduplication moves tocommon/node_pool.py(node_pool_configs), including consistent taint normalization (dict/object →NodePoolTaint). Azure/GCP publicNodePooldataclasses gaindesired_size; tests cover sizing and translation.Reviewed by Cursor Bugbot for commit b1e02f9. Bugbot is set up for automated code reviews on this repo. Configure here.