feat: Moved the Routing Sidecar from its own repo to the inference-scheduler repo#379
feat: Moved the Routing Sidecar from its own repo to the inference-scheduler repo#379shmuelk merged 10 commits intollm-d:mainfrom
Conversation
|
@shmuelk I think we need Lionel for this review. |
pkg/sidecar/proxy/proxy.go
Outdated
| // ConnectorNIXLV1 enables the (now deprecated) P/D NIXL v1 protocol | ||
| ConnectorNIXLV1 = "nixl" |
There was a problem hiding this comment.
This is maybe a good time to get rid of deprecated protocols. AFAIK, only nixlv2 is used.
There was a problem hiding this comment.
I'll remove nixl V1. There is talk about LMCache....
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
| package main |
There was a problem hiding this comment.
Can you llm-d-routing-sidecar to something shorted? I would recommend proxy as this is a well-known term in the P/D context. Or pd_proxy if you want the context to be explicit.
There was a problem hiding this comment.
Renamed to pd-sidecar
pkg/sidecar/signals/signals.go
Outdated
| limitations under the License. | ||
| */ | ||
|
|
||
| package signals |
There was a problem hiding this comment.
This package is not needed anymore. Instead use https://github.com/kubernetes-sigs/controller-runtime/tree/main/pkg/manager/signals
shmuelk
force-pushed
the
sidecar
branch
2 times, most recently
from
0658063 to
24ac8b9
Compare
elevran
left a comment
elevran
left a comment
There was a problem hiding this comment.
please open an issue to change makefile targets and variables so that EPP and sidecar are treated the same.
| shell: bash | ||
| run: | | ||
| make test | ||
| make test sidecar-test |
There was a problem hiding this comment.
nit: suggest using consistent and qualified target names across scheduler and sidecar and retaining the unqualified targets for cross exe chores.
For example: test calls test-sidecar and test-epp. Same for build.
Same naming can be applied to docker files (e.g. dockerfile-sidecar) and that means we can use % targets for commanlity.
Not critical for this PR, perhaps open an issue?
| TARGETOS ?= $(shell go env GOOS) | ||
| TARGETARCH ?= $(shell go env GOARCH) | ||
| PROJECT_NAME ?= llm-d-inference-scheduler | ||
| SIDECAR_IMAGE_NAME ?= llm-d-routing-sidecar |
There was a problem hiding this comment.
nit: ditto for naming conventions of variables in the Makfile (EPP_ and SIDECAR_...)
|
/lgtm |
github-actions
bot
added
the
lgtm
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
@shmuelk please sign all commits before PR can be merged. Currently most are missing a verified signature |
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
|
/lgtm |
github-project-automation
bot
moved this from In review
to Done
in llm-d-inference-scheduler
…heduler repo (llm-d#379) * Moved prefill header definition to common import Signed-off-by: Shmuel Kallner <kallner@il.ibm.com> * Moved Routing Sidecar into this repo Signed-off-by: Shmuel Kallner <kallner@il.ibm.com> * Moved Routing Sidecar tests into this repo Signed-off-by: Shmuel Kallner <kallner@il.ibm.com> * Moved Routing Sidecar Dockerfile into this repo Signed-off-by: Shmuel Kallner <kallner@il.ibm.com> * Added Routing Sidecar to Makefile Signed-off-by: Shmuel Kallner <kallner@il.ibm.com> * Added Routing Sidecar to CI stream Signed-off-by: Shmuel Kallner <kallner@il.ibm.com> * Fixed lint error Signed-off-by: Shmuel Kallner <kallner@il.ibm.com> * Review fixes and added version info Signed-off-by: Shmuel Kallner <kallner@il.ibm.com> * Test Nixl V2 instead of the deleted Nixl V1 Signed-off-by: Shmuel Kallner <kallner@il.ibm.com> * Fixed lint errors Signed-off-by: Shmuel Kallner <kallner@il.ibm.com> --------- Signed-off-by: Shmuel Kallner <kallner@il.ibm.com>
4 participants
This PR moves the Routing Sidecar from its own repo into the inference-scheduler repo.
This includes:
The Routing Sidecar End to End tests will be added in a followup PR
Refs: #335