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.

Sign up for GitHub

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

Jump to bottom

[TF] GKE Cluster - fix import on some convenience fields #1434

Merged
merged 3 commits into from
Feb 28, 2019
Merged

[TF] GKE Cluster - fix import on some convenience fields #1434

merged 3 commits into from
Feb 28, 2019

Conversation

emilymye
Copy link
Contributor

Fixes hashicorp/terraform-provider-google#3064

[all]

[terraform]

container cluster - fix import on some convenience fields

[terraform-beta]

container cluster - fix import on some convenience fields

[ansible]

[inspec]

@emilymye emilymye force-pushed the cluster_node_pool_update branch from 5ab773d to 03a2f5d Compare February 28, 2019 00:07
@emilymye emilymye requested a review from rileykarson February 28, 2019 00:07
@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician, I work on Magic Modules.
This PR seems not to have generated downstream PRs before, as of 03a2f5d.

Pull request statuses

No diff detected in Ansible.
No diff detected in Inspec.

New Pull Requests

I built this PR into one or more new PRs on other repositories, and when those are closed, this PR will also be merged and closed.
depends: hashicorp/terraform-provider-google-beta#476
depends: hashicorp/terraform-provider-google#3146

rileykarson
rileykarson approved these changes Feb 28, 2019
View reviewed changes
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.

Are we able to test the default node pool deletion behaviour? I'd like to, since it's complex behaviour, but I'm not sure if the test gauntlet makes it possible.

third_party/terraform/resources/resource_container_cluster.go.erb
@@ -2108,29 +2107,55 @@ func flattenPodSecurityPolicyConfig(c *containerBeta.PodSecurityPolicyConfig) []
<% end -%>

func resourceContainerClusterStateImporter(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Ah rip this will conflict with https://github.com/GoogleCloudPlatform/magic-modules/pull/1427/files#diff-29c7fa35b303cd012accf212d201bd5dR2200

Copy link
Contributor Author

Choose a reason for hiding this comment

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

rip indeed, very sorry

third_party/terraform/website/docs/r/container_cluster.html.markdown Outdated Show resolved Hide resolved
@emilymye
Copy link
Contributor Author

@rileykarson I believe we already are testing the default node behavior (TestAccContainerCluster_withDefaultNodePoolRemoved for true, other tests for false). Without the new behavior, deleting the Import ignores in the tests would make them fail otherwise. Were you looking for something to test in particular?

@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician, I work on Magic Modules.
I see that this PR has already had some downstream PRs generated. Any open downstreams are already updated to your most recent commit, ed31d9c.

Pull request statuses

terraform-provider-google-beta already has an open PR.
terraform-provider-google already has an open PR.
No diff detected in Ansible.
No diff detected in Inspec.

New Pull Requests

I didn't open any new pull requests because of this PR.

emilymye and others added 2 commits February 28, 2019 10:30
@emilymye emilymye force-pushed the cluster_node_pool_update branch from fcbda1f to d1334c4 Compare February 28, 2019 18:32
@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician, I work on Magic Modules.
I see that this PR has already had some downstream PRs generated. Any open downstreams are already updated to your most recent commit, fcbda1f.

Pull request statuses

terraform-provider-google-beta already has an open PR.
terraform-provider-google already has an open PR.
No diff detected in Ansible.
No diff detected in Inspec.

New Pull Requests

I didn't open any new pull requests because of this PR.

@modular-magician
Copy link
Collaborator

Hi! I'm the modular magician, I work on Magic Modules.
I see that this PR has already had some downstream PRs generated. Any open downstreams are already updated to your most recent commit, d1334c4.

Pull request statuses

terraform-provider-google-beta already has an open PR.
terraform-provider-google already has an open PR.
No diff detected in Ansible.
No diff detected in Inspec.

New Pull Requests

I didn't open any new pull requests because of this PR.

@emilymye
Copy link
Contributor Author

=== RUN TestAccContainerCluster_withDefaultNodePoolRemoved
=== PAUSE TestAccContainerCluster_withDefaultNodePoolRemoved
=== CONT TestAccContainerCluster_withDefaultNodePoolRemoved
--- PASS: TestAccContainerCluster_withDefaultNodePoolRemoved (573.96s)
PASS
ok github.com/terraform-providers/terraform-provider-google/google 574.371s

@modular-magician
Update tracked submodules -> HEAD on Thu Feb 28 21:12:49 UTC 2019
31543d4 
Tracked submodules are build/terraform-beta build/terraform build/ansible build/inspec.
@modular-magician modular-magician merged commit 62b803d into GoogleCloudPlatform:master Feb 28, 2019
@nicktrav nicktrav mentioned this pull request Nov 21, 2019
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rileykarson rileykarson rileykarson approved these changes

Assignees
No one assigned
Labels
automerged cla: yes downstream-generated no-op terraform-provider-google terraform-provider-google-beta
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

google_container_cluster does not import remove_default_node_pool correctly
4 participants
@emilymye @modular-magician @rileykarson @googlebot