-
Notifications
You must be signed in to change notification settings - Fork 78
Make NodePool optional for LKE-E in python sdk #630
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
Release v5.37.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR modifies the Python SDK to make the node_pools parameter optional when creating LKE Enterprise clusters, while maintaining the requirement for standard LKE clusters. The change reorders function parameters to place kube_version before node_pools and adds validation logic to enforce that standard clusters must have at least one node pool.
Key changes:
- Reordered parameters in
cluster_create()to movekube_versionbeforenode_pools - Added validation to require at least one node pool for standard tier clusters
- Made
node_poolsoptional with an empty list default value
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| linode_api4/groups/lke.py | Reordered cluster_create parameters and added validation logic for standard vs enterprise cluster node pool requirements |
| test/unit/groups/lke_test.py | Added comprehensive unit tests for enterprise clusters without node pools and validation error cases |
| test/unit/objects/lke_test.py | Updated test calls to match new parameter order |
| test/unit/linode_client_test.py | Updated test calls to match new parameter order |
| test/integration/models/lke/test_lke.py | Updated integration test fixtures to match new parameter order |
| test/integration/linode_client/test_linode_client.py | Updated integration test to match new parameter order |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
a3adea0 to
7a7cf5a
Compare
7a7cf5a to
c5fc286
Compare
…to optional-nodepools
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| tier="ENTERPRISE", | ||
| ) | ||
|
|
||
| assert m.call_data["tier"] == "ENTERPRISE" |
Copilot
AI
Jan 9, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test validates case-insensitive tier comparison in the validation logic, but the assertion checks that the tier is passed through as-is to the API. Consider also asserting that node_pools is empty to ensure the validation logic correctly allows enterprise clusters without node pools regardless of case.
ezilber-akamai
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution! Works well locally. Just need to fix the lint error.
📝 Description
What does this PR do and why is this change necessary?
node_poolsargument in cluster_create optional for LKE-E clusters.✔️ How to Test
What are the steps to reproduce the issue or verify the changes?