Skip to content

Persist spot and reservation across node pool updates - #312

Open
JyotinderSingh wants to merge 2 commits into
keras-team:mainfrom
JyotinderSingh:fix/pool-spot-reservation-persistence
Open

Persist spot and reservation across node pool updates#312
JyotinderSingh wants to merge 2 commits into
keras-team:mainfrom
JyotinderSingh:fix/pool-spot-reservation-persistence

Conversation

@JyotinderSingh

Copy link
Copy Markdown
Collaborator

Description

The accelerators stack export recorded a pool's accelerator shape but not its spot or reservation settings. Because kinetic up, pool add, and pool remove all rebuild the full pool list from that export and re-declare every pool, the settings came back as their defaults (spot=False, no reservation affinity). Both land in the GKE node pool's node_config, which is ForceNew in the GCP provider, so the next pool operation silently replaced a Spot pool with an on-demand one and dropped reservation affinity.

Record both fields in the per-pool export and read them back in _export_to_node_pool. Exports written before this change lack the keys and fall back to False / None.

Also surface both in the pool list and status tables — spot as a provisioning model, reservation only when set. Fields missing from an older export are omitted rather than shown as a guessed default.

Contributor Agreement

  • I am a human, and not a bot.
  • I will be responsible for responding to review comments in a timely manner.
  • I will work with the maintainers to push this PR forward until submission.
  • I will test the changes on my cloud setup and provide proof of successful validation.

The `accelerators` stack export recorded a pool's accelerator shape but
not its `spot` or `reservation` settings. Because `kinetic up`, `pool
add`, and `pool remove` all rebuild the full pool list from that export
and re-declare every pool, the settings came back as their defaults
(spot=False, no reservation affinity). Both land in the GKE node pool's
node_config, which is ForceNew in the GCP provider, so the next pool
operation silently replaced a Spot pool with an on-demand one and
dropped reservation affinity.

Record both fields in the per-pool export and read them back in
`_export_to_node_pool`. Exports written before this change lack the
keys and fall back to False / None.

Also surface both in the `pool list` and `status` tables — spot as a
provisioning model, reservation only when set. Fields missing from an
older export are omitted rather than shown as a guessed default.
Record in the Spot and reservation guides that Kinetic keeps these
settings across later pool commands, and that `kinetic pool list` shows
them. Add a note about a pool from an earlier version of Kinetic: the
cluster state has no record of its setting, so the next pool command
drops the setting.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request adds support for tracking and displaying the provisioning model (Spot vs. On-demand) and reservation affinity of GKE node pools in Kinetic. It updates the Pulumi stack exports to record these settings, ensures they survive round-trips during pool updates, and updates the CLI output to display them. Comprehensive unit tests and documentation updates are also included. I have no feedback to provide as there are no review comments.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 15.11628% with 146 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@a6adb41). Learn more about missing BASE report.

Files with missing lines Patch % Lines
kinetic/cli/infra/program_test.py 0.00% 53 Missing ⚠️
kinetic/cli/infra/stack_manager_test.py 0.00% 52 Missing ⚠️
kinetic/cli/output_test.py 0.00% 41 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #312   +/-   ##
=======================================
  Coverage        ?   36.86%           
=======================================
  Files           ?       83           
  Lines           ?    15370           
  Branches        ?        0           
=======================================
  Hits            ?     5666           
  Misses          ?     9704           
  Partials        ?        0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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