feat(cluster): support regional Autopilot clusters in environments where the API only accepts region-level locations#2552
Conversation
…ere the API only accepts region-level locations
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
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 Autopilot cluster module to support deployment in GCP-like environments that have specific API constraints regarding regional clusters. It addresses issues where the Container API might only accept region-level locations for version lookups or reject explicit zone names during cluster creation. The changes introduce flexible configuration options to adapt to these environments while maintaining compatibility with standard GCP. Highlights
Changelog
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 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 counter productive. 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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request adds support for regional Autopilot clusters in environments with specific API limitations by introducing new optional variables and logic. It also standardizes the GKE version lookup for regional clusters across multiple modules. The changes are well-documented.
|
This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days |
Support for GCP environments with region/zone API limitations
Why this change
Some GCP-like environments (sovereign or partner clouds) use a Container API that does not behave like standard GCP for regional clusters:
locationforgoogle_container_engine_versions. A zone-level lookup (e.g.u-france-east1-a) can return "Location does not exist".locationbut reject explicit zone names innode_locationsat create time (e.g. "Specified location is not a valid zone"). If we then send the same zones on a later apply, the API can report "Cluster location change not allowed" because it has already populated zones itself.This PR keeps the module working for standard GCP and adds optional behaviour so it also works in those environments.
What we changed
1. Regional clusters: no zone-level version lookup
main.tf:data "google_container_engine_versions" "zone"is only created whenvar.regionalisfalse(count = var.regional ? 0 : 1). For regional clusters we never call the API with a zone.data.google_container_engine_versions.region; the existingmaster_version_*locals are adjusted so the zonal data source is used only when it exists.When it matters: Use the module as today for standard GCP. In environments where the Container API accepts only region-level location for version lookup, regional clusters no longer fail on that data source.
2. Optional cluster location override
variables.tf: New optional variablecluster_location_override(string, defaultnull).main.tf: New localscluster_location_inputanduse_zone_location. The cluster resource usescluster_location_inputforlocation; when the override is set to the first zone andregional = true,node_locationsis set to the remaining zones so the cluster is created as zonal from the API’s point of view.When it matters: If the API rejects a region as cluster
location, callers can setcluster_location_overrideto an accepted value (e.g. a zone). No change required if you do not set it.3. Optional empty
node_locationsfor regional create (no drift)variables.tf: New optional variableomit_node_locations_for_regional(bool, defaultfalse).main.tf: Whenomit_node_locations_for_regional && regional, we setcluster_node_locations = []. A single localcluster_selfpoints to the cluster resource that is actually created (see below). All existing outputs and internals usecluster_selfso the rest of the module is unchanged.cluster.tf: We use two cluster resources with oppositecount:primary: created whenomit_node_locations_for_regionalis false; uses explicitnode_locationsas before.primary_omit_nodelocs: created whenomit_node_locations_for_regionalis true; setsnode_locations = []so the API accepts create, andlifecycle { ignore_changes = [node_locations] }so we never try to update the API-populated zones. That avoids drift and allows destroy/recreate (recreate again sends[]).outputs.tfandfirewall.tf: All references to the cluster uselocal.cluster_self; outputs’depends_onlist both resources so dependency order is correct.When it matters: In environments where the API accepts the region as
locationbut rejects explicit zones at create, setomit_node_locations_for_regional = true. Create succeeds withnode_locations = []; the API fills zones; we do not touchnode_locationsagain, so no drift and destroy/recreate works.Backward compatibility
null/false.regional = true.node_locationsbehaviour are unchanged.omit_node_locations_for_regionalis false, onlyprimaryis created (count=1), as today. When true, onlyprimary_omit_nodelocsis created. Outputs and behaviour are the same from the caller’s perspective.Documentation
modules/beta-autopilot-private-cluster/README.md: "Kubernetes version lookup" and "Cluster location override" sections describe the new behaviour and when to use the new variables.variables.tfexplaincluster_location_overrideandomit_node_locations_for_regional.Testing
primaryis used; zone data source still created for zonal clusters.omit_node_locations_for_regional = trueandregional = true→ onlyprimary_omit_nodelocsis created; create/apply/destroy and destroy+recreate should succeed in environments that require this path.Existing tests that use regional or zonal clusters should pass without modification.