Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resource/ecs_cluster: Add ability to enable ECS Cluster Insights #9720

Merged
merged 1 commit into from
Sep 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions aws/data_source_aws_ecs_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,23 @@ func dataSourceAwsEcsCluster() *schema.Resource {
Type: schema.TypeInt,
Computed: true,
},

"setting": {
Type: schema.TypeSet,
Computed: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"name": {
Type: schema.TypeString,
Computed: true,
},
"value": {
Type: schema.TypeString,
Computed: true,
},
},
},
},
},
}
}
Expand Down Expand Up @@ -77,5 +94,9 @@ func dataSourceAwsEcsClusterRead(d *schema.ResourceData, meta interface{}) error
d.Set("running_tasks_count", cluster.RunningTasksCount)
d.Set("registered_container_instances_count", cluster.RegisteredContainerInstancesCount)

if err := d.Set("setting", flattenEcsSettings(cluster.Settings)); err != nil {
return fmt.Errorf("error setting setting: %s", err)
}

return nil
}
59 changes: 59 additions & 0 deletions aws/data_source_aws_ecs_cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,28 @@ func TestAccAWSEcsDataSource_ecsCluster(t *testing.T) {
})
}

func TestAccAWSEcsDataSource_ecsClusterContainerInsights(t *testing.T) {
resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
Config: testAccCheckAwsEcsClusterDataSourceConfigContainerInsights,
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr("data.aws_ecs_cluster.default", "status", "ACTIVE"),
resource.TestCheckResourceAttr("data.aws_ecs_cluster.default", "pending_tasks_count", "0"),
resource.TestCheckResourceAttr("data.aws_ecs_cluster.default", "running_tasks_count", "0"),
resource.TestCheckResourceAttr("data.aws_ecs_cluster.default", "registered_container_instances_count", "0"),
resource.TestCheckResourceAttrSet("data.aws_ecs_cluster.default", "arn"),
resource.TestCheckResourceAttr("data.aws_ecs_cluster.default", "setting.#", "1"),
resource.TestCheckResourceAttr("data.aws_ecs_cluster.default", "setting.4047805881.name", "containerInsights"),
resource.TestCheckResourceAttr("data.aws_ecs_cluster.default", "setting.4047805881.value", "enabled"),
),
},
},
})
}

var testAccCheckAwsEcsClusterDataSourceConfig = fmt.Sprintf(`
resource "aws_ecs_cluster" "default" {
name = "default-%d"
Expand Down Expand Up @@ -59,3 +81,40 @@ data "aws_ecs_cluster" "default" {
cluster_name = "${aws_ecs_cluster.default.name}"
}
`, acctest.RandInt())

var testAccCheckAwsEcsClusterDataSourceConfigContainerInsights = fmt.Sprintf(`
resource "aws_ecs_cluster" "default" {
name = "default-%d"
setting {
name = "containerInsights"
value = "enabled"
}
}

resource "aws_ecs_task_definition" "mongo" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Since the aws_ecs_task_definition and aws_ecs_service resources are not necessary for testing the aws_ecs_cluster data source, we can omit them.

family = "mongodb"
container_definitions = <<DEFINITION
[
{
"cpu": 128,
"essential": true,
"image": "mongo:latest",
"memory": 128,
"memoryReservation": 64,
"name": "mongodb"
}
]
DEFINITION
}

resource "aws_ecs_service" "mongo" {
name = "mongodb"
cluster = "${aws_ecs_cluster.default.id}"
task_definition = "${aws_ecs_task_definition.mongo.arn}"
desired_count = 1
}

data "aws_ecs_cluster" "default" {
cluster_name = "${aws_ecs_cluster.default.name}"
}
`, acctest.RandInt())
85 changes: 83 additions & 2 deletions aws/resource_aws_ecs_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/aws/aws-sdk-go/service/ecs"
"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/helper/schema"
"github.com/hashicorp/terraform/helper/validation"
)

func resourceAwsEcsCluster() *schema.Resource {
Expand All @@ -33,6 +34,26 @@ func resourceAwsEcsCluster() *schema.Resource {
Type: schema.TypeString,
Computed: true,
},
"setting": {
Type: schema.TypeSet,
Optional: true,
Computed: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"name": {
Type: schema.TypeString,
Required: true,
ValidateFunc: validation.StringInSlice([]string{
ecs.ClusterSettingNameContainerInsights,
}, false),
},
"value": {
Type: schema.TypeString,
Required: true,
},
},
},
},
},
}
}
Expand All @@ -55,10 +76,16 @@ func resourceAwsEcsClusterCreate(d *schema.ResourceData, meta interface{}) error
clusterName := d.Get("name").(string)
log.Printf("[DEBUG] Creating ECS cluster %s", clusterName)

out, err := conn.CreateCluster(&ecs.CreateClusterInput{
input := ecs.CreateClusterInput{
ClusterName: aws.String(clusterName),
Tags: tagsFromMapECS(d.Get("tags").(map[string]interface{})),
})
}

if v, ok := d.GetOk("setting"); ok {
input.Settings = expandEcsSettings(v.(*schema.Set).List())
}

out, err := conn.CreateCluster(&input)
if err != nil {
return err
}
Expand Down Expand Up @@ -134,6 +161,10 @@ func resourceAwsEcsClusterRead(d *schema.ResourceData, meta interface{}) error {
d.Set("arn", cluster.ClusterArn)
d.Set("name", cluster.ClusterName)

if err := d.Set("setting", flattenEcsSettings(cluster.Settings)); err != nil {
return fmt.Errorf("error setting setting: %s", err)
}

if err := d.Set("tags", tagsToMapECS(cluster.Tags)); err != nil {
return fmt.Errorf("error setting tags: %s", err)
}
Expand All @@ -144,6 +175,18 @@ func resourceAwsEcsClusterRead(d *schema.ResourceData, meta interface{}) error {
func resourceAwsEcsClusterUpdate(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).ecsconn

if d.HasChange("setting") {
input := ecs.UpdateClusterSettingsInput{
Cluster: aws.String(d.Id()),
Settings: expandEcsSettings(d.Get("setting").(*schema.Set).List()),
}

_, err := conn.UpdateClusterSettings(&input)
if err != nil {
return fmt.Errorf("error changing ECS cluster settings (%s): %s", d.Id(), err)
}
}

if d.HasChange("tags") {
oldTagsRaw, newTagsRaw := d.GetChange("tags")
oldTagsMap := oldTagsRaw.(map[string]interface{})
Expand Down Expand Up @@ -260,3 +303,41 @@ func ecsClusterInactive(out *ecs.DescribeClustersOutput, clusterName string) boo
}
return false
}

func expandEcsSettings(configured []interface{}) []*ecs.ClusterSetting {
if len(configured) == 0 {
return nil
}

settings := make([]*ecs.ClusterSetting, 0, len(configured))

for _, raw := range configured {
data := raw.(map[string]interface{})

setting := &ecs.ClusterSetting{
Name: aws.String(data["name"].(string)),
Value: aws.String(data["value"].(string)),
}

settings = append(settings, setting)
}

return settings
}

func flattenEcsSettings(list []*ecs.ClusterSetting) []map[string]interface{} {
if len(list) == 0 {
return nil
}

result := make([]map[string]interface{}, 0, len(list))
for _, setting := range list {
l := map[string]interface{}{
"name": *setting.Name,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: To prevent potential panics (unlikely in this case, but a general pattern for consistency), we should use the AWS Go SDK conversion functions (e.g. aws.StringValue())

Suggested change
"name": *setting.Name,
"name": aws.StringValue(setting.Name),

"value": *setting.Value,
}

result = append(result, l)
}
return result
}
65 changes: 65 additions & 0 deletions aws/resource_aws_ecs_cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,47 @@ func TestAccAWSEcsCluster_Tags(t *testing.T) {
})
}

func TestAccAWSEcsCluster_containerInsights(t *testing.T) {
var cluster1 ecs.Cluster
rName := acctest.RandomWithPrefix("tf-acc-test")
resourceName := "aws_ecs_cluster.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSEcsClusterDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSEcsClusterConfig(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSEcsClusterExists(resourceName, &cluster1),
testAccCheckResourceAttrRegionalARN(resourceName, "arn", "ecs", fmt.Sprintf("cluster/%s", rName)),
),
},
{
Config: testAccAWSEcsClusterConfigContainerInsights(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSEcsClusterExists(resourceName, &cluster1),
testAccCheckResourceAttrRegionalARN(resourceName, "arn", "ecs", fmt.Sprintf("cluster/%s", rName)),
resource.TestCheckResourceAttr(resourceName, "name", rName),
resource.TestCheckResourceAttr(resourceName, "setting.#", "1"),
resource.TestCheckResourceAttr(resourceName, "setting.4047805881.name", "containerInsights"),
resource.TestCheckResourceAttr(resourceName, "setting.4047805881.value", "enabled"),
),
},
{
Config: testAccAWSEcsClusterConfigContainerInsightsDisable(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSEcsClusterExists(resourceName, &cluster1),
resource.TestCheckResourceAttr(resourceName, "setting.#", "1"),
resource.TestCheckResourceAttr(resourceName, "setting.1157067080.name", "containerInsights"),
resource.TestCheckResourceAttr(resourceName, "setting.1157067080.value", "disabled"),
),
},
},
})
}

func testAccCheckAWSEcsClusterDestroy(s *terraform.State) error {
conn := testAccProvider.Meta().(*AWSClient).ecsconn

Expand Down Expand Up @@ -262,3 +303,27 @@ resource "aws_ecs_cluster" "test" {
}
`, rName, tag1Key, tag1Value, tag2Key, tag2Value)
}

func testAccAWSEcsClusterConfigContainerInsights(rName string) string {
return fmt.Sprintf(`
resource "aws_ecs_cluster" "test" {
name = %q
setting {
name = "containerInsights"
value = "enabled"
}
}
`, rName)
}

func testAccAWSEcsClusterConfigContainerInsightsDisable(rName string) string {
return fmt.Sprintf(`
resource "aws_ecs_cluster" "test" {
name = %q
setting {
name = "containerInsights"
value = "disabled"
}
}
`, rName)
}
1 change: 1 addition & 0 deletions website/docs/d/ecs_cluster.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,4 @@ In addition to all arguments above, the following attributes are exported:
* `pending_tasks_count` - The number of pending tasks for the ECS Cluster
* `running_tasks_count` - The number of running tasks for the ECS Cluster
* `registered_container_instances_count` - The number of registered container instances for the ECS Cluster
* `setting` - The settings associated with the ECS Cluster.
8 changes: 8 additions & 0 deletions website/docs/r/ecs_cluster.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,15 @@ The following arguments are supported:

* `name` - (Required) The name of the cluster (up to 255 letters, numbers, hyphens, and underscores)
* `tags` - (Optional) Key-value mapping of resource tags
* `setting` - (Optional) The setting to use when creating a cluster. This parameter is used to enable CloudWatch Container Insights for a cluster. Defined below.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Prefer the wording of "configuration block" over "parameter" (which incorrectly may signal it can work as an argument with setting = ...)

Suggested change
* `setting` - (Optional) The setting to use when creating a cluster. This parameter is used to enable CloudWatch Container Insights for a cluster. Defined below.
* `setting` - (Optional) Configuration block(s) with cluster settings. For example, this can be used to enable CloudWatch Container Insights for a cluster. Defined below.


## setting

The `setting` configuration block supports the following:

* `name` - (Required) Name of the setting to manage. Valid values: `containerInsights`.
* `value` - (Required) The value to assign to the setting. Value values are `enabled` and `disabled`.

## Attributes Reference

In addition to all arguments above, the following attributes are exported:
Expand Down