fix(e2e): make managed-image GPU builds network-independent - #8366
fix(e2e): make managed-image GPU builds network-independent#8366ericksoa wants to merge 11 commits into
Conversation
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change adds local BuildKit cache export and offline cache import for protected managed-image builds. It connects the startup and runtime workflows through a pinned amd64 cache artifact. Boundary validators and tests cover selection, dependencies, permissions, artifact handling, ordering, and build arguments. ChangesProtected managed-image cache handoff
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Selector
participant managed-image-multiarch-startup
participant ArtifactStore
participant managed-image-protected-runtime
participant BuildKit
Selector->>managed-image-multiarch-startup: select protected-runtime lane
managed-image-multiarch-startup->>BuildKit: export amd64 build cache
managed-image-multiarch-startup->>ArtifactStore: publish cache artifact
managed-image-protected-runtime->>ArtifactStore: download exact cache artifact
managed-image-protected-runtime->>BuildKit: build image with offline cache
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit af77aa7 in the TypeScript / code-coverage/cliThe overall coverage in commit af77aa7 in the Show a code coverage summary of the most impacted files.
Updated |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/e2e.yaml:
- Line 1985: Replace runner.temp with github.workspace in both job-level cache
paths in .github/workflows/e2e.yaml at lines 1985 and 2592. Update the boundary
assertions in tools/e2e/managed-image-multiarch-workflow-boundary.mts lines
153-156 and tools/e2e/managed-image-protected-runtime-workflow-boundary.mts
lines 122-125 to require the workspace-based paths.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 51b7d8e9-240d-401f-b6ae-a864749c9216
📒 Files selected for processing (6)
.github/workflows/e2e.yamlscripts/checks/build-protected-managed-images.shtest/e2e/support/managed-image-protected-runtime-workflow.test.tstest/protected-managed-image-build-script.test.tstools/e2e/managed-image-multiarch-workflow-boundary.mtstools/e2e/managed-image-protected-runtime-workflow-boundary.mts
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
3 terminology differences from the second opinionAdvisory only. These are normalized differences from the primary terminology receipt.
Second-opinion terminology and E2E selections are advisory. They do not change the primary assessment or E2E / PR Gate. Since last review: 0 prior items resolved · 0 still apply · 0 new items found 2 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: 1 optional E2E recommendation
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tools/e2e/upload-e2e-artifacts-workflow-boundary.mts`:
- Line 37: Update the managed-image boundary around the upload step matcher so
it validates both with.name and with.path, and requires exactly one direct
upload step match instead of allowing loose or multiple matches. Adjust the test
in upload-e2e-artifacts-workflow-boundary.mts to cover changed upload inputs and
the duplicate-step case, using the existing
MANAGED_IMAGE_BUILD_CACHE_PUBLISH_STEP as the exact-step anchor.
In `@tools/e2e/workflow-boundary.mts`:
- Around line 2524-2533: Update requireOrderedSteps for the
managed-image-protected-runtime case to track the canonical protected
cache-download index and require authIndex to equal cacheDownloadIndex + 1,
rather than deriving the expected position solely from checkoutIndex. Preserve
the existing Jetson dispatch guard and standard checkout ordering behavior and
error messages.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: dff1c817-8b00-410f-8d8c-93ff11541982
📒 Files selected for processing (3)
test/e2e/support/upload-e2e-artifacts-workflow-boundary.test.tstools/e2e/upload-e2e-artifacts-workflow-boundary.mtstools/e2e/workflow-boundary.mts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/e2e.yaml:
- Around line 2584-2586: Remove the repository-local removalCondition comment
from the workflow. Keep only documentation describing functional workflow
behavior, and retain the migration status and completion criteria in the
associated GitHub issue or PR instead.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 89d06d56-668e-496a-9db0-06d8df2ce45e
📒 Files selected for processing (2)
.github/workflows/e2e.yamltest/protected-managed-image-build-script.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- test/protected-managed-image-build-script.test.ts
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
ericksoa
added
area: ci
1 participant
Summary
Why
This is a trusted-workflow prerequisite for #8261 and the #7744 Podman epic. Protected run 31012875592 passed the all-agent amd64/arm64 startup lanes, but its GPU/local-inference job failed twice at the same 368.8-second npm install point while building exact candidate images on the GPU runner. Neither attempt reached the GPU, Ollama, NIM, or vLLM assertions.
The hosted amd64 lane already builds and directly starts the exact candidate images without credentials. Reusing its BuildKit cache removes the GPU runners broken npm-network path while preserving an exact candidate rebuild on the GPU host. The rebuild runs with network none, so any cache miss fails instead of silently reaching the network.
This PR does not activate buildless or Podman support and does not change product runtime behavior. It must land before #8261 can be rerun against the corrected trusted workflow.
Temporary handoff removal
Remove the hosted-cache handoff after three consecutive protected GPU cold builds of all three managed images reach the GPU, Ollama, NIM, and vLLM assertions without the hosted cache.
Documentation Writer Review
no-docs-neededValidation
Signed-off-by: Aaron Erickson aerickson@nvidia.com
Summary by CodeRabbit
New Features
managed-image-protected-runtimeselection.Bug Fixes
Tests