-
Notifications
You must be signed in to change notification settings - Fork 977
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
Initial commit for OCI dist spec v1.1.0 agent support #3539
base: master
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: rchincha The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
a93c6f4
to
99fab15
Compare
Cross-posting here ... https://kccnceu2024.sched.com/event/1YeLi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you update the PR description?
@lizzzcai Thanks for pointing out #3043 - lots of relevant discussion there. I am still learning about kserve so hazarding a possiblly incorrect guess. This PR should give the ability to serve models/artifacts from a OCI registry - OCI dist-spec v1.1.0 supports arbitrary artifacts. An example is perhaps this: https://kserve.github.io/website/0.7/modelserving/storage/s3/s3/ There is still a kubeflow workflow to push artifacts into a OCI registry to consider. |
Done, pls take a look. |
pkg/agent/storage/provider.go
Outdated
@@ -29,9 +29,11 @@ const ( | |||
// File Protocol = "file://" | |||
HTTPS Protocol = "https://" | |||
HTTP Protocol = "http://" | |||
// OCI dist spec v1.1.0 | |||
OCI Protocol = "oci://" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modelcars also use oci://
as a schema prefix, so that might be conflicting here. Modelcars in fact, are conventional OCI images as they have also an active component for keeping a container alive (notably sleep
and also a ln -s
for creating the symbolic link in the emptyDir volume to point to the model data).
I'm not sure how the PR proposed here infers with the logic of the modelcar as defined in the storage initializer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The images here are going to be stored from a OCI artifact registry, so indeed they are also oci images. The question then is the transport schema. Maybe "oci-registry://" similar to "model-registry://"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rchincha sorry, I'm not deep in this PR, but how do you plan to make the modeldata in the OCI image accessible to the runtime ? For me it looks like that the existing modelcar feature is already quite similar what you are trying to achieve with this PR. It would be cool, if we could merge your work with that.
If not, what would be the difference to https://kserve.github.io/website/latest/modelserving/storage/oci/ ? (just for my understanding)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rhuss indeed, the "effect" is the same, except the modelcar approach needs a running container whereas this PR is downloading model data and purely a storage approach. Also my slow progress on this PR as I am trying to understand more. No question that the modelcar approach although one can argue is a "hack", solves lots of pertinent issues and very effective.
Just FYI that there are some discussions happening in the opencontainers (OCI) side about this use case - pulling large things and making them available to container workloads.
Partially addresses kubeflow/community#682 OCI image and distribution specs v1.1.0 has added support for pushing and pulling arbitrary artifacts to a conformant registry, and not just container images. Since a registry is already needed to deploy inference workloads as containers, and that it would be desirable to avoid another piece of infrastructure just to store inference data, a OCI conformant registry could become that ideal store to combine and colocate both use cases. This plugin adds that support. Uses the oras-go library. References: https://opencontainers.org/posts/blog/2024-03-13-image-and-distribution-1-1/ Signed-off-by: Ramkumar Chinchani <rchincha@cisco.com>
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #
kubeflow/community#682
Type of changes
Please delete options that are not relevant.
Feature/Issue validation/testing:
Please describe the tests that you ran to verify your changes and relevant result summary. Provide instructions so it can be reproduced.
Please also list any relevant details for your test configuration.
Deploy a conformant registry such as (
zot
), push model data and then deploy a workload that consumes the data.Logs
Special notes for your reviewer:
Checklist:
Release note: