Skip to content

feat(deployment): gateway/dataservice feature parity #170

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sk-portkey merged 3 commits into from
Apr 3, 2026
+40 −1
Merged

feat(deployment): gateway/dataservice feature parity #170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2808472
feat(deployment): gateway/dataservice feature parity
Mar 24, 2026
4058d94
Update charts/portkey-gateway/templates/dataservice/deployment.yaml
camjay Mar 25, 2026
3593ac8
docs(values): add startupProbe commented example for gateway
Mar 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion charts/portkey-gateway/templates/dataservice/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,10 @@ spec:
{{- toYaml . | nindent 8 }}
{{- end }}
spec:
imagePullSecrets:
{{- if hasKey .Values.dataservice.deployment "terminationGracePeriodSeconds" }}
terminationGracePeriodSeconds: {{ .Values.dataservice.deployment.terminationGracePeriodSeconds }}
{{- end }}
imagePullSecrets:
{{- range .Values.imageCredentials }}
- name: {{ .name }}
{{- end }}
Expand All @@ -54,6 +57,10 @@ spec:
command:
{{- toYaml . | nindent 12 }}
{{- end }}
{{- if .Values.dataservice.envFrom }}
envFrom:
{{- toYaml .Values.dataservice.envFrom | nindent 12 }}
{{- end }}
env:
- name: GATEWAY_BASE_URL
value: http://{{ include "portkeyenterprise.fullname" . }}:{{ .Values.service.port }}
Expand Down
11 changes: 11 additions & 0 deletions charts/portkey-gateway/templates/gateway/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,23 @@ spec:
containerPort: {{ include "mcp.containerPort" . }}
protocol: TCP
{{- end }}
{{- if .Values.envFrom }}
envFrom:
{{- toYaml .Values.envFrom | nindent 12 }}
{{- end }}
env:
{{- if .Values.dataservice.enabled }}
- name: DATASERVICE_BASEPATH
value: http://{{ include "portkeyenterprise.fullname" . }}-{{ .Values.dataservice.name }}:{{ .Values.dataservice.service.port }}
{{- end }}
{{- include "portkeyenterprise.commonEnv" . | nindent 12 }}
{{- with .Values.extraEnv }}
{{- toYaml . | nindent 12 }}
{{- end }}
{{- with .Values.startupProbe }}
startupProbe:
{{- toYaml . | nindent 12 }}
{{- end }}
Comment on lines +84 to +87
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

startupProbe is rendered from .Values.startupProbe, but values.yaml doesn’t define/document this value alongside the existing livenessProbe/readinessProbe settings. Add a startupProbe: section (or commented example) to values.yaml so users can discover/configure it consistently.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@camjay camjay Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a commented example in values.yaml in 3593ac8.

livenessProbe:
{{- toYaml .Values.livenessProbe | nindent 12 }}
readinessProbe:
Expand Down
21 changes: 21 additions & 0 deletions charts/portkey-gateway/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,17 @@ resources: {}
# cpu: 100m
# memory: 128Mi

# startupProbe is optional and rendered only when defined.
# Useful for slow-starting containers to avoid premature liveness kills.
# Example:
# startupProbe:
# httpGet:
# path: /v1/health
# port: 8787
# initialDelaySeconds: 10
# periodSeconds: 5
# failureThreshold: 30

livenessProbe:
httpGet:
path: /v1/health
Expand Down Expand Up @@ -263,6 +274,14 @@ volumeMounts: []

extraContainerConfig: {}

# envFrom allows loading all keys from secrets/configmaps as environment variables.
# Useful for external secret managers (ESO, Vault CSI, AWS Secrets Manager).
# Variables in env (from environment.data) take precedence over envFrom.
envFrom: []

# Additional environment variables appended after commonEnv.
extraEnv: []
Comment on lines +277 to +283
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The chart now supports envFrom/extraEnv and (in the template) startupProbe, but startupProbe isn’t present in values.yaml near the other probe settings. Add a startupProbe: block (disabled by default) to keep values.yaml in sync with the template and improve discoverability.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@camjay camjay Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same — added in 3593ac8 as a commented example, consistent with how livenessProbe and readinessProbe are already documented in this file.


# Additional sidecar containers injected into the gateway pod.
# Each entry is a full Kubernetes container spec.
# Example: nginx TLS-termination sidecar
Expand Down Expand Up @@ -318,12 +337,14 @@ dataservice:
containerPort: 8081
finetuneBucket: ""
logexportsBucket: ""
envFrom: []
env:
DEBUG_ENABLED: false
SERVICE_NAME: "portkeyenterprise-dataservice"
deployment:
autoRestart: true
replicas: 1
# terminationGracePeriodSeconds: 30
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dataservice.deployment.terminationGracePeriodSeconds is supported in the template, but values.yaml only contains a commented example and no actual key. Define terminationGracePeriodSeconds: under dataservice.deployment (empty by default) so it’s discoverable and can be set without relying on implicit/undocumented values.

Suggested change
# terminationGracePeriodSeconds: 30
terminationGracePeriodSeconds:

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@camjay camjay Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intentional — this mirrors how the gateway template already handles it (added in #156). The commented example documents the field without setting a default that would override Kubernetes's own 30s default.

labels: {}
selectorLabels: {}
annotations: {}
Expand Down