Skip to content

Commit

Permalink
Merge pull request #25641 from hashicorp/ecs-service-timeout
Browse files Browse the repository at this point in the history 
resource/aws_ecs_service: Add customizable timeouts for Create and Update
  • Loading branch information
@gdavison
gdavison committed Jul 4, 2022
2 parents 3bc094e + d184c49 commit 9af6805
Show file tree
Hide file tree
Showing 7 changed files with 224 additions and 172 deletions.
11 changes: 11 additions & 0 deletions .changelog/25641.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
```release-note:bug
resource/aws_ecs_service: Fix error where tags are sometimes not retrieved.
```

```release-note:bug
resource/aws_ecs_service: Fix "unexpected new value" errors on creation.
```

```release-note:enhancement
resource/aws_ecs_service: Add configurable timeouts for Create and Delete.
```
102 changes: 102 additions & 0 deletions internal/service/ecs/find.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package ecs

import (
"context"
"fmt"
"log"

"github.com/aws/aws-sdk-go/aws"
Expand Down Expand Up @@ -98,3 +99,104 @@ func FindClusterByNameOrARN(ctx context.Context, conn *ecs.ECS, nameOrARN string

return output.Clusters[0], nil
}

func FindServiceByID(ctx context.Context, conn *ecs.ECS, id, cluster string) (*ecs.Service, error) {
input := &ecs.DescribeServicesInput{
Cluster: aws.String(cluster),
Include: aws.StringSlice([]string{ecs.ServiceFieldTags}),
Services: aws.StringSlice([]string{id}),
}

return FindService(ctx, conn, input)
}

func FindServiceNoTagsByID(ctx context.Context, conn *ecs.ECS, id, cluster string) (*ecs.Service, error) {
input := &ecs.DescribeServicesInput{
Cluster: aws.String(cluster),
Services: aws.StringSlice([]string{id}),
}

return FindService(ctx, conn, input)
}

type expectActiveError struct {
status string
}

func newExpectActiveError(status string) *expectActiveError {
return &expectActiveError{
status: status,
}
}

func (e *expectActiveError) Error() string {
return fmt.Sprintf("expected status %[1]q, was %[2]q", serviceStatusActive, e.status)
}

func FindServiceByIDWaitForActive(ctx context.Context, conn *ecs.ECS, id, cluster string) (*ecs.Service, error) {
var service *ecs.Service
// Use the resource.Retry function instead of WaitForState() because we don't want the timeout error, if any
err := resource.Retry(serviceDescribeTimeout, func() *resource.RetryError {
var err error
service, err = FindServiceByID(ctx, conn, id, cluster)
if tfresource.NotFound(err) {
return resource.RetryableError(err)
}
if err != nil {
return resource.NonRetryableError(err)
}

if status := aws.StringValue(service.Status); status != serviceStatusActive {
return resource.RetryableError(newExpectActiveError(status))
}

return nil
})
if tfresource.TimedOut(err) {
service, err = FindServiceByID(ctx, conn, id, cluster)
}

return service, err
}

func FindService(ctx context.Context, conn *ecs.ECS, input *ecs.DescribeServicesInput) (*ecs.Service, error) {
output, err := conn.DescribeServices(input)

if verify.CheckISOErrorTagsUnsupported(conn.PartitionID, err) && input.Include != nil {
id := aws.StringValueSlice(input.Services)[0]
log.Printf("[WARN] failed describing ECS Service (%s) with tags: %s; retrying without tags", id, err)

input.Include = nil
output, err = conn.DescribeServices(input)
}

// As of AWS SDK for Go v1.44.42, DescribeServices does not return the error code ecs.ErrCodeServiceNotFoundException
// Keep this here in case it ever does
if tfawserr.ErrCodeEquals(err, ecs.ErrCodeServiceNotFoundException) {
return nil, &resource.NotFoundError{
LastError: err,
LastRequest: input,
}
}
if err != nil {
return nil, err
}

// When an ECS Service is not found by DescribeServices(), it will return a Failure struct with Reason = "MISSING"
for _, v := range output.Failures {
if aws.StringValue(v.Reason) == "MISSING" {
return nil, &resource.NotFoundError{
LastRequest: input,
}
}
}

if len(output.Services) == 0 {
return nil, tfresource.NewEmptyResultError(input)
}
if n := len(output.Services); n > 1 {
return nil, tfresource.NewTooManyResultsError(n, input)
}

return output.Services[0], nil
}
114 changes: 36 additions & 78 deletions internal/service/ecs/service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package ecs
import (
"bytes"
"context"
"errors"
"fmt"
"log"
"math"
Expand Down Expand Up @@ -36,6 +37,8 @@ func ResourceService() *schema.Resource {
},

Timeouts: &schema.ResourceTimeout{
Create: schema.DefaultTimeout(20 * time.Minute),
Update: schema.DefaultTimeout(20 * time.Minute),
Delete: schema.DefaultTimeout(20 * time.Minute),
},

Expand Down Expand Up @@ -528,18 +531,18 @@ func resourceServiceCreate(d *schema.ResourceData, meta interface{}) error {

log.Printf("[DEBUG] Creating ECS Service: %s", input)

output, err := retryServiceCreate(conn, input)
output, err := serviceCreateWithRetry(conn, input)

// Some partitions (i.e., ISO) may not support tag-on-create
if input.Tags != nil && verify.CheckISOErrorTagsUnsupported(conn.PartitionID, err) {
log.Printf("[WARN] ECS tagging failed creating Service (%s) with tags: %s. Trying create without tags.", d.Get("name").(string), err)
log.Printf("[WARN] failed creating ECS Service (%s) with tags: %s. Trying create without tags.", d.Get("name").(string), err)
input.Tags = nil

output, err = retryServiceCreate(conn, input)
output, err = serviceCreateWithRetry(conn, input)
}

if err != nil {
return fmt.Errorf("failed creating ECS service (%s): %w", d.Get("name").(string), err)
return fmt.Errorf("error creating ECS service (%s): %w", d.Get("name").(string), err)
}

if output == nil || output.Service == nil {
Expand All @@ -552,11 +555,11 @@ func resourceServiceCreate(d *schema.ResourceData, meta interface{}) error {
cluster := d.Get("cluster").(string)

if d.Get("wait_for_steady_state").(bool) {
if err := waitServiceStable(conn, d.Id(), cluster); err != nil {
if _, err := waitServiceStable(conn, d.Id(), cluster, d.Timeout(schema.TimeoutCreate)); err != nil {
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 {
if _, err := waitServiceActive(conn, d.Id(), cluster, d.Timeout(schema.TimeoutCreate)); err != nil {
return fmt.Errorf("error waiting for ECS service (%s) to become active after creation: %w", d.Id(), err)
}
}
Expand All @@ -567,7 +570,7 @@ func resourceServiceCreate(d *schema.ResourceData, meta interface{}) error {

// If default tags only, log and continue. Otherwise, error.
if v, ok := d.GetOk("tags"); (!ok || len(v.(map[string]interface{})) == 0) && verify.CheckISOErrorTagsUnsupported(conn.PartitionID, err) {
log.Printf("[WARN] ECS tagging failed adding tags after create for Service (%s): %s", d.Id(), err)
log.Printf("[WARN] failed adding tags after create for ECS Service (%s): %s", d.Id(), err)
return resourceServiceRead(d, meta)
}

Expand All @@ -584,64 +587,33 @@ func resourceServiceRead(d *schema.ResourceData, meta interface{}) error {
defaultTagsConfig := meta.(*conns.AWSClient).DefaultTagsConfig
ignoreTagsConfig := meta.(*conns.AWSClient).IgnoreTagsConfig

log.Printf("[DEBUG] Reading ECS service %s", d.Id())
input := ecs.DescribeServicesInput{
Cluster: aws.String(d.Get("cluster").(string)),
Include: aws.StringSlice([]string{ecs.ServiceFieldTags}),
Services: aws.StringSlice([]string{d.Id()}),
}

output, err := conn.DescribeServices(&input)

// Some partitions (i.e., ISO) may not support tagging, giving error
if verify.CheckISOErrorTagsUnsupported(conn.PartitionID, err) {
log.Printf("[WARN] ECS tagging failed describing Service (%s) with tags: %s; retrying without tags", d.Id(), err)
cluster := d.Get("cluster").(string)

input.Include = nil
output, err = conn.DescribeServices(&input)
}
service, err := FindServiceByIDWaitForActive(context.TODO(), conn, d.Id(), cluster)

if !d.IsNewResource() && tfawserr.ErrCodeEquals(err, ecs.ErrCodeServiceNotFoundException) {
log.Printf("[WARN] ECS service (%s) not found, removing from state", d.Id())
if !d.IsNewResource() && tfresource.NotFound(err) {
log.Printf("[WARN] ECS Service (%s) not found, removing from state", d.Id())
d.SetId("")
return nil
}

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))
}

if tfawserr.ErrCodeEquals(err, ecs.ErrCodeClusterNotFoundException) {
log.Printf("[WARN] ECS Service %s parent cluster %s not found, removing from state.", d.Id(), d.Get("cluster").(string))
log.Printf("[WARN] ECS Service (%s) parent cluster (%s) not found, removing from state.", d.Id(), cluster)
d.SetId("")
return nil
}

if err != nil {
return fmt.Errorf("error reading ECS service (%s): %w", d.Id(), err)
}

if len(output.Services) < 1 {
if d.IsNewResource() {
return fmt.Errorf("ECS service not created: %q", d.Id())
}
log.Printf("[WARN] Removing ECS service %s (%s) because it's gone", d.Get("name").(string), d.Id())
var ea *expectActiveError
if errors.As(err, &ea) {
log.Printf("[WARN] ECS Service (%s) in status %q, removing from state.", d.Id(), ea.status)
d.SetId("")
return nil
}

service := output.Services[0]

// Status==INACTIVE means deleted service
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
if err != nil {
return fmt.Errorf("error reading ECS service (%s): %w", d.Id(), err)
}

log.Printf("[DEBUG] Received ECS service %s", service)

d.SetId(aws.StringValue(service.ServiceArn))
d.Set("name", service.ServiceName)

Expand Down Expand Up @@ -1113,11 +1085,11 @@ func resourceServiceUpdate(d *schema.ResourceData, meta interface{}) error {

cluster := d.Get("cluster").(string)
if d.Get("wait_for_steady_state").(bool) {
if err := waitServiceStable(conn, d.Id(), cluster); err != nil {
if _, err := waitServiceStable(conn, d.Id(), cluster, d.Timeout(schema.TimeoutUpdate)); err != nil {
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 {
if _, err := waitServiceActive(conn, d.Id(), cluster, d.Timeout(schema.TimeoutUpdate)); err != nil {
return fmt.Errorf("error waiting for ECS service (%s) to become active after update: %w", d.Id(), err)
}
}
Expand All @@ -1130,12 +1102,12 @@ func resourceServiceUpdate(d *schema.ResourceData, meta interface{}) error {

// Some partitions (i.e., ISO) may not support tagging, giving error
if verify.CheckISOErrorTagsUnsupported(conn.PartitionID, err) {
log.Printf("[WARN] ECS tagging failed updating tags for Service (%s): %s", d.Id(), err)
log.Printf("[WARN] failed updating tags for ECS Service (%s): %s", d.Id(), err)
return resourceServiceRead(d, meta)
}

if err != nil {
return fmt.Errorf("ECS tagging failed updating tags for Service (%s): %w", d.Id(), err)
return fmt.Errorf("failed updating tags for ECS Service (%s): %w", d.Id(), err)
}
}

Expand All @@ -1145,34 +1117,21 @@ func resourceServiceUpdate(d *schema.ResourceData, meta interface{}) error {
func resourceServiceDelete(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*conns.AWSClient).ECSConn

// Check if it's not already gone
output, err := conn.DescribeServices(&ecs.DescribeServicesInput{
Services: aws.StringSlice([]string{d.Id()}),
Cluster: aws.String(d.Get("cluster").(string)),
})

if err != nil {
if tfawserr.ErrCodeEquals(err, ecs.ErrCodeServiceNotFoundException) {
log.Printf("[DEBUG] Removing ECS Service from state, %q is already gone", d.Id())
return nil
}
return err
}

if len(output.Services) == 0 {
log.Printf("[DEBUG] Removing ECS Service from state, %q is already gone", d.Id())
service, err := FindServiceNoTagsByID(context.TODO(), conn, d.Id(), d.Get("cluster").(string))
if tfresource.NotFound(err) {
return nil
}
if err != nil {
return fmt.Errorf("error retrieving ECS Service (%s) for deletion: %w", d.Id(), err)
}

log.Printf("[DEBUG] ECS service %s is currently %s", d.Id(), aws.StringValue(output.Services[0].Status))

if aws.StringValue(output.Services[0].Status) == "INACTIVE" {
if aws.StringValue(service.Status) == serviceStatusInactive {
return nil
}

// Drain the ECS service
if aws.StringValue(output.Services[0].Status) != "DRAINING" && aws.StringValue(output.Services[0].SchedulingStrategy) != ecs.SchedulingStrategyDaemon {
log.Printf("[DEBUG] Draining ECS service %s", d.Id())
if aws.StringValue(service.Status) != serviceStatusDraining && aws.StringValue(service.SchedulingStrategy) != ecs.SchedulingStrategyDaemon {
log.Printf("[DEBUG] Draining ECS Service (%s)", d.Id())
_, err = conn.UpdateService(&ecs.UpdateServiceInput{
Service: aws.String(d.Id()),
Cluster: aws.String(d.Get("cluster").(string)),
Expand Down Expand Up @@ -1211,14 +1170,13 @@ func resourceServiceDelete(d *schema.ResourceData, meta interface{}) error {
}

if err != nil {
return fmt.Errorf("error deleting ECS service (%s): %w", d.Id(), err)
return fmt.Errorf("error deleting ECS Service (%s): %w", d.Id(), err)
}

if err := waitServiceInactive(conn, d.Id(), d.Get("cluster").(string)); err != nil {
return fmt.Errorf("error deleting ECS service (%s): %w", d.Id(), err)
if err := waitServiceInactive(conn, d.Id(), d.Get("cluster").(string), d.Timeout(schema.TimeoutDelete)); err != nil {
return fmt.Errorf("error waiting for ECS Service (%s) to be deleted: %w", d.Id(), err)
}

log.Printf("[DEBUG] ECS service %s deleted.", d.Id())
return nil
}

Expand All @@ -1237,7 +1195,7 @@ func resourceLoadBalancerHash(v interface{}) int {
return create.StringHashcode(buf.String())
}

func retryServiceCreate(conn *ecs.ECS, input ecs.CreateServiceInput) (*ecs.CreateServiceOutput, error) {
func serviceCreateWithRetry(conn *ecs.ECS, input ecs.CreateServiceInput) (*ecs.CreateServiceOutput, error) {
var output *ecs.CreateServiceOutput
err := resource.Retry(propagationTimeout+serviceCreateTimeout, func() *resource.RetryError {
var err error
Expand Down
Loading

0 comments on commit 9af6805

Please sign in to comment.