Skip to content

chore: enable volume extra test - #754

Merged
mckornfield merged 2 commits into
mainfrom
jobs-reenable-volume-test/mck
Jul 22, 2026
Merged

chore: enable volume extra test#754
mckornfield merged 2 commits into
mainfrom
jobs-reenable-volume-test/mck

Conversation

@mckornfield

@mckornfield mckornfield commented Jul 17, 2026

Copy link
Copy Markdown
Contributor
  • Also fix nss test

Summary by CodeRabbit

  • New Features
    • Added E2E Helm configuration for Kubernetes/Volcano-style job executors to attach an extra PVC-backed volume mounted at /mnt/additional_storage.
    • Enabled an additional-volume CPU executor profile across the E2E inference gateway and minikube/kind environments.
  • Tests
    • Updated the additional-volume job E2E test to run based on whether the required /mnt/additional_storage mount is available.
    • Improved Safe Synthesizer E2E model-fileset setup by loading the setup logic in-process and validating the expected fileset count.
    • Adjusted the online evaluator mock model flow to control virtual-model auto-provisioning behavior explicitly.

@mckornfield
mckornfield requested review from a team as code owners July 17, 2026 13:50
@github-actions github-actions Bot added the chore label Jul 17, 2026
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 087db9ec-1e69-4bff-a27a-60dcc929b00f

📥 Commits

Reviewing files that changed from the base of the PR and between bb75b8f and d52d5c8.

📒 Files selected for processing (1)
  • e2e/test_evaluator_plugin.py

📝 Walkthrough

Walkthrough

E2E Kubernetes job profiles now mount a PVC-backed volume at /mnt/additional_storage, and the additional-volume test selects or skips the profile based on service configuration. Safe Synthesizer setup runs in-process, while evaluator mock-model provisioning uses explicit virtual-model controls.

Changes

E2E job storage

Layer / File(s) Summary
PVC-backed job storage configuration
e2e/k8s/values/*.yaml
Kubernetes job executor profiles define nmp-additional-volume, its PVC wiring, and the /mnt/additional_storage mount.
Conditional additional-volume validation
e2e/test_jobs.py
The test receives RunningServices, selects the configured profile for both job steps, and skips when the required mount is unavailable.

Safe Synthesizer fileset setup

Layer / File(s) Summary
In-process model-fileset registration
e2e/test_safe_synthesizer.py
The fixture dynamically loads setup_model_filesets.py, invokes create_filesets, validates the created-fileset count, and removes subprocess-specific handling.

Evaluator mock-model provisioning

Layer / File(s) Summary
Explicit virtual-model provisioning controls
e2e/test_evaluator_plugin.py
Mock provider creation and passthrough virtual-model setup explicitly disable auto-provisioning and mark the resulting model as non-autoprovisioned.

Possibly related PRs

Suggested labels: test

Suggested reviewers: tylersbray, svvarom, anastasia-nesterenko

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the main change: enabling the additional-volume E2E test.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jobs-reenable-volume-test/mck

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor
Suite Lines Covered Line Rate Branch Rate
Unit Tests 26391/34046 77.5% 61.7%
Integration Tests 15161/32671 46.4% 18.6%

* Also fix nss test

Signed-off-by: Matt Kornfield <mkornfield@nvidia.com>
@mckornfield
mckornfield force-pushed the jobs-reenable-volume-test/mck branch from b254027 to bb75b8f Compare July 21, 2026 18:35

@coderabbitai coderabbitai Bot 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.

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 `@e2e/k8s/values/minikube.yaml`:
- Line 40: Update the volume_permissions_image entry in the Minikube values
configuration to define it under core.storage, matching the path used by the
other values files and the configured core.volumePermissionsImage value. Ensure
the resulting Helm values structure applies the configured volume permissions
image for this profile.
🪄 Autofix (Beta)

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: 515de610-9706-457b-888b-4c943d6598dc

📥 Commits

Reviewing files that changed from the base of the PR and between b254027 and bb75b8f.

📒 Files selected for processing (7)
  • e2e/k8s/values/default.yaml
  • e2e/k8s/values/kind.yaml
  • e2e/k8s/values/minikube-auth-portforward.yaml
  • e2e/k8s/values/minikube-auth.yaml
  • e2e/k8s/values/minikube.yaml
  • e2e/test_jobs.py
  • e2e/test_safe_synthesizer.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • e2e/test_jobs.py
  • e2e/test_safe_synthesizer.py

Comment thread e2e/k8s/values/minikube.yaml
Signed-off-by: Matt Kornfield <mkornfield@nvidia.com>
@mckornfield
mckornfield added this pull request to the merge queue Jul 21, 2026
Merged via the queue into main with commit def1e4d Jul 22, 2026
60 checks passed
@mckornfield
mckornfield deleted the jobs-reenable-volume-test/mck branch July 22, 2026 00:15
soluwalana pushed a commit that referenced this pull request Jul 22, 2026
* chore: enable volume extra test

* Also fix nss test

Signed-off-by: Matt Kornfield <mkornfield@nvidia.com>

* chore: attempt to fix a flake in eval test

Signed-off-by: Matt Kornfield <mkornfield@nvidia.com>

---------

Signed-off-by: Matt Kornfield <mkornfield@nvidia.com>
Signed-off-by: Sam Oluwalana <soluwalana@nvidia.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