-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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: Only call replace Iam Instance Profile on existing machines #12922
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add a regression test. Our current test didn't catch this. To my knowledge the only one that uses an iam_instance_profile
is TestAccAWSInstance_instanceProfileChange
which first creates an instance without a Profile, then adds one, thus avoiding this issue
machines Fixes: #12898 The way aws_instance works is that we call the Create func then the Update func then the Read func. The way the work to implement the change to iam_instance_profile was added meant that when a machine was created with an iam_instance_profile, it would then try and update that iam_instance_profile because the state hadn't been updated at that point We have changed the Update func to only check for the change to iam_instance_profile when it *is an existing machine* - this will solve the problem of those bringing up new machines and getting hit with the permissions error As requested, added a test that adds an IAM Instance Profile from creation ``` % make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSInstance_withIamInstanceProfile' ==> Checking that code complies with gofmt requirements... go generate $(go list ./... | grep -v /terraform/vendor/) 2017/03/21 17:51:32 Generated command/internal_plugin_list.go TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSInstance_withIamInstanceProfile -timeout 120m === RUN TestAccAWSInstance_withIamInstanceProfile --- PASS: TestAccAWSInstance_withIamInstanceProfile (154.29s) PASS ok github.com/hashicorp/terraform/builtin/providers/aws 154.325s ```
81b2038
to
f2e1e59
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the regression test
) machines Fixes: #12898 The way aws_instance works is that we call the Create func then the Update func then the Read func. The way the work to implement the change to iam_instance_profile was added meant that when a machine was created with an iam_instance_profile, it would then try and update that iam_instance_profile because the state hadn't been updated at that point We have changed the Update func to only check for the change to iam_instance_profile when it *is an existing machine* - this will solve the problem of those bringing up new machines and getting hit with the permissions error As requested, added a test that adds an IAM Instance Profile from creation ``` % make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSInstance_withIamInstanceProfile' ==> Checking that code complies with gofmt requirements... go generate $(go list ./... | grep -v /terraform/vendor/) 2017/03/21 17:51:32 Generated command/internal_plugin_list.go TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSInstance_withIamInstanceProfile -timeout 120m === RUN TestAccAWSInstance_withIamInstanceProfile --- PASS: TestAccAWSInstance_withIamInstanceProfile (154.29s) PASS ok github.com/hashicorp/terraform/builtin/providers/aws 154.325s ```
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. |
Fixes: #12898
Fixes: #12927
The way aws_instance works is that we call the Create func then the
Update func then the Read func. The way the work to implement the change
to iam_instance_profile was added meant that when a machine was created
with an iam_instance_profile, it would then try and update that
iam_instance_profile because the state hadn't been updated at that point
We have changed the Update func to only check for the change to
iam_instance_profile when it is an existing machine - this will solve
the problem of those bringing up new machines and getting hit with the
permissions error