-
Notifications
You must be signed in to change notification settings - Fork 108
Make NodePool optional for LKE-E in terraform #2217
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
Conversation
9a7836f to
024698a
Compare
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.
Manual testing appears to work as intended! It looks like you may have forgotten to add integration tests using the new templates you added. Can you add those please.
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 makes node pools optional for enterprise tier LKE clusters in the Terraform provider, while maintaining the requirement that standard tier clusters must have at least one node pool.
Changes:
- Updated the
node_poolsschema field from required to optional with updated documentation - Added custom diff validation to enforce pool requirement only for standard tier clusters
- Added test template helper functions and a new template for creating clusters without node pools
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| linode/lke/schema_resource.go | Changed node_pools from required to optional and updated description |
| linode/lke/resource.go | Added custom diff validation to enforce pool requirement for standard tier |
| linode/lke/tmpl/template.go | Added helper functions for generating test templates without node pools |
| linode/lke/tmpl/lke_no_pools.gotf | New template for creating LKE clusters without node pools |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ovider-linode into optional-nodepools
…ovider-linode into optional-nodepools
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!
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 5 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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 5 out of 5 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lgarber-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.
LGTM and tests are passing on my end. Thanks for the contribution and sorry again for the delay!
📝 Description
What does this PR do and why is this change necessary?
linode_lke_cluster✔️ How to Test
make buildand copy the generated binary to~/.terraform.d/plugins/local/lab/linode/99.0.0/darwin_arm64.