-
Notifications
You must be signed in to change notification settings - Fork 292
[coordinator] e2e tests: containerize and share deploy components #1975
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
50606ea
9c546fc
ee93866
552e270
84d0041
38a6e0c
a1d28ad
dd54d40
aa03ab3
1090ff1
89bceae
434c04a
ba5a836
e34ebe7
4f7b1d7
15321da
487b1a3
e281824
66d1145
9c44925
b6b2021
00432f9
80fe028
33d0ae4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,106 @@ | ||
| name: CI - Coordinator Test | ||
|
|
||
| # Runs coordinator unit tests and e2e tests. Path filter excludes paths not | ||
| # exercised by the coordinator. | ||
| on: | ||
| push: | ||
| branches: | ||
| - main | ||
| - 'release-*' | ||
| pull_request: | ||
| branches: | ||
| - main | ||
|
|
||
| permissions: | ||
| contents: read | ||
| actions: read | ||
|
|
||
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.event.pull_request.number || github.ref }} | ||
| cancel-in-progress: true | ||
|
|
||
| jobs: | ||
| check-changes: | ||
| runs-on: ubuntu-latest | ||
| outputs: | ||
| src: ${{ steps.filter.outputs.src }} | ||
| steps: | ||
| - name: Checkout source | ||
| uses: actions/checkout@v7 | ||
| - uses: dorny/paths-filter@v4 | ||
| id: filter | ||
| with: | ||
| filters: | | ||
| src: | ||
| - '**/*.go' | ||
| - Dockerfile.* | ||
| - Makefile* | ||
| - go.mod | ||
| - go.sum | ||
| - scripts/** | ||
| - hack/** | ||
| - test/** | ||
| - .github/actions/e2e-runner-setup/action.yml | ||
| - .github/workflows/ci-coordinator.yaml | ||
| - '!Dockerfile.sidecar' | ||
| - '!cmd/pd-sidecar/**' | ||
| - '!pkg/sidecar/**' | ||
| - '!test/sidecar/**' | ||
| - '!pkg/generator/**' | ||
| - '!test/e2e/**' | ||
| - '!test/integration/**' | ||
| - '!test/profiling/**' | ||
| - '!Makefile' | ||
|
|
||
| unit-tests: | ||
| needs: check-changes | ||
| if: ${{ needs.check-changes.outputs.src == 'true' }} | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 30 | ||
| steps: | ||
| - name: Checkout source | ||
| uses: actions/checkout@v7 | ||
|
|
||
| - name: Create Go cache dirs | ||
| id: go-cache | ||
| run: | | ||
| mkdir -p "$HOME/.cache/llm-d-gomodcache" "$HOME/.cache/llm-d-gobuildcache" | ||
| echo "mod=$HOME/.cache/llm-d-gomodcache" >> "$GITHUB_OUTPUT" | ||
| echo "build=$HOME/.cache/llm-d-gobuildcache" >> "$GITHUB_OUTPUT" | ||
|
|
||
| - name: Cache Go modules and build cache | ||
| uses: actions/cache@v6 | ||
| with: | ||
| path: | | ||
| ${{ steps.go-cache.outputs.mod }} | ||
| ${{ steps.go-cache.outputs.build }} | ||
| key: go-cache-${{ runner.os }}-${{ hashFiles('go.sum') }} | ||
| restore-keys: | | ||
| go-cache-${{ runner.os }}- | ||
|
|
||
| - name: Run coordinator unit tests | ||
| shell: bash | ||
| env: | ||
| GO_MOD_CACHE_VOL: ${{ steps.go-cache.outputs.mod }} | ||
| GO_BUILD_CACHE_VOL: ${{ steps.go-cache.outputs.build }} | ||
| run: make -f Makefile.coord.mk test-unit | ||
|
|
||
| e2e-tests: | ||
| needs: check-changes | ||
| if: ${{ needs.check-changes.outputs.src == 'true' }} | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 60 | ||
| steps: | ||
| - name: Checkout source | ||
| uses: actions/checkout@v7 | ||
|
|
||
| - name: Set up e2e runner | ||
| id: e2e-setup | ||
| uses: ./.github/actions/e2e-runner-setup | ||
|
|
||
| - name: Run coordinator e2e tests | ||
| shell: bash | ||
| env: | ||
| GO_MOD_CACHE_VOL: ${{ steps.e2e-setup.outputs.go-mod-cache }} | ||
| GO_BUILD_CACHE_VOL: ${{ steps.e2e-setup.outputs.go-build-cache }} | ||
| run: make -f Makefile.coord.mk test-e2e-coordinator | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,6 +45,11 @@ jobs: | |
| - .github/actions/docker-build-and-push/action.yml | ||
| - .github/actions/e2e-runner-setup/action.yml | ||
| - .github/workflows/ci-pr-checks.yaml | ||
| - '!Dockerfile.coordinator' | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are these consistent with the previous items? None contain |
||
| - '!Makefile.coord.mk' | ||
| - '!cmd/coordinator/**' | ||
| - '!pkg/coordinator/**' | ||
| - '!test/coordinator/**' | ||
| helm: | ||
| - 'config/charts/**' | ||
| - 'hack/verify-helm.sh' | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,23 +1,35 @@ | ||
| SHELL := /usr/bin/env bash | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you please confirm the need for maintaining a coordinator makefile vs inlining targets into existing makefiles? |
||
|
|
||
| # Image registry + dev-environment image tags (single source of truth). | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we return versions.mk?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| include versions.mk | ||
|
|
||
| # Export all dev-env image references so the e2e suite sees them. | ||
| export IMAGE_REGISTRY COORDINATOR_TAG VLLM_SIMULATOR_TAG EPP_TAG | ||
| export COORDINATOR_IMAGE VLLM_IMAGE EPP_IMAGE | ||
|
|
||
| # Defaults | ||
| TARGETOS ?= $(shell command -v go >/dev/null 2>&1 && go env GOOS || uname -s | tr '[:upper:]' '[:lower:]') | ||
| TARGETARCH ?= $(shell command -v go >/dev/null 2>&1 && go env GOARCH || uname -m | sed 's/x86_64/amd64/; s/aarch64/arm64/; s/armv7l/arm/') | ||
| PROJECT_NAME ?= llm-d-coordinator | ||
| BUILDER_IMAGE_NAME ?= llm-d-coordinator-builder | ||
| IMAGE_REGISTRY ?= ghcr.io/llm-d | ||
|
|
||
| # Image tags | ||
| COORDINATOR_TAG ?= dev | ||
| VLLM_SIMULATOR_TAG ?= v0.10.2 | ||
| EPP_TAG ?= dev | ||
|
|
||
| # Full image references (derived; override only if you need a non-standard repo) | ||
| COORDINATOR_IMAGE ?= $(IMAGE_REGISTRY)/llm-d-coordinator:$(COORDINATOR_TAG) | ||
| VLLM_IMAGE ?= $(IMAGE_REGISTRY)/llm-d-inference-sim:$(VLLM_SIMULATOR_TAG) | ||
| EPP_IMAGE ?= $(IMAGE_REGISTRY)/llm-d-router-endpoint-picker:$(EPP_TAG) | ||
|
|
||
| # vllm-render defaults to the same image as the other simulated vLLM roles; override | ||
| # independently to point it at a real vLLM image (e.g. vllm/vllm-openai-cpu:v0.21.0). | ||
| VLLM_RENDER_IMAGE ?= $(VLLM_IMAGE) | ||
| VLLM_RENDER_PORT ?= 8082 | ||
|
|
||
| # Internal variable mappings for the generic image-build-% target. | ||
| coordinator_IMAGE = $(COORDINATOR_IMAGE) | ||
| epp_IMAGE = $(EPP_IMAGE) | ||
|
|
||
| # Export all dev-env image references so the e2e suite sees them. | ||
| export IMAGE_REGISTRY COORDINATOR_TAG VLLM_SIMULATOR_TAG EPP_TAG | ||
| export COORDINATOR_IMAGE VLLM_IMAGE EPP_IMAGE VLLM_RENDER_IMAGE VLLM_RENDER_PORT | ||
|
|
||
| BUILDER_TAG ?= dev | ||
| BUILDER_TAG_BASE ?= $(IMAGE_REGISTRY)/$(BUILDER_IMAGE_NAME) | ||
| export BUILDER_IMAGE ?= $(BUILDER_TAG_BASE):$(BUILDER_TAG) | ||
|
|
@@ -39,7 +51,7 @@ LINT_NEW_ONLY ?= false | |
| # Optional: override the runtime base image used in container builds. | ||
| BASE_IMAGE ?= | ||
|
|
||
| TEST_PACKAGES = $$(go list ./... | grep -v /test/ | tr '\n' ' ') | ||
| TEST_PACKAGES = $$(go list ./pkg/coordinator/... ./cmd/coordinator/... | tr '\n' ' ') | ||
|
|
||
| # Common flags for running the builder container: mounts source, Go caches, and runs as current user. | ||
| # Podman rootless requires --userns=keep-id to correctly map host UID; docker uses -u directly. | ||
|
|
@@ -56,11 +68,64 @@ endif | |
|
|
||
| BUILDER_RUN_FLAGS = --rm $(BUILDER_USER_FLAGS) \ | ||
| -v $$(pwd):/app:Z -w /app \ | ||
| -v $(GO_MOD_CACHE_VOL):/go/pkg/mod \ | ||
| -v $(GO_BUILD_CACHE_VOL):/go/cache | ||
| -v $(GO_MOD_CACHE_VOL):/go/pkg/mod:z \ | ||
| -v $(GO_BUILD_CACHE_VOL):/go/cache:z | ||
|
|
||
| BUILDER_RUN = $(CONTAINER_RUNTIME) run $(BUILDER_RUN_FLAGS) $(BUILDER_IMAGE) sh -c | ||
|
|
||
| # Mount the container runtime socket and set CONTAINER_HOST so podman --remote | ||
| # inside the builder can talk to the host's container runtime. | ||
| ifeq ($(CONTAINER_RUNTIME),podman) | ||
| CONTAINER_SOCK ?= $(or $(shell podman info --format '{{.Host.RemoteSocket.Path}}' 2>/dev/null | sed 's|^unix://||'),/run/podman/podman.sock) | ||
| BUILDER_SOCK_FLAGS = --security-opt label=disable \ | ||
| -v $(CONTAINER_SOCK):$(CONTAINER_SOCK) \ | ||
| -e CONTAINER_HOST=unix://$(CONTAINER_SOCK) \ | ||
| -e DOCKER_HOST=unix://$(CONTAINER_SOCK) \ | ||
| -e CONTAINER_RUNTIME=podman \ | ||
| -e KIND_EXPERIMENTAL_PROVIDER=podman | ||
| else | ||
| CONTAINER_SOCK ?= /var/run/docker.sock | ||
| ifeq ($(TARGETOS),darwin) | ||
| DOCKER_SOCK_GID := $(shell stat -f '%g' $(CONTAINER_SOCK) 2>/dev/null) | ||
| else | ||
| DOCKER_SOCK_GID := $(shell stat -c '%g' $(CONTAINER_SOCK) 2>/dev/null) | ||
| endif | ||
| ifneq ($(DOCKER_SOCK_GID),) | ||
| DOCKER_GROUP_PARAM := --group-add $(DOCKER_SOCK_GID) | ||
| else | ||
| DOCKER_GROUP_PARAM := | ||
| endif | ||
| BUILDER_SOCK_FLAGS = $(DOCKER_GROUP_PARAM) \ | ||
| -v $(CONTAINER_SOCK):$(CONTAINER_SOCK) \ | ||
| -e DOCKER_HOST=unix://$(CONTAINER_SOCK) \ | ||
| -e CONTAINER_RUNTIME=docker | ||
| endif | ||
|
|
||
| # Respect host KUBECONFIG if set; fall back to ~/.kube/config. | ||
| HOST_KUBECONFIG ?= $(or $(KUBECONFIG),$(HOME)/.kube/config) | ||
|
|
||
| # When K8S_CONTEXT is set, mount the host kubeconfig so the e2e suite can call | ||
| # config.GetConfigWithContext(K8S_CONTEXT) against an existing cluster instead of | ||
| # creating a new kind cluster. | ||
| ifdef K8S_CONTEXT | ||
| BUILDER_E2E_KUBECONFIG_FLAGS = -v $(HOST_KUBECONFIG):/.kube/config:ro -e KUBECONFIG=/.kube/config | ||
| else | ||
| BUILDER_E2E_KUBECONFIG_FLAGS = | ||
| endif | ||
|
|
||
| # Env vars forwarded into the e2e test container. | ||
| E2E_ENV_VARS = COORDINATOR_IMAGE VLLM_IMAGE EPP_IMAGE VLLM_RENDER_IMAGE VLLM_RENDER_PORT \ | ||
| E2E_GATEWAY_PORT E2E_KEEP_CLUSTER_ON_FAILURE \ | ||
| E2E_PRINT_COORDINATOR_LOGS K8S_CONTEXT READY_TIMEOUT MODEL_NAME | ||
| BUILDER_E2E_ENV_FLAGS = $(foreach v,$(E2E_ENV_VARS),$(if $($(v)),-e '$(v)=$($(v))')) | ||
| ifneq ($(filter command line environment,$(origin NAMESPACE)),) | ||
| BUILDER_E2E_ENV_FLAGS += -e NAMESPACE=$(NAMESPACE) | ||
| endif | ||
|
|
||
| # E2e tests create their own kind cluster, need host network (for NodePort access) | ||
| # and the container socket (for kind), but not the host kubeconfig. | ||
| BUILDER_E2E_FLAGS = --network=host $(BUILDER_SOCK_FLAGS) $(BUILDER_E2E_ENV_FLAGS) $(BUILDER_E2E_KUBECONFIG_FLAGS) | ||
|
|
||
| BUILDER_STAMP = build/.builder.stamp | ||
|
|
||
| .PHONY: help | ||
|
|
@@ -130,9 +195,15 @@ test-unit: image-build-builder | |
| @printf "\033[33;1m==== Running Unit Tests ====\033[0m\n" | ||
| $(BUILDER_RUN) "go test -v -race $(TEST_PACKAGES)" | ||
|
|
||
| .PHONY: test-e2e-coordinator-run | ||
| test-e2e-coordinator-run: image-pull ## Ensure images are present, then run coordinator e2e tests | ||
| @printf "\033[33;1m==== Running Coordinator End to End Tests ====\033[0m\n" | ||
| $(CONTAINER_RUNTIME) run $(BUILDER_RUN_FLAGS) $(BUILDER_E2E_FLAGS) \ | ||
| $(BUILDER_IMAGE) test/coordinator/scripts/run_e2e_coordinator.sh | ||
|
|
||
| .PHONY: test-e2e-coordinator | ||
| test-e2e-coordinator: image-build-coordinator image-build-epp image-build-builder image-pull ## Run coordinator e2e tests against a new kind cluster | ||
| test/coordinator/scripts/run_e2e_coordinator.sh | ||
| test-e2e-coordinator: image-build-builder image-build-coordinator image-build-epp ## Build images and run coordinator e2e tests | ||
| $(MAKE) -f Makefile.coord.mk test-e2e-coordinator-run | ||
|
|
||
| .PHONY: build | ||
| build: image-build-builder ## Build the coordinator binary | ||
|
|
@@ -182,12 +253,8 @@ image-build-builder: check-container-tool ## Build builder image if missing loca | |
| touch $(BUILDER_STAMP); \ | ||
| fi | ||
|
|
||
| .PHONY: image-build-epp | ||
| image-build-epp: ## Clone llm-d-inference-scheduler at pinned commit and build EPP image | ||
| scripts/build-epp-image.sh | ||
|
|
||
| .PHONY: image-pull | ||
| image-pull: check-container-tool ## Pull all related images using $(CONTAINER_RUNTIME) | ||
| @printf "\033[33;1m==== Pulling Container images ====\033[0m\n" | ||
| ./scripts/pull_images.sh | ||
| PULL_EPP_IMAGE=false PULL_SIDECAR_IMAGE=false ./scripts/pull_images.sh | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.