hashicorp / terraform-provider-aws

The AWS Provider enables Terraform to manage AWS resources.
https://registry.terraform.io/providers/hashicorp/aws
Mozilla Public License 2.0
9.75k stars 9.1k forks source link

aws_ecs_task_definition computes and apply changes when there are none. #2369

Closed deweysasser closed 6 years ago

deweysasser commented 6 years ago

Terraform Version

Terraform v0.11.0
+ provider.aws v1.3.0

Affected Resource(s)

Terraform Configuration Files


provider "aws" {
}

variable "cluster" {
   default = "default-cluster"
}

resource aws_ecs_task_definition "task" {
  family = "terraform-bug"
  container_definitions = <<EOF
[
   {
         "Name": "nginx",
         "Image": "nginx",
         "PortMappings": [
            {
               "ContainerPort": 80,
               "Protocol": "tcp",
               "HostPort": 8080
            }
         ],
         "MemoryReservation": 12,
         "Memory": 64,
         "Essential": true
      }
  ]
EOF
}

resource aws_ecs_service "terraform-bug" {
     name = "terraform-bug"
     cluster = "${var.cluster}"
     task_definition = "${aws_ecs_task_definition.task.id}"
     desired_count=1
}```

### Debug Output

https://gist.github.com/deweysasser/65df4e520cd62ce73db8fba92ac8b22e

### Panic Output

No panic output

### Expected Behavior

When applied twice, the resource should not be updated the 2nd time.  As nothing about the resource changed, `terraform plan` should have reported that all resources were up to date.

### Actual Behavior

`terraform plan` determines that the resource changed due to 2 fields (`id` and `container_definitions`) and updates the task and the dependent service.  Fortunately this does *NOT* force the container to restart.

### Steps to Reproduce

- Create a ECS cluster using any means
- Apply the above template to your cluster
- Apply the template a 2nd time, notice that the task definition version updates to :2 and the service is updated to reflect this change.

### Important Factoids

Just to make sure it wasn't some odd Cygwin thing, I ran this on Ubuntu Linux 16.04 with same results.
bforchhammer commented 6 years ago

I have the same problem with container_definitions appearing as changed again after applying changes. It seems that null values are not filtered out anymore, and the order of environment variables seems to be different...

    container_definitions:   "g9:batch:archive_inactive_users"
                                  ],
                                  "cpu": 100,
                                  "environment": [
                                    {
                             +        "name": "redis_request_1",
                             +        "value": "{snip}"
                             +      },
                             +      {
                                      "name": "RAILS_ENV",
                                      "value": "ci"
                                    },
                                    {
                                      "name": "maria_db_host",
                                    {
                                      "name": "riak_host",
                                      "value": "{snip}"
                                    },
                                    {
                             -        "name": "redis_request_1",
                             -        "value": "{snip}"
                             -      },
                             -      {
                             -        "name": "NEWRELIC_AGENT_ENABLED",
                             -        "value": "false"
                             +        "name": "polly_host",
                             +        "value": "{snip}"
                                    },
                                    {
                                      "name": "THIN_COUNT",
                                      "value": "2"
                                    },
                                    {
                             -        "name": "polly_host",
                             -        "value": "{snip}"
                             +        "name": "NEWRELIC_AGENT_ENABLED",
                             +        "value": "false"
                                    }
                                  ],
                                  "essential": true,
                                  "image": "{snip}",
                                  "links": [
                                  ],
                                  "name": "archiver",
                                  "portMappings": [

                                  ],
                             +    "privileged": null,
                             -    "volumesFrom": [
                             -
                             -    ],
                                }
                              ]

This only started happening after upgrading the provider version to version: ~> 1.3.

bforchhammer commented 6 years ago

Hm, and it looks like this is fixed by https://github.com/terraform-providers/terraform-provider-aws/pull/2339... so the issue can probably be closed.

Ninir commented 6 years ago

@bforchhammer Could you check it on your own, compiling the master branch, and let us know?

Thanks!

deweysasser commented 6 years ago

(@bforchhammer) I built master and tried this test case under linux. It does indeed seem that #2339 fixed this issue.

For any who comes after looking at this issue, in order to make everything work properly you should reference the arn of the task definition in the aws_ecs_service, rather than the id.

In other words: task_definition = "${aws_ecs_task_definition.task.arn}" and NOT task_definition = "${aws_ecs_task_definition.task.id}"

The issue is that the task_definition field of the aws_ecs_service gets the version number appended, which is always different than the unversioned task name. Also note that this change does NOT fix the original issue without #2339 applied.

paddycarver commented 6 years ago

I'm going to close this out, as #2339 reportedly fixes it. Sorry for the issues!

ghost commented 4 years ago

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!