Change advertised_ip_ranges fields in google_compute_router_peer and google_compute_router resources to Sets
[breaking change for 6.0.0 major release]
Change advertised_ip_ranges fields in google_compute_router_peer and google_compute_router resources to Sets.
Fixes hashicorp/terraform-provider-google/issues/9353
compute: changed the `advertised_ip_ranges` fields in both `google_compute_router_peer` and `google_compute_router` resources to be sets. This will reduce diffs due to re-ordering.
Hello! I am a robot. Tests will require approval from a repository maintainer to run.
@roaks3, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look.
You can help make sure that review is quick by doing a self-review and by running impacted tests locally.
@SarahFrench I opened this because I wasn't able to work on https://github.com/GoogleCloudPlatform/magic-modules/pull/10118. In case the @Maarc-D is able to move forward with the other I'm happy to abandon this one.
I applied the changes you recommended and it's well understood this is a breaking change that can be merged in 6.0.0.
Hi there, I'm the Modular magician. I've detected the following information about your changes:
Diff report
Your PR generated some diffs in downstreams - here they are.
google provider: Diff ( 2 files changed, 25 insertions(+), 18 deletions(-))
google-beta provider: Diff ( 2 files changed, 25 insertions(+), 18 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 1 insertion(+))
Errors
google provider:
- The diff processor failed to build. This is usually due to the downstream provider failing to compile.
google-beta provider:
- The diff processor failed to build. This is usually due to the downstream provider failing to compile.
Tests analytics
Total tests: 0
Passed tests: 0
Skipped tests: 0
Affected tests: 0
Click here to see the affected service packages
- compute
$\textcolor{red}{\textsf{Errors occurred during REPLAYING mode. Please fix them to complete your PR.}}$ View the build log
I'll leave review to @roaks3.
If this PR addresses ordering diffs on this field by converting it to a Set in 6.0.0 then this PR will also need to remove the ordering logic added in https://github.com/GoogleCloudPlatform/magic-modules/pull/10572/files
Ah yes! Good point. I’ll do that.
Il giorno ven 31 mag 2024 alle 16:26 Sarah French @.***> ha scritto:
I'll leave review to @roaks3 https://github.com/roaks3.
If this PR addresses ordering diffs on this field by converting it to a Set in 6.0.0 then this PR will also need to remove the ordering logic added in https://github.com/GoogleCloudPlatform/magic-modules/pull/10572/files
— Reply to this email directly, view it on GitHub https://github.com/GoogleCloudPlatform/magic-modules/pull/10845#issuecomment-2142352486, or unsubscribe https://github.com/notifications/unsubscribe-auth/AARY7UABU6WZF7G46WHBOCLZFCCBZAVCNFSM6AAAAABIS3YMHGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBSGM2TENBYGY . You are receiving this because you authored the thread.Message ID: @.***>
I'll leave review to @roaks3.
If this PR addresses ordering diffs on this field by converting it to a Set in 6.0.0 then this PR will also need to remove the ordering logic added in https://github.com/GoogleCloudPlatform/magic-modules/pull/10572/files
@SarahFrench done! Please, check if it looks good to you. @roaks3 this should be ready (I know you won't be able to merge until the release...)
Hi there, I'm the Modular magician. I've detected the following information about your changes:
Diff report
Your PR generated some diffs in downstreams - here they are.
google provider: Diff ( 3 files changed, 39 insertions(+), 36 deletions(-))
google-beta provider: Diff ( 3 files changed, 39 insertions(+), 36 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 1 insertion(+))
Errors
google provider:
- The diff processor failed to build. This is usually due to the downstream provider failing to compile.
google-beta provider:
- The diff processor failed to build. This is usually due to the downstream provider failing to compile.
Tests analytics
Total tests: 0
Passed tests: 0
Skipped tests: 0
Affected tests: 0
Click here to see the affected service packages
- compute
$\textcolor{red}{\textsf{Errors occurred during REPLAYING mode. Please fix them to complete your PR.}}$ View the build log
Hi there, I'm the Modular magician. I've detected the following information about your changes:
Diff report
Your PR generated some diffs in downstreams - here they are.
google provider: Diff ( 3 files changed, 39 insertions(+), 36 deletions(-))
google-beta provider: Diff ( 3 files changed, 39 insertions(+), 36 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 1 insertion(+))
Errors
google provider:
- The diff processor failed to build. This is usually due to the downstream provider failing to compile.
google-beta provider:
- The diff processor failed to build. This is usually due to the downstream provider failing to compile.
Tests analytics
Total tests: 0
Passed tests: 0
Skipped tests: 0
Affected tests: 0
Click here to see the affected service packages
- compute
$\textcolor{red}{\textsf{Errors occurred during REPLAYING mode. Please fix them to complete your PR.}}$ View the build log
This PR has been waiting for review for 2 weekdays. Please take a look! Use the label disable-review-reminders to disable these notifications.
@SarahFrench @roaks3 I see in my local tests that something is failing in the generated router code:
make testacc TEST=./google-beta/services/compute TESTARGS='-run=
TestAccComputeRouter_routerBasicExample'
==> Checking that code complies with gofmt requirements...
go vet
TF_ACC=1 TF_SCHEMA_PANIC_ON_ERROR=1 go test ./google-beta/services/compute -v -run=TestAccComputeRouter_routerBasicExample -timeout 240m -ldflags="-X=github.com/hashicorp/terraform-provider-google-beta/version.ProviderVersion=acc"
=== RUN TestAccComputeRouter_routerBasicExample
=== PAUSE TestAccComputeRouter_routerBasicExample
=== CONT TestAccComputeRouter_routerBasicExample
panic: interface conversion: interface {} is *schema.Set, not []interface {}
goroutine 307 [running]:
github.com/hashicorp/terraform-provider-google-beta/google-beta/services/compute.resourceComputeRouterCustomDiff({0x0?, 0x0?}, 0x0?, {0x0?, 0x0?})
/Users/lucaprete/go/src/github.com/hashicorp/terraform-provider-google-beta/google-beta/services/compute/resource_compute_router.go:42 +0x1aa
github.com/hashicorp/terraform-provider-google-beta/google-beta/services/compute.ResourceComputeRouter.All.func1({0x56e9e20, 0xc00104a780}, 0x4de4eda?, {0x4d9f5c0, 0xc00026f000})
/Users/lucaprete/go/pkg/mod/github.com/hashicorp/terraform-plugin-sdk/[email protected]/helper/customdiff/compose.go:52 +0xd2
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.schemaMap.Diff(0xc000981380, {0x56e9e20, 0xc00104a780}, 0xc0011b8d00, 0xc001ba1770, 0xc000480d40, {0x4d9f5c0, 0xc00026f000}, 0x0)
/Users/lucaprete/go/pkg/mod/github.com/hashicorp/terraform-plugin-sdk/[email protected]/helper/schema/schema.go:698 +0x4b4
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*Resource).SimpleDiff(0x56ea088?, {0x56e9e20?, 0xc00104a780?}, 0xc0011b8d00, 0x47711c0?, {0x4d9f5c0?, 0xc00026f000?})
/Users/lucaprete/go/pkg/mod/github.com/hashicorp/terraform-plugin-sdk/[email protected]/helper/schema/resource.go:962 +0xd5
github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.(*GRPCProviderServer).PlanResourceChange(0xc000ff6e70, {0x56e9e20?, 0xc00104a6c0?}, 0xc001ba1630)
/Users/lucaprete/go/pkg/mod/github.com/hashicorp/terraform-plugin-sdk/[email protected]/helper/schema/grpc_provider.go:798 +0x9b6
github.com/hashicorp/terraform-plugin-mux/tf5muxserver.(*muxServer).PlanResourceChange(0xc00015f570, {0x56e9e20?, 0xc00104a420?}, 0xc001ba1630)
/Users/lucaprete/go/pkg/mod/github.com/hashicorp/[email protected]/tf5muxserver/mux_server_PlanResourceChange.go:73 +0x2ad
github.com/hashicorp/terraform-plugin-go/tfprotov5/tf5server.(*server).PlanResourceChange(0xc0004a8dc0, {0x56e9e20?, 0xc001c7fbf0?}, 0xc000e609a0)
/Users/lucaprete/go/pkg/mod/github.com/hashicorp/[email protected]/tfprotov5/tf5server/server.go:811 +0x3d0
github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5._Provider_PlanResourceChange_Handler({0x4d45080?, 0xc0004a8dc0}, {0x56e9e20, 0xc001c7fbf0}, 0xc002709c80, 0x0)
/Users/lucaprete/go/pkg/mod/github.com/hashicorp/[email protected]/tfprotov5/internal/tfplugin5/tfplugin5_grpc.pb.go:500 +0x169
google.golang.org/grpc.(*Server).processUnaryRPC(0xc001020600, {0x56e9e20, 0xc001c7fb60}, {0x56f5aa8, 0xc00053de00}, 0xc001aea360, 0xc001068840, 0x754c080, 0x0)
/Users/lucaprete/go/pkg/mod/google.golang.org/[email protected]/server.go:1369 +0xe23
google.golang.org/grpc.(*Server).handleStream(0xc001020600, {0x56f5aa8, 0xc00053de00}, 0xc001aea360)
/Users/lucaprete/go/pkg/mod/google.golang.org/[email protected]/server.go:1780 +0x1016
google.golang.org/grpc.(*Server).serveStreams.func2.1()
/Users/lucaprete/go/pkg/mod/google.golang.org/[email protected]/server.go:1019 +0x8b
created by google.golang.org/grpc.(*Server).serveStreams.func2 in goroutine 245
/Users/lucaprete/go/pkg/mod/google.golang.org/[email protected]/server.go:1030 +0x135
FAIL github.com/hashicorp/terraform-provider-google-beta/google-beta/services/compute 4.509s
FAIL
make: *** [testacc] Error 1
On the other hand, here I see some test failures that seem completely unrelated to this change. I'll try to re-trigger them and see if at least I get something consistent with what I get on my device.
Any idea what I am missing, given it's just a one line modification?
Hi there, I'm the Modular magician. I've detected the following information about your changes:
Diff report
Your PR generated some diffs in downstreams - here they are.
google provider: Diff ( 3 files changed, 61 insertions(+), 59 deletions(-))
google-beta provider: Diff ( 3 files changed, 61 insertions(+), 59 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 1 insertion(+))
Breaking Change(s) Detected
The following breaking change(s) were detected within your pull request.
- Field
advertised_ip_rangeschanged from TypeList to TypeSet ongoogle_compute_router_peer- reference - Field
bgp.advertised_ip_rangeschanged from TypeList to TypeSet ongoogle_compute_router- reference
If you believe this detection to be incorrect please raise the concern with your reviewer.
If you intend to make this change you will need to wait for a major release window.
An override-breaking-change label can be added to allow merging.
Tests analytics
Total tests: 78
Passed tests: 18
Skipped tests: 58
Affected tests: 2
Click here to see the affected service packages
- compute
Action taken
Found 2 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy|TestAccComputeRegionAutoscaler_update
$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy[Error message] [Debug log]
TestAccComputeRegionAutoscaler_update[Error message] [Debug log]
$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$ View the build log or the debug log for each test
Hi there, I'm the Modular magician. I've detected the following information about your changes:
Diff report
Your PR generated some diffs in downstreams - here they are.
google provider: Diff ( 3 files changed, 61 insertions(+), 59 deletions(-))
google-beta provider: Diff ( 3 files changed, 61 insertions(+), 59 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 1 insertion(+))
Breaking Change(s) Detected
The following breaking change(s) were detected within your pull request.
- Field
advertised_ip_rangeschanged from TypeList to TypeSet ongoogle_compute_router_peer- reference - Field
bgp.advertised_ip_rangeschanged from TypeList to TypeSet ongoogle_compute_router- reference
If you believe this detection to be incorrect please raise the concern with your reviewer.
If you intend to make this change you will need to wait for a major release window.
An override-breaking-change label can be added to allow merging.
Tests analytics
Total tests: 80
Passed tests: 19
Skipped tests: 60
Affected tests: 1
Click here to see the affected service packages
- compute
Action taken
Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy
Hi there, I'm the Modular magician. I've detected the following information about your changes:
Diff report
Your PR generated some diffs in downstreams - here they are.
google provider: Diff ( 3 files changed, 59 insertions(+), 57 deletions(-))
google-beta provider: Diff ( 3 files changed, 59 insertions(+), 57 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 1 insertion(+))
Breaking Change(s) Detected
The following breaking change(s) were detected within your pull request.
- Field
advertised_ip_rangeschanged from TypeList to TypeSet ongoogle_compute_router_peer- reference - Field
bgp.advertised_ip_rangeschanged from TypeList to TypeSet ongoogle_compute_router- reference
If you believe this detection to be incorrect please raise the concern with your reviewer.
If you intend to make this change you will need to wait for a major release window.
An override-breaking-change label can be added to allow merging.
Tests analytics
Total tests: 80
Passed tests: 19
Skipped tests: 60
Affected tests: 1
Click here to see the affected service packages
- compute
Action taken
Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy
$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy[Error message] [Debug log]
$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$ View the build log or the debug log for each test
$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy[Error message] [Debug log]
$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$ View the build log or the debug log for each test
Hi there, I'm the Modular magician. I've detected the following information about your changes:
Diff report
Your PR generated some diffs in downstreams - here they are.
google provider: Diff ( 3 files changed, 59 insertions(+), 57 deletions(-))
google-beta provider: Diff ( 3 files changed, 59 insertions(+), 57 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 1 insertion(+))
Breaking Change(s) Detected
The following breaking change(s) were detected within your pull request.
- Field
advertised_ip_rangeschanged from TypeList to TypeSet ongoogle_compute_router_peer- reference - Field
bgp.advertised_ip_rangeschanged from TypeList to TypeSet ongoogle_compute_router- reference
If you believe this detection to be incorrect please raise the concern with your reviewer.
If you intend to make this change you will need to wait for a major release window.
An override-breaking-change label can be added to allow merging.
Tests analytics
Total tests: 80
Passed tests: 19
Skipped tests: 60
Affected tests: 1
Click here to see the affected service packages
- compute
Action taken
Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy
$\textcolor{red}{\textsf{Tests failed during RECORDING mode:}}$
TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy[Error message] [Debug log]
$\textcolor{red}{\textsf{Please fix these to complete your PR.}}$ View the build log or the debug log for each test
@GoogleCloudPlatform/terraform-team This PR has been waiting for review for 1 week. Please take a look! Use the label disable-review-reminders to disable these notifications.
@GoogleCloudPlatform/terraform-team @roaks3 This PR has been waiting for review for 2 weeks. Please take a look! Use the label disable-review-reminders to disable these notifications.
This PR is approved and has been waiting for merge for 1 week. Is it ready to merge? Use the label disable-review-reminders to disable these notifications.
Follow-up note: this should be changed to point to the feature branch since it is a breaking change (see https://googlecloudplatform.github.io/magic-modules/develop/breaking-changes/make-a-breaking-change/)
Follow-up note: this should be changed to point to the feature branch since it is a breaking change (see https://googlecloudplatform.github.io/magic-modules/develop/breaking-changes/make-a-breaking-change/)
Thanks @roaks3 I just rebased, hopefully on the correct branch. Please, let me know if there's anything else I should do.