Skip to content

Conversation

@zdtsw
Copy link
Contributor

@zdtsw zdtsw commented Nov 19, 2025

Changelist

  • split Makefile into 3 different parts:
  1. tools+dependency - including install tools, check tools, download dependency(gcc etc) and tokenizer
  2. cluster(including k8s and ocp) : deploy resource, apply rbac, etc
  3. kind: only "kind" related tasks
  • download tools to "bin" folder instead of using global ones, avoid version conflicts
  • rename "openshift-base" folder to "kubernetes-base"
  • uplift go lint version from 1.62.0 to 2.1.6 to algin with GitHub Action's setting
  • rename make target for better usage, deprecating old targets
  • add more env variable in "make env"

Result

>make help
Usage:
  make <target>
  help             Print help

Development
  clean            Clean build artifacts, tools and caches
  format           Format Go source files
  lint             Run lint
  install-hooks    Install git hooks
  test             Run all tests (unit and e2e)
  test-unit        Run unit tests
  test-integration  Run integration tests
  test-e2e         Run end-to-end tests against a new kind cluster

Build
  build            Build the project for both epp and sidecar

Container image Build/Push/Pull
  image-build      Build Container image using $(CONTAINER_RUNTIME)
  image-push       Push container images to registry using $(CONTAINER_RUNTIME)
  image-pull       Pull all related images using $(CONTAINER_RUNTIME)

Container Run
  run-container    Run app in container using $(CONTAINER_RUNTIME)
  stop-container   Stop and remove container

Environment
  env              Print environment variables
  print-namespace  Print the current namespace
  print-project-name  Print the current project name

Deprecated aliases for backwards compatibility
  install-docker   DEPRECATED: Use 'make run-container' instead
  uninstall-docker  DEPRECATED: Use 'make stop-container' instead
  install          DEPRECATED: Use 'make run-container' instead
  uninstall        DEPRECATED: Use 'make stop-container' instead

Tools
  install-tools    Install all development tools and dependencies
  install-dependencies  Install development dependencies based on OS/ARCH
  check-tools      Check that all required tools are installed

Cluster Development Environments
  env-dev-kubernetes  Deploy full dev environment (vLLM + Gateway + EPP) to K8s/OpenShift cluster
  clean-env-dev-kubernetes  Clean up full dev environment from K8s/OpenShift cluster

RBAC Targets
  install-rbac     Apply RBAC configuration to cluster
  uninstall-rbac   Remove RBAC configuration from cluster

Kubernetes Targets
  install-k8s      Deploy resources to Kubernetes
  uninstall-k8s    Remove resources from Kubernetes

OpenShift Targets
  install-openshift  Deploy resources to OpenShift
  uninstall-openshift  Remove resources from OpenShift

Kind Development Environments
  env-dev-kind     Run under kind ($(KIND_CLUSTER_NAME))
  clean-env-dev-kind  Cleanup kind setup (delete cluster $(KIND_CLUSTER_NAME))

Alias checking
  check-alias      Check if the container alias works correctly

Test

  • enable pre-commit
  • run "make test" passed unit and e2e
- ==== Running Unit Tests ====
=== RUN   TestProxy
Running Suite: Proxy Suite - /home/wenzhou/upstream/llm-d/llm-d-inference-scheduler/pkg/sidecar/proxy
=====================================================================================================
Random Seed: 1763545447

Will run 22 of 22 specs
••••••••••••••••••••••

Ran 22 of 22 Specs in 22.745 seconds
SUCCESS! -- 22 Passed | 0 Failed | 0 Pending | 0 Skipped
- Ran 5 of 5 Specs in 137.447 seconds
SUCCESS! -- 5 Passed | 0 Failed | 0 Pending | 0 Skipped
--- PASS: TestEndToEnd (137.45s)
PASS
ok      github.com/llm-d/llm-d-inference-scheduler/test/e2e     137.461s
✔️  All checks passed!

@shmuelk
Copy link
Collaborator

shmuelk commented Nov 19, 2025

This PR will need re-work once PR #453 merges.

@zdtsw
Copy link
Contributor Author

zdtsw commented Nov 19, 2025

This PR will need re-work once PR #453 merges.

thanks for the headsup, mark PR to draft for now.

@zdtsw zdtsw marked this pull request as draft November 19, 2025 13:44
@zdtsw zdtsw marked this pull request as ready for review November 20, 2025 11:08
@zdtsw zdtsw force-pushed the refactor_1 branch 2 times, most recently from e492d43 to 3b7888e Compare November 20, 2025 13:53
@zdtsw
Copy link
Contributor Author

zdtsw commented Nov 21, 2025

This PR will need re-work once PR #453 merges.

@shmuelk if you might have time to give a review?

```

> [!NOTE]
> The full image reference will be constructed as `${EPP_IMAGE}:${EPP_TAG}`, where `EPP_IMAGE` defaults to `${IMAGE_REGISTRY}/llm-d-inference-scheduler`. For example, with `IMAGE_REGISTRY=quay.io/<my-id>` and `EPP_TAG=v1.0.0`, the final image will be `quay.io/<my-id>/llm-d-inference-scheduler:v1.0.0`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

EPP_IMAGE is the entire image including the tag

It defaults to ${IMAGE_REGISTRY}/llm-d-inference-scheduler:${EPP_TAG}, where EPP_TAG defaults to dev

**1. Setting the EPP image and tag:**

You can optionally set a custom EPP image (otherwise, the default will be used):
You can optionally set a custom EPP image and tag (otherwise, defaults will be used):
Copy link
Collaborator

Choose a reason for hiding this comment

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

See comment below on EPP_IMAGE

GATEWAY_HOST_PORT=$(KIND_GATEWAY_HOST_PORT) \
IMAGE_REGISTRY=$(IMAGE_REGISTRY) \
EPP_IMAGE=$(EPP_IMAGE) \
EPP_TAG=$(EPP_TAG) \
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is no need to pass in EPP_TAG

EPP_IMAGE=$(EPP_IMAGE) \
EPP_TAG=$(EPP_TAG) \
VLLM_SIMULATOR_IMAGE=${VLLM_SIMULATOR_IMAGE} \
VLLM_SIMULATOR_TAG=$(VLLM_SIMULATOR_TAG) \
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is no need to pass in VLLM_SIMULATOR_TAG

VLLM_SIMULATOR_IMAGE=${VLLM_SIMULATOR_IMAGE} \
VLLM_SIMULATOR_TAG=$(VLLM_SIMULATOR_TAG) \
SIDECAR_IMAGE=$(SIDECAR_IMAGE) \
SIDECAR_TAG=$(SIDECAR_TAG) \
Copy link
Collaborator

Choose a reason for hiding this comment

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

There is no need to pass in SIDECAR_TAG

GOLANGCI_LINT_VERSION ?= v2.1.6
KUSTOMIZE_VERSION ?= v5.5.0
TYPOS_VERSION ?= v1.34.0
VLLM_SIMULATOR_TAG ?= v0.6.1
Copy link
Collaborator

Choose a reason for hiding this comment

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

I want all of the images setup in the main Makefile

You need to export VLLM_SIMULATOR_IMAGE and no VLLM_SIMULATOR_TAG

@shmuelk
Copy link
Collaborator

shmuelk commented Nov 23, 2025

This PR was not correctly updated to reflect the changes in PR #453

@zdtsw
Copy link
Contributor Author

zdtsw commented Nov 23, 2025

This PR was not correctly updated to reflect the changes in PR #453

Thanks for the review!
I've addressed them in new commit

@zdtsw zdtsw requested a review from shmuelk November 24, 2025 06:30
@zdtsw zdtsw force-pushed the refactor_1 branch 2 times, most recently from 24a5342 to 200a48a Compare November 24, 2025 12:50
@zdtsw zdtsw changed the title refactor: Makefile, update docs, fix Go version refactor: Makefile, update docs Nov 25, 2025
@zdtsw zdtsw force-pushed the refactor_1 branch 2 times, most recently from b13ab07 to 319a637 Compare November 26, 2025 15:22
- split Makefile
  1. tools: include install tools, check tools, download dependency(gcc
     etc) and tokenizer. these will be download into "bin" folder than
     global path
  2. cluster: include k8s and ocp
  3. kind
- rename "openshift-base" to "kubernetes-base" to be clear for purpose
- uplift Go lint version to 2.1.6 to align with the same one set in
  Github Action
- rename make targets for better visibility, deprcating old ones
- add more print in "make env"

Signed-off-by: Wen Zhou <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

2 participants