-
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: fix instance profile creation false negative #11678
provider/aws: fix instance profile creation false negative #11678
Conversation
fixes hashicorp#9474 discussion of approach in hashicorp#11634
Hey @catsby , here's the PR you suggested! |
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
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSIAMInstanceProfile -timeout 120m
=== RUN TestAccAWSIAMInstanceProfile_importBasic
--- PASS: TestAccAWSIAMInstanceProfile_importBasic (12.36s)
=== RUN TestAccAWSIAMInstanceProfile_basic
--- PASS: TestAccAWSIAMInstanceProfile_basic (10.78s)
=== RUN TestAccAWSIAMInstanceProfile_namePrefix
--- PASS: TestAccAWSIAMInstanceProfile_namePrefix (11.19s)
PASS
ok github.com/hashicorp/terraform/builtin/providers/aws 34.374s
Also used this config to make sure the instance+profile bit worked:
data "aws_ami" "nat_ami" {
most_recent = true
filter {
name = "owner-alias"
values = ["amazon"]
}
filter {
name = "name"
values = ["amzn-ami-vpc-nat*"]
}
filter {
name = "virtualization-type"
values = ["hvm"]
}
filter {
name = "root-device-type"
values = ["ebs"]
}
filter {
name = "block-device-mapping.volume-type"
values = ["standard"]
}
}
resource "aws_iam_role_policy" "instance_policy" {
name = "prefix-project_name-policy"
role = "${aws_iam_role.instance_role.id}"
policy = <<EOF
{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Action": "s3:*",
"Resource": "*"
}
]
}
EOF
}
resource "aws_iam_role" "instance_role" {
name = "prefix-project_name-policy-role"
assume_role_policy = <<EOF
{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Principal": {
"Service": "ec2.amazonaws.com"
},
"Action": "sts:AssumeRole"
}
]
}
EOF
}
resource "aws_instance" "tf_test" {
count = 7
ami = "${data.aws_ami.nat_ami.id}"
instance_type = "t2.micro"
associate_public_ip_address = true
iam_instance_profile = "${aws_iam_instance_profile.test_profile.name}"
tags {
Name = "terraform test"
}
}
resource "aws_iam_instance_profile" "test_profile" {
name = "test_profile"
roles = ["${aws_iam_role.instance_role.name}"]
}
Merged and cherry-picked into |
@radeksimko Am I right in thinking that this means that the naive 15s wait on instance creation https://github.com/hashicorp/terraform/blob/master/builtin/providers/aws/resource_aws_instance.go#L371 can now be removed? |
I don't believe so – IAM has been a source of inconsistency in the API for some time, so while the IAM API may report the profile to exist, an immediate follow up call from EC2 may not get the same result, or perhaps nay not report that the profile has any Roles yet. Ideally everything will be fine, and leaving the retry will be harmless because we'll never need to retry. However, APIs for distributed clouds are not always ideal so we should leave the retry for now 😄 |
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 #9474
discussion of approach in #11634