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

[model-controller] Investigate serving integration from cluster data #249

Closed
lampajr opened this issue Dec 18, 2023 · 5 comments · Fixed by opendatahub-io/odh-model-controller#135
Assignees
Labels

Comments

@lampajr
Copy link
Contributor

lampajr commented Dec 18, 2023

Is your feature request related to a problem? Please describe.
Followup opendatahub-io/model-registry#104
Evaluate/investigate the other direction of the aforementioned workflow:

Describe the solution you'd like
Workflow:

  1. React to ISVC create/update (is delete also required here?)
  2. Check if the ISVC has a specific label/annotation (TBD) that identifies the model version or registered model in model registry (we could have two different labels).
  3. Check if in the model registry there exist an ISVC with that name and associated to tha model/version
    a. NO: then create a new IS in model registry, then update the ISVC labels/annotation adding this new links and removing previous ones
    b. YES: TBD

3.a reconcile the data such that it won't be processed the next round but it will go through the original workflow opendatahub-io/model-registry#104

Describe alternatives you've considered
n/a

Additional context
n/a

@lampajr
Copy link
Contributor Author

lampajr commented Jan 3, 2024

New feature implementation in custom branch https://github.com/lampajr/odh-model-controller/tree/lampajr20231219_gh249_reconciler_from_isvc, waiting for a final decision on which workflow we would like to go with. After that I will finalize the selected branch, either MR --> ISVC or ISVC --> MR

@rareddy
Copy link
Contributor

rareddy commented Jan 3, 2024

IMO, we need to support both. When do we need to support both may be in question.

@lampajr
Copy link
Contributor Author

lampajr commented Jan 15, 2024

As agreed we will go with this implementation at first round, here the summarized workflow:

sequenceDiagram
    actor U as UI Dashboard
    participant K as Kubernetes
    participant MC as ODH Model Controller
    participant MR as Model Registry
    U->>+MR: Retrieve indexed model version
    MR-->>-U: Indexed model version
    U->>K: Create InferenceService (ISVC)
    Note right of U: Annotate/Label the ISVC with indexed <br/> model information, like RegisteredModel and <br/>ModelVersion IDs.
    Note right of K: Here all operators/controllers in charge to deploy<br/> the model will make<br/> their actions, e.g., KServe or ModelMesh.
    loop Every ISVC creation/deletion/update
      K-->>+MC: Send notification
      MC->>+K: Retrieve affected ISVC in the cluster
      K-->>-MC: ISVC resource
      MC->>+MR: Create/Update InferenceService in Model Registry
      Note left of MR: InferenceService records in Model Registry<br/>are used to keep track of every deployment that<br/>occurred in the monitored Kubernetes cluster.
      MR-->>-MC: InferenceService record
      MC-->>-K: Update ISVC with Model Registry record ID
    end
Loading

As highlighted here, the ODH Model Controller logic will have a more passive behavior as the InferenceService CRs are still created and managed by the UI Dashboard. The Model Controller will just sync those occurrence into the Model Registry such that we can keep track of every deployment that occurred in the cluster for indexed models.

@tarilabs
Copy link
Member

tarilabs commented Mar 7, 2024

@lampajr thank you, please be aware the webhook is no longer active back to JIRA

@lampajr
Copy link
Contributor Author

lampajr commented Mar 7, 2024

@lampajr thank you, please be aware the webhook is no longer active back to JIRA

Thanks for highlighting that the webhook is no more active 🙏
Anyway IIRC the jira ticket should be already resolved, I just missed this GH issue :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging a pull request may close this issue.

3 participants