[coordinator] e2e tests: containerize and share deploy components by revit13 · Pull Request #1975 · llm-d/llm-d-router

revit13 · 2026-07-12T12:07:00Z

What type of PR is this?
/kind test

What this PR does / why we need it:
Reworks how the coordinator's e2e tests run and removes duplication between the coordinator's deploy manifests and the main router's, fixing several bugs found along the way.

Containerize test-e2e-coordinator: the target now runs the whole suite (including kind create cluster) inside the builder container via docker run, matching the main router's test-e2e-run pattern, instead of executing directly on the host.
Share deploy components with the router: crds-gateway-api, crds-gie, vllm-encode, and vllm-prefill now reference the router's shared deploy/components/* instead of coordinator-local vendored/duplicated copies. vllm-decode intentionally stays coordinator-local (the router's decode carries a routing-sidecar the coordinator's pipeline doesn't use) with a comment explaining why.
Drop the unused Gateway API CRD install from the coordinator e2e suite — the coordinator never creates a Gateway/HTTPRoute object (it routes on an EPP-Phase header via a hand-rolled Envoy config), so only GIE CRDs are installed now.
Parameterize vllm-render: port and image are now ${VLLM_RENDER_PORT}/${VLLM_RENDER_IMAGE} (previously hardcoded 8082 and reusing ${VLLM_IMAGE}), and dropped unused PORT/POD_NAME/POD_NAMESPACE env vars from the container.
Bug fixes:
--force-dummy-tokenizer was being passed twice to the encode/prefill/decode simulator containers (baked into the base manifest and re-added by the simulator overlay patch).
Relative paths in the coordinator's Go e2e test package were off by one directory level, so kustomize/manifest lookups resolved to the wrong location.
image-build-epp called a script that doesn't exist (scripts/build-epp-image.sh); it now falls through to the existing generic image-build-% pattern.
Docs: README.coord.md gains a Table of Contents, documents test-e2e-coordinator, E2E_KEEP_CLUSTER_ON_FAILURE, accessing the kind cluster's kubeconfig after a preserved failure, and the full e2e environment-variable table.

Depends on #1601 — once that merges, further cleanup can be applied here (e.g. sharing the vllm-render component with the router too, the same way vllm-encode/vllm-prefill already are)

Which issue(s) this PR fixes:

#1877

Release note (write NONE if no user-facing change):

NONE

roytman · 2026-07-12T13:31:32Z

@@ -3,6 +3,7 @@
 #
 # Encoder deployment for multimodal disaggregation scenarios (E/PD, E/P/D).
 # Includes a PVC for shared encoder embeddings cache.


can you remove this line? And rephrase the comments (we need PVC only for shared storage EC connector)

roytman · 2026-07-12T13:43:37Z

+- ../../../../components/vllm-encode/
+- ../../../../components/vllm-prefill/
+# vllm-decode is coordinator-local, not the shared deploy/components/vllm-decode: the
+# router's decode carries a routing-sidecar proxying 8000->8200 for connector-based P/D


Why can't we reuse the shared deploy/components/vllm-decode? it is similar as the epd scenario without the sidecar

roytman · 2026-07-12T15:20:43Z

@@ -1,23 +1,35 @@
 SHELL := /usr/bin/env bash

-# Image registry + dev-environment image tags (single source of truth).


should we return versions.mk?

Checked, it's not actually shared here, the original pull_images.sh read versions.mk, but this repo's version (reused from the router) doesn't. So it'd only have one consumer, same as keeping it inline. Can restore it if you'd rather match upstream's layout for easier syncing.

roytman · 2026-07-12T17:01:18Z

CI tests do not include the coordinator

roytman · 2026-07-12T19:26:16Z

We need to add rank-port topology (for DP)

revit13 · 2026-07-13T07:20:30Z

We need to add rank-port topology (for DP)

I will address it in a separate PR if thats OK.

revit13 · 2026-07-13T07:21:10Z

CI tests do not include the coordinator

I added a new workflow for the coordinator.

Signed-off-by: Revital Sur <eres@il.ibm.com>

Copilot

Pull request overview

This PR refactors the coordinator end-to-end (e2e) testing and deployment-manifest layout to align with the main router’s infrastructure, reduce duplicated YAML, and improve test reliability/maintainability.

Changes:

Containerizes test-e2e-coordinator to run fully inside the builder container (including Kind cluster lifecycle), matching the router e2e execution pattern.
Shares coordinator deploy components with the router where appropriate (CRDs + vLLM encode/prefill), while keeping coordinator-specific decode behavior via a local patch overlay.
Improves coordinator e2e configurability and cleanup diagnostics (namespace/process helpers, render image/port parameters, suite-failure pod/log dumping) and removes unused Gateway API CRD installation from coordinator e2e.

Reviewed changes

Copilot reviewed 29 out of 30 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
test/utils/parallel.go	Adds shared helpers for per-process port/namespace calculations and Kind `extraPortMappings` generation.
test/e2e/e2e_suite_test.go	Refactors router e2e suite to use shared parallel helpers and centralizes default ports.
test/coordinator/e2e/coordinator/utils_test.go	Updates coordinator e2e helpers to use per-process namespace and adds suite-failure pod/log dumping.
test/coordinator/e2e/coordinator/setup_test.go	Uses per-process namespace consistently, removes unused Gateway API CRD install, and updates coordinator/render substitutions.
test/coordinator/e2e/coordinator/pools_check_test.go	Ensures pool checks operate within the per-process namespace.
test/coordinator/e2e/coordinator/e2e_suite_test.go	Adds coordinator e2e parallel-process plumbing, Kind port mappings, and suite-failure diagnostics; switches coordinator service away from NodePort usage in tests.
test/coordinator/e2e/coordinator/coordinator_test.go	Routes requests via the gateway helper and passes the resolved namespace into log verification.
test/coordinator/e2e/coordinator/configs_test.go	Parameterizes render service address by `${VLLM_RENDER_PORT}`.
scripts/pull_images.sh	Adds a toggle to skip pulling the EPP image (useful when CI already loads artifacts).
README.coord.md	Adds a TOC and documents coordinator unit/e2e test workflows, env vars, and cluster preservation.
Makefile.coord.mk	Reworks coordinator build/test targets to run e2e inside the builder container with host networking + container socket wiring; updates image/tag handling.
Makefile	Excludes coordinator packages from the EPP unit-test package list.
deploy/coordinator/environments/dev/epd-pools/kustomization.yaml	Switches encode/prefill to shared components and documents why decode remains coordinator-local via patching.
deploy/coordinator/components/vllm-render/deployment.yaml	Parameterizes render image/port and removes unused container env vars.
deploy/coordinator/components/vllm-prefill/kustomization.yaml	Removes coordinator-local prefill kustomization (now shared).
deploy/coordinator/components/vllm-prefill/deployment.yaml	Removes duplicated coordinator-local prefill Deployment manifest.
deploy/coordinator/components/vllm-encode/kustomization.yaml	Removes coordinator-local encode kustomization (now shared).
deploy/coordinator/components/vllm-encode/deployment.yaml	Removes duplicated coordinator-local encode Deployment manifest.
deploy/coordinator/components/vllm-decode/patch-decode.yaml	Adds a patch overlay to remove the routing-sidecar and adapt decode ports for coordinator use.
deploy/coordinator/components/vllm-decode/kustomization.yaml	Converts decode to a shared-base + coordinator patch overlay.
deploy/coordinator/components/vllm-decode/deployment.yaml	Removes duplicated coordinator-local decode Deployment manifest.
deploy/coordinator/components/crds-gie/kustomization.yaml	Removes coordinator-local vendored GIE CRD kustomization (now referenced from shared components).
deploy/coordinator/components/crds-gie/gie.yaml	Removes coordinator-local vendored GIE CRD bundle.
deploy/coordinator/components/crds-gateway-api/kustomization.yaml	Removes coordinator-local vendored Gateway API CRD kustomization (no longer installed by coordinator e2e).
deploy/coordinator/components/coordinator/deployment.yaml	Changes coordinator Service from NodePort to ClusterIP for the coordinator component.
deploy/components/vllm-prefill/kustomization.yaml	Updates component header comment to note coordinator reuse.
deploy/components/vllm-encode/kustomization.yaml	Clarifies the PVC inclusion rationale in comments.
.github/workflows/ci-pr-checks.yaml	Excludes coordinator-only paths from the main PR checks workflow filter.
.github/workflows/ci-coordinator.yaml	Adds a dedicated CI workflow to run coordinator unit + e2e tests when coordinator-relevant paths change.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Revital Sur <eres@il.ibm.com>

roytman · 2026-07-14T17:23:20Z

+// ProcessPort returns the NodePort for the current Ginkgo parallel process for
+// a service whose first process uses basePort. When tests run in parallel,
+// each process gets its own NodePort of the form basePort + processPortOffset
+// * (the process number minus one), so process one gets exactly basePort.


The file has zero unit tests

No other file in test/utils/ has a unit test file. The unit test target (make test-unit) explicitly excludes the test/ subtree, so any *_test.go added here would not be picked up by it.
The functions that call ginkgo.GinkgoParallelProcess() (ProcessPort, Namespace) also can't be meaningfully tested outside a running Ginkgo suite. The pure helpers could have tests, but they'd need a separate make target to run.

roytman · 2026-07-14T17:28:01Z

 	modelName        = env.GetEnvString("MODEL_NAME", "Qwen/Qwen3-VL-2B-Instruct", ginkgo.GinkgoLogr)

-	nsName     = env.GetEnvString("NAMESPACE", "default", ginkgo.GinkgoLogr)
+	numProcesses = env.GetEnvInt("E2E_NUM_PROCS", 1, ginkgo.GinkgoLogr)


E2E_NUM_PROCS is never set by any workflow, and neither run script passes -ginkgo.procs/-p to go test. So numProcesses is always 1 in every CI run — the baseName-N / port+100*idx branches have never run, before or after this PR. This PR generalizes that already-dormant router code into a shared package and hands the coordinator suite the identical, still-unexercised capability. Pre-existing pattern, not introduced by this PR, but worth flagging since the PR explicitly reframes it as new "parallel test infrastructure" — infrastructure that's currently only proven at n=1.

Probably it is a part of #1976

grep -r E2E_NUM_PROCS * test/coordinator/e2e/coordinator/e2e_suite_test.go:92: numProcesses = env.GetEnvInt("E2E_NUM_PROCS", 1, ginkgo.GinkgoLogr) test/utils/parallel.go:71:// E2E_NUM_PROCS environment variable) matches the actual number of Ginkgo test/utils/parallel.go:76: ginkgo.Fail(fmt.Sprintf("The value of the environment variable `E2E_NUM_PROCS` (%d) is not equal to the number of ginkgo processes being run (%d)", test/e2e/e2e_suite_test.go:87: numProcesses = env.GetEnvInt("E2E_NUM_PROCS", 1, ginkgo.GinkgoLogr)

Right, as far as I know @shmuelk is planning to have a follow up PR.

roytman · 2026-07-15T10:55:24Z

patch-decode.yaml uses $patch: replace on the vllm container's ports list — correct today, but if the router ever adds a port to the shared vllm-decode base, this silently drops it with no test failure. A one-line comment noting the list is fully replaced (not merged) would save someone a confusing debug session later.
Neither ci-coordinator.yaml's nor ci-pr-checks.yaml's path filters include deploy/**, so a PR touching only a shared deploy component won't trigger either e2e suite. Pre-existing gap, but this PR is the point where a second consumer starts depending on that tree.
LocalhostURL in parallel.go is missing a doc comment that its sibling exported functions all have.
The new ci-coordinator.yaml builds builder+coordinator+epp images serially in one job, unlike the router's matrixed/cached image-build jobs in ci-pr-checks.yaml. Probably fine as a simpler first pass, flagging for awareness only.
This can be a part of a separate PR
crds-gie and vllm-decode shared kustomizations lack the "also used by the coordinator" heads-up comment that vllm-prefill got. A router contributor touching either has no signal that the coordinator now depends on them.

Signed-off-by: Revital Sur <eres@il.ibm.com>

roytman

/lgtm

elevran

Containerization and component-sharing are well-scoped - thanks.
See inline comments on the new copy of dumpPodsAndLogs/dumpContainerLogs.

elevran · 2026-07-16T12:17:30Z

+// dumpPodsAndLogs prints pod statuses and container logs for the given namespace
+// to the Ginkgo writer. Call this before cleanup to ensure the information is
+// available when CI tests fail.
+func dumpPodsAndLogs(nsName string) {


This introducs a duplication of dumpPodsAndLogs and the helper below from test/e2e/utils_test.go:487, which he PR otherwise works to eliminate.
Moving both helpers into test/utils (alongside parallel.go) keeps the dedupe theme consistent across the diff. Note that the router's copy has slightly richer signatures (previous bool, explicit context) but the minimum useful shared shape is func DumpPodsAndLogs(nsName string) matching this version.

elevran · 2026-07-16T12:17:30Z

+	})
+	stream, err := req.Stream(testConfig.Context)
+	if err != nil {
+		ginkgo.GinkgoWriter.Printf("(failed to fetch logs: %v)\n", err)


nit:
when GetLogs/Stream fails (often because cleanup races and the pod is gone), the dump prints (failed to fetch logs: %v) without the pod/container context. Wrap with the pod/container names so an operator scanning a failure dump can tell which container failed. Same suggestion applies to the existing router copy in test/e2e/utils_test.go:586.

Signed-off-by: Revital Sur <eres@il.ibm.com>

elevran · 2026-07-16T15:05:07Z

@roytman @revit13 see #2048 which settles on main instead of dev tags. Left note there.

capri-xiyue · 2026-07-17T19:30:29Z

 	objects = append(objects, testutils.CreateObjsFromYaml(testConfig, docs, nsName)...)

 	podsInDeploymentsReady(objects)
-	if k8sContext != "" {


why do we remove this?

This was the direct-NodePort path (the port-forward plus a second /readyz poll against coordinatorBaseURL()). Nothing exercises it anymore, all traffic goes through the Envoy gateway, so it was dead code. Readiness is still covered: podsInDeploymentsReady confirms the pod is up, and the gateway /readyz poll still guards against Envoy's STRICT_DNS resolution lagging the per-test Service. Removed coordinatorBaseURL/getCoordinatorPort too since they had no other callers.

capri-xiyue · 2026-07-17T19:33:06Z

+// DumpPodsAndLogs prints pod statuses and container logs for the given namespace
+// to the Ginkgo writer. Call this before cleanup to ensure the information is
+// available when CI tests fail.
+func DumpPodsAndLogs(cfg *TestConfig, nsName string) {


do you need UT for this?

Not sure about that. This is best-effort diagnostic output that runs on E2E failure, it has no return value, swallows all errors, and only affects how readable CI logs are, never a test result. It's exercised by the E2E suites themselves. Nothing else in test/utils (context, network, parallel, server, ...) is unit-tested either, so adding a UT just here would be inconsistent. A UT would mostly end up asserting on formatted column output, which is brittle and low-value. Happy to add one if you'd still like coverage, e.g. a fake.NewSimpleClientset case, but I'd lean against it.

capri-xiyue · 2026-07-17T19:47:28Z


 ```
-Client -> Coordinator -> Inference Gateway -> EPP -> vLLM Workers
+Client -> Inference Gateway -> Coordinator -> Inference Gateway -> EPP -> vLLM Workers


I think the diagram would be clearer if we used a single double-ended arrow between the Inference Gateway and the Coordinator, rather than writing it out as Inference Gateway -> Coordinator -> Inference Gateway which is quite confusing.

Signed-off-by: Revital Sur <eres@il.ibm.com>

Signed-off-by: Revital Sur <eres@il.ibm.com> # Conflicts: # test/coordinator/e2e/coordinator/e2e_suite_test.go

Signed-off-by: Revital Sur <eres@il.ibm.com>

elevran

@revit13 please open a follow up issue and PR to determine if and what shared build and CICD code can be leveraged. I have concerns about the additional variables and targets that might be duplicated across GH actions and Makefiles.

elevran · 2026-07-19T09:59:17Z

@@ -0,0 +1,106 @@
+name: CI - Coordinator Test


should CI for coordinator share the existing CICD files instead of dedicated actions?
Just wonder how much is duplicated and whether the can be combined instead of introducing new actions and merge gates.

elevran · 2026-07-19T10:00:39Z

              - .github/actions/docker-build-and-push/action.yml
              - .github/actions/e2e-runner-setup/action.yml
              - .github/workflows/ci-pr-checks.yaml
+              - '!Dockerfile.coordinator'


are these consistent with the previous items? None contain ! prefixes.

elevran · 2026-07-19T10:02:48Z

@@ -1,23 +1,35 @@
 SHELL := /usr/bin/env bash


can you please confirm the need for maintaining a coordinator makefile vs inlining targets into existing makefiles?

revit13 requested a review from a team as a code owner July 12, 2026 12:07

revit13 requested review from elevran and vMaroon July 12, 2026 12:07

revit13 marked this pull request as draft July 12, 2026 12:07

github-actions Bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jul 12, 2026

roytman reviewed Jul 12, 2026

View reviewed changes

revit13 force-pushed the coordinator-e2e branch 2 times, most recently from 02cff23 to d2bffe6 Compare July 13, 2026 07:46

revit13 added 6 commits July 13, 2026 11:27

[coordinator] e2e tests: containerize and share deploy components

50606ea

Signed-off-by: Revital Sur <eres@il.ibm.com>

Address review comments.

9c546fc

Signed-off-by: Revital Sur <eres@il.ibm.com>

Address review comments and add github workflow test.

ee93866

Signed-off-by: Revital Sur <eres@il.ibm.com>

Minor fix.

552e270

Signed-off-by: Revital Sur <eres@il.ibm.com>

Additional change.

84d0041

Signed-off-by: Revital Sur <eres@il.ibm.com>

Skip EPP and sidecar image pulls in coordinator e2e.

38a6e0c

Signed-off-by: Revital Sur <eres@il.ibm.com>

revit13 force-pushed the coordinator-e2e branch from d2bffe6 to 38a6e0c Compare July 13, 2026 08:27

revit13 marked this pull request as ready for review July 13, 2026 09:04

revit13 added 8 commits July 13, 2026 22:12

Merge remote-tracking branch 'origin/main' into coordinator-e2e

a1d28ad

Signed-off-by: Revital Sur <eres@il.ibm.com>

Add test-e2e-coordinator-run target.

dd54d40

Signed-off-by: Revital Sur <eres@il.ibm.com>

Mirror the router's parallel-testing infrastructure.

aa03ab3

Signed-off-by: Revital Sur <eres@il.ibm.com>

Fix line error.

1090ff1

Signed-off-by: Revital Sur <eres@il.ibm.com>

Merge branch 'main' into coordinator-e2e

89bceae

Signed-off-by: Revital Sur <eres@il.ibm.com>

Use constants for ports.

434c04a

Signed-off-by: Revital Sur <eres@il.ibm.com>

Drop unused direct NodePort access to the coordinator

ba5a836

Signed-off-by: Revital Sur <eres@il.ibm.com>

Fix comments.

e34ebe7

Signed-off-by: Revital Sur <eres@il.ibm.com>

roytman requested a review from Copilot July 14, 2026 13:38

Copilot started reviewing on behalf of roytman July 14, 2026 13:40 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Comment thread test/coordinator/e2e/coordinator/utils_test.go Outdated

revit13 and others added 2 commits July 14, 2026 16:50

Potential fix for pull request finding

4f7b1d7

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Revital Sur <eres@il.ibm.com>

Merge branch 'main' into coordinator-e2e

15321da

roytman reviewed Jul 15, 2026

View reviewed changes

revit13 added 2 commits July 15, 2026 14:49

Address revie comments.

487b1a3

Signed-off-by: Revital Sur <eres@il.ibm.com>

Merge branch 'main' into coordinator-e2e

e281824

roytman previously approved these changes Jul 16, 2026

View reviewed changes

elevran reviewed Jul 16, 2026

View reviewed changes

elevran self-assigned this Jul 16, 2026

Address review comments.

66d1145

Signed-off-by: Revital Sur <eres@il.ibm.com>

revit13 dismissed roytman’s stale review via 66d1145 July 16, 2026 14:27

Merge branch 'main' into coordinator-e2e

9c44925

elevran mentioned this pull request Jul 16, 2026

Consolidating images and artifacts #2048

Merged

Merge branch 'main' into coordinator-e2e

b6b2021

capri-xiyue reviewed Jul 17, 2026

View reviewed changes

revit13 added 3 commits July 19, 2026 07:31

Address review comments.

00432f9

Signed-off-by: Revital Sur <eres@il.ibm.com>

Merge branch 'main' into coordinator-e2e

80fe028

Signed-off-by: Revital Sur <eres@il.ibm.com> # Conflicts: # test/coordinator/e2e/coordinator/e2e_suite_test.go

Update simulator tag in coord resources.

33d0ae4

Signed-off-by: Revital Sur <eres@il.ibm.com>

elevran approved these changes Jul 19, 2026

View reviewed changes

elevran reviewed Jul 19, 2026

View reviewed changes

shmuelk merged commit b418271 into llm-d:main Jul 19, 2026
35 checks passed

		@@ -1,23 +1,35 @@
		SHELL := /usr/bin/env bash

		# Image registry + dev-environment image tags (single source of truth).

Uh oh!

Conversation

revit13 commented Jul 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

roytman commented Jul 12, 2026

Uh oh!

roytman commented Jul 12, 2026

Uh oh!

revit13 commented Jul 13, 2026

Uh oh!

revit13 commented Jul 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

roytman commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

roytman left a comment

Choose a reason for hiding this comment

Uh oh!

elevran left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elevran commented Jul 16, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elevran left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

revit13 commented Jul 12, 2026 •

edited

Loading

roytman commented Jul 15, 2026 •

edited

Loading