Simplify chart#784
Merged
robholland merged 26 commits intov1from Dec 5, 2025
Merged
Conversation
Temporal core allows for the registration and configuration of custom data stores.
However, there is a disparity in how the helm chart translates persistence
configuration between helm values and the resulting configmap.
As an example, the temporal config for a custom driver for the default store may look
as so:
```
persistence:
defaultStore: my-custom-store
visibilityStore: es-visibility
numHistoryShards: 4
datastores:
my-custom-store:
customDatastore:
name: "my-custom"
options:
hosts: "mycustom.example.com"
keyspace: "temporal"
```
Translating to helm values would look like
```
server:
config:
persistence:
defaultStore: my-custom-store
additionalStores:
my-custom-store:
customDatastore:
name: "my-custom"
options:
hosts: "mycustom.example.com"
keyspace: "temporal"
```
This gets us most of the way there, but there are a few problems:
1. The charts always assume defaultStore = "default" and thus have opinions
about subordinate structures such as default.driver. This results in objects
such as Secrets and Jobs being incorrectly emitted for default.driver = "cassandra" even
though we are trying to specify an alternate store. This logic should probably
be $defaultStore.driver and $visibilityStore.driver instead.
2. The additionalStores configuration does not lend itself to first-class
representation in conditional statements, even though it can be used to
emit the resulting configmap correctly.
One could argue that we should clean up the helm representation to more closely align
with the temporal-server yaml, but this is a bigger task (and a breaking change). Instead,
this patch ties into the existing model of using $store.driver = {cassandra, sql, elasticsearch}
and adds a 4th first-class type "custom."
The user may configure this as so:
```
server:
config:
persistence:
default:
driver: custom
visibility:
driver: custom
```
And in doing so, we instruct the various templates to elide the objects (Jobs, Secrets, etc.)
related to that store, leaving it to the user to supply the rest of the required configuration
using a combination of additionalStores, additionalEnv, additionalVolumeMounts, etc., to inject
configuration and secrets. They would also, of course, be responsible for any DDL setup or
migration.
Signed-off-by: Greg Haskins <greg@manetu.com>
This change was made by an automated process to ensure all GitHub Actions workflows have explicitly defined permissions as per best practices.
This removes sub-charts so that our chart is just responsible for installing Temporal. Also change the values format to use the native server config YAML rather than bad abstractions. This frees us to more easily support server features in the future (such as secondary visibility). Removes assumptions made about persistance store naming. Greatly simplifies our helpers which were the biggest challenge to confidence while making changes.
|
Semgrep found 1 No explicit |
|
|
Now that we no longer use sub-charts, databases are always external.
michaely520
approved these changes
Dec 5, 2025
robholland
added a commit
that referenced
this pull request
Dec 18, 2025
* Remove prometheus dependency (#534) * Remove prometheus from dependencies and examples * Remove grafana dependency (#526) * Adjust some helper wiring. * Update Server Configmap (#794) * .ENV to env and add config template var * Simplify chart (#784) Simplify our helm chart, remove sub-charts. * Install cluster variations as part of CI. (#796) * Skip dockerize for 1.29.* images. This allows us to use server templated config files safely on 1.29.* and 1.30.* so that we can rollforward/backward without having to maintain two sets of configs. * Schema refactor (#798) * Refactor schema management. This uses temporal-elasticsearch-tool where available, and a shell equivalent otherwise. Elasticsearch will now perform schema updates like Cassandra/SQL do. Simplify the broken setup vs upgrade toggles which did not properly control the behaviour. We don't have a way to with fixed toggles to separate creation from upgrade. Use a single toggle for management, the user can toggle that after install if they want to stop automatic upgrades. * v1 doesn't have deps. (#800) * Push to the branch on which the action was triggered. * Update Chart to 1.0.0-rc.1. * Add some upgrading notes. (#805) * Fix version specifier. (#806) * Change image pull secrets to array instead of map (#701) * Correct config option name. (#807) * Pin helm unittest version due to helm 3 vs 4 compat issue. (#808) * Switch ct to testing against main. --------- Co-authored-by: Brian Kopp <briankopp.usa@gmail.com> Co-authored-by: PhilHenning <phillyp.henning@gmail.com> Co-authored-by: LeoDiazL <leo@bitovi.com> Co-authored-by: Phil Henning <philh@bitovi.com> Co-authored-by: Alex Stanfield <13949480+chaptersix@users.noreply.github.com> Co-authored-by: Temporal Data <commander-data@temporal.io> Co-authored-by: Matthew Brandman <matthb6@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was changed
Why?
Checklist
Closes
How was this tested: