From 914c637f77f33ce7fba6f65cdb082cb9473b552d Mon Sep 17 00:00:00 2001 From: drfaust92 Date: Sun, 8 Nov 2020 13:05:59 +0200 Subject: [PATCH 01/14] sagemaker image --- .../service/sagemaker/finder/finder.go | 19 ++ .../service/sagemaker/waiter/status.go | 27 +++ .../service/sagemaker/waiter/waiter.go | 40 ++++ aws/provider.go | 3 +- aws/resource_aws_sagemaker_image.go | 209 ++++++++++++++++++ aws/resource_aws_sagemaker_image_test.go | 183 +++++++++++++++ 6 files changed, 480 insertions(+), 1 deletion(-) create mode 100644 aws/resource_aws_sagemaker_image.go create mode 100644 aws/resource_aws_sagemaker_image_test.go diff --git a/aws/internal/service/sagemaker/finder/finder.go b/aws/internal/service/sagemaker/finder/finder.go index 025fd543c0cc..0abb4a7a7e91 100644 --- a/aws/internal/service/sagemaker/finder/finder.go +++ b/aws/internal/service/sagemaker/finder/finder.go @@ -23,3 +23,22 @@ func CodeRepositoryByName(conn *sagemaker.SageMaker, name string) (*sagemaker.De return output, nil } + +// ImageByName returns the code repository corresponding to the specified name. +// Returns nil if no code repository is found. +func ImageByName(conn *sagemaker.SageMaker, name string) (*sagemaker.DescribeImageOutput, error) { + input := &sagemaker.DescribeImageInput{ + ImageName: aws.String(name), + } + + output, err := conn.DescribeImage(input) + if err != nil { + return nil, err + } + + if output == nil { + return nil, nil + } + + return output, nil +} diff --git a/aws/internal/service/sagemaker/waiter/status.go b/aws/internal/service/sagemaker/waiter/status.go index 6a4943e22abd..8d615f69f937 100644 --- a/aws/internal/service/sagemaker/waiter/status.go +++ b/aws/internal/service/sagemaker/waiter/status.go @@ -9,6 +9,8 @@ import ( const ( SagemakerNotebookInstanceStatusNotFound = "NotFound" + SagemakerImageStatusNotFound = "NotFound" + SagemakerImageStatusFailed = "Failed" ) // NotebookInstanceStatus fetches the NotebookInstance and its Status @@ -35,3 +37,28 @@ func NotebookInstanceStatus(conn *sagemaker.SageMaker, notebookName string) reso return output, aws.StringValue(output.NotebookInstanceStatus), nil } } + +// ImageStatus fetches the Image and its Status +func ImageStatus(conn *sagemaker.SageMaker, name string) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + input := &sagemaker.DescribeImageInput{ + ImageName: aws.String(name), + } + + output, err := conn.DescribeImage(input) + + if tfawserr.ErrMessageContains(err, "ValidationException", "RecordNotFound") { + return nil, SagemakerImageStatusNotFound, nil + } + + if err != nil { + return nil, SagemakerImageStatusFailed, err + } + + if output == nil { + return nil, SagemakerImageStatusNotFound, nil + } + + return output, aws.StringValue(output.ImageStatus), nil + } +} diff --git a/aws/internal/service/sagemaker/waiter/waiter.go b/aws/internal/service/sagemaker/waiter/waiter.go index 61330bea2407..42a901ebefad 100644 --- a/aws/internal/service/sagemaker/waiter/waiter.go +++ b/aws/internal/service/sagemaker/waiter/waiter.go @@ -11,6 +11,7 @@ const ( NotebookInstanceInServiceTimeout = 10 * time.Minute NotebookInstanceStoppedTimeout = 10 * time.Minute NotebookInstanceDeletedTimeout = 10 * time.Minute + ImageCreatedTimeout = 10 * time.Minute ) // NotebookInstanceInService waits for a NotebookInstance to return InService @@ -76,3 +77,42 @@ func NotebookInstanceDeleted(conn *sagemaker.SageMaker, notebookName string) (*s return nil, err } + +// ImageCreated waits for a Image to return Created +func ImageCreated(conn *sagemaker.SageMaker, name string) (*sagemaker.DescribeImageOutput, error) { + stateConf := &resource.StateChangeConf{ + Pending: []string{ + sagemaker.ImageStatusCreating, + sagemaker.ImageStatusUpdating, + }, + Target: []string{sagemaker.ImageStatusCreated}, + Refresh: ImageStatus(conn, name), + Timeout: ImageCreatedTimeout, + } + + outputRaw, err := stateConf.WaitForState() + + if output, ok := outputRaw.(*sagemaker.DescribeImageOutput); ok { + return output, err + } + + return nil, err +} + +// ImageDeleted waits for a Image to return Deleted +func ImageDeleted(conn *sagemaker.SageMaker, name string) (*sagemaker.DescribeImageOutput, error) { + stateConf := &resource.StateChangeConf{ + Pending: []string{sagemaker.ImageStatusDeleting}, + Target: []string{}, + Refresh: ImageStatus(conn, name), + Timeout: ImageCreatedTimeout, + } + + outputRaw, err := stateConf.WaitForState() + + if output, ok := outputRaw.(*sagemaker.DescribeImageOutput); ok { + return output, err + } + + return nil, err +} diff --git a/aws/provider.go b/aws/provider.go index 79d050c6a0af..5fda836eed79 100644 --- a/aws/provider.go +++ b/aws/provider.go @@ -857,9 +857,10 @@ func Provider() *schema.Provider { "aws_default_route_table": resourceAwsDefaultRouteTable(), "aws_route_table_association": resourceAwsRouteTableAssociation(), "aws_sagemaker_code_repository": resourceAwsSagemakerCodeRepository(), - "aws_sagemaker_model": resourceAwsSagemakerModel(), "aws_sagemaker_endpoint_configuration": resourceAwsSagemakerEndpointConfiguration(), + "aws_sagemaker_image": resourceAwsSagemakerImage(), "aws_sagemaker_endpoint": resourceAwsSagemakerEndpoint(), + "aws_sagemaker_model": resourceAwsSagemakerModel(), "aws_sagemaker_notebook_instance_lifecycle_configuration": resourceAwsSagemakerNotebookInstanceLifeCycleConfiguration(), "aws_sagemaker_notebook_instance": resourceAwsSagemakerNotebookInstance(), "aws_secretsmanager_secret": resourceAwsSecretsManagerSecret(), diff --git a/aws/resource_aws_sagemaker_image.go b/aws/resource_aws_sagemaker_image.go new file mode 100644 index 000000000000..56f6bb512a0a --- /dev/null +++ b/aws/resource_aws_sagemaker_image.go @@ -0,0 +1,209 @@ +package aws + +import ( + "fmt" + "log" + "regexp" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/sagemaker" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/sagemaker/finder" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/sagemaker/waiter" +) + +func resourceAwsSagemakerImage() *schema.Resource { + return &schema.Resource{ + Create: resourceAwsSagemakerImageCreate, + Read: resourceAwsSagemakerImageRead, + Update: resourceAwsSagemakerImageUpdate, + Delete: resourceAwsSagemakerImageDelete, + Importer: &schema.ResourceImporter{ + State: schema.ImportStatePassthrough, + }, + + Schema: map[string]*schema.Schema{ + "arn": { + Type: schema.TypeString, + Computed: true, + }, + + "image_name": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + ValidateFunc: validation.All( + validation.StringLenBetween(1, 63), + validation.StringMatch(regexp.MustCompile(`^[a-zA-Z0-9](-*[a-zA-Z0-9])*$`), "Valid characters are a-z, A-Z, 0-9, and - (hyphen)."), + ), + }, + "role_arn": { + Type: schema.TypeString, + Required: true, + ValidateFunc: validateArn, + }, + "display_name": { + Type: schema.TypeString, + Optional: true, + ValidateFunc: validation.StringLenBetween(1, 128), + }, + "description": { + Type: schema.TypeString, + Optional: true, + ValidateFunc: validation.StringLenBetween(1, 512), + }, + "tags": tagsSchema(), + }, + } +} + +func resourceAwsSagemakerImageCreate(d *schema.ResourceData, meta interface{}) error { + conn := meta.(*AWSClient).sagemakerconn + + name := d.Get("image_name").(string) + input := &sagemaker.CreateImageInput{ + ImageName: aws.String(name), + RoleArn: aws.String(d.Get("role_arn").(string)), + } + + if v, ok := d.GetOk("display_name"); ok { + input.DisplayName = aws.String(v.(string)) + } + + if v, ok := d.GetOk("description"); ok { + input.Description = aws.String(v.(string)) + } + + if v, ok := d.GetOk("tags"); ok { + input.Tags = keyvaluetags.New(v.(map[string]interface{})).IgnoreAws().SagemakerTags() + } + + log.Printf("[DEBUG] sagemaker Image create config: %#v", *input) + _, err := conn.CreateImage(input) + if err != nil { + return fmt.Errorf("error creating SageMaker Image: %w", err) + } + + d.SetId(name) + + if _, err := waiter.ImageCreated(conn, d.Id()); err != nil { + return fmt.Errorf("error waiting for sagemaker image (%s) to create: %w", d.Id(), err) + } + + return resourceAwsSagemakerImageRead(d, meta) +} + +func resourceAwsSagemakerImageRead(d *schema.ResourceData, meta interface{}) error { + conn := meta.(*AWSClient).sagemakerconn + ignoreTagsConfig := meta.(*AWSClient).IgnoreTagsConfig + + image, err := finder.ImageByName(conn, d.Id()) + if err != nil { + if isAWSErr(err, "ValidationException", "Cannot find Image") { + d.SetId("") + log.Printf("[WARN] Unable to find SageMaker Image (%s); removing from state", d.Id()) + return nil + } + return fmt.Errorf("error reading SageMaker Image (%s): %w", d.Id(), err) + + } + + arn := aws.StringValue(image.ImageArn) + d.Set("image_name", image.ImageName) + d.Set("arn", arn) + d.Set("role_arn", image.RoleArn) + d.Set("display_name", image.DisplayName) + d.Set("description", image.Description) + + tags, err := keyvaluetags.SagemakerListTags(conn, arn) + + if err != nil { + return fmt.Errorf("error listing tags for Sagemaker Image (%s): %w", d.Id(), err) + } + + if err := d.Set("tags", tags.IgnoreAws().IgnoreConfig(ignoreTagsConfig).Map()); err != nil { + return fmt.Errorf("error setting tags: %w", err) + } + + return nil +} + +func resourceAwsSagemakerImageUpdate(d *schema.ResourceData, meta interface{}) error { + conn := meta.(*AWSClient).sagemakerconn + needsUpdate := false + + input := &sagemaker.UpdateImageInput{ + ImageName: aws.String(d.Id()), + } + + var deleteProperties []*string + + if d.HasChange("role_arn") { + input.Description = aws.String(d.Get("role_arn").(string)) + } + + if d.HasChange("description") { + if v, ok := d.GetOk("description"); ok { + input.Description = aws.String(v.(string)) + } else { + deleteProperties = append(deleteProperties, aws.String("Description")) + } + needsUpdate = true + } + + if d.HasChange("display_name") { + if v, ok := d.GetOk("display_name"); ok { + input.DisplayName = aws.String(v.(string)) + } else { + deleteProperties = append(deleteProperties, aws.String("DisplayName")) + } + needsUpdate = true + } + + if needsUpdate { + log.Printf("[DEBUG] sagemaker Image update config: %#v", *input) + _, err := conn.UpdateImage(input) + if err != nil { + return fmt.Errorf("error updating SageMaker Image: %w", err) + } + + if _, err := waiter.ImageCreated(conn, d.Id()); err != nil { + return fmt.Errorf("error waiting for sagemaker image (%s) to update: %w", d.Id(), err) + } + } + + if d.HasChange("tags") { + o, n := d.GetChange("tags") + + if err := keyvaluetags.SagemakerUpdateTags(conn, d.Get("arn").(string), o, n); err != nil { + return fmt.Errorf("error updating Sagemaker Image (%s) tags: %s", d.Id(), err) + } + } + + return resourceAwsSagemakerImageRead(d, meta) +} + +func resourceAwsSagemakerImageDelete(d *schema.ResourceData, meta interface{}) error { + conn := meta.(*AWSClient).sagemakerconn + + input := &sagemaker.DeleteImageInput{ + ImageName: aws.String(d.Id()), + } + + if _, err := conn.DeleteImage(input); err != nil { + if isAWSErr(err, "ValidationException", "Cannot find Image") { + return nil + } + return fmt.Errorf("error deleting SageMaker Image (%s): %w", d.Id(), err) + } + + if _, err := waiter.ImageDeleted(conn, d.Id()); err != nil { + if !isAWSErr(err, sagemaker.ErrCodeResourceNotFound, "") { + return fmt.Errorf("error waiting for sagemaker image (%s) to delete: %w", d.Id(), err) + } + } + + return nil +} diff --git a/aws/resource_aws_sagemaker_image_test.go b/aws/resource_aws_sagemaker_image_test.go new file mode 100644 index 000000000000..71494ed51399 --- /dev/null +++ b/aws/resource_aws_sagemaker_image_test.go @@ -0,0 +1,183 @@ +package aws + +import ( + "fmt" + "log" + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/sagemaker" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "github.com/terraform-providers/terraform-provider-aws/aws/internal/service/sagemaker/finder" +) + +func init() { + resource.AddTestSweepers("aws_sagemaker_image", &resource.Sweeper{ + Name: "aws_sagemaker_image", + F: testSweepSagemakerImages, + }) +} + +func testSweepSagemakerImages(region string) error { + client, err := sharedClientForRegion(region) + if err != nil { + return fmt.Errorf("error getting client: %s", err) + } + conn := client.(*AWSClient).sagemakerconn + + err = conn.ListImagesPages(&sagemaker.ListImagesInput{}, func(page *sagemaker.ListImagesOutput, lastPage bool) bool { + for _, Image := range page.Images { + name := aws.StringValue(Image.ImageName) + + input := &sagemaker.DeleteImageInput{ + ImageName: Image.ImageName, + } + + log.Printf("[INFO] Deleting SageMaker Image: %s", name) + if _, err := conn.DeleteImage(input); err != nil { + log.Printf("[ERROR] Error deleting SageMaker Image (%s): %s", name, err) + continue + } + } + + return !lastPage + }) + + if testSweepSkipSweepError(err) { + log.Printf("[WARN] Skipping SageMaker Image sweep for %s: %s", region, err) + return nil + } + + if err != nil { + return fmt.Errorf("Error retrieving SageMaker Images: %w", err) + } + + return nil +} + +func TestAccAWSSagemakerImage_basic(t *testing.T) { + var notebook sagemaker.DescribeImageOutput + rName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_sagemaker_image.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSSagemakerImageDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSSagemakerImageBasicConfig(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSagemakerImageExists(resourceName, ¬ebook), + resource.TestCheckResourceAttr(resourceName, "image_name", rName), + testAccCheckResourceAttrRegionalARN(resourceName, "arn", "sagemaker", fmt.Sprintf("image/%s", rName)), + resource.TestCheckResourceAttrPair(resourceName, "role_arn", "aws_iam_role.test", "arn"), + resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccAWSSagemakerImage_disappears(t *testing.T) { + var notebook sagemaker.DescribeImageOutput + rName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_sagemaker_image.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSSagemakerImageDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSSagemakerImageBasicConfig(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSagemakerImageExists(resourceName, ¬ebook), + testAccCheckResourceDisappears(testAccProvider, resourceAwsSagemakerImage(), resourceName), + ), + ExpectNonEmptyPlan: true, + }, + }, + }) +} + +func testAccCheckAWSSagemakerImageDestroy(s *terraform.State) error { + conn := testAccProvider.Meta().(*AWSClient).sagemakerconn + + for _, rs := range s.RootModule().Resources { + if rs.Type != "aws_sagemaker_image" { + continue + } + + Image, err := finder.ImageByName(conn, rs.Primary.ID) + if err != nil { + return nil + } + + if aws.StringValue(Image.ImageName) == rs.Primary.ID { + return fmt.Errorf("sagemaker Image %q still exists", rs.Primary.ID) + } + } + + return nil +} + +func testAccCheckAWSSagemakerImageExists(n string, image *sagemaker.DescribeImageOutput) resource.TestCheckFunc { + return func(s *terraform.State) error { + rs, ok := s.RootModule().Resources[n] + if !ok { + return fmt.Errorf("Not found: %s", n) + } + + if rs.Primary.ID == "" { + return fmt.Errorf("No sagmaker Image ID is set") + } + + conn := testAccProvider.Meta().(*AWSClient).sagemakerconn + resp, err := finder.ImageByName(conn, rs.Primary.ID) + if err != nil { + return err + } + + *image = *resp + + return nil + } +} + +func testAccAWSSagemakerImageConfigBase(rName string) string { + return fmt.Sprintf(` +resource "aws_iam_role" "test" { + name = %[1]q + path = "/" + assume_role_policy = data.aws_iam_policy_document.test.json +} + +data "aws_iam_policy_document" "test" { + statement { + actions = ["sts:AssumeRole"] + + principals { + type = "Service" + identifiers = ["sagemaker.amazonaws.com"] + } + } +} +`, rName) +} + +func testAccAWSSagemakerImageBasicConfig(rName string) string { + return testAccAWSSagemakerImageConfigBase(rName) + fmt.Sprintf(` +resource "aws_sagemaker_image" "test" { + image_name = %[1]q + role_arn = aws_iam_role.test.arn +} +`, rName) +} From 7cfce5532e9f852dc99d8b3671b6179251a0d75b Mon Sep 17 00:00:00 2001 From: drfaust92 Date: Sun, 8 Nov 2020 14:25:28 +0200 Subject: [PATCH 02/14] basic tests passing --- aws/resource_aws_sagemaker_image.go | 10 ++++++---- aws/resource_aws_sagemaker_image_test.go | 4 ++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/aws/resource_aws_sagemaker_image.go b/aws/resource_aws_sagemaker_image.go index 56f6bb512a0a..b8ef838473e1 100644 --- a/aws/resource_aws_sagemaker_image.go +++ b/aws/resource_aws_sagemaker_image.go @@ -101,7 +101,7 @@ func resourceAwsSagemakerImageRead(d *schema.ResourceData, meta interface{}) err image, err := finder.ImageByName(conn, d.Id()) if err != nil { - if isAWSErr(err, "ValidationException", "Cannot find Image") { + if isAWSErr(err, sagemaker.ErrCodeResourceNotFound, "No Image with the name") { d.SetId("") log.Printf("[WARN] Unable to find SageMaker Image (%s); removing from state", d.Id()) return nil @@ -193,16 +193,18 @@ func resourceAwsSagemakerImageDelete(d *schema.ResourceData, meta interface{}) e } if _, err := conn.DeleteImage(input); err != nil { - if isAWSErr(err, "ValidationException", "Cannot find Image") { + if isAWSErr(err, sagemaker.ErrCodeResourceNotFound, "No Image with the name") { return nil } return fmt.Errorf("error deleting SageMaker Image (%s): %w", d.Id(), err) } if _, err := waiter.ImageDeleted(conn, d.Id()); err != nil { - if !isAWSErr(err, sagemaker.ErrCodeResourceNotFound, "") { - return fmt.Errorf("error waiting for sagemaker image (%s) to delete: %w", d.Id(), err) + if isAWSErr(err, sagemaker.ErrCodeResourceNotFound, "No Image with the name") { + return nil } + return fmt.Errorf("error waiting for sagemaker image (%s) to delete: %w", d.Id(), err) + } return nil diff --git a/aws/resource_aws_sagemaker_image_test.go b/aws/resource_aws_sagemaker_image_test.go index 71494ed51399..5fd0858c467b 100644 --- a/aws/resource_aws_sagemaker_image_test.go +++ b/aws/resource_aws_sagemaker_image_test.go @@ -87,7 +87,7 @@ func TestAccAWSSagemakerImage_basic(t *testing.T) { } func TestAccAWSSagemakerImage_disappears(t *testing.T) { - var notebook sagemaker.DescribeImageOutput + var image sagemaker.DescribeImageOutput rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_sagemaker_image.test" @@ -99,7 +99,7 @@ func TestAccAWSSagemakerImage_disappears(t *testing.T) { { Config: testAccAWSSagemakerImageBasicConfig(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSSagemakerImageExists(resourceName, ¬ebook), + testAccCheckAWSSagemakerImageExists(resourceName, &image), testAccCheckResourceDisappears(testAccProvider, resourceAwsSagemakerImage(), resourceName), ), ExpectNonEmptyPlan: true, From b792b208d4c52e02efc863c37c65a70f6a65e723 Mon Sep 17 00:00:00 2001 From: drfaust92 Date: Sun, 8 Nov 2020 14:34:39 +0200 Subject: [PATCH 03/14] add tags test --- aws/resource_aws_sagemaker_image_test.go | 75 +++++++++++++++++++++++- 1 file changed, 73 insertions(+), 2 deletions(-) diff --git a/aws/resource_aws_sagemaker_image_test.go b/aws/resource_aws_sagemaker_image_test.go index 5fd0858c467b..23158277a462 100644 --- a/aws/resource_aws_sagemaker_image_test.go +++ b/aws/resource_aws_sagemaker_image_test.go @@ -58,7 +58,7 @@ func testSweepSagemakerImages(region string) error { } func TestAccAWSSagemakerImage_basic(t *testing.T) { - var notebook sagemaker.DescribeImageOutput + var image sagemaker.DescribeImageOutput rName := acctest.RandomWithPrefix("tf-acc-test") resourceName := "aws_sagemaker_image.test" @@ -70,7 +70,7 @@ func TestAccAWSSagemakerImage_basic(t *testing.T) { { Config: testAccAWSSagemakerImageBasicConfig(rName), Check: resource.ComposeTestCheckFunc( - testAccCheckAWSSagemakerImageExists(resourceName, ¬ebook), + testAccCheckAWSSagemakerImageExists(resourceName, &image), resource.TestCheckResourceAttr(resourceName, "image_name", rName), testAccCheckResourceAttrRegionalARN(resourceName, "arn", "sagemaker", fmt.Sprintf("image/%s", rName)), resource.TestCheckResourceAttrPair(resourceName, "role_arn", "aws_iam_role.test", "arn"), @@ -86,6 +86,50 @@ func TestAccAWSSagemakerImage_basic(t *testing.T) { }) } +func TestAccAWSSagemakerImage_tags(t *testing.T) { + var image sagemaker.DescribeImageOutput + rName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_sagemaker_image.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSSagemakerImageDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSSagemakerImageConfigTags1(rName, "key1", "value1"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSagemakerImageExists(resourceName, &image), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccAWSSagemakerImageConfigTags2(rName, "key1", "value1updated", "key2", "value2"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSagemakerImageExists(resourceName, &image), + resource.TestCheckResourceAttr(resourceName, "tags.%", "2"), + resource.TestCheckResourceAttr(resourceName, "tags.key1", "value1updated"), + resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), + ), + }, + { + Config: testAccAWSSagemakerImageConfigTags1(rName, "key2", "value2"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSagemakerImageExists(resourceName, &image), + resource.TestCheckResourceAttr(resourceName, "tags.%", "1"), + resource.TestCheckResourceAttr(resourceName, "tags.key2", "value2"), + ), + }, + }, + }) +} + func TestAccAWSSagemakerImage_disappears(t *testing.T) { var image sagemaker.DescribeImageOutput rName := acctest.RandomWithPrefix("tf-acc-test") @@ -181,3 +225,30 @@ resource "aws_sagemaker_image" "test" { } `, rName) } + +func testAccAWSSagemakerImageConfigTags1(rName, tagKey1, tagValue1 string) string { + return testAccAWSSagemakerImageConfigBase(rName) + fmt.Sprintf(` +resource "aws_sagemaker_image" "test" { + image_name = %[1]q + role_arn = aws_iam_role.test.arn + + tags = { + %[2]q = %[3]q + } +} +`, rName, tagKey1, tagValue1) +} + +func testAccAWSSagemakerImageConfigTags2(rName, tagKey1, tagValue1, tagKey2, tagValue2 string) string { + return testAccAWSSagemakerImageConfigBase(rName) + fmt.Sprintf(` +resource "aws_sagemaker_image" "test" { + image_name = %[1]q + role_arn = aws_iam_role.test.arn + + tags = { + %[2]q = %[3]q + %[4]q = %[5]q + } +} +`, rName, tagKey1, tagValue1, tagKey2, tagValue2) +} From f1fa30ef053f01f38ad5bb837c1d38c775bd9d39 Mon Sep 17 00:00:00 2001 From: drfaust92 Date: Sun, 8 Nov 2020 19:30:54 +0200 Subject: [PATCH 04/14] add description and displayname tests --- aws/resource_aws_sagemaker_image.go | 2 + aws/resource_aws_sagemaker_image_test.go | 100 +++++++++++++++++++++++ 2 files changed, 102 insertions(+) diff --git a/aws/resource_aws_sagemaker_image.go b/aws/resource_aws_sagemaker_image.go index b8ef838473e1..ff5eba23144a 100644 --- a/aws/resource_aws_sagemaker_image.go +++ b/aws/resource_aws_sagemaker_image.go @@ -149,6 +149,7 @@ func resourceAwsSagemakerImageUpdate(d *schema.ResourceData, meta interface{}) e input.Description = aws.String(v.(string)) } else { deleteProperties = append(deleteProperties, aws.String("Description")) + input.DeleteProperties = deleteProperties } needsUpdate = true } @@ -158,6 +159,7 @@ func resourceAwsSagemakerImageUpdate(d *schema.ResourceData, meta interface{}) e input.DisplayName = aws.String(v.(string)) } else { deleteProperties = append(deleteProperties, aws.String("DisplayName")) + input.DeleteProperties = deleteProperties } needsUpdate = true } diff --git a/aws/resource_aws_sagemaker_image_test.go b/aws/resource_aws_sagemaker_image_test.go index 23158277a462..f6d2851c6ba5 100644 --- a/aws/resource_aws_sagemaker_image_test.go +++ b/aws/resource_aws_sagemaker_image_test.go @@ -86,6 +86,86 @@ func TestAccAWSSagemakerImage_basic(t *testing.T) { }) } +func TestAccAWSSagemakerImage_description(t *testing.T) { + var image sagemaker.DescribeImageOutput + rName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_sagemaker_image.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSSagemakerImageDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSSagemakerImageDescription(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSagemakerImageExists(resourceName, &image), + resource.TestCheckResourceAttr(resourceName, "description", rName), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccAWSSagemakerImageBasicConfig(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSagemakerImageExists(resourceName, &image), + resource.TestCheckResourceAttr(resourceName, "description", ""), + ), + }, + { + Config: testAccAWSSagemakerImageDescription(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSagemakerImageExists(resourceName, &image), + resource.TestCheckResourceAttr(resourceName, "description", rName), + ), + }, + }, + }) +} + +func TestAccAWSSagemakerImage_displayName(t *testing.T) { + var image sagemaker.DescribeImageOutput + rName := acctest.RandomWithPrefix("tf-acc-test") + resourceName := "aws_sagemaker_image.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSSagemakerImageDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSSagemakerImageDisplayName(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSagemakerImageExists(resourceName, &image), + resource.TestCheckResourceAttr(resourceName, "display_name", rName), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccAWSSagemakerImageBasicConfig(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSagemakerImageExists(resourceName, &image), + resource.TestCheckResourceAttr(resourceName, "display_name", ""), + ), + }, + { + Config: testAccAWSSagemakerImageDisplayName(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSSagemakerImageExists(resourceName, &image), + resource.TestCheckResourceAttr(resourceName, "display_name", rName), + ), + }, + }, + }) +} + func TestAccAWSSagemakerImage_tags(t *testing.T) { var image sagemaker.DescribeImageOutput rName := acctest.RandomWithPrefix("tf-acc-test") @@ -226,6 +306,26 @@ resource "aws_sagemaker_image" "test" { `, rName) } +func testAccAWSSagemakerImageDescription(rName string) string { + return testAccAWSSagemakerImageConfigBase(rName) + fmt.Sprintf(` +resource "aws_sagemaker_image" "test" { + image_name = %[1]q + role_arn = aws_iam_role.test.arn + description = %[1]q +} +`, rName) +} + +func testAccAWSSagemakerImageDisplayName(rName string) string { + return testAccAWSSagemakerImageConfigBase(rName) + fmt.Sprintf(` +resource "aws_sagemaker_image" "test" { + image_name = %[1]q + role_arn = aws_iam_role.test.arn + display_name = %[1]q +} +`, rName) +} + func testAccAWSSagemakerImageConfigTags1(rName, tagKey1, tagValue1 string) string { return testAccAWSSagemakerImageConfigBase(rName) + fmt.Sprintf(` resource "aws_sagemaker_image" "test" { From 7b059dfc6312cf900431777ac68cbae18cfa8a0f Mon Sep 17 00:00:00 2001 From: drfaust92 Date: Sun, 8 Nov 2020 19:35:51 +0200 Subject: [PATCH 05/14] docs --- website/docs/r/sagemaker_image.html.markdown | 47 ++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 website/docs/r/sagemaker_image.html.markdown diff --git a/website/docs/r/sagemaker_image.html.markdown b/website/docs/r/sagemaker_image.html.markdown new file mode 100644 index 000000000000..ebe5416cee10 --- /dev/null +++ b/website/docs/r/sagemaker_image.html.markdown @@ -0,0 +1,47 @@ +--- +subcategory: "Sagemaker" +layout: "aws" +page_title: "AWS: aws_sagemaker_image" +description: |- + Provides a Sagemaker Image resource. +--- + +# Resource: aws_sagemaker_image + +Provides a Sagemaker Image resource. + +## Example Usage + +### Basic usage + +```hcl +resource "aws_sagemaker_image" "example" { + image_name = "example" + role_arn = aws_iam_role.test.arn +} +``` + +## Argument Reference + +The following arguments are supported: + +* `image_name` - (Required) The name of the image. Must be unique to your account. +* `role_arn` - (Required) The Amazon Resource Name (ARN) of an IAM role that enables Amazon SageMaker to perform tasks on your behalf. +* `display_name` - (Optional) The display name of the image. When the image is added to a domain (must be unique to the domain). +* `description` - (Optional) The description of the image. +* `tags` - (Optional) A map of tags to assign to the resource. + +## Attributes Reference + +The following attributes are exported: + +* `id` - The name of the Image. +* `arn` - The Amazon Resource Name (ARN) assigned by AWS to this Image. + +## Import + +Sagemaker Code Images can be imported using the `name`, e.g. + +``` +$ terraform import aws_sagemaker_image.test_image my-code-repo +``` From 401900edf7dc00b86612c36b0f802a8a89e26a77 Mon Sep 17 00:00:00 2001 From: drfaust92 Date: Sun, 8 Nov 2020 19:38:23 +0200 Subject: [PATCH 06/14] delete timeput --- aws/internal/service/sagemaker/waiter/waiter.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/aws/internal/service/sagemaker/waiter/waiter.go b/aws/internal/service/sagemaker/waiter/waiter.go index 42a901ebefad..e6ff40fb82a0 100644 --- a/aws/internal/service/sagemaker/waiter/waiter.go +++ b/aws/internal/service/sagemaker/waiter/waiter.go @@ -12,6 +12,7 @@ const ( NotebookInstanceStoppedTimeout = 10 * time.Minute NotebookInstanceDeletedTimeout = 10 * time.Minute ImageCreatedTimeout = 10 * time.Minute + ImageDeletedTimeout = 10 * time.Minute ) // NotebookInstanceInService waits for a NotebookInstance to return InService @@ -105,7 +106,7 @@ func ImageDeleted(conn *sagemaker.SageMaker, name string) (*sagemaker.DescribeIm Pending: []string{sagemaker.ImageStatusDeleting}, Target: []string{}, Refresh: ImageStatus(conn, name), - Timeout: ImageCreatedTimeout, + Timeout: ImageDeletedTimeout, } outputRaw, err := stateConf.WaitForState() From ba3dc6d62eecd5be7dd17367f8af8e595733b5a4 Mon Sep 17 00:00:00 2001 From: drfaust92 Date: Sun, 8 Nov 2020 19:39:36 +0200 Subject: [PATCH 07/14] fmt --- aws/resource_aws_sagemaker_image_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/aws/resource_aws_sagemaker_image_test.go b/aws/resource_aws_sagemaker_image_test.go index f6d2851c6ba5..d6876d83a6c5 100644 --- a/aws/resource_aws_sagemaker_image_test.go +++ b/aws/resource_aws_sagemaker_image_test.go @@ -286,12 +286,12 @@ resource "aws_iam_role" "test" { data "aws_iam_policy_document" "test" { statement { - actions = ["sts:AssumeRole"] + actions = ["sts:AssumeRole"] - principals { - type = "Service" - identifiers = ["sagemaker.amazonaws.com"] - } + principals { + type = "Service" + identifiers = ["sagemaker.amazonaws.com"] + } } } `, rName) From 79c57648c051281d864c2918571f93d2ef863fff Mon Sep 17 00:00:00 2001 From: Ilia Lazebnik Date: Mon, 9 Nov 2020 22:54:20 +0200 Subject: [PATCH 08/14] Update aws/resource_aws_sagemaker_image.go Co-authored-by: Kit Ewbank --- aws/resource_aws_sagemaker_image.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws/resource_aws_sagemaker_image.go b/aws/resource_aws_sagemaker_image.go index ff5eba23144a..a8079504c2f2 100644 --- a/aws/resource_aws_sagemaker_image.go +++ b/aws/resource_aws_sagemaker_image.go @@ -141,7 +141,7 @@ func resourceAwsSagemakerImageUpdate(d *schema.ResourceData, meta interface{}) e var deleteProperties []*string if d.HasChange("role_arn") { - input.Description = aws.String(d.Get("role_arn").(string)) + input.RoleArn= aws.String(d.Get("role_arn").(string)) } if d.HasChange("description") { From cee842aefd374341d9292cf4967dac6c7a882476 Mon Sep 17 00:00:00 2001 From: Ilia Lazebnik Date: Tue, 10 Nov 2020 09:04:32 +0200 Subject: [PATCH 09/14] Apply suggestions from code review Co-authored-by: Kit Ewbank --- aws/resource_aws_sagemaker_image.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/aws/resource_aws_sagemaker_image.go b/aws/resource_aws_sagemaker_image.go index a8079504c2f2..9a603bad8d5a 100644 --- a/aws/resource_aws_sagemaker_image.go +++ b/aws/resource_aws_sagemaker_image.go @@ -89,7 +89,7 @@ func resourceAwsSagemakerImageCreate(d *schema.ResourceData, meta interface{}) e d.SetId(name) if _, err := waiter.ImageCreated(conn, d.Id()); err != nil { - return fmt.Errorf("error waiting for sagemaker image (%s) to create: %w", d.Id(), err) + return fmt.Errorf("error waiting for SageMaker Image (%s) to create: %w", d.Id(), err) } return resourceAwsSagemakerImageRead(d, meta) @@ -120,7 +120,7 @@ func resourceAwsSagemakerImageRead(d *schema.ResourceData, meta interface{}) err tags, err := keyvaluetags.SagemakerListTags(conn, arn) if err != nil { - return fmt.Errorf("error listing tags for Sagemaker Image (%s): %w", d.Id(), err) + return fmt.Errorf("error listing tags for SageMaker Image (%s): %w", d.Id(), err) } if err := d.Set("tags", tags.IgnoreAws().IgnoreConfig(ignoreTagsConfig).Map()); err != nil { @@ -172,7 +172,7 @@ func resourceAwsSagemakerImageUpdate(d *schema.ResourceData, meta interface{}) e } if _, err := waiter.ImageCreated(conn, d.Id()); err != nil { - return fmt.Errorf("error waiting for sagemaker image (%s) to update: %w", d.Id(), err) + return fmt.Errorf("error waiting for SageMaker Image (%s) to update: %w", d.Id(), err) } } @@ -180,7 +180,7 @@ func resourceAwsSagemakerImageUpdate(d *schema.ResourceData, meta interface{}) e o, n := d.GetChange("tags") if err := keyvaluetags.SagemakerUpdateTags(conn, d.Get("arn").(string), o, n); err != nil { - return fmt.Errorf("error updating Sagemaker Image (%s) tags: %s", d.Id(), err) + return fmt.Errorf("error updating SageMaker Image (%s) tags: %s", d.Id(), err) } } From 46d47430e3e0342a29fb98884f5d41f8b73dc396 Mon Sep 17 00:00:00 2001 From: Ilia Lazebnik Date: Tue, 10 Nov 2020 09:05:07 +0200 Subject: [PATCH 10/14] Apply suggestions from code review Co-authored-by: Kit Ewbank --- aws/resource_aws_sagemaker_image.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws/resource_aws_sagemaker_image.go b/aws/resource_aws_sagemaker_image.go index 9a603bad8d5a..69ad2364c2cb 100644 --- a/aws/resource_aws_sagemaker_image.go +++ b/aws/resource_aws_sagemaker_image.go @@ -205,7 +205,7 @@ func resourceAwsSagemakerImageDelete(d *schema.ResourceData, meta interface{}) e if isAWSErr(err, sagemaker.ErrCodeResourceNotFound, "No Image with the name") { return nil } - return fmt.Errorf("error waiting for sagemaker image (%s) to delete: %w", d.Id(), err) + return fmt.Errorf("error waiting for SageMaker Image (%s) to delete: %w", d.Id(), err) } From ef0a2a393f7a41aae8937fe3021323d14729af52 Mon Sep 17 00:00:00 2001 From: Ilia Lazebnik Date: Tue, 10 Nov 2020 15:56:06 +0200 Subject: [PATCH 11/14] Update aws/resource_aws_sagemaker_image.go Co-authored-by: Kit Ewbank --- aws/resource_aws_sagemaker_image.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws/resource_aws_sagemaker_image.go b/aws/resource_aws_sagemaker_image.go index 69ad2364c2cb..c7556bc47c99 100644 --- a/aws/resource_aws_sagemaker_image.go +++ b/aws/resource_aws_sagemaker_image.go @@ -141,7 +141,7 @@ func resourceAwsSagemakerImageUpdate(d *schema.ResourceData, meta interface{}) e var deleteProperties []*string if d.HasChange("role_arn") { - input.RoleArn= aws.String(d.Get("role_arn").(string)) + input.RoleArn = aws.String(d.Get("role_arn").(string)) } if d.HasChange("description") { From 2cd0552f85a56e30932313489837865807b9b4cc Mon Sep 17 00:00:00 2001 From: drfaust92 Date: Mon, 16 Nov 2020 17:47:35 +0200 Subject: [PATCH 12/14] wait before create --- .../service/sagemaker/waiter/status.go | 8 +++- aws/resource_aws_sagemaker_image.go | 40 ++++++++++++++----- aws/resource_aws_sagemaker_image_test.go | 5 ++- 3 files changed, 39 insertions(+), 14 deletions(-) diff --git a/aws/internal/service/sagemaker/waiter/status.go b/aws/internal/service/sagemaker/waiter/status.go index 8d615f69f937..1f8f5d00dbe7 100644 --- a/aws/internal/service/sagemaker/waiter/status.go +++ b/aws/internal/service/sagemaker/waiter/status.go @@ -1,6 +1,8 @@ package waiter import ( + "fmt" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/sagemaker" "github.com/hashicorp/aws-sdk-go-base/tfawserr" @@ -47,7 +49,7 @@ func ImageStatus(conn *sagemaker.SageMaker, name string) resource.StateRefreshFu output, err := conn.DescribeImage(input) - if tfawserr.ErrMessageContains(err, "ValidationException", "RecordNotFound") { + if tfawserr.ErrMessageContains(err, sagemaker.ErrCodeResourceNotFound, "No Image with the name") { return nil, SagemakerImageStatusNotFound, nil } @@ -59,6 +61,10 @@ func ImageStatus(conn *sagemaker.SageMaker, name string) resource.StateRefreshFu return nil, SagemakerImageStatusNotFound, nil } + if aws.StringValue(output.ImageStatus) == sagemaker.ImageStatusCreateFailed { + return output, sagemaker.ImageStatusCreateFailed, fmt.Errorf("%s", aws.StringValue(output.FailureReason)) + } + return output, aws.StringValue(output.ImageStatus), nil } } diff --git a/aws/resource_aws_sagemaker_image.go b/aws/resource_aws_sagemaker_image.go index c7556bc47c99..03db1228ffe5 100644 --- a/aws/resource_aws_sagemaker_image.go +++ b/aws/resource_aws_sagemaker_image.go @@ -4,9 +4,12 @@ import ( "fmt" "log" "regexp" + "strings" + "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/sagemaker" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" @@ -42,6 +45,7 @@ func resourceAwsSagemakerImage() *schema.Resource { "role_arn": { Type: schema.TypeString, Required: true, + ForceNew: true, ValidateFunc: validateArn, }, "display_name": { @@ -80,16 +84,34 @@ func resourceAwsSagemakerImageCreate(d *schema.ResourceData, meta interface{}) e input.Tags = keyvaluetags.New(v.(map[string]interface{})).IgnoreAws().SagemakerTags() } + // for some reason even if the operation is retried the same response is given even though the role is valid. a short sleep before creation solves it. + time.Sleep(1 * time.Minute) log.Printf("[DEBUG] sagemaker Image create config: %#v", *input) - _, err := conn.CreateImage(input) - if err != nil { - return fmt.Errorf("error creating SageMaker Image: %w", err) - } + err := resource.Retry(1*time.Minute, func() *resource.RetryError { + var err error + _, err = conn.CreateImage(input) + if err != nil { + return resource.NonRetryableError(fmt.Errorf("error creating SageMaker Image: %w", err)) + } + + d.SetId(name) - d.SetId(name) + out, err := waiter.ImageCreated(conn, d.Id()) - if _, err := waiter.ImageCreated(conn, d.Id()); err != nil { - return fmt.Errorf("error waiting for SageMaker Image (%s) to create: %w", d.Id(), err) + if strings.Contains(aws.StringValue(out.FailureReason), "Unable to assume role with RoleArn") { + return resource.RetryableError(err) + } + if err != nil { + return resource.NonRetryableError(fmt.Errorf("error waiting for SageMaker Image (%s) to create: %w", d.Id(), err)) + } + return nil + }) + if isResourceTimeoutError(err) { + _, err = conn.CreateImage(input) + _, err = waiter.ImageCreated(conn, d.Id()) + } + if err != nil { + return fmt.Errorf("error creating SageMaker Image %s: %w", name, err) } return resourceAwsSagemakerImageRead(d, meta) @@ -140,10 +162,6 @@ func resourceAwsSagemakerImageUpdate(d *schema.ResourceData, meta interface{}) e var deleteProperties []*string - if d.HasChange("role_arn") { - input.RoleArn = aws.String(d.Get("role_arn").(string)) - } - if d.HasChange("description") { if v, ok := d.GetOk("description"); ok { input.Description = aws.String(v.(string)) diff --git a/aws/resource_aws_sagemaker_image_test.go b/aws/resource_aws_sagemaker_image_test.go index d6876d83a6c5..d2f25826f590 100644 --- a/aws/resource_aws_sagemaker_image_test.go +++ b/aws/resource_aws_sagemaker_image_test.go @@ -278,9 +278,10 @@ func testAccCheckAWSSagemakerImageExists(n string, image *sagemaker.DescribeImag func testAccAWSSagemakerImageConfigBase(rName string) string { return fmt.Sprintf(` +data "aws_partition" "current" {} + resource "aws_iam_role" "test" { name = %[1]q - path = "/" assume_role_policy = data.aws_iam_policy_document.test.json } @@ -290,7 +291,7 @@ data "aws_iam_policy_document" "test" { principals { type = "Service" - identifiers = ["sagemaker.amazonaws.com"] + identifiers = ["sagemaker.${data.aws_partition.current.dns_suffix}"] } } } From 43c6942bba423f7e44b2c474647e64bd9bee14cf Mon Sep 17 00:00:00 2001 From: drfaust92 Date: Mon, 16 Nov 2020 17:48:06 +0200 Subject: [PATCH 13/14] comment --- aws/resource_aws_sagemaker_image.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws/resource_aws_sagemaker_image.go b/aws/resource_aws_sagemaker_image.go index 03db1228ffe5..4a434c315884 100644 --- a/aws/resource_aws_sagemaker_image.go +++ b/aws/resource_aws_sagemaker_image.go @@ -84,7 +84,7 @@ func resourceAwsSagemakerImageCreate(d *schema.ResourceData, meta interface{}) e input.Tags = keyvaluetags.New(v.(map[string]interface{})).IgnoreAws().SagemakerTags() } - // for some reason even if the operation is retried the same response is given even though the role is valid. a short sleep before creation solves it. + // for some reason even if the operation is retried the same error response is given even though the role is valid. a short sleep before creation solves it. time.Sleep(1 * time.Minute) log.Printf("[DEBUG] sagemaker Image create config: %#v", *input) err := resource.Retry(1*time.Minute, func() *resource.RetryError { From f7e181fb06b0f8e280dd88f9139e83e6f5b03be0 Mon Sep 17 00:00:00 2001 From: drfaust92 Date: Thu, 10 Dec 2020 17:52:45 +0200 Subject: [PATCH 14/14] reduce retry logic --- aws/resource_aws_sagemaker_image.go | 33 ++++++----------------------- 1 file changed, 7 insertions(+), 26 deletions(-) diff --git a/aws/resource_aws_sagemaker_image.go b/aws/resource_aws_sagemaker_image.go index 4a434c315884..3da9bdb809ce 100644 --- a/aws/resource_aws_sagemaker_image.go +++ b/aws/resource_aws_sagemaker_image.go @@ -4,12 +4,10 @@ import ( "fmt" "log" "regexp" - "strings" "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/sagemaker" - "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" "github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags" @@ -86,34 +84,17 @@ func resourceAwsSagemakerImageCreate(d *schema.ResourceData, meta interface{}) e // for some reason even if the operation is retried the same error response is given even though the role is valid. a short sleep before creation solves it. time.Sleep(1 * time.Minute) - log.Printf("[DEBUG] sagemaker Image create config: %#v", *input) - err := resource.Retry(1*time.Minute, func() *resource.RetryError { - var err error - _, err = conn.CreateImage(input) - if err != nil { - return resource.NonRetryableError(fmt.Errorf("error creating SageMaker Image: %w", err)) - } - - d.SetId(name) - - out, err := waiter.ImageCreated(conn, d.Id()) - - if strings.Contains(aws.StringValue(out.FailureReason), "Unable to assume role with RoleArn") { - return resource.RetryableError(err) - } - if err != nil { - return resource.NonRetryableError(fmt.Errorf("error waiting for SageMaker Image (%s) to create: %w", d.Id(), err)) - } - return nil - }) - if isResourceTimeoutError(err) { - _, err = conn.CreateImage(input) - _, err = waiter.ImageCreated(conn, d.Id()) - } + _, err := conn.CreateImage(input) if err != nil { return fmt.Errorf("error creating SageMaker Image %s: %w", name, err) } + d.SetId(name) + + if _, err := waiter.ImageCreated(conn, d.Id()); err != nil { + return fmt.Errorf("error waiting for SageMaker Image (%s) to be created: %w", d.Id(), err) + } + return resourceAwsSagemakerImageRead(d, meta) }