fix(kserve): build localmodel operands for e2e-kserve-module - #84074
fix(kserve): build localmodel operands for e2e-kserve-module#84074bartoszmajsak wants to merge 1 commit into
Conversation
The kserve-module e2e deploys its operands from the tags in params.env, while the manifests configuring them come from the PR under test. Those move independently, so a change that adds a flag to a binary and the arg that passes it lands as a pair but gets tested as new arg against the previously published binary, which rejects it and crash-loops. The localmodel controller and node agent were the operands still reading published tags and actually running in this job; build them and pass them through. The agent and router images were already built here but never handed to the test, so wire those up too at no extra build cost. Every operand the job deploys now comes from the PR. The consuming side already exists in the repo. Signed-off-by: Bartosz Majsak <bartosz.majsak@gmail.com>
|
/pj-rehearse-auto-ack |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (2)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. WalkthroughThe KServe master and release v0.17 CI configurations now build local model images and pass those images, along with existing KServe dependencies, to the KServe module E2E test. ChangesKServe local model CI
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change builds and supplies the localmodel images used by the end-to-end test, keeping the tested binaries aligned with the manifests under test. No actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (2 skipped: 2 unsupported.) Full details: Stable And Deterministic Test NamesExplanation The pull request does not add or change any Ginkgo test title. Its six changed files contain CI image and dependency configuration only. The exact diff adds Dockerfile image entries and Full details: Test Structure And QualityExplanation PASS: The pull request changes only CI YAML configuration and generated job YAML. The diff adds image build entries, test image dependencies, and Dockerfile path filters. It does not modify Ginkgo test code or any It, BeforeEach, AfterEach, Eventually, or Consistently blocks, so the listed test-structure requirements are not applicable. Full details: Microshift Test CompatibilityExplanation The check is not applicable. The pull request changes only CI YAML and generated Prow job YAML. The diff adds Docker image builds, image dependencies, and sparse-checkout entries. It adds no Ginkgo test declarations or test source, and the existing Full details: Single Node Openshift (Sno) Test CompatibilityExplanation PASS: The pull request adds only CI YAML image/dependency configuration and generated job sparse-checkout entries. The commit changes no Go or e2e test files and adds no Ginkgo It(), Describe(), Context(), or When() declarations. Therefore, the SNO check is not applicable. Full details: Topology-Aware Scheduling CompatibilityExplanation PASS: The pull request changes only CI image and job configuration. It adds Dockerfile build entries, image dependencies, and sparse-checkout filenames. It does not modify deployment manifests, operator code, controllers, replica settings, affinity, topology spread, node selectors, tolerations, or PDBs. Therefore, it introduces none of the stated topology-compatibility failures. Full details: Ote Binary Stdout ContractExplanation PASS — The pull request changes only CI YAML. The committed diff adds two image build entries, test image dependencies, and Dockerfile sparse-checkout paths. It does not modify Go or test-suite source, Full details: Ipv6 And Disconnected Network Test CompatibilityExplanation PASS: The pull request adds only CI YAML image-build entries, test image dependencies, and generated job triggers. The exact diff changes no Ginkgo test code and adds no IPv4 address, IP parsing, URL construction, external host, download, or public-registry access. The existing Full details: No-Weak-CryptoExplanation PASS. The pull request changes only CI YAML files. The added lines add Dockerfile entries, image targets, sparse-checkout paths, and test dependency mappings. The diff contains no MD5, SHA1, DES, RC4, 3DES, Blowfish, ECB, custom crypto implementation, or non-constant-time secret/token comparison. Full details: Container-PrivilegesExplanation The pull request adds image-build entries, test image dependencies, and Dockerfile sparse-checkout paths only. The parent-to-HEAD diff adds no Full details: No-Sensitive-Data-In-LogsExplanation PASS. The commit changes only CI image build entries and E2E dependency mappings in the two YAML files. The added values are Dockerfile and image identifiers. No logging command, secret, token, password, personal data, session identifier, or customer data was added. Existing log-gather commands and the existing BASE_DOMAIN value are unchanged. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
openshift-ci
Bot
added
the
approved
|
@bartoszmajsak: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bartoszmajsak, pierDipi The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
[REHEARSALNOTIFIER]
Prior to this PR being merged, you will need to either run and acknowledge or opt to skip these rehearsals. Interacting with pj-rehearseComment: Once you are satisfied with the results of the rehearsals, comment: |
|
@bartoszmajsak: job(s): /pj-rehearse-auto-ack either don't exist or were not found to be affected, and cannot be rehearsed |
|
/pj-rehearse auto-ack |
|
@bartoszmajsak: now processing your pj-rehearse request. Please allow up to 10 minutes for jobs to trigger or cancel. |
openshift-merge-bot
Bot
added
the
rehearsals-ack
|
@bartoszmajsak: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
2 participants
Builds the two localmodel operand images for
opendatahub-io/kserveand passes them to thee2e-kserve-moduletest.Why
The ModelCache e2e deploys the localmodel controller and node agent from the image tags in
params.env, while the manifests that configure them come from the PR under test. Those two move independently, so a change that adds a flag to a localmodel binary together with the arg that passes it gets tested as new arg against previously published binary. The binary rejects the unknown flag, crash-loops,ModelCacheReadynever goes true, and the ModelCache tests time out.This is currently breaking
e2e-kserve-moduleonrelease-v0.17PRs. The five other operands the test deploys are already immune because the job builds and injects them; localmodel was the only one left reading a published tag.Summary by CodeRabbit
The OpenShift CI configuration for
opendatahub-io/kservenow builds the local model controller and node agent images from the PR source.The
e2e-kserve-moduletest now uses these images, along with the KServe agent and router images. This keeps binaries and manifests compatible during CI testing.