Skip to content

Remove jira ref form code#1293

Merged
mwaykole merged 6 commits intoopendatahub-io:mainfrom
mwaykole:jiracleanup
Mar 25, 2026
Merged

Remove jira ref form code#1293
mwaykole merged 6 commits intoopendatahub-io:mainfrom
mwaykole:jiracleanup

Conversation

@mwaykole
Copy link
Copy Markdown
Member

@mwaykole mwaykole commented Mar 25, 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
    • Two KServe OCI image tests previously skipped are now enabled.
    • A KServe authentication test that was gated as an expected failure was removed.
    • Simplified an authentication test fixture by removing an unused parameter and related pod-wait logic to shorten setup and reduce flakiness.
  • Chores
    • Added non-functional comments/formatting markers to utility helpers (no behavior changes).

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

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

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

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 25, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Removed admin-client usage and logger from an authentication fixture; simplified fixture to unconditionally clear KServe auth annotation and wait for ISVC readiness. Deleted a JIRA-gated xfail test and re-enabled two OCI storage tests by removing skip decorators. Minor formatting/comment edits to utilities.

Changes

Cohort / File(s) Summary
Authentication Fixture Cleanup
tests/model_serving/model_server/kserve/authentication/conftest.py
Removed admin_client parameter and module logger. Deleted imports/usages of get_pods_by_isvc_label and is_jira_open. Fixture now unconditionally sets metadata.annotations[Annotations.KserveAuth.SECURITY] = "false" via ResourceEditor, waits for ISVC READY/True with wait_for_condition before yielding, and waits again after ResourceEditor exits.
Test Removal (JIRA-gated)
tests/model_serving/model_server/kserve/authentication/test_kserve_token_authentication_raw.py
Removed test_raw_disable_enable_authentication_no_pod_rollout and its @pytest.mark.xfail(...) referencing RHOAIENG-52129; removed imports related to pod lookup/status and JIRA checks.
OCI Tests Re-enabled
tests/model_serving/model_server/kserve/storage/oci/test_oci_image.py
Removed @pytest.mark.skip(...) decorators from test_model_car_using_rest and test_model_status_loaded, allowing them to run under existing test markers.
Jira Utility Comment
utilities/jira.py
Added trailing # skip-unused-code comment to the is_jira_open function definition line; no logic change.
Infra Formatting Comment
utilities/infra.py
Reformatted check_pod_status_in_time signature across multiple lines and added # skip-unused-code trailing comment; signature and logic unchanged.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Security findings (actionable)

  • Race/timing (TOCTOU): Removing the pod-rollout/unconditional pod-wait logic can cause test flakiness or assertions against stale pod state. Action: ensure deterministic synchronization where tests rely on pod identity/state (add polling/wait-for-rollout or explicit readiness checks). Cite: CWE-362.
  • Privilege/coverage regression: Dropping admin_client from the fixture removes admin-path execution and may reduce coverage for privileged operations. Action: add explicit admin-enabled tests or a separate fixture to exercise admin flows. Cite: CWE-269.
🚥 Pre-merge checks | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is entirely unfilled template boilerplate with no substantive content, missing all required information about changes, testing, and related issues. Complete all sections: provide a detailed summary of the changes, reference the JIRA tickets being cleaned up, document testing performed locally and in Jenkins, and address the additional requirements checklist.
Title check ❓ Inconclusive The title partially describes the changeset by referencing JIRA reference removal, which is a real aspect of the changes, but it is vague and doesn't clearly convey what was actually removed from the codebase. Provide a more specific title that clarifies the scope of removal, such as 'Remove unused JIRA status checks from authentication tests' or 'Clean up JIRA-dependent test fixtures and logic'.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@mwaykole mwaykole requested a review from a team as a code owner March 25, 2026 06:12
Raghul-M
Raghul-M previously approved these changes Mar 25, 2026
fege
fege previously approved these changes Mar 25, 2026
Copy link
Copy Markdown
Contributor

@fege fege left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

Signed-off-by: Milind waykole <mwaykole@redhat.com>
@mwaykole mwaykole merged commit 850de4b into opendatahub-io:main Mar 25, 2026
11 checks passed
@github-actions
Copy link
Copy Markdown

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.

5 participants