Maintain Bigtable cluster order from config

[nickmoorman]

(Note: this PR depends on the changes in #2488. A cleaner diff can be seen here.)

This PR alters the way the clusters for a Bigtable instance are persisted to Terraform state. As part of another change I was working on (hashicorp/terraform-provider-google#4702), I ensured that clusters in the Terraform state stay in the order in which they're defined in the config. But when I pulled in upstream changes, tests started failing due to the changes in hashicorp/terraform-provider-google#4598 / #2409. I attempted to remedy this with the changes in this PR, and everything works in practice, but I ran into a roadblock for which I'm not sure there's a good solution. When I run the acceptance tests in terraform-provider-google for this resource, I get a single test failure:

--- FAIL: TestAccBigtableInstance_cluster (10.65s)
    testing.go:569: Step 2 error: ImportStateVerify attributes not equivalent. Difference is shown below. Top is actual, bottom is expected.
        
        (map[string]string) (len=4) {
         (string) (len=20) "cluster.2.cluster_id": (string) (len=20) "tf-test-qjp0q3s3g0-d",
         (string) (len=14) "cluster.2.zone": (string) (len=13) "us-central1-f",
         (string) (len=20) "cluster.3.cluster_id": (string) (len=20) "tf-test-qjp0q3s3g0-c",
         (string) (len=14) "cluster.3.zone": (string) (len=13) "us-central1-c"
        }
        
        
        (map[string]string) (len=4) {
         (string) (len=20) "cluster.2.cluster_id": (string) (len=20) "tf-test-qjp0q3s3g0-c",
         (string) (len=14) "cluster.2.zone": (string) (len=13) "us-central1-c",
         (string) (len=20) "cluster.3.cluster_id": (string) (len=20) "tf-test-qjp0q3s3g0-d",
         (string) (len=14) "cluster.3.zone": (string) (len=13) "us-central1-f"
        }

I tracked the failure down to the fact that the Bigtable Admin API doesn't always save the clusters in the same order they were in at creation time, and ImportStateVerify tests ignore CustomizeDiff customizations, meaning that order does matter for such tests. I'm not sure if there's a good way to get around this, or if I should just abandon these changes and the attempt to keep clusters in config order...

cc @rileykarson

Release Note Template for Downstream PRs (will be copied)

[modular-magician]

Hello! I am a robot who works on Magic Modules PRs.

I have detected that you are a community contributor, so your PR will be assigned to someone with a commit-bit on this repo for initial review. They will authorize it to run through our CI pipeline, which will generate downstream PRs.

Thanks for your contribution! A human will be with you soon.

@tysen, please review this PR or find an appropriate assignee.

[rileykarson]

Meta-comment for the handful of PRs I picked up yesterday: I've got another outstanding PR I'm reviewing adding the ability to add/remove clusters, and a change I'd requested to the bigtable library we use that makes that easier to do that's close to merging.

It's gonna take me a bit to put all the changes together to make sure nothing incompatible happens, and I won't have time for that today (we're in the middle of a move at my office). I'll get back Monday/Tuesday, and if I haven't by ~3PM (PST) Tuesday please ping me here!

[nickmoorman]

Thanks for the update, Riley! Let me know if there's anything I can do to help!

[rileykarson]

I'm not sure these changes will be necessary- see hashicorp/terraform-provider-google#4701 (comment).

[nickmoorman]

Thanks for the ping, I was just in the process of writing up some thoughts around my changes.

Per the comment you linked above, this isn't necessary to resolve the bug that I reported. It makes sense to me to try to keep the clusters in state in their config order, but there are a few problems with that:

1. Test failures, as described above in my PR description.
2. Interdependency issues, for lack of a better description...
   1. If the Bigtable instance was created with version 2.13.0 or earlier of TPG, this change depends on the changes in Migrate state for google_bigtable_instance hashicorp/terraform-provider-google#4702, otherwise it causes the same kind of crash described in my original bug report.
   2. If the instance was created with version 2.17.0 of TPG, the changes in this PR seem to simply do nothing. (I didn't test any intermediate versions.)
3. A very brief look at some other resources indicates that it may be a bit of an anti-pattern to depend on existing config in a Read function.
4. The Bigtable Admin API doesn't have any guarantees around order here, so perhaps it's best if we don't try to shoehorn logic in around this in the first place...

[rileykarson]

Per 3), it's generally exceptional when resources need to pull values from state.

I don't think we really get anything out of maintaining cluster order in state, other than maybe interpolations working better. However, cluster should be treated as a set. It's only implemented as a list due to technical limitations in the SDK.

Because the API doesn't have an ordering guarantee / ordering doesn't appear to matter, and given that the unordered solution works while ordering the values would require more code + more iteration, I feel that it's sufficient to keep the values unordered.

[nickmoorman]

Ah, lucky me that my first foray into TPG code happened to expose me to some pretty uncommon stuff. Good to know. ;)

I agree with your assessment re: closing. At least I learned a lot along the way while digging into this stuff! Thanks again for all your feedback!

[rileykarson]

Ha, Bigtable especially is rife with exceptions. Thanks for your patience and understanding!