-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
Fixes for more resource.Retry calls #8893
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.
Two little things then this should be good to go 👍
@@ -370,7 +370,7 @@ func TestAccAWSElasticacheCluster_vpc(t *testing.T) { | |||
testAccCheckAWSElasticacheClusterExists("aws_elasticache_cluster.bar", &ec), | |||
testAccCheckAWSElasticacheClusterAttributes(&ec), | |||
resource.TestCheckResourceAttr( | |||
"aws_elasticache_cluster.bar", "availability_zone", "us-west-2a"), | |||
"aws_elasticache_cluster.bar", "availability_zone", "us-east-1a"), |
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.
These changes appear to be unrelated. 😉
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.
Yes, but I was unable to get the tests to pass before or after my changes without this one. Is this something that should go in a separate PR though?
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.
Yes, please! Its likely the parallel testing issue we talked about earlier today where one of the resource configurations has provider "aws"
with us-east-1 region declared (or a acceptance test function is monkeying with the AWS_DEFAULT_REGION
environment variable for EC2-Classic reasons), which works fine in TeamCity as that environment runs each test in separate go test
processes, but not locally where the test provider is shared in the same go test
process. We'll eventually clean up the oddball us-east-1
testing setups as part of a larger project with a more elegant solution.
If you do see hardcoded availability zones and such, the best course of action with them is instead using the aws_availability_zones
data source, e.g.
data "aws_availability_zones" "available" {
state = "available"
}
# ... other configuration ...
resource "aws_subnet" "foo" {
# ... other configuration ...
availability_zone = "${data.aws_availability_zones.available.names[0]}"
}
And the checks within the acceptance test can use something like the below instead of hardcoding the AZ:
resource.TestCheckResourceAttrPair("aws_elasticache_cluster.bar", "availability_zone", "data.aws_availability_zones.available", "names.0"),
@@ -759,7 +759,7 @@ resource "aws_lambda_function" "authorizer" { | |||
function_name = "tf-acc-test-authorizer-%d" | |||
role = "${aws_iam_role.iam_for_lambda.arn}" | |||
handler = "main.authenticate" | |||
runtime = "nodejs6.10" | |||
runtime = "nodejs8.10" |
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.
👍
Co-Authored-By: Brian Flad <[email protected]>
@@ -1171,7 +1171,7 @@ resource "aws_subnet" "foo" { | |||
resource "aws_subnet" "bar" { | |||
vpc_id = "${aws_vpc.foo.id}" | |||
cidr_block = "192.168.16.0/20" | |||
availability_zone = "us-west-2b" |
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.
@ryndaniels FYI looks like there was one last one 😄
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! |
Community Note
Reference #7873
Release note for CHANGELOG:
Output from acceptance testing: