chore: bump model registry dependency to 0.3.3 #596
Conversation
Signed-off-by: Alessio Pragliola <[email protected]>
WalkthroughUpdated Go module dependencies in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
Signed-off-by: Alessio Pragliola <[email protected]>
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
go.mod(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run on Ubuntu
🔇 Additional comments (3)
go.mod (3)
5-38: Verify testing of all dependency updates before merge.The PR objectives checklist is entirely unchecked, notably:
- "Testing instructions added for non-obvious changes"
- "Developer has manually tested and verified the changes"
Given the breadth of dependency updates (Kubernetes, controller-runtime, gRPC, OpenTelemetry, protobuf, and others), comprehensive testing is essential to catch subtle compatibility issues, particularly around the controller-runtime and Kubernetes ecosystem integration.
Please ensure:
- Unit and integration tests pass with all updated dependencies
- Manual verification of the model-registry log level change behavior in this codebase
- Verification that KEDA (which has a known compatibility workaround) continues to function correctly
- Confirmation that kserve integration works with the updated model-registry v0.3.3
14-15: Let me examine how the model-registry imports are actually used in the main reconciler code:Let me examine how the model-registry controller is actually used in the main reconciler code:The search results don't directly show kubeflow/model-registry v0.3.3 release notes. Let me search more specifically:
No changes needed—model-registry v0.3.3 breaking changes do not affect this codebase.
Verification confirms that the identified breaking changes in model-registry v0.3.3 (removal of Google MLMD dependency, type ID schema changes, and artifactTypes enum migration) do not impact odh-model-controller. The codebase uses the
pkg/inferenceservice-controllerpackage which provides a high-level reconciliation interface that abstracts away these internal schema changes. The codebase contains no direct assumptions about artifact types, type IDs, or MLMD—only test configuration references. Both pkg/inferenceservice-controller and pkg/openapi submodules are confirmed available in v0.3.3.
12-12: No compatibility concerns found with kserve v0.15.2.kserve v0.15.2 contains no formal breaking changes, and the specific behavior changes (ModelCar enablement, Knative autoscaler reconciliation order, predictor_config addition) are not used in this codebase. Kubernetes v0.33.5 is already the active dependency. Model Registry references appear only in test data (v0.1.1), with no production integration requiring the v0.3.3 update. The upgrade path is safe.
| k8s.io/utils v0.0.0-20250321185631-1f6e0b77f77e | ||
| knative.dev/pkg v0.0.0-20250117084104-c43477f0052b | ||
| sigs.k8s.io/controller-runtime v0.20.4 | ||
| sigs.k8s.io/controller-runtime v0.21.0 |
There was a problem hiding this comment.
🧩 Analysis chain
Verify controller-runtime v0.21.0 compatibility with Kubernetes v0.33.5.
The PR bumps controller-runtime to v0.21.0 in line with Kubernetes API/apimachinery/client-go updates to v0.33.5. Additionally, line 174 contains a replace directive for KEDA compatibility that pins controller-runtime to v0.19.1. Verify that:
- controller-runtime v0.21.0 is compatible with Kubernetes v0.33.5
- The KEDA workaround at line 174 is still valid or needs updating for the new controller-runtime version
- No breaking changes exist in controller-runtime v0.21.0 that affect this codebase
🌐 Web query:
Is controller-runtime v0.21.0 compatible with Kubernetes v0.33.5?
💡 Result:
Yes. controller-runtime v0.21.0 targets the k8s.io/* 0.33 line (bumped to v0.33.0), so using Kubernetes/client-go/apimachinery v0.33.5 is compatible (patch releases within the same minor are supported). [1][2]
Sources:
[1] controller-runtime compatibility table (README).
[2] controller-runtime v0.21.0 release notes.
---
🏁 Script executed:
```shell
#!/bin/bash
# Check line 174 in go.mod and surrounding context
sed -n '170,180p' go.mod
Length of output: 384
🏁 Script executed:
#!/bin/bash
# Search for KEDA references in the codebase
rg -i "keda" --type yaml --type go -C 2Length of output: 79967
🏁 Script executed:
#!/bin/bash
# Search web for KEDA and controller-runtime v0.21.0 compatibility
echo "Will search for KEDA controller-runtime compatibility if needed"Length of output: 63
🌐 Web query:
KEDA controller-runtime v0.21.0 v0.20.0 compatibility support
💡 Result:
Short answer: controller-runtime v0.21 is a breaking-minor bump (k8s.io/* → v0.33, Go 1.24, several API/behavior changes) so it is not drop‑in compatible with older controller-runtime minors; KEDA has reported incompatibility problems with controller-runtime ≥ v0.20 (webhook.Validator/deprecated webhook API issues). See the controller-runtime release notes/versioning and the KEDA issue for details. [1][2][3]
Sources
- controller-runtime v0.21.0 release notes (breaking changes: k8s.io/* v0.33, Go 1.24, deprecations). [1]
- controller-runtime compatibility/versioning table (CR v0.21 → k8s.io v0.33; CR v0.20 → k8s.io v0.32). [2]
- KEDA GitHub issue reporting incompatibility with controller-runtime ≥ v0.20 (webhook.Validator deprecation). [3]
Conflicting controller-runtime versions in go.mod: line 34 declares v0.21.0 but line 174 replace directive forces v0.19.1.
The replace directive at line 174 overrides the v0.21.0 specified at line 34, meaning the actual controller-runtime version will be v0.19.1, not v0.21.0. While KEDA has reported incompatibility with controller-runtime ≥ v0.20 (validating the workaround), this creates a contradiction in the PR:
- If v0.21.0 cannot be used due to KEDA incompatibility, the declaration at line 34 should remain v0.19.1
- If v0.21.0 is intended, the replace directive must be removed or updated, and KEDA compatibility must be resolved
Clarify the intent and reconcile these conflicting directives.
🤖 Prompt for AI Agents
In go.mod around lines 34 and 174, there is a conflict: line 34 declares
controller-runtime v0.21.0 while a replace directive at line 174 forces v0.19.1;
reconcile by choosing one source of truth — either change the module declaration
at line 34 to v0.19.1 to match the replace (if KEDA incompatibility prevents
using v0.20+), or remove/update the replace directive to allow v0.21.0 and then
resolve the KEDA compatibility issues; after making the change, run go mod tidy
(and rebuild/tests) to update go.sum and verify the project compiles with the
selected controller-runtime version.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Al-Pragliola, spolti The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
2 participants
Description
jira: RHOAIENG-28128
This PR lowers the log to Info instead of Error when the labels/annotation needed by the mr inferenceservice controller are missing
How Has This Been Tested?
unit tests
Merge criteria:
Summary by CodeRabbit