Skip to content

fix: Replace exec liveness probe with tcpSocket for LMCache cache server - #1065

Open
Asthenia0412 wants to merge 2 commits into
vllm-project:mainfrom
Asthenia0412:fix/lmcache-tcp-probe
Open

fix: Replace exec liveness probe with tcpSocket for LMCache cache server#1065
Asthenia0412 wants to merge 2 commits into
vllm-project:mainfrom
Asthenia0412:fix/lmcache-tcp-probe

Conversation

@Asthenia0412

Copy link
Copy Markdown

Description

The LMCache cache server livenessProbe uses an exec probe that runs /workspace/LMCache/examples/kubernetes/health_probe.py, but this script does not exist in the LMCache v0.5.4 Docker image. The cache server communicates via TCP socket protocol, not HTTP.

Changes

  • Changed the liveness probe from exec to tcpSocket in deployment-cache-server.yaml
  • Updated the corresponding Helm test in cacheserver_test.yaml

A tcpSocket probe is more appropriate because:

  1. The LMCache cache server uses a TCP socket protocol
  2. No external script is required - Kubernetes can check the port directly
  3. Works with all LMCache versions without depending on a specific script

Closes #1057

The LMCache cache server uses a TCP socket protocol, not an HTTP server.
The exec liveness probe referenced a health_probe.py script at
/workspace/LMCache/examples/kubernetes/health_probe.py which does not
exist in the LMCache v0.5.4 Docker image, causing the liveness check
to fail.

Switching to a tcpSocket probe is more appropriate since:
1. The cache server communicates via TCP socket protocol
2. No external script is needed - Kubernetes can check the port directly
3. Works with all LMCache versions without requiring a specific script

Closes vllm-project#1057

Signed-off-by: Asthenia <asthenia0412@gmail.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request replaces the Python-based exec liveness probe in the cache server deployment with a simpler tcpSocket probe, updating the corresponding test cases as well. The reviewer recommends using a named port (e.g., 'caserver-cport') instead of referencing the port directly in both the deployment template and the test assertions to align with Kubernetes best practices.

Comment on lines +80 to +81
tcpSocket:
port: {{ .Values.cacheserverSpec.containerPort }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Using a named port (e.g., caserver-cport) instead of referencing the port number directly in the tcpSocket probe is a Kubernetes best practice. This decouples the probe configuration from the specific port number, making the chart more maintainable and robust if the port number changes in the future.

            tcpSocket:
              port: caserver-cport

Comment thread helm/tests/cacheserver_test.yaml Outdated
Comment on lines +175 to +176
tcpSocket:
port: 8000

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Update the test assertion to expect the named port caserver-cport instead of the hardcoded port number 8000 to align with the template change.

          tcpSocket:
            port: caserver-cport

Use the named port 'caserver-cport' instead of the numeric port value
in the tcpSocket liveness probe, following Kubernetes best practices.

Signed-off-by: Asthenia <asthenia0412@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: LMCache cache server livenessProbe is incompatible with LMCache v0.5.4 image

1 participant