feat: add optional strategy and terminationGracePeriodSeconds to gateway deployment - #156
Conversation
…way deployment Add support for zero-downtime deployments: - strategy: configurable deployment strategy (e.g., RollingUpdate) - terminationGracePeriodSeconds: configurable grace period for pod termination
There was a problem hiding this comment.
Pull request overview
This PR adds two optional configuration fields to the gateway deployment in the Portkey Gateway Helm chart: deployment strategy and termination grace period settings. These additions provide users with more control over pod lifecycle management.
Changes:
- Added
strategyfield to configure Kubernetes deployment strategy (e.g., RollingUpdate settings) - Added
terminationGracePeriodSecondsfield to configure pod termination grace period - Both fields are optional with sensible defaults (empty for strategy, Kubernetes default 30s for terminationGracePeriodSeconds)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| charts/portkey-gateway/values.yaml | Adds configuration parameters for deployment strategy and termination grace period with inline documentation and examples |
| charts/portkey-gateway/templates/gateway/deployment.yaml | Implements the strategy and terminationGracePeriodSeconds configurations in the gateway deployment template |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {{- toYaml . | nindent 8 }} | ||
| {{- end }} | ||
| spec: | ||
| {{- if .Values.terminationGracePeriodSeconds }} |
There was a problem hiding this comment.
The conditional check {{- if .Values.terminationGracePeriodSeconds }} will evaluate to false when the value is 0, which is a valid Kubernetes value meaning "delete immediately". This prevents users from explicitly setting terminationGracePeriodSeconds to 0. Consider using a more specific check like {{- if ne .Values.terminationGracePeriodSeconds nil }} or {{- if hasKey .Values "terminationGracePeriodSeconds" }} to allow 0 as a valid value.
| {{- if .Values.terminationGracePeriodSeconds }} | |
| {{- if ne .Values.terminationGracePeriodSeconds nil }} |
| # if you want to restart instance automatically | ||
| autoRestart: false | ||
|
|
||
| # Optional: Grace period for pod termination (in seconds) |
There was a problem hiding this comment.
The comment includes "Optional:" prefix which is inconsistent with other similar optional configuration fields in this file (e.g., strategy, hostAlias). Consider removing "Optional:" to maintain consistency with the established commenting style: "Grace period for pod termination (in seconds)".
| # Optional: Grace period for pod termination (in seconds) | |
| # Grace period for pod termination (in seconds) |
Brings gateway and dataservice to parity on deployment features that already exist on one component but not the other: Gateway (new — already exists on dataservice): - envFrom: bulk inject env vars from secrets/configmaps - extraEnv: additional env vars after commonEnv - startupProbe: configurable startup health check Dataservice (new — gateway already has these via Portkey-AI#156): - envFrom: same pattern as gateway - terminationGracePeriodSeconds: configurable grace period All default to empty/disabled. No behavioral change when unconfigured.
Add support for following: