Skip to content

Conversation

@carlory
Copy link
Contributor

@carlory carlory commented Oct 21, 2025

What this PR does / why we need it:

## Prerequisites

- Go Version 1.24.1 or higher (matches the module requirements)
- Rust Version 1.90.0 or higher (for Candle bindings, supports 2024 edition)

related-to #176

Which issue(s) this PR fixes:

Fixes #

Release Notes: Yes/No

@netlify
Copy link

netlify bot commented Oct 21, 2025

Deploy Preview for vllm-semantic-router ready!

Name Link
🔨 Latest commit 431e98e
🔍 Latest deploy log https://app.netlify.com/projects/vllm-semantic-router/deploys/690029b7ebb47400080f0c16
😎 Deploy Preview https://deploy-preview-499--vllm-semantic-router.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-actions github-actions bot added the hold label Oct 21, 2025
@rootfs
Copy link
Collaborator

rootfs commented Oct 24, 2025

with #266 merged, @carlory can you continue this work?

@rootfs rootfs requested a review from Copilot October 24, 2025 16:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR downgrades the Rust version from 1.85 to 1.90 across Dockerfiles to align with the project's minimum Rust version requirement of 1.90.0 for Candle bindings support.

  • Standardized Rust version to 1.90 in Docker base images

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
Dockerfile.extproc.cross Updated Rust base image from 1.85 to 1.90 for cross-compilation builder
Dockerfile.extproc Updated Rust base image from 1.85 to 1.90 for standard builder

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@carlory carlory force-pushed the match-rust-version-as-doc branch from dccc689 to d9da6b7 Compare October 27, 2025 03:09
@github-actions
Copy link

github-actions bot commented Oct 27, 2025

👥 vLLM Semantic Team Notification

The following members have been identified for the changed files in this PR and have been automatically assigned:

📁 Root Directory

Owners: @rootfs, @Xunzhuo
Files changed:

  • Dockerfile.extproc
  • Dockerfile.extproc.cross

vLLM

🎉 Thanks for your contributions!

This comment was automatically generated based on the OWNER files in the repository.

@carlory
Copy link
Contributor Author

carlory commented Oct 27, 2025

@rootfs rebased.

@rootfs
Copy link
Collaborator

rootfs commented Oct 27, 2025

@carlory can you update this too to make sure Rust 1.9 is fully tested?
https://github.com/vllm-project/semantic-router/blob/main/.github/workflows/test-and-build.yml#L21

@carlory carlory force-pushed the match-rust-version-as-doc branch from ada8c8c to 431e98e Compare October 28, 2025 02:25
@carlory
Copy link
Contributor Author

carlory commented Oct 28, 2025

After upgrading rust image version to 1.90, it will fails on centos:stream9 due to incompatibility with glibc.

error message:

/app/extproc-server: /lib64/libc.so.6: version `GLIBC_2.39' not found (required by /app/lib/libcandle_semantic_router.so)

readelf output:

bash-5.1# readelf -Ws /app/lib/libcandle_semantic_router.so | grep GLIBC_2.39
   125: 0000000000000000     0 FUNC    WEAK   DEFAULT  UND pidfd_getpid@GLIBC_2.39 (12)
   126: 0000000000000000     0 FUNC    WEAK   DEFAULT  UND pidfd_spawnp@GLIBC_2.39 (12)

ldd version on centos:streamX

(base) ➜  ~ docker run --rm quay.io/centos/centos:stream9 ldd --version

ldd (GNU libc) 2.34
Copyright (C) 2021 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
Written by Roland McGrath and Ulrich Drepper.
(base) ➜  ~ docker run --rm quay.io/centos/centos:stream10 ldd --version

ldd (GNU libc) 2.39
Copyright (C) 2024 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
Written by Roland McGrath and Ulrich Drepper.

ldd version on rust:X

(base) ➜  ~ docker run --rm rust:1.85 ldd --version
ldd (Debian GLIBC 2.36-9+deb12u10) 2.36
Copyright (C) 2022 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
Written by Roland McGrath and Ulrich Drepper.
(base) ➜  ~ docker run --rm rust:1.90 ldd --version
ldd (Debian GLIBC 2.41-12) 2.41
Copyright (C) 2024 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
Written by Roland McGrath and Ulrich Drepper.

It seems that this issue can be solved by replacing centos:stream9 with centos:stream10. But it is not a good solution because the ldd version on rust:1.90 is higher than the ldd version on centos:stream10. Is it possible to change rust-builder image to centos:stream9 and install rust:1.90 on that image?

Do you have any better suggestion? @rootfs

@rootfs
Copy link
Collaborator

rootfs commented Oct 28, 2025

@carlory maybe some crates also need upgrade. I am in the process of upgrading candle from 0.8.4 to latest, stay tuned.

@rootfs
Copy link
Collaborator

rootfs commented Oct 28, 2025

@carlory it looks new rust may have more restriction per https://github.com/vllm-project/semantic-router/actions/runs/18881201529/job/53884511892?pr=549

I don't have this issue on my end with Rust 1.75.

@rootfs
Copy link
Collaborator

rootfs commented Oct 28, 2025

@carlory 1.90 actually works with newer candle. When you fix the CI, let's merge this PR.

@rootfs
Copy link
Collaborator

rootfs commented Oct 28, 2025

@JaredforReal the kubernetes test failed on timeout. The init container failed to download the models before the readiness probe failed. We may need longer probe interval.

@rootfs rootfs merged commit 585e13d into vllm-project:main Oct 28, 2025
22 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants