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.

Sign up for GitHub

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

feat: allow the creation of cross-region rds replicas #346

Merged
merged 5 commits into from
Jul 7, 2021
Merged

feat: allow the creation of cross-region rds replicas #346

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b82c831
fix to allow the creation of cross-region rds replicas
riccardo-salamanna Jun 28, 2021
ae38512
refactored cross region replica condition and added postgres cross re…
riccardo-salamanna Jul 1, 2021
8466b96
fixed the postgres example for cross region with multiple vpc
riccardo-salamanna Jul 1, 2021
3bcc9f3
Apply suggestions from code review
antonbabenko Jul 7, 2021
ef97577
Fixed example and docs
antonbabenko Jul 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@ Users have the ability to:
| <a name="input_publicly_accessible"></a> [publicly\_accessible](#input\_publicly\_accessible) | Bool to control if instance is publicly accessible | `bool` | `false` | no |
| <a name="input_random_password_length"></a> [random\_password\_length](#input\_random\_password\_length) | (Optional) Length of random password to create. (default: 10) | `number` | `10` | no |
| <a name="input_replicate_source_db"></a> [replicate\_source\_db](#input\_replicate\_source\_db) | Specifies that this resource is a Replicate database, and to use this value as the source database. This correlates to the identifier of another Amazon RDS Database to replicate. | `string` | `null` | no |
| <a name="input_cross_region_replica"></a> [cross\_region\_replica](#input\_cross\_region\_replica) | Specifies if the replica should be cross region. It allows the use of a subnet group in a region different than the master instance | `string` | `false` | no |
| <a name="input_restore_to_point_in_time"></a> [restore\_to\_point\_in\_time](#input\_restore\_to\_point\_in\_time) | Restore to a point in time (MySQL is NOT supported) | `map(string)` | `null` | no |
| <a name="input_s3_import"></a> [s3\_import](#input\_s3\_import) | Restore from a Percona Xtrabackup in S3 (only MySQL is supported) | `map(string)` | `null` | no |
| <a name="input_skip_final_snapshot"></a> [skip\_final\_snapshot](#input\_skip\_final\_snapshot) | Determines whether a final DB snapshot is created before the DB instance is deleted. If true is specified, no DBSnapshot is created. If false is specified, a DB snapshot is created before the DB instance is deleted, using the value from final\_snapshot\_identifier | `bool` | `false` | no |
Expand Down
2 changes: 1 addition & 1 deletion main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
locals {
master_password = var.create_db_instance && var.create_random_password ? random_password.master_password[0].result : var.password
db_subnet_group_name = var.replicate_source_db != null ? null : coalesce(var.db_subnet_group_name, module.db_subnet_group.db_subnet_group_id, var.identifier)
db_subnet_group_name = var.cross_region_replica != true && var.replicate_source_db != null ? null : coalesce(var.db_subnet_group_name, module.db_subnet_group.db_subnet_group_id, var.identifier)
Copy link
Member

Choose a reason for hiding this comment

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

@bryantbiggs local.db_subnet_group_name does not seem to be used in module db_subnet_group on line 24. Is it correct?

Copy link
Member

Choose a reason for hiding this comment

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

correct - we only create the subnet group when create_db_subnet_group = true and the name is either db_subnet_group_name or identifier.

then, based on the logic on line 3 - we provide the subnet group name to the instance itself

terraform-aws-rds/main.tf

Line 89 in e2636f9

db_subnet_group_name = local.db_subnet_group_name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah the issue is that if want to create a read only replica, db_subnet_group_name defaults to null, which is necessary for same region replicas but it is not correct for cross-region (different vpc, different subnets etc etc). I proposed the use of a different variable so that we can override that behaviour, and at the same time allow the use of an existing subnet group or the creation of a new one with the modukle, preserving the functionality of the module itself as is. Thank you :)

Copy link
Member

Choose a reason for hiding this comment

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

@bryantbiggs WDYT? yes/no? :)

Copy link
Member

Choose a reason for hiding this comment

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

I think this is ok - maybe we could change var.cross_region_replica != true && ... to !var.cross_region_replica && ...

we could also update the replica examples to add an instance in another region to ensure this works (and continues to work as intended with any future changes)

terraform-aws-rds/examples/replica-postgres/main.tf

Line 112 in e2636f9

module "replica" {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm willing to do that for you if that is ok :)


parameter_group_name_id = var.create_db_parameter_group ? module.db_parameter_group.db_parameter_group_id : var.parameter_group_name

Expand Down
6 changes: 6 additions & 0 deletions variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ variable "replicate_source_db" {
default = null
}

variable "cross_region_replica" {
description = "Specifies if the replica should be cross region"
antonbabenko marked this conversation as resolved.
Show resolved Hide resolved
type = string
antonbabenko marked this conversation as resolved.
Show resolved Hide resolved
default = false
}

variable "license_model" {
description = "License model information for this DB instance. Optional, but required for some DB engines, i.e. Oracle SE1"
type = string
Expand Down