Add new data source batch_compute_environment #4270
Conversation
bflad
added
new-data-source
| return err | ||
| } | ||
|
|
||
| for _, computeEnvironment := range desc.ComputeEnvironments { |
There was a problem hiding this comment.
We should check for response errors first to see if there are multiple matches and for some reason no matches (that didn't trigger an API error), e.g.
if len(desc.ComputeEnvironments) == 0 {
return fmt.Errorf("no matches found for name: %s", name)
}
if len(desc.ComputeEnvironments) > 1 {
return fmt.Errorf("multiple matches found for name: %s", name)
}Then we can simplify the code to remove the for loop:
computeEnvironment := desc.ComputeEnvironments[0]
d.SetId(aws.StringValue(computeEnvironment.ComputeEnvironmentArn))
// ...There was a problem hiding this comment.
yes, that's fair. I was basing this on data_source_aws_ecs_cluster.go which would benefit from the same change.
| { | ||
| Config: testAccCheckAwsBatchComputeEnvironmentDataSourceConfig, | ||
| Check: resource.ComposeTestCheckFunc( | ||
| resource.TestCheckResourceAttr("data.aws_batch_compute_environment.default", "type", "MANAGED"), |
There was a problem hiding this comment.
For testing data sources, its generally easier to compare the attributes returned by the resource and the datasource against each other. Check out how this is done with the aws_sqs_queue data source for a pretty simple example:
There was a problem hiding this comment.
Thanks, I've updated the test and based it on data_source_aws_sqs_queue_test
| "github.com/hashicorp/terraform/helper/resource" | ||
| ) | ||
|
|
||
| func TestAccAWSBatchDataSource_ecsCluster(t *testing.T) { |
There was a problem hiding this comment.
Minor naming nitpick: I'd rename this to TestAccAWSBatchComputeEnvironmentDataSource_basic
| } | ||
|
|
||
| data "aws_batch_compute_environment" "default" { | ||
| compute_environment_name = "${aws_batch_compute_environment.sample.name}" |
There was a problem hiding this comment.
This is currently failing testing:
=== RUN TestAccAWSBatchDataSource_ecsCluster
--- FAIL: TestAccAWSBatchDataSource_ecsCluster (3.65s)
testing.go:518: Step 0 error: Error planning: 1 error(s) occurred:
* data.aws_batch_compute_environment.default: 1 error(s) occurred:
* data.aws_batch_compute_environment.default: Resource 'aws_batch_compute_environment.sample' does not have attribute 'name' for variable 'aws_batch_compute_environment.sample.name'
|
@bflad thanks for the feedback. Please let me know if there are any more issues. |
|
I've updated data_source_aws_ecs_cluster accordingly, see #4286 |
| func testAccDataSourceAwsBatchComputeEnvironmentConfig(rName string) string { | ||
| return fmt.Sprintf(` | ||
| resource "aws_iam_role" "ecs_instance_role" { | ||
| name = "ecs_instance_role" |
There was a problem hiding this comment.
It looks like the names need to be randomized as well. Simply passing "%[1]s" to these (IAM role name, instance profile name, security group name, etc) so they get rName should be sufficient.
--- FAIL: TestAccDataSourceAwsBatchComputeEnvironment (5.65s)
testing.go:518: Step 0 error: Error applying: 3 error(s) occurred:
* aws_iam_role.ecs_instance_role: 1 error(s) occurred:
* aws_iam_role.ecs_instance_role: Error creating IAM Role ecs_instance_role: EntityAlreadyExists: Role with name ecs_instance_role already exists.
status code: 409, request id: 94b460c1-44c7-11e8-a015-65a54b838417
* aws_iam_role.aws_batch_service_role: 1 error(s) occurred:
* aws_iam_role.aws_batch_service_role: Error creating IAM Role aws_batch_service_role: EntityAlreadyExists: Role with name aws_batch_service_role already exists.
status code: 409, request id: 94d863a2-44c7-11e8-9265-15fd1e8827db
* aws_security_group.sample: 1 error(s) occurred:
* aws_security_group.sample: Error creating Security Group: InvalidGroup.Duplicate: The security group 'aws_batch_compute_environment_security_group' already exists for VPC 'vpc-6290a707'
status code: 400, request id: b22f1f6f-a782-4d9b-8633-4899bfd1d7a5
|
This has been released in version 1.16.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. |
|
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! |
ghost
locked and limited conversation to collaborators
No description provided.