New Resource & Data Source: azurerm_data_migration_service #5258

magodo · 2019-12-26T02:05:36Z

Add new resource: data migration service.

This is the first resource under DataMigration resource provider.

There are more resources to come (which are not included in this PR):

data migration project
data migration task

I will implement those two resources after this PR is merged since they depends on data migration service.

Check feature request #5257 for more details.

magodo · 2019-12-26T03:05:26Z

This PR depends on #5259

katbyte

Thanks for the PR @magodo, iv'e left some comments inline that need to be addressed before we merge this

azurerm/internal/services/datamigration/data_source_data_migration_service.go

azurerm/internal/services/datamigration/resource_arm_data_migration_service.go

azurerm/internal/services/datamigration/tests/resource_arm_data_migration_service_test.go

website/docs/d/data_migration_service.html.markdown

website/docs/r/data_migration_service.html.markdown

katbyte

Thanks for the PR @magodo, iv'e left some comments inline that need to be addressed before we merge this

Mostly include: - order schema - add validation func for `name`, `sku_name` - unexport `provisioning_state` - unexport `delete_running_task` - add nil check for `sku` - add dedicated update acctest - modify naming in acctest config - modify document for r/d

katbyte

Thanks for the revisions @magodo, i've left some more comments inline

azurerm/internal/services/datamigration/data_source_data_migration_service.go

azurerm/internal/services/datamigration/resource_arm_data_migration_service.go

azurerm/internal/services/datamigration/data_source_data_migration_service.go

azurerm/internal/services/datamigration/resource_arm_data_migration_service.go

azurerm/internal/services/datamigration/data_source_data_migration_service.go

azurerm/internal/services/datamigration/resource_arm_data_migration_service.go

azurerm/internal/services/datamigration/tests/resource_arm_data_migration_service_test.go

tombuildsstuff

hey @magodo

Thanks for this PR :)

Taking a look through this is looking good to me - if we can remove the data source, fix up the pending comments (and the tests pass) then this should otherwise be good 👍

Thanks!

azurerm/internal/services/datamigration/resource_arm_data_migration_service.go

azurerm/internal/services/datamigration/data_source_data_migration_service.go

azurerm/internal/services/datamigration/tests/resource_arm_data_migration_service_test.go

website/docs/r/data_migration_service.html.markdown

azurerm/internal/services/datamigration/resource_arm_data_migration_service.go

Changes include: - remove `kind` property from resource - `resource_group_name` schema case sensitive - acctest always ended with a import test step - document changes TBD: - Whether need to remove data source? - Shall we show this resource on website sidebar?

katbyte

Thanks for the updates @magodo, in addition to my comments it looks like you have a test failure:

Test Failed

------- Stdout: -------
=== RUN   TestAccAzureRMDataMigrationService_complete

panic: Invalid address to set: []string{"id"}



goroutine 328 [running]:

github.com/hashicorp/terraform-plugin-sdk/helper/schema.(*ResourceData).Set(0xc000a317a0, 0x4061c0c, 0x2, 0x39b0140, 0xc0013a0ea0, 0x0, 0xdf8475800)

	/opt/teamcity-agent/work/dbea4b84960ec72a/pkg/mod/github.com/hashicorp/[email protected]/helper/schema/resource_data.go:193 +0x354

github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/datamigration.resourceArmDataMigrationServiceRead(0xc000a317a0, 0x3aa3100, 0xc001122240, 0x0, 0x0)

	/opt/teamcity-agent/work/dbea4b84960ec72a/src/github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/datamigration/resource_arm_data_migration_service.go:158 +0x5ba

github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/datamigration.resourceArmDataMigrationServiceCreate(0xc000a317a0, 0x3aa3100, 0xc001122240, 0x0, 0x0)

	/opt/teamcity-agent/work/dbea4b84960ec72a/src/github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/datamigration/resource_arm_data_migration_service.go:133 +0xa38

github.com/hashicorp/terraform-plugin-sdk/helper/schema.(*Resource).Apply(0xc000bfb980, 0xc000796050, 0xc00162c2a0, 0x3aa3100, 0xc001122240, 0xc001630301, 0xc001984150, 0xc0016303d0)

	/opt/teamcity-agent/work/dbea4b84960ec72a/pkg/mod/github.com/hashicorp/[email protected]/helper/schema/resource.go:305 +0x365

github.com/hashicorp/terraform-plugin-sdk/helper/schema.(*Provider).Apply(0xc00148e400, 0xc001039918, 0xc000796050, 0xc00162c2a0, 0xc0016303e0, 0xc0008b9160, 0x3a7eae0)

	/opt/teamcity-agent/work/dbea4b84960ec72a/pkg/mod/github.com/hashicorp/[email protected]/helper/schema/provider.go:294 +0x99

github.com/hashicorp/terraform-plugin-sdk/internal/helper/plugin.(*GRPCProviderServer).ApplyResourceChange(0xc0008b84b8, 0x47dcf00, 0xc001942cc0, 0xc0015e3680, 0xc0008b84b8, 0xc001942cc0, 0xc00255da80)

	/opt/teamcity-agent/work/dbea4b84960ec72a/pkg/mod/github.com/hashicorp/[email protected]/internal/helper/plugin/grpc_provider.go:885 +0x882

github.com/hashicorp/terraform-plugin-sdk/internal/tfplugin5._Provider_ApplyResourceChange_Handler(0x3e50c60, 0xc0008b84b8, 0x47dcf00, 0xc001942cc0, 0xc0015e3620, 0x0, 0x47dcf00, 0xc001942cc0, 0xc001629180, 0x370)

	/opt/teamcity-agent/work/dbea4b84960ec72a/pkg/mod/github.com/hashicorp/[email protected]/internal/tfplugin5/tfplugin5.pb.go:3189 +0x217

google.golang.org/grpc.(*Server).processUnaryRPC(0xc002be5080, 0x482eee0, 0xc001252000, 0xc0015f9100, 0xc0013fc0c0, 0x71d1ae0, 0x0, 0x0, 0x0)

	/opt/teamcity-agent/work/dbea4b84960ec72a/pkg/mod/google.golang.org/[email protected]/server.go:995 +0x460

google.golang.org/grpc.(*Server).handleStream(0xc002be5080, 0x482eee0, 0xc001252000, 0xc0015f9100, 0x0)

	/opt/teamcity-agent/work/dbea4b84960ec72a/pkg/mod/google.golang.org/[email protected]/server.go:1275 +0xd97

google.golang.org/grpc.(*Server).serveStreams.func1.1(0xc001097ea0, 0xc002be5080, 0x482eee0, 0xc001252000, 0xc0015f9100)

	/opt/teamcity-agent/work/dbea4b84960ec72a/pkg/mod/google.golang.org/[email protected]/server.go:710 +0xbb

created by google.golang.org/grpc.(*Server).serveStreams.func1

	/opt/teamcity-agent/work/dbea4b84960ec72a/pkg/mod/google.golang.org/[email protected]/server.go:708 +0xa1

FAIL	github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/datamigration/tests	942.276s

azurerm/internal/services/datamigration/data_source_data_migration_service.go

azurerm/internal/services/datamigration/resource_arm_data_migration_service.go

website/docs/d/data_migration_service.html.markdown

website/docs/r/data_migration_service.html.markdown

website/docs/d/data_migration_service.html.markdown

website/docs/r/data_migration_service.html.markdown

magodo · 2020-01-01T10:32:10Z

Hi Kat, I rerun the test locally after apply your review comment, it works as expected. Not sure the cause of the error attached above.

katbyte · 2020-01-02T02:51:30Z

@magodo, we run tests with TF_SCHEMA_PANIC_ON_ERROR="1" as it'll do more checks such as erroring when you try and set an invalid property

magodo · 2020-01-02T05:10:19Z

@katbyte, I run the acc test via makefile of the project. I think it had already export TF_SCHEMA_PANIC_ON_ERROR beforehead. Would you please recheck? (maybe the error has something to do with the review comment you pointed previously, about set Id a second time?)

azurerm/internal/services/datamigration/resource_arm_data_migration_service.go

katbyte · 2020-01-09T22:15:29Z

Tests are passing for us now 🎉

katbyte

Thanks for the changes @magodo, LGTM 👍

katbyte

@magodo, looks like CI errors needs to be fixed up, but once they are thus should be gtg

magodo · 2020-02-03T03:10:48Z

@katbyte
The remaining CI error is because that the list of allowed resource providers doesn't contain this new resource provider.

1. rename resource to "database_migration_service" to comply to the naming of azure official document. 2. introduce custom id parse function.

katbyte

Thanks for the revisions @magodo,

I've left some more (mostly) minor comments inline that one addressed this should be good to merge!

azurerm/internal/services/databasemigration/data_source_database_migration_service.go

azurerm/internal/services/databasemigration/resource_arm_database_migration_service.go

...rm/internal/services/databasemigration/tests/resource_arm_database_migration_service_test.go

website/docs/r/database_migration_service.html.markdown

[review](hashicorp#5258 (review))

katbyte

Thanks for those changes @magodo. LGTM now 👍

ghost · 2020-04-04T14:51:54Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

new resource: data migration service

624ead8

ghost added size/XL documentation labels Dec 26, 2019

katbyte requested changes Dec 26, 2019

View reviewed changes

katbyte added the service/database-migration label Dec 26, 2019

vendor: vendoring datamigration version 2018-04-19

7b41cf4

ghost added size/XXL dependencies and removed size/XL labels Dec 26, 2019

Modify per review comment

de27be1

Mostly include: - order schema - add validation func for `name`, `sku_name` - unexport `provisioning_state` - unexport `delete_running_task` - add nil check for `sku` - add dedicated update acctest - modify naming in acctest config - modify document for r/d

magodo force-pushed the data_migration branch from b575f77 to de27be1 Compare December 26, 2019 08:54

magodo requested a review from katbyte December 26, 2019 08:55

katbyte requested changes Dec 28, 2019

View reviewed changes

katbyte added the waiting-response label Dec 28, 2019

refine after 2nd review

653ac00

magodo requested a review from katbyte December 29, 2019 02:26

tombuildsstuff suggested changes Dec 31, 2019

View reviewed changes

magodo requested a review from tombuildsstuff December 31, 2019 08:42

katbyte requested changes Jan 1, 2020

View reviewed changes

modification per 4th review

433328a

ghost removed the waiting-response label Jan 1, 2020

magodo requested a review from katbyte January 1, 2020 10:32

katbyte requested changes Jan 9, 2020

View reviewed changes

azurerm/internal/services/datamigration/resource_arm_data_migration_service.go Outdated Show resolved Hide resolved

magodo requested a review from katbyte January 10, 2020 01:32

Update data_migration_service.html.markdown

2134440

katbyte changed the title ~~new resource: data migration service~~ New Resource & Data Source: azurerm_data_migration_service Jan 25, 2020

katbyte approved these changes Jan 25, 2020

View reviewed changes

Merge branch 'master' into data_migration

431f457

katbyte added this to the v1.42.0 milestone Jan 25, 2020

katbyte requested changes Jan 25, 2020

View reviewed changes

magodo added 3 commits January 27, 2020 09:49

fix pkg dep non consistent issue

1d76d65

remove sidebar_current from doc

c922193

format terraform cfg snippet

8e0b6da

katbyte modified the milestones: v1.42.0, v1.43.0 Jan 27, 2020

magodo requested a review from katbyte January 29, 2020 05:52

ghost removed the waiting-response label Feb 3, 2020

tombuildsstuff removed this from the v1.43.0 milestone Feb 3, 2020

magodo added 3 commits February 20, 2020 13:43

rename resource and update for latest code style

1214bec

1. rename resource to "database_migration_service" to comply to the naming of azure official document. 2. introduce custom id parse function.

Merge remote-tracking branch 'upstream/master' into data_migration

d36887a

Pass CI

a446994

katbyte requested changes Mar 4, 2020

View reviewed changes

magodo added 2 commits March 4, 2020 14:22

modify per review

0c0b1c2

[review](hashicorp#5258 (review))

doc add provider header

ff6ea33

magodo requested a review from katbyte March 4, 2020 08:49

magodo and others added 2 commits March 4, 2020 20:16

rename resource in import check

d60b618

Update database_migration_service.html.markdown

bc0c4b9

katbyte approved these changes Mar 4, 2020

View reviewed changes

katbyte merged commit a5267dd into hashicorp:master Mar 4, 2020

katbyte added a commit that referenced this pull request Mar 4, 2020

Update CHANGELOG.mdto include #5258

fa7b9eb

ghost locked and limited conversation to collaborators Apr 4, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New Resource & Data Source: azurerm_data_migration_service #5258

New Resource & Data Source: azurerm_data_migration_service #5258

magodo commented Dec 26, 2019 •

edited

Loading

magodo commented Dec 26, 2019

katbyte left a comment

katbyte left a comment

katbyte left a comment

tombuildsstuff left a comment

katbyte left a comment

magodo commented Jan 1, 2020

katbyte commented Jan 2, 2020

magodo commented Jan 2, 2020

katbyte commented Jan 9, 2020 •

edited

Loading

katbyte left a comment

katbyte left a comment

magodo commented Feb 3, 2020

katbyte left a comment

katbyte left a comment

ghost commented Apr 4, 2020

New Resource & Data Source: azurerm_data_migration_service #5258

New Resource & Data Source: azurerm_data_migration_service #5258

Conversation

magodo commented Dec 26, 2019 • edited Loading

magodo commented Dec 26, 2019

katbyte left a comment

Choose a reason for hiding this comment

katbyte left a comment

Choose a reason for hiding this comment

katbyte left a comment

Choose a reason for hiding this comment

tombuildsstuff left a comment

Choose a reason for hiding this comment

katbyte left a comment

Choose a reason for hiding this comment

magodo commented Jan 1, 2020

katbyte commented Jan 2, 2020

magodo commented Jan 2, 2020

katbyte commented Jan 9, 2020 • edited Loading

katbyte left a comment

Choose a reason for hiding this comment

katbyte left a comment

Choose a reason for hiding this comment

magodo commented Feb 3, 2020

katbyte left a comment

Choose a reason for hiding this comment

katbyte left a comment

Choose a reason for hiding this comment

ghost commented Apr 4, 2020

magodo commented Dec 26, 2019 •

edited

Loading

katbyte commented Jan 9, 2020 •

edited

Loading