banzaicloud/banzai-charts

[Cadence] Allow passing clusterNames instead of hardcoded

longquanzheng opened this issue · 2 comments

Hi team,
I saw you recently updated this clusterMetadata section:

Thanks for doing that.
However, I would like to point out that this is not a good template to use.

You probably copied from https://github.com/uber/cadence/blob/40c5f1896565cee5f8f2a67e3ea73342b6fb4d6d/docker/config_template.yaml#L203

However, changing this hardcoded clusterNames would break existing clusters(if they have enabled global domain).
https://github.com/banzaicloud/banzai-charts/blame/031c49f4f1f542f60a88bb9f5fc56c22a518d773/cadence/templates/server-configmap.yaml#L157

To be compatible, we need to provide options to change the cluster name from "primary/secondary" to "active" back.

Also, the name "primary/secondary" are not always good names in production.

Ideally they should be something like "ProdClusterA/ProdClusterB" or "ProdClusterDCA1" etc.
In Cadence global domain, clusters are equally treated. Only a domain would choose which cluster to be active on. And the domain can be updated to become active in another cluster anytime. So users don't have to use "primary" concepts. We used that in docker-compose just for testing, because there is only one domain for testing.

Also sorta related to #1275 probably they can be solved at the same time.

From chart 0.21.0 Cadence cluster names are configurable through the chart.