feat: use MIG profile layout for partitioning - #9129
Open
Karen Chen (karenychen) wants to merge 7 commits into
Open
feat: use MIG profile layout for partitioning#9129Karen Chen (karenychen) wants to merge 7 commits into
Karen Chen (karenychen) wants to merge 7 commits into
Conversation
Karen Chen (karenychen)
requested review from
Chou Hu (AbelHu),
Devinwong,
Sri Harsha (SriHarsha001),
Nishchay (awesomenix),
Calvin S. (calvin197),
Cameron Meissner (cameronmeissner),
Sylvain Boily (djsly),
Ganeshkumar Ashokavardhanan (ganeshkumarashok),
lilypan26,
Mark Ibrahim (mxj220),
Peter Damianov (pdamianov-dev),
Patrick W. Healy (phealy),
r2k1,
Runzhen (runzhen),
sulixu,
Tim Wright (timmy-wright),
Thibault Cohen (titilambert),
Xu Xue (xuexu6666) and
Zachary (zachary-bailey)
as code owners
August 4, 2026 17:25
Contributor
|
The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).
|
Contributor
Windows Unit Test Results 3 files 12 suites 50s ⏱️ Results for commit 1bf4cd9. ♻️ This comment has been updated with latest results. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR completes the runtime support for the ordered MIGProfileLayout contract (introduced in #9081) across classic CSE and aks-node-controller, enabling MIG partitioning based on an explicit ordered layout while preserving legacy GPUInstanceProfile uniform expansion behavior.
Changes:
- Treats nodes as MIG-enabled when either
MIGProfileLayoutor legacyGPUInstanceProfileis populated, withMIGProfileLayouttaking precedence when non-empty. - Updates
mig-partition.shto map the ordered layout directly to NVIDIA profile IDs (preserving order/duplicates) and to propagate failures fromnvidia-smi mig -cgiandnvidia-smi mig -cci. - Adds/updates focused ShellSpec coverage and GPU e2e coverage for legacy, Single, and Mixed MIG scenarios, plus additional validators for exact advertised resources and MIG strategy.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| spec/parts/linux/cloud-init/artifacts/mig_partition_spec.sh | Adds ShellSpec coverage for legacy and layout-driven MIG partitioning behavior and failure propagation. |
| pkg/agent/variables.go | Sets MIG_NODE based on either legacy scalar or layout being populated. |
| pkg/agent/datamodel/helper.go | Extends IsMIGNode to consider both legacy and layout inputs. |
| pkg/agent/baker.go | Uses the shared IsMIGNode logic when templating MIG enablement. |
| pkg/agent/baker_test.go | Updates tests to assert MIG env vars for legacy and layout-driven cases. |
| parts/linux/cloud-init/artifacts/mig-partition.sh | Implements authoritative ordered layout parsing and legacy uniform expansion; propagates nvidia-smi failures. |
| e2e/validators.go | Adds validators for MIG strategy, exact GPU resource advertisement, and exact MIG instance profile counts. |
| e2e/scenario_gpu_managed_experience_test.go | Adds/updates Ubuntu 24.04 MIG scenarios for legacy field, layout+Single, and layout+Mixed with stricter validation. |
| e2e/node_config.go | Propagates MIG-related fields (instance profile, strategy, layout) into AKS node config v1 for e2e. |
| aks-node-controller/proto/aksnodeconfig/v1/gpu_config.proto | Clarifies MIG strategy semantics when MIG is enabled via either legacy scalar or layout. |
| aks-node-controller/pkg/gen/aksnodeconfig/v1/gpu_config.pb.go | Regenerates Go bindings reflecting the updated proto comment/metadata. |
| aks-node-controller/parser/parser.go | Sets MIG_NODE based on either legacy scalar or layout being populated. |
| aks-node-controller/parser/parser_test.go | Updates parser tests to assert MIG env vars and MIG enablement for layout-only inputs. |
| aks-node-controller/parser/helper.go | Extends MIG enablement helper to consider both legacy and layout inputs. |
Files not reviewed (1)
- aks-node-controller/pkg/gen/aksnodeconfig/v1/gpu_config.pb.go: Generated file
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- aks-node-controller/pkg/gen/aksnodeconfig/v1/gpu_config.pb.go: Generated file
Suppressed comments (2)
pkg/agent/datamodel/helper.go:131
- IsMIGNode treats any non-empty migProfileLayout slice as enabling MIG, but the layout is serialized via strings.Join. A slice like []string{""} makes MIG_NODE true while NVIDIA_MIG_PROFILE_LAYOUT becomes an empty string, causing mig-partition.sh to treat the layout as unset and fail with the misleading "neither ... is set" path. Consider only returning true when the layout contains at least one non-empty element (or trim-space non-empty) so MIG_NODE aligns with the serialized env var.
// IsMIGNode check if the node should be partitioned.
func IsMIGNode(gpuInstanceProfile string, migProfileLayout []string) bool {
return gpuInstanceProfile != "" || len(migProfileLayout) > 0
}
aks-node-controller/parser/helper.go:255
- getIsMIGNode uses len(migProfileLayout)>0, which can make MIG_NODE true even when the serialized NVIDIA_MIG_PROFILE_LAYOUT ends up empty (e.g., []string{""}). That leads to inconsistent behavior (MIG_NODE true, but mig-partition.sh sees no layout). Consider requiring at least one non-empty profile entry before enabling MIG based on migProfileLayout.
func getIsMIGNode(gpuInstanceProfile string, migProfileLayout []string) bool {
return gpuInstanceProfile != "" || len(migProfileLayout) > 0
}
Contributor
Author
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
Comment on lines
128
to
131
| // IsMIGNode check if the node should be partitioned. | ||
| func IsMIGNode(gpuInstanceProfile string) bool { | ||
| return gpuInstanceProfile != "" | ||
| func IsMIGNode(gpuInstanceProfile string, migProfileLayout []string) bool { | ||
| return gpuInstanceProfile != "" || len(migProfileLayout) > 0 | ||
| } |
Contributor
🔍 AgentBaker Linux Gate DetectiveBuild: 175174418 | PR: #9129 | Failed job/stage: �uild → �uild2604minimalgen2containerd → Build VHD (task �188ffa6 / �d2d3f37, 4 attempts) TL;DRUbuntu 26.04 minimal VHD build fails consistently because linux-modules-extra-azure-lts-26.04 is not available in the Ubuntu apt repository. The provisioning script purges the existing kernel then attempts to install the LTS metapackage, which does not exist. All 4 Packer retries fail identically. This is not caused by changes in this PR. Failure Details
Three-Level RCALevel 1 — Surface failure: Packer provisioner exits 100 (non-zero) during kernel replacement step. The script successfully purges existing linux-image-7.0.0-1011-azure / linux-modules-7.0.0-1011-azure, then fails on install of LTS metapackage. Level 2 — Corroboration (≥2 independent sources):
Level 3 — Root-cause challenge:
Verdict
Evidence Links
|
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- aks-node-controller/pkg/gen/aksnodeconfig/v1/gpu_config.pb.go: Generated file
Suppressed comments (1)
parts/linux/cloud-init/artifacts/mig-partition.sh:82
if $is_last_profile; thenexecutes the variable as a command (relying ontrue/falsebeing available in PATH). Using a string comparison avoids accidental command execution if this variable is ever refactored or assigned unexpectedly.
if $is_last_profile; then
Contributor
There was a problem hiding this comment.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Note
This error may be related to your runner configuration. You can now configure runners for Copilot code review separately from Copilot cloud agent by creating a copilot-code-review.yml file with your setup steps. Read the docs for details.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Completes the runtime half of the MIG profile layout contract introduced by #9081.
MIGProfileLayoutor legacyGPUInstanceProfileas a MIG node in classic CSE and aks-node-controllerMIGProfileLayoutauthoritative when both fields are populatedMigStrategyinterpretationGPUInstanceProfileis populatednvidia-smi mig -cgiandnvidia-smi mig -cciMigStrategysolely for nvidia-device-plugin advertisement behaviorThis supersedes the runtime/test portion of closed #8869. AKS RP layout calculation remains out of scope.
Validation
make generateregenerated Go test data and the manifest without changes; its repository-wide ShellCheck tail stops on the existing POSIXSC3014baselinego test ./pkg/agent/... -count=1go test ./parser/... -count=1fromaks-node-controllergo test . -run '^$' -count=1frome2emig-partition.shand its ShellSpecAzure/AgentBakermaingit diff --check