From d2146f51618891476c144a703312ac1b12408d16 Mon Sep 17 00:00:00 2001 From: Roberth Kulbin <6707630+roberth-k@users.noreply.github.com> Date: Wed, 19 Jan 2022 23:52:37 +0000 Subject: [PATCH 1/8] r/aws_ecs_cluster_capacity_providers: new resource --- internal/provider/provider.go | 15 +- .../service/ecs/cluster_capacity_providers.go | 189 ++++++++ .../ecs/cluster_capacity_providers_test.go | 412 ++++++++++++++++++ internal/service/ecs/find.go | 16 + internal/service/ecs/flex.go | 41 ++ internal/service/ecs/service.go | 40 -- internal/service/ecs/validate.go | 12 + ...s_cluster_capacity_providers.html.markdown | 63 +++ 8 files changed, 741 insertions(+), 47 deletions(-) create mode 100644 internal/service/ecs/cluster_capacity_providers.go create mode 100644 internal/service/ecs/cluster_capacity_providers_test.go create mode 100644 website/docs/r/ecs_cluster_capacity_providers.html.markdown diff --git a/internal/provider/provider.go b/internal/provider/provider.go index 82837012758..ee1b5407c04 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -1188,13 +1188,14 @@ func Provider() *schema.Provider { "aws_ecrpublic_repository": ecrpublic.ResourceRepository(), "aws_ecrpublic_repository_policy": ecrpublic.ResourceRepositoryPolicy(), - "aws_ecs_account_setting_default": ecs.ResourceAccountSettingDefault(), - "aws_ecs_capacity_provider": ecs.ResourceCapacityProvider(), - "aws_ecs_cluster": ecs.ResourceCluster(), - "aws_ecs_service": ecs.ResourceService(), - "aws_ecs_tag": ecs.ResourceTag(), - "aws_ecs_task_definition": ecs.ResourceTaskDefinition(), - "aws_ecs_task_set": ecs.ResourceTaskSet(), + "aws_ecs_account_setting_default": ecs.ResourceAccountSettingDefault(), + "aws_ecs_capacity_provider": ecs.ResourceCapacityProvider(), + "aws_ecs_cluster": ecs.ResourceCluster(), + "aws_ecs_cluster_capacity_providers": ecs.ResourceClusterCapacityProviders(), + "aws_ecs_service": ecs.ResourceService(), + "aws_ecs_tag": ecs.ResourceTag(), + "aws_ecs_task_definition": ecs.ResourceTaskDefinition(), + "aws_ecs_task_set": ecs.ResourceTaskSet(), "aws_efs_access_point": efs.ResourceAccessPoint(), "aws_efs_backup_policy": efs.ResourceBackupPolicy(), diff --git a/internal/service/ecs/cluster_capacity_providers.go b/internal/service/ecs/cluster_capacity_providers.go new file mode 100644 index 00000000000..99f231d80f2 --- /dev/null +++ b/internal/service/ecs/cluster_capacity_providers.go @@ -0,0 +1,189 @@ +package ecs + +import ( + "context" + "log" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ecs" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "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/hashicorp/terraform-provider-aws/internal/conns" + "github.com/hashicorp/terraform-provider-aws/internal/flex" + "github.com/hashicorp/terraform-provider-aws/internal/tfresource" +) + +func ResourceClusterCapacityProviders() *schema.Resource { + return &schema.Resource{ + CreateContext: resourceClusterCapacityProvidersPut, + ReadContext: resourceClusterCapacityProvidersRead, + UpdateContext: resourceClusterCapacityProvidersPut, + DeleteContext: resourceClusterCapacityProvidersDelete, + + Importer: &schema.ResourceImporter{ + StateContext: schema.ImportStatePassthroughContext, + }, + + Schema: map[string]*schema.Schema{ + "capacity_providers": { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Schema{ + Type: schema.TypeString, + }, + }, + "cluster_name": { + Type: schema.TypeString, + Required: true, + ForceNew: true, + // The API accepts both an ARN and a name in a generic "cluster" + // parameter, but allowing that would force the resource to guess + // which one to return on read. + ValidateFunc: validateClusterName, + }, + "default_capacity_provider_strategy": { + Type: schema.TypeSet, + Optional: true, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "base": { + Type: schema.TypeInt, + Default: 0, + Optional: true, + ValidateFunc: validation.IntBetween(0, 100000), + }, + "capacity_provider": { + Type: schema.TypeString, + Required: true, + }, + "weight": { + Type: schema.TypeInt, + Default: 0, + Optional: true, + ValidateFunc: validation.IntBetween(0, 1000), + }, + }, + }, + }, + }, + } +} + +func resourceClusterCapacityProvidersPut(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + conn := meta.(*conns.AWSClient).ECSConn + + clusterName := d.Get("cluster_name").(string) + + input := &ecs.PutClusterCapacityProvidersInput{ + Cluster: aws.String(clusterName), + CapacityProviders: flex.ExpandStringSet(d.Get("capacity_providers").(*schema.Set)), + DefaultCapacityProviderStrategy: expandCapacityProviderStrategy(d.Get("default_capacity_provider_strategy").(*schema.Set)), + } + + log.Printf("[DEBUG] Updating ECS cluster capacity providers: %s", input) + + err := retryClusterCapacityProvidersPut(ctx, conn, input) + + if err != nil { + return diag.Errorf("error updating ECS Cluster (%s) capacity providers: %s", clusterName, err) + } + + if _, err := waitClusterAvailable(conn, clusterName); err != nil { + return diag.Errorf("error waiting for ECS Cluster (%s) to become available: %s", clusterName, err) + } + + d.SetId(clusterName) + + return resourceClusterCapacityProvidersRead(ctx, d, meta) +} + +func resourceClusterCapacityProvidersRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + conn := meta.(*conns.AWSClient).ECSConn + + cluster, err := FindOneClusterByNameOrARN(ctx, conn, d.Id()) + + if tfresource.NotFound(err) { + diag.Errorf("[WARN] ECS Cluster (%s) not found, removing from state", d.Id()) + d.SetId("") + return nil + } + + if err != nil { + return diag.Errorf("error reading ECS Cluster (%s): %s", d.Id(), err) + } + + // Status==INACTIVE means deleted cluster + if aws.StringValue(cluster.Status) == "INACTIVE" { + diag.Errorf("[WARN] ECS Cluster (%s) deleted, removing from state", d.Id()) + d.SetId("") + return nil + } + + if err := d.Set("capacity_providers", aws.StringValueSlice(cluster.CapacityProviders)); err != nil { + return diag.Errorf("error setting capacity_providers: %s", err) + } + + d.Set("cluster_name", cluster.ClusterName) + + if err := d.Set("default_capacity_provider_strategy", flattenCapacityProviderStrategy(cluster.DefaultCapacityProviderStrategy)); err != nil { + return diag.Errorf("error setting default_capacity_provider_strategy: %s", err) + } + + return nil +} + +func resourceClusterCapacityProvidersDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { + conn := meta.(*conns.AWSClient).ECSConn + + input := &ecs.PutClusterCapacityProvidersInput{ + Cluster: aws.String(d.Id()), + CapacityProviders: []*string{}, + DefaultCapacityProviderStrategy: []*ecs.CapacityProviderStrategyItem{}, + } + + log.Printf("[DEBUG] Removing ECS cluster (%s) capacity providers", d.Id()) + + err := retryClusterCapacityProvidersPut(ctx, conn, input) + + if tfawserr.ErrCodeEquals(err, ecs.ErrCodeClusterNotFoundException) { + return nil + } + + if err != nil { + return diag.Errorf("error deleting ECS Cluster (%s) capacity providers: %s", d.Id(), err) + } + + if _, err := waitClusterAvailable(conn, d.Id()); err != nil { + return diag.Errorf("error waiting for ECS Cluster (%s) to become available: %s", d.Id(), err) + } + + return nil +} + +func retryClusterCapacityProvidersPut(ctx context.Context, conn *ecs.ECS, input *ecs.PutClusterCapacityProvidersInput) error { + err := resource.RetryContext(ctx, ecsClusterTimeoutUpdate, func() *resource.RetryError { + _, err := conn.PutClusterCapacityProvidersWithContext(ctx, input) + if err != nil { + if tfawserr.ErrMessageContains(err, ecs.ErrCodeClientException, "Cluster was not ACTIVE") { + return resource.RetryableError(err) + } + if tfawserr.ErrMessageContains(err, ecs.ErrCodeResourceInUseException, "") { + return resource.RetryableError(err) + } + if tfawserr.ErrMessageContains(err, ecs.ErrCodeUpdateInProgressException, "") { + return resource.RetryableError(err) + } + return resource.NonRetryableError(err) + } + return nil + }) + + if tfresource.TimedOut(err) { + _, err = conn.PutClusterCapacityProvidersWithContext(ctx, input) + } + + return err +} diff --git a/internal/service/ecs/cluster_capacity_providers_test.go b/internal/service/ecs/cluster_capacity_providers_test.go new file mode 100644 index 00000000000..91412ae1d96 --- /dev/null +++ b/internal/service/ecs/cluster_capacity_providers_test.go @@ -0,0 +1,412 @@ +package ecs_test + +import ( + "fmt" + "testing" + + "github.com/aws/aws-sdk-go/service/ecs" + sdkacctest "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-provider-aws/internal/acctest" + tfecs "github.com/hashicorp/terraform-provider-aws/internal/service/ecs" +) + +func TestAccECSClusterCapacityProviders_basic(t *testing.T) { + var cluster ecs.Cluster + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_ecs_cluster_capacity_providers.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, ecs.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckClusterDestroy, + Steps: []resource.TestStep{ + { + Config: testAccClusterCapacityProvidersConfig(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckClusterExists("aws_ecs_cluster.test", &cluster), + resource.TestCheckResourceAttr(resourceName, "capacity_providers.#", "1"), + resource.TestCheckTypeSetElemAttr(resourceName, "capacity_providers.*", "FARGATE"), + resource.TestCheckResourceAttr(resourceName, "cluster_name", rName), + resource.TestCheckResourceAttr(resourceName, "default_capacity_provider_strategy.#", "1"), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "default_capacity_provider_strategy.*", map[string]string{ + "base": "1", + "weight": "100", + "capacity_provider": "FARGATE", + }), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccECSClusterCapacityProviders_disappears(t *testing.T) { + var cluster ecs.Cluster + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_ecs_cluster_capacity_providers.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, ecs.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckClusterDestroy, + Steps: []resource.TestStep{ + { + Config: testAccClusterCapacityProvidersConfig(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckClusterExists("aws_ecs_cluster.test", &cluster), + acctest.CheckResourceDisappears(acctest.Provider, tfecs.ResourceCluster(), resourceName), + ), + ExpectNonEmptyPlan: true, + }, + }, + }) +} + +func TestAccECSClusterCapacityProviders_defaults(t *testing.T) { + var cluster ecs.Cluster + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_ecs_cluster_capacity_providers.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, ecs.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckClusterDestroy, + Steps: []resource.TestStep{ + { + Config: testAccClusterCapacityProvidersConfig_defaults(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckClusterExists("aws_ecs_cluster.test", &cluster), + resource.TestCheckResourceAttr(resourceName, "capacity_providers.#", "0"), + resource.TestCheckResourceAttr(resourceName, "cluster_name", rName), + resource.TestCheckResourceAttr(resourceName, "default_capacity_provider_strategy.#", "0"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccECSClusterCapacityProviders_update_capacityProviders(t *testing.T) { + var cluster ecs.Cluster + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_ecs_cluster_capacity_providers.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, ecs.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckClusterDestroy, + Steps: []resource.TestStep{ + { + Config: testAccClusterCapacityProvidersConfig_withCapacityProviders1(rName, "FARGATE"), + Check: resource.ComposeTestCheckFunc( + testAccCheckClusterExists("aws_ecs_cluster.test", &cluster), + resource.TestCheckResourceAttr(resourceName, "capacity_providers.#", "1"), + resource.TestCheckTypeSetElemAttr(resourceName, "capacity_providers.*", "FARGATE"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccClusterCapacityProvidersConfig_withCapacityProviders2(rName, "FARGATE", "FARGATE_SPOT"), + Check: resource.ComposeTestCheckFunc( + testAccCheckClusterExists("aws_ecs_cluster.test", &cluster), + resource.TestCheckResourceAttr(resourceName, "capacity_providers.#", "2"), + resource.TestCheckTypeSetElemAttr(resourceName, "capacity_providers.*", "FARGATE"), + resource.TestCheckTypeSetElemAttr(resourceName, "capacity_providers.*", "FARGATE_SPOT"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccClusterCapacityProvidersConfig_withCapacityProviders0(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckClusterExists("aws_ecs_cluster.test", &cluster), + resource.TestCheckResourceAttr(resourceName, "capacity_providers.#", "0"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccClusterCapacityProvidersConfig_withCapacityProviders1(rName, "FARGATE"), + Check: resource.ComposeTestCheckFunc( + testAccCheckClusterExists("aws_ecs_cluster.test", &cluster), + resource.TestCheckResourceAttr(resourceName, "capacity_providers.#", "1"), + resource.TestCheckTypeSetElemAttr(resourceName, "capacity_providers.*", "FARGATE"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccECSClusterCapacityProviders_update_defaultCapacityProviderStrategy(t *testing.T) { + var cluster ecs.Cluster + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + resourceName := "aws_ecs_cluster_capacity_providers.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, ecs.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckClusterDestroy, + Steps: []resource.TestStep{ + { + Config: testAccClusterCapacityProvidersConfig_withDefaultCapacityProviderStrategy1(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckClusterExists("aws_ecs_cluster.test", &cluster), + resource.TestCheckResourceAttr(resourceName, "default_capacity_provider_strategy.#", "1"), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "default_capacity_provider_strategy.*", map[string]string{ + "base": "1", + "weight": "100", + "capacity_provider": "FARGATE", + }), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccClusterCapacityProvidersConfig_withDefaultCapacityProviderStrategy2(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckClusterExists("aws_ecs_cluster.test", &cluster), + resource.TestCheckResourceAttr(resourceName, "default_capacity_provider_strategy.#", "2"), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "default_capacity_provider_strategy.*", map[string]string{ + "base": "1", + "weight": "50", + "capacity_provider": "FARGATE", + }), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "default_capacity_provider_strategy.*", map[string]string{ + "base": "", + "weight": "50", + "capacity_provider": "FARGATE_SPOT", + }), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccClusterCapacityProvidersConfig_withDefaultCapacityProviderStrategy3(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckClusterExists("aws_ecs_cluster.test", &cluster), + resource.TestCheckResourceAttr(resourceName, "default_capacity_provider_strategy.#", "2"), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "default_capacity_provider_strategy.*", map[string]string{ + "base": "2", + "weight": "25", + "capacity_provider": "FARGATE", + }), + resource.TestCheckTypeSetElemNestedAttrs(resourceName, "default_capacity_provider_strategy.*", map[string]string{ + "base": "", + "weight": "75", + "capacity_provider": "FARGATE_SPOT", + }), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccClusterCapacityProvidersConfig_withDefaultCapacityProviderStrategy4(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckClusterExists("aws_ecs_cluster.test", &cluster), + resource.TestCheckResourceAttr(resourceName, "default_capacity_provider_strategy.#", "0"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func testAccClusterCapacityProvidersConfig(rName string) string { + return fmt.Sprintf(` +resource "aws_ecs_cluster" "test" { + name = %[1]q +} + +resource "aws_ecs_cluster_capacity_providers" "test" { + cluster_name = aws_ecs_cluster.test.name + + capacity_providers = ["FARGATE"] + + default_capacity_provider_strategy { + base = 1 + weight = 100 + capacity_provider = "FARGATE" + } +} +`, rName) +} + +func testAccClusterCapacityProvidersConfig_defaults(rName string) string { + return fmt.Sprintf(` +resource "aws_ecs_cluster" "test" { + name = %[1]q +} + +resource "aws_ecs_cluster_capacity_providers" "test" { + cluster_name = aws_ecs_cluster.test.name +} +`, rName) +} + +func testAccClusterCapacityProvidersConfig_withCapacityProviders0(rName string) string { + return fmt.Sprintf(` +resource "aws_ecs_cluster" "test" { + name = %[1]q +} + +resource "aws_ecs_cluster_capacity_providers" "test" { + cluster_name = aws_ecs_cluster.test.name + + capacity_providers = [] +} +`, rName) +} + +func testAccClusterCapacityProvidersConfig_withCapacityProviders1(rName, provider1 string) string { + return fmt.Sprintf(` +resource "aws_ecs_cluster" "test" { + name = %[1]q +} + +resource "aws_ecs_cluster_capacity_providers" "test" { + cluster_name = aws_ecs_cluster.test.name + + capacity_providers = [%[2]q] +} +`, rName, provider1) +} + +func testAccClusterCapacityProvidersConfig_withCapacityProviders2(rName, provider1, provider2 string) string { + return fmt.Sprintf(` +resource "aws_ecs_cluster" "test" { + name = %[1]q +} + +resource "aws_ecs_cluster_capacity_providers" "test" { + cluster_name = aws_ecs_cluster.test.name + + capacity_providers = [%[2]q, %[3]q] +} +`, rName, provider1, provider2) +} + +func testAccClusterCapacityProvidersConfig_withDefaultCapacityProviderStrategy1(rName string) string { + return fmt.Sprintf(` +resource "aws_ecs_cluster" "test" { + name = %[1]q +} + +resource "aws_ecs_cluster_capacity_providers" "test" { + cluster_name = aws_ecs_cluster.test.name + + capacity_providers = ["FARGATE", "FARGATE_SPOT"] + + default_capacity_provider_strategy { + base = 1 + weight = 100 + capacity_provider = "FARGATE" + } +} +`, rName) +} + +func testAccClusterCapacityProvidersConfig_withDefaultCapacityProviderStrategy2(rName string) string { + return fmt.Sprintf(` +resource "aws_ecs_cluster" "test" { + name = %[1]q +} + +resource "aws_ecs_cluster_capacity_providers" "test" { + cluster_name = aws_ecs_cluster.test.name + + capacity_providers = ["FARGATE", "FARGATE_SPOT"] + + default_capacity_provider_strategy { + base = 1 + weight = 50 + capacity_provider = "FARGATE" + } + + default_capacity_provider_strategy { + weight = 50 + capacity_provider = "FARGATE_SPOT" + } +} +`, rName) +} + +func testAccClusterCapacityProvidersConfig_withDefaultCapacityProviderStrategy3(rName string) string { + return fmt.Sprintf(` +resource "aws_ecs_cluster" "test" { + name = %[1]q +} + +resource "aws_ecs_cluster_capacity_providers" "test" { + cluster_name = aws_ecs_cluster.test.name + + capacity_providers = ["FARGATE", "FARGATE_SPOT"] + + default_capacity_provider_strategy { + base = 2 + weight = 25 + capacity_provider = "FARGATE" + } + + default_capacity_provider_strategy { + weight = 75 + capacity_provider = "FARGATE_SPOT" + } +} +`, rName) +} + +func testAccClusterCapacityProvidersConfig_withDefaultCapacityProviderStrategy4(rName string) string { + return fmt.Sprintf(` +resource "aws_ecs_cluster" "test" { + name = %[1]q +} + +resource "aws_ecs_cluster_capacity_providers" "test" { + cluster_name = aws_ecs_cluster.test.name + + capacity_providers = ["FARGATE", "FARGATE_SPOT"] +} +`, rName) +} diff --git a/internal/service/ecs/find.go b/internal/service/ecs/find.go index c0b4502ad5d..2097472c20e 100644 --- a/internal/service/ecs/find.go +++ b/internal/service/ecs/find.go @@ -1,6 +1,8 @@ package ecs import ( + "context" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ecs" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" @@ -59,3 +61,17 @@ func FindClusterByNameOrARN(conn *ecs.ECS, nameOrARN string) (*ecs.DescribeClust return output, nil } + +func FindOneClusterByNameOrARN(ctx context.Context, conn *ecs.ECS, nameOrArn string) (*ecs.Cluster, error) { + output, err := FindClusterByNameOrARN(conn, nameOrArn) + + if err != nil { + return nil, err + } + + if len(output.Clusters) == 0 || output.Clusters[0] == nil { + return nil, &resource.NotFoundError{} + } + + return output.Clusters[0], nil +} diff --git a/internal/service/ecs/flex.go b/internal/service/ecs/flex.go index 35911df9000..1bf224b2d28 100644 --- a/internal/service/ecs/flex.go +++ b/internal/service/ecs/flex.go @@ -3,8 +3,49 @@ package ecs import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ecs" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) +func expandCapacityProviderStrategy(cps *schema.Set) []*ecs.CapacityProviderStrategyItem { + list := cps.List() + results := make([]*ecs.CapacityProviderStrategyItem, 0) + for _, raw := range list { + cp := raw.(map[string]interface{}) + ps := &ecs.CapacityProviderStrategyItem{} + if val, ok := cp["base"]; ok { + ps.Base = aws.Int64(int64(val.(int))) + } + if val, ok := cp["weight"]; ok { + ps.Weight = aws.Int64(int64(val.(int))) + } + if val, ok := cp["capacity_provider"]; ok { + ps.CapacityProvider = aws.String(val.(string)) + } + + results = append(results, ps) + } + return results +} + +func flattenCapacityProviderStrategy(cps []*ecs.CapacityProviderStrategyItem) []map[string]interface{} { + if cps == nil { + return nil + } + results := make([]map[string]interface{}, 0) + for _, cp := range cps { + s := make(map[string]interface{}) + s["capacity_provider"] = aws.StringValue(cp.CapacityProvider) + if cp.Weight != nil { + s["weight"] = aws.Int64Value(cp.Weight) + } + if cp.Base != nil { + s["base"] = aws.Int64Value(cp.Base) + } + results = append(results, s) + } + return results +} + // Takes the result of flatmap. Expand for an array of load balancers and // returns ecs.LoadBalancer compatible objects func expandLoadBalancers(configured []interface{}) []*ecs.LoadBalancer { diff --git a/internal/service/ecs/service.go b/internal/service/ecs/service.go index 4afd657df94..4f853da4638 100644 --- a/internal/service/ecs/service.go +++ b/internal/service/ecs/service.go @@ -860,46 +860,6 @@ func expandNetworkConfiguration(nc []interface{}) *ecs.NetworkConfiguration { return &ecs.NetworkConfiguration{AwsvpcConfiguration: awsVpcConfig} } -func expandCapacityProviderStrategy(cps *schema.Set) []*ecs.CapacityProviderStrategyItem { - list := cps.List() - results := make([]*ecs.CapacityProviderStrategyItem, 0) - for _, raw := range list { - cp := raw.(map[string]interface{}) - ps := &ecs.CapacityProviderStrategyItem{} - if val, ok := cp["base"]; ok { - ps.Base = aws.Int64(int64(val.(int))) - } - if val, ok := cp["weight"]; ok { - ps.Weight = aws.Int64(int64(val.(int))) - } - if val, ok := cp["capacity_provider"]; ok { - ps.CapacityProvider = aws.String(val.(string)) - } - - results = append(results, ps) - } - return results -} - -func flattenCapacityProviderStrategy(cps []*ecs.CapacityProviderStrategyItem) []map[string]interface{} { - if cps == nil { - return nil - } - results := make([]map[string]interface{}, 0) - for _, cp := range cps { - s := make(map[string]interface{}) - s["capacity_provider"] = aws.StringValue(cp.CapacityProvider) - if cp.Weight != nil { - s["weight"] = aws.Int64Value(cp.Weight) - } - if cp.Base != nil { - s["base"] = aws.Int64Value(cp.Base) - } - results = append(results, s) - } - return results -} - func expandPlacementConstraints(tfList []interface{}) ([]*ecs.PlacementConstraint, error) { if len(tfList) == 0 { return nil, nil diff --git a/internal/service/ecs/validate.go b/internal/service/ecs/validate.go index 10a0b4195ef..9b887db021b 100644 --- a/internal/service/ecs/validate.go +++ b/internal/service/ecs/validate.go @@ -2,8 +2,20 @@ package ecs import ( "fmt" + "regexp" + + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" ) +func validateClusterName(v interface{}, k string) (ws []string, errors []error) { + return validation.All( + validation.StringLenBetween(1, 255), + validation.StringMatch( + regexp.MustCompile("[a-zA-Z0-9_-]+"), + "The cluster name must consist of alphanumerics, hyphens, and underscores."), + )(v, k) +} + // Validates that ECS Placement Constraints are set correctly // Takes type, and expression as strings func validPlacementConstraint(constType, constExpr string) error { diff --git a/website/docs/r/ecs_cluster_capacity_providers.html.markdown b/website/docs/r/ecs_cluster_capacity_providers.html.markdown new file mode 100644 index 00000000000..5afee04985e --- /dev/null +++ b/website/docs/r/ecs_cluster_capacity_providers.html.markdown @@ -0,0 +1,63 @@ +--- +subcategory: "ECS" +layout: "aws" +page_title: "AWS: aws_ecs_cluster_capacity_providers" +description: |- + Provides an ECS cluster capacity providers resource. +--- + +# Resource: aws_ecs_cluster_capacity_providers + +Manages the capacity providers of an ECS Cluster. + +More information about capacity providers can be found in the [ECS User Guide](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/cluster-capacity-providers.html). + +~> **NOTE on Clusters and Cluster Capacity Providers:** Terraform provides both a standalone [`aws_ecs_cluster_capacity_providers`](ecs_cluster_capacity_providers.html) resource, as well as allowing the capacity providers and default strategies to be managed in-line by the [`aws_ecs_cluster`](ecs_cluster.html) resource. You cannot use a Cluster with in-line capacity providers in conjunction with the Capacity Providers resource, as doing so will cause a conflict and will lead to mutual overwrites. + +## Example Usage + +```terraform +resource "aws_ecs_cluster" "example" { + name = "my-cluster" +} + +resource "aws_ecs_cluster_capacity_providers" "test" { + cluster_name = aws_ecs_cluster.test.name + + capacity_providers = ["FARGATE"] + + default_capacity_provider_strategy { + base = 1 + weight = 100 + capacity_provider = "FARGATE" + } +} +``` + +## Argument Reference + +The following arguments are supported: + +* `capacity_providers` - (Optional) Set of names of one or more capacity providers to associate with the cluster. Valid values also include `FARGATE` and `FARGATE_SPOT`. +* `cluster_name` - (Required, Forces new resource) Name of the ECS cluster to manage capacity providers for. +* `default_capacity_provider_strategy` - (Optional) Set of capacity provider strategies to use by default for the cluster. Detailed below. + +### default_capacity_provider_strategy Configuration Block + +* `capacity_provider` - (Required) Name of the capacity provider. +* `weight` - (Optional) The relative percentage of the total number of launched tasks that should use the specified capacity provider. The `weight` value is taken into consideration after the `base` count of tasks has been satisfied. Defaults to `0`. +* `base` - (Optional) The number of tasks, at a minimum, to run on the specified capacity provider. Only one capacity provider in a capacity provider strategy can have a base defined. Defaults to `0`. + +## Attributes Reference + +In addition to all arguments above, the following attributes are exported: + +* `id` - Same as `cluster_name`. + +## Import + +ECS cluster capacity providers can be imported using the `cluster_name` attribute. For example: + +``` +$ terraform import aws_ecs_cluster_capacity_providers.example my-cluster +``` From 013e17ba5afb98413c7817f48e70ac147bfeb9b7 Mon Sep 17 00:00:00 2001 From: Roberth Kulbin <6707630+roberth-k@users.noreply.github.com> Date: Thu, 20 Jan 2022 00:20:15 +0000 Subject: [PATCH 2/8] r/aws_ecs_cluster: adapt for r/aws_ecs_cluster_capacity_providers --- internal/service/ecs/cluster.go | 26 ++++++------------------ website/docs/r/ecs_cluster.html.markdown | 2 ++ 2 files changed, 8 insertions(+), 20 deletions(-) diff --git a/internal/service/ecs/cluster.go b/internal/service/ecs/cluster.go index ff88d06c9ca..e0549cc0378 100644 --- a/internal/service/ecs/cluster.go +++ b/internal/service/ecs/cluster.go @@ -1,6 +1,7 @@ package ecs import ( + "context" "fmt" "log" "time" @@ -42,7 +43,7 @@ func ResourceCluster() *schema.Resource { Type: schema.TypeString, Required: true, ForceNew: true, - ValidateFunc: validation.StringLenBetween(1, 255), + ValidateFunc: validateClusterName, }, "arn": { Type: schema.TypeString, @@ -51,6 +52,7 @@ func ResourceCluster() *schema.Resource { "capacity_providers": { Type: schema.TypeSet, Optional: true, + Computed: true, Elem: &schema.Schema{ Type: schema.TypeString, }, @@ -114,6 +116,7 @@ func ResourceCluster() *schema.Resource { "default_capacity_provider_strategy": { Type: schema.TypeSet, Optional: true, + Computed: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "base": { @@ -373,25 +376,8 @@ func resourceClusterUpdate(d *schema.ResourceData, meta interface{}) error { DefaultCapacityProviderStrategy: expandCapacityProviderStrategy(d.Get("default_capacity_provider_strategy").(*schema.Set)), } - err := resource.Retry(ecsClusterTimeoutUpdate, func() *resource.RetryError { - _, err := conn.PutClusterCapacityProviders(&input) - if err != nil { - if tfawserr.ErrMessageContains(err, ecs.ErrCodeClientException, "Cluster was not ACTIVE") { - return resource.RetryableError(err) - } - if tfawserr.ErrMessageContains(err, ecs.ErrCodeResourceInUseException, "") { - return resource.RetryableError(err) - } - if tfawserr.ErrMessageContains(err, ecs.ErrCodeUpdateInProgressException, "") { - return resource.RetryableError(err) - } - return resource.NonRetryableError(err) - } - return nil - }) - if tfresource.TimedOut(err) { - _, err = conn.PutClusterCapacityProviders(&input) - } + err := retryClusterCapacityProvidersPut(context.Background(), conn, &input) + if err != nil { return fmt.Errorf("error changing ECS cluster capacity provider settings (%s): %w", d.Id(), err) } diff --git a/website/docs/r/ecs_cluster.html.markdown b/website/docs/r/ecs_cluster.html.markdown index 3aa94394f93..73670ca2e33 100644 --- a/website/docs/r/ecs_cluster.html.markdown +++ b/website/docs/r/ecs_cluster.html.markdown @@ -10,6 +10,8 @@ description: |- Provides an ECS cluster. +~> **NOTE on Clusters and Cluster Capacity Providers:** Terraform provides both a standalone [`aws_ecs_cluster_capacity_providers`](ecs_cluster_capacity_providers.html) resource, as well as allowing the capacity providers and default strategies to be managed in-line by the [`aws_ecs_cluster`](ecs_cluster.html) resource. You cannot use a Cluster with in-line capacity providers in conjunction with the Capacity Providers resource, as doing so will cause a conflict and will lead to mutual overwrites. + ## Example Usage ```terraform From 6051dc2761a6e33e95b99c60a49f6b01755bc5d1 Mon Sep 17 00:00:00 2001 From: Roberth Kulbin <6707630+roberth-k@users.noreply.github.com> Date: Thu, 20 Jan 2022 00:21:52 +0000 Subject: [PATCH 3/8] service/ecs: refactor FindClusterByNameOrARN such that it returns a single cluster or error --- internal/service/ecs/cluster.go | 32 ++++++------------- .../service/ecs/cluster_capacity_providers.go | 2 +- internal/service/ecs/cluster_data_source.go | 12 ++----- internal/service/ecs/cluster_test.go | 19 +++++------ internal/service/ecs/find.go | 27 +++++++--------- 5 files changed, 32 insertions(+), 60 deletions(-) diff --git a/internal/service/ecs/cluster.go b/internal/service/ecs/cluster.go index e0549cc0378..6744ff49f45 100644 --- a/internal/service/ecs/cluster.go +++ b/internal/service/ecs/cluster.go @@ -250,26 +250,26 @@ func resourceClusterRead(d *schema.ResourceData, meta interface{}) error { defaultTagsConfig := meta.(*conns.AWSClient).DefaultTagsConfig ignoreTagsConfig := meta.(*conns.AWSClient).IgnoreTagsConfig - var out *ecs.DescribeClustersOutput + var cluster *ecs.Cluster err := resource.Retry(2*time.Minute, func() *resource.RetryError { var err error - out, err = FindClusterByNameOrARN(conn, d.Id()) + cluster, err = FindClusterByNameOrARN(context.Background(), conn, d.Id()) - if err != nil { + if tfresource.NotFound(err) { + if d.IsNewResource() { + return resource.RetryableError(err) + } return resource.NonRetryableError(err) } - if out == nil || len(out.Failures) > 0 { - if d.IsNewResource() { - return resource.RetryableError(&resource.NotFoundError{}) - } - return resource.NonRetryableError(&resource.NotFoundError{}) + if err != nil { + return resource.NonRetryableError(err) } return nil }) if tfresource.TimedOut(err) { - out, err = FindClusterByNameOrARN(conn, d.Id()) + cluster, err = FindClusterByNameOrARN(context.Background(), conn, d.Id()) } if tfresource.NotFound(err) { @@ -282,20 +282,6 @@ func resourceClusterRead(d *schema.ResourceData, meta interface{}) error { return fmt.Errorf("error reading ECS Cluster (%s): %s", d.Id(), err) } - var cluster *ecs.Cluster - for _, c := range out.Clusters { - if aws.StringValue(c.ClusterArn) == d.Id() { - cluster = c - break - } - } - - if cluster == nil { - log.Printf("[WARN] ECS Cluster (%s) not found, removing from state", d.Id()) - d.SetId("") - return nil - } - // Status==INACTIVE means deleted cluster if aws.StringValue(cluster.Status) == "INACTIVE" { log.Printf("[WARN] ECS Cluster (%s) deleted, removing from state", d.Id()) diff --git a/internal/service/ecs/cluster_capacity_providers.go b/internal/service/ecs/cluster_capacity_providers.go index 99f231d80f2..4c339893007 100644 --- a/internal/service/ecs/cluster_capacity_providers.go +++ b/internal/service/ecs/cluster_capacity_providers.go @@ -103,7 +103,7 @@ func resourceClusterCapacityProvidersPut(ctx context.Context, d *schema.Resource func resourceClusterCapacityProvidersRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { conn := meta.(*conns.AWSClient).ECSConn - cluster, err := FindOneClusterByNameOrARN(ctx, conn, d.Id()) + cluster, err := FindClusterByNameOrARN(ctx, conn, d.Id()) if tfresource.NotFound(err) { diag.Errorf("[WARN] ECS Cluster (%s) not found, removing from state", d.Id()) diff --git a/internal/service/ecs/cluster_data_source.go b/internal/service/ecs/cluster_data_source.go index 2aa010825f9..e05360e4cd7 100644 --- a/internal/service/ecs/cluster_data_source.go +++ b/internal/service/ecs/cluster_data_source.go @@ -1,6 +1,7 @@ package ecs import ( + "context" "fmt" "github.com/aws/aws-sdk-go/aws" @@ -67,21 +68,12 @@ func dataSourceClusterRead(d *schema.ResourceData, meta interface{}) error { conn := meta.(*conns.AWSClient).ECSConn clusterName := d.Get("cluster_name").(string) - desc, err := FindClusterByNameOrARN(conn, d.Get("cluster_name").(string)) + cluster, err := FindClusterByNameOrARN(context.Background(), conn, d.Get("cluster_name").(string)) if err != nil { return fmt.Errorf("error reading ECS Cluster (%s): %w", clusterName, err) } - if len(desc.Clusters) == 0 { - return fmt.Errorf("no matches found for name: %s", clusterName) - } - - if len(desc.Clusters) > 1 { - return fmt.Errorf("multiple matches found for name: %s", clusterName) - } - - cluster := desc.Clusters[0] d.SetId(aws.StringValue(cluster.ClusterArn)) d.Set("arn", cluster.ClusterArn) d.Set("status", cluster.Status) diff --git a/internal/service/ecs/cluster_test.go b/internal/service/ecs/cluster_test.go index c231b8b8344..2f24d816e54 100644 --- a/internal/service/ecs/cluster_test.go +++ b/internal/service/ecs/cluster_test.go @@ -1,6 +1,7 @@ package ecs_test import ( + "context" "fmt" "testing" @@ -345,16 +346,14 @@ func testAccCheckClusterDestroy(s *terraform.State) error { continue } - out, err := tfecs.FindClusterByNameOrARN(conn, rs.Primary.ID) + c, err := tfecs.FindClusterByNameOrARN(context.Background(), conn, rs.Primary.ID) if err != nil { return err } - for _, c := range out.Clusters { - if aws.StringValue(c.ClusterArn) == rs.Primary.ID && aws.StringValue(c.Status) != "INACTIVE" { - return fmt.Errorf("ECS cluster still exists:\n%s", c) - } + if aws.StringValue(c.ClusterArn) == rs.Primary.ID && aws.StringValue(c.Status) != "INACTIVE" { + return fmt.Errorf("ECS cluster still exists:\n%s", c) } } @@ -369,17 +368,15 @@ func testAccCheckClusterExists(resourceName string, cluster *ecs.Cluster) resour } conn := acctest.Provider.Meta().(*conns.AWSClient).ECSConn - output, err := tfecs.FindClusterByNameOrARN(conn, rs.Primary.ID) + c, err := tfecs.FindClusterByNameOrARN(context.Background(), conn, rs.Primary.ID) if err != nil { return fmt.Errorf("error reading ECS Cluster (%s): %w", rs.Primary.ID, err) } - for _, c := range output.Clusters { - if aws.StringValue(c.ClusterArn) == rs.Primary.ID && aws.StringValue(c.Status) != "INACTIVE" { - *cluster = *c - return nil - } + if aws.StringValue(c.ClusterArn) == rs.Primary.ID && aws.StringValue(c.Status) != "INACTIVE" { + *cluster = *c + return nil } return fmt.Errorf("ECS Cluster (%s) not found", rs.Primary.ID) diff --git a/internal/service/ecs/find.go b/internal/service/ecs/find.go index 2097472c20e..5ecc8a2b7ee 100644 --- a/internal/service/ecs/find.go +++ b/internal/service/ecs/find.go @@ -5,6 +5,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ecs" + "github.com/hashicorp/aws-sdk-go-base/tfawserr" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-provider-aws/internal/tfresource" ) @@ -40,37 +41,33 @@ func FindCapacityProviderByARN(conn *ecs.ECS, arn string) (*ecs.CapacityProvider return capacityProvider, nil } -func FindClusterByNameOrARN(conn *ecs.ECS, nameOrARN string) (*ecs.DescribeClustersOutput, error) { +func FindClusterByNameOrARN(ctx context.Context, conn *ecs.ECS, nameOrARN string) (*ecs.Cluster, error) { input := &ecs.DescribeClustersInput{ Clusters: aws.StringSlice([]string{nameOrARN}), Include: aws.StringSlice([]string{ecs.ClusterFieldTags, ecs.ClusterFieldConfigurations, ecs.ClusterFieldSettings}), } - output, err := conn.DescribeClusters(input) + output, err := conn.DescribeClustersWithContext(ctx, input) - if err != nil { + if tfawserr.ErrCodeEquals(err, ecs.ErrCodeClusterNotFoundException) { return nil, &resource.NotFoundError{ LastError: err, LastRequest: input, } } - if output == nil { - return nil, tfresource.NewEmptyResultError(input) - } - - return output, nil -} - -func FindOneClusterByNameOrARN(ctx context.Context, conn *ecs.ECS, nameOrArn string) (*ecs.Cluster, error) { - output, err := FindClusterByNameOrARN(conn, nameOrArn) - if err != nil { return nil, err } - if len(output.Clusters) == 0 || output.Clusters[0] == nil { - return nil, &resource.NotFoundError{} + if output == nil || len(output.Clusters) == 0 || output.Clusters[0] == nil { + return nil, &resource.NotFoundError{ + LastRequest: input, + } + } + + if count := len(output.Clusters); count > 1 { + return nil, tfresource.NewTooManyResultsError(count, input) } return output.Clusters[0], nil From 688933c1e6bd7233a48600f26f7f3ccc597edb89 Mon Sep 17 00:00:00 2001 From: Roberth Kulbin <6707630+roberth-k@users.noreply.github.com> Date: Thu, 20 Jan 2022 00:26:50 +0000 Subject: [PATCH 4/8] service/ecs: pass context to waitClusterAvailable --- internal/service/ecs/cluster.go | 6 +++--- internal/service/ecs/cluster_capacity_providers.go | 4 ++-- internal/service/ecs/status.go | 13 +++++-------- internal/service/ecs/wait.go | 9 +++++---- 4 files changed, 15 insertions(+), 17 deletions(-) diff --git a/internal/service/ecs/cluster.go b/internal/service/ecs/cluster.go index 6744ff49f45..495087352be 100644 --- a/internal/service/ecs/cluster.go +++ b/internal/service/ecs/cluster.go @@ -223,7 +223,7 @@ func resourceClusterCreate(d *schema.ResourceData, meta interface{}) error { d.SetId(aws.StringValue(out.Cluster.ClusterArn)) - if _, err := waitClusterAvailable(conn, d.Id()); err != nil { + if _, err := waitClusterAvailable(context.Background(), conn, d.Id()); err != nil { return fmt.Errorf("error waiting for ECS Cluster (%s) to become Available: %w", d.Id(), err) } @@ -350,7 +350,7 @@ func resourceClusterUpdate(d *schema.ResourceData, meta interface{}) error { return fmt.Errorf("error changing ECS cluster (%s): %w", d.Id(), err) } - if _, err := waitClusterAvailable(conn, d.Id()); err != nil { + if _, err := waitClusterAvailable(context.Background(), conn, d.Id()); err != nil { return fmt.Errorf("error waiting for ECS Cluster (%s) to become Available: %w", d.Id(), err) } } @@ -368,7 +368,7 @@ func resourceClusterUpdate(d *schema.ResourceData, meta interface{}) error { return fmt.Errorf("error changing ECS cluster capacity provider settings (%s): %w", d.Id(), err) } - if _, err := waitClusterAvailable(conn, d.Id()); err != nil { + if _, err := waitClusterAvailable(context.Background(), conn, d.Id()); err != nil { return fmt.Errorf("error waiting for ECS Cluster (%s) to become Available: %w", d.Id(), err) } } diff --git a/internal/service/ecs/cluster_capacity_providers.go b/internal/service/ecs/cluster_capacity_providers.go index 4c339893007..cb9fdb02b70 100644 --- a/internal/service/ecs/cluster_capacity_providers.go +++ b/internal/service/ecs/cluster_capacity_providers.go @@ -91,7 +91,7 @@ func resourceClusterCapacityProvidersPut(ctx context.Context, d *schema.Resource return diag.Errorf("error updating ECS Cluster (%s) capacity providers: %s", clusterName, err) } - if _, err := waitClusterAvailable(conn, clusterName); err != nil { + if _, err := waitClusterAvailable(ctx, conn, clusterName); err != nil { return diag.Errorf("error waiting for ECS Cluster (%s) to become available: %s", clusterName, err) } @@ -156,7 +156,7 @@ func resourceClusterCapacityProvidersDelete(ctx context.Context, d *schema.Resou return diag.Errorf("error deleting ECS Cluster (%s) capacity providers: %s", d.Id(), err) } - if _, err := waitClusterAvailable(conn, d.Id()); err != nil { + if _, err := waitClusterAvailable(ctx, conn, d.Id()); err != nil { return diag.Errorf("error waiting for ECS Cluster (%s) to become available: %s", d.Id(), err) } diff --git a/internal/service/ecs/status.go b/internal/service/ecs/status.go index ae9dd2c21fd..030a87e6d49 100644 --- a/internal/service/ecs/status.go +++ b/internal/service/ecs/status.go @@ -1,6 +1,7 @@ package ecs import ( + "context" "log" "github.com/aws/aws-sdk-go/aws" @@ -85,11 +86,11 @@ func statusService(conn *ecs.ECS, id, cluster string) resource.StateRefreshFunc } } -func statusCluster(conn *ecs.ECS, arn string) resource.StateRefreshFunc { +func statusCluster(ctx context.Context, conn *ecs.ECS, arn string) resource.StateRefreshFunc { return func() (interface{}, string, error) { - output, err := FindClusterByNameOrARN(conn, arn) + cluster, err := FindClusterByNameOrARN(ctx, conn, arn) - if tfawserr.ErrCodeEquals(err, ecs.ErrCodeClusterNotFoundException) { + if tfresource.NotFound(err) { return nil, clusterStatusNone, nil } @@ -97,11 +98,7 @@ func statusCluster(conn *ecs.ECS, arn string) resource.StateRefreshFunc { return nil, clusterStatusError, err } - if len(output.Clusters) == 0 { - return nil, clusterStatusNone, nil - } - - return output, aws.StringValue(output.Clusters[0].Status), err + return cluster, aws.StringValue(cluster.Status), err } } diff --git a/internal/service/ecs/wait.go b/internal/service/ecs/wait.go index 1ede62d9374..9f2821af388 100644 --- a/internal/service/ecs/wait.go +++ b/internal/service/ecs/wait.go @@ -1,6 +1,7 @@ package ecs import ( + "context" "time" "github.com/aws/aws-sdk-go/aws" @@ -122,16 +123,16 @@ func waitServiceDescribeReady(conn *ecs.ECS, id, cluster string) (*ecs.DescribeS return nil, err } -func waitClusterAvailable(conn *ecs.ECS, arn string) (*ecs.Cluster, error) { //nolint:unparam +func waitClusterAvailable(ctx context.Context, conn *ecs.ECS, arn string) (*ecs.Cluster, error) { //nolint:unparam stateConf := &resource.StateChangeConf{ Pending: []string{"PROVISIONING"}, Target: []string{"ACTIVE"}, - Refresh: statusCluster(conn, arn), + Refresh: statusCluster(ctx, conn, arn), Timeout: clusterAvailableTimeout, Delay: clusterAvailableDelay, } - outputRaw, err := stateConf.WaitForState() + outputRaw, err := stateConf.WaitForStateContext(ctx) if v, ok := outputRaw.(*ecs.Cluster); ok { return v, err @@ -144,7 +145,7 @@ func waitClusterDeleted(conn *ecs.ECS, arn string) (*ecs.Cluster, error) { stateConf := &resource.StateChangeConf{ Pending: []string{"ACTIVE", "DEPROVISIONING"}, Target: []string{"INACTIVE"}, - Refresh: statusCluster(conn, arn), + Refresh: statusCluster(context.Background(), conn, arn), Timeout: clusterDeleteTimeout, } From 1812535e5de9bd45cdfa5d210357ea806ee224b8 Mon Sep 17 00:00:00 2001 From: Roberth Kulbin <6707630+roberth-k@users.noreply.github.com> Date: Thu, 20 Jan 2022 01:15:11 +0000 Subject: [PATCH 5/8] add changelog entry for #22672 --- .changelog/22672.txt | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changelog/22672.txt diff --git a/.changelog/22672.txt b/.changelog/22672.txt new file mode 100644 index 00000000000..e22b61a6371 --- /dev/null +++ b/.changelog/22672.txt @@ -0,0 +1,3 @@ +```release-note:new-resource +aws_ecs_cluster_capacity_providers +``` From 5f2b92e533dfbdabf1dc32a7843cb428730d022c Mon Sep 17 00:00:00 2001 From: Roberth Kulbin <6707630+roberth-k@users.noreply.github.com> Date: Wed, 26 Jan 2022 15:14:38 +0000 Subject: [PATCH 6/8] r/aws_ecs_cluster_capacity_providers: add test for #11409 --- .../ecs/cluster_capacity_providers_test.go | 210 ++++++++++++++++++ 1 file changed, 210 insertions(+) diff --git a/internal/service/ecs/cluster_capacity_providers_test.go b/internal/service/ecs/cluster_capacity_providers_test.go index 91412ae1d96..0931ee681b5 100644 --- a/internal/service/ecs/cluster_capacity_providers_test.go +++ b/internal/service/ecs/cluster_capacity_providers_test.go @@ -4,9 +4,11 @@ import ( "fmt" "testing" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ecs" sdkacctest "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/hashicorp/terraform-provider-aws/internal/acctest" tfecs "github.com/hashicorp/terraform-provider-aws/internal/service/ecs" ) @@ -98,6 +100,42 @@ func TestAccECSClusterCapacityProviders_defaults(t *testing.T) { }) } +func TestAccECSClusterCapacityProviders_destroy(t *testing.T) { + // This test proves that https://github.com/hashicorp/terraform-provider-aws/issues/11409 + // has been addressed by aws_ecs_cluster_capacity_providers. + // + // If we were configuring capacity providers directly on the cluster, the + // test would fail with a timeout error. + + var cluster ecs.Cluster + rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acctest.PreCheck(t) }, + ErrorCheck: acctest.ErrorCheck(t, ecs.EndpointsID), + Providers: acctest.Providers, + CheckDestroy: testAccCheckClusterDestroy, + Steps: []resource.TestStep{ + { + Config: testAccClusterCapacityProvidersConfig_destroy_before(rName), + Check: resource.ComposeTestCheckFunc( + testAccCheckClusterExists("aws_ecs_cluster.test", &cluster), + func(s *terraform.State) error { + if aws.Int64Value(cluster.RegisteredContainerInstancesCount) != 2 { + return fmt.Errorf("expected the cluster to have 2 registered container instances") + } + + return nil + }, + ), + }, + { + Config: testAccClusterCapacityProvidersConfig_destroy_after(rName), + }, + }, + }) +} + func TestAccECSClusterCapacityProviders_update_capacityProviders(t *testing.T) { var cluster ecs.Cluster rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) @@ -410,3 +448,175 @@ resource "aws_ecs_cluster_capacity_providers" "test" { } `, rName) } + +func testAccClusterCapacityProvidersConfig_destroy_before(rName string) string { + return fmt.Sprintf(` +data "aws_ami" "test" { + most_recent = true + owners = ["amazon"] + + filter { + name = "name" + values = ["amzn2-ami-ecs-hvm-2.0.*-x86_64-ebs"] + } +} + +resource "aws_vpc" "test" { + cidr_block = "10.0.0.0/16" + + tags = { + Name = %[1]q + } +} + +resource "aws_subnet" "test" { + vpc_id = aws_vpc.test.id + cidr_block = "10.0.0.0/24" + map_public_ip_on_launch = true + + tags = { + Name = %[1]q + } +} + +resource "aws_route_table" "test" { + vpc_id = aws_vpc.test.id + + route { + cidr_block = "0.0.0.0/0" + gateway_id = aws_internet_gateway.test.id + } +} + +resource "aws_internet_gateway" "test" { + vpc_id = aws_vpc.test.id +} + +resource "aws_route_table_association" "test" { + route_table_id = aws_route_table.test.id + subnet_id = aws_subnet.test.id +} + +resource "aws_security_group" "test" { + vpc_id = aws_vpc.test.id + + egress { + from_port = 0 + to_port = 0 + protocol = "-1" + cidr_blocks = ["0.0.0.0/0"] + } + + ingress { + from_port = 0 + to_port = 0 + protocol = "-1" + cidr_blocks = ["0.0.0.0/0"] + } + + tags = { + Name = %[1]q + } +} + +resource "aws_ecs_cluster" "test" { + name = %[1]q +} + +resource "aws_ecs_capacity_provider" "test" { + name = %[1]q + + auto_scaling_group_provider { + auto_scaling_group_arn = aws_autoscaling_group.test.arn + } +} + +resource "aws_ecs_cluster_capacity_providers" "test" { + cluster_name = aws_ecs_cluster.test.name + capacity_providers = [aws_ecs_capacity_provider.test.name] + + default_capacity_provider_strategy { + capacity_provider = aws_ecs_capacity_provider.test.name + } +} + +resource "aws_iam_role" "test" { + name = %[1]q + + assume_role_policy = jsonencode({ + Version = "2012-10-17" + Statement = { + Effect = "Allow" + Principal = { + Service = "ec2.amazonaws.com" + } + Action = "sts:AssumeRole" + } + }) +} + +data "aws_partition" "test" {} + +resource "aws_iam_role_policy_attachment" "test" { + policy_arn = "arn:${data.aws_partition.test.partition}:iam::aws:policy/service-role/AmazonEC2ContainerServiceforEC2Role" + role = aws_iam_role.test.id +} + +resource "aws_iam_instance_profile" "test" { + depends_on = [aws_iam_role_policy_attachment.test] + role = aws_iam_role.test.name +} + +resource "aws_launch_template" "test" { + image_id = data.aws_ami.test.id + instance_type = "t3.micro" + instance_initiated_shutdown_behavior = "terminate" + vpc_security_group_ids = [aws_security_group.test.id] + + iam_instance_profile { + name = aws_iam_instance_profile.test.name + } + + user_data = base64encode(<> /etc/ecs/ecs.config +EOL + ) +} + +resource "aws_autoscaling_group" "test" { + desired_capacity = 2 + max_size = 4 + min_size = 2 + name = %[1]q + vpc_zone_identifier = [aws_subnet.test.id] + + instance_refresh { + strategy = "Rolling" + } + + launch_template { + id = aws_launch_template.test.id + version = aws_launch_template.test.latest_version + } + + tag { + key = "AmazonECSManaged" + value = "" + propagate_at_launch = true + } +} +`, rName) +} + +func testAccClusterCapacityProvidersConfig_destroy_after(rName string) string { + return fmt.Sprintf(` +resource "aws_vpc" "test" { + cidr_block = "10.0.0.0/16" + + tags = { + Name = %[1]q + } +} +`, rName) +} From f4c79371abab74894a034a8e2151bd61f9ed055f Mon Sep 17 00:00:00 2001 From: Roberth Kulbin <6707630+roberth-k@users.noreply.github.com> Date: Wed, 26 Jan 2022 15:25:58 +0000 Subject: [PATCH 7/8] address review comments of #22672 --- internal/service/ecs/cluster.go | 9 +++------ internal/service/ecs/cluster_capacity_providers_test.go | 4 ++-- internal/service/ecs/wait.go | 1 + website/docs/r/ecs_cluster.html.markdown | 2 +- .../docs/r/ecs_cluster_capacity_providers.html.markdown | 6 +++--- 5 files changed, 10 insertions(+), 12 deletions(-) diff --git a/internal/service/ecs/cluster.go b/internal/service/ecs/cluster.go index 495087352be..e5c724d218e 100644 --- a/internal/service/ecs/cluster.go +++ b/internal/service/ecs/cluster.go @@ -251,15 +251,12 @@ func resourceClusterRead(d *schema.ResourceData, meta interface{}) error { ignoreTagsConfig := meta.(*conns.AWSClient).IgnoreTagsConfig var cluster *ecs.Cluster - err := resource.Retry(2*time.Minute, func() *resource.RetryError { + err := resource.Retry(clusterReadTimeout, func() *resource.RetryError { var err error cluster, err = FindClusterByNameOrARN(context.Background(), conn, d.Id()) - if tfresource.NotFound(err) { - if d.IsNewResource() { - return resource.RetryableError(err) - } - return resource.NonRetryableError(err) + if d.IsNewResource() && tfresource.NotFound(err) { + return resource.RetryableError(err) } if err != nil { diff --git a/internal/service/ecs/cluster_capacity_providers_test.go b/internal/service/ecs/cluster_capacity_providers_test.go index 0931ee681b5..446c20559df 100644 --- a/internal/service/ecs/cluster_capacity_providers_test.go +++ b/internal/service/ecs/cluster_capacity_providers_test.go @@ -136,7 +136,7 @@ func TestAccECSClusterCapacityProviders_destroy(t *testing.T) { }) } -func TestAccECSClusterCapacityProviders_update_capacityProviders(t *testing.T) { +func TestAccECSClusterCapacityProviders_Update_capacityProviders(t *testing.T) { var cluster ecs.Cluster rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_ecs_cluster_capacity_providers.test" @@ -203,7 +203,7 @@ func TestAccECSClusterCapacityProviders_update_capacityProviders(t *testing.T) { }) } -func TestAccECSClusterCapacityProviders_update_defaultCapacityProviderStrategy(t *testing.T) { +func TestAccECSClusterCapacityProviders_Update_defaultStrategy(t *testing.T) { var cluster ecs.Cluster rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix) resourceName := "aws_ecs_cluster_capacity_providers.test" diff --git a/internal/service/ecs/wait.go b/internal/service/ecs/wait.go index 9f2821af388..d4bc3322829 100644 --- a/internal/service/ecs/wait.go +++ b/internal/service/ecs/wait.go @@ -22,6 +22,7 @@ const ( clusterAvailableTimeout = 10 * time.Minute clusterDeleteTimeout = 10 * time.Minute clusterAvailableDelay = 10 * time.Second + clusterReadTimeout = 2 * time.Second taskSetCreateTimeout = 10 * time.Minute taskSetDeleteTimeout = 10 * time.Minute diff --git a/website/docs/r/ecs_cluster.html.markdown b/website/docs/r/ecs_cluster.html.markdown index 73670ca2e33..82830183fca 100644 --- a/website/docs/r/ecs_cluster.html.markdown +++ b/website/docs/r/ecs_cluster.html.markdown @@ -10,7 +10,7 @@ description: |- Provides an ECS cluster. -~> **NOTE on Clusters and Cluster Capacity Providers:** Terraform provides both a standalone [`aws_ecs_cluster_capacity_providers`](ecs_cluster_capacity_providers.html) resource, as well as allowing the capacity providers and default strategies to be managed in-line by the [`aws_ecs_cluster`](ecs_cluster.html) resource. You cannot use a Cluster with in-line capacity providers in conjunction with the Capacity Providers resource, as doing so will cause a conflict and will lead to mutual overwrites. +~> **NOTE on Clusters and Cluster Capacity Providers:** Terraform provides both a standalone [`aws_ecs_cluster_capacity_providers`](/docs/providers/aws/r/ecs_cluster_capacity_providers.html) resource, as well as allowing the capacity providers and default strategies to be managed in-line by the `aws_ecs_cluster` resource. You cannot use a Cluster with in-line capacity providers in conjunction with the Capacity Providers resource, nor use more than one Capacity Providers resource with a single Cluster, as doing so will cause a conflict and will lead to mutual overwrites. ## Example Usage diff --git a/website/docs/r/ecs_cluster_capacity_providers.html.markdown b/website/docs/r/ecs_cluster_capacity_providers.html.markdown index 5afee04985e..85d91459215 100644 --- a/website/docs/r/ecs_cluster_capacity_providers.html.markdown +++ b/website/docs/r/ecs_cluster_capacity_providers.html.markdown @@ -12,7 +12,7 @@ Manages the capacity providers of an ECS Cluster. More information about capacity providers can be found in the [ECS User Guide](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/cluster-capacity-providers.html). -~> **NOTE on Clusters and Cluster Capacity Providers:** Terraform provides both a standalone [`aws_ecs_cluster_capacity_providers`](ecs_cluster_capacity_providers.html) resource, as well as allowing the capacity providers and default strategies to be managed in-line by the [`aws_ecs_cluster`](ecs_cluster.html) resource. You cannot use a Cluster with in-line capacity providers in conjunction with the Capacity Providers resource, as doing so will cause a conflict and will lead to mutual overwrites. +~> **NOTE on Clusters and Cluster Capacity Providers:** Terraform provides both a standalone `aws_ecs_cluster_capacity_providers` resource, as well as allowing the capacity providers and default strategies to be managed in-line by the [`aws_ecs_cluster`](/docs/providers/aws/r/ecs_cluster.html) resource. You cannot use a Cluster with in-line capacity providers in conjunction with the Capacity Providers resource, nor use more than one Capacity Providers resource with a single Cluster, as doing so will cause a conflict and will lead to mutual overwrites. ## Example Usage @@ -21,8 +21,8 @@ resource "aws_ecs_cluster" "example" { name = "my-cluster" } -resource "aws_ecs_cluster_capacity_providers" "test" { - cluster_name = aws_ecs_cluster.test.name +resource "aws_ecs_cluster_capacity_providers" "example" { + cluster_name = aws_ecs_cluster.example.name capacity_providers = ["FARGATE"] From fac6b6f7d15f81de2e708b7eb02b9fb8af1aac5b Mon Sep 17 00:00:00 2001 From: Dirk Avery Date: Wed, 26 Jan 2022 13:14:22 -0500 Subject: [PATCH 8/8] Add to changelog --- .changelog/22672.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.changelog/22672.txt b/.changelog/22672.txt index e22b61a6371..56f5d160116 100644 --- a/.changelog/22672.txt +++ b/.changelog/22672.txt @@ -1,3 +1,7 @@ ```release-note:new-resource aws_ecs_cluster_capacity_providers ``` + +```release-note:bug +resource/aws_ecs_cluster: Provide new resource `aws_ecs_cluster_capacity_providers` to avoid bugs using `capacity_providers` and `default_capacity_provider_strategy`, which arguments will be deprecated in a future version +``` \ No newline at end of file