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: WIP Docs for RDS Cluster, Cluster Instance (Aurora) #2935

Merged
merged 5 commits into from
Oct 7, 2015

Conversation

catsby
Copy link
Contributor

@catsby catsby commented Aug 4, 2015

Adds RDS Cluster and RDS Cluster Instance resources (Amazon Aurora).

Usage:

resource "aws_rds_cluster_instance" "cluster_instances" {
  count = 2
  instance_identifier = "aurora-cluster-demo"
  cluster_identifer = "${aws_rds_cluster.default.id}"
  instance_class = "db.r3.large"
}

resource "aws_rds_cluster" "default" {
  cluster_identifier = "aurora-cluster-demo"
  availability_zones = ["us-west-2a","us-west-2b","us-west-2c"]
  database_name = "mydb"
  master_username = "foo"
  master_password = "bar"
}

Scaling can be done with the lifecycle block:

resource "aws_rds_cluster" "default" {
  cluster_identifier = "tf-aurora-cluster"
  availability_zones = ["us-west-2a","us-west-2b","us-west-2c"]
  database_name = "mydb"
  master_username = "foo"
  master_password = "mustbeeightcharaters"

  lifecycle {
    create_before_destroy = true
  }
}

resource "aws_rds_cluster_instance" "replicas" {
  count = 2
  cluster_identifier = "${aws_rds_cluster.default.id}"
  # can change this to add new instances to the cluster
  # which will join and sync before the existing ones are destroyed
  instance_class = "db.r3.xlarge"

  lifecycle {
    create_before_destroy = true
  }
}

Noteworthy:

  • The cluster is it's own resource, and does not require any actual database instances
  • Without any instances, a cluster doesn't show up in the RDS Console
  • With Aurora, there is no designation of primary or replica instances. You simply add instances, and Aurora sorts that out. This has a bit of auto-promotion included, if the current primary is unavailable or destroyed. It also means that it should, in theory, support Terraforms count feature
  • The planned implementation is intentionally limited. There are other knobs we can turn, eventually, but what I've planned offers bare minimum functionality

TODO:

  • Docs
  • Implementation
  • Tests

@stack72
Copy link
Contributor

stack72 commented Aug 17, 2015

@catsby does this PR mean that Terraform can already manage aurora?

@catsby
Copy link
Contributor Author

catsby commented Aug 17, 2015

@stack72 when this PR gets merged, yes. Or you can build based off of the branch if you want, but I put it through some paces with no real troubles 😄

Feedback on the code, or how it works, most welcome!

@stack72
Copy link
Contributor

stack72 commented Aug 17, 2015

@catsby perfect - i'll have a look at it tomorrow. I'm already running my tf version from master so switching branches is fine :)

Will feedback when i test it out

return fmt.Errorf("[WARN] Error waiting for RDS Cluster state to be \"available\": %s", err)
}

// tags := tagsFromMapRDS(d.Get("tags").(map[string]interface{}))
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be cleaned up?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 998749d .... sadly I ripped it out. It turns out you can make tags on creation, but you can't list or update them because it's not documented how to make an ARN for an RDS Cluster 😦 ... opening a support ticket for that.

@dalehamel
Copy link

oh i see you have a PR ! Ignore my other comment :)

@dalehamel
Copy link

@catsby i'm curious why you've taken the approach of making aurora a separate resource, when it's really just a different RDS engine? Unless amazon has a totally different API for this...

@dalehamel
Copy link

Does aurora support parameter groups / option groups? Those are fairly important from an administrative perspective for DBAs

@dalehamel
Copy link

I really like the lifecycle concept if it works as expected. Currently aurora offers the prospect of not needing to take downtime to scale storage, but with 'create before destroy' it would be super cool if you didn't have to take downtime to change instance sizes, which seems to be the case.

@catsby
Copy link
Contributor Author

catsby commented Aug 20, 2015

i'm curious why you've taken the approach of making aurora a separate resource, when it's really just a different RDS engine

It is kind of just a new engine. Mostly. But there exists things like the RDSCluster (not database), which are separate, which is why you see 2 resources in this PR. In the console, you don't see the Cluster being created separately, but there is one behind the curtain.

The DB instances themselves are much more similar, and maybe we could consider scrapping rds_cluster_instance, but I felt that the separation / duplication wasn't that bad at all. You do get attributes like writer and cluster_identifier that only apply to Aurora though. And if it was just bundled inside DB Instance, we'd end up with a lot of if "aurora" type logical check everywhere. Not a huge problem, but ends up adding to the maintenance overhead.

Furthermore, DB Instance requires some fields that an Aurora instance may not, for instance name (or username, password) in DB Instance is for naming the database. While you can do that in an Aurora instance, I believe the best behavior is to define that in the RDS Cluster resource. I may be wrong, so let me know if you find otherwise 😄

An even better example is allocated_storage. You must define this for a DB Instance, but you can't define it for Aurora at all.

Does aurora support parameter groups / option groups? Those are fairly important from an administrative perspective for DBAs

Yes! I have not implemented them yet. They are entirely separate APIs, and I wanted to get a foundation laid for Aurora before tackling those. Better yet, a community member may chip in 😄 .

This is also just a proposal (separate resources vs. piggy backing on db instance). I wanted to get enough out there to 👍 / 👎 the idea, before spending time on those other things.

I do realize those are important, I'm sorry they aren't included here, but we'll get them.

I really like the lifecycle concept if it works as expected [...] didn't have to take downtime to change instance sizes, which seems to be the case

I tested this! In the example above, you can change instance_class to a larger instance, and Terraform will provision them add them to the cluster. When they're in-sync, it destroys the old ones. It was pretty magical ✨

Or course, I had essentially zero data in them, so there may exist a possibility of some weirdness (?). The Cluster uses a shared volume though, so I would think not... again, more ✨

@dalehamel
Copy link

@catsby thanks for the thorough reply!

I'm super pumped, and playing with this branch right now.

Now that I read a bit more, i think the separation of 'cluster' and 'instance' makes a lot of sense here 👍

I'll keep my eyes peeled. I'd love to see this get merged, and would be happy to help get option/parameter group support added :)

@dalehamel
Copy link

Hmm, are you working with VPCs at all? I'm having some weird issues when I try the below code with a VPC that already exists, defined in a remote state:

provider "aws" {
  region = "${var.region}"
}
variable "name" {
  default = "aurora-test"
  description = "Name used as identifier of the instance"
}

variable "port" {
  default = 3306
}

variable "environment" {
  default = "db"
  description = "Name used as identifier of the instance"
}
variable "instance_class" {
  default = "db.r3.xlarge"
}

variable "root_username" {
  default = "root"
}

resource "terraform_remote_state" "vpc" {
  backend = "s3"
  config {
    bucket = "somebucket"
    key = "${var.region}/${var.environment}/vpc"
  }
}

resource "aws_rds_cluster_instance" "cluster_instances" {
  count = 2
  identifier = "${var.name}-${var.environment}-instance"
  cluster_identifier = "${aws_rds_cluster.default.id}"
  instance_class = "${var.instance_class}"
}

resource "aws_rds_cluster" "default" {
  cluster_identifier = "${var.name}-${var.environment}"
  availability_zones = ["us-east-1e","us-east-1b","us-east-1d"]
  database_name = "${replace(var.name, "-", "_")}"
  master_username = "${var.root_username}"
  master_password = "foobar"
  vpc_security_group_ids = [ "${aws_security_group.main.id}" ]
  db_subnet_group_name = "${aws_db_subnet_group.rds_public_subnet_group.name}"
  port = "${var.port}"

}

resource "aws_db_subnet_group" "rds_public_subnet_group" {
  name = "aurora-${var.name}-${var.environment}-${var.region}-rds-public-subnetgroup"
  description = "Public subnets for ${var.environment}-${var.region} RDS instance"
  subnet_ids = ["${split(",", terraform_remote_state.vpc.output.public_subnets)}"]
}

resource "aws_security_group" "main" {
  name = "${var.name}-sg"
  description = "Allow MySQL traffic to ${var.name}"
  vpc_id = "${terraform_remote_state.vpc.output.vpc}"
}

resource "aws_security_group_rule" "allow_vpc" {
  type = "ingress"
  from_port = 3306
  to_port = 3306
  protocol = "tcp"
  security_group_id = "${aws_security_group.main.id}"
  source_security_group_id = "${terraform_remote_state.vpc.output.default_security_group}"
}

Gives me:

* aws_rds_cluster_instance.cluster_instances.1: InvalidParameterCombination: Subnet group default(239106) is different cluster's subnet group id 239104
        status code: 400, request id: [d19da494-4785-11e5-8eb2-4743b37caa47]
* aws_rds_cluster_instance.cluster_instances.0: InvalidParameterCombination: Subnet group default(239107) is different cluster's subnet group id 239104
        status code: 400, request id: [d1749896-4785-11e5-aac5-e7509ee47337]

@catsby
Copy link
Contributor Author

catsby commented Aug 21, 2015

@dalehamel ah, no, I had not. I was able to reproduce with your excellent example, and added db_subnet_group_name attribute to Cluster Instance in 8eeb734 and it works! Here's the .tf file I used:

resource "aws_rds_cluster" "default" {
  cluster_identifier = "tf-aurora-cluster"
  database_name = "mydb"
  master_username = "foo"
  master_password = "mustbeeightcharaters"

  vpc_security_group_ids = [ "${aws_security_group.main.id}" ]

  db_subnet_group_name = "${aws_db_subnet_group.rds_public_subnet_group.name}"

  lifecycle {
    create_before_destroy = true
  }
}

resource "aws_rds_cluster_instance" "bar" {
  count = 1
  cluster_identifier = "${aws_rds_cluster.default.id}"
  instance_class = "db.r3.large"
  db_subnet_group_name = "${aws_db_subnet_group.rds_public_subnet_group.name}"
}

resource "aws_db_subnet_group" "rds_public_subnet_group" {
  name = "aurora-rds-public-subnetgroup"
  description = "Public subnets for RDS instance"
  subnet_ids = [
    "${aws_subnet.main.id}", 
    "${aws_subnet.other.id}", 
    "${aws_subnet.third.id}", 
   ]
}

resource "aws_subnet" "main" {
  vpc_id = "${aws_vpc.foo.id}"
  availability_zone = "us-west-2a"
  cidr_block = "10.0.1.0/24"
}

resource "aws_subnet" "other" {
  vpc_id = "${aws_vpc.foo.id}"
  availability_zone = "us-west-2b"
  cidr_block = "10.0.2.0/24"
}

resource "aws_subnet" "third" {
  vpc_id = "${aws_vpc.foo.id}"
  availability_zone = "us-west-2c"
  cidr_block = "10.0.3.0/24"
}

resource "aws_vpc" "foo" {
  cidr_block = "10.0.0.0/16"
  enable_dns_hostnames = true
  tags {
    Name = "rds-subnet-vpc"
  } 
}

resource "aws_security_group" "main" {
  name = "rds-sg"
  description = "Allow MySQL traffic to rds"
  vpc_id = "${aws_vpc.foo.id}"
}

@dalehamel
Copy link

❤️ @catsby

@catsby
Copy link
Contributor Author

catsby commented Aug 25, 2015

@phinze Also ready for review, but may take more time so whenever you get a moment

@@ -0,0 +1,87 @@
---
layout: "aws"
page_title: "AWS: aws_rds_cluster"
Copy link
Contributor

Choose a reason for hiding this comment

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

aws_rds_cluster_instance

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops, thanks! Fixed in 249f5f7

@thegedge
Copy link
Contributor

I've successfully spun up a cluster and two instances with this PR, but I had trouble connecting to the cluster. I noticed that the issue is that there's no way to set "publicly accessible" to true (verified by manually spinning up a publicly accessible instance), so that's something that should be added to cluster instances (otherwise you can only access them in the VPC or through tunnels).

I'd also recommend supporting some other aws_db_instance attributes on the cluster:

  • tags
  • backups (retention period, preferred window)
  • preferred maintenance windows
  • snapshot-related things (restore from snapshot, final snapshot identifier)

We're looking to play around some more with Aurora, so we're excited to see this one get merged. Thanks, @catsby!

@rosmo
Copy link

rosmo commented Sep 15, 2015

There are some typos in the documentation (identifer vs identifier). Also there is no "instance_identifier" variable (I believe it's just "identifier"?).

@catsby
Copy link
Contributor Author

catsby commented Sep 17, 2015

@rosmo thanks, I cleaned those up in 1d2f5a1

@ravbaba
Copy link

ravbaba commented Sep 24, 2015

@catsby - I've tested it and it works. Is it possible to set the parameter somehow "publicly_accessible"?
It would be really useful.

Thanks

@rosmo
Copy link

rosmo commented Sep 24, 2015

@ravbaba, providing vpc_security_group_ids seems to turn off the publicly_accessible flag.

@ravbaba
Copy link

ravbaba commented Sep 25, 2015

@rosmo I've done it with vpc_security_group_ids, so it works inside VPC so it is exactly what I needed but it would be nice to have a possibility to set the publicly_accessible flag, I know it might be not secure but RDS interface allows you to set it so I think terraform should does the same but it is just my opinion.

@catsby catsby force-pushed the f-aws-rds-cluster branch 2 times, most recently from dce1315 to 7c5f5de Compare October 5, 2015 20:02
@catsby
Copy link
Contributor Author

catsby commented Oct 6, 2015

I've added publicly_accessible and related docs in 53889d7. The logic and conditions to get the default correct according to AWS (see PubliclyAccessible) are complicated and situational, so rather than replicate that locally and try to play catchup to any possible changes, we default to false, which seems like the safer bet.

@phinze take a gander when you can

_, err := conn.DeleteDBCluster(&rds.DeleteDBClusterInput{
DBClusterIdentifier: aws.String(d.Id()),
SkipFinalSnapshot: aws.Bool(true),
// final snapshot identifier
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment a TODO?

@phinze
Copy link
Contributor

phinze commented Oct 6, 2015

@catsby a few minor inline notes. generally LGTM! 👍

@catsby catsby force-pushed the f-aws-rds-cluster branch from 41695bf to 71b1cb1 Compare October 7, 2015 16:27
catsby added a commit that referenced this pull request Oct 7, 2015
provider/aws: RDS Cluster, Cluster Instance (Aurora)
@catsby catsby merged commit e26d3a1 into master Oct 7, 2015
@vjanelle
Copy link

@catsby this doesn't support cluster parameter groups, does it?

@catsby
Copy link
Contributor Author

catsby commented Oct 15, 2015

@vjanelle sorry to say that no, those are not yet included.

dpetzold pushed a commit to clearcare/terraform that referenced this pull request Nov 12, 2015
I needed `db_subnet_group_name` in the rds_cluster resource as well when creating on a non-default VPC.

hashicorp#2935 (comment)
@maxim
Copy link

maxim commented Nov 22, 2015

@catsby I wonder if parameter_group support for individual instances (not cluster parameter_group) is easier. Terraform's aws_db_instance allows you to specify parameter_group_name, but aws_rds_cluster_instance doesn't, aren't they the same old thing? Or is that also a completely different API? This alone would already be very helpful.

@mitchellh mitchellh deleted the f-aws-rds-cluster branch March 22, 2016 19:21
bmcustodio pushed a commit to bmcustodio/terraform that referenced this pull request Sep 26, 2017
@ghost
Copy link

ghost commented Apr 27, 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 Apr 27, 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.