feat(initializer): validate initializer Pod uses correct volume name#3714
feat(initializer): validate initializer Pod uses correct volume name#3714HarshPopat23 wants to merge 4 commits into
Conversation
…validation in the JobSet plugin's Validate() function to ensure the dataset/model initializer ReplicatedJob has a Volume named jobsetplgconsts.VolumeNameInitializer. Previously this was only a string constant with no enforcement, meaning a misconfigured runtime template could silently reference the wrong volume name. Fixes kubeflow#3710 Signed-off-by: HarshPopat23 <musichk61@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR strengthens admission-time validation in the JobSet runtime plugin so misconfigured dataset/model initializer templates fail fast (with a clear validation error) rather than later during Pod scheduling/mount.
Changes:
- Adds tracking/validation of volume names per
ReplicatedJoband requires the initializer volume name to bejobsetplgconsts.VolumeNameInitializer. - Updates JobSet plugin validation tests to assert the new volume-name error cases.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| pkg/runtime/framework/plugins/jobset/jobset.go | Adds initializer-volume name validation to Validate() for dataset/model initializer jobs. |
| pkg/runtime/framework/plugins/jobset/jobset_test.go | Extends validation test expectations and adds cases covering missing initializer volume. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: HarshPopat23 <musichk61@gmail.com>
Signed-off-by: HarshPopat23 <musichk61@gmail.com>
andreyvelich
left a comment
There was a problem hiding this comment.
Thanks for the contribution, @HarshPopat23! 🙏 A few comments below. Note: this review was performed with the help of AI tools.
Signed-off-by: HarshPopat23 <musichk61@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Updated the PR to address the Copilot and maintainer review feedback:
Tests:
I also attempted:
but it failed locally because my Windows disk/cache is full: |
What this PR does / why we need it ?:
Adds validation in the JobSet plugin's
Validate()function to ensure thedataset/model initializer
ReplicatedJobhas a Volume namedjobsetplgconsts.VolumeNameInitializer.Previously
VolumeNameInitializerwas only a string constant with noenforcement — a misconfigured runtime template could silently reference
the wrong volume name and only fail later at Pod scheduling/mount time,
instead of at admission time with a clear error.
This follows the same pattern already used in this function for validating
container names (
rJobContainerNames), extended to also track volumenames (
rJobVolumeNames) per ReplicatedJob.Which issue(s) this PR fixes (optional, in
Fixes #<issue number>, #<issue number>, ...format, will close the issue(s) when PR gets merged):Fixes #3710
Checklist: