[BugFix][Feat]: fix serviceEngineSpec probe field and improve probe management in helm template #809

emanuelecassese · 2026-01-30T15:46:11Z

Changes Made

Values File Modification:

The serviceEngineSpec.probe.httpGet section in the values file has been commented
Added equivalent values to the template defaults to prevent the httpGet field from being automatically included in the deployment template.

Helper File Update:

Enhanced the define.probes in the helpers file.
Attempted to accommodate all possible fields to closely align with the Kubernetes probe definitions.

Signed-off-by: emanuelecassese <emanuele.cassese@gmail.com>

gemini-code-assist · 2026-01-30T15:46:29Z

Summary of Changes

Hello @emanuelecassese, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refines the Helm chart's handling of Kubernetes probes for servingEngineSpec. It addresses an issue where httpGet probe configurations were automatically generated, even when not explicitly desired, by commenting out these defaults in the values.yaml. Concurrently, the _helpers.tpl file has been significantly updated to provide a more robust and granular definition of startup, liveness, and readiness probes, allowing for explicit configuration of various probe types and their parameters, thereby improving the flexibility and control over probe behavior.

Highlights

Enhanced Probe Management: The Helm chart's probe management has been significantly enhanced in _helpers.tpl to explicitly define Kubernetes probe fields and support various probe types including exec, tcpSocket, grpc, and httpGet.
Fix for Automatic httpGet Inclusion: The default httpGet probe configurations for startupProbe, livenessProbe, and readinessProbe have been commented out in values.yaml to prevent their automatic inclusion when not explicitly desired, resolving a bug.
Granular Probe Configuration: The chart.probes definition now allows for granular configuration of probe parameters such as initialDelaySeconds, periodSeconds, failureThreshold, timeoutSeconds, and successThreshold.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

emanuelecassese · 2026-01-30T15:47:23Z

These are some of the test values i've use to test my changes
tests.yml

gemini-code-assist

Code Review

This pull request addresses an issue with probe configuration by moving default values into the Helm template for more granular control and expanding probe support. The changes in values.yaml are appropriate. The new implementation in _helpers.tpl is a significant improvement, but there are opportunities to enhance maintainability, consistency, and robustness, as detailed in the review comments.

helm/templates/_helpers.tpl

gemini-code-assist · 2026-01-30T15:48:16Z

helm/templates/_helpers.tpl

 {{- define "chart.probes" -}}
-{{-   if .Values.servingEngineSpec.startupProbe  }}
+{{- if .Values.servingEngineSpec.startupProbe }}
 startupProbe:
-{{-     with .Values.servingEngineSpec.startupProbe }}
-{{-       toYaml . | nindent 2 }}
-{{-     end }}
-{{-   end }}
-{{-   if .Values.servingEngineSpec.livenessProbe  }}
+  {{- with .Values.servingEngineSpec.startupProbe }}
+  initialDelaySeconds: {{ .initialDelaySeconds | default 15 }}
+  periodSeconds: {{ .periodSeconds | default 10 }}
+  failureThreshold: {{ .failureThreshold | default 3 }}
+  {{- if .timeoutSeconds }}
+  timeoutSeconds: {{ .timeoutSeconds }}
+  {{- end }}
+  {{- if .successThreshold }}
+  successThreshold: {{ .successThreshold }}
+  {{- end }}
+  {{- if .exec }}
+  exec:
+    command: {{- range .exec.command }}
+      - {{.}} {{- end}}
+  {{- else if .tcpSocket }}
+  tcpSocket:
+    {{- if .tcpSocket.host }}
+    host: {{ .tcpSocket.host }}
+    {{- end }}
+    port: {{ .tcpSocket.port }}
+  {{- else if .grpc }}
+  grpc:
+    {{- if .grpc.service }}
+    service: {{ .grpc.service }}
+    {{- end }}
+    port: {{ .grpc.port }}
+  {{- else }}
+  httpGet:
+    path:  {{ .httpGet.path | default "/health" }}
+    port: {{ .httpGet.port | default 8000 }}
+    {{- if .httpGet.httpHeaders }}
+    httpHeaders: {{- range .httpGet.httpHeaders }}
+      - name: {{ .name }}
+        value: {{ .value }}
+    {{- end }}
+    {{- end }}
+    {{- if .httpGet.host }}
+    host: {{ .httpGet.host }}
+    {{- end }}
+    {{- if .httpGet.scheme }}
+    scheme: {{ .httpGet.scheme }}
+    {{- end }}
+  {{- end }}
+  {{- end }}
+{{- end }}
+{{- if .Values.servingEngineSpec.livenessProbe }}
 livenessProbe:
