-
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
potential fix for gke provider upgrade problem #6546
Conversation
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccComputeInstance_soleTenantNodeAffinities|TestAccCGCSnippet_eventarcWorkflowsExample|TestAccFirebaserulesRelease_BasicRelease |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform Beta: Diff ( 1 file changed, 9 insertions(+)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccComputeInstance_soleTenantNodeAffinities|TestAccComputeForwardingRule_update |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
@shuyama1: I noticed that this field uses a custom DSF rather than setting the field to O+C: https://github.com/GoogleCloudPlatform/magic-modules/pull/6133/files#diff-c8cc57371d31377352c614135ade5ab3273f80b595ee86b222b1271ab6d5b984R1027 The DSF description sounds an awful lot like an O+C field, so it may be worth looking in to whether we can use O+C here instead (assuming it resolves the problem)
Not sure if that's the right path to go down, or if conversion is safe to do in a minor version (although with a preexisting bug here is probably is). In particular, the DSF suggests it compares both sides when suppressing, which is different than what O+C will do. |
Tested it locally and can confirm that setting the field optional computed solves the problem (as expected). Investigating if it's safe to make such conversion. |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform Beta: Diff ( 1 file changed, 318 insertions(+), 311 deletions(-)) |
Tests analyticsTotal tests: Errors occurred during REPLAYING mode. Please fix them to complete your PR |
Hi! I'm the modular magician. Your PR generated some diffs in downstreams - here they are. Diff report:Terraform Beta: Diff ( 1 file changed, 13 insertions(+), 10 deletions(-)) |
Tests analyticsTotal tests: Action takenTriggering VCR tests in RECORDING mode for the tests that failed during VCR. Click here to see the failed testsTestAccComputeInstance_soleTenantNodeAffinities|TestAccFirebaserulesRelease_BasicRelease|TestAccCGCSnippet_eventarcWorkflowsExample |
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
Removed the DSF and set the fields to O+C. Tested it locally and it should fix the problem.
This resolves hashicorp/terraform-provider-google#12422 and resolves hashicorp/terraform-provider-google#12549
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)