terraform-provider-aws
terraform-provider-aws copied to clipboard
[WIP] New Resource: aws_rds_reserved_instance
Community Note
- Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
- Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request
Closes #8521
Output from acceptance testing:
$ make testacc TESTS=TestAccXXX PKG=ec2
...
Currently trying to think through how to write acc tests for this type of resource as reservations cannot be deleted and are valid for the duration of the specified offering id once created.
@brittandeyoung Thanks for your work on this! After a quick skim, it's looking very promising.
We've also been thinking about how to test. Testing is important but there is precedent for things we simply cannot test. Rather than hold something up indefinitely, we may have to do best effort and rely on the community for reports of problems.
Is this really it for API operations for reserved instances?
There has been some discussion with the provider team about this potentially needing to be written as a framework resource instead of using the standard SDK method. I am currently waiting on direction from the team on this, and will make required modifications when I hear back.
Hi @brittandeyoung , any update regarding the feature? It's awesome and I think that our team is not the only one who's waiting for it
@zhelanov Thank you for the bump on this PR. I have been working on a few other contributions as well and forgot about this one. I will be this updated to resolve the conflicts and have passing tests. Then wait for the provider team to review.
@YakDriver Sorry for the delay in getting back to this PR. It has been updated with all recommendations and checks are all passing.
Here's a regex that can be helpful for finding db instance classes in the tests and docs:
db\.[a-z]{1,2}\d[a-z]?\.[a-z0-9]{0,4}(large|medium|small|micro|nano)
I have added a data source for aws_rds_reserved_instance_offering
to the pull request with passing tests. Working on making the tests for the reservation now.
The cheapest test we will be able to run is the 102$ up front instance offering.
make testacc TESTARGS='-run=TestAccRDSInstanceOffering_' PKG_NAME=internal/service/rds
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/rds/... -v -count 1 -parallel 20 -run=TestAccRDSInstanceOffering_ -timeout 180m
=== RUN TestAccRDSInstanceOffering_basic
=== PAUSE TestAccRDSInstanceOffering_basic
=== CONT TestAccRDSInstanceOffering_basic
--- PASS: TestAccRDSInstanceOffering_basic (11.80s)
PASS
ok github.com/hashicorp/terraform-provider-aws/internal/service/rds 13.670s
@YakDriver I have added the aws_rds_reserved_instance_offering
datasource as recommended with passing tests, and added tests for the aws_rds_reserved_instance
resource that uses the new datasource and will only run when the enviornment variable RUN_RDS_RESERVED_INSTANCE_TESTS
is set to true
.
This test will cost around $102 per run and vary based on the region testing is done in. I do not have an AWS account where I can simply rack up a $102 charge each time I attempt to validate my tests =) . Does the Hashi team have an account where these tests can be validated in?
At this point my code is ready for another review.
Something is up with the Pull Request Target (All types) / PullRequestComments (pull_request_target)
check. False failures
This functionality has been released in v4.35.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.
For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!
I'm going to lock this pull request 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 related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.