Omit Elasticsearch cluster settings#901
Conversation
#896 Option to skip cluster settings when using a shared Elasticsearch cluster
robholland
left a comment
There was a problem hiding this comment.
Having looked at the cluster settings (https://github.com/temporalio/temporal/blob/main/schema/elasticsearch/visibility/cluster_settings_v7.json) it's only the one setting, and we don't need that in helm as we enforce ordering via hooks so that the index is created before it gets used by the service. Let's just omit the cluster settings unconditionally. I think elasticsearch-tool update-schema without --index will create the template if it doesn't exist.
| {{- end }} | ||
| {{- else if eq $store.driver "elasticsearch" }} | ||
| {{- if $store.config.skipClusterSettings }} | ||
| - temporal-elasticsearch-tool create-index && temporal-elasticsearch-tool update-schema --index "$ES_VISIBILITY_INDEX" |
There was a problem hiding this comment.
This isn't quite right, if we create-index without the template being there then the index will lack the proper settings/mappings.
robholland
left a comment
There was a problem hiding this comment.
Please update the PR description before merging.
| - equal: | ||
| path: spec.template.spec.initContainers[?(@.name=="manage-schema-default-store")].env[?(@.name=="SQL_PORT")].value | ||
| value: "5432" | ||
| - it: does not use setup-schema for elasticsearch |
There was a problem hiding this comment.
I don't think this is the right layer to test this, you can drop the tests.
#896
Omitting Elasticsearch cluster settings
What was changed
auto_create_index: falsecan cause issues for shared Elasticsearch clusters. This is the only setting in https://github.com/temporalio/temporal/blob/main/schema/elasticsearch/visibility/cluster_settings_v7.json , so applying these settings is now being omitted.Why?
From the issue "This disables auto-creation of indices cluster-wide, which is problematic when Elasticsearch is shared with other services that rely on automatic index creation."
Checklist
Closes [Feature Request] Support skipping cluster settings when managing Elasticsearch schema #896
How was this tested:
No