Skip to content

Conversation

@yardenc2003
Copy link
Contributor

@yardenc2003 yardenc2003 commented Sep 2, 2025

This PR updates the trino/ Helm chart so that coordinator/worker.deployment.strategy is empty by default instead of pre-filled with a RollingUpdate default K8s configurations.

Motivation:

Currently, the chart always renders a .spec.strategy.rollingUpdate block in the deployment manifest, even when users want to use .spec.strategy.type==Recreate. This leads to confusing output, especially when using wrapper charts (in which rollingUpdate is impossible to override) or custom values files (which make you set rollingUpdate to null while using Recreate type).
K8s fails to create the deployment when .spec.strategy.rollingUpdate appears in conjunction with .spec.strategy.type==Recreate.

Solution:

The following changes eliminate the need for workarounds like null to override the rollingUpdate field, making it more flexible and user-friendly.

Backward Compatibility:

The default behavior

strategy:
  type: RollingUpdate
  rollingUpdate:
    maxSurge: 25%
    maxUnavailable: 25%

is unchanged if users do not override strategy, as these are the default values according to K8s official docs.

@cla-bot cla-bot bot added the cla-signed label Sep 2, 2025
@nineinchnick nineinchnick added the bug Something isn't working label Sep 2, 2025
@nineinchnick
Copy link
Member

I labeled this as a bug because even though there's a workaround, I think it's rather difficult to figure out. Thanks for the excellent PR description!

@nineinchnick nineinchnick changed the title Improve Trino Deployments Strategy Customization Remove default deployment strategy values Sep 2, 2025
@nineinchnick nineinchnick merged commit 8027b63 into trinodb:main Sep 3, 2025
20 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working cla-signed

Development

Successfully merging this pull request may close these issues.

2 participants