Skip to content

Feature/collapse regional and zonal clusters #223

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

Merged
merged 1 commit into from
Aug 15, 2019
Merged

Feature/collapse regional and zonal clusters #223

merged 1 commit into from
Aug 15, 2019

Conversation

kopachevsky
Copy link
Contributor

@kopachevsky kopachevsky commented Aug 1, 2019

Refactor/flatten duplicate resources #161

  Collapsed regional and zonal clusters into one cluster with location attribute instead usage of count, zones and regions.
  Selection of latest nodes and master version same both for regional and zonal/multi-zonal clusters.
  For Master version in case or regional cluster following code check latest version by location (region),
  and for zonal, location is var.zone[0]:
    latest_master_version       = data.google_container_engine_versions.master.latest_master_version
  For Node version in case of regional cluster used latest master version of first of available zones, and in case of
  zonal cluster - latest master version of var.zones[0] zone
    location = local.zone_count == 0 ? data.google_compute_zones.available.names[0] : var.zones[0]

  Fixes terraform-google-modules/terraform-google-kubernetes-engine#161

ivankorn
ivankorn previously approved these changes Aug 8, 2019
ingwarr
ingwarr previously approved these changes Aug 8, 2019
@kopachevsky kopachevsky marked this pull request as ready for review August 8, 2019 15:45
@morgante morgante mentioned this pull request Aug 14, 2019
3 tasks
@morgante
Copy link
Contributor

@kopachevsky Please resolve merge conflicts.

@kopachevsky kopachevsky dismissed stale reviews from ingwarr and ivankorn via bba606c August 14, 2019 08:46
ivankorn
ivankorn previously approved these changes Aug 14, 2019
ingwarr
ingwarr previously approved these changes Aug 14, 2019
Copy link
Contributor

@morgante morgante left a comment

Choose a reason for hiding this comment

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

Almost there, just 1 small issue.

Collapsed regional and zonal clusters into one cluster with location attribute instead usage of count, zones and regions.
Selection of latest nodes and master version same both for regional and zonal/multi-zonal clusters.
    For Master version in case or regional cluster following code check latest version by location (region),
and for zonal, location is var.zone[0]:
    latest_master_version       = data.google_container_engine_versions.master.latest_master_version
For Node version in case of regional cluster used latest master version of first of available zones, and in case of
zonal cluster - latest master version of var.zones[0] zone
    location = local.zone_count == 0 ? data.google_compute_zones.available.names[0] : var.zones[0]
Fixes #161
@kopachevsky kopachevsky dismissed stale reviews from ingwarr and ivankorn via 687703b August 15, 2019 10:58
@kopachevsky
Copy link
Contributor Author

@morgante I've tested locally, tests passed except 2 stub_domains_* ones, it's not possible to test them locallly. Please review.

@kopachevsky kopachevsky requested a review from morgante August 15, 2019 11:12
Copy link
Contributor

@morgante morgante left a comment

Choose a reason for hiding this comment

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

LGTM, but waiting to merge until I can test state migrations.

@morgante morgante merged commit 2f4b198 into terraform-google-modules:master Aug 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants