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

How to pass the secret for model-registry csi in kserve #745

Open
Yokesh-RS opened this issue Feb 2, 2025 · 5 comments
Open

How to pass the secret for model-registry csi in kserve #745

Yokesh-RS opened this issue Feb 2, 2025 · 5 comments

Comments

@Yokesh-RS
Copy link

if the registered model has s3:///modelname

I passed model-registry:///.

I have made a secret and service account to pass in the creds , however I end up in this error

kubectl logs -n model-workloads gpt-2-modelregistry-predictor-00001-deployment-568cc645d4-zljks -c storage-initializer
Initializing, args: src_uri [model-registry://gpt-model/1] dest_path[ [/mnt/models]
Falling back to base url model-registry-service.kubeflow.svc.cluster.local:8080 for model registry service
Download model indexed in model registry: modelName=, storageUri=model-registry://gpt-model/1, modelDir=/mnt/models
Parsed storageUri=model-registry://gpt-model/1 as: modelRegistryUrl=model-registry-service.kubeflow.svc.cluster.local:8080, registeredModelName=gpt-model, versionName=0xc0008142b0
Fetching model: registeredModelName=gpt-model, versionName=0xc0008142b0
Fetching model version: model=&{0xc0008100c8 0xc0008123a0 gpt-model 0xc0008123b0 0xc000812390 0xc0008123c0 0xc0008123d0 0xc0008123e0}
Fetching model artifacts: version=&{0xc00088c070 0xc000812200 1 0xc000812230 0xc00088e1d0 3 0xc000812210 0xc00088e1e0 0xc000812220}
Error downloading the model: unable to get batch objects RequestError: send request failed
caused by: Get "http://modelserving.10.10.10.10:12345/?prefix=gpt2": dial tcp: lookup modelserving.10.10.10.10 on 19.25.25.10:53: no such host

the s3 uri is s3://modelserving/gpt2

the modelregistry uri is : model-registry://gpt-model/1

@lampajr
Copy link
Member

lampajr commented Feb 4, 2025

Hello @Yokesh-RS, thanks for trying the CSI 🚀

Overall the logs look fine (correctly fetched the model from the model registry instance) until it tries to download the actual model from the remote location retrieved from the storage URI in the ModelArtifact.

Error downloading the model: unable to get batch objects RequestError: send request failed
caused by: Get "http://modelserving.10.10.10.10:12345/?prefix=gpt2": dial tcp: lookup modelserving.10.10.10.10 on 19.25.25.10:53: no such host

Looking at your error my first guess is that the CSI cannot connect to the host where the actual model is stored. Can you check whether you can reach it from the pod?

CSI was mainly tested for public models, don't think there is any existing test that checks the behavior with private models :(. BTW under the hood the CSI simply makes use of the KServe storage API to retrieve/download the model and iirc KServe relies on some env variables to be exported and available. I would suggest to give a quick look at KServe storage API doc.

@tarilabs @Al-Pragliola anything I am missing here?

@Al-Pragliola
Copy link
Contributor

seems like the download uri should be s3://modelserving/gpt2 but instead is http://modelserving.10.10.10.10:12345/?prefix=gpt2. If I have to guess, looking at csi code:

	artifacts, _, err := p.Client.ModelRegistryServiceAPI.GetModelVersionArtifacts(context.Background(), *version.Id).
		OrderBy(openapi.ORDERBYFIELD_CREATE_TIME).
		SortOrder(openapi.SORTORDER_DESC).
		Execute()
	if err != nil {
		return err
	}

Is it possible that there is more than one artifact associated with the model version?

You can check this by sending this request to your MR service

curl 'http://$MODEL_REGISTRY_URL/api/model_registry/v1alpha3/model_versions/1/artifacts'

@Al-Pragliola
Copy link
Contributor

Al-Pragliola commented Feb 4, 2025

A nice UX improvement, as already mentioned by @tarilabs in kserve/kserve#3823
would be to be able to update the status of the inferenceservice using the storageinitializer result (among other things) instead of checking the logs of the initContainer

--

You can check the secrets passed to the initContainer at https://kserve.github.io/website/master/modelserving/storage/s3/s3/#create-s3-secret

For example

apiVersion: "serving.kserve.io/v1beta1"
kind: "InferenceService"
metadata:
  name: "iris-model"
spec:
  predictor:
    serviceAccountName: sa
    model:
      modelFormat:
        name: sklearn
      storageUri: "model-registry://iris/v1"

You just need to add the serviceAccountName which points to a service account in the same namespace as the inference service

@tarilabs
Copy link
Member

tarilabs commented Feb 4, 2025

thank you all for the discussion so far, I'm wondering whether --similarly to the other RFE-- we might need a KServe API support here, that is in addition to access Status, we can derive and supply additional fields of the Isvc which may not be known at Isvc creation time, but could be determined by the KServe Storage Initializer. For completeness, this might require another GH issue/RFE to KServe

edit: on discussion with @Al-Pragliola realizing even if the mechanism to pass storage.key would be available from API pov (via Env variables, or similar) it would not be of pragmatic help, as the secret would have needed to be mounted before the Storage Initializer is acting... so a bit of chicken-egg problem, and the SA solution highlighted earlier here by Alessio sounds the most viable to me. We can always ask some KServe SME to share more perspectives :)

@Al-Pragliola
Copy link
Contributor

Continuing the discussion with @tarilabs, he gave a nice idea about converting the existing CSI for model registry into a mutating webhook -- let's say we have an inferenceservice with a storageUri prefixed with model-registry instead create an initContainer and then inside it select which provider we want to handle after reading the metadata we can do that before creating the deployment and also check in the metadata if there is a storage metadata that we can leverage using the storage field https://github.com/kserve/kserve/tree/release-0.9/docs/samples/storage/storageSpec --
It's just a general idea, it remains to be seen if it's feasible and if it can be generalized as the concept of the ClusterStorageContainer CRD.

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

No branches or pull requests

4 participants