Skip to content

chore: add warning for nodePool validation - #129

Merged
InftyAI-Agent merged 1 commit into
InftyAI:mainfrom
kerthcet:cleanup/widen-nodepool
Oct 3, 2026
Merged

InftyAI-Agent merged 1 commit into
InftyAI:mainfrom
kerthcet:cleanup/widen-nodepool

Conversation

@kerthcet

@kerthcet kerthcet commented Oct 3, 2026 •

Copy link
Copy Markdown
Member

What this PR does / why we need it

Which issue(s) this PR fixes

Fixes #

Special notes for your reviewer

Does this PR introduce a user-facing change?


Summary by CodeRabbit

  • New Features
    • NodePool validation now warns when a known provider does not support the requested capacity tier or egress mode, and when no known provider can place Pods with the requested settings.
    • NodePools using Modal with Spot capacity are no longer rejected during validation.
  • Bug Fixes
    • Placement now consistently checks provider support for capacity tiers and egress policies.

Signed-off-by: kerthcet <kerthcet@gmail.com>
Copilot AI balanced review requested due to automatic review settings October 3, 2026 11:20
@InftyAI-Agent InftyAI-Agent added needs-triage Indicates an issue or PR lacks a label and requires one. needs-priority Indicates a PR lacks a label and requires one. do-not-merge/needs-kind Indicates a PR lacks a label and requires one. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Oct 3, 2026
@coderabbitai

coderabbitai Bot commented Oct 3, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 82edef16-cf0b-4d05-8d29-53a42a55507d
📥 Commits

Reviewing files that changed from the base of the PR and between c605d6c and c1d4e29.

📒 Files selected for processing (5)
  • internal/controller/pod_placement_helpers.go
  • internal/webhook/v1/nodepool_webhook.go
  • internal/webhook/v1/nodepool_webhook_test.go
  • pkg/provider/catalog/base.go
  • pkg/provider/provider.go
 ___________________________________________________________________________
< In Vino Veritas, In Codice Bugas. In wine, there is truth; in code, bugs. >
 ---------------------------------------------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       /   づ
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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

Copilot review overview

🟡 Changes recommended

The user-facing admission change needs the release note required by the PR template.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
What changed in this PR

Adds admission warnings for NodePool configurations that registered providers cannot serve.

Changes:

  • Centralizes capacity-tier and egress capability checks.
  • Warns instead of rejecting unsupported configurations.
  • Adds webhook warning tests.
File Description
pkg/​provider/​provider.go Adds capability-check helpers.
pkg/​provider/​catalog/​base.go Updates helper reference.
internal/​webhook/​v1/​nodepool_webhook.go Generates NodePool warnings.
internal/​webhook/​v1/​nodepool_webhook_test.go Tests warning scenarios.
internal/​controller/​pod_placement_helpers.go Uses centralized checks.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread internal/webhook/v1/nodepool_webhook.go
@kerthcet

kerthcet commented Oct 3, 2026

Copy link
Copy Markdown
Member Author

/lgtm
/kind cleanup

@InftyAI-Agent InftyAI-Agent added lgtm Looks good to me, indicates that a PR is ready to be merged. cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. and removed do-not-merge/needs-kind Indicates a PR lacks a label and requires one. labels Oct 3, 2026

@InftyAI-Agent InftyAI-Agent left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approved: PR has both lgtm and approved labels

@InftyAI-Agent InftyAI-Agent left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approved: PR has both lgtm and approved labels

@InftyAI-Agent InftyAI-Agent left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approved: PR has both lgtm and approved labels

@InftyAI-Agent
InftyAI-Agent merged commit 65e0a50 into InftyAI:main Oct 3, 2026
44 of 47 checks passed
@kerthcet
kerthcet deleted the cleanup/widen-nodepool branch October 3, 2026 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm Looks good to me, indicates that a PR is ready to be merged. needs-priority Indicates a PR lacks a label and requires one. needs-triage Indicates an issue or PR lacks a label and requires one.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants