hashicorp / terraform-aws-consul

A Terraform Module for how to run Consul on AWS using Terraform and Packer
Apache License 2.0
401 stars 484 forks source link

Add variable for ASG service role #94

Closed briandbecker closed 5 years ago

briandbecker commented 5 years ago
briandbecker commented 5 years ago

I used it as part of a test consul cluster build. Working on getting setup for the test framework but figured I would get feedback first.

briandbecker commented 5 years ago

@brikis98 I added a new test passing specifically for this use

brikis98 commented 5 years ago

Got a test failure:

TestConsulClusterWithCustomASGRoleUbuntuAmi 2018-11-01T02:35:30Z command.go:98: 
TestConsulClusterWithCustomASGRoleUbuntuAmi 2018-11-01T02:35:30Z command.go:98: Error: Error applying plan:
TestConsulClusterWithCustomASGRoleUbuntuAmi 2018-11-01T02:35:30Z command.go:98: 
TestConsulClusterWithCustomASGRoleUbuntuAmi 2018-11-01T02:35:30Z command.go:98: 1 error(s) occurred:
TestConsulClusterWithCustomASGRoleUbuntuAmi 2018-11-01T02:35:30Z command.go:98: 
TestConsulClusterWithCustomASGRoleUbuntuAmi 2018-11-01T02:35:30Z command.go:98: * aws_iam_service_linked_role.consul_asg_role: 1 error(s) occurred:
TestConsulClusterWithCustomASGRoleUbuntuAmi 2018-11-01T02:35:30Z command.go:98: 
TestConsulClusterWithCustomASGRoleUbuntuAmi 2018-11-01T02:35:30Z command.go:98: * aws_iam_service_linked_role.consul_asg_role: Error creating service-linked role with name autoscaling.amazonaws.com: InvalidInput: Service role name AWSServiceRoleForAutoScaling_test-consul-service-linked-role has been taken in this account, please try a different suffix.
TestConsulClusterWithCustomASGRoleUbuntuAmi 2018-11-01T02:35:30Z command.go:98:     status code: 400, request id: 8d368b9c-dd7e-11e8-9283-5d3e9cab26f7

@briandbecker I'm on a flight, so haven't had time to dig in, but if the error is obvious to you, let me know!

briandbecker commented 5 years ago

I do see the issue. I'm going to make a change to add a timestamp on the end of the aws_iam_service_linked_role custom_suffix to ensure uniqueness. Since this is merged I'll do a new PR.

Apologies...

brikis98 commented 5 years ago

No worries at all! Thank you for the contribution and follow-up fix!