Skip to content
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

New Resource: aws_rds_reserved_instance #26025

Merged
merged 28 commits into from
Oct 8, 2022

Conversation

brittandeyoung
Copy link
Collaborator

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

...

@github-actions github-actions bot added provider Pertains to the provider itself, rather than any interaction with AWS. service/rds Issues and PRs that pertain to the rds service. needs-triage Waiting for first response or review from a maintainer. size/L Managed by automation to categorize the size of a PR. documentation Introduces or discusses updates to documentation. labels Jul 28, 2022
@brittandeyoung
Copy link
Collaborator Author

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.

@github-actions github-actions bot added size/XL Managed by automation to categorize the size of a PR. and removed size/L Managed by automation to categorize the size of a PR. labels Jul 28, 2022
@YakDriver
Copy link
Member

YakDriver commented Jul 28, 2022

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

Copy link
Member

@bschaatsbergen bschaatsbergen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work 👍 👀

@brittandeyoung
Copy link
Collaborator Author

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.

@justinretzolk justinretzolk added new-resource Introduces a new resource. and removed needs-triage Waiting for first response or review from a maintainer. labels Aug 22, 2022
@YakDriver YakDriver self-assigned this Sep 16, 2022
@zhelanov
Copy link

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

@brittandeyoung
Copy link
Collaborator Author

@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 brittandeyoung changed the title [WIP] New Resource: aws_rds_reserved_instance New Resource: aws_rds_reserved_instance Sep 26, 2022
@brittandeyoung
Copy link
Collaborator Author

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

@YakDriver
Copy link
Member

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)

@github-actions github-actions bot added tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. and removed tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Sep 29, 2022
@brittandeyoung
Copy link
Collaborator Author

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

@github-actions github-actions bot added the tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. label Sep 30, 2022
@brittandeyoung
Copy link
Collaborator Author

@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
Copy link
Collaborator Author

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

brittandeyoung and others added 21 commits October 7, 2022 14:26
* Added website documentation for the `aws_rds_reserved_instance` resource.
* remove commented code for update functions as this type of resource cannot be updated.
* updated resource deletion warning wording
* updated resource deletion warning to be bold
* Add update function to support updating tags
* Amend `offering_id` field to ForceNew
* Amend reserved instance resource constants
* Amend rds const for tags
* Amend Docs to better match examples
* Amend imports to work with new changes to main
* Amend resource schema field recurring_charges to TypeSet
* Created flattening function for recurring charges schema item
* update start_time field to convert time value to string
* Added PR Change Log File
Add the datasource for reserved instance offerings.
Add tests for `aws_rds_reserved_instance` resource
Added the website documentation for the `aws_rds_reserved_instance_offering` datasource.
updated the `aws_rds_reserved_instance` docs to use the newly created datasource for reserved instance offerings
Resolve terraform parsing issue by using number as string
Update the change log to also include the new data source
* remove string conversion of variable
Copy link
Member

@YakDriver YakDriver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! 🎉

@YakDriver YakDriver merged commit f2bd7ee into hashicorp:main Oct 8, 2022
@github-actions github-actions bot added this to the v4.35.0 milestone Oct 8, 2022
@github-actions
Copy link

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
Copy link

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 github-actions bot locked as resolved and limited conversation to collaborators Nov 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. new-resource Introduces a new resource. provider Pertains to the provider itself, rather than any interaction with AWS. service/rds Issues and PRs that pertain to the rds service. size/XL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for RDS Reserved Instances
5 participants