hashicorp / terraform-provider-google

Terraform Provider for Google Cloud Platform
https://registry.terraform.io/providers/hashicorp/google/latest/docs
Mozilla Public License 2.0
2.35k stars 1.75k forks source link

node_config tags shouldn't cause diffs on ordering change #12966

Open tomasgareau opened 2 years ago

tomasgareau commented 2 years ago

Community Note

Description

The google_container_cluster and google_container_node_pool's node_config block should ideally treat tags as an unordered set of tags, rather than an (ordered) list of tags. This would help to avoid unnecessary diffs in case the order of the tags changes.

For example, in the Terraform plan output below, this node pool needs to be replaced because the order of tags changed, even though there are no changes to the set of tags on this node pool:

      # Terraform plan output...

      ~ node_config {
          ~ tags              = [ # forces replacement
                # (4 unchanged elements hidden)
                "tag-a",
              - "tag-c",
                "tag-b",
              + "tag-c",
            ]

      # more Terraform plan...
        }

The main place I've bumped into this is when using the terraform-google-kubernetes-engine module, which sets the tags variable by concatenating a number of input variables. I've had situations where I've had to move tags from applying to "all" node pools vs a specific node pool, and this changed the order of tags built by the module.

New or Affected Resource(s)

Potential Terraform Configuration

resource "google_container_node_pool" "my_pool" {
  # ...
  node_config {
    tags = [
      "a",
      "b",
      "c",
    ]
  }
}

The change below should cause no diffs:

resource "google_container_node_pool" "my_pool" {
  # ...
  node_config {
    tags = [
      "c",
      "a",
      "b",
    ]
  }
}

Considerations

The GKE API spec for NodeConfig models tags as an (ordered) list of strings.

I don't know if it's the GKE API that determines whether that list of tags forces replacement (in which case we're probably out of luck) or whether the Terraform Google provider can determine that the set of tags has not changed, even if the order has.

b/300616557

wyardley commented 3 weeks ago

My changes in https://github.com/GoogleCloudPlatform/magic-modules/pull/12014 partially resolve this - there will be a diff on ordering change, but no permadiff, as the tags will get updated in-place and so the diff will resolve after the first apply

I am guessing running a sort() on the tags could help further minimize this?

roaks3 commented 3 weeks ago

Agreed that @wyardley 's fix might help here, I think we should check back once that is merged. It would depend how deterministic things are behaving. But also, the "real" fix here would be to update the implementation to be order-agnostic.

wyardley commented 3 weeks ago

Thanks, and agree on the "real" fix (not sure whether the API itself has ordering, though I'm guessing not, so it might be enough to use a fixed order within the state representation?).

I did do a functional test locally with the built provider when I commented, and it did fix the permadiff. So probably this is Good Enough (TM) fix.

It does actually try to update the nodepool, but I'm not sure if the actual labels as returned by the API change at all, or if it just updates the representation of it within Terraform state.