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!

andrewwhitecdw added a commit to andrewwhitecdw/Export-Deploy that referenced this pull request Aug 31, 2026
This combines three related CI fixes so they can be reviewed together:

- `.github/actions/test-template/action.yml`: corrects the `is_optional`
  description so it matches the actual behavior.
- `.github/workflows/cicd-main.yml`: adds missing `strategy`/`matrix` blocks to
  the trt-onnx, vllm, and inframework e2e jobs, and uses `${{ matrix.script }}`
  and `${{ matrix.is_optional }}` instead of hard-coded values.

Tests
- `unit_tests/github_actions/test_optional_description.py`: verifies the
  `is_optional` description no longer inverts the behavior.
- `tests/test_cicd_yaml.py`: verifies the e2e jobs define a strategy block
  before referencing matrix context.

Closes NVIDIA-NeMo#728, NVIDIA-NeMo#749, NVIDIA-NeMo#750.

Signed-off-by: Andrew White <andrewwhitecdw@users.noreply.github.com>
andrewwhitecdw added a commit to andrewwhitecdw/Export-Deploy that referenced this pull request Aug 31, 2026
This combines three related CI fixes so they can be reviewed together:

- `.github/actions/test-template/action.yml`: corrects the `is_optional`
  description so it matches the actual behavior.
- `.github/workflows/cicd-main.yml`: adds missing `strategy`/`matrix` blocks to
  the trt-onnx, vllm, and inframework e2e jobs, and uses `${{ matrix.script }}`
  and `${{ matrix.is_optional }}` instead of hard-coded values.

Tests
- `unit_tests/github_actions/test_optional_description.py`: verifies the
  `is_optional` description no longer inverts the behavior.
- `tests/test_cicd_yaml.py`: verifies the e2e jobs define a strategy block
  before referencing matrix context.

Closes NVIDIA-NeMo#728, NVIDIA-NeMo#749, NVIDIA-NeMo#750.

Signed-off-by: Andrew White <andrewwhitecdw@users.noreply.github.com>
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