Skip to content

feat(azure,gcp): give a cell's node pools a warm start - #103

Open
IliaFeldgun wants to merge 2 commits into
mainfrom
ilia/warm-node-pools
Open

feat(azure,gcp): give a cell's node pools a warm start#103
IliaFeldgun wants to merge 2 commits into
mainfrom
ilia/warm-node-pools

Conversation

@IliaFeldgun

@IliaFeldgun IliaFeldgun commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

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 -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.

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

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Infrastructure change (CI configs, etc)
  • Non-code change (docs, etc)
  • None of the above: (explain here)

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 install has nodes at creation instead of waiting on serial autoscaler rounds (which was timing out Azure e2e).

NodePoolConfig now treats desired_size as cross-cloud (default 3), adds initial_count() (clamped between min_size and max_size), and raises default max_size to 12. AKS agent pools use count=initial_count() instead of min_count; GKE sets initial_node_count and switches autoscaling to total_min_node_count / total_max_node_count so limits are pool totals across zones, not per zone.

Pulumi drift: AKS count and GKE initialNodeCount are added to ignore_changes (like EKS already does for desired size), since autoscaler owns live counts and GKE can replace pools if initialNodeCount changes.

Shared mapping: Per-cloud NodePoolNodePoolConfig duplication moves to common/node_pool.py (node_pool_configs), including consistent taint normalization (dict/object → NodePoolTaint). Azure/GCP public NodePool dataclasses gain desired_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.

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>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ 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.

Comment thread pulumi_pinecone_byoc/gcp/gke.py Outdated
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>
@IliaFeldgun

Copy link
Copy Markdown
Collaborator Author

Azure still flaky, retriaging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants