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

provider/aws: Fix EMR Bootstrap Action Ordering #13580

Merged
merged 4 commits into from
Apr 12, 2017

Conversation

catsby
Copy link
Contributor

@catsby catsby commented Apr 12, 2017

aws_emr_cluster has a bootstrap_actions parameter, which itself has an args parameter. args is stored as a TypeSet, which does not preserve ordering of it's items. See #10691.

In this PR we change the args from TypeSet to TypeList.

This is considered a Breaking Change.. kind of (BC)

Users who upgrade to this version will experience a diff on their next plan run, however, this is deemed necessary because of the lack of ordering that's currently in place. Essentially, users have specified the order, but Terraform is not honoring that, and the API may have a different order. Changing this is required, and it can't be migrated because the change will need to happen on the API side as well.

Fixes #10691


The failing test, before the fix:

TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSEMRCluster_bootstrap_ordering -timeout 120m
=== RUN   TestAccAWSEMRCluster_bootstrap_ordering
--- FAIL: TestAccAWSEMRCluster_bootstrap_ordering (645.01s)
        testing.go:280: Step 0 error: Check failed: Check 2/2 error: Error matching Bootstrap args:
                        expected: []string{"1", "2", "3", "4", "5", "6", "7", "8", "9", "10"}
                        got: []string{"7", "3", "1", "5", "9", "10", "4", "8", "2", "6"}
FAIL
exit status 1
FAIL    github.com/hashicorp/terraform/builtin/providers/aws    645.039s

After the fix(es):

TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSEMRCluster_bootstrap_ordering -timeout 120m
=== RUN   TestAccAWSEMRCluster_bootstrap_ordering
--- PASS: TestAccAWSEMRCluster_bootstrap_ordering (603.95s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    603.970s

@catsby catsby changed the title [WIP] provider/aws: Fix EMR Bootstrap Action Ordering provider/aws: Fix EMR Bootstrap Action Ordering Apr 12, 2017
@catsby catsby requested a review from radeksimko April 12, 2017 14:57
Copy link
Contributor

@apparentlymart apparentlymart left a comment

Choose a reason for hiding this comment

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

LGTM, though I would defer to someone who knows this resource better than I do if such a person is available. 😀

Copy link
Contributor

@grubernaut grubernaut left a comment

Choose a reason for hiding this comment

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

LGTM

@catsby catsby merged commit 9ef9501 into master Apr 12, 2017
@catsby catsby deleted the b-aws-emr-bootstrap-order branch April 12, 2017 19:19
@ghost
Copy link

ghost commented Apr 14, 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 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.

@ghost ghost locked and limited conversation to collaborators Apr 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

aws_emr_cluster: parameters passed to bootstrap_action are in the wrong order
4 participants