terraform-aws-modules / terraform-aws-dms

Terraform module to create AWS DMS (Database Migration Service) resources 🇺🇦
https://registry.terraform.io/modules/terraform-aws-modules/dms/aws
Apache License 2.0
60 stars 93 forks source link

Add a possibility to create multiple instances (using type `map`) #30

Closed antonbabenko closed 1 year ago

antonbabenko commented 1 year ago

Is your request related to a problem? Please describe.

Create from 0 to many aws_dms_replication_instance resources defined as a map. All resources should be created conditionally.

Describe alternatives you've considered.

My friend has forked this module to add this functionality. Not good :)

Additional context

Friends don't want friends to fork terraform-aws-modules. :)

bryantbiggs commented 1 year ago

hmm, thats going to get quite complex. I'm curious to hear the use case - is this one centralized instance of DMS which is running multiple tasks? If so, DMS recommends to setup individual instances that associate 1 to a few replication tasks with a single instance. For example, if you have a bunch of databases and you need to run 30 tasks - you'd probably look at instantiating this module 10-15 times to create 10-15 different instances that map back to 2-3 replication tasks (varies based on load, etc.)

bryantbiggs commented 1 year ago

In other words - its not a clustering construct like Kubernetes, ECS, Autoscaling group where you just continue to add more compute to keep supporting a growing number of replication tasks

antonbabenko commented 1 year ago

I ask @oleksandrdohtiev to provide all the details here since he has been working on this and he's been asking for this feature request.

oleksandrdohtiev commented 1 year ago

@bryantbiggs Currently when we call terraform-aws-dms module we can create only one replication instance at once, but meanwhile we can create multiple endpoints, replication tasks and event subscriptions. Because they have type map. If we reconfigure type to map for replication instance we will be able to create multiple replication instances as well. Also now we can not create replication tasks without creating replication instance. But sometimes we would like to create replication tasks without creating new replication instance, for example if we want to use already existing replication instances for new tasks. It would be better to remove this strict dependency.

bryantbiggs commented 1 year ago

If we reconfigure type to map for replication instance we will be able to create multiple replication instances as well.

A module should create one "construct" since we now have the ability to for_each over a module to create multiples of that "construct". Here, that construct is a DMS replication instance which per the AWS API supports multiple endpoints, tasks, etc. This is similar to an IAM role module where the module creates just one role but users can add/attach 0 or more policies, etc. to the role and if they wish to mass produce roles, they can use for_each at the module level

Also now we can not create replication tasks without creating replication instance. But sometimes we would like to create replication tasks without creating new replication instance, for example if we want to use already existing replication instances for new tasks. It would be better to remove this strict dependency.

If I read this correctly, you are saying you want to reference a replication instance that was NOT created by Terraform? This should be possible to support by adding a variable like var.create_replication_instance which defaults to true and an associated variable var.replication_instance_arn; however, I don't know that we will be able to support this external replication instance with event subscriptions

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has been open 30 days with no activity. Remove stale label or comment or this issue will be closed in 10 days

github-actions[bot] commented 1 year ago

This issue was automatically closed because of stale in 10 days

github-actions[bot] commented 1 year ago

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.