Use opendatahub-io/kserve's latest ODH release as our kserve dependency#499
Use opendatahub-io/kserve's latest ODH release as our kserve dependency#499hdefazio wants to merge 8 commits intoopendatahub-io:incubatingfrom
Conversation
Signed-off-by: Hannah DeFazio <h2defazio@gmail.com>
Signed-off-by: Hannah DeFazio <h2defazio@gmail.com>
Signed-off-by: Hannah DeFazio <h2defazio@gmail.com>
Signed-off-by: Hannah DeFazio <h2defazio@gmail.com>
Signed-off-by: Hannah DeFazio <h2defazio@gmail.com>
Signed-off-by: Hannah DeFazio <h2defazio@gmail.com>
Signed-off-by: Hannah DeFazio <h2defazio@gmail.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hdefazio 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 |
WalkthroughProject-wide upgrade to Go 1.24 across dev containers, CI workflows, container build, and documentation. go.mod updated to Go 1.24.6 with multiple dependency bumps, additions, removals, and a replace directive. Devspace image updated with a new profile for Go 1.22 fallback. No application code changes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
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. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
.github/workflows/test.yml (1)
15-19: Use go-version-file and enable module cache to avoid drift and speed up CIReading the version from go.mod keeps workflows in sync with the repo toolchain, and enabling cache speeds up builds/tests.
- name: Setup Go uses: actions/setup-go@v5 with: - go-version: '~1.24' + go-version-file: 'go.mod' + cache: true.devcontainer/devcontainer.json (1)
3-3: Consider pinning the patch version for reproducibilityUsing golang:1.24 floats to the latest 1.24.x. Pinning to the same patch as go.mod (e.g., 1.24.6) reduces drift between local dev and CI.
- "image": "golang:1.24", + "image": "golang:1.24.6",.github/workflows/lint.yml (1)
15-19: Read Go version from go.mod and enable cacheKeeps lint workflow synced to the repo toolchain and speeds up runs.
- name: Setup Go uses: actions/setup-go@v5 with: - go-version: '~1.24' + go-version-file: 'go.mod' + cache: true.github/workflows/test-e2e.yml (1)
16-20: Use go-version-file and enable cache for E2E as wellThis keeps E2E in lockstep with go.mod and speeds dependency resolution.
- name: Setup Go uses: actions/setup-go@v5 with: - go-version: '~1.24' + go-version-file: 'go.mod' + cache: trueContainerfile (1)
2-2: Pin builder image to exact patch (1.24.6) to match go.mod and improve reproducibilityUsing a floating minor tag can drift over time. Given go.mod is set to go 1.24.6, pin the builder image to the same patch to keep builds reproducible and consistent with CI.
Apply this diff:
-FROM registry.access.redhat.com/ubi9/go-toolset:1.24 as builder +FROM registry.access.redhat.com/ubi9/go-toolset:1.24.6 as builderIf 1.24.6 is unavailable in the registry, pin to the latest available 1.24.x tag or to a digest.
dev_tools/devspace.yaml (1)
58-58: Dev image version: consider pinning by digest or aligning to go 1.24.6The base dev image is bumped to Go 1.24.2. To avoid toolchain drift between dev, CI, and the Containerfile’s builder, either:
- align to 1.24.6 (to match go.mod), or
- pin this tag by digest for reproducibility.
go.mod (2)
168-171: Replace to ODH KServe: verify commit aligns with odh-v2.33 release; prefer tag if availableThe replace correctly switches to opendatahub-io/kserve. Please confirm the pinned commit f704774f7e6d corresponds to the intended odh-v2.33 release point. If the ODH repo exposes a tag for odh-v2.33, prefer pinning to that tag for clarity and discoverability.
Optionally (if a tag exists), switch to it:
-replace github.com/kserve/kserve => github.com/opendatahub-io/kserve v0.0.0-20250812054942-f704774f7e6d +replace github.com/kserve/kserve => github.com/opendatahub-io/kserve odh-v2.33
167-167: Controller-runtime replace to v0.19.1: double-check compatibility with k8s 0.33.1 and your codeYou require k8s.io/* v0.33.1 but override controller-runtime to v0.19.1 (to work around KEDA #6660). That downgrade can introduce version skew versus the Kubernetes libs and controller-runtime’s own compatibility matrix. Please ensure:
- controller-runtime v0.19.1 is compatible with k8s v0.33.x used here, and
- your code (and dependencies) don’t rely on APIs introduced in v0.20+.
If feasible, consider upgrading KEDA to a version that supports controller-runtime >= v0.20 to remove this global override.
I can help check controller-runtime/k8s compatibility guidance and propose options (e.g., bumping KEDA or isolating the dependency) if you share your target cluster versions and constraints.
📜 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.
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (8)
.devcontainer/devcontainer.json(1 hunks).github/workflows/lint.yml(1 hunks).github/workflows/test-e2e.yml(1 hunks).github/workflows/test.yml(1 hunks)Containerfile(1 hunks)README.md(1 hunks)dev_tools/devspace.yaml(2 hunks)go.mod(6 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
dev_tools/devspace.yaml
[error] 106-106: trailing spaces
(trailing-spaces)
🔇 Additional comments (6)
.github/workflows/test.yml (1)
18-18: All toolchain and kserve replace checks passed
- go.mod specifies
go 1.24.6replace github.com/kserve/kserve => github.com/opendatahub-io/kserve …is present- No unintended Go 1.22 toolchain references (only indirect dependencies and devspace profile)
- All CI workflows use
actions/setup-go@v5withgo-version: '~1.24'README.md (1)
20-20: LGTM: prerequisites aligned with toolchain upgradeThe Go prerequisite now matches the repo-wide upgrade.
.github/workflows/lint.yml (1)
23-24: LGTM: golangci-lint version bumpPinned to v1.64.8, which aligns with Go 1.24 usage.
dev_tools/devspace.yaml (1)
105-105: Nice addition: go1.22 fallback profileThe selective patching via a profile is a clean way to preserve the legacy setup when needed.
Also applies to: 107-110
go.mod (2)
3-3: Go toolchain bumped to 1.24.6LGTM. This matches the broader toolchain upgrade in the repo; just ensure all build surfaces (builder image, dev images, CI) are aligned to avoid patch-level drift.
27-31: Gateway API dependencies added but not used—only controller-runtime compatibility needs attention
- Indirect deps found in go.mod:
- sigs.k8s.io/gateway-api v1.2.1
- sigs.k8s.io/gateway-api-inference-extension v0.3.0
- No direct imports of these modules in code.
- No Gateway API CRD kinds (Gateway, HTTPRoute, ReferenceGrant, BackendTLSPolicy, GRPCRoute, TLSRoute, UDPRoute, TCPRoute) in
config/.Action: verify your controller-runtime version (v0.19.1) is compatible with Kubernetes API v0.33.1.
| # path: ./ui # Path-based dependencies (for monorepos) | ||
|
|
||
| profiles: | ||
| - name: go1.22 |
There was a problem hiding this comment.
Fix YAML lint error: trailing space on profile name
Yamllint flags a trailing space at Line 106. Remove it to satisfy linting.
Apply this diff:
- - name: go1.22
+ - name: go1.22📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: go1.22 | |
| - name: go1.22 |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 106-106: trailing spaces
(trailing-spaces)
🤖 Prompt for AI Agents
In dev_tools/devspace.yaml around line 106 the profile name has a trailing space
("go1.22 "), which triggers a yamllint trailing-space error; remove the trailing
space so the entry reads "go1.22" (no extra whitespace), save the file and
re-run the linter to confirm the error is resolved.
|
@hdefazio: The following test failed, say
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. |
|
related threads from the previous pr: |
|
closing in favor of #501 to resolve rebasing issues |
1 participant
Description
RHOAIENG-31386
Replaces the kserve/kserve dependency in go.mod with the odh-v2.33 release of opendatahub-io/kserve
How Has This Been Tested?
I updated the odh-model-controller deployment to use the image quay.io/opendatahub/odh-model-controller:pr-498
Created the serving runtimes:
then applied the following isvc:
The created
keda-testISVC was created successfully, with theauthenticationReffield now reserved on the firstoc apply. Without this change,authenticationRefwas being removed from the ISVC because the kserve version defined in go.mod did not have theauthenticationReftype defined.Summary by CodeRabbit
Chores
Documentation