-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Move Bigtable config to cluster block. #2161
Conversation
Match the bigtable_instance resource to the API structure, deprecating the inlined config parameters. This is a complicated deprecation, because we don't want to ForceNew when updating to use the new undeprecated fields, but we want to ForceNew if they change. This is achieved using a CustomizeDiff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a test with multiple clusters?
Does using a default zone
make sense in the multiple-cluster world? Can we make a Cloud Bigtable with 2 clusters in the same zone? I don't think that makes sense and they should be in multiple zones, so the API probably won't allow it.
I have no idea. Since this wasn't supposed to be enhancement work, just deprecation, how would you feel about me just setting |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM. We should have that done for 2.0.0 though probably and not just "some other time"? Or removing the default zone done then at least, as it's a breaking change.
I'd be down for removing |
Oh yup! That's what I meant by that* |
Only allow one cluster for bigtable instances. We can expand to 2 for 2.0.0.
Cool, I already have #1672 in the 2.0.0 milestone, so we've got an issue to track that enhancement. |
…able_clusters Move Bigtable config to cluster block.
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks! |
Match the bigtable_instance resource to the API structure, deprecating
the inlined config parameters. This is a complicated deprecation,
because we don't want to ForceNew when updating to use the new
undeprecated fields, but we want to ForceNew if they change.
This is achieved using a CustomizeDiff.