terraform-provider-aws icon indicating copy to clipboard operation
terraform-provider-aws copied to clipboard

[WIP] New Resource: aws_rds_reserved_instance

Open brittandeyoung opened this issue 2 years ago • 3 comments

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

...

brittandeyoung avatar Jul 28 '22 13:07 brittandeyoung

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 avatar Jul 28 '22 14:07 brittandeyoung

@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?

YakDriver avatar Jul 28 '22 20:07 YakDriver

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.

brittandeyoung avatar Aug 10 '22 12:08 brittandeyoung

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 avatar Sep 23 '22 14:09 zhelanov

@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.

brittandeyoung avatar Sep 23 '22 18:09 brittandeyoung

@YakDriver Sorry for the delay in getting back to this PR. It has been updated with all recommendations and checks are all passing.

brittandeyoung avatar Sep 26 '22 15:09 brittandeyoung

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)

YakDriver avatar Sep 26 '22 21:09 YakDriver

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

brittandeyoung avatar Sep 29 '22 20:09 brittandeyoung

@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.

brittandeyoung avatar Sep 30 '22 18:09 brittandeyoung

Something is up with the Pull Request Target (All types) / PullRequestComments (pull_request_target) check. False failures

brittandeyoung avatar Sep 30 '22 18:09 brittandeyoung

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!

github-actions[bot] avatar Oct 17 '22 15:10 github-actions[bot]

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.

github-actions[bot] avatar Nov 17 '22 02:11 github-actions[bot]