Skip to content

feat(Iluvatar): Add configured Iluvatar mock GPU resources - #28

Closed
spencercjh wants to merge 1 commit into
Project-HAMi:masterfrom
spencercjh:feat/mock-iluvatar
Closed

spencercjh wants to merge 1 commit into
Project-HAMi:masterfrom
spencercjh:feat/mock-iluvatar

Conversation

@spencercjh

@spencercjh spencercjh commented Aug 25, 2026 •

Copy link
Copy Markdown

Most of this Pull Request was generated or revised with the assistance of AI tools(Raft with GPT-5.6 Sol High). I have reviewed the resulting content and take full responsibility for its accuracy, security, licensing compliance, and inclusion in this project.

What changed

  • Initialize one mock device handler for each iluvatars entry in the HAMi device configuration.
  • Register the configured count, vMem, and vCore resources from hami.io/node-<commonWord>-register annotations.
  • Parse the current seven-field and nine-field legacy CSV annotation forms strictly, rejecting malformed numbers, booleans, indexes, duplicate IDs, negative capacities, and partial records instead of coercing invalid values.
  • Validate CommonWords, generated annotation keys, full extended-resource names, vendor namespaces, and collisions before replacing the global device map.
  • Document and test the four HAMi default models: MR-V100, MR-V50, BI-V150, and BI-V100.

Why

HAMi's scheduler already understands configured Iluvatar vGPU resources, but the mock device plugin could not expose that contract on hardware-free test clusters. This adds an opt-in mock path that mirrors HAMi's current Iluvatar configuration and annotation semantics while failing closed on invalid configuration or node data.

Impact

For healthy annotation entries, the mock registers:

  • vGPU count as the sum of count;
  • vMem as floor(sum(devmem MiB) / 256);
  • vCore as the sum of devcore.

Unhealthy entries do not contribute resources. Iluvatar count is fully generated by the mock, so no separate node-status count patch is required. Clusters without iluvatars entries retain their existing behavior.

Validation

  • go test ./...
  • go test -race ./...
  • go vet ./...
  • golangci-lint run --new-from-rev d3b1687d79ade6e964f0213ff5bbd7d4f94cc3e1 ./...
  • make build
  • git diff --check d3b1687d79ade6e964f0213ff5bbd7d4f94cc3e1...HEAD

Summary by CodeRabbit

  • New Features

    • Added support for registering and managing Iluvatar devices.
    • Added Iluvatar device discovery, health-aware resource reporting, and annotation parsing.
    • Added vendor-specific memory resource calculations using a 256 MiB conversion factor.
  • Bug Fixes

    • Invalid, duplicate, malformed, or conflicting device configurations are now rejected safely without partial registration.
  • Documentation

    • Expanded setup guidance for mocked resources, health checks, vendor-specific calculations, managed resources, and Iluvatar usage.

Signed-off-by: spencercjh <spencercjh@gmail.com>
@hami-robot

hami-robot Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: spencercjh
Once this PR has been reviewed and has the lgtm label, please assign archlitchi for approval. 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:

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

@coderabbitai

coderabbitai Bot commented Aug 25, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ec564615-51e7-40b6-89c7-5494f5f91a01

📥 Commits

Reviewing files that changed from the base of the PR and between d3b1687 and 189a5f7.

📒 Files selected for processing (5)
  • README.md
  • internal/pkg/api/device/iluvatar/device.go
  • internal/pkg/api/device/iluvatar/device_test.go
  • internal/pkg/config/config.go
  • internal/pkg/config/iluvatar_test.go

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

This change adds Iluvatar device support. It validates configurations, decodes node annotations, aggregates healthy-device resources, starts mock registration, checks cross-vendor collisions, and commits device registration atomically. README documentation describes the new configuration and resource behavior.

Changes

Iluvatar device support

Layer / File(s) Summary
Configuration and annotation handling
internal/pkg/api/device/iluvatar/device.go, internal/pkg/api/device/iluvatar/device_test.go
Adds Iluvatar configuration validation, strict CSV annotation decoding, device lookup, and tests for valid and invalid inputs.
Resource aggregation and manager wiring
internal/pkg/api/device/iluvatar/device.go, internal/pkg/api/device/iluvatar/device_test.go, README.md
Aggregates healthy device count, memory, and cores. Memory uses the fixed 256-MiB conversion. Documentation describes Iluvatar annotations and resource rules.
Global registration and collision validation
internal/pkg/config/config.go, internal/pkg/config/iluvatar_test.go
Initializes Iluvatar devices, rejects common-word and resource-name collisions, and commits the device map only after successful validation. Tests verify atomic failure behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: ⚪ Minimal · up to 189a5

This adds configured Iluvatar mock resources while preserving existing behavior for clusters without Iluvatar entries; no actionable merge-blocking risk remains beyond normal checks and review.

Suggested labels: enhancement

Suggested reviewers: archlitchi, dsfans2014

Poem

A rabbit hops through device arrays bright
Iluvatar resources now register right
CSV fields stand in a careful row
Healthy cards tell the totals to show
Collisions stop before maps are changed
“Hop-hop!” says the rabbit, “the paths are arranged!”

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 9.52% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 4 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding configured Iluvatar mock GPU resource support.
Full details: Docstring Coverage

Explanation

Docstring coverage is 9.52% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 4 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@spencercjh spencercjh changed the title [codex] Add configured Iluvatar mock GPU resources feat: Add configured Iluvatar mock GPU resources Aug 26, 2026
@spencercjh spencercjh changed the title feat: Add configured Iluvatar mock GPU resources feat(Iluvatar): Add configured Iluvatar mock GPU resources Aug 26, 2026
@spencercjh

spencercjh commented Aug 26, 2026 •

Copy link
Copy Markdown
Author

Tested with HAMi v2.10.0 on a fresh local K3d cluster.

Procedure:

  1. Installed HAMi with devicePlugin.enabled=false, mockDevicePlugin.enabled=true, devices.iluvatar.enabled=true, and scheduler.leaderElect=false.
  2. Built this PR image locally and imported mock-device-plugin:pr28-189a5f7 into the K3d node.
  3. Added this Node annotation on k3d-hami-mock-pr28-server-0:
hami.io/node-MR-V100-register=ILUVATAR-MOCK-0,10,8192,100,MR-V100,0,true,0,hami-core:ILUVATAR-MOCK-1,10,8192,100,MR-V100,0,true,1,hami-core:
  1. The node has only normal K3d/K3s labels; no Iluvatar label was added. Mock registered these allocatable resources:
iluvatar.ai/MR-V100-vgpu=20
iluvatar.ai/MR-V100.vMem=64
iluvatar.ai/MR-V100.vCore=200
  1. Created a Pod with schedulerName: hami-scheduler and requests/limits MR-V100-vgpu=1, MR-V100.vMem=1, and MR-V100.vCore=50.

Result: PASS. The Pod is Running on k3d-hami-mock-pr28-server-0. Scheduler selected ILUVATAR-MOCK-1 and wrote:

hami.io/MR-V100-devices-to-allocate=ILUVATAR-MOCK-1,MR-V100,256,50:;
hami.io/MR-V100-devices-allocated=ILUVATAR-MOCK-1,MR-V100,256,50:;
hami.io/vgpu-node=k3d-hami-mock-pr28-server-0

Mock logs also contain three Into Allocate calls. During this manual test I removed the stale hami.io/node-handshake-MR-V100 annotation created before the inventory annotation was added, then Scheduler registered the Node. A normal device plugin owns this handshake flow.

@spencercjh
spencercjh marked this pull request as ready for review August 26, 2026 04:19
@coderabbitai coderabbitai Bot added the enhancement New feature or request label Aug 26, 2026
@moezdil

moezdil commented Sep 22, 2026

Copy link
Copy Markdown

Old PRs and issues are being closed because they are no longer relevant. Or it was closed because of th
inactivity or PR/issues owners.
If you believe the issue is still relevant, please submit feedback.

@moezdil moezdil closed this Sep 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants