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.

Sign up for GitHub

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

Jump to bottom

Various ECS bugfixes (IAM, destroy timeout) #3061

Merged
merged 4 commits into from
Aug 25, 2015
Merged

Various ECS bugfixes (IAM, destroy timeout) #3061

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fad019e
ecs_service: Retry if IAM policy isn't ready yet
radeksimko Aug 23, 2015
669d196
ecs_service: Role name can be used in iam_role (ARN was supported)
radeksimko Aug 23, 2015
9c2a3e7
ecs_service: Add note about race condition w/ IAM policy
radeksimko Aug 23, 2015
00646b1
ecs_service: Remove unused code
radeksimko Aug 23, 2015
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
68 changes: 35 additions & 33 deletions builtin/providers/aws/resource_aws_ecs_service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import (
"time"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/service/ecs"
"github.com/aws/aws-sdk-go/service/iam"
"github.com/hashicorp/terraform/helper/hashcode"
"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/helper/schema"
Expand Down Expand Up @@ -105,7 +105,30 @@ func resourceAwsEcsServiceCreate(d *schema.ResourceData, meta interface{}) error
}

log.Printf("[DEBUG] Creating ECS service: %s", input)
out, err := conn.CreateService(&input)

// Retry due to AWS IAM policy eventual consistency
// See https://github.com/hashicorp/terraform/issues/2869
var out *ecs.CreateServiceOutput
var err error
err = resource.Retry(2*time.Minute, func() error {
out, err = conn.CreateService(&input)

if err != nil {
ec2err, ok := err.(awserr.Error)
if !ok {
return &resource.RetryError{Err: err}
}
if ec2err.Code() == "InvalidParameterException" {
log.Printf("[DEBUG] Trying to create ECS service again: %q",
ec2err.Message())
return err
}

return &resource.RetryError{Err: err}
}

return nil
})
if err != nil {
return err
}
Expand Down Expand Up @@ -154,8 +177,14 @@ func resourceAwsEcsServiceRead(d *schema.ResourceData, meta interface{}) error {
d.Set("desired_count", *service.DesiredCount)
d.Set("cluster", *service.ClusterArn)

// Save IAM role in the same format
if service.RoleArn != nil {
d.Set("iam_role", *service.RoleArn)
if strings.HasPrefix(d.Get("iam_role").(string), "arn:aws:iam:") {
d.Set("iam_role", *service.RoleArn)
} else {
roleARN := buildIamRoleNameFromARN(*service.RoleArn)
d.Set("iam_role", roleARN)
}
}

if service.LoadBalancers != nil {
Expand Down Expand Up @@ -277,36 +306,9 @@ func buildFamilyAndRevisionFromARN(arn string) string {
return strings.Split(arn, "/")[1]
}

func buildTaskDefinitionARN(taskDefinition string, meta interface{}) (string, error) {
// If it's already an ARN, just return it
if strings.HasPrefix(taskDefinition, "arn:aws:ecs:") {
return taskDefinition, nil
}

// Parse out family & revision
family, revision, err := parseTaskDefinition(taskDefinition)
if err != nil {
return "", err
}

iamconn := meta.(*AWSClient).iamconn
region := meta.(*AWSClient).region

// An zero value GetUserInput{} defers to the currently logged in user
resp, err := iamconn.GetUser(&iam.GetUserInput{})
if err != nil {
return "", fmt.Errorf("GetUser ERROR: %#v", err)
}

// arn:aws:iam::0123456789:user/username
userARN := *resp.User.Arn
accountID := strings.Split(userARN, ":")[4]

// arn:aws:ecs:us-west-2:01234567890:task-definition/mongodb:3
arn := fmt.Sprintf("arn:aws:ecs:%s:%s:task-definition/%s:%s",
region, accountID, family, revision)
log.Printf("[DEBUG] Built task definition ARN: %s", arn)
return arn, nil
func buildIamRoleNameFromARN(arn string) string {
// arn:aws:iam::0123456789:role/EcsService
return strings.Split(arn, "/")[1]
}

func parseTaskDefinition(taskDefinition string) (string, string, error) {
Expand Down
111 changes: 111 additions & 0 deletions builtin/providers/aws/resource_aws_ecs_service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,22 @@ func TestAccAWSEcsServiceWithRenamedCluster(t *testing.T) {
})
}

func TestAccAWSEcsService_withIamRole(t *testing.T) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a test which is accidentally testing all three bugs.

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSEcsServiceDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: testAccAWSEcsService_withIamRole,
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSEcsServiceExists("aws_ecs_service.ghost"),
),
},
},
})
}

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

Expand Down Expand Up @@ -253,6 +269,101 @@ resource "aws_ecs_service" "mongo" {
}
`

var testAccAWSEcsService_withIamRole = `
resource "aws_ecs_cluster" "main" {
name = "terraformecstest11"
}

resource "aws_ecs_task_definition" "ghost" {
family = "ghost_service"
container_definitions = <<DEFINITION
[
{
"cpu": 128,
"essential": true,
"image": "ghost:latest",
"memory": 128,
"name": "ghost",
"portMappings": [
{
"containerPort": 2368,
"hostPort": 8080
}
]
}
]
DEFINITION
}

resource "aws_iam_role" "ecs_service" {
name = "EcsService"
assume_role_policy = <<EOF
{
"Version": "2008-10-17",
"Statement": [
{
"Action": "sts:AssumeRole",
"Principal": {"AWS": "*"},
"Effect": "Allow",
"Sid": ""
}
]
}
EOF
}

resource "aws_iam_role_policy" "ecs_service" {
name = "EcsService"
role = "${aws_iam_role.ecs_service.name}"
policy = <<EOF
{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Action": [
"elasticloadbalancing:*",
"ec2:*",
"ecs:*"
],
"Resource": [
"*"
]
}
]
}
EOF
}

resource "aws_elb" "main" {
name = "foobar-terraform-test"
availability_zones = ["us-west-2a"]

listener {
instance_port = 8080
instance_protocol = "http"
lb_port = 80
lb_protocol = "http"
}
}

resource "aws_ecs_service" "ghost" {
name = "ghost"
cluster = "${aws_ecs_cluster.main.id}"
task_definition = "${aws_ecs_task_definition.ghost.arn}"
desired_count = 1
iam_role = "${aws_iam_role.ecs_service.name}"

load_balancer {
elb_name = "${aws_elb.main.id}"
container_name = "ghost"
container_port = "2368"
}

depends_on = ["aws_iam_role_policy.ecs_service"]
}
`

var testAccAWSEcsServiceWithFamilyAndRevision = `
resource "aws_ecs_cluster" "default" {
name = "terraformecstest2"
Expand Down
3 changes: 3 additions & 0 deletions website/source/docs/providers/aws/r/ecs_service.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ description: |-

# aws\_ecs\_service

-> **Note:** To prevent race condition during service deletion, make sure to set `depends_on` to related `aws_iam_role_policy`, otherwise policy may be destroyed too soon and ECS service will then stuck in `DRAINING` state.

Provides an ECS service - effectively a task that is expected to run until an error occures or user terminates it (typically a webserver or a database).

See [ECS Services section in AWS developer guide](http://docs.aws.amazon.com/AmazonECS/latest/developerguide/ecs_services.html).
Expand All @@ -21,6 +23,7 @@ resource "aws_ecs_service" "mongo" {
task_definition = "${aws_ecs_task_definition.mongo.arn}"
desired_count = 3
iam_role = "${aws_iam_role.foo.arn}"
depends_on = ["aws_iam_role_policy.foo"]

load_balancer {
elb_name = "${aws_elb.foo.id}"
Expand Down