-
Notifications
You must be signed in to change notification settings - Fork 178
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
replication_specs
do not support deep diff
#1544
Comments
Thanks for opening this issue! Please make sure you've followed our guidelines when opening the issue. In short, to help us reproduce the issue we need:
The ticket INTMDB-1216 was created for internal tracking. |
HI @yulifelimited thanks for reporting this to us. Is this happening only when you try to change the analytics node? Could you provide a more exhaustive information on what changes within replication_specs trigger this behaviour? Also any debug outputs would be appreciated in this case. We want to understand if the issue is within the Terraform script or it's actually the expected behaviour since replication specs might not be edited but only recreated depending on what param you are specifying. |
Any changes in the ExamplesChange to
Change to
There is an ❯ autogrunt state pull > after.json
❯ diff before.json after.json
6c6
< "serial": 12,
---
> "serial": 13,
326c326
< "zone_name": "ZoneName managed by Terraform"
---
> "zone_name": "test"
334c334
< "next_snapshot": "2023-10-19T11:04:19Z",
---
> "next_snapshot": "2023-10-20T23:04:51Z", There is a Gist with a bare terraform module to deploy a M10 cluster to get debug logs: https://gist.github.com/yulifelimited/4c86f3a8b588feebc468cca750457a33#file-debug-log I only tried to change the Thanks |
After some investigation @yulifelimited, I think this is related to the fact that I am going to look at our https://github.com/mongodb/terraform-provider-mongodbatlas/blob/master/CHANGELOG.md because I am spotting some previous cases. My answer right now is that I don't think we plan to change it. The only way we could change it is by moving to a TypeList but I don't think we want that. This TypeSet behaviour is something controlled by Terraform itself. Question: what is this practically implying? I re-produced the scenario (btw thanks for the detailed description) and in the end the update works as expected. |
@marcosuma thanks for looking into it in details. The problem is the noise this generates when reviewing plans, which makes it easy for mistakes to be missed. To give a different perspective, imagine the same changes on a multi-regional cluster, deployed on 4 AWS regions (which was our use case when opening this ticket). We end up with this kind of diff in the plan: Scenario: adding
What you may not see above is that there was an More region, more noise, to the point it may not fit on a single monitor depending on how many nodes are in your cluster. This is a serious problem when you consider the risks associated with unexpected changes to a MongoDB cluster - this can end up with service affecting changes that may take a long time to recover from since we would first have to wait for mongodb.com to finish applying the first set of changes before being able to re-configure the cluster again. |
@yulifelimited let me discuss with the team if we have other options but as I mentioned it doesn't really depend on us. Would it be worth it for you to raise an issue to Terraform? Maybe they can further address this situation. As I mentioned there is not really much we can do here. If we were to switch (back) to schema.TypeList we would end up with a worse situation because order mattered in that case so you would find yourself with undesired changes just because perhaps the Atlas Admin API returned the object in a different order. |
@yulifelimited I created an internal issue to track of this work: https://jira.mongodb.org/browse/INTMDB-1234 We decided we should migrate from TypeSet to TypeList similarly to what we did for
Closing this issue for now as this is not a bug but a FR and we'll track this internally. Lastly I want to thank you for reporting this to us and help us improving our code base. |
Thanks Marco! It is appreciated you are able to sort this out. If |
@yulifelimited yes you can indeed migrate it to the |
Hello!
Changes to
replication_specs
inmongodbatlas_cluster
always show the entirereplication_specs
as having to be re-created instead of a diff on individual properties that have changed.Terraform CLI and Terraform MongoDB Atlas Provider Version
We use terragrunt, but that won't change the output.
Terraform Configuration File
Those are excerpts from our Dedicated terraform modules. The exact config doesn't matter so much, you can replicate by making any changes to
replication_specs
with any cluster configuration.main.tf
variables.tf
Steps to Reproduce
Terraform a MongoDB cluster using
replication_specs
. Make a change to any of thereplication_specs
nested values, and the entirereplication_specs
will show as being re-created in the terraform plan. This is further emphasised the more complex configuration you use in thereplication_specs
(such as a multi-regional cluster with RO and/or Analytics nodes)The changes below remove an
analytics_node
from the cluster:variables
Expected Behavior
Deep diff highlights the exact changes to the
analytics_nodes
. Quick and easy review (region_name
should always be displayed to identify the relevant region)plan
Actual Behavior
Entire
replication_specs
show as being re-created.plan
The text was updated successfully, but these errors were encountered: