New Resource: `azurerm_mobile_network_data_network`; New DataSource: `azurerm_mobile_network_data_network` #20338

ziyeqf · 2023-02-07T04:30:18Z

an overall list is below. Detail of this RP could be found here

Though these PRs may have conflicts in merge sequence, but I think it should not block the review progress...

Over view

order | name 
0     | azurerm_mobile_network                              ✅
1     | azurerm_mobile_network_site                         #20334
1     | azurerm_mobile_network_data_network                 <- this one
1     | azurerm_mobile_network_service                      #20337
1     | azurerm_mobile_network_slice                        #20336 
1     | azurerm_mobile_network_sim_group                    #20339
2     | azurerm_mobile_network_packet_core_control_plane    
2     | azurerm_mobile_network_attached_data_network        
2     | azurerm_mobile_network_sim_policy                   
3     | azurerm_mobile_network_packet_core_data_plane       
4     | azurerm_mobile_network_attached_data_network        
5     | azurerm_mobile_network_sim

Test

terraform-provider-azurerm ❯ TF_ACC=1 go test -v ./internal/services/mobilenetwork -run=TestAccMobileNetworkDataNetwork -timeout=600m                                                  [ tengzh/mn_dn][$+][🐹 v1.19.5][⏱ 3s][12:14:35]
=== RUN   TestAccMobileNetworkDataNetworkDataSource_complete
=== PAUSE TestAccMobileNetworkDataNetworkDataSource_complete
=== RUN   TestAccMobileNetworkDataNetwork_basic
=== PAUSE TestAccMobileNetworkDataNetwork_basic
=== RUN   TestAccMobileNetworkDataNetwork_requiresImport
=== PAUSE TestAccMobileNetworkDataNetwork_requiresImport
=== RUN   TestAccMobileNetworkDataNetwork_complete
=== PAUSE TestAccMobileNetworkDataNetwork_complete
=== RUN   TestAccMobileNetworkDataNetwork_update
=== PAUSE TestAccMobileNetworkDataNetwork_update
=== CONT  TestAccMobileNetworkDataNetworkDataSource_complete
=== CONT  TestAccMobileNetworkDataNetwork_complete
=== CONT  TestAccMobileNetworkDataNetwork_update
=== CONT  TestAccMobileNetworkDataNetwork_requiresImport
=== CONT  TestAccMobileNetworkDataNetwork_basic
--- PASS: TestAccMobileNetworkDataNetworkDataSource_complete (421.15s)
--- PASS: TestAccMobileNetworkDataNetwork_basic (429.70s)
--- PASS: TestAccMobileNetworkDataNetwork_complete (432.41s)
--- PASS: TestAccMobileNetworkDataNetwork_requiresImport (449.69s)
--- PASS: TestAccMobileNetworkDataNetwork_update (515.62s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/mobilenetwork 516.867s

…`azurerm_mobile_network_data_network`

katbyte

LGTM ⛈️

catriona-m

Thanks @ziyeqf I've left two minor comments here but otherwise this LGTM!

website/docs/d/mobile_network_data_network.html.markdown

internal/services/mobilenetwork/mobile_network_data_network_resource_test.go

catriona-m

Thanks @ziyeqf LGTM!

catriona-m · 2023-02-14T13:42:18Z

Looks like this has some conflicts from merging the previous mobilenetwork PR, but we can marge this once these get resolved 👍

tombuildsstuff

hey @ziyeqf

Thanks for pushing those changes - I've taken a look through and left some comments inline, if we can fix those up then this should otherwise be good to go 👍

Thanks!

internal/services/mobilenetwork/mobile_network_data_network_resource.go

tombuildsstuff · 2023-02-15T10:17:04Z

internal/services/mobilenetwork/mobile_network_data_network_resource.go

+				return fmt.Errorf("deleting %s: %+v", id, err)
+			}
+
+			if err := resourceMobileNetworkChildWaitForDeletion(ctx, id.ID(), func() (*http.Response, error) {


is there an API bug tracking this, so we know when this is safe to remove?

It's worth noting that these workarounds shouldn't be needed in time with the new base layer, since that should both check the LRO and continue polling until the resource is fully provisioned/gone

I just know it should because some infra issue.. actually what I need is some delay, the resource has already been 404, but when delete the parent resource, it will get an error.. I can open an issue but I dont think it could be fixed..

just opened an issue for this..Azure/azure-rest-api-specs#22691

Also, it might be better to add the workaround on delete function of azurerm_mobile_network instead of on children resources, WDYT?

One of the design guarantees of Terraform is that it won't mark a resource as provisioned/gone until the resource is fully provisioned/removed - so we'd need this to be present within the child resources rather than the parent azurerm_mobile_network resource

website/docs/d/mobile_network_data_network.html.markdown

website/docs/r/mobile_network_data_network.html.markdown

tombuildsstuff

hey @ziyeqf

Thanks for pushing those changes - taking a look through here this is mostly looking good to me now, I've left some minor comments inline (and called out a few FYI's, which aren't blockers) but if we can fix those up this should otherwise be good to go 👍

Thanks!

tombuildsstuff · 2023-02-22T14:55:00Z

internal/services/mobilenetwork/client/client.go

+	"github.com/hashicorp/go-azure-sdk/resource-manager/mobilenetwork/2022-11-01/datanetwork"
 	"github.com/hashicorp/go-azure-sdk/resource-manager/mobilenetwork/2022-11-01/mobilenetwork"
 	"github.com/hashicorp/go-azure-sdk/resource-manager/mobilenetwork/2022-11-01/service"
 	"github.com/hashicorp/go-azure-sdk/resource-manager/mobilenetwork/2022-11-01/simgroup"


This isn't something we need to change now, but I wanted to call out that hashicorp/go-azure-sdk supports Meta Clients for each API Version - so if an entire Service is using a single API Version (as is the case here) - rather than defining the Client and the Resources to use inline (as we're doing here) we can instead use the Meta Client as per datadog to automatically have access to all clients, rather than defining these by hand.

Again, this isn't something we need to change now, but should make it easier to import the Client one-time (since we try and stick to a single API Version for each Service where possible) rather than adding these across different PR's :)

internal/services/mobilenetwork/mobile_network_data_network_resource.go

tombuildsstuff · 2023-02-22T15:00:16Z

internal/services/mobilenetwork/mobile_network_data_network_resource.go

+				return fmt.Errorf("deleting %s: %+v", id, err)
+			}
+
+			if err := resourceMobileNetworkChildWaitForDeletion(ctx, id.ID(), func() (*http.Response, error) {


One of the design guarantees of Terraform is that it won't mark a resource as provisioned/gone until the resource is fully provisioned/removed - so we'd need this to be present within the child resources rather than the parent azurerm_mobile_network resource

internal/services/mobilenetwork/mobile_network_data_network_resource.go

ziyeqf · 2023-02-27T06:44:24Z

test

terraform-provider-azurerm ❯ TF_ACC=1 go test -v ./internal/services/mobilenetwork  -run=TestAccMobileNetworkDataNetwork -timeout=600m                                                 [ tengzh/mn_dn][$!][🐹 v1.19.6][⏱ 1s][13:57:32]
=== RUN   TestAccMobileNetworkDataNetworkDataSource_complete
=== PAUSE TestAccMobileNetworkDataNetworkDataSource_complete
=== RUN   TestAccMobileNetworkDataNetwork_basic
=== PAUSE TestAccMobileNetworkDataNetwork_basic
=== RUN   TestAccMobileNetworkDataNetwork_requiresImport
=== PAUSE TestAccMobileNetworkDataNetwork_requiresImport
=== RUN   TestAccMobileNetworkDataNetwork_complete
=== PAUSE TestAccMobileNetworkDataNetwork_complete
=== RUN   TestAccMobileNetworkDataNetwork_update
=== PAUSE TestAccMobileNetworkDataNetwork_update
=== CONT  TestAccMobileNetworkDataNetworkDataSource_complete
=== CONT  TestAccMobileNetworkDataNetwork_complete
=== CONT  TestAccMobileNetworkDataNetwork_requiresImport
=== CONT  TestAccMobileNetworkDataNetwork_update
=== CONT  TestAccMobileNetworkDataNetwork_basic
--- PASS: TestAccMobileNetworkDataNetwork_basic (677.01s)
--- PASS: TestAccMobileNetworkDataNetwork_complete (679.38s)
--- PASS: TestAccMobileNetworkDataNetworkDataSource_complete (691.17s)
--- PASS: TestAccMobileNetworkDataNetwork_requiresImport (709.30s)
--- PASS: TestAccMobileNetworkDataNetwork_update (918.91s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/mobilenetwork 920.113s

katbyte

Thanks @ziyeqf - LGTM ☎️

github-actions · 2023-03-06T14:44:11Z

This functionality has been released in v3.46.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

github-actions · 2023-04-06T02:08:32Z

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

New Resource: azurerm_mobile_network_data_network; New DataSource: …

5c070d4

…`azurerm_mobile_network_data_network`

github-actions bot added dependencies documentation service/mobile-network size/XL labels Feb 7, 2023

remove unused forcenew

a829033

katbyte approved these changes Feb 10, 2023

View reviewed changes

ziyeqf added 3 commits February 10, 2023 13:54

Merge branch 'main' into tengzh/mn_dn

c222e16

Merge branch 'main' into tengzh/mn_dn

e5c7fd8

update code

b7c24bf

catriona-m requested changes Feb 13, 2023

View reviewed changes

website/docs/d/mobile_network_data_network.html.markdown Outdated Show resolved Hide resolved

internal/services/mobilenetwork/mobile_network_data_network_resource_test.go Outdated Show resolved Hide resolved

github-actions bot added the waiting-response label Feb 13, 2023

update doc

8ac5f43

github-actions bot removed the waiting-response label Feb 14, 2023

catriona-m approved these changes Feb 14, 2023

View reviewed changes

Merge branch 'main' into tengzh/mn_dn

885a9f5

tombuildsstuff suggested changes Feb 15, 2023

View reviewed changes

github-actions bot added the waiting-response label Feb 15, 2023

update per comments

48f4a34

github-actions bot removed the waiting-response label Feb 15, 2023

ziyeqf added 3 commits February 15, 2023 19:18

Merge remote-tracking branch 'hashi/main' into tengzh/mn_dn

0c5cf5d

add comment

59f49cf

Merge remote-tracking branch 'hashi/main' into tengzh/mn_dn

1f5bddc

ziyeqf requested a review from tombuildsstuff February 21, 2023 03:00

tombuildsstuff reviewed Feb 22, 2023

View reviewed changes

github-actions bot added the waiting-response label Feb 22, 2023

update code per comments

122583c

github-actions bot removed the waiting-response label Feb 23, 2023

katbyte added the waiting-response label Feb 24, 2023

update Update timeout

4693303

github-actions bot removed the waiting-response label Feb 27, 2023

katbyte approved these changes Feb 28, 2023

View reviewed changes

katbyte merged commit 1f1567a into hashicorp:main Feb 28, 2023

github-actions bot added this to the v3.46.0 milestone Feb 28, 2023

katbyte added a commit that referenced this pull request Feb 28, 2023

CHANGELOG #20338

3fb97ff

squaremo mentioned this pull request Mar 14, 2023

Upgrade terraform-provider-azurerm to v3.47.0 pulumi/pulumi-azure#1231

Closed

5 tasks

github-actions bot locked as resolved and limited conversation to collaborators Apr 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Resource: `azurerm_mobile_network_data_network`; New DataSource: `azurerm_mobile_network_data_network` #20338

New Resource: `azurerm_mobile_network_data_network`; New DataSource: `azurerm_mobile_network_data_network` #20338

ziyeqf commented Feb 7, 2023 •

edited

Loading

katbyte left a comment

catriona-m left a comment

catriona-m left a comment

catriona-m commented Feb 14, 2023

tombuildsstuff left a comment

tombuildsstuff Feb 15, 2023

ziyeqf Feb 15, 2023

ziyeqf Feb 17, 2023

ziyeqf Feb 17, 2023

tombuildsstuff Feb 22, 2023

tombuildsstuff left a comment

tombuildsstuff Feb 22, 2023

tombuildsstuff Feb 22, 2023

ziyeqf commented Feb 27, 2023

katbyte left a comment

github-actions bot commented Mar 6, 2023

github-actions bot commented Apr 6, 2023

New Resource: azurerm_mobile_network_data_network; New DataSource: azurerm_mobile_network_data_network #20338

New Resource: azurerm_mobile_network_data_network; New DataSource: azurerm_mobile_network_data_network #20338

Conversation

ziyeqf commented Feb 7, 2023 • edited Loading

Over view

Test

katbyte left a comment

Choose a reason for hiding this comment

catriona-m left a comment

Choose a reason for hiding this comment

catriona-m left a comment

Choose a reason for hiding this comment

catriona-m commented Feb 14, 2023

tombuildsstuff left a comment

Choose a reason for hiding this comment

tombuildsstuff Feb 15, 2023

Choose a reason for hiding this comment

ziyeqf Feb 15, 2023

Choose a reason for hiding this comment

ziyeqf Feb 17, 2023

Choose a reason for hiding this comment

ziyeqf Feb 17, 2023

Choose a reason for hiding this comment

tombuildsstuff Feb 22, 2023

Choose a reason for hiding this comment

tombuildsstuff left a comment

Choose a reason for hiding this comment

tombuildsstuff Feb 22, 2023

Choose a reason for hiding this comment

tombuildsstuff Feb 22, 2023

Choose a reason for hiding this comment

ziyeqf commented Feb 27, 2023

test

katbyte left a comment

Choose a reason for hiding this comment

github-actions bot commented Mar 6, 2023

github-actions bot commented Apr 6, 2023

New Resource: `azurerm_mobile_network_data_network`; New DataSource: `azurerm_mobile_network_data_network` #20338

New Resource: `azurerm_mobile_network_data_network`; New DataSource: `azurerm_mobile_network_data_network` #20338

ziyeqf commented Feb 7, 2023 •

edited

Loading