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

r/ecs_service: Added ordered placement strategy #4390

Merged
merged 2 commits into from
Apr 30, 2018
Merged

r/ecs_service: Added ordered placement strategy #4390

merged 2 commits into from
Apr 30, 2018

Conversation

atsushi-ishibashi
Copy link
Contributor

@atsushi-ishibashi atsushi-ishibashi commented Apr 30, 2018 •
edited
Loading

Related: #1476

make testacc TEST=./aws TESTARGS='-run=TestAccAWSEcsService*'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSEcsService* -timeout 120m
=== RUN   TestAccAWSEcsService_withARN
--- PASS: TestAccAWSEcsService_withARN (90.39s)
=== RUN   TestAccAWSEcsService_basicImport
--- PASS: TestAccAWSEcsService_basicImport (69.87s)
=== RUN   TestAccAWSEcsService_withUnnormalizedPlacementStrategy
--- PASS: TestAccAWSEcsService_withUnnormalizedPlacementStrategy (57.94s)
=== RUN   TestAccAWSEcsService_withFamilyAndRevision
--- PASS: TestAccAWSEcsService_withFamilyAndRevision (70.74s)
=== RUN   TestAccAWSEcsService_withRenamedCluster
--- PASS: TestAccAWSEcsService_withRenamedCluster (99.11s)
=== RUN   TestAccAWSEcsService_healthCheckGracePeriodSeconds
--- PASS: TestAccAWSEcsService_healthCheckGracePeriodSeconds (356.86s)
=== RUN   TestAccAWSEcsService_withIamRole
--- PASS: TestAccAWSEcsService_withIamRole (137.31s)
=== RUN   TestAccAWSEcsService_withDeploymentValues
--- PASS: TestAccAWSEcsService_withDeploymentValues (46.42s)
=== RUN   TestAccAWSEcsService_withLbChanges
--- PASS: TestAccAWSEcsService_withLbChanges (228.93s)
=== RUN   TestAccAWSEcsService_withEcsClusterName
--- PASS: TestAccAWSEcsService_withEcsClusterName (49.66s)
=== RUN   TestAccAWSEcsService_withAlb
--- PASS: TestAccAWSEcsService_withAlb (303.50s)
=== RUN   TestAccAWSEcsService_withPlacementStrategy
--- PASS: TestAccAWSEcsService_withPlacementStrategy (136.09s)
=== RUN   TestAccAWSEcsService_withPlacementConstraints
--- PASS: TestAccAWSEcsService_withPlacementConstraints (56.23s)
=== RUN   TestAccAWSEcsService_withPlacementConstraints_emptyExpression
--- PASS: TestAccAWSEcsService_withPlacementConstraints_emptyExpression (62.02s)
=== RUN   TestAccAWSEcsService_withLaunchTypeFargate
--- FAIL: TestAccAWSEcsService_withLaunchTypeFargate (510.55s)
	testing.go:518: Step 0 error: After applying this step, the plan was not empty:
		
		DIFF:
		
		UPDATE: aws_security_group.allow_all_b
		  ingress.#:                             "0" => "1"
		  ingress.2269895920.cidr_blocks.#:      "0" => "1"
		  ingress.2269895920.cidr_blocks.0:      "" => "10.10.0.0/16"
		  ingress.2269895920.description:        "" => ""
		  ingress.2269895920.from_port:          "" => "80"
		  ingress.2269895920.ipv6_cidr_blocks.#: "0" => "0"
		  ingress.2269895920.protocol:           "" => "tcp"
		  ingress.2269895920.security_groups.#:  "0" => "0"
		  ingress.2269895920.self:               "" => "false"
		  ingress.2269895920.to_port:            "" => "8000"
		
		STATE:
		
		aws_ecs_cluster.main:
		  ID = arn:aws:ecs:us-east-1:481473273857:cluster/tf-acc-cluster-svc-w-ltf-gy77qbah
		  provider = provider.aws
		  arn = arn:aws:ecs:us-east-1:481473273857:cluster/tf-acc-cluster-svc-w-ltf-gy77qbah
		  name = tf-acc-cluster-svc-w-ltf-gy77qbah
		aws_ecs_service.main:
		  ID = arn:aws:ecs:us-east-1:481473273857:service/tf-acc-svc-w-ltf-gy77qbah
		  provider = provider.aws
		  cluster = arn:aws:ecs:us-east-1:481473273857:cluster/tf-acc-cluster-svc-w-ltf-gy77qbah
		  deployment_maximum_percent = 200
		  deployment_minimum_healthy_percent = 100
		  desired_count = 1
		  health_check_grace_period_seconds = 0
		  iam_role = aws-service-role
		  launch_type = FARGATE
		  load_balancer.# = 0
		  name = tf-acc-svc-w-ltf-gy77qbah
		  network_configuration.# = 1
		  network_configuration.0.assign_public_ip = true
		  network_configuration.0.security_groups.# = 2
		  network_configuration.0.security_groups.2328280639 = sg-cbe8d382
		  network_configuration.0.security_groups.372139329 = sg-49eed500
		  network_configuration.0.subnets.# = 2
		  network_configuration.0.subnets.4057621718 = subnet-002bc42e
		  network_configuration.0.subnets.4073193208 = subnet-2694256c
		  ordered_placement_strategy.# = 0
		  placement_constraints.# = 0
		  service_registries.# = 0
		  task_definition = arn:aws:ecs:us-east-1:481473273857:task-definition/tf-acc-td-svc-w-ltf-gy77qbah:2
		
		  Dependencies:
		    aws_ecs_cluster.main
		    aws_ecs_task_definition.mongo
		    aws_security_group.allow_all_a
		    aws_security_group.allow_all_b
		    aws_subnet.main.*
		aws_ecs_task_definition.mongo:
		  ID = tf-acc-td-svc-w-ltf-gy77qbah
		  provider = provider.aws
		  arn = arn:aws:ecs:us-east-1:481473273857:task-definition/tf-acc-td-svc-w-ltf-gy77qbah:2
		  container_definitions = [{"cpu":256,"environment":[],"essential":true,"image":"mongo:latest","memory":512,"mountPoints":[],"name":"mongodb","portMappings":[],"volumesFrom":[]}]
		  cpu = 256
		  execution_role_arn = 
		  family = tf-acc-td-svc-w-ltf-gy77qbah
		  memory = 512
		  network_mode = awsvpc
		  placement_constraints.# = 0
		  requires_compatibilities.# = 1
		  requires_compatibilities.3072437307 = FARGATE
		  revision = 2
		  task_role_arn = 
		  volume.# = 0
		aws_security_group.allow_all_a:
		  ID = sg-49eed500
		  provider = provider.aws
		  arn = arn:aws:ec2:us-east-1:481473273857:security-group/sg-49eed500
		  description = Allow all inbound traffic
		  egress.# = 0
		  ingress.# = 1
		  ingress.2269895920.cidr_blocks.# = 1
		  ingress.2269895920.cidr_blocks.0 = 10.10.0.0/16
		  ingress.2269895920.description = 
		  ingress.2269895920.from_port = 80
		  ingress.2269895920.ipv6_cidr_blocks.# = 0
		  ingress.2269895920.protocol = tcp
		  ingress.2269895920.security_groups.# = 0
		  ingress.2269895920.self = false
		  ingress.2269895920.to_port = 8000
		  name = tf-acc-sg-1-svc-w-ltf-gy77qbah
		  owner_id = 481473273857
		  revoke_rules_on_delete = false
		  tags.% = 0
		  vpc_id = vpc-d31047a8
		
		  Dependencies:
		    aws_vpc.main
		aws_security_group.allow_all_b:
		  ID = sg-cbe8d382
		  provider = provider.aws
		  arn = arn:aws:ec2:us-east-1:481473273857:security-group/sg-cbe8d382
		  description = Allow all inbound traffic
		  egress.# = 0
		  ingress.# = 0
		  name = tf-acc-sg-2-svc-w-ltf-gy77qbah
		  owner_id = 481473273857
		  revoke_rules_on_delete = false
		  tags.% = 0
		  vpc_id = vpc-d31047a8
		
		  Dependencies:
		    aws_vpc.main
		aws_subnet.main.0:
		  ID = subnet-002bc42e
		  provider = provider.aws
		  assign_ipv6_address_on_creation = false
		  availability_zone = us-east-1a
		  cidr_block = 10.10.0.0/24
		  map_public_ip_on_launch = false
		  tags.% = 1
		  tags.Name = tf-acc-ecs-service-with-launch-type-fargate
		  vpc_id = vpc-d31047a8
		
		  Dependencies:
		    aws_vpc.main
		    data.aws_availability_zones.available
		aws_subnet.main.1:
		  ID = subnet-2694256c
		  provider = provider.aws
		  assign_ipv6_address_on_creation = false
		  availability_zone = us-east-1b
		  cidr_block = 10.10.1.0/24
		  map_public_ip_on_launch = false
		  tags.% = 1
		  tags.Name = tf-acc-ecs-service-with-launch-type-fargate
		  vpc_id = vpc-d31047a8
		
		  Dependencies:
		    aws_vpc.main
		    data.aws_availability_zones.available
		aws_vpc.main:
		  ID = vpc-d31047a8
		  provider = provider.aws
		  assign_generated_ipv6_cidr_block = false
		  cidr_block = 10.10.0.0/16
		  default_network_acl_id = acl-914ce4eb
		  default_route_table_id = rtb-d61b29aa
		  default_security_group_id = sg-17e9d25e
		  dhcp_options_id = dopt-c679a6a0
		  enable_classiclink = false
		  enable_classiclink_dns_support = false
		  enable_dns_hostnames = false
		  enable_dns_support = true
		  instance_tenancy = default
		  main_route_table_id = rtb-d61b29aa
		  tags.% = 1
		  tags.Name = terraform-testacc-ecs-service-with-launch-type-fargate
		data.aws_availability_zones.available:
		  ID = 2018-04-30 01:50:15.96365987 +0000 UTC
		  provider = provider.aws
		  names.# = 6
		  names.0 = us-east-1a
		  names.1 = us-east-1b
		  names.2 = us-east-1c
		  names.3 = us-east-1d
		  names.4 = us-east-1e
		  names.5 = us-east-1f
=== RUN   TestAccAWSEcsService_withLaunchTypeEC2AndNetworkConfiguration
--- PASS: TestAccAWSEcsService_withLaunchTypeEC2AndNetworkConfiguration (186.37s)
=== RUN   TestAccAWSEcsService_withServiceRegistries
--- PASS: TestAccAWSEcsService_withServiceRegistries (128.50s)
FAIL
FAIL	github.com/terraform-providers/terraform-provider-aws/aws	2590.537s
make: *** [testacc] Error 1

TestAccAWSEcsService_withLaunchTypeFargate isn't associated with this change.

@ghost ghost added the size/L Managed by automation to categorize the size of a PR. label Apr 30, 2018
@Puneeth-n
Copy link
Contributor

@atsushi-ishibashi Can't wait for this! :)

@bflad bflad added enhancement Requests to existing resources that expand the functionality or scope. service/ecs Issues and PRs that pertain to the ecs service. labels Apr 30, 2018
@bflad bflad added this to the v1.17.0 milestone Apr 30, 2018
bflad
bflad reviewed Apr 30, 2018
View reviewed changes
aws/resource_aws_ecs_service.go
if err := d.Set("placement_strategy", flattenPlacementStrategy(service.PlacementStrategy)); err != nil {
log.Printf("[ERR] Error setting placement_strategy for (%s): %s", d.Id(), err)
if _, ok := d.GetOk("placement_strategy"); ok {
d.Set("placement_strategy", flattenPlacementStrategyDeprecated(service.PlacementStrategy))
Copy link
Contributor

Choose a reason for hiding this comment

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

We should check for errors when setting non-scalar values into the Terraform state:

if err := d.Set("placement_strategy", flattenPlacementStrategyDeprecated(service.PlacementStrategy)); err != nil {
  return fmt.Errorf("error setting placement_strategy: %s", err)
}

I will fix this for both of these on merge.

bflad
bflad approved these changes Apr 30, 2018
View reviewed changes
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

Thanks for this @atsushi-ishibashi, looks good 🚀

 make testacc TEST=./aws TESTARGS='-run=TestAccAWSEcsService'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSEcsService -timeout 120m
=== RUN   TestAccAWSEcsService_withARN
--- PASS: TestAccAWSEcsService_withARN (39.34s)
=== RUN   TestAccAWSEcsService_basicImport
--- PASS: TestAccAWSEcsService_basicImport (33.45s)
=== RUN   TestAccAWSEcsService_withUnnormalizedPlacementStrategy
--- PASS: TestAccAWSEcsService_withUnnormalizedPlacementStrategy (41.26s)
=== RUN   TestAccAWSEcsService_withFamilyAndRevision
--- PASS: TestAccAWSEcsService_withFamilyAndRevision (36.92s)
=== RUN   TestAccAWSEcsService_withRenamedCluster
--- PASS: TestAccAWSEcsService_withRenamedCluster (82.45s)
=== RUN   TestAccAWSEcsService_healthCheckGracePeriodSeconds
--- PASS: TestAccAWSEcsService_healthCheckGracePeriodSeconds (289.60s)
=== RUN   TestAccAWSEcsService_withIamRole
--- PASS: TestAccAWSEcsService_withIamRole (115.29s)
=== RUN   TestAccAWSEcsService_withDeploymentValues
--- PASS: TestAccAWSEcsService_withDeploymentValues (31.31s)
=== RUN   TestAccAWSEcsService_withLbChanges
--- PASS: TestAccAWSEcsService_withLbChanges (237.38s)
=== RUN   TestAccAWSEcsService_withEcsClusterName
--- PASS: TestAccAWSEcsService_withEcsClusterName (41.86s)
=== RUN   TestAccAWSEcsService_withAlb
--- PASS: TestAccAWSEcsService_withAlb (265.51s)
=== RUN   TestAccAWSEcsService_withPlacementStrategy
--- PASS: TestAccAWSEcsService_withPlacementStrategy (75.28s)
=== RUN   TestAccAWSEcsService_withPlacementConstraints
--- PASS: TestAccAWSEcsService_withPlacementConstraints (53.16s)
=== RUN   TestAccAWSEcsService_withPlacementConstraints_emptyExpression
--- PASS: TestAccAWSEcsService_withPlacementConstraints_emptyExpression (17.99s)
=== RUN   TestAccAWSEcsService_withLaunchTypeFargate
--- PASS: TestAccAWSEcsService_withLaunchTypeFargate (210.09s)
=== RUN   TestAccAWSEcsService_withLaunchTypeEC2AndNetworkConfiguration
--- PASS: TestAccAWSEcsService_withLaunchTypeEC2AndNetworkConfiguration (49.96s)
=== RUN   TestAccAWSEcsService_withServiceRegistries
--- PASS: TestAccAWSEcsService_withServiceRegistries (50.93s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	1671.811s

@bflad bflad merged commit db16bf2 into hashicorp:master Apr 30, 2018
bflad added a commit that referenced this pull request Apr 30, 2018
@bflad
Update CHANGELOG for #4390
ce9d8f5
@nathanielks
Copy link
Contributor

Thank you so much, @atsushi-ishibashi! This is a tremendous help!

@nathanielks
Copy link
Contributor

Thanks to @bflad and the Hashicorp team for getting this in as well!

@bflad
Copy link
Contributor

bflad commented May 2, 2018

This has been released in version 1.17.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

@austinbutler austinbutler mentioned this pull request May 7, 2018
Closed
@ghost
Copy link

ghost commented Apr 6, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Apr 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bflad bflad bflad approved these changes

Assignees
No one assigned
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/ecs Issues and PRs that pertain to the ecs service. size/L Managed by automation to categorize the size of a PR.
Projects
None yet
Milestone
v1.17.0
Development

Successfully merging this pull request may close these issues.
4 participants
@atsushi-ishibashi @Puneeth-n @nathanielks @bflad