From 5cad27bb2e5e316005ccc424ab69e38c1d3e8355 Mon Sep 17 00:00:00 2001 From: Paul Stack Date: Wed, 5 Apr 2017 06:48:37 +0100 Subject: [PATCH] provider/aws: Migrate aws_dms_* resources away from AWS waiters (#13291) The AWS waiter package has changed location in the 1.8.0 version of the SDK. DMS will need to mitigate a breaking change because of this Between @radeksimko and myself, we think that we should migrate the DMS resources to using the Terraform state refresh func pattern that is used across the entire of the AWS provider. DMS is the *only* resource that currently uses the AWS waiters, so the LOE to migrate is pretty low --- .../aws/resource_aws_dms_endpoint.go | 40 +--- .../aws/resource_aws_dms_endpoint_test.go | 39 ++-- .../resource_aws_dms_replication_instance.go | 137 +++++++------ ...ource_aws_dms_replication_instance_test.go | 63 ++---- ...e_aws_dms_replication_subnet_group_test.go | 22 -- .../aws/resource_aws_dms_replication_task.go | 188 +++++++----------- .../resource_aws_dms_replication_task_test.go | 85 +++----- 7 files changed, 201 insertions(+), 373 deletions(-) diff --git a/builtin/providers/aws/resource_aws_dms_endpoint.go b/builtin/providers/aws/resource_aws_dms_endpoint.go index 07cd7a272..586ed9f7c 100644 --- a/builtin/providers/aws/resource_aws_dms_endpoint.go +++ b/builtin/providers/aws/resource_aws_dms_endpoint.go @@ -6,7 +6,6 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" - "github.com/aws/aws-sdk-go/private/waiter" dms "github.com/aws/aws-sdk-go/service/databasemigrationservice" "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/helper/validation" @@ -168,6 +167,7 @@ func resourceAwsDmsEndpointRead(d *schema.ResourceData, meta interface{}) error }) if err != nil { if dmserr, ok := err.(awserr.Error); ok && dmserr.Code() == "ResourceNotFoundFault" { + log.Printf("[DEBUG] DMS Replication Endpoint %q Not Found", d.Id()) d.SetId("") return nil } @@ -283,11 +283,6 @@ func resourceAwsDmsEndpointDelete(d *schema.ResourceData, meta interface{}) erro return err } - waitErr := waitForEndpointDelete(conn, d.Get("endpoint_id").(string), 30, 20) - if waitErr != nil { - return waitErr - } - return nil } @@ -310,36 +305,3 @@ func resourceAwsDmsEndpointSetState(d *schema.ResourceData, endpoint *dms.Endpoi return nil } - -func waitForEndpointDelete(client *dms.DatabaseMigrationService, endpointId string, delay int, maxAttempts int) error { - input := &dms.DescribeEndpointsInput{ - Filters: []*dms.Filter{ - { - Name: aws.String("endpoint-id"), - Values: []*string{aws.String(endpointId)}, - }, - }, - } - - config := waiter.Config{ - Operation: "DescribeEndpoints", - Delay: delay, - MaxAttempts: maxAttempts, - Acceptors: []waiter.WaitAcceptor{ - { - State: "success", - Matcher: "path", - Argument: "length(Endpoints[]) > `0`", - Expected: false, - }, - }, - } - - w := waiter.Waiter{ - Client: client, - Input: input, - Config: config, - } - - return w.Wait() -} diff --git a/builtin/providers/aws/resource_aws_dms_endpoint_test.go b/builtin/providers/aws/resource_aws_dms_endpoint_test.go index 6f8662283..59c3d87c7 100644 --- a/builtin/providers/aws/resource_aws_dms_endpoint_test.go +++ b/builtin/providers/aws/resource_aws_dms_endpoint_test.go @@ -8,7 +8,6 @@ import ( dms "github.com/aws/aws-sdk-go/service/databasemigrationservice" "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/resource" - "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/terraform" ) @@ -67,11 +66,6 @@ func dmsEndpointDestroy(s *terraform.State) error { } func checkDmsEndpointExists(n string) resource.TestCheckFunc { - providers := []*schema.Provider{testAccProvider} - return checkDmsEndpointExistsWithProviders(n, &providers) -} - -func checkDmsEndpointExistsWithProviders(n string, providers *[]*schema.Provider) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] if !ok { @@ -81,29 +75,26 @@ func checkDmsEndpointExistsWithProviders(n string, providers *[]*schema.Provider if rs.Primary.ID == "" { return fmt.Errorf("No ID is set") } - for _, provider := range *providers { - // Ignore if Meta is empty, this can happen for validation providers - if provider.Meta() == nil { - continue - } - conn := provider.Meta().(*AWSClient).dmsconn - _, err := conn.DescribeEndpoints(&dms.DescribeEndpointsInput{ - Filters: []*dms.Filter{ - { - Name: aws.String("endpoint-id"), - Values: []*string{aws.String(rs.Primary.ID)}, - }, + conn := testAccProvider.Meta().(*AWSClient).dmsconn + resp, err := conn.DescribeEndpoints(&dms.DescribeEndpointsInput{ + Filters: []*dms.Filter{ + { + Name: aws.String("endpoint-id"), + Values: []*string{aws.String(rs.Primary.ID)}, }, - }) + }, + }) - if err != nil { - return fmt.Errorf("DMS endpoint error: %v", err) - } - return nil + if err != nil { + return fmt.Errorf("DMS endpoint error: %v", err) } - return fmt.Errorf("DMS endpoint not found") + if resp.Endpoints == nil { + return fmt.Errorf("DMS endpoint not found") + } + + return nil } } diff --git a/builtin/providers/aws/resource_aws_dms_replication_instance.go b/builtin/providers/aws/resource_aws_dms_replication_instance.go index 63552d28e..2b6948936 100644 --- a/builtin/providers/aws/resource_aws_dms_replication_instance.go +++ b/builtin/providers/aws/resource_aws_dms_replication_instance.go @@ -3,11 +3,12 @@ package aws import ( "fmt" "log" + "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" - "github.com/aws/aws-sdk-go/private/waiter" dms "github.com/aws/aws-sdk-go/service/databasemigrationservice" + "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/helper/schema" ) @@ -174,12 +175,23 @@ func resourceAwsDmsReplicationInstanceCreate(d *schema.ResourceData, meta interf return err } - err = waitForInstanceCreated(conn, d.Get("replication_instance_id").(string), 30, 20) + d.SetId(d.Get("replication_instance_id").(string)) + + stateConf := &resource.StateChangeConf{ + Pending: []string{"creating"}, + Target: []string{"available"}, + Refresh: resourceAwsDmsReplicationInstanceStateRefreshFunc(d, meta), + Timeout: d.Timeout(schema.TimeoutCreate), + MinTimeout: 10 * time.Second, + Delay: 30 * time.Second, // Wait 30 secs before starting + } + + // Wait, catching any errors + _, err = stateConf.WaitForState() if err != nil { return err } - d.SetId(d.Get("replication_instance_id").(string)) return resourceAwsDmsReplicationInstanceRead(d, meta) } @@ -196,6 +208,7 @@ func resourceAwsDmsReplicationInstanceRead(d *schema.ResourceData, meta interfac }) if err != nil { if dmserr, ok := err.(awserr.Error); ok && dmserr.Code() == "ResourceNotFoundFault" { + log.Printf("[DEBUG] DMS Replication Instance %q Not Found", d.Id()) d.SetId("") return nil } @@ -287,6 +300,21 @@ func resourceAwsDmsReplicationInstanceUpdate(d *schema.ResourceData, meta interf return err } + stateConf := &resource.StateChangeConf{ + Pending: []string{"modifying"}, + Target: []string{"available"}, + Refresh: resourceAwsDmsReplicationInstanceStateRefreshFunc(d, meta), + Timeout: d.Timeout(schema.TimeoutCreate), + MinTimeout: 10 * time.Second, + Delay: 30 * time.Second, // Wait 30 secs before starting + } + + // Wait, catching any errors + _, err = stateConf.WaitForState() + if err != nil { + return err + } + return resourceAwsDmsReplicationInstanceRead(d, meta) } @@ -307,9 +335,19 @@ func resourceAwsDmsReplicationInstanceDelete(d *schema.ResourceData, meta interf return err } - waitErr := waitForInstanceDeleted(conn, d.Get("replication_instance_id").(string), 30, 20) - if waitErr != nil { - return waitErr + stateConf := &resource.StateChangeConf{ + Pending: []string{"deleting"}, + Target: []string{}, + Refresh: resourceAwsDmsReplicationInstanceStateRefreshFunc(d, meta), + Timeout: d.Timeout(schema.TimeoutCreate), + MinTimeout: 10 * time.Second, + Delay: 30 * time.Second, // Wait 30 secs before starting + } + + // Wait, catching any errors + _, err = stateConf.WaitForState() + if err != nil { + return err } return nil @@ -355,68 +393,35 @@ func resourceAwsDmsReplicationInstanceSetState(d *schema.ResourceData, instance return nil } -func waitForInstanceCreated(client *dms.DatabaseMigrationService, id string, delay int, maxAttempts int) error { - input := &dms.DescribeReplicationInstancesInput{ - Filters: []*dms.Filter{ - { - Name: aws.String("replication-instance-id"), - Values: []*string{aws.String(id)}, +func resourceAwsDmsReplicationInstanceStateRefreshFunc( + d *schema.ResourceData, meta interface{}) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + conn := meta.(*AWSClient).dmsconn + + v, err := conn.DescribeReplicationInstances(&dms.DescribeReplicationInstancesInput{ + Filters: []*dms.Filter{ + { + Name: aws.String("replication-instance-id"), + Values: []*string{aws.String(d.Id())}, // Must use d.Id() to work with import. + }, }, - }, - } + }) + if err != nil { + if dmserr, ok := err.(awserr.Error); ok && dmserr.Code() == "ResourceNotFoundFault" { + return nil, "", nil + } + log.Printf("Error on retrieving DMS Replication Instance when waiting: %s", err) + return nil, "", err + } - config := waiter.Config{ - Operation: "DescribeReplicationInstances", - Delay: delay, - MaxAttempts: maxAttempts, - Acceptors: []waiter.WaitAcceptor{ - { - State: "success", - Matcher: "pathAll", - Argument: "ReplicationInstances[].ReplicationInstanceStatus", - Expected: "available", - }, - }, - } + if v == nil { + return nil, "", nil + } - w := waiter.Waiter{ - Client: client, - Input: input, - Config: config, - } + if v.ReplicationInstances == nil { + return nil, "", fmt.Errorf("Error on retrieving DMS Replication Instance when waiting for State") + } - return w.Wait() -} - -func waitForInstanceDeleted(client *dms.DatabaseMigrationService, id string, delay int, maxAttempts int) error { - input := &dms.DescribeReplicationInstancesInput{ - Filters: []*dms.Filter{ - { - Name: aws.String("replication-instance-id"), - Values: []*string{aws.String(id)}, - }, - }, - } - - config := waiter.Config{ - Operation: "DescribeReplicationInstances", - Delay: delay, - MaxAttempts: maxAttempts, - Acceptors: []waiter.WaitAcceptor{ - { - State: "success", - Matcher: "path", - Argument: "length(ReplicationInstances[]) > `0`", - Expected: false, - }, - }, - } - - w := waiter.Waiter{ - Client: client, - Input: input, - Config: config, - } - - return w.Wait() + return v, *v.ReplicationInstances[0].ReplicationInstanceStatus, nil + } } diff --git a/builtin/providers/aws/resource_aws_dms_replication_instance_test.go b/builtin/providers/aws/resource_aws_dms_replication_instance_test.go index 3b6bb0d0e..0cb8a37d5 100644 --- a/builtin/providers/aws/resource_aws_dms_replication_instance_test.go +++ b/builtin/providers/aws/resource_aws_dms_replication_instance_test.go @@ -8,7 +8,6 @@ import ( dms "github.com/aws/aws-sdk-go/service/databasemigrationservice" "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/resource" - "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/terraform" ) @@ -47,11 +46,6 @@ func TestAccAwsDmsReplicationInstanceBasic(t *testing.T) { } func checkDmsReplicationInstanceExists(n string) resource.TestCheckFunc { - providers := []*schema.Provider{testAccProvider} - return checkDmsReplicationInstanceExistsWithProviders(n, &providers) -} - -func checkDmsReplicationInstanceExistsWithProviders(n string, providers *[]*schema.Provider) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] if !ok { @@ -61,29 +55,24 @@ func checkDmsReplicationInstanceExistsWithProviders(n string, providers *[]*sche if rs.Primary.ID == "" { return fmt.Errorf("No ID is set") } - for _, provider := range *providers { - // Ignore if Meta is empty, this can happen for validation providers - if provider.Meta() == nil { - continue - } - - conn := provider.Meta().(*AWSClient).dmsconn - _, err := conn.DescribeReplicationInstances(&dms.DescribeReplicationInstancesInput{ - Filters: []*dms.Filter{ - { - Name: aws.String("replication-instance-id"), - Values: []*string{aws.String(rs.Primary.ID)}, - }, + conn := testAccProvider.Meta().(*AWSClient).dmsconn + resp, err := conn.DescribeReplicationInstances(&dms.DescribeReplicationInstancesInput{ + Filters: []*dms.Filter{ + { + Name: aws.String("replication-instance-id"), + Values: []*string{aws.String(rs.Primary.ID)}, }, - }) + }, + }) - if err != nil { - return fmt.Errorf("DMS replication instance error: %v", err) - } - return nil + if err != nil { + return fmt.Errorf("DMS replication instance error: %v", err) + } + if resp.ReplicationInstances == nil { + return fmt.Errorf("DMS replication instance not found") } - return fmt.Errorf("DMS replication instance not found") + return nil } } @@ -104,22 +93,11 @@ func dmsReplicationInstanceDestroy(s *terraform.State) error { func dmsReplicationInstanceConfig(randId string) string { return fmt.Sprintf(` -resource "aws_iam_role" "dms_iam_role" { - name = "dms-vpc-role" - assume_role_policy = "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Sid\":\"\",\"Effect\":\"Allow\",\"Principal\":{\"Service\":\"dms.amazonaws.com\"},\"Action\":\"sts:AssumeRole\"}]}" -} - -resource "aws_iam_role_policy_attachment" "dms_iam_role_policy" { - role = "${aws_iam_role.dms_iam_role.name}" - policy_arn = "arn:aws:iam::aws:policy/service-role/AmazonDMSVPCManagementRole" -} - resource "aws_vpc" "dms_vpc" { cidr_block = "10.1.0.0/16" tags { Name = "tf-test-dms-vpc-%[1]s" } - depends_on = ["aws_iam_role_policy_attachment.dms_iam_role_policy"] } resource "aws_subnet" "dms_subnet_1" { @@ -146,7 +124,6 @@ resource "aws_dms_replication_subnet_group" "dms_replication_subnet_group" { replication_subnet_group_id = "tf-test-dms-replication-subnet-group-%[1]s" replication_subnet_group_description = "terraform test for replication subnet group" subnet_ids = ["${aws_subnet.dms_subnet_1.id}", "${aws_subnet.dms_subnet_2.id}"] - depends_on = ["aws_iam_role_policy_attachment.dms_iam_role_policy"] } resource "aws_dms_replication_instance" "dms_replication_instance" { @@ -168,22 +145,11 @@ resource "aws_dms_replication_instance" "dms_replication_instance" { func dmsReplicationInstanceConfigUpdate(randId string) string { return fmt.Sprintf(` -resource "aws_iam_role" "dms_iam_role" { - name = "dms-vpc-role-%[1]s" - assume_role_policy = "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Sid\":\"\",\"Effect\":\"Allow\",\"Principal\":{\"Service\":\"dms.amazonaws.com\"},\"Action\":\"sts:AssumeRole\"}]}" -} - -resource "aws_iam_role_policy_attachment" "dms_iam_role_policy" { - role = "${aws_iam_role.dms_iam_role.name}" - policy_arn = "arn:aws:iam::aws:policy/service-role/AmazonDMSVPCManagementRole" -} - resource "aws_vpc" "dms_vpc" { cidr_block = "10.1.0.0/16" tags { Name = "tf-test-dms-vpc-%[1]s" } - depends_on = ["aws_iam_role_policy_attachment.dms_iam_role_policy"] } resource "aws_subnet" "dms_subnet_1" { @@ -210,7 +176,6 @@ resource "aws_dms_replication_subnet_group" "dms_replication_subnet_group" { replication_subnet_group_id = "tf-test-dms-replication-subnet-group-%[1]s" replication_subnet_group_description = "terraform test for replication subnet group" subnet_ids = ["${aws_subnet.dms_subnet_1.id}", "${aws_subnet.dms_subnet_2.id}"] - depends_on = ["aws_iam_role_policy_attachment.dms_iam_role_policy"] } resource "aws_dms_replication_instance" "dms_replication_instance" { diff --git a/builtin/providers/aws/resource_aws_dms_replication_subnet_group_test.go b/builtin/providers/aws/resource_aws_dms_replication_subnet_group_test.go index a6da50c9c..608382ccc 100644 --- a/builtin/providers/aws/resource_aws_dms_replication_subnet_group_test.go +++ b/builtin/providers/aws/resource_aws_dms_replication_subnet_group_test.go @@ -101,22 +101,11 @@ func dmsReplicationSubnetGroupDestroy(s *terraform.State) error { func dmsReplicationSubnetGroupConfig(randId string) string { return fmt.Sprintf(` -resource "aws_iam_role" "dms_iam_role" { - name = "dms-vpc-role-%[1]s" - assume_role_policy = "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Sid\":\"\",\"Effect\":\"Allow\",\"Principal\":{\"Service\":\"dms.amazonaws.com\"},\"Action\":\"sts:AssumeRole\"}]}" -} - -resource "aws_iam_role_policy_attachment" "dms_iam_role_policy" { - role = "${aws_iam_role.dms_iam_role.name}" - policy_arn = "arn:aws:iam::aws:policy/service-role/AmazonDMSVPCManagementRole" -} - resource "aws_vpc" "dms_vpc" { cidr_block = "10.1.0.0/16" tags { Name = "tf-test-dms-vpc-%[1]s" } - depends_on = ["aws_iam_role_policy_attachment.dms_iam_role_policy"] } resource "aws_subnet" "dms_subnet_1" { @@ -164,22 +153,11 @@ resource "aws_dms_replication_subnet_group" "dms_replication_subnet_group" { func dmsReplicationSubnetGroupConfigUpdate(randId string) string { return fmt.Sprintf(` -resource "aws_iam_role" "dms_iam_role" { - name = "dms-vpc-role" - assume_role_policy = "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Sid\":\"\",\"Effect\":\"Allow\",\"Principal\":{\"Service\":\"dms.amazonaws.com\"},\"Action\":\"sts:AssumeRole\"}]}" -} - -resource "aws_iam_role_policy_attachment" "dms_iam_role_policy" { - role = "${aws_iam_role.dms_iam_role.name}" - policy_arn = "arn:aws:iam::aws:policy/service-role/AmazonDMSVPCManagementRole" -} - resource "aws_vpc" "dms_vpc" { cidr_block = "10.1.0.0/16" tags { Name = "tf-test-dms-vpc-%[1]s" } - depends_on = ["aws_iam_role_policy_attachment.dms_iam_role_policy"] } resource "aws_subnet" "dms_subnet_1" { diff --git a/builtin/providers/aws/resource_aws_dms_replication_task.go b/builtin/providers/aws/resource_aws_dms_replication_task.go index a9ab72ce4..f137c186c 100644 --- a/builtin/providers/aws/resource_aws_dms_replication_task.go +++ b/builtin/providers/aws/resource_aws_dms_replication_task.go @@ -8,8 +8,8 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/awserr" - "github.com/aws/aws-sdk-go/private/waiter" dms "github.com/aws/aws-sdk-go/service/databasemigrationservice" + "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/helper/validation" ) @@ -121,13 +121,23 @@ func resourceAwsDmsReplicationTaskCreate(d *schema.ResourceData, meta interface{ } taskId := d.Get("replication_task_id").(string) + d.SetId(taskId) - err = waitForTaskCreated(conn, taskId, 30, 10) + stateConf := &resource.StateChangeConf{ + Pending: []string{"creating"}, + Target: []string{"ready"}, + Refresh: resourceAwsDmsReplicationTaskStateRefreshFunc(d, meta), + Timeout: d.Timeout(schema.TimeoutCreate), + MinTimeout: 10 * time.Second, + Delay: 30 * time.Second, // Wait 30 secs before starting + } + + // Wait, catching any errors + _, err = stateConf.WaitForState() if err != nil { return err } - d.SetId(taskId) return resourceAwsDmsReplicationTaskRead(d, meta) } @@ -144,6 +154,7 @@ func resourceAwsDmsReplicationTaskRead(d *schema.ResourceData, meta interface{}) }) if err != nil { if dmserr, ok := err.(awserr.Error); ok && dmserr.Code() == "ResourceNotFoundFault" { + log.Printf("[DEBUG] DMS Replication Task %q Not Found", d.Id()) d.SetId("") return nil } @@ -213,7 +224,17 @@ func resourceAwsDmsReplicationTaskUpdate(d *schema.ResourceData, meta interface{ return err } - err = waitForTaskUpdated(conn, d.Get("replication_task_id").(string), 30, 10) + stateConf := &resource.StateChangeConf{ + Pending: []string{"modifying"}, + Target: []string{"ready"}, + Refresh: resourceAwsDmsReplicationTaskStateRefreshFunc(d, meta), + Timeout: d.Timeout(schema.TimeoutCreate), + MinTimeout: 10 * time.Second, + Delay: 30 * time.Second, // Wait 30 secs before starting + } + + // Wait, catching any errors + _, err = stateConf.WaitForState() if err != nil { return err } @@ -235,12 +256,27 @@ func resourceAwsDmsReplicationTaskDelete(d *schema.ResourceData, meta interface{ _, err := conn.DeleteReplicationTask(request) if err != nil { + if dmserr, ok := err.(awserr.Error); ok && dmserr.Code() == "ResourceNotFoundFault" { + log.Printf("[DEBUG] DMS Replication Task %q Not Found", d.Id()) + d.SetId("") + return nil + } return err } - waitErr := waitForTaskDeleted(conn, d.Get("replication_task_id").(string), 30, 10) - if waitErr != nil { - return waitErr + stateConf := &resource.StateChangeConf{ + Pending: []string{"deleting"}, + Target: []string{}, + Refresh: resourceAwsDmsReplicationTaskStateRefreshFunc(d, meta), + Timeout: d.Timeout(schema.TimeoutCreate), + MinTimeout: 10 * time.Second, + Delay: 30 * time.Second, // Wait 30 secs before starting + } + + // Wait, catching any errors + _, err = stateConf.WaitForState() + if err != nil { + return err } return nil @@ -261,119 +297,35 @@ func resourceAwsDmsReplicationTaskSetState(d *schema.ResourceData, task *dms.Rep return nil } -func waitForTaskCreated(client *dms.DatabaseMigrationService, id string, delay int, maxAttempts int) error { - input := &dms.DescribeReplicationTasksInput{ - Filters: []*dms.Filter{ - { - Name: aws.String("replication-task-id"), - Values: []*string{aws.String(id)}, - }, - }, - } +func resourceAwsDmsReplicationTaskStateRefreshFunc( + d *schema.ResourceData, meta interface{}) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + conn := meta.(*AWSClient).dmsconn - config := waiter.Config{ - Operation: "DescribeReplicationTasks", - Delay: delay, - MaxAttempts: maxAttempts, - Acceptors: []waiter.WaitAcceptor{ - { - State: "retry", - Matcher: "pathAll", - Argument: "ReplicationTasks[].Status", - Expected: "creating", + v, err := conn.DescribeReplicationTasks(&dms.DescribeReplicationTasksInput{ + Filters: []*dms.Filter{ + { + Name: aws.String("replication-task-id"), + Values: []*string{aws.String(d.Id())}, // Must use d.Id() to work with import. + }, }, - { - State: "success", - Matcher: "pathAll", - Argument: "ReplicationTasks[].Status", - Expected: "ready", - }, - }, - } + }) + if err != nil { + if dmserr, ok := err.(awserr.Error); ok && dmserr.Code() == "ResourceNotFoundFault" { + return nil, "", nil + } + log.Printf("Error on retrieving DMS Replication Task when waiting: %s", err) + return nil, "", err + } - w := waiter.Waiter{ - Client: client, - Input: input, - Config: config, - } + if v == nil { + return nil, "", nil + } - return w.Wait() -} - -func waitForTaskUpdated(client *dms.DatabaseMigrationService, id string, delay int, maxAttempts int) error { - input := &dms.DescribeReplicationTasksInput{ - Filters: []*dms.Filter{ - { - Name: aws.String("replication-task-id"), - Values: []*string{aws.String(id)}, - }, - }, - } - - config := waiter.Config{ - Operation: "DescribeReplicationTasks", - Delay: delay, - MaxAttempts: maxAttempts, - Acceptors: []waiter.WaitAcceptor{ - { - State: "retry", - Matcher: "pathAll", - Argument: "ReplicationTasks[].Status", - Expected: "modifying", - }, - { - State: "success", - Matcher: "pathAll", - Argument: "ReplicationTasks[].Status", - Expected: "ready", - }, - }, - } - - w := waiter.Waiter{ - Client: client, - Input: input, - Config: config, - } - - return w.Wait() -} - -func waitForTaskDeleted(client *dms.DatabaseMigrationService, id string, delay int, maxAttempts int) error { - input := &dms.DescribeReplicationTasksInput{ - Filters: []*dms.Filter{ - { - Name: aws.String("replication-task-id"), - Values: []*string{aws.String(id)}, - }, - }, - } - - config := waiter.Config{ - Operation: "DescribeReplicationTasks", - Delay: delay, - MaxAttempts: maxAttempts, - Acceptors: []waiter.WaitAcceptor{ - { - State: "retry", - Matcher: "pathAll", - Argument: "ReplicationTasks[].Status", - Expected: "deleting", - }, - { - State: "success", - Matcher: "path", - Argument: "length(ReplicationTasks[]) > `0`", - Expected: false, - }, - }, - } - - w := waiter.Waiter{ - Client: client, - Input: input, - Config: config, - } - - return w.Wait() + if v.ReplicationTasks != nil { + log.Printf("[DEBUG] DMS Replication Task status for instance %s: %s", d.Id(), *v.ReplicationTasks[0].Status) + } + + return v, *v.ReplicationTasks[0].Status, nil + } } diff --git a/builtin/providers/aws/resource_aws_dms_replication_task_test.go b/builtin/providers/aws/resource_aws_dms_replication_task_test.go index 8b20abf86..9105a3109 100644 --- a/builtin/providers/aws/resource_aws_dms_replication_task_test.go +++ b/builtin/providers/aws/resource_aws_dms_replication_task_test.go @@ -8,7 +8,6 @@ import ( dms "github.com/aws/aws-sdk-go/service/databasemigrationservice" "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/resource" - "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/terraform" ) @@ -44,11 +43,6 @@ func TestAccAwsDmsReplicationTaskBasic(t *testing.T) { } func checkDmsReplicationTaskExists(n string) resource.TestCheckFunc { - providers := []*schema.Provider{testAccProvider} - return checkDmsReplicationTaskExistsWithProviders(n, &providers) -} - -func checkDmsReplicationTaskExistsWithProviders(n string, providers *[]*schema.Provider) resource.TestCheckFunc { return func(s *terraform.State) error { rs, ok := s.RootModule().Resources[n] if !ok { @@ -58,29 +52,25 @@ func checkDmsReplicationTaskExistsWithProviders(n string, providers *[]*schema.P if rs.Primary.ID == "" { return fmt.Errorf("No ID is set") } - for _, provider := range *providers { - // Ignore if Meta is empty, this can happen for validation providers - if provider.Meta() == nil { - continue - } - conn := provider.Meta().(*AWSClient).dmsconn - _, err := conn.DescribeReplicationTasks(&dms.DescribeReplicationTasksInput{ - Filters: []*dms.Filter{ - { - Name: aws.String("replication-task-id"), - Values: []*string{aws.String(rs.Primary.ID)}, - }, + conn := testAccProvider.Meta().(*AWSClient).dmsconn + resp, err := conn.DescribeReplicationTasks(&dms.DescribeReplicationTasksInput{ + Filters: []*dms.Filter{ + { + Name: aws.String("replication-task-id"), + Values: []*string{aws.String(rs.Primary.ID)}, }, - }) + }, + }) - if err != nil { - return fmt.Errorf("DMS replication subnet group error: %v", err) - } - return nil + if err != nil { + return err } - return fmt.Errorf("DMS replication subnet group not found") + if resp.ReplicationTasks == nil { + return fmt.Errorf("DMS replication task error: %v", err) + } + return nil } } @@ -90,9 +80,22 @@ func dmsReplicationTaskDestroy(s *terraform.State) error { continue } - err := checkDmsReplicationTaskExists(rs.Primary.ID) - if err == nil { - return fmt.Errorf("Found replication subnet group that was not destroyed: %s", rs.Primary.ID) + conn := testAccProvider.Meta().(*AWSClient).dmsconn + resp, err := conn.DescribeReplicationTasks(&dms.DescribeReplicationTasksInput{ + Filters: []*dms.Filter{ + { + Name: aws.String("replication-task-id"), + Values: []*string{aws.String(rs.Primary.ID)}, + }, + }, + }) + + if err != nil { + return nil + } + + if resp != nil && len(resp.ReplicationTasks) > 0 { + return fmt.Errorf("DMS replication task still exists: %v", err) } } @@ -101,22 +104,11 @@ func dmsReplicationTaskDestroy(s *terraform.State) error { func dmsReplicationTaskConfig(randId string) string { return fmt.Sprintf(` -resource "aws_iam_role" "dms_iam_role" { - name = "dms-vpc-role-%[1]s" - assume_role_policy = "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Sid\":\"\",\"Effect\":\"Allow\",\"Principal\":{\"Service\":\"dms.amazonaws.com\"},\"Action\":\"sts:AssumeRole\"}]}" -} - -resource "aws_iam_role_policy_attachment" "dms_iam_role_policy" { - role = "${aws_iam_role.dms_iam_role.name}" - policy_arn = "arn:aws:iam::aws:policy/service-role/AmazonDMSVPCManagementRole" -} - resource "aws_vpc" "dms_vpc" { cidr_block = "10.1.0.0/16" tags { Name = "tf-test-dms-vpc-%[1]s" } - depends_on = ["aws_iam_role_policy_attachment.dms_iam_role_policy"] } resource "aws_subnet" "dms_subnet_1" { @@ -148,7 +140,6 @@ resource "aws_dms_endpoint" "dms_endpoint_source" { port = 3306 username = "tftest" password = "tftest" - depends_on = ["aws_iam_role_policy_attachment.dms_iam_role_policy"] } resource "aws_dms_endpoint" "dms_endpoint_target" { @@ -160,14 +151,12 @@ resource "aws_dms_endpoint" "dms_endpoint_target" { port = 3306 username = "tftest" password = "tftest" - depends_on = ["aws_iam_role_policy_attachment.dms_iam_role_policy"] } resource "aws_dms_replication_subnet_group" "dms_replication_subnet_group" { replication_subnet_group_id = "tf-test-dms-replication-subnet-group-%[1]s" replication_subnet_group_description = "terraform test for replication subnet group" subnet_ids = ["${aws_subnet.dms_subnet_1.id}", "${aws_subnet.dms_subnet_2.id}"] - depends_on = ["aws_iam_role_policy_attachment.dms_iam_role_policy"] } resource "aws_dms_replication_instance" "dms_replication_instance" { @@ -199,22 +188,11 @@ resource "aws_dms_replication_task" "dms_replication_task" { func dmsReplicationTaskConfigUpdate(randId string) string { return fmt.Sprintf(` -resource "aws_iam_role" "dms_iam_role" { - name = "dms-vpc-role" - assume_role_policy = "{\"Version\":\"2012-10-17\",\"Statement\":[{\"Sid\":\"\",\"Effect\":\"Allow\",\"Principal\":{\"Service\":\"dms.amazonaws.com\"},\"Action\":\"sts:AssumeRole\"}]}" -} - -resource "aws_iam_role_policy_attachment" "dms_iam_role_policy" { - role = "${aws_iam_role.dms_iam_role.name}" - policy_arn = "arn:aws:iam::aws:policy/service-role/AmazonDMSVPCManagementRole" -} - resource "aws_vpc" "dms_vpc" { cidr_block = "10.1.0.0/16" tags { Name = "tf-test-dms-vpc-%[1]s" } - depends_on = ["aws_iam_role_policy_attachment.dms_iam_role_policy"] } resource "aws_subnet" "dms_subnet_1" { @@ -246,7 +224,6 @@ resource "aws_dms_endpoint" "dms_endpoint_source" { port = 3306 username = "tftest" password = "tftest" - depends_on = ["aws_iam_role_policy_attachment.dms_iam_role_policy"] } resource "aws_dms_endpoint" "dms_endpoint_target" { @@ -258,14 +235,12 @@ resource "aws_dms_endpoint" "dms_endpoint_target" { port = 3306 username = "tftest" password = "tftest" - depends_on = ["aws_iam_role_policy_attachment.dms_iam_role_policy"] } resource "aws_dms_replication_subnet_group" "dms_replication_subnet_group" { replication_subnet_group_id = "tf-test-dms-replication-subnet-group-%[1]s" replication_subnet_group_description = "terraform test for replication subnet group" subnet_ids = ["${aws_subnet.dms_subnet_1.id}", "${aws_subnet.dms_subnet_2.id}"] - depends_on = ["aws_iam_role_policy_attachment.dms_iam_role_policy"] } resource "aws_dms_replication_instance" "dms_replication_instance" {