Skip to content

Commit 4a47d56

Browse files
fix: set non-empty modelPath for http storageURI (#382)
#### Motivation When using the `storageURI` form of an HTTP (non Azure blob) address to download a model, the `modelPath` needs to be an non-empty string. Before this change, the `storageURI: http://models.r.us/my-model.json` form would be equivalent to the following `storage` spec: ``` storage: type: http path: '' # this being empty is problematic for later processing parameters: url: http://models.r.us/path/to/my-model.json ``` The http storage type is currently the only way to have a valid storage configuration with an empty `path` (mainly because it has a "url" parameter that could include the full path). That said, I'm not sure if we should make a `path` required for the HTTP storage type. In particular, if the `url` is just `http://models.r.us/`, there is no path portion. Related: kserve/modelmesh-runtime-adapter#41 (comment) #### Modifications Set `modelPath` to the URL's Path and set the `url` parameter to not have the URL Path. #### Result With these changes, the `storageURI` example above changes to have a `path` field: ``` storage: type: http path: path/to/my-model.json parameters: url: http://models.r.us/ ``` Signed-off-by: Travis Johnson <[email protected]> Signed-off-by: Christian Kadner <[email protected]> Co-authored-by: Christian Kadner <[email protected]>
1 parent 4808804 commit 4a47d56

File tree

6 files changed

+53
-5
lines changed

6 files changed

+53
-5
lines changed

fvt/storage/storage_suite_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ var _ = SynchronizedBeforeSuite(func() []byte {
5858

5959
// ensure a stable deploy state, on each process since we updated the storage config
6060
Log.Info("Waiting for stable deploy state")
61-
WaitForStableActiveDeployState(time.Second * 60)
61+
WaitForStableActiveDeployState(time.Second * 120)
6262

6363
return nil
6464
}, func(_ []byte) {

fvt/storage/storage_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,17 @@ import (
2424
)
2525

2626
var isvcFiles = map[string]string{
27+
"isvc-https": "isvc-https.yaml",
2728
"isvc-pvc-storage-uri": "isvc-pvc-uri.yaml",
2829
"isvc-pvc-storage-path": "isvc-pvc-path.yaml",
2930
"isvc-pvc2": "isvc-pvc-2.yaml",
3031
"isvc-pvc3": "isvc-pvc-3.yaml",
3132
"isvc-pvc4": "isvc-pvc-4.yaml",
3233
}
3334

35+
// ISVC using a model from GitHub via HTTPS URI
36+
var isvcViaHttps = "isvc-https"
37+
3438
// ISVCs using PVCs from the FVT `storage-config` Secret (config/dependencies/fvt.yaml)
3539
var isvcWithPvcInStorageConfig = []string{"isvc-pvc-storage-uri", "isvc-pvc-storage-path", "isvc-pvc2"}
3640

@@ -43,6 +47,30 @@ var isvcWithNonExistentPvc = "isvc-pvc4"
4347

4448
var _ = Describe("ISVCs", func() {
4549

50+
Describe("with HTTPS storage URI", Ordered, func() {
51+
var isvcName string
52+
var fileName string
53+
54+
BeforeAll(func() {
55+
isvcName = isvcViaHttps
56+
fileName = isvcFiles[isvcName]
57+
})
58+
59+
It("should successfully load a model", func() {
60+
isvcObject := NewIsvcForFVT(fileName)
61+
isvcName = isvcObject.GetName()
62+
CreateIsvcAndWaitAndExpectReady(isvcObject, PredictorTimeout)
63+
})
64+
65+
It("should successfully run inference", func() {
66+
ExpectSuccessfulInference_sklearnMnistSvm(isvcName)
67+
})
68+
69+
AfterAll(func() {
70+
FVTClientInstance.DeleteIsvc(isvcName)
71+
})
72+
})
73+
4674
Describe("with PVC in storage-config", Ordered, func() {
4775

4876
for _, name := range isvcWithPvcInStorageConfig {

fvt/testdata/isvcs/isvc-https.yaml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
apiVersion: serving.kserve.io/v1beta1
2+
kind: InferenceService
3+
metadata:
4+
name: isvc-https
5+
annotations:
6+
serving.kserve.io/deploymentMode: ModelMesh
7+
spec:
8+
predictor:
9+
model:
10+
modelFormat:
11+
name: sklearn
12+
storageUri: "https://github.com/kserve/modelmesh-minio-examples/blob/main/sklearn/mnist-svm.joblib?raw=true"

go.mod

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ require (
1111
github.com/manifestival/controller-runtime-client v0.4.0
1212
github.com/manifestival/manifestival v0.7.1
1313
github.com/moverest/mnist v0.0.0-20160628192128-ec5d9d203b59
14-
github.com/onsi/ginkgo/v2 v2.9.2
15-
github.com/onsi/gomega v1.27.6
14+
github.com/onsi/ginkgo/v2 v2.9.7
15+
github.com/onsi/gomega v1.27.7
1616
github.com/operator-framework/operator-lib v0.10.0
1717
github.com/pkg/errors v0.9.1
1818
github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring v0.55.0
@@ -105,7 +105,7 @@ require (
105105
golang.org/x/term v0.8.0 // indirect
106106
golang.org/x/text v0.9.0 // indirect
107107
golang.org/x/time v0.3.0 // indirect
108-
golang.org/x/tools v0.8.0 // indirect
108+
golang.org/x/tools v0.9.1 // indirect
109109
golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect
110110
gomodules.xyz/jsonpatch/v2 v2.2.0 // indirect
111111
google.golang.org/api v0.122.0 // indirect

go.sum

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,8 @@ github.com/onsi/ginkgo/v2 v2.0.0/go.mod h1:vw5CSIxN1JObi/U8gcbwft7ZxR2dgaR70JSE3
480480
github.com/onsi/ginkgo/v2 v2.1.3/go.mod h1:vw5CSIxN1JObi/U8gcbwft7ZxR2dgaR70JSE3/PpL4c=
481481
github.com/onsi/ginkgo/v2 v2.9.2 h1:BA2GMJOtfGAfagzYtrAlufIP0lq6QERkFmHLMLPwFSU=
482482
github.com/onsi/ginkgo/v2 v2.9.2/go.mod h1:WHcJJG2dIlcCqVfBAwUCrJxSPFb6v4azBwgxeMeDuts=
483+
github.com/onsi/ginkgo/v2 v2.9.7 h1:06xGQy5www2oN160RtEZoTvnP2sPhEfePYmCDc2szss=
484+
github.com/onsi/ginkgo/v2 v2.9.7/go.mod h1:cxrmXWykAwTwhQsJOPfdIDiJ+l2RYq7U8hFU+M/1uw0=
483485
github.com/onsi/gomega v0.0.0-20170829124025-dcabb60a477c/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA=
484486
github.com/onsi/gomega v0.0.0-20190113212917-5533ce8a0da3/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY=
485487
github.com/onsi/gomega v1.7.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY=
@@ -490,6 +492,8 @@ github.com/onsi/gomega v1.17.0/go.mod h1:HnhC7FXeEQY45zxNK3PPoIUhzk/80Xly9PcubAl
490492
github.com/onsi/gomega v1.18.1/go.mod h1:0q+aL8jAiMXy9hbwj2mr5GziHiwhAIQpFmmtT5hitRs=
491493
github.com/onsi/gomega v1.27.6 h1:ENqfyGeS5AX/rlXDd/ETokDz93u0YufY1Pgxuy/PvWE=
492494
github.com/onsi/gomega v1.27.6/go.mod h1:PIQNjfQwkP3aQAH7lf7j87O/5FiNr+ZR8+ipb+qQlhg=
495+
github.com/onsi/gomega v1.27.7 h1:fVih9JD6ogIiHUN6ePK7HJidyEDpWGVB5mzM7cWNXoU=
496+
github.com/onsi/gomega v1.27.7/go.mod h1:1p8OOlwo2iUUDsHnOrjE5UKYJ+e3W8eQ3qSlRahPmr4=
493497
github.com/opencontainers/go-digest v1.0.0 h1:apOUWs51W5PlhuyGyz9FCeeBIOUDA/6nW8Oi/yOhh5U=
494498
github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3IKzErnv2BNG4W4MAM=
495499
github.com/operator-framework/operator-lib v0.10.0 h1:tTjrt8Udi0msABkMpgxKHp7sXKnC73jFPO5Col0tWso=
@@ -903,6 +907,8 @@ golang.org/x/tools v0.1.5/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk=
903907
golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc=
904908
golang.org/x/tools v0.8.0 h1:vSDcovVPld282ceKgDimkRSC8kpaH1dgyc9UMzlt84Y=
905909
golang.org/x/tools v0.8.0/go.mod h1:JxBZ99ISMI5ViVkT1tr6tdNmXeTrcpVSD3vZ1RsRdN4=
910+
golang.org/x/tools v0.9.1 h1:8WMNJAz3zrtPmnYC7ISf5dEn3MT0gY7jBJfw27yrrLo=
911+
golang.org/x/tools v0.9.1/go.mod h1:owI94Op576fPu3cIGQeHs3joujW/2Oc6MtlxbF5dfNc=
906912
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
907913
golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
908914
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=

pkg/predictor_source/inferenceservice_registry.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,8 +188,10 @@ func processInferenceServiceStorage(inferenceService *v1beta1.InferenceService,
188188
uriParameters["account_name"] = hostParts[0]
189189
modelPath = strings.Join(pathParts[1:], "/")
190190
} else {
191+
modelPath = strings.TrimPrefix(u.Path, "/")
192+
u.Path = ""
191193
uriParameters["type"] = "http"
192-
uriParameters["url"] = *storageUri
194+
uriParameters["url"] = u.String()
193195
}
194196
default:
195197
err = fmt.Errorf("the InferenceService %v has an unsupported storageUri scheme %v", nname, u.Scheme)

0 commit comments

Comments
 (0)