mongodb / terraform-provider-mongodbatlas

Terraform MongoDB Atlas Provider: Deploy, update, and manage MongoDB Atlas infrastructure as code through HashiCorp Terraform
https://registry.terraform.io/providers/mongodb/mongodbatlas
Mozilla Public License 2.0
242 stars 168 forks source link

mongodbatlas_cluster bi_connector state changes on terraform CLI 0.14.2 even without any bi_connector configuration - terraform 14 #364

Closed dbamohsin closed 3 years ago

dbamohsin commented 3 years ago

Terraform CLI and Terraform MongoDB Atlas Provider Version

[go@infra terraform]$ terraform version
Terraform v0.14.2
+ provider registry.terraform.io/hashicorp/mongodbatlas v0.7.0

Terraform Configuration File


provider "mongodbatlas" {
  version = "0.7.0"
}

resource "mongodbatlas_project" "my_project" {
  name   = "test-project"
  org_id = var.organisation_id
}

variable "mongo_cluster_spec_dbe" {
  type = map
  description = "specification for dbe testing cluster - manage difference between environments"
}

# DBE
mongo_cluster_spec_dbe = {
  "node_size" = "M10"
  "backup_enabled" = false
  "provider_backup_enabled" = true
  "disk_autogrow" = true
  "paused" = false
  "mongodb_major_version" = "4.2"
  "primary_region" = "EUROPE_WEST_4"
  "bcp_region" = "WESTERN_EUROPE"
}

# Create Cluster: dbe
resource "mongodbatlas_cluster" "dbe" {
  count                 = var.mongo_create_cluster_map["dbe"] ? 1 : 0
  name                  = "dbe-testing"
  project_id            = mongodbatlas_project.my_project.id
  provider_name         = "GCP"
  cluster_type          = "REPLICASET"

  replication_specs {
      num_shards = 1
      zone_name  = "Zone 1"
      regions_config {
        region_name     = var.mongo_cluster_spec_dbe["primary_region"]
        electable_nodes = 2
        priority        = 7
      }
      regions_config {
        region_name     = var.mongo_cluster_spec_dbe["bcp_region"]
        electable_nodes = 1
        priority        = 6
      }
    }

  # Options which can differ per environment.
  provider_instance_size_name   = var.mongo_cluster_spec_dbe["node_size"]
  pit_enabled                   = var.mongo_cluster_spec_dbe["backup_enabled"]
  provider_backup_enabled       = var.mongo_cluster_spec_dbe["provider_backup_enabled"]
  auto_scaling_disk_gb_enabled  = var.mongo_cluster_spec_dbe["disk_autogrow"]
  mongo_db_major_version        = var.mongo_cluster_spec_dbe["mongodb_major_version"]

  depends_on = [module.mongodb-atlas]

  lifecycle {
    ignore_changes = [disk_size_gb]
    prevent_destroy = false
  }
}

Steps to Reproduce

create cluster without any bi_connector configuration run plan and apply run plan again to see new bi_connector changes appear withou any configuration changes

Expected Behavior

Given that we are not setting the bi_connector and it is an optional setting. We would expect no infrastructure changes to bi_connector.

Actual Behavior

Everytime the configuration is planned or applied, there are teo bi_connector settings which are attempted to set.

