make workloadmanager build more explicit by hzxuzhonghu · Pull Request #264 · volcano-sh/agentcube

hzxuzhonghu · 2026-04-09T09:41:13Z

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

make workloadmanager build more clear
remove running locally, because of port conflict

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Starting in Kubernetes v1.36, the DRAResourceClaimGranularStatusAuthorization feature gate (beta, on-by-default) enforces fine-grained authorization checks for ResourceClaim status updates. Schedulers that modify status.allocation or status.reservedFor now require additional permissions on the resourceclaims/binding synthetic subresource. This adds the required RBAC rule for the agent scheduler. These permissions are inert on Kubernetes versions prior to v1.36. Ref: kubernetes/kubernetes#138149 Signed-off-by: Cairon <cairon-ab@users.noreply.github.com>

Copilot

Pull request overview

This PR updates build/dev tooling so the WorkloadManager image build is called out explicitly, and removes local “run” Makefile targets (per the stated port-conflict concern), while aligning E2E and developer docs with the new workflow.

Changes:

Rename the WorkloadManager image build/push targets to docker-*-workloadmanager and update E2E script/docs to use them.
Simplify Kind image loading via make kind-load (now loads workloadmanager/router/picod) and remove kind-load-router.
Extend Volcano agent-scheduler (development) RBAC to allow updates/patches to resourceclaims/binding.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
test/e2e/run_e2e.sh	Use `make docker-build-workloadmanager` during E2E image build step.
test/e2e/README.md	Document updated E2E image build command set.
manifests/charts/base/templates/volcano-agent-scheduler-development.yaml	Add RBAC rule for `resourceclaims/binding` updates/patches.
Makefile	Rename WorkloadManager docker build/push targets; remove local run/install targets; make `kind-load` load all three images.
docs/agentcube/docs/developer-guide/local-development.md	Update local dev instructions to the new image build/load commands and remove local “run” section.
docker/Dockerfile.workloadmanager	No functional change (appears to be a no-op rewrite).

Copilot · 2026-04-09T09:44:01Z

 ### Build Specific Components

- **Workload Manager**: `make build`
+- **Workload Manager**: `make build-workloadmanager`


make build-workloadmanager is referenced here, but the Makefile only defines build (which builds the workloadmanager). Either update this doc to reference make build, or add a build-workloadmanager target/alias in the Makefile to match the documentation and the PR goal of making the build more explicit.

Suggested change

- **Workload Manager**: `make build-workloadmanager`

- **Workload Manager**: `make build`

codecov-commenter · 2026-04-09T09:47:31Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 43.70%. Comparing base (845b798) to head (05c8ae3).
⚠️ Report is 186 commits behind head on main.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #264      +/-   ##
==========================================
+ Coverage   35.60%   43.70%   +8.09%     
==========================================
  Files          29       30       +1     
  Lines        2533     2755     +222     
==========================================
+ Hits          902     1204     +302     
+ Misses       1505     1419      -86     
- Partials      126      132       +6

Flag	Coverage Δ
unittests	`43.70% <ø> (+8.09%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

gemini-code-assist

Code Review

This pull request refactors the Makefile to streamline build and Docker image management, including renaming docker-build targets for clarity and consolidating kind-load into a single command for all images. It also updates the volcano-agent-scheduler role with additional permissions for resourceclaims/binding. The documentation and e2e tests have been updated to reflect these Makefile changes. Feedback suggests re-introducing multi-architecture build targets without push for local development and adding a docker-build-all target to simplify building all Docker images.

gemini-code-assist · 2026-04-09T09:58:57Z

-docker-buildx:
-	@echo "Building multi-architecture Docker image..."
-	docker buildx build -f docker/Dockerfile --platform linux/amd64,linux/arm64 -t $(WORKLOAD_MANAGER_IMAGE) .
+	docker build -f docker/Dockerfile.workloadmanager -t $(WORKLOAD_MANAGER_IMAGE) .


The docker-buildx, docker-buildx-router, and docker-buildx-picod targets for building multi-architecture images without pushing have been removed. This removes potentially useful functionality for developers who want to build and test multi-arch images locally without pushing to a registry. Please consider re-adding these targets with the new naming convention, for example:

# Multi-architecture build (supports amd64, arm64) docker-buildx-workloadmanager: @echo "Building multi-architecture Docker image..." docker buildx build -f docker/Dockerfile.workloadmanager --platform linux/amd64,linux/arm64 -t $(WORKLOAD_MANAGER_IMAGE) .

Similar targets could be added for router and picod.

gemini-code-assist · 2026-04-09T09:58:57Z

 # Picod Docker targets
 docker-build-picod:
 	@echo "Building Picod Docker image..."
 	docker build -f docker/Dockerfile.picod -t $(PICOD_IMAGE) .


To improve developer experience, consider adding a target to build all docker images at once, similar to build-all. This would complement the change I've suggested in docs/agentcube/docs/developer-guide/local-development.md. You could add this after the docker-build-picod target:

docker-build-all: docker-build-workloadmanager docker-build-router docker-build-picod ## Build all docker images

gemini-code-assist · 2026-04-09T09:58:57Z

+make docker-build-workloadmanager
+make docker-build-router
+make docker-build-picod


It would be more convenient for users to build all Docker images with a single command. I've made a suggestion on the Makefile to add a docker-build-all target. If that is implemented, this documentation can be simplified.

Suggested change

make docker-build-workloadmanager

make docker-build-router

make docker-build-picod

make docker-build-all

Signed-off-by: Zhonghu Xu <xuzhonghu@huawei.com>

Copilot

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.

Copilot · 2026-04-14T03:04:17Z

-	go run ./cmd/router/main.go \
-		--port=8080 \
-		--debug
+build-all: build-workloadmanager build-agentd build-router


build-all still depends on the removed build target, so make build-all (and default make all) will fail. Update this dependency to build-workloadmanager (or reintroduce an alias target) to keep builds working.

Copilot · 2026-04-14T03:04:17Z

@@ -68,7 +69,7 @@ gen-check: gen-all ## Check if generated code is up to date
 .PHONY: build run clean test deps


The .PHONY declaration still lists build and run, but those targets no longer exist. This can be confusing for maintainers and makes it harder to reason about intended targets; update the .PHONY list to match the current target set.

Suggested change

.PHONY: build run clean test deps

.PHONY: clean test deps

Copilot · 2026-04-14T03:04:17Z

 # Multi-architecture build and push
-docker-buildx-push:
+docker-buildx-push-workloadmanager:
 	@if [ -z "$(IMAGE_REGISTRY)" ]; then \
-		echo "Error: IMAGE_REGISTRY not set. Usage: make docker-buildx-push IMAGE_REGISTRY=your-registry.com"; \
+		echo "Error: IMAGE_REGISTRY not set. Usage: make docker-buildx-push-workloadmanager IMAGE_REGISTRY=your-registry.com"; \
 		exit 1; \
 	fi
 	@echo "Building and pushing multi-architecture Docker image to $(IMAGE_REGISTRY)/$(WORKLOAD_MANAGER_IMAGE)..."
-	docker buildx build -f docker/Dockerfile --platform linux/amd64,linux/arm64 \
+	docker buildx build -f docker/Dockerfile.workloadmanager --platform linux/amd64,linux/arm64 \
 		-t $(IMAGE_REGISTRY)/$(WORKLOAD_MANAGER_IMAGE) \
 		--push .


The workload manager multi-arch release target was renamed to docker-buildx-push-workloadmanager, but existing automation still invokes make docker-buildx-push (e.g., .github/workflows/build-push-release.yml). Without updating those callers (or keeping a backward-compatible alias), release image publishing will break.

YaoZengzeng · 2026-04-17T02:14:46Z


      - name: Build Agentcube API Server Docker image
-        run: make docker-build
+        run: make docker-build-workloadmanager


only build workload manager?

previous docker-build = build-workloadmanager

As you can see Build Agentcube API Server Docker image

So it'd be better to build all the images in the workflow now.

sounds reasonable

Co-authored-by: Zhou Zihang <mail@zzhgo.com> Signed-off-by: Tiger Xu / Zhonghu Xu <xuzhonghu@huawei.com>

Copilot

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.

Copilot · 2026-04-17T08:23:12Z

 .PHONY: build run clean test deps

 # Build targets
-build: generate ## Build workloadmanager binary
+build-workloadmanager: generate ## Build workloadmanager binary
 	@echo "Building workloadmanager..."
 	go build -o bin/workloadmanager ./cmd/workload-manager



The Makefile no longer defines a build target, but .PHONY still lists build (and run) and existing docs/tools/workflows may still call make build. Consider keeping build as an alias for build-workloadmanager (or restoring the old target name) and removing/adjusting phony entries for targets that no longer exist to avoid confusing failures.

Copilot · 2026-04-17T08:23:13Z

+docker-build-workloadmanager:
 	@echo "Building Docker image..."
-	docker build -f docker/Dockerfile -t $(WORKLOAD_MANAGER_IMAGE) .
-
-# Multi-architecture build (supports amd64, arm64)
-docker-buildx:
-	@echo "Building multi-architecture Docker image..."
-	docker buildx build -f docker/Dockerfile --platform linux/amd64,linux/arm64 -t $(WORKLOAD_MANAGER_IMAGE) .
+	docker build -f docker/Dockerfile.workloadmanager -t $(WORKLOAD_MANAGER_IMAGE) .

 # Multi-architecture build and push
-docker-buildx-push:
+docker-buildx-push-workloadmanager:
 	@if [ -z "$(IMAGE_REGISTRY)" ]; then \
-		echo "Error: IMAGE_REGISTRY not set. Usage: make docker-buildx-push IMAGE_REGISTRY=your-registry.com"; \
+		echo "Error: IMAGE_REGISTRY not set. Usage: make docker-buildx-push-workloadmanager IMAGE_REGISTRY=your-registry.com"; \
 		exit 1; \
 	fi
 	@echo "Building and pushing multi-architecture Docker image to $(IMAGE_REGISTRY)/$(WORKLOAD_MANAGER_IMAGE)..."
-	docker buildx build -f docker/Dockerfile --platform linux/amd64,linux/arm64 \
+	docker buildx build -f docker/Dockerfile.workloadmanager --platform linux/amd64,linux/arm64 \
 		-t $(IMAGE_REGISTRY)/$(WORKLOAD_MANAGER_IMAGE) \
 		--push .

-docker-push: docker-build
+docker-push-workloadmanager: docker-build-workloadmanager
 	@if [ -z "$(IMAGE_REGISTRY)" ]; then \
-		echo "Error: IMAGE_REGISTRY not set. Usage: make docker-push IMAGE_REGISTRY=your-registry.com"; \
+		echo "Error: IMAGE_REGISTRY not set. Usage: make docker-push-workloadmanager IMAGE_REGISTRY=your-registry.com"; \


Renaming workloadmanager image targets removed the previous docker-buildx-push/docker-push entrypoints. The repo still has automation that calls make docker-buildx-push (e.g., .github/workflows/build-push-release.yml), so releases will fail unless that workflow is updated or compatibility alias targets are added (e.g., docker-buildx-push: docker-buildx-push-workloadmanager).

Copilot · 2026-04-17T08:23:13Z

      - name: Set up Docker Buildx
        uses: docker/setup-buildx-action@v3

      - name: Verify Docker installation
        run: |
          docker --version
          docker buildx version



This workflow now only runs make build-all, but it still sets up Docker Buildx and verifies Docker. If the intent is to build binaries only, these Docker steps are no longer needed and add time/complexity to the job; consider removing them or switching the build step back to an image build if Docker coverage is desired.

YaoZengzeng · 2026-04-17T08:36:09Z

/lgtm
/approve

volcano-sh-bot · 2026-04-17T08:36:19Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: YaoZengzeng
Once this PR has been reviewed and has the lgtm label, please ask for approval from hzxuzhonghu. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copilot AI review requested due to automatic review settings April 9, 2026 09:41

volcano-sh-bot added the kind/cleanup label Apr 9, 2026

volcano-sh-bot requested review from LiZhenCheng9527, YaoZengzeng and acsoto April 9, 2026 09:41

volcano-sh-bot added the size/L label Apr 9, 2026

Copilot started reviewing on behalf of hzxuzhonghu April 9, 2026 09:41 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

gemini-code-assist Bot reviewed Apr 9, 2026

View reviewed changes

make workloadmanager build more explicit

198d07a

Signed-off-by: Zhonghu Xu <xuzhonghu@huawei.com>

hzxuzhonghu force-pushed the make branch from f253e30 to 198d07a Compare April 9, 2026 10:12

acsoto reviewed Apr 10, 2026

View reviewed changes

Comment thread Makefile Outdated

Comment thread Makefile Outdated

Copilot AI review requested due to automatic review settings April 14, 2026 03:01

Copilot AI reviewed Apr 14, 2026

View reviewed changes

YaoZengzeng reviewed Apr 17, 2026

View reviewed changes

hzxuzhonghu force-pushed the make branch from eabbe9e to dbee7d8 Compare April 17, 2026 08:18

Update Makefile

05c8ae3

Co-authored-by: Zhou Zihang <mail@zzhgo.com> Signed-off-by: Tiger Xu / Zhonghu Xu <xuzhonghu@huawei.com>

Copilot AI review requested due to automatic review settings April 17, 2026 08:20

hzxuzhonghu force-pushed the make branch from dbee7d8 to 05c8ae3 Compare April 17, 2026 08:20

Copilot started reviewing on behalf of hzxuzhonghu April 17, 2026 08:20 View session

Copilot AI reviewed Apr 17, 2026

View reviewed changes

volcano-sh-bot assigned YaoZengzeng Apr 17, 2026

volcano-sh-bot added the lgtm label Apr 17, 2026

	- Workload Manager: `make build-workloadmanager`
	- Workload Manager: `make build`

		@@ -68,7 +69,7 @@ gen-check: gen-all ## Check if generated code is up to date
		.PHONY: build run clean test deps

Conversation

hzxuzhonghu commented Apr 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

YaoZengzeng Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

hzxuzhonghu Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

hzxuzhonghu Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

YaoZengzeng Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

hzxuzhonghu Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

YaoZengzeng commented Apr 17, 2026

Uh oh!

volcano-sh-bot commented Apr 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

codecov-commenter commented Apr 9, 2026 •

edited

Loading