Ability to Pin model version with S3 `versionId`

[andreaTP]

Hi and thanks for the project!

I'm trying to pin the versionId of a model using the InferenceService CR. The use case is to have stable and reproducible deployments even if the model in the S3 bucket gets updated.

I cannot find a relevant field in the CR, and trying to append ?versionId=<.....> to the available fields always lead to a non-functional setup. Is it possible to achieve this result?

[ckadner]

A model (and it's version) is uniquely identified by the storage key and path. So if you replace the model in the same bucket, same prefix, same filename, then ModelMesh will treat it as the same version of the same model. So a new model version would need a new model file path. Once the ISVC is updated there should be a new revision.

@njhill -- do we have any good documentation on how to update an existing model with a new version and how to denote the version in the metadata? The KServe inference protocol v2 has model_name and model_version and APIs to list/create/update/delete versions but I am not sure how it all ties together.

[andreaTP]

Hi @ckadner thanks a lot for getting back, much appreciated! Let me go through your comment.

A model (and it's version) is uniquely identified by the storage key and path. So if you replace the model in the same bucket, same prefix, same filename, then ModelMesh will treat it as the same version of the same model.

Is this a deliberate design decision? Seems to be a pretty opinionated decision as it strongly limits leveraging basic S3 capabilities and imposes extra care in the management of the S3 storage to prevent irreproducible deployments. Would it be possible to read a little more about the rationale behind this decision?

a new model version would need a new model file path.

That's fine by me, as soon as I can express an immutable identifier for the S3 object in the bucket, but here I'm failing 🙁

[tjohnson31415]

I think there may be a little confusion. @andreaTP what you are referring to is versioning of objects with a versionId which is a particular feature of S3 implementations supports, right? (Amazon S3 documentation here)

Also ModelMesh serving mentions "S3" support, it is using that in the generic sense of services that implement the S3 API (i.e. inclusive of MinIO, IBM Cloud Object Storage). Use of versionId is new to me, but maybe support for it is already standard as part of anything that implements the "S3 API" (a quick search shows that MinIO and IBM COS do support versioning).

Is this a deliberate design decision?

Assumed immutability of what a ISVC points to is a deliberate design decision. From the ModelMesh perspective, it does not have any explicit knowledge on the content of the S3 path, all it has is the path as the identifier. It also manages model loads/unloads dynamically and routes to all instances of the model equally. The assumption that the model referenced by a path is immutable makes the dynamic management of the models much simpler. I think that's the main reason for the design: it is simpler than needing a way to monitor and react to changes to objects in S3.

Not supporting versionId is not part of that design decision. I think we can add support for versionId. I would think that this would just require some changes to the S3 storage provider implementation: https://github.com/kserve/modelmesh-runtime-adapter/tree/main/pullman/storageproviders/s3

[andreaTP]

Hi @tjohnson31415 and thanks for reverting back!

I'm referring to versionId as the S3 feature, you are correct.

Tl;DR: To enable basic reproducible deployments we need a way to let the user specify additional information about the content of the S3 object (etag , versionId) or, at least, verify the content against a user-provided hash (sha, md5).

The longer explanation:

Assumed immutability of what a ISVC points to is a deliberate design decision.

This is interesting, but I want to remark on the fact that, with the current implementation, this is a very opinionated design decision, and should be, at least, correctly communicated in docs and fields' descriptions in the CRDs. S3 is a mutable storage, doing assumptions over the content of an S3 object without options to specify the etag or the versionId is going to result in very confusing runtime behavior.

The assumption that the model is immutable makes the dynamic management of the models much simpler.

This is completely fair and understandable, as long as we can consider adding the necessary fields to enforce and guarantee the immutability of the models.

I think that's the main reason for the design: it is simpler than needing a way to monitor and react to changes to objects in S3.

Again this is perfectly fine, but the current mechanism has to be improved to effectively guarantee reproducible models' deployments.

Currently, we cannot guarantee that 2 subsequent model deployments are going to serve the same model without locking to "read-only" the S3 bucket.

[njhill]

I agree with @tjohnson31415 that this seems like it would be a good thing to add, and should hopefully be a trivial change to the s3 storage provider, to recognize/use an additional optional versionId attribute within the storage spec.