Skip to content

Commit b2c28c2

Browse files
revit13revit13
committed
Address review comments and add github workflow test.
Signed-off-by: Revital Sur <eres@il.ibm.com>
1 parent 4a1f879 commit b2c28c2

8 files changed

Lines changed: 151 additions & 80 deletions

File tree

‎.github/workflows/ci-coordinator.yaml‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
name: CI - Coordinator Test
2+
3+
# Runs coordinator unit tests and e2e tests. Path filter excludes the
4+
# sidecar-only paths so this workflow only triggers for coordinator changes.
5+
on:
6+
push:
7+
branches:
8+
- main
9+
- 'release-*'
10+
pull_request:
11+
branches:
12+
- main
13+
14+
permissions:
15+
contents: read
16+
actions: read
17+
18+
concurrency:
19+
group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.ref }}
20+
cancel-in-progress: true
21+
22+
jobs:
23+
check-changes:
24+
runs-on: ubuntu-latest
25+
outputs:
26+
src: ${{ steps.filter.outputs.src }}
27+
steps:
28+
- name: Checkout source
29+
uses: actions/checkout@v7
30+
- uses: dorny/paths-filter@v4
31+
id: filter
32+
with:
33+
filters: |
34+
src:
35+
- '**/*.go'
36+
- Dockerfile.*
37+
- Makefile*
38+
- go.mod
39+
- go.sum
40+
- scripts/**
41+
- hack/**
42+
- test/**
43+
- .github/actions/docker-build-and-push/action.yml
44+
- .github/actions/e2e-runner-setup/action.yml
45+
- .github/workflows/ci-coordinator.yaml
46+
- '!Dockerfile.sidecar'
47+
- '!cmd/pd-sidecar/**'
48+
- '!pkg/sidecar/**'
49+
- '!test/sidecar/**'
50+
51+
unit-tests:
52+
needs: check-changes
53+
if: ${{ needs.check-changes.outputs.src == 'true' }}
54+
runs-on: ubuntu-latest
55+
timeout-minutes: 30
56+
steps:
57+
- name: Checkout source
58+
uses: actions/checkout@v7
59+
60+
- name: Create Go cache dirs
61+
id: go-cache
62+
run: |
63+
mkdir -p "$HOME/.cache/llm-d-gomodcache" "$HOME/.cache/llm-d-gobuildcache"
64+
echo "mod=$HOME/.cache/llm-d-gomodcache" >> "$GITHUB_OUTPUT"
65+
echo "build=$HOME/.cache/llm-d-gobuildcache" >> "$GITHUB_OUTPUT"
66+
67+
- name: Cache Go modules and build cache
68+
uses: actions/cache@v6
69+
with:
70+
path: |
71+
${{ steps.go-cache.outputs.mod }}
72+
${{ steps.go-cache.outputs.build }}
73+
key: go-cache-${{ runner.os }}-${{ hashFiles('go.sum') }}
74+
restore-keys: |
75+
go-cache-${{ runner.os }}-
76+
77+
- name: Run coordinator unit tests
78+
shell: bash
79+
env:
80+
GO_MOD_CACHE_VOL: ${{ steps.go-cache.outputs.mod }}
81+
GO_BUILD_CACHE_VOL: ${{ steps.go-cache.outputs.build }}
82+
run: make -f Makefile.coord.mk test-unit
83+
84+
e2e-tests:
85+
needs: check-changes
86+
if: ${{ needs.check-changes.outputs.src == 'true' }}
87+
runs-on: ubuntu-latest
88+
timeout-minutes: 60
89+
steps:
90+
- name: Checkout source
91+
uses: actions/checkout@v7
92+
93+
- name: Set up e2e runner
94+
id: e2e-setup
95+
uses: ./.github/actions/e2e-runner-setup
96+
97+
- name: Run coordinator e2e tests
98+
shell: bash
99+
env:
100+
GO_MOD_CACHE_VOL: ${{ steps.e2e-setup.outputs.go-mod-cache }}
101+
GO_BUILD_CACHE_VOL: ${{ steps.e2e-setup.outputs.go-build-cache }}
102+
run: make -f Makefile.coord.mk test-e2e-coordinator

‎.github/workflows/ci-pr-checks.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,11 @@ jobs:
4242
- .github/actions/docker-build-and-push/action.yml
4343
- .github/actions/e2e-runner-setup/action.yml
4444
- .github/workflows/ci-pr-checks.yaml
45+
- '!Dockerfile.coordinator'
46+
- '!Makefile.coord.mk'
47+
- '!cmd/coordinator/**'
48+
- '!pkg/coordinator/**'
49+
- '!test/coordinator/**'
4550
helm:
4651
- 'config/charts/**'
4752
- 'hack/verify-helm.sh'

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ LDFLAGS ?= -s -w
157157
BASE_IMAGE ?=
158158

159159
# test packages
160-
epp_TEST_PACKAGES = $$(go list ./... | grep -v /test/ | grep -v ./pkg/sidecar/ | tr '\n' ' ')
160+
epp_TEST_PACKAGES = $$(go list ./... | grep -v /test/ | grep -v ./pkg/sidecar/ | grep -v ./pkg/coordinator/ | grep -v ./cmd/coordinator | tr '\n' ' ')
161161
sidecar_TEST_PACKAGES = ./pkg/sidecar/...
162162

163163
# Internal variables for generic targets

‎Makefile.coord.mk‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ LINT_NEW_ONLY ?= false
5151
# Optional: override the runtime base image used in container builds.
5252
BASE_IMAGE ?=
5353

54-
TEST_PACKAGES = $$(go list ./... | grep -v /test/ | tr '\n' ' ')
54+
TEST_PACKAGES = $$(go list ./pkg/coordinator/... ./cmd/coordinator/... | tr '\n' ' ')
5555

5656
# Common flags for running the builder container: mounts source, Go caches, and runs as current user.
5757
# Podman rootless requires --userns=keep-id to correctly map host UID; docker uses -u directly.

‎deploy/coordinator/components/vllm-decode/deployment.yaml‎

Lines changed: 0 additions & 74 deletions
This file was deleted.

‎deploy/coordinator/components/vllm-decode/kustomization.yaml‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,7 @@ apiVersion: kustomize.config.k8s.io/v1beta1
22
kind: Kustomization
33

44
resources:
5-
- deployment.yaml
5+
- ../../../components/vllm-decode/
6+
7+
patches:
8+
- path: patch-decode.yaml

‎deploy/coordinator/components/vllm-decode/patch-decode.yaml‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
apiVersion: apps/v1
2+
kind: Deployment
3+
metadata:
4+
name: vllm-d
5+
spec:
6+
template:
7+
spec:
8+
initContainers:
9+
- name: routing-sidecar
10+
$patch: delete
11+
containers:
12+
- name: vllm
13+
args:
14+
- "--port=8000"
15+
- "--model=${MODEL_NAME}"
16+
- "--data-parallel-size=${VLLM_DATA_PARALLEL_SIZE}"
17+
- "${VLLM_EXTRA_ARGS_D}"
18+
ports:
19+
- $patch: replace
20+
- name: http
21+
containerPort: 8000
22+
protocol: TCP
23+
startupProbe:
24+
httpGet:
25+
port: 8000
26+
readinessProbe:
27+
httpGet:
28+
port: 8000
29+
livenessProbe:
30+
httpGet:
31+
port: 8000
32+
env:
33+
- name: PORT
34+
value: "8000"

‎deploy/coordinator/environments/dev/epd-pools/kustomization.yaml‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,10 @@ kind: Kustomization
44
resources:
55
- ../../../../components/vllm-encode/
66
- ../../../../components/vllm-prefill/
7-
# vllm-decode is coordinator-local, not the shared deploy/components/vllm-decode: the
8-
# router's decode carries a routing-sidecar proxying 8000->8200 for connector-based P/D
9-
# disaggregation, which the coordinator's own pipeline orchestration doesn't use.
7+
# vllm-decode is a coordinator-local overlay of the shared deploy/components/vllm-decode/:
8+
# it patches out the routing-sidecar that proxies 8000->8200 for connector-based P/D
9+
# disaggregation, which the coordinator's own pipeline orchestration doesn't use. This
10+
# mirrors deploy/environments/dev/epd/, the router's own no-disaggregation scenario.
1011
- ../../../components/vllm-decode/
1112

1213
components:

0 commit comments

Comments
 (0)