Skip to content

feat(mnnvl): add group-aware MNNVL injection into PodSpec#557

Merged
shmuel-runai merged 3 commits into
ai-dynamo:mainfrom
shmuel-runai:grove-417/main-inject-mnnvl-group
Apr 28, 2026
Merged

feat(mnnvl): add group-aware MNNVL injection into PodSpec#557
shmuel-runai merged 3 commits into
ai-dynamo:mainfrom
shmuel-runai:grove-417/main-inject-mnnvl-group

Conversation

@shmuel-runai

Copy link
Copy Markdown
Contributor

Summary

  • When injecting MNNVL into a PodSpec, the controller now uses the correct RCT name for the target MNNVL group
  • Controllers resolve the MNNVL group from annotations hierarchically — clique annotations take priority over PCS/PCSG annotations
  • Export ResolveGroupName from the mnnvl package and add groupName parameter to InjectMNNVLIntoPodSpec

Key changes

  • Move resolveGroupName to mnnvl.ResolveGroupName (exported)
  • Add groupName parameter to InjectMNNVLIntoPodSpec
  • Update PCS and PCSG controllers with hierarchical group resolution
  • Add group-aware test cases for both PCS and PCSG injection

Which issue(s) this PR fixes

Ref #417

Test plan

  • Unit tests for ResolveGroupName in helpers_test.go
  • Unit tests for InjectMNNVLIntoPodSpec with named groups in injection_test.go
  • PCS controller TestBuildResource_MNNVLInjection — group on PCS, clique override, clique-only
  • PCSG controller TestBuildResource_MNNVLInjection — group on PCSG, clique override, clique-only
  • make check passes (no dirty tree)

Made with Cursor

When injecting MNNVL into a PodSpec, the controller now uses the correct
RCT name for the target MNNVL group. Controllers resolve the MNNVL group
from annotations hierarchically — clique annotations take priority over
PCS/PCSG annotations.

Key changes:
- Move resolveGroupName to mnnvl.ResolveGroupName (exported)
- Add groupName parameter to InjectMNNVLIntoPodSpec
- Update PCS and PCSG controllers with hierarchical group resolution
- Add group-aware test cases for both PCS and PCSG injection

Made-with: Cursor
@copy-pr-bot

copy-pr-bot Bot commented Apr 28, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@shmuel-runai shmuel-runai self-assigned this Apr 28, 2026
danbar2
danbar2 previously approved these changes Apr 28, 2026
@danbar2 danbar2 self-requested a review April 28, 2026 09:36
danbar2
danbar2 previously approved these changes Apr 28, 2026
…ution

Replace the manual two-step ResolveGroupName fallback in both PCS and
PCSG controllers with a variadic ResolveGroupNameHierarchically helper.
The first annotation layer that requests MNNVL wins, letting a child
layer intentionally override its parent's group (including escaping a
named group back to the default).

Add code comments clarifying that PCS-level annotations are already
propagated onto the PCSG via propagateMNNVLAnnotations, so the
two-layer check (PCLQ → PCSG) is sufficient.

Made-with: Cursor
@shmuel-runai shmuel-runai merged commit 4ddbe96 into ai-dynamo:main Apr 28, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants