chore: add gpu vhd builder checkin pipeline #7750
Conversation
lilypan26
requested review from
AbelHu,
Devinwong,
awesomenix,
cameronmeissner,
djsly,
ganeshkumarashok,
juan-lee,
junjiezhang1997,
phealy,
r2k1,
timmy-wright and
zachary-bailey
as code owners
lilypan26
changed the title
There was a problem hiding this comment.
Pull request overview
This PR adds a new PR checkin pipeline for GPU VHD builds that triggers GPU end-to-end tests when VHD-related files change. This complements the existing standalone GPU e2e pipeline by ensuring VHDs are rebuilt and tested together when necessary.
Changes:
- Creates a new
.vsts-vhd-builder-gpu.yamlpipeline that builds GPU-capable VHDs (Ubuntu 22.04, 24.04, and AzureLinux V3) and runs GPU e2e tests - Updates
e2e-gpu.yamlto excludecomponents.jsonchanges, delegating those to the new VHD builder pipeline
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| .pipelines/.vsts-vhd-builder-gpu.yaml | New GPU-focused VHD builder pipeline that builds three Linux distributions and runs GPU e2e tests |
| .pipelines/e2e-gpu.yaml | Moves components.json from include to exclude paths, as component changes are now handled by the VHD builder GPU pipeline |
| - schemas | ||
| - vhdbuilder/packer | ||
| - vhdbuilder/scripts/linux | ||
| - .pipelines/.vsts-vhd-builder.yaml |
There was a problem hiding this comment.
This path reference should be changed to .pipelines/.vsts-vhd-builder-gpu.yaml to correctly reference this GPU pipeline file itself. Currently, it references the non-GPU VHD builder pipeline .vsts-vhd-builder.yaml, which means changes to this GPU pipeline file won't trigger the PR checks for this pipeline.
| - .pipelines/.vsts-vhd-builder.yaml | |
| - .pipelines/.vsts-vhd-builder-gpu.yaml |
| - .pipelines/templates/.template-copy-file.yaml | ||
| - .pipelines/templates/e2e-template.yaml | ||
| - packer.mk | ||
| - aks-node-controller/** |
There was a problem hiding this comment.
The path include pattern is missing parts/linux/* which is present in the similar .vsts-vhd-builder.yaml pipeline (line 19 of that file). Since this GPU pipeline builds Linux VHDs and should trigger on changes to Linux-specific parts, this path should be included to maintain consistency with the non-GPU VHD builder pipeline. Without it, changes to files under parts/linux/ won't trigger this GPU pipeline's PR checks.
| - aks-node-controller/** | |
| - aks-node-controller/** | |
| - parts/linux/* |
| - pkg/agent/testdata/AKSWindows* # Windows test data | ||
| - staging/cse/windows/README | ||
| - /**/*.md | ||
| - parts/common/components.json |
There was a problem hiding this comment.
The exclusion of parts/common/components.json should include an explanatory comment for consistency with the same exclusion in .pipelines/e2e.yaml (line 30), which has the comment "# centralized components management file". This helps other developers understand why component changes don't trigger GPU E2E tests.
| - parts/common/components.json | |
| - parts/common/components.json # centralized components management file |
| echo '##vso[task.setvariable variable=AZURE_VM_SIZE]Standard_D16ds_v5' | ||
| echo '##vso[task.setvariable variable=FEATURE_FLAGS]None' | ||
| echo '##vso[task.setvariable variable=ARCHITECTURE]X86_64' | ||
| echo '##vso[task.setvariable variable=ENABLE_FIPS]false' |
There was a problem hiding this comment.
Inconsistent boolean capitalization for ENABLE_FIPS within this file. Line 66 uses 'False' (capital F) while lines 86 and 106 use 'false' (lowercase f). For consistency, all instances should use the same capitalization. The file consistently uses 'False' (capital) for ENABLE_TRUSTED_LAUNCH (lines 67, 87, 107), suggesting capital 'False' should be used here as well.
| echo '##vso[task.setvariable variable=ENABLE_FIPS]false' | |
| echo '##vso[task.setvariable variable=ENABLE_FIPS]False' |
| echo '##vso[task.setvariable variable=AZURE_VM_SIZE]Standard_D16ds_v5' | ||
| echo '##vso[task.setvariable variable=FEATURE_FLAGS]None' | ||
| echo '##vso[task.setvariable variable=ARCHITECTURE]X86_64' | ||
| echo '##vso[task.setvariable variable=ENABLE_FIPS]false' |
There was a problem hiding this comment.
Inconsistent boolean capitalization for ENABLE_FIPS within this file. Line 66 uses 'False' (capital F) while lines 86 and 106 use 'false' (lowercase f). For consistency, all instances should use the same capitalization. The file consistently uses 'False' (capital) for ENABLE_TRUSTED_LAUNCH (lines 67, 87, 107), suggesting capital 'False' should be used here as well.
| - group: build_performance | ||
| - group: aks-vuln-to-kusto-tme | ||
| - name: TAGS_TO_SKIP | ||
| value: gpu=true,os=windows |
There was a problem hiding this comment.
The pipeline-level TAGS_TO_SKIP variable is set to "gpu=true,os=windows" which seems inconsistent for a GPU-focused pipeline. This variable is overridden in the e2e stage (line 118) to "os=windows" to properly run GPU tests. Since the build stage doesn't use this variable (the .builder-release-template.yaml doesn't reference TAGS_TO_SKIP), this pipeline-level setting has no effect and is misleading. Consider removing this line or updating it to reflect the GPU focus of this pipeline, such as setting it to "os=windows" to match the e2e stage override.
| value: gpu=true,os=windows | |
| value: os=windows |
What this PR does / why we need it:
Adds PR checkin pipeline to run GPU e2es when there are changes to VHD
Which issue(s) this PR fixes:
Fixes #