Merge pull request #24223 from hashicorp/issue-20452

r/ecs_service: add wait condition after create/update and handle errors when using `wait_for_steady_state`

.changelog/24223.txt

@@ -0,0 +1,7 @@
+```release-note:bug
+resource/aws_ecs_service: Retry when using the `wait_for_steady_state` parameter and `ResourceNotReady` errors are returned from the AWS API
+```
+
+```release-note:bug
+resource/aws_ecs_service: Wait for service to reach an active state after create and update operations
+```

internal/conns/conns.go

@@ -1221,6 +1221,26 @@ func (c *Config) Client(ctx context.Context) (interface{}, diag.Diagnostics) {
 		}
 	})
 
+	client.ECSConn.Handlers.Retry.PushBack(func(r *request.Request) {
+		// By design the "WaitUntilServicesStable" method will poll every 15 seconds until a successful state
+		// has been reached. This will exit with a return code of 255 (ResourceNotReady) after 40 failed checks.
+		// Thus, here we retry the operation a set number of times as
+		// described in https://github.com/hashicorp/terraform-provider-aws/pull/23747.
+		if r.Operation.Name == "WaitUntilServicesStable" {
+			if tfawserr.ErrCodeEquals(r.Error, "ResourceNotReady") {
+				// We only want to retry briefly as the default max retry count would
+				// excessively retry when the error could be legitimate.
+				// We currently depend on the DefaultRetryer exponential backoff here.
+				// ~10 retries gives a fair backoff of a few seconds.
+				if r.RetryCount < 9 {
+					r.Retryable = aws.Bool(true)
+				} else {
+					r.Retryable = aws.Bool(false)
+				}
+			}
+		}
+	})
+
 	client.FMSConn.Handlers.Retry.PushBack(func(r *request.Request) {
 		// Acceptance testing creates and deletes resources in quick succession.
 		// The FMS onboarding process into Organizations is opaque to consumers.

internal/service/ecs/service.go

@@ -550,14 +550,15 @@ func resourceServiceCreate(d *schema.ResourceData, meta interface{}) error {
 	log.Printf("[DEBUG] ECS service created: %s", aws.StringValue(output.Service.ServiceArn))
 	d.SetId(aws.StringValue(output.Service.ServiceArn))
 
-	if d.Get("wait_for_steady_state").(bool) {
-		cluster := ""
-		if v, ok := d.GetOk("cluster"); ok {
-			cluster = v.(string)
-		}
+	cluster := d.Get("cluster").(string)
 
+	if d.Get("wait_for_steady_state").(bool) {
 		if err := waitServiceStable(conn, d.Id(), cluster); err != nil {
-			return fmt.Errorf("error waiting for ECS service (%s) to become ready: %w", d.Id(), err)
+			return fmt.Errorf("error waiting for ECS service (%s) to reach steady state after creation: %w", d.Id(), err)
+		}
+	} else {
+		if _, err := waitServiceDescribeReady(conn, d.Id(), cluster); err != nil {
+			return fmt.Errorf("error waiting for ECS service (%s) to become active after creation: %w", d.Id(), err)
 		}
 	}
 
@@ -608,6 +609,7 @@ func resourceServiceRead(d *schema.ResourceData, meta interface{}) error {
 	}
 
 	if err != nil {
+		log.Printf("[DEBUG] Waiting for ECS Service (%s) to become active", d.Id())
 		output, err = waitServiceDescribeReady(conn, d.Id(), d.Get("cluster").(string))
 	}
 
@@ -618,7 +620,7 @@ func resourceServiceRead(d *schema.ResourceData, meta interface{}) error {
 	}
 
 	if err != nil {
-		return fmt.Errorf("error reading ECS service: %w", err)
+		return fmt.Errorf("error reading ECS service (%s): %w", d.Id(), err)
 	}
 
 	if len(output.Services) < 1 {
@@ -633,7 +635,7 @@ func resourceServiceRead(d *schema.ResourceData, meta interface{}) error {
 	service := output.Services[0]
 
 	// Status==INACTIVE means deleted service
-	if aws.StringValue(service.Status) == "INACTIVE" {
+	if aws.StringValue(service.Status) == serviceStatusInactive {
 		log.Printf("[WARN] Removing ECS service %q because it's INACTIVE", aws.StringValue(service.ServiceArn))
 		d.SetId("")
 		return nil
@@ -1110,14 +1112,14 @@ func resourceServiceUpdate(d *schema.ResourceData, meta interface{}) error {
 			return fmt.Errorf("error updating ECS Service (%s): %w", d.Id(), err)
 		}
 
+		cluster := d.Get("cluster").(string)
 		if d.Get("wait_for_steady_state").(bool) {
-			cluster := ""
-			if v, ok := d.GetOk("cluster"); ok {
-				cluster = v.(string)
-			}
-
 			if err := waitServiceStable(conn, d.Id(), cluster); err != nil {
-				return fmt.Errorf("error waiting for ECS service (%s) to become ready: %w", d.Id(), err)
+				return fmt.Errorf("error waiting for ECS service (%s) to reach steady state after update: %w", d.Id(), err)
+			}
+		} else {
+			if _, err := waitServiceDescribeReady(conn, d.Id(), cluster); err != nil {
+				return fmt.Errorf("error waiting for ECS service (%s) to become active after update: %w", d.Id(), err)
 			}
 		}
 	}