-{{-     with .Values.servingEngineSpec.livenessProbe }}
-{{-       toYaml . | nindent 2 }}
-{{-     end }}
-{{-   end }}
-{{-   if .Values.servingEngineSpec.readinessProbe  }}
+  {{- with .Values.servingEngineSpec.livenessProbe }}
+  initialDelaySeconds: {{ .initialDelaySeconds | default 15 }}
+  periodSeconds: {{ .periodSeconds | default 10 }}
+  failureThreshold: {{ .failureThreshold | default 3 }}
+  {{- if .timeoutSeconds }}
+  timeoutSeconds: {{ .timeoutSeconds }}
+  {{- end }}
+  {{- if .successThreshold }}
+  successThreshold: {{ .successThreshold }}
+  {{- end }}
+  {{- if .exec }}
+  exec:
+    command: {{- range .exec.command }}
+      - {{.}} {{- end}}
+  {{- else if .grpc }}
+  grpc:
+    {{- if .grpc.service }}
+    service: {{ .grpc.service }}
+    {{- end }}
+    port: {{ .grpc.port }}
+  {{- else if .tcpSocket }}
+  tcpSocket:
+    {{- if .tcpSocket.host }}
+    host: {{ .tcpSocket.host }}
+    {{- end }}
+    port: {{ .tcpSocket.port }}
+  {{- else }}
+  httpGet:
+    path:  {{ .httpGet.path | default "/health" }}
+    port: {{ .httpGet.port | default 8000 }}
+    {{- if .httpGet.httpHeaders }}
+    httpHeaders: {{- range .httpGet.httpHeaders }}
+      - name: {{ .name }}
+        value: {{ .value }}
+    {{- end }}
+    {{- end }}
+    {{- if .httpGet.host }}
+    host: {{ .httpGet.host }}
+    {{- end }}
+    {{- if .httpGet.scheme }}
+    scheme: {{ .httpGet.scheme }}
+    {{- end }}
+  {{- end }}
+  {{- end }}
+{{- end }}
+{{- if .Values.servingEngineSpec.readinessProbe }}
 readinessProbe:
-{{-     with .Values.servingEngineSpec.readinessProbe }}
-{{-       toYaml . | nindent 2 }}
-{{-     end }}
-{{-   end }}
+  {{- with .Values.servingEngineSpec.readinessProbe }}
+  initialDelaySeconds: {{ .initialDelaySeconds | default 15 }}
+  periodSeconds: {{ .periodSeconds | default 10 }}
+  failureThreshold: {{ .failureThreshold | default 3 }}
+  {{- if .timeoutSeconds }}
+  timeoutSeconds: {{ .timeoutSeconds }}
+  {{- end }}
+  {{- if .successThreshold }}
+  successThreshold: {{ .successThreshold }}
+  {{- end }}
+  {{- if .exec }}
+  exec:
+    command: {{- range .exec.command }}
+      - {{.}} {{- end}}
+  {{- else if .tcpSocket }}
+  tcpSocket:
+    {{- if .tcpSocket.host }}
+    host: {{ .tcpSocket.host }}
+    {{- end }}
+    port: {{ .tcpSocket.port }}
+  {{- else if .grpc }}
+  grpc:
+    {{- if .grpc.service }}
+    service: {{ .grpc.service }}
+    {{- end }}
+    port: {{ .grpc.port }}
+  {{- else }}
+  httpGet:
+    path:  {{ .httpGet.path | default "/health" }}
+    port: {{ .httpGet.port | default 8000 }}
+    {{- if .httpGet.httpHeaders }}
+    httpHeaders: {{- range .httpGet.httpHeaders }}
+      - name: {{ .name }}
+        value: {{ .value }}
+    {{- end }}
+    {{- end }}
+    {{- if .httpGet.host }}
+    host: {{ .httpGet.host }}
+    {{- end }}
+    {{- if .httpGet.scheme }}
+    scheme: {{ .httpGet.scheme }}
+    {{- end }}
+  {{- end }}
+  {{- end }}
+{{- end }}
+
 {{- end }}


The logic for startupProbe, livenessProbe, and readinessProbe is nearly identical. This significant code duplication makes the template harder to maintain, as any change needs to be applied in three separate places. To improve maintainability, consider refactoring this logic into a new, single helper template that can be called for each probe type. This would centralize the probe logic, making it easier to read and update in the future.

gemini-code-assist · 2026-01-30T15:48:16Z

helm/templates/_helpers.tpl

+  {{- else if .grpc }}
+  grpc:
+    {{- if .grpc.service }}
+    service: {{ .grpc.service }}
+    {{- end }}
+    port: {{ .grpc.port }}
+  {{- else if .tcpSocket }}
+  tcpSocket:
+    {{- if .tcpSocket.host }}
+    host: {{ .tcpSocket.host }}
+    {{- end }}
+    port: {{ .tcpSocket.port }}
+  {{- else }}


The order of checking for probe handlers in livenessProbe (exec, grpc, tcpSocket) is different from startupProbe and readinessProbe (exec, tcpSocket, grpc). While Kubernetes only allows one handler per probe, this inconsistency can be confusing and lead to unexpected behavior if a user accidentally specifies multiple handlers. For consistency, the order should be the same across all probes.

{{- else if .tcpSocket }} tcpSocket: {{- if .tcpSocket.host }} host: {{ .tcpSocket.host }} {{- end }} port: {{ .tcpSocket.port }} {{- else if .grpc }} grpc: {{- if .grpc.service }} service: {{ .grpc.service }} {{- end }} port: {{ .grpc.port }} {{- else }}

Signed-off-by: emanuelecassese <emanuele.cassese@gmail.com>

ruizhang0101

Hi! Thanks for bringing this up.
For each specific type, could you use toyaml since there are way too many duplication?

Signed-off-by: emanuelecassese <emanuele.cassese@gmail.com>

emanuelecassese · 2026-02-06T14:31:52Z

Hi @ruizhang0101,
thanks for the idea
I have improved the chart adding a probe template in the helpers to avoid duplication
I would like to keep the explicit field definition instead of using toYaml

ruizhang0101

Thanks for the modification! I like the idea of using the probe template. :) Meanwhile, can you remove some unnecessary modifications?

ruizhang0101 · 2026-02-06T18:17:43Z

helm/values.yaml

      # -- Configuration of the Kubelet http request on the server
    httpGet:
-      # -- Path to access on the HTTP server
+    #  # -- Path to access on the HTTP server


Can you remove these unnecessary modifications? Same with line R270, R283, R285

Oh yeah, sorry about that
Done

Signed-off-by: emanuelecassese <emanuele.cassese@gmail.com>

Signed-off-by: Rui Zhang <51696593+ruizhang0101@users.noreply.github.com>

ruizhang0101

LGTM :)) Thanks for the contribution!

emanuelecassese added 2 commits January 30, 2026 14:01

fix: httpGet probe force and improve probe evaluation

1ea2c4d

Signed-off-by: emanuelecassese <emanuele.cassese@gmail.com>

fix: mutually exlusive handlers

644fd72

Signed-off-by: emanuelecassese <emanuele.cassese@gmail.com>

gemini-code-assist bot reviewed Jan 30, 2026

View reviewed changes

emanuelecassese and others added 2 commits February 1, 2026 15:22

Merge branch 'main' into main

bf5a317

fix: add quote to exec.command and httpHeaders.value

d6d6b5d

Signed-off-by: emanuelecassese <emanuele.cassese@gmail.com>

emanuelecassese force-pushed the main branch from 38f12b1 to d6d6b5d Compare February 2, 2026 08:30

ruizhang0101 requested changes Feb 3, 2026

View reviewed changes

emanuelecassese and others added 2 commits February 6, 2026 10:59

fix: solved httpGet empty value and optimized chart probe with template

5546547

Signed-off-by: emanuelecassese <emanuele.cassese@gmail.com>

Merge branch 'vllm-project:main' into main

0530f30

ruizhang0101 requested changes Feb 6, 2026

View reviewed changes

emanuelecassese and others added 4 commits February 9, 2026 09:10

Merge branch 'vllm-project:main' into main

a571991

fix: removed useless changes

fbb949e

Signed-off-by: emanuelecassese <emanuele.cassese@gmail.com>

Merge branch 'main' into main

f77e11c

Fix comments formatting in values.yaml

6be2362

Signed-off-by: Rui Zhang <51696593+ruizhang0101@users.noreply.github.com>

ruizhang0101 approved these changes Feb 10, 2026

View reviewed changes

ruizhang0101 merged commit 5fed085 into vllm-project:main Feb 10, 2026
10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BugFix][Feat]: fix serviceEngineSpec probe field and improve probe management in helm template #809

[BugFix][Feat]: fix serviceEngineSpec probe field and improve probe management in helm template #809

emanuelecassese commented Jan 30, 2026

Uh oh!

gemini-code-assist bot commented Jan 30, 2026

Uh oh!

emanuelecassese commented Jan 30, 2026

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

Uh oh!

gemini-code-assist bot Jan 30, 2026

Uh oh!

gemini-code-assist bot Jan 30, 2026

Uh oh!

ruizhang0101 left a comment

Uh oh!

emanuelecassese commented Feb 6, 2026

Uh oh!

ruizhang0101 left a comment

Uh oh!

ruizhang0101 Feb 6, 2026

Uh oh!

emanuelecassese Feb 9, 2026

Uh oh!

ruizhang0101 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

[BugFix][Feat]: fix serviceEngineSpec probe field and improve probe management in helm template #809

[BugFix][Feat]: fix serviceEngineSpec probe field and improve probe management in helm template #809

Conversation

emanuelecassese commented Jan 30, 2026

Changes Made

Values File Modification:

Helper File Update:

Uh oh!

gemini-code-assist bot commented Jan 30, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

emanuelecassese commented Jan 30, 2026

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

gemini-code-assist bot Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

ruizhang0101 left a comment

Choose a reason for hiding this comment

Uh oh!

emanuelecassese commented Feb 6, 2026

Uh oh!

ruizhang0101 left a comment

Choose a reason for hiding this comment

Uh oh!

ruizhang0101 Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

emanuelecassese Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

ruizhang0101 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants