-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Add support for skipping handler registration and QoS manager initialization based on configuration in Volcano Agent #4409
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
Add support for skipping handler registration and QoS manager initialization based on configuration in Volcano Agent #4409
Conversation
|
Welcome @ShuhanYan! |
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.
Pull Request Overview
This PR adds conditional support for skipping the registration of handlers and the initialization of the QoS manager based on configuration flags in the Volcano Agent. Key changes include:
- Adding a nil-check around network QoS health check.
- Conditionally registering event handlers based on supported features.
- Introducing conditional initialization of the NetworkQoSManager in the agent run logic.
- Adding a new Makefile target for building the vc-agent image.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/agent/healthcheck/healthcheck.go | Adds a guard to skip QoS health checks if the network QoS manager is not initialized. |
| pkg/agent/events/eventsmgr.go | Adds a feature check to conditionally register or log the skipping of event handlers. |
| cmd/agent/app/agent.go | Updates the NetworkQoSManager initialization based on feature support. |
| Makefile | Introduces a new target for building the vc-agent image. |
|
/ok-to-test |
537512a to
4f028c8
Compare
|
@JesseStutler Thanks for reminder! signed |
|
Great! Thanks for your contribution. It's very useful. |
Signed-off-by: shuhanyan <shuhan.yan@outlook.com> Update pkg/agent/events/eventsmgr.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: shuhanyan <shuhan.yan@outlook.com>
4f028c8 to
3971475
Compare
|
sure, updated |
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Monokaix The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind feature
What this PR does / why we need it:
This PR adds support for skipping handler registration and QoS manager initialization based on configuration in Volcano Agent, which is needed when the user only need a set of feature, and cannot enable some feature as they cannot run it on specified OS.
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
test:




when the supported-features=OverSubscription,Eviction
the initialization of network QoS manager will be skipped
meanwhile the health check can pass
and unrelated handler will be skip
Does this PR introduce a user-facing change?