# mongodbatlas_cluster.dbe[0] will be updated in-place
  ~ resource "mongodbatlas_cluster" "dbe" {
      ~ bi_connector                            = {
          - "enabled"         = "false" -> null
          - "read_preference" = "secondary" -> null
        }
        id                                      = "xxxxxx"
        name                                    = "dbe-testing"
        # (26 unchanged attributes hidden)

        # (2 unchanged blocks hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Can be worked around by explicitly adding in the following configuration:

  bi_connector = {
        enabled         = false
        read_preference = "secondary"
  }

however shouldnt need to do this as it is an optional setting

Debug Output

Crash Output

Additional Context

References

themantissa commented 3 years ago

@dbamohsin did this work as expected with Terraform 13? We have not tested 14. Add: though looking at this I'd assume it would be the same in 13. Please do note it is not enabling the bi connector and this is likely a timing issue with the state refresh so a bug, but not one that modifying your configuration.

themantissa commented 3 years ago

@nikhil-mongo would you have time to repro this for logs as well and also open a PRODTRIAGE internally? Thank you.

themantissa commented 3 years ago

@dbamohsin I ran with Terraform 13 and cannot seem to reproduce this error. @nikhil-mongo any luck?

dbamohsin commented 3 years ago

Hi @themantissa - the issue appeared with an upgrade to terraform 14.2 it seems. Are there any plans to get the provider supported with 14 anytime soon?

themantissa commented 3 years ago

@dbamohsin it's still in beta so no ETA but will grab it myself and see if I can repro it. I do believe 14 has some new checks so we may seem some artifacts like this as more move to it post GA.

dbamohsin commented 3 years ago

Hi @themantissa - as far as I know, 0.14 is not beta. It was released as GA on 2nd December.

https://github.com/hashicorp/terraform/blob/v0.14.2/CHANGELOG.md

themantissa commented 3 years ago

Well it's official I have too much on my multi-tasking plate - I totally missed the GA ... It's been a long year. With that in mind we'll address issues as they arise until we can get time to do more testing. I'll try to repro to get logs soon or if @nikhil-mongo can do it sooner, that would be appreciated, but if you can include DEBUG level logs (ensuring you redact any info) that helps a great deal.

themantissa commented 3 years ago

@nikhil-mongo in case you get to this first I have a sense this may be due to the new feature in 14 - https://www.hashicorp.com/blog/terraform-0-14-adds-a-new-concise-diff-format-to-terraform-plans but need to test.

nikhil-mongo commented 3 years ago

@themantissa I could repro this with 0.14 terraform. This terraform version is changing the state file of the undefined bi-connector configuration from false->null.

 ~ update in-place

Terraform will perform the following actions:

  # mongodbatlas_cluster.dbe will be updated in-place
  ~ resource "mongodbatlas_cluster" "dbe" {
      ~ bi_connector                            = {
          - "enabled"         = "false" -> null
          - "read_preference" = "secondary" -> null
        }
        id                                      = "Y2x1c3Rlcl9pZA==:NWZkYzRmMTMyYzRmYzUxODZhNmUyYjBm-Y2x1c3Rlcl9uYW1l:ZGJlLXRlc3Rpbmc=-cHJvamVjdF9pZA==:NWZkYzRmMGZmNDQzMGYxZDRlNjE3NjJi-cHJvdmlkZXJfbmFtZQ==:R0NQ"
        name                                    = "dbe-testing"
        # (26 unchanged attributes hidden)

        # (2 unchanged blocks hidden)
    }

But the 0.13 terraform version is not making such change to the terraform state file.

mongodbatlas_project.my_project: Refreshing state... [id=5fdc4f0ff4430f1d4e61762b]
mongodbatlas_cluster.dbe: Refreshing state... [id=Y2x1c3Rlcl9pZA==:NWZkYzRmMTMyYzRmYzUxODZhNmUyYjBm-Y2x1c3Rlcl9uYW1l:ZGJlLXRlc3Rpbmc=-cHJvamVjdF9pZA==:NWZkYzRmMGZmNDQzMGYxZDRlNjE3NjJi-cHJvdmlkZXJfbmFtZQ==:R0NQ]

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

https://www.hashicorp.com/blog/terraform-0-14-adds-a-new-concise-diff-format-to-terraform-plans this has nothing to do as this only brings in the change to show just the changing block rather than the entire resource block.

trace.log

As seen in the trace.log, line 1184 shows the PATCH request with "biConnector": {},.

Also, line 1178, shows the possible cause -

The following problems may be the cause of any confusing errors from downstream operations:
1779       - .bi_connector: new element "enabled" has appeared
1780       - .bi_connector: new element "read_preference" has appeared

Please let me know if I need to file PRODTRIAGE or HELP.

marinsalinas commented 3 years ago

@themantissa @nikhil-mongo, I think we are having this issue because we are using TypeMap with named attributes, Terraform used to tolerate this in order to keep backwards compatibility, for now, the recommendation is either to remove the named attributes in the map and check if the warning goes out or change to this attribute as TypeList with MaxValue=1 like we did with connection_strings but we will have to handle an schema migration.

Actually, AFAIK the new Terraform SDK v2 comes with an internal linter that warns about this kind of practices (using map with nested attributes and other ones) and forces the provider to follow the best practices.

themantissa commented 3 years ago

@nikhil-mongo thank you so much! If you can file a PRODTRIAGE I'll follow up with moving it to the work queue. @marinsalinas this is really helpful - thank you!! We may want to ensure we catch any of the v2 SDK warnings on this and fix those proactively before more upgrade to 14.

Finally thank you @dbamohsin for surfacing this so early in the 14 release and for helping me keep up with the Terraform GA launch :D

nikhil-mongo commented 3 years ago

PRODTRIAGE-1266

themantissa commented 3 years ago

Fixed in v0.9.0