Skip to content

Commit

Permalink
Merge pull request #31683 from hashicorp/b-aws_ecs_service.alarms-crash
Browse files Browse the repository at this point in the history 
r/aws_ecs_service: Fix crash when just `alarms` is updated
  • Loading branch information
@ewbankkit
ewbankkit authored May 31, 2023
2 parents 08966f7 + 8ab138b commit 5e430e4
Show file tree
Hide file tree
Showing 3 changed files with 150 additions and 58 deletions.
3 changes: 3 additions & 0 deletions .changelog/31683.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_ecs_service: Fix crash when just `alarms` is updated
```
112 changes: 59 additions & 53 deletions internal/service/ecs/service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -785,28 +785,20 @@ func resourceServiceUpdate(ctx context.Context, d *schema.ResourceData, meta int
Service: aws.String(d.Id()),
}

schedulingStrategy := d.Get("scheduling_strategy").(string)

switch schedulingStrategy {
case ecs.SchedulingStrategyDaemon:
if d.HasChange("deployment_minimum_healthy_percent") {
input.DeploymentConfiguration = &ecs.DeploymentConfiguration{
MinimumHealthyPercent: aws.Int64(int64(d.Get("deployment_minimum_healthy_percent").(int))),
}
}
case ecs.SchedulingStrategyReplica:
if d.HasChange("desired_count") {
input.DesiredCount = aws.Int64(int64(d.Get("desired_count").(int)))
if d.HasChange("alarms") {
if input.DeploymentConfiguration == nil {
input.DeploymentConfiguration = &ecs.DeploymentConfiguration{}
}

if d.HasChanges("deployment_maximum_percent", "deployment_minimum_healthy_percent") {
input.DeploymentConfiguration = &ecs.DeploymentConfiguration{
MaximumPercent: aws.Int64(int64(d.Get("deployment_maximum_percent").(int))),
MinimumHealthyPercent: aws.Int64(int64(d.Get("deployment_minimum_healthy_percent").(int))),
}
if v, ok := d.GetOk("alarms"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil {
input.DeploymentConfiguration.Alarms = expandAlarms(v.([]interface{})[0].(map[string]interface{}))
}
}

if d.HasChange("capacity_provider_strategy") {
input.CapacityProviderStrategy = expandCapacityProviderStrategy(d.Get("capacity_provider_strategy").(*schema.Set))
}

if d.HasChange("deployment_circuit_breaker") {
if input.DeploymentConfiguration == nil {
input.DeploymentConfiguration = &ecs.DeploymentConfiguration{}
Expand All @@ -820,6 +812,52 @@ func resourceServiceUpdate(ctx context.Context, d *schema.ResourceData, meta int
}
}

switch schedulingStrategy := d.Get("scheduling_strategy").(string); schedulingStrategy {
case ecs.SchedulingStrategyDaemon:
if d.HasChange("deployment_minimum_healthy_percent") {
if input.DeploymentConfiguration == nil {
input.DeploymentConfiguration = &ecs.DeploymentConfiguration{}
}

input.DeploymentConfiguration.MinimumHealthyPercent = aws.Int64(int64(d.Get("deployment_minimum_healthy_percent").(int)))
}
case ecs.SchedulingStrategyReplica:
if d.HasChanges("deployment_maximum_percent", "deployment_minimum_healthy_percent") {
if input.DeploymentConfiguration == nil {
input.DeploymentConfiguration = &ecs.DeploymentConfiguration{}
}

input.DeploymentConfiguration.MaximumPercent = aws.Int64(int64(d.Get("deployment_maximum_percent").(int)))
input.DeploymentConfiguration.MinimumHealthyPercent = aws.Int64(int64(d.Get("deployment_minimum_healthy_percent").(int)))
}

if d.HasChange("desired_count") {
input.DesiredCount = aws.Int64(int64(d.Get("desired_count").(int)))
}
}

if d.HasChange("enable_ecs_managed_tags") {
input.EnableECSManagedTags = aws.Bool(d.Get("enable_ecs_managed_tags").(bool))
}

if d.HasChange("enable_execute_command") {
input.EnableExecuteCommand = aws.Bool(d.Get("enable_execute_command").(bool))
}

if d.HasChange("health_check_grace_period_seconds") {
input.HealthCheckGracePeriodSeconds = aws.Int64(int64(d.Get("health_check_grace_period_seconds").(int)))
}

if d.HasChange("load_balancer") {
if v, ok := d.Get("load_balancer").(*schema.Set); ok && v != nil {
input.LoadBalancers = expandLoadBalancers(v.List())
}
}

if d.HasChange("network_configuration") {
input.NetworkConfiguration = expandNetworkConfiguration(d.Get("network_configuration").([]interface{}))
}

if d.HasChange("ordered_placement_strategy") {
// Reference: https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_UpdateService.html#ECS-UpdateService-request-placementStrategy
// To remove an existing placement strategy, specify an empty object.
Expand Down Expand Up @@ -852,46 +890,10 @@ func resourceServiceUpdate(ctx context.Context, d *schema.ResourceData, meta int
}
}

if d.HasChange("alarms") {
if v, ok := d.GetOk("alarms"); ok && len(v.([]interface{})) > 0 && v.([]interface{})[0] != nil {
input.DeploymentConfiguration.Alarms = expandAlarms(v.([]interface{})[0].(map[string]interface{}))
}
}

if d.HasChange("platform_version") {
input.PlatformVersion = aws.String(d.Get("platform_version").(string))
}

if d.HasChange("health_check_grace_period_seconds") {
input.HealthCheckGracePeriodSeconds = aws.Int64(int64(d.Get("health_check_grace_period_seconds").(int)))
}

if d.HasChange("task_definition") {
input.TaskDefinition = aws.String(d.Get("task_definition").(string))
}

if d.HasChange("network_configuration") {
input.NetworkConfiguration = expandNetworkConfiguration(d.Get("network_configuration").([]interface{}))
}

if d.HasChange("capacity_provider_strategy") {
input.CapacityProviderStrategy = expandCapacityProviderStrategy(d.Get("capacity_provider_strategy").(*schema.Set))
}

if d.HasChange("enable_execute_command") {
input.EnableExecuteCommand = aws.Bool(d.Get("enable_execute_command").(bool))
}

if d.HasChange("enable_ecs_managed_tags") {
input.EnableECSManagedTags = aws.Bool(d.Get("enable_ecs_managed_tags").(bool))
}

if d.HasChange("load_balancer") {
if v, ok := d.Get("load_balancer").(*schema.Set); ok && v != nil {
input.LoadBalancers = expandLoadBalancers(v.List())
}
}

if d.HasChange("propagate_tags") {
input.PropagateTags = aws.String(d.Get("propagate_tags").(string))
}
Expand All @@ -904,6 +906,10 @@ func resourceServiceUpdate(ctx context.Context, d *schema.ResourceData, meta int
input.ServiceRegistries = expandServiceRegistries(d.Get("service_registries").([]interface{}))
}

if d.HasChange("task_definition") {
input.TaskDefinition = aws.String(d.Get("task_definition").(string))
}

// Retry due to IAM eventual consistency
err := retry.RetryContext(ctx, propagationTimeout+serviceUpdateTimeout, func() *retry.RetryError {
_, err := conn.UpdateServiceWithContext(ctx, input)
Expand Down
93 changes: 88 additions & 5 deletions internal/service/ecs/service_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ func TestAccECSService_DeploymentControllerType_external(t *testing.T) {
})
}

func TestAccECSService_Alarms(t *testing.T) {
func TestAccECSService_alarmsAdd(t *testing.T) {
ctx := acctest.Context(t)
var service ecs.Service
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
Expand All @@ -524,10 +524,50 @@ func TestAccECSService_Alarms(t *testing.T) {
CheckDestroy: testAccCheckServiceDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccServiceConfig_alarms(rName),
Config: testAccServiceConfig_noAlarms(rName),
Check: resource.ComposeTestCheckFunc(
testAccCheckServiceExists(ctx, resourceName, &service),
resource.TestCheckResourceAttr(resourceName, "alarms.#", "0"),
),
},
{
Config: testAccServiceConfig_alarms(rName, true),
Check: resource.ComposeTestCheckFunc(
testAccCheckServiceExists(ctx, resourceName, &service),
resource.TestCheckResourceAttr(resourceName, "alarms.#", "1"),
resource.TestCheckResourceAttr(resourceName, "alarms.0.enable", "true"),
),
},
},
})
}

func TestAccECSService_alarmsUpdate(t *testing.T) {
ctx := acctest.Context(t)
var service ecs.Service
rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
resourceName := "aws_ecs_service.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { acctest.PreCheck(ctx, t) },
ErrorCheck: acctest.ErrorCheck(t, ecs.EndpointsID),
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
CheckDestroy: testAccCheckServiceDestroy(ctx),
Steps: []resource.TestStep{
{
Config: testAccServiceConfig_alarms(rName, true),
Check: resource.ComposeTestCheckFunc(
testAccCheckServiceExists(ctx, resourceName, &service),
resource.TestCheckResourceAttr(resourceName, "alarms.#", "1"),
resource.TestCheckResourceAttr(resourceName, "alarms.0.enable", "true"),
),
},
{
Config: testAccServiceConfig_alarms(rName, false),
Check: resource.ComposeTestCheckFunc(
testAccCheckServiceExists(ctx, resourceName, &service),
resource.TestCheckResourceAttr(resourceName, "alarms.#", "1"),
resource.TestCheckResourceAttr(resourceName, "alarms.0.enable", "false"),
),
},
},
Expand Down Expand Up @@ -2542,7 +2582,7 @@ resource "aws_ecs_service" "test" {
`, rName))
}

