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

Set computed flag for 'max_pods_per_node' in google_container_node_pool #2077

Merged

Conversation

thevilledev
Copy link
Contributor

@thevilledev thevilledev commented Sep 19, 2018

After upgrading terraform-provider-google to version 1.18 users will notice an unnecessary Terraform plan difference in existing node pool configurations. This is caused by a new configuration parameter max_pods_per_node which was introduced in #2038. At the moment it expects end users to explicitly define the parameter value in the configuration or it expects to find it from the existing state. Since neither of these will be found the provider will fallback to default value of 0.

Example:

$ terraform init -upgrade
...
- Downloading plugin for provider "google" (1.18.0)...
...

$ terraform plan
  ~ google_container_node_pool.ingress
      max_pods_per_node: "110" => "0"

This PR fixes the issue by letting the Google Container API to decide what the value is for existing clusters, which is 110 in this case. If the value is not defined in the Terraform configuration then it won't be used either.

After upgrading the provider to 1.18 users will notice an
unnecessary Terraform plan difference, where the
'max_pods_per_node' parameter will change from 110 to 0.
This commit changes the behaviour so that for state that does
not have the parameter configured, no changes will be planned.
@ghost ghost added the size/xs label Sep 19, 2018
@thevilledev thevilledev changed the title Set computed flag for 'max_pods_per_node' Set computed flag for 'max_pods_per_node' in google_container_node_pool Sep 19, 2018
@nat-henderson
Copy link
Contributor

Interesting - might that have been set in the console, outside of Terraform? I ask because we're not seeing that behavior in our unit tests. Still, probably a good idea to have it as Computed.

@nat-henderson nat-henderson merged commit 80e03f7 into hashicorp:master Sep 19, 2018
@thevilledev
Copy link
Contributor Author

@ndmckinley at least in my case all node pools were created in Terraform with version 1.17 of the provider. Also I had a completely new environment from early this week. I noticed the plan difference after upgrading the provider, so I guess it should be reproducible if someone wants to try that.

Thanks for merging!

@ghost
Copy link

ghost commented Nov 16, 2018

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Nov 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants