Skip to content

Fix test in sanity for Rawdeployment dsc check#1180

Merged
mwaykole merged 4 commits intoopendatahub-io:mainfrom
mwaykole:fix-sanity
Mar 6, 2026
Merged

Fix test in sanity for Rawdeployment dsc check#1180
mwaykole merged 4 commits intoopendatahub-io:mainfrom
mwaykole:fix-sanity

Conversation

@mwaykole
Copy link
Copy Markdown
Member

@mwaykole mwaykole commented Mar 6, 2026

Pull Request

Summary

Related Issues

  • Fixes:
  • JIRA:

How it has been tested

  • Locally
  • Jenkins

Additional Requirements

  • If this PR introduces a new test image, did you create a PR to mirror it in disconnected environment?
  • If this PR introduces new marker(s)/adds a new component, was relevant ticket created to update relevant Jenkins job?

Summary by CodeRabbit

  • Tests
    • Updated model-serving tests to include networking exposure settings for KServe deployments, ensuring services are reachable when required.
    • Refined test markers and parametrization for deployment-mode suites to rely on module-level marks and improve selective test collection.
    • Corrected log message formatting in test utilities so deployment-mode values are rendered clearly in logs.

Signed-off-by: Milind waykole <mwaykole@redhat.com>
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 6, 2026

The following are automatically added/executed:

  • PR size label.
  • Run pre-commit
  • Run tox
  • Add PR author as the PR assignee
  • Build image based on the PR

Available user actions:

  • To mark a PR as WIP, add /wip in a comment. To remove it from the PR comment /wip cancel to the PR.
  • To block merging of a PR, add /hold in a comment. To un-block merging of PR comment /hold cancel.
  • To mark a PR as approved, add /lgtm in a comment. To remove, add /lgtm cancel.
    lgtm label removed on each new commit push.
  • To mark PR as verified comment /verified to the PR, to un-verify comment /verified cancel to the PR.
    verified label removed on each new commit push.
  • To Cherry-pick a merged PR /cherry-pick <target_branch_name> to the PR. If <target_branch_name> is valid,
    and the current PR is merged, a cherry-picked PR would be created and linked to the current PR.
  • To build and push image to quay, add /build-push-pr-image in a comment. This would create an image with tag
    pr-<pr_number> to quay repository. This image tag, however would be deleted on PR merge or close action.
Supported labels

{'/build-push-pr-image', '/hold', '/cherry-pick', '/wip', '/verified', '/lgtm'}

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 6, 2026

📝 Walkthrough

Walkthrough

Three KServe test files updated: conftest adds KServe networking labels to ISVC creation, a block-level pytest marker was removed relying on module-level markers, and a log message was fixed to interpolate the deployment_mode variable.

Changes

Cohort / File(s) Summary
Configuration & Fixtures
tests/model_serving/model_server/kserve/components/kserve_dsc_deployment_mode/conftest.py
Imported Labels and passed a labels mapping to create_isvc() setting Labels.Kserve.NETWORKING_KSERVE_IOLabels.Kserve.EXPOSED, enabling KServe network I/O exposure.
Test Markers
tests/model_serving/model_server/kserve/components/kserve_dsc_deployment_mode/test_kserve_dsc_default_deployment_mode.py
Removed a block-level @pytest.mark.rawdeployment decorator; tests now rely on existing module-level markers for collection.
Logging
tests/model_serving/model_server/kserve/components/kserve_dsc_deployment_mode/utils.py
Fixed log message in wait_for_default_deployment_mode_in_cm() to interpolate the deployment_mode variable instead of printing a literal placeholder.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Security notes (actionable):

  • Exposing network I/O via KServe label can increase attack surface; validate that only intended services are exposed and review network policies (relevant: CWE-200 Information Exposure).
  • Confirm no sensitive values are written to logs after the interpolation change; avoid logging secrets (relevant: CWE-532 Exposure of Sensitive Information to an Unauthorized Actor).
🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning Pull request description is empty template boilerplate with no substantive implementation details, objectives, or testing information provided. Fill in the Summary section with actual changes made. Document why these changes were necessary. Specify related issues/JIRA tickets and provide concrete testing details.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main changes: fixing a test issue related to Rawdeployment and DSC checks in the sanity test suite.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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

@mwaykole mwaykole enabled auto-merge (squash) March 6, 2026 11:22
@mwaykole mwaykole disabled auto-merge March 6, 2026 11:46
@mwaykole mwaykole enabled auto-merge (squash) March 6, 2026 11:46
Signed-off-by: Milind waykole <mwaykole@redhat.com>
@mwaykole mwaykole merged commit f42ddab into opendatahub-io:main Mar 6, 2026
7 checks passed
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 6, 2026

Status of building tag latest: success.
Status of pushing tag latest to image registry: success.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants