feat: add MIG profile layout configuration - #9081
Conversation
|
The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).
|
Windows Unit Test Results 3 files 11 suites 52s ⏱️ Results for commit 479c574. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Pull request overview
Adds “layout-only” contract plumbing for an ordered NVIDIA MIG partition execution plan across the classic AgentBaker path and the aks-node-controller path, without changing runtime partitioning behavior yet (keeps MIG_NODE driven by legacy GPUInstanceProfile).
Changes:
- Adds
NodeBootstrappingConfiguration.MIGProfileLayout []stringand exposes it to templates asNVIDIA_MIG_PROFILE_LAYOUT(comma-joined, order/duplicates preserved). - Extends aks-node-controller GPU proto contract with
repeated string mig_profile_layout = 9and propagates it into the generated CSE env. - Adds focused unit tests validating order + duplicates are preserved and that layout-only input does not flip
MIG_NODEyet.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/agent/variables.go | Adds a MIG layout variable to the CSE variables map (currently unused by templates). |
| pkg/agent/datamodel/types.go | Adds MIGProfileLayout []string to NodeBootstrappingConfiguration. |
| pkg/agent/baker.go | Adds GetMIGProfileLayout template func (comma-joins layout). |
| pkg/agent/baker_test.go | Adds test ensuring layout is propagated but MIG_NODE remains false. |
| parts/linux/cloud-init/artifacts/cse_config.sh | Writes NVIDIA_MIG_PROFILE_LAYOUT into mig-partition systemd drop-in. |
| parts/linux/cloud-init/artifacts/cse_cmd.sh | Exports NVIDIA_MIG_PROFILE_LAYOUT in classic CSE env vars. |
| aks-node-controller/proto/README.md | Documents the new env var in GPUConfig field mapping table. |
| aks-node-controller/proto/aksnodeconfig/v1/gpu_config.proto | Adds mig_profile_layout (field 9) to GPUConfig proto. |
| aks-node-controller/pkg/gen/aksnodeconfig/v1/gpu_config.pb.go | Updates generated Go bindings for the new proto field. |
| aks-node-controller/parser/parser.go | Populates NVIDIA_MIG_PROFILE_LAYOUT from proto into CSE env. |
| aks-node-controller/parser/parser_test.go | Adds test verifying layout env propagation and MIG_NODE remains false. |
Files not reviewed (1)
- aks-node-controller/pkg/gen/aksnodeconfig/v1/gpu_config.pb.go: Generated file
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 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
Comments suppressed due to low confidence (2)
parts/linux/cloud-init/artifacts/cse_config.sh:919
- This change introduces new behavior in
ensureMigPartition()(writingNVIDIA_MIG_PROFILE_LAYOUTinto the systemd drop-in), but there is no ShellSpec coverage forensureMigPartitioninspec/parts/linux/cloud-init/artifacts/cse_config_spec.sh. Given the repo’s convention of adding ShellSpec coverage for shell script changes inparts/linux/, please add a focused test that asserts the generated drop-in includes the new environment line (and that order/format is preserved).
tee /etc/systemd/system/mig-partition.service.d/10-mig-profile.conf > /dev/null <<EOF
[Service]
Environment="GPU_INSTANCE_PROFILE=${GPU_INSTANCE_PROFILE}"
Environment="NVIDIA_MIG_PROFILE_LAYOUT=${NVIDIA_MIG_PROFILE_LAYOUT}"
EOF
pkg/agent/variables.go:129
migProfileLayoutis added to the CSE command variables map, but there are no template references tomigProfileLayout(onlyGetMIGProfileLayoutis used incse_cmd.sh). Keeping an unused variable key increases contract surface area and can mislead future readers into thinking it is consumed somewhere. Consider removing it until there is an actual template consumer.
"configGPUDriverIfNeeded": config.ConfigGPUDriverIfNeeded,
"enableGPUDevicePluginIfNeeded": config.EnableGPUDevicePluginIfNeeded,
"migNode": strconv.FormatBool(datamodel.IsMIGNode(config.GPUInstanceProfile)),
"gpuInstanceProfile": config.GPUInstanceProfile,
"migProfileLayout": strings.Join(config.MIGProfileLayout, ","),
What this PR does
Adds the inert AgentBaker contract plumbing for an ordered MIG execution plan:
NodeBootstrappingConfiguration.MIGProfileLayout []stringGpuConfig.mig_profile_layout(field 9)NVIDIA_MIG_PROFILE_LAYOUTin classic CSE and aks-node-controllerDeliberate split boundary
This PR does not change
mig-partition.sh, activate partitioning for layout-only input, or add GPU e2e scenarios.MIG_NODEremains driven by the legacyGPUInstanceProfileuntil the follow-up runtime PR lands. This prevents the existing script from being started with an empty legacy scalar while RP begins populating the new field.After this merges, a second PR will atomically:
MIGProfileLayoutor legacyGPUInstanceProfileas a MIG nodemig-partition.shSupersedes #8869.
Validation
go test ./pkg/agent/... -count=1go test ./parser/... -count=1(aks-node-controller)go test -run '^$' ./...(e2e compile)buf lintbuf breakingagainstAzure/AgentBaker/maincse_config_spec.sh: 145 examples, 0 failuresmake generateregenerated Go test data successfully, then the repository-wide POSIX ShellCheck phase hit the existing SC3014 baseline in unrelated scripts.