Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Oneroute #48

Conversation

VedantMahabaleshwarkar
Copy link
Contributor

Description

This PR modifies odh-model-controller as follows :

  • 1 route will be created PER namespace for accessing all models in the namespace.
  • Route creation/deletion is controller by creation/deletion of ServingRuntimes instead of InferenceServices
  • Old routes will automatically not be deleted if they were created BEFORE the upgrade (one routes being the ones created per inferenceservice)
  • Old route will be deleted if the associated inferenceservice is deleted (same behavior as before)

Testing

  • Deploy existing production RHODS
  • Deploy a model server and isvc and confirm 1 route is created for the isvc
  • Uninstall RHODS (do not delete the model server and isvc)
  • Install rhods using http://quay.io/vedantm/rhods-operator-live-catalog:1.28.0-oneroute
  • Navigate to the NS with the model server
  • Verify 2 routes exist (old route + new -model-route)
  • Create a NEW isvc with a different name, verify this new isvc does not get it's own route
  • Delete original isvc, verify if the route associated with the isvc got deleted but the new -model-route still exists.
  • Delete model server and verify the associated route is deleted.

JIRA: https://issues.redhat.com/browse/RHODS-5888
Doc: https://docs.google.com/document/d/1PCL3wgkk_OSY9Km9vOt2kZ4t6lEtitj223dYea-Uuk4/edit#heading=h.w5e37xee1yqq
ADR : TBD

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 1, 2023

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 1, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: VedantMahabaleshwarkar

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:
  • OWNERS [VedantMahabaleshwarkar]

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

@openshift-ci openshift-ci bot added the approved label Jun 1, 2023
@VedantMahabaleshwarkar
Copy link
Contributor Author

PR is ready for review. However the merge for this PR should happen after kserve/modelmesh#90 has made it's way downstream

@VedantMahabaleshwarkar
Copy link
Contributor Author

/test all

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jun 1, 2023

@VedantMahabaleshwarkar: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/unit 9a9d4d9 link true /test unit

Full PR test history. Your PR dashboard.

Instructions 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/test-infra repository. I understand the commands that are listed here.

@spolti
Copy link
Member

spolti commented Jan 26, 2024

@VedantMahabaleshwarkar is this still valid?

@openshift-merge-robot
Copy link
Contributor

PR needs rebase.

Instructions 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/test-infra repository.

@israel-hdez
Copy link
Contributor

@spolti not valid, anymore. Let's close this.

@spolti
Copy link
Member

spolti commented Jan 26, 2024

/close

@spolti spolti closed this Jan 26, 2024
heyselbi pushed a commit to heyselbi/odh-model-controller that referenced this pull request Jun 21, 2024
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.

4 participants