r/aws_db_instance: Move create/delete/update pending states to their own variables and add storage-optimization

[bflad]

Closes #2408

There are some things that should be noted though, according to the DB Instance Status docs:

• For storage-optimization: "The storage optimization process is usually short, but can sometimes take up to and even beyond 24 hours." Likely this means this will generally hit the timeout. Should this status be moved to the resource.StateChangeConf Target instead?
• Any reason why configuring-enhanced-monitoring is not in delete pending states?
• I noticed we don't have statuses like stopping and starting in any of the pending states, should we add them? There might be others too.

Thoughts and opinions on the above are appreciated!

[radeksimko]

Hi @bflad
thanks for the PR! Replies below.

• For storage-optimization: "The storage optimization process is usually short, but can sometimes take up to and even beyond 24 hours." Likely this means this will generally hit the timeout. Should this status be moved to the resource.StateChangeConf Target instead?

Moving it to Target (during creation and updates, not deletion) sounds like a reasonable thing to do, given described circumstances.

• Any reason why configuring-enhanced-monitoring is not in delete pending states?

I can't think of any reason, probably just a new state that popped up recently and nobody noticed. Do you mind adding it?

• I noticed we don't have statuses like stopping and starting in any of the pending states, should we add them? There might be others too.

Sounds like a good idea, please add them. 👍

---

In regards to the code, I have mixed feelings about decoupling the slices from the CRUD. I don't see much value in such decoupling and I'm slightly more inclined to keeping it as is (i.e. within functions), but I won't block the PR on this - just a thought/opinion to consider.

[bflad]

I have mixed feelings about decoupling the slices from the CRUD. I don't see much value in such decoupling and I'm slightly more inclined to keeping it as is (i.e. within functions)

I was on the fence about this, however I was thinking it was nice to:

• Have the documentation link in one place
• Have the states in one place to notice items missing/not changing in all places consistently across the various usages. I think this is easier to maintain when Amazon decides to further refine these.
• At the very least make the slices more readable -- a blob of unalphabetized states across a line or two is harder to quickly understand in my opinion. We could just as easily do the same line breaks for the slice items, but now the function is a lot more LoC when it could just be stuck somewhere else... hence separated into its own variables. 😄

I can change it back though and add the documentation in each of the spots if you wish. I definitely don't want to hold this up. I'll address the other items in the meantime.

[bflad]

PR updated! 🚀

[radeksimko]

LGTM, thanks.

I will merge this after we ship 1.5.0 with MQ support later today - i.e. this bugfix will be part of 1.5.1.

[yelvert]

So when can 1.5.1 be expected to be released? Currently I can't even create Postgres instances with gp2 storage. MySQL instances with gp2 seem to work fine. I think they both go through the storage-optimization state, but MySQL's is almost instantaneous, whereas Postgres' takes forever (currently at 15 minutes and counting).

Here's their configurations, they are almost identical

resource "aws_db_instance" "mysql" {
  count                   = "${var.enabled == "true" ? 1 : 0}"
  identifier              = "mysql-${var.build_name}-${var.build_env}"
  snapshot_identifier     = "${join("", data.aws_db_snapshot.mysql.*.id)}"
  engine                  = "mariadb"
  allocated_storage       = 50
  storage_type            = "gp2"
  engine_version          = "${var.maria_version["full"]}"
  instance_class          = "${var.instance_type}"
  password                = "${local.mysql_password}"
  db_subnet_group_name    = "${join("", aws_db_subnet_group.main.*.name)}"
  parameter_group_name    = "${join("", aws_db_parameter_group.mysql.*.name)}"
  option_group_name       = "${join("", aws_db_option_group.mysql.*.name)}"
  skip_final_snapshot     = true

  tags = "${merge(var.default_tags, map(
    "Name", "${var.name_tag_prefix}-mysql"
  ))}"
}

resource "aws_db_instance" "postgres" {
  count                   = "${var.enabled == "true" ? 1 : 0}"
  identifier              = "postgres-${var.build_name}-${var.build_env}"
  snapshot_identifier     = "${join("", data.aws_db_snapshot.postgres.*.id)}"
  engine                  = "postgres"
  allocated_storage       = 50
  storage_type            = "gp2"
  engine_version          = "${var.postgres_version["full"]}"
  instance_class          = "${var.instance_type}"
  password                = "${local.postgres_password}"
  db_subnet_group_name    = "${join("", aws_db_subnet_group.main.*.name)}"
  parameter_group_name    = "${join("", aws_db_parameter_group.postgres.*.name)}"
  option_group_name       = "${join("", aws_db_option_group.postgres.*.name)}"
  skip_final_snapshot     = true

  tags = "${merge(var.default_tags, map(
    "Name", "${var.name_tag_prefix}-postgres"
  ))}"
}```

[ghost]

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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!