Skip to content
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

feat: Add fully configurable resource usage export block in GA #491

Merged
merged 2 commits into from
Apr 23, 2020
Merged

feat: Add fully configurable resource usage export block in GA #491

merged 2 commits into from
Apr 23, 2020

Conversation

gcamus59
Copy link
Contributor

@gcamus59 gcamus59 commented Apr 20, 2020

Add GKE metering block which it's now in GA into Google Provider version since 3.16.
In the previous implementation on the BETA modules, only the BigQuery dataset was editable. In this version, all parameters can be updated such as:

  • Network egress
  • Resource consumption metering
  • BigQuery dataset ID

⚠️ Tests cases with fixed version of the Google provider older than 3.16 are failing.

See #316

@gcamus59 gcamus59 requested review from bharathkkb, Jberlinsky and a team as code owners April 20, 2020 14:22
@gcamus59
Copy link
Contributor Author

Waiting your feedback regarding the requirement of the Google provider in 3.16+ version (is it the right implementation for this kind fo new feature only available in the latest google provider versions, should I update other tests cases requirements, etc..)

@bharathkkb
Copy link
Member

Yes I think its reasonable to bump provider versions. In fact I think we should be doing ~> 3.x instead of ~> 3.x.0 so that we keep getting the latest 3.x provider. Any thoughts on changing all examples to ~> 3.x @morgante ?

@morgante
Copy link
Contributor

In terms of upgrading provider versions:

  1. In the examples, feel free to upgrade to a new pinned provider (ex. ~> 3.16.0). The reason we pin to a specific minor version is to avoid cases where the minimum required provider version is silently changed w/o us knowing. We want to be able to call that out in release notes.
  2. In the module versions.tf we can change the pin to the minimum required (ex. ~> 3.16).

@@ -188,6 +188,12 @@ variable "node_pools_metadata" {
default-node-pool = {}
}
}

variable "resource_usage_export" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of switching to a direct object list (which is also a very confusing data type), we should retain the existing resource_usage_export_dataset_id variable and add enable_network_egress_export and enable_resource_consumption_export variables.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, thanks for the feedback. I'll update it.

@gcamus59 gcamus59 requested a review from morgante April 22, 2020 15:10
@bharathkkb
Copy link
Member

LGTM, CI error seems unrelated but there are some conflicts. Please rebase

morgante
morgante previously approved these changes Apr 23, 2020
@morgante morgante merged commit 54eca6b into terraform-google-modules:master Apr 23, 2020
CPL-markus pushed a commit to WALTER-GROUP/terraform-google-kubernetes-engine that referenced this pull request Jul 15, 2024
…grade GCP provider (terraform-google-modules#491)

BREAKING CHANGE: Minimum Google provider version increased to 3.16.
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.

None yet

3 participants