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

Add GKE Resource Consumption Metering, promote resource export to GA #2705

Closed

Conversation

maxlip
Copy link
Contributor

@maxlip maxlip commented Nov 15, 2019

Add GKE Resource Consumption Metering, promote resource export block to GA
Should resolve hashicorp/terraform-provider-google#4634
Relates to terraform-google-modules/terraform-google-kubernetes-engine#316

Release Note Template for Downstream PRs (will be copied)

@modular-magician
Copy link
Collaborator

Hello! I am a robot who works on Magic Modules PRs.

I have detected that you are a community contributor, so your PR will be assigned to someone with a commit-bit on this repo for initial review. They will authorize it to run through our CI pipeline, which will generate downstream PRs.

Thanks for your contribution! A human will be with you soon.

@rileykarson, please review this PR or find an appropriate assignee.

@rileykarson
Copy link
Member

Sorry about the delay on my end! I've been busy with 3.0.0-beta.1 and then out for a few days. I should get to this today or tomorrow.

Copy link
Member

@rileykarson rileykarson left a comment

Choose a reason for hiding this comment

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

We're trying to avoid cases where empty nested blocks can be specified, so I'd like to top-level the field. See my comment inline.

@@ -881,6 +881,20 @@ func resourceContainerCluster() *schema.Resource {
Optional: true,
Default: false,
},
"enable_resource_consumption_metering": {
Copy link
Member

Choose a reason for hiding this comment

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

Can you break this schema out so that it's just the following? As well as make the required transformations in code to send/read the right request. enable_shielded_nodes is an example where we made a similar transformation, I believe.

"enable_resource_consumption_metering": {
    Type:     schema.TypeBool,
    Optional: true,
    Default:  true,
},

(defaulting to whatever the API does)

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, I'm trying to find the time for this - I'm having a hard time figuring how to nest the Shielded style code into another resource block.
I do think more fields are coming to this block in the future, in case that matters.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not aware of any additional subfields upcoming, so I'd rather we pull it up. We can add them in a compatible way if any are added.

hashicorp/terraform-provider-google-beta@6e56d74 is a good reference for what adding enable_shielded_nodes looked like (although it's munged with shielded_instance_config. Only the parts in google_container_cluster.go matter.)

@rileykarson
Copy link
Member

superseded with #3303

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

resource_usage_export_config in google_container_cluster should be supported in GA provider
4 participants