Allow add/removing Bigtable clusters #2923
Conversation
|
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesNo diff detected in terraform-google-conversion. New Pull RequestsI built this PR into one or more new PRs on other repositories, and when those are closed, this PR will also be merged and closed. |
|
Right, vendoring. I'll send those PRs. |
| clusters[newId.(string)] = c | ||
| } | ||
|
|
||
| // create a list of clusters using the old order when possible to minimise |
There was a problem hiding this comment.
Is it possible to use the logic from https://github.com/GoogleCloudPlatform/magic-modules/blob/master/templates/terraform/unordered_list_customize_diff.erb instead of reordering? If not, can you comment as to why?
There was a problem hiding this comment.
Commented inline- I think that the TF-side ForceNew behaviour is incompatible with that method. We'd could do it, but we still end up needing to compare zone and storage_type across clusters.
| if err != nil { | ||
| return fmt.Errorf("Error setting cluster diff: %s", err) | ||
| } | ||
|
|
||
| // Clusters can't have their zone / storage_type updated, ForceNew if it's |
There was a problem hiding this comment.
Does this have to be in the same function as the reordering, or could it be handled separately?
There was a problem hiding this comment.
It requires the reordering to have happened already, so I prefer putting it here instead of breaking it out.
| @@ -318,46 +317,110 @@ func resourceBigtableInstanceValidateDevelopment(diff *schema.ResourceDiff, meta | |||
| return nil | |||
| } | |||
|
|
|||
| // resourceBigtableInstanceClusterReorderTypeList causes the cluster block to | |||
There was a problem hiding this comment.
Would we be able to get rid of this entirely if we made cluster into a set? If so, let's get that on the 4.0.0 docket.
There was a problem hiding this comment.
Nope, because we need the ForceNew behaviour. hashicorp/terraform#15420
|
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
| * `storage_type` - (Optional) The storage type to use. One of `"SSD"` or | ||
| `"HDD"`. Defaults to `"SSD"`. | ||
|
|
||
| !> Modifying the `storage_type` or `zone` of an existing cluster (by |
There was a problem hiding this comment.
Can you add a **Warning:** prefix? https://github.com/hashicorp/terraform-website#callouts
|
Hi! I'm the modular magician, I work on Magic Modules. Pull request statusesterraform-provider-google-beta already has an open PR. New Pull RequestsI didn't open any new pull requests because of this PR. |
modular-magician
force-pushed
the
bt-cluster
branch
from
5b4b73b to
b461524
Compare
Fixes hashicorp/terraform-provider-google#4318
Release Note Template for Downstream PRs (will be copied)