func testAccServiceConfig_alarms(rName string) string {
func testAccServiceConfig_alarms(rName string, enable bool) string {
return fmt.Sprintf(`
resource "aws_ecs_cluster" "test" {
name = %[1]q
Expand Down Expand Up @@ -2571,14 +2611,57 @@ resource "aws_ecs_service" "test" {
desired_count = 1
alarms {
enable = true
rollback = true
enable = %[2]t
rollback = %[2]t
alarm_names = [
aws_cloudwatch_metric_alarm.test.alarm_name
]
}
}
resource "aws_cloudwatch_metric_alarm" "test" {
alarm_name = %[1]q
comparison_operator = "GreaterThanOrEqualToThreshold"
evaluation_periods = "2"
metric_name = "CPUReservation"
namespace = "AWS/ECS"
period = "120"
statistic = "Average"
threshold = "80"
insufficient_data_actions = []
}
`, rName, enable)
}

func testAccServiceConfig_noAlarms(rName string) string {
return fmt.Sprintf(`
resource "aws_ecs_cluster" "test" {
name = %[1]q
}
resource "aws_ecs_task_definition" "test" {
family = %[1]q
container_definitions = <<DEFINITION
[
{
"cpu": 128,
"essential": true,
"image": "mongo:latest",
"memory": 128,
"name": "mongodb"
}
]
DEFINITION
}
resource "aws_ecs_service" "test" {
name = %[1]q
cluster = aws_ecs_cluster.test.id
task_definition = aws_ecs_task_definition.test.arn
desired_count = 1
}
resource "aws_cloudwatch_metric_alarm" "test" {
alarm_name = %[1]q
comparison_operator = "GreaterThanOrEqualToThreshold"
Expand Down

0 comments on commit 5e430e4

Please sign in to comment.