Repository navigation
Conversation
Change-Id: Ia6156332164a449115c2d4b9294dbbe788f3b40a
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the GKE modules within the Cluster Toolkit by providing granular control over resource operation timeouts. By exposing 'timeout_delete' and ensuring consistent 'timeouts' blocks are applied to all cluster and node pool resources, users can now explicitly manage provisioning and teardown durations, improving reliability for large-scale deployments and automated testing. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request adds a new timeout_delete variable and introduces input validation for timeout_create, timeout_update, and timeout_delete in both the gke-node-pool and gke-cluster modules. The validation ensures that the timeout values are either null or positive duration strings with 's', 'm', or 'h' units. The resource timeout blocks and README documentation have been updated accordingly. I have no feedback to provide as the changes are well-implemented and follow Terraform best practices.
Summary
Adds
timeout_deletesupport and symmetrictimeoutsblocks across Cluster Toolkit's GKE modules (modules/scheduler/gke-clusterandmodules/compute/gke-node-pool), along with positive duration validation and documentation ontimeout_create,timeout_update, andtimeout_delete.Context
Previously,
modules/scheduler/gke-clusterandmodules/compute/gke-node-poolexposedtimeout_createandtimeout_update, but omittedtimeout_delete. Additionally, the inline node pool resources insidemodules/scheduler/gke-cluster(google_container_node_pool.system_node_poolsandgoogle_container_node_pool.cpu_np) did not wiretimeoutsblocks at all.Exposing
timeout_delete(default = null) and wiringcreate,update, anddeleteacross all GKE cluster and node pool resources allows large-scale GKE deployments and automated test harnesses to configure explicit teardown and provisioning timeouts when needed, while defaulting tonull(falling back to the provider defaults with zero impact or replacement diff on existing clusters).Changes
modules/compute/gke-node-pool:timeout_deletevariable (type = string,default = null) and wireddelete = var.timeout_deleteintogoogle_container_node_pool.node_pool.s,m,hunits) and updated descriptions ontimeout_create,timeout_update, andtimeout_delete.modules/scheduler/gke-cluster:timeout_deletevariable (type = string,default = null) and wireddelete = var.timeout_deleteintogoogle_container_cluster.gke_cluster.timeouts { create = var.timeout_create, update = var.timeout_update, delete = var.timeout_delete }togoogle_container_node_pool.system_node_poolsandgoogle_container_node_pool.cpu_np.s,m,hunits) and updated descriptions ontimeout_create,timeout_update, andtimeout_delete.README.mdtables for both modules viatools/autodoc/terraform_docs.sh.Terraform Provider Default Timeout References
When
timeout_create,timeout_update, ortimeout_deleteare left at their default (null), Terraform falls back to the upstreamgoogle/google-betaprovider defaults:google_container_cluster(90mforcreate,read,update,delete):google_container_cluster#timeoutscontainer_cluster.html.markdown)resource_container_cluster.go)google_container_node_pool(2hforcreate,update,delete):google_container_node_pool#timeoutscontainer_node_pool.html.markdown)resource_container_node_pool.go)Validation
terraform fmt -check,tflint, andterraform validateacross bothmodules/compute/gke-node-poolandmodules/scheduler/gke-cluster.validationrules across valid (null,"30m","60m","2h","1h30m") and invalid ("0s","0m","-5m","60","") inputs.go test ./pkg/... ./cmd/...andtools/autodoc/terraform_docs.sh.