chore: add odh-model-controller as infra component#133
chore: add odh-model-controller as infra component#133jland-redhat merged 1 commit intoopendatahub-io:mainfrom
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds the odh-model-controller base (ref=incubating) to the KServe kustomization resources, expanding the manifests applied by this deployment overlay. Changes
Sequence Diagram(s)sequenceDiagram
actor Dev as Developer
participant Kz as Kustomize (kserve overlay)
participant K8s as Kubernetes API Server
participant KS as KServe Resources
participant ODH as ODH Model Controller
Dev->>Kz: kustomize build/apply
Kz->>K8s: Submit manifests
K8s-->>KS: Create/Update KServe resources
rect rgba(200,235,255,0.3)
note right of ODH: New in this change
K8s-->>ODH: Create/Update odh-model-controller (config/base?ref=incubating)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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 |
![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)
maas-api/deploy/infra/odh/kustomization.yaml(1 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: build
| @@ -8,6 +8,7 @@ metadata: | |||
| # For now, install only Model Serving pieces | |||
| resources: | |||
| - github.com/opendatahub-io/kserve/config/overlays/odh?ref=release-v0.15 | |||
| - github.com/opendatahub-io/odh-model-controller/config/base?ref=incubating | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Pin odh-model-controller to an immutable ref.
Referencing the incubating branch means this build will drift whenever that branch changes. Please pin to a released tag or commit SHA (similar to the existing release-v0.15 reference) so infra deployments stay reproducible.
🤖 Prompt for AI Agents
In maas-api/deploy/infra/odh/kustomization.yaml around line 11, the kustomize
remote reference uses the mutable branch ref "incubating"; change it to a fixed
immutable ref by replacing "?ref=incubating" with a specific released tag or
commit SHA (for example match the style used elsewhere like "?ref=release-v0.15"
or use an exact commit hash). Update the line to point to the chosen tag/SHA and
ensure any related documentation or deployment notes reflect the pinned version
for reproducible infra deployments.
There was a problem hiding this comment.
For now we need the latest code base for testing
There was a problem hiding this comment.
@bartoszmajsak, understood! Using the incubating branch makes sense for active testing and development.
Once testing is complete and you're ready to stabilize, consider:
- Pinning to a specific commit SHA or tag for production deployments
- Adding a TODO comment in the kustomization file to track this decision
- Creating a follow-up issue to revisit this before merging to ensure reproducible deployments
Would you like me to open an issue to track the stabilization task for when testing is complete?
|
Before we merge it we probably need two things
I also think this should be a |
|
Just an additional thought |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: israel-hdez, jland-redhat 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 |
Strictly, `odh-model-controller` would not act as infrastructure component. It will provide automation for creating the required Roles and RoleBindings for giving access to the models enrolled as-a-service. However, this seems to be the best place to put it in. Signed-off-by: Edgar Hernández <23639005+israel-hdez@users.noreply.github.com>
israel-hdez
force-pushed
the
add-omc-to-infra
branch
from
cfd952c to
3f4b86b
Compare
|
New changes are detected. LGTM label has been removed. |
israel-hdez
changed the title
Strictly, `odh-model-controller` would not act as infrastructure component. It will provide automation for creating the required Roles and RoleBindings for giving access to the models enrolled as-a-service. However, this seems to be the best place to put it in. Signed-off-by: Edgar Hernández <23639005+israel-hdez@users.noreply.github.com>
4 participants
Strictly,
odh-model-controllerwould not act as infrastructurecomponent. It will provide automation for creating the required Roles and
RoleBindings for giving access to the models enrolled as-a-service. (PR 534)
However, this seems to be the best place to put it in.
How Has This Been Tested?
Run
kustomize build maas-api/deploy/infra/odhMerge criteria:
Summary by CodeRabbit
New Features
Chores