feat: support default pg meta store #80
Conversation
| specObj := map[string]attr.Value{ | ||
| "risingwave_config": types.StringValue(cluster.RwConfig), | ||
| "compute": types.ObjectValueMust( |
There was a problem hiding this comment.
we will support scale between standalone/distributed soonly https://github.com/risingwavelabs/risingwave-cloud/pull/9653
Should we also add standalone here?
There was a problem hiding this comment.
For an existing cluster resource with etcd spec specified, I'm wondering what the behavior will be when user change the version to v2.1+:
During upgrade, we will migrate their metastore from etcd to pg. Next time the provider call GET api, will the provider see a diff as etcd is still specified in the tf config?
Looking at func metaStoreEqual, seems that it will? How will the provider handle this diff?
There was a problem hiding this comment.
This case is covered.
There was a problem hiding this comment.
Discussed offline, confirmed that it will generate a diff and the apply will fail.
Suggest we add some instructions to ask users to remove the etcd_meta_store field in the error message/documentation.
…terraform-provider-risingwavecloud into mike/support-pgmetastore
docs/resources/cluster.md
Outdated
| @@ -233,7 +219,7 @@ Required: | |||
|
|
|||
| Optional: | |||
|
|
|||
| - `etcd_meta_store` (Attributes) (see [below for nested schema](#nestedatt--spec--meta--etcd_meta_store)) | |||
| - `etcd_meta_store` (Attributes) The etcd meta store is no longer supported in new RisingWave versions, this field is kept for compatibility, please migrate to PostgreSQL meta store (see [below for nested schema](#nestedatt--spec--meta--etcd_meta_store)) | |||
There was a problem hiding this comment.
please migrate to PostgreSQL meta store
This seem to be a very vague statement. How does user migrate to PostgreSQL meta store?
Maybe simply tell them the meta store will be decided by the server and ask user to not specify this field ?
There was a problem hiding this comment.
Discussed offline, confirmed that it will generate a diff and the apply will fail.
Suggest we add some instructions to ask users to remove the etcd_meta_store field in the error message/documentation.
| Optional: true, | ||
| Description: "The etcd meta store is no longer supported in new RisingWave versions, this field is kept for compatibility, please remove it if your RisingWave version is above v2.1.0", | ||
| }, | ||
| "meta_store": schema.SingleNestedAttribute{ |
There was a problem hiding this comment.
hmm.. this is a lot of changes since the approval.
I'm not sure we really want this: in general we don't want users to control what metastore use, and IIRC most of the options you listed here are read only attributes that are not allowed to be specified in a POST request.
I would suggest we don't expose this as an argument but display it as a read-only attributes instead.
Update: just realized these new schema are read only attributes. Then it looks good to me! Apology for the confusion XD
cc @Gogomoe to for another review too.
wjf3121
dismissed stale reviews from Gogomoe and themself
Large changes since the review, need another round of reivew.
Allow tf plugin to function correctly when etcd_meta_store is not set.
Currently, the pg meta store is not exposed in tf config, we let the cloud to provision the default pg meta store.
Acceptance test is passed.