fix race condition between iam_role and iam_policies

[thiagonache]

See Issue #6082

[stack72]

why are we adding a sleep here?

[thiagonache]

Because the IAM Instance Profile was being deleted between my GetInstanceProfile and the remove itself. Without the sleep, it runs okay a lot of time, but sometimes it fails.

[kpeder]

<     roles = ["S3ReadOnlyAccess"]
---
>     roles = ["${aws_iam_role.S3ReadOnlyAccess.name}"]

[kpeder]

Did some testing and had a discussion about this, and determined that the issue here is that the 'IAM Instance Profile' destroy needs to occur before the 'IAM Role' destroy. If this dependency is fixed, no change is required to the function patched here by @thiagonache.

The patch submitted only works inconsistently (hence the sleep timer) due to the race condition between the 'IAM Instance Profile' destroy function and the patched 'IAM Role' destroy function in processing the removal of instance profiles/policies from the IAM role. Remove the race condition and it seems to work fine; we have tested this by using 'depends_on' explicitly in our resource templates to ensure that these don't execute in parallel.

I there a better way to ensure these are identified as dependent during execution planning by default?

--adding a further note, we may have a similar dependency on destruction of the IAM role resource to its policy attachments, although I have not encountered this on any run. Perhaps this dependency is properly managed by terraform already.

---it appears from the graph output below that the policy attachment dependency is also not handled by default, as speculated directly above.

[kpeder]

Adding sample graphs... first without and then with 'depends_on' set in the template.
graph (default behaviour):

graph_with_depends_on (manual deps specified):

The graph_with_depends_on reflects the required dependencies for a successful run, manually specified in the template.

[kpeder]

Deeper investigation indicates that this scenario is resolved by ensuring that the terraform graphing code has verticies on which to identify dependencies. We introduce the scenario by referencing an attribute (such as a role name or policy arn) in string format. This constitutes no vertex on which the plan can identify the dependency. It's just a string attribute like any other.

To solve the issue, build your resources in such a way that they reference each other by their resource attribute instead of a string, such as:

<     roles = ["S3ReadOnlyAccess"]
---
>     roles = ["${aws_iam_role.S3ReadOnlyAccess.name}"]

...this provides terraform with the linkage between resource objects that it needs to identify and graph the dependencies properly.

[apparentlymart]

Hello @thiagonache, and thanks for working on this!

As part of the the Terraform 0.10 release earlier this year, all of the Terraform providers were moved to their own repositories in the terraform-providers GitHub organization, and removed from the Terraform Core repository.

Unfortunately due to the fact that new issues and pull requests are being opened constantly, it was not possible for the various provider maintainers to merge all outstanding pull requests before this split, and there is no automatic way to migrate a pull request to a new repository.

As a result, this pull request can sadly no longer be applied as-is, and so I'm going to close it.

If you or someone else has the time and motivation to apply same changes to the aws provider repository and open a new PR there, the maintainers of that provider should be able to review and merge it. Alternatively, from the discussion here it sounds like it's possible to work around the race condition using depends_on.

Thanks again for working on this, and sorry it was not able to be merged before the provider repository changes.

[ghost]

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.