-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Fix inconsistent locations in some resources #4377
Fix inconsistent locations in some resources #4377
Conversation
Fork Update
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. Thanks for your contribution! A human will be with you soon. @melinath, please review this PR or find an appropriate assignee. |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 16 files changed, 80 insertions(+), 90 deletions(-)) |
Getting weird behavior with per_instance_config resources. TF is complaining about a missing MIG but it actually exists and it deletes the same MIG right after the failure.
|
@upodroid it looks like the projects are not quite the same:
Specifically, the object that exists is in |
Ah, rookie mistake. All of them are working except the per_instance_config resources. I'll get them fixed for Monday. |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 22 files changed, 108 insertions(+), 117 deletions(-)) |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform GA: Diff ( 22 files changed, 108 insertions(+), 113 deletions(-)) |
@upodroid could you split this up into separate PRs based on product? That would make it a lot easier to review & merge. If you ping me on the new PRs I can have a look at them tomorrow. |
Hmm, that is long. How about:
|
@upodroid sure, that sounds good to me. Thanks! |
Co-authored-by: upodroid <[email protected]>
fa3d8fa
to
f7a7f08
Compare
It is done. This one has the GCE resources only. |
Thanks! /gcbrun |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=166693" |
1 similar comment
I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccInstanceTemplateDatasource_filter|TestAccInstanceTemplateDatasource_filter_mostRecent|TestAccDataSourceGoogleSubnetwork|TestAccDataSourceStorageBucketObjectContent_Basic|TestAccActiveDirectoryDomainTrust_activeDirectoryDomainTrustBasicExample|TestAccApigeeOrganization_apigeeOrganizationCloudFullTestExample|TestAccCloudRunService_cloudRunServiceSqlExample|TestAccComputePerInstanceConfig_update|TestAccContainerCluster_withPrivateClusterConfigMissingCidrBlock You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=166696" |
region: !ruby/object:Overrides::Terraform::PropertyOverride | ||
required: false | ||
ignore_read: true | ||
default_from_api: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would expect default_from_api
to conflict with ignore_read
- if the api provides a default but we ignore reading it, will it be available locally? This also seems to be different than the treatment in #4394 which moved things from ignore_read
to default_from_api
- why is there a difference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure. It seems to work though.
third_party/terraform/tests/resource_compute_per_instance_config_test.go
Show resolved
Hide resolved
third_party/terraform/tests/resource_compute_region_per_instance_config_test.go
Show resolved
Hide resolved
/gcbrun to see if the failures are transient. |
I have triggered VCR tests based on this PR's diffs. See the results here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=167109" |
I have triggered VCR tests in RECORDING mode for the following tests that failed during VCR: TestAccInstanceTemplateDatasource_filter|TestAccInstanceTemplateDatasource_filter_mostRecent|TestAccDataSourceStorageBucketObjectContent_Basic|TestAccCloudRunServiceIamMemberGenerated|TestAccCloudRunServiceIamBindingGenerated|TestAccCloudRunServiceIamPolicyGenerated|TestAccActiveDirectoryDomainTrust_activeDirectoryDomainTrustBasicExample|TestAccApigeeOrganization_apigeeOrganizationCloudFullTestExample|TestAccApigeeOrganization_apigeeOrganizationCloudBasicTestExample|TestAccCloudRunService_cloudRunServiceBasicExample|TestAccCloudRunDomainMapping_cloudRunDomainMappingBasicExample|TestAccCloudRunService_cloudRunServiceNoauthExample|TestAccComputeHealthCheck_typeTransition|TestAccComputeHealthCheck_tcp_update|TestAccComputePerInstanceConfig_update|TestAccComputeRegionBackendService_basic|TestAccComputeRegionNetworkEndpointGroup_regionNetworkEndpointGroupCloudrunExample|TestAccContainerCluster_withPrivateClusterConfigMissingCidrBlock You can view the result here: "https://ci-oss.hashicorp.engineering/viewQueued.html?itemId=167119" |
Fixes: hashicorp/terraform-provider-google#8027
If this PR is for Terraform, I acknowledge that I have:
make test
andmake lint
to ensure it passes unit and linter tests.Release Note Template for Downstream PRs (will be copied)