Skip to content

Replace the upstream kserve dependency with the latest odh release#501

Merged
openshift-merge-bot[bot] merged 1 commit intoopendatahub-io:incubatingfrom
hdefazio:dev/odh_kserve_dep
Aug 20, 2025
Merged

Replace the upstream kserve dependency with the latest odh release#501
openshift-merge-bot[bot] merged 1 commit intoopendatahub-io:incubatingfrom
hdefazio:dev/odh_kserve_dep

Conversation

@hdefazio
Copy link
Copy Markdown
Contributor

@hdefazio hdefazio commented Aug 20, 2025

Description

Problem RHOAIENG-31386: The problem was that the odh-model-controller was acting on the object but didn't understand all of its fields (specifically the ExtMetricAuthentication field) , so it was accidentally damaging it before the kserve-controller could do its job correctly.

Fix: Replaces the kserve/kserve dependency in go.mod with the odh-v2.33 release of opendatahub-io/kserve so that ExtMetricAuthentication is defined in the kserve version that we are using

How Has This Been Tested?

I updated the odh-model-controller deployment to use the image quay.io/opendatahub/odh-model-controller:pr-498 (the pr # is 498 because this change was originally included in the same pr as the go 1.24 upgrade. They have since been split into separate prs)

Created the serving runtimes:

podman push kserve/sklearnserver:latest quay.io/hdefazio/sklearnserver

export SKLEARN_IMAGE=quay.io/hdefazio/sklearnserver:latest
export STORAGE_INITIALIZER_IMAGE=quay.io/opendatahub/kserve-storage-initializer:latest

kustomize build $PROJECT_ROOT/config/overlays/test/clusterresources |
  sed 's/ClusterServingRuntime/ServingRuntime/' |
  sed "s|kserve/sklearnserver:latest|${SKLEARN_IMAGE}|" |
  sed "s|kserve/storage-initializer:latest|${STORAGE_INITIALIZER_IMAGE}|" |
  oc apply -n rhoaieng-31386 -f -

then applied the following isvc:

apiVersion: serving.kserve.io/v1beta1
kind: InferenceService
metadata:
  annotations:
    serving.kserve.io/autoscalerClass: keda
    serving.kserve.io/deploymentMode: RawDeployment
  name: keda-test
  labels:
    networking.kserve.io/visibility: exposed
spec:
  predictor:
    autoScaling:
      metrics:
        - external:
            metric:
              backend: prometheus
              namespace: kserve-ci-e2e-test
              query: http_requests_per_second
              serverAddress: 'https://thanos-querier.openshift-monitoring.svc.cluster.local:9092'
            target:
              type: Value
              value: '50'
            authenticationRef:
              authModes: "bearer"
              authenticationRef:
                name: "inference-prometheus-auth"
          type: External
    maxReplicas: 5
    minReplicas: 1
    model:
      modelFormat:
        name: sklearn
      name: ''
      resources:
        limits:
          cpu: 100m
          memory: 256Mi
        requests:
          cpu: 50m
          memory: 128Mi
      storageUri: 'gs://kfserving-examples/models/sklearn/1.0/model'

The created keda-test ISVC was created successfully, with the authenticationRef field now reserved on the first oc apply. Without this change, authenticationRef was being removed from the ISVC because the kserve version defined in go.mod did not have the authenticationRef type defined.

Summary by CodeRabbit

  • Chores
    • Updated underlying dependencies to improve compatibility with Kubernetes Gateway APIs.
    • Aligned vendor sources with the current release branch to ensure consistency and stability.
    • No user-facing behavior changes; this is a maintenance update to support future enhancements.

Signed-off-by: Hannah DeFazio <h2defazio@gmail.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Aug 20, 2025

Walkthrough

Updated go.mod to add two indirect dependencies (gateway-api and gateway-api-inference-extension) and to replace the kserve module source with an Open Data Hub fork pinned to a specific pseudo-version, annotated with comments referencing the ODH release branch odh-v2.33.

Changes

Cohort / File(s) Summary of Changes
Dependencies Added (Indirect)
go.mod
Added indirect requires: sigs.k8s.io/gateway-api v1.2.1, sigs.k8s.io/gateway-api-inference-extension v0.3.0.
Module Replace Directive
go.mod
Added replace: github.com/kserve/kserve => github.com/opendatahub-io/kserve v0.0.0-20250812054942-f704774f7e6d; added comments noting use of ODH release branch odh-v2.33.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I thump my paws on modules’ ground,
New gateways sprout, dependencies found.
A hop to ODH, the branch in view—
Replace the trail, pin versions true.
Carrot commits and tidy burrows,
Go.mod hums as daylight narrows.

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 4ab2953 and 41ec7e3.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (1)
  • go.mod (2 hunks)
🔇 Additional comments (2)
go.mod (2)

157-158: Indirect gateway-API dependencies are appropriate

I confirmed there are no direct imports of sigs.k8s.io/gateway-api or sigs.k8s.io/gateway-api-inference-extension in the codebase. Keeping them as indirect requires is correct. If you later import these APIs directly, you can promote them to direct require entries.


168-170: Confirm module replace, imports coverage, and authenticationRef availability

  • All Go imports in the codebase reference github.com/kserve/kserve (no direct imports of the ODH fork), so the replace directive will apply uniformly.
  • go.sum has been updated with the ODH fork pseudo-version (v0.0.0-20250812054942-f704774f7e6d).
  • Please manually verify that the ODH fork at commit f704774f7e6d includes the authenticationRef API type (e.g. under pkg/apis/serving/v1alpha1) so your consumer code can import and use it.
  • Optional: if the ODH repo has a stable tag matching this commit (e.g. odh-v2.33), consider using that tag instead of the pseudo-version for clearer provenance.
✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

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

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@hdefazio
Copy link
Copy Markdown
Contributor Author

related threads from the previous pr:
#498 (comment)
#498 (comment)

@VedantMahabaleshwarkar
Copy link
Copy Markdown
Contributor

/approve

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Aug 20, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hdefazio, VedantMahabaleshwarkar

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [VedantMahabaleshwarkar,hdefazio]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mholder6
Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added the lgtm label Aug 20, 2025
@openshift-merge-bot openshift-merge-bot Bot merged commit 42beef0 into opendatahub-io:incubating Aug 20, 2025
8 checks passed
hdefazio added a commit to hdefazio/odh-model-controller that referenced this pull request Sep 2, 2025
openshift-merge-bot Bot pushed a commit that referenced this pull request Sep 17, 2025
)

Signed-off-by: Hannah DeFazio <h2defazio@gmail.com>
Signed-off-by: Vedant Mahabaleshwarkar <vmahabal@redhat.com>
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.

3 participants