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

provider/aws: Add AWS RDS Read Replica #1946

Merged
merged 6 commits into from
May 21, 2015
Merged

Conversation

catsby
Copy link
Contributor

@catsby catsby commented May 13, 2015

My proposal for adding support to for read replicas to resource_aws_db, by way of adding a replicate_source_db attribute, referencing the identifier of the source database.

Caveats:

  • Replicas inherit many of their attributes from the primary database (username, password, engine). Many of these are required when creating a database instance, but not when creating a replica. The trouble there is if being a replica is just a flag on a resource, that resource still needs those required attributes, even if they're ignored (inherited) from the source. Until Terraform has a ValidateFunc or RequiredIf concept(s), we're stuck repeating ourselves.
  • Replicas can be promoted to full, standalone databases. Terraform can support this by sending a PromoteReadReplica API call when the replicate_source_db attribute is removed in the configuration. This may be a surprise to some users, since we can't notify them that "removing this attribute makes a full read/write database" during plan.
  • If you use the example config below and let TF do the inheritance of attributes, you may get in a weird state when you promote. Manual managing of the config file will be required if you need to separate the two database more.

Example config:

resource "aws_db_instance" "primary" {
  identifier = "tf-primary-db"
  allocated_storage = 5
  engine = "mysql"
  engine_version = "5.6.19b"
  instance_class = "db.t1.micro"
  name = "baz"
  password = "fofofofxx"
  username = "foo"
  multi_az = false
  apply_immediately = true
  backup_retention_period = 1
  tags {
    Name = "tf-primary-db"
  }
  parameter_group_name = "default.mysql5.6"
}

resource "aws_db_instance" "replica" {
  identifier = "tf-replica-db"
  replicate_source_db = "${aws_db_instance.primary.identifier}"
  allocated_storage = "${aws_db_instance.primary.allocated_storage}"
  engine = "${aws_db_instance.primary.engine}"
  engine_version = "${aws_db_instance.primary.engine_version}"
  instance_class = "${aws_db_instance.primary.instance_class}"
  password = "${aws_db_instance.primary.password}"
  username = "${aws_db_instance.primary.username}"
  tags {
    Name = "tf-replica-db"
  }
}

In practice, users may want to manually duplicate all those fields. In the implementation, we'll have to ignore most of them that don't apply.

The fields that are applicable to replica creation:

AutoMinorVersionUpgrade *bool 
AvailabilityZone *string
DBInstanceClass *string
DBInstanceIdentifier *string
DBSubnetGroupName *string
IOPS *int64
OptionGroupName *string
Port *int64
PubliclyAccessible *bool
SourceDBInstanceIdentifier *string
StorageType *string
Tags []*Tag

Relevant docs:

Things:

  • Docs (mostly)
  • Example config
  • Implementation
  • Acceptance tests

I welcome thoughts on this approach

@catsby
Copy link
Contributor Author

catsby commented May 13, 2015

Implementation is (mostly) done. You can create, destroy, and promote a replica. To promote, simply remove the replicate_source_db attribute in the config, that will send a PromoteReadReplica API call and sever the replication link.

The acceptance test is incomplete, but I wanted to see if anyone had feedback on implementation / usage.

@catsby
Copy link
Contributor Author

catsby commented May 13, 2015

cc @radeksimko, @ryanking as at least 2 people who I know requested this in some fashion 😄

@ryanking
Copy link
Contributor

Interface lgtm.

@radeksimko
Copy link
Member

Overall looks good, I have not been looking at the API docs yet, but since we gonna be able to build a M-S-S RDS cluster, it would be nice to test typical scenarios, like engine upgrade.

I know that RDS does some clever things like "upgrade each slave separately first", then upgrade master, while promoting one of the slaves to master = no downtime caused. I'm not sure if all this happens under the hood automatically, I haven't tried that yet, but it's a typical scenario that should work well.

@catsby
Copy link
Contributor Author

catsby commented May 14, 2015

I know that RDS does some clever things like "upgrade each slave separately first", then upgrade master, while promoting one of the slaves to master = no downtime caused.

Wow, is this process documented somewhere? I didn't read the docs entirely so I may have just missed it. Seems like promoting a follower would be a bad thing, since it has different address. Unless you're talking about their HA setup (multi_az), which is a bit different; they'll swap out DNS internally in that case.

What are you thinking I need to do here to accommodate?

@radeksimko
Copy link
Member

What are you thinking I need to do here to accommodate?

Like I mentioned, I did not actually experienced this, I've just read it somewhere, can't remember where, sorry...

What I was rather trying to suggest is to battle test this in situations, when you upgrade certain things. Just make sure that the most basic expectable upgrade paths are working.

@catsby catsby changed the title provider/aws: Add docs for AWS RDS Read Replicate provider/aws: Add AWS RDS Read Replicate May 19, 2015
@catsby
Copy link
Contributor Author

catsby commented May 20, 2015

Rebased on master and added an acceptance test.
I apologize in advance for time spent running that acceptance test... creating primary, backup primary, create replica, test things, then destroy primary, destroy replica ... it takes a long time.

@phinze please review when you get a chance.

more information on using Replication.


~> **NOTE:** Removing the `relicate_source` attribute from an existing RDS
Copy link
Member

Choose a reason for hiding this comment

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

Oops, typo here - relicate_source => replicate_source

@radeksimko
Copy link
Member

Overall LGTM. 👍

make testacc TEST=github.com/hashicorp/terraform/builtin/providers/aws TESTARGS='-run=DBInstance' 2>/dev/null
go generate ./...
TF_ACC=1 go test github.com/hashicorp/terraform/builtin/providers/aws -v -run=DBInstance -timeout 45m
=== RUN TestAccAWSDBInstance
--- PASS: TestAccAWSDBInstance (618.04s)
=== RUN TestAccAWSDBInstanceReplica
--- PASS: TestAccAWSDBInstanceReplica (1620.66s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    2238.710s

yet I haven't actually tried building a replicated RDS outside of the test suite.

This resource should make decent amount of people happier. 😄

@radeksimko radeksimko changed the title provider/aws: Add AWS RDS Read Replicate provider/aws: Add AWS RDS Read Replica May 21, 2015
remove typos, debugging, and try spelling things correctly
@@ -65,6 +66,17 @@ The following arguments are supported:
* `apply_immediately` - (Optional) Specifies whether any database modifications
are applied immediately, or during the next maintenance window. Default is
`false`. See [Amazon RDS Documentation for more for more information.](http://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/Overview.DBInstance.Modifying.html)
* `replicate_source` - (Optional) Specifies that this resource is a Replicate
Copy link
Contributor

Choose a reason for hiding this comment

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

It's replicate_source_db in code and replicate_source here.

@phinze
Copy link
Contributor

phinze commented May 21, 2015

One wee docs thing, but otherwise LGTM!

catsby added a commit that referenced this pull request May 21, 2015
provider/aws: Add AWS RDS Read Replica
@catsby catsby merged commit ec06e81 into master May 21, 2015
@catsby catsby deleted the f-aws-db-read-replicas branch May 21, 2015 20:14
@ghost
Copy link

ghost commented May 2, 2020

I'm going to lock this issue 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 similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators May 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants