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.

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

aws_ecs_task_definition: Regression from issue #1195 causing an endless cycle of diffs #2336

Closed
ziggythehamster opened this issue Nov 17, 2017 · 4 comments · Fixed by #2339
Closed
Labels
bug Addresses a defect in current functionality. regression Pertains to a degraded workflow resulting from an upstream patch or internal enhancement.
Milestone

Comments

@ziggythehamster
Copy link
Contributor

The change in #1195 is causing issues with existing ECS task definitions. The task definitions coming back from AWS are logically equivalent but are structurally different. For instance, my environment array is in a different order than what I have on disk. Also, there are a few properties which are empty arrays (like volumesFrom) which come back despite not being specified in my file. And the protocol for a port comes back as tcp even though that's optional and the default.

Anyway, all of these "changes" result in a diff. The code handling the JSON from AWS needs to transform the input JSON and AWS's JSON so that the data is homogenized the same way to avoid unnecessary diffs.

(Also, sorry for not filling in the form, if I didn't give enough detail please let me know and I'll fill this in correctly tomorrow.)

@nathanielks
Copy link
Contributor

Just noticed the same thing!

@radeksimko
Copy link
Member

Thanks for the report and sorry for the troubles.
Would you mind providing an example config which is causing this?

we're well aware of empty arrays and those should be handled appropriately:

https://github.com/terraform-providers/terraform-provider-aws/blob/a301ad908ed20a528c338cc59a8e5122d753910a/aws/ecs_task_definition_equivalency.go#L66-L75

as well as the tcp protocol:

https://github.com/terraform-providers/terraform-provider-aws/blob/a301ad908ed20a528c338cc59a8e5122d753910a/aws/ecs_task_definition_equivalency.go#L49-L57

I assume the only thing we're not handling - which may be causing this is different ordering of environment. I can try to come up with a config to reproduce, but if you have one that would be wonderful!

@radeksimko radeksimko added the bug Addresses a defect in current functionality. label Nov 17, 2017
@radeksimko
Copy link
Member

Reproduced with the following config:

resource "aws_ecs_task_definition" "test" {
  family = "test"
  container_definitions = <<TASK_DEFINITION
[
	{
		"cpu": 10,
		"command": ["sleep", "10"],
		"entryPoint": ["/"],
		"environment": [
			{"name": "VARNAME1", "value": "VARVAL1"},
			{"name": "VARNAME2", "value": "VARVAL2"},
			{"name": "VARNAME3", "value": "VARVAL3"},
			{"name": "VARNAME4", "value": "VARVAL4"},
			{"name": "VARNAME5", "value": "VARVAL5"}
		],
		"essential": true,
		"image": "jenkins",
		"memory": 128,
		"name": "test"
	}
]
TASK_DEFINITION
}

I'll check all arrays to see if the ordering is significant and then treat it as necessary (e.g. sort it before comparing).

@radeksimko radeksimko added this to the v1.3.1 milestone Nov 17, 2017
@radeksimko radeksimko added the regression Pertains to a degraded workflow resulting from an upstream patch or internal enhancement. label Nov 17, 2017
@ghost
Copy link

ghost commented Apr 10, 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 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. regression Pertains to a degraded workflow resulting from an upstream patch or internal enhancement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants