Skip to content

fix: vllm e2e job uses matrix context without strategy block - #750

Closed
andrewwhitecdw wants to merge 1 commit into
NVIDIA-NeMo:mainfrom
andrewwhitecdw:bugfix/cicd-main-vllm-e2e-job-uses-matrix-context
Closed

fix: vllm e2e job uses matrix context without strategy block#750
andrewwhitecdw wants to merge 1 commit into
NVIDIA-NeMo:mainfrom
andrewwhitecdw:bugfix/cicd-main-vllm-e2e-job-uses-matrix-context

Conversation

@andrewwhitecdw

Copy link
Copy Markdown
Contributor

This PR addresses the following issue in .github/workflows/cicd-main.yml: vllm e2e job uses matrix context without strategy block.

Changes

  • .github/workflows/cicd-main.yml: vllm e2e job uses matrix context without strategy block.

Details

--- a/.github/workflows/cicd-main.yml
+++ b/.github/workflows/cicd-main.yml
@@ -1,19 +1,37 @@
-  cicd-e2e-tests-trt-onnx:
-    needs: [pre-flight, cicd-unit-tests-vllm]
-    runs-on: ${{ needs.pre-flight.outputs.runner_prefix }}
-    name: ${{ matrix.is_optional && 'PLEASEFIXME_' || '' }}${{ matrix.script }}
-...
-          script: L2_ONNX_TRT
-          is_optional: ${{ matrix.is_optional || false }}
-
-  cicd-e2e-tests-vllm:
-    needs: [cicd-unit-tests-vllm, pre-flight]
-...
-          script: L2_Launch_vLLM
-          is_optional: ${{ matrix.is_optional || false }}
-
-  cicd-e2e-tests-inframework:
-    needs: [pre-flight, cicd-unit-tests-vllm]
-...
-          script: L2_Launch_InFramework
-          is_optional: ${{ matrix.is_optional || false }}
+  cicd-e2e-tests-trt-onnx:
+    strategy:
+      fail-fast: false
+      matrix:
+        include:
+          - script: L2_ONNX_TRT
+            is_optional: false
+    needs: [pre-flight, cicd-unit-tests-vllm]
+    runs-on: ${{ needs.pre-flight.outputs.runner_prefix }}
+    name: ${{ matrix.is_optional && 'PLEASEFIXME_' || '' }}${{ matrix.script }}
+...
+          script: ${{ matrix.script }}
+          is_optional: ${{ matrix.is_optional }}
+
+  cicd-e2e-tests-vllm:
+    strategy:
+      fail-fast: false
+      matrix:
+        include:
+          - script: L2_Launch_vLLM
+            is_optional: false
+    needs: [cicd-unit-tests-vllm, pre-flight]
+...
+          script: ${{ matrix.script }}
+          is_optional: ${{ matrix.is_optional }}
+
+  cicd-e2e-tests-inframework:
+    strategy:
+      fail-fast: false
+      matrix:
+        include:
+          - script: L2_Launch_InFramework
+            is_optional: false
+    needs: [pre-flight, cicd-unit-tests-vllm]
+...
+          script: ${{ matrix.script }}
+          is_optional: ${{ matrix.is_optional }}

Tests

Let me know if you want tests added for this fix or not.

Contributor guidelines

Per this repo's CONTRIBUTING.md:

  • All commits are signed off (Signed-off-by trailer, DCO).

Signed-off-by: andrewwhitecdw <andrewwhitecdw@users.noreply.github.com>
@andrewwhitecdw
andrewwhitecdw requested a review from a team as a code owner August 17, 2026 20:19
@copy-pr-bot

copy-pr-bot Bot commented Aug 17, 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.

@github-actions github-actions Bot added the CI label Aug 17, 2026
@chtruong814

Copy link
Copy Markdown
Contributor

/ok to test 57c3538

@chtruong814

Copy link
Copy Markdown
Contributor

@andrewwhitecdw Thanks for the PR for cleaning this up. Could you actually combine your changes from these other PRs? It'll be faster for us to review together since it's CI related.

@chtruong814 chtruong814 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see the comment about aggregating the other CI proposed changes into this PR.

@andrewwhitecdw

Copy link
Copy Markdown
Contributor Author

Consolidating these related CI fixes into a single PR: #755. Thanks for the suggestion!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants