Skip to content

Conversation

@jorhett
Copy link

@jorhett jorhett commented Nov 30, 2025

Why?

The helm chart currently creates a job to setup and update the schema named temporal-schema-{{Helm chart version}} since #564 which is often just temporal-schema-1. The lifetime of this chart is 1 day, which has two separate impacts:

What was changed

  1. Give the schema update job and pod a name including the major-minor version of Temporal. This will allow multiple minor version upgrades to succeed without having to manually remove the job.

  2. Remove the job removal TTL the value of which was debatable, but regardless of value will show sync issues with Helm diff and ArgoCD sync status. As the use of Helm hooks was explicitly removed in Setup for ci install. #522 to resolve issues with --wait, this provides another way to retain diff/sync. This also means that job update logs are retained and available for life of the helm release.

  3. Updated the README to make it clear the manual update steps aren't required if update is enabled.

Checklist

  1. Closes [Bug] helm upgrade does not work if schema.enabled is true #554 [Feature Request] Allow Schema Job ttlSecondsAfterFinished to be configurable #735 Inconsistent diff using Helm diff plugin #636 [Feature Request] Support ArgoCD deployments #709 [Bug] schema job fails #737

  2. How was this tested: it's only a name change, helm upgrade did it all 😁

  3. Any docs updates needed?

Updates to the README are included.

Create a job name which contains the minor version
Remove the TTL on the job which causes diff/sync issues
Update the README instructions
@jorhett jorhett requested a review from a team as a code owner November 30, 2025 09:02
@CLAassistant
Copy link

CLAassistant commented Nov 30, 2025

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@robholland
Copy link
Contributor

Please could you update this now that v1 has been merged to master. Please add a test, and consider how this might work given we're currently doing rc releases? Might need to have support for pre/alpha/rc style stuff in the schema job name as well.

@robholland robholland added the needs revision Team has requested some changes label Dec 19, 2025
@jorhett
Copy link
Author

jorhett commented Dec 22, 2025

consider how this might work given we're currently doing rc releases?

It's kind of odd to respond to a submission and say "please go solve this new problem specific to our releases" -- especially when I know nothing of your versioning or release strategy 😁

I've demonstrated a useful way to solve this problem and minimize problems that lots of people were experiencing in many different ways. I would think details like adapting it to changes in your versioning strategy / etc are important to you, but orthogonal to the value provided by this PR. Please take this answer and implement it however you feel is best.

@jorhett
Copy link
Author

jorhett commented Dec 22, 2025

Also keep in mind that AFAICT the "changes" required would be altering a printf() call to properly represent your versions. Trivial (<10 chars?) change for someone who knows your versioning strategy, and a large investment of research by someone who doesn't.

I'm not in any way trying to suggest that version handling isn't important, I'm saying an engineer who knows your versioning can do it between breaths 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs revision Team has requested some changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] helm upgrade does not work if schema.enabled is true

3 participants