-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Support Valkey upgrades on elasticache_global_replication_group #42636
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
base: main
Are you sure you want to change the base?
Support Valkey upgrades on elasticache_global_replication_group #42636
Conversation
Community GuidelinesThis comment is added to every new Pull Request to provide quick reference to how the Terraform AWS Provider is maintained. Please review the information below, and thank you for contributing to the community that keeps the provider thriving! 🚀 Voting for Prioritization
Pull Request Authors
|
✅ Thank you for correcting the previously detected issues! The maintainers appreciate your efforts to make the review process as smooth as possible. |
08cf9f2
to
65c5a1a
Compare
One lint issue left would appreciate a second pair of eyes on this PR @justinretzolk |
whoops. from a merge conflict. I will fix and rebase on latest again |
Once a replication group is added to a global datastore, all of it's engine upgrades need to be controlled on the global datastore. This resource did not support setting an explicit engine value on global datastores, preventing upgrades (hashicorp#41618). Further, if creating a Valkey global datastore by inheriting version from the primary (or manually upgrading the engine of the global datastore in AWS Console), then you could not continue manage the secondary replication groups in terraform. A Valkey engine would cause an incorrect destroy & re-create plan (if using defaults for engine) or a conflict (if trying to set the engine correctly). We remove the default engine value, mark it as computed and add DiffSuppressFunc to engine, engine_version and parmeter_group_name to ignore changes to these attributes in plans when the resource belongs to a global datastore (primary or secondary). Fixes hashicorp#40786 and hashicorp#41713. This also has the benefit of removing the need to add explicit ignore_changes lifecycle blocks to attempt to work around these plans. We update existing documentation on around this.
This is redis6.x specific. It has nothing to do with my DiffSuppressFunc changes or removing the lifecycle { ignore_changes } blocks. Still needed to pass tests :) ``` Step 2/2 error running import: ImportStateVerify attributes not equivalent. Difference is shown below. The - symbol indicates attributes missing after import. map[string]string{ - "engine_version": "6.x", + "engine_version": "6.2", } ```
65c5a1a
to
31697b9
Compare
Once a replication group is added to a global datastore, all of it's engine upgrades need to be controlled on the global datastore. This resource did not support setting an explicit engine value on global datastores, preventing upgrades (#41618).
Further, if creating a Valkey global datastore by inheriting version from the primary (or manually upgrading the engine of the global datastore in AWS Console), then you could not continue manage the secondary replication groups in terraform. A Valkey engine would cause an incorrect destroy & re-create plan (if using defaults for engine) or a conflict (if trying to set the engine correctly). We remove the default engine value, mark it as computed and add DiffSuppressFunc to engine, engine_version and parmeter_group_name to ignore changes to these attributes in plans when the resource belongs to a global datastore (primary or secondary). Fixes #40786 and #41713.
This also has the benefit of removing the need to add explicit ignore_changes lifecycle blocks to attempt to work around these plans. We update existing documentation on around this.
Rollback Plan
If a change needs to be reverted, we will publish an updated version of the library.
Revert the PR. Changes meant to be backwards compatible.
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.
No.
Description
Relations
References
Output from Acceptance Testing
I ran my new tests that cover each of the issues and I sampled through a subset of existing tests (these are long).