tf-serving support model on NFS #688

Jyonn · 2018-04-19T20:39:30Z

As for now, tf-serving can only support model located on cloud. I changed tf-serving.libsonnet file, add 2 more params: modelStorageType and nfsPVC. For now, modelStorageType can only detect if it's nfs. If so, container will mount a persistentVolumeClaim(PVC) specified by nfcPVC. We can support more choices in the future.
A guideline for running nfs-based model is put on components/k8s-model-server folder.
As a new param is added, I changed user_guide.md, Serve a model using Tensorflow Serving section, and add ks param set ${MODEL_COMPONENT} modelLocate ${MODEL_STORAGE_TYPE}. Also give a quick guideline of Create a component for your model located on nfs following Create a component for your model located on cloud.

This change is

googlebot · 2018-04-19T20:39:33Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

Jyonn · 2018-04-19T20:41:15Z

/assign @jlewi

jlewi · 2018-04-20T06:43:44Z

/unassign jlewi
/assign @lluunn

elsonrodriguez · 2018-04-20T21:00:59Z

components/k8s-model-server/Serve a local model using Tensorflow Serving.md

+$ NFS_PVC_NAME=nfs
+$ ks generate tf-serving ${MODEL_COMPONENT} --name=${MODEL_NAME}
+$ ks param set ${MODEL_COMPONENT} modelPath ${MODEL_PATH}
+$ ks param set ${MODEL_COMPONENT} modelLocate ${MODEL_LOCATE}


modelLocate may make more sense as modelStorageType

Thanks! I've changed modelLocate to modelStorageType

lluunn · 2018-04-23T17:48:27Z

CLA not working now.
/ok-to-test

lluunn · 2018-04-23T17:48:50Z

user_guide.md


 ```
 MODEL_COMPONENT=serveInception
 MODEL_NAME=inception
 MODEL_PATH=gs://kubeflow-models/inception
+MODEL_STORAGE_TYPE=cloud


this is not required right?

yes, now i delete this line and detect if 'modelStorageType' is an attribute of $.params, so that when model is in gs, the user don't need to specify MODEL_STORAGE_TYPE=cloud

Jyonn · 2018-04-24T22:58:23Z

I signed it!

googlebot · 2018-04-24T22:58:26Z

CLAs look good, thanks!

jose5918 · 2018-04-24T23:32:15Z

kubeflow/tf-serving/tf-serving.libsonnet

@@ -147,6 +147,13 @@
        runAsUser: 1000,
        fsGroup: 1000,
      },
+      volumeMounts+: if 'modelStorageType' om $.params then


I think it's the om here

yes! thanks for telling!

lluunn · 2018-04-25T19:31:03Z

kubeflow/tf-serving/tf-serving.libsonnet

@@ -147,6 +147,13 @@
        runAsUser: 1000,
        fsGroup: 1000,
      },
+      volumeMounts+: if 'modelStorageType' in $.params then


do you need this "if 'modelStorageType' in $.params then"? It seems redundant?
I think you can add modelStorageType into param and default to cloud or empty string.

lluunn · 2018-04-25T19:31:20Z

kubeflow/tf-serving/tf-serving.libsonnet

@@ -217,6 +224,16 @@
              if $.util.toBool($.params.deployHttpProxy) then
                $.parts.httpProxyContainer,
            ],
+            volumes+: if 'modelStorageType' in $.params then 


Reasonable. Yesterday I tried to make modelStorageType as a param for ks generate tf-serving but failed (so far). So I add storageType into params. When I use modelStorageType as a param, it shows some error (maybe meets some conflicts).

I was thinking just use a param, modelStorageType = cloud by default.
What's the error?

If using modelStorageType like this:

params:: { ... ...(other params) modelName: $.params.name, modelPath: null, modelStorageType: if "modelStorageType" in $.params then $.params.modelStorageType else "cloud",

It will shows an error:

ERROR generate objects for namespace : unable to read /home/ciscoai/my-kubeflow/environments/default/main.jsonnet: RUNTIME ERROR: Max stack frames exceeded. ------------------------------------------------- /home/ciscoai/my-kubeflow/components/serveInception.jsonnet:(15:12)-(17:4) object <anonymous> params+: updatedParams { name: name, }, ------------------------------------------------- <builtin> builtin function <operator+> ------------------------------------------------- <std>:966:25-26 thunk from <function <anonymous>> std.objectHasEx(o, f, true), ------------------------------------------------- <builtin> builtin function <objectHasEx> ------------------------------------------------- ------------------------------------------------- /home/ciscoai/my-kubeflow/vendor/kubeflow/tf-serving/tf-serving.libsonnet:14:7-32 object <anonymous> $.params.modelStorageType ------------------------------------------------- /home/ciscoai/my-kubeflow/vendor/kubeflow/tf-serving/tf-serving.libsonnet:14:7-32 object <anonymous> $.params.modelStorageType ------------------------------------------------- /home/ciscoai/my-kubeflow/vendor/kubeflow/tf-serving/tf-serving.libsonnet:14:7-32 object <anonymous> $.params.modelStorageType ------------------------------------------------- /home/ciscoai/my-kubeflow/vendor/kubeflow/tf-serving/tf-serving.libsonnet:14:7-32 object <anonymous> $.params.modelStorageType ------------------------------------------------- /home/ciscoai/my-kubeflow/vendor/kubeflow/tf-serving/tf-serving.libsonnet:14:7-32 object <anonymous> $.params.modelStorageType ------------------------------------------------- ... (skipped 481 frames) ------------------------------------------------- <std>:971:33-35 thunk from <function <anonymous>> if !std.primitiveEquals(ta, tb) then ------------------------------------------------- <builtin> builtin function <primitiveEquals> ------------------------------------------------- <std>:971:12-40 function <anonymous> if !std.primitiveEquals(ta, tb) then ------------------------------------------------- ------------------------------------------------- <std>:1025:45-71 function <anonymous> for x in std.objectFields(a) if isContent(std.prune(a[x])) ------------------------------------------------- <builtin> builtin function <flatMap> ------------------------------------------------- <builtin> builtin function <$objectFlatMerge> ------------------------------------------------- /home/ciscoai/my-kubeflow/components/serveInception.jsonnet:20:1-52 $ std.prune(k.core.v1.list.new(tfServing.components)) ------------------------------------------------- <extvar:__ksonnet/components>:2:19-86 object <anonymous> serveInception: import "/home/ciscoai/my-kubeflow/components/serveInception.jsonnet", ------------------------------------------------- During manifestation

When I replace modelStorageType to storageType(or other identifier):

storageType: if "modelStorageType" in $.params then $.params.modelStorageType else "cloud",

It runs well.

I mean, just doing

params:: {
modelStorageType: "cloud",
}

It will get overridden if you pass the param modelStorageType

oh ok! thanks for telling!

lluunn · 2018-04-27T19:13:57Z

Thanks
/lgtm

pdmack · 2018-05-02T21:45:57Z

/approve

k8s-ci-robot · 2018-05-02T21:46:01Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pdmack

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [pdmack]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

* tf-serving support model on NFS * modelLocate -> modelStorageType * make default modelStorageType 'cloud' * mistake om -> in * remove redundant if clause * make 'cloud' as default value of modelStorageType

tf-serving support model on NFS

6f344d0

k8s-ci-robot requested review from jimexist and willingc April 19, 2018 20:39

k8s-ci-robot added needs-ok-to-test size/L labels Apr 19, 2018

k8s-ci-robot assigned jlewi Apr 19, 2018

k8s-ci-robot assigned lluunn and unassigned jlewi Apr 20, 2018

elsonrodriguez reviewed Apr 20, 2018

View reviewed changes

modelLocate -> modelStorageType

8f384cb

k8s-ci-robot removed the needs-ok-to-test label Apr 23, 2018

lluunn reviewed Apr 23, 2018

View reviewed changes

make default modelStorageType 'cloud'

61ce6b4

jose5918 reviewed Apr 24, 2018

View reviewed changes

mistake om -> in

a4ecab0

lluunn reviewed Apr 25, 2018

View reviewed changes

This was referenced Apr 25, 2018

How to detect if import stuff is exist? google/jsonnet#506

Closed

Detect if param is sent from cmd (import "param://xxx") ksonnet/ksonnet#489

Closed

adelliu added 2 commits April 26, 2018 10:52

remove redundant if clause

8602b6b

make 'cloud' as default value of modelStorageType

0778285

k8s-ci-robot added the lgtm label Apr 27, 2018

k8s-ci-robot added the approved label May 2, 2018

k8s-ci-robot merged commit 3920d14 into kubeflow:master May 2, 2018

jlewi mentioned this pull request Jun 28, 2018

TFJobs UI doesn't work behind IAP; React APP needs support IAP? #574

Closed

yanniszark pushed a commit to arrikto/kubeflow that referenced this pull request Feb 15, 2021

Add example readme for v1alpha2 (kubeflow#688)

f80a7d2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tf-serving support model on NFS #688

tf-serving support model on NFS #688

Jyonn commented Apr 19, 2018 •

edited

googlebot commented Apr 19, 2018

Jyonn commented Apr 19, 2018

jlewi commented Apr 20, 2018

elsonrodriguez Apr 20, 2018

Jyonn Apr 20, 2018

lluunn commented Apr 23, 2018

lluunn Apr 23, 2018

Jyonn Apr 23, 2018

Jyonn commented Apr 24, 2018

googlebot commented Apr 24, 2018

jose5918 Apr 24, 2018

Jyonn Apr 24, 2018

lluunn Apr 25, 2018

lluunn Apr 25, 2018

Jyonn Apr 26, 2018

lluunn Apr 27, 2018

Jyonn Apr 27, 2018

lluunn Apr 27, 2018

Jyonn Apr 27, 2018

lluunn commented Apr 27, 2018

pdmack commented May 2, 2018

k8s-ci-robot commented May 2, 2018

tf-serving support model on NFS #688

tf-serving support model on NFS #688

Conversation

Jyonn commented Apr 19, 2018 • edited

googlebot commented Apr 19, 2018

What to do if you already signed the CLA

Individual signers

Corporate signers

Jyonn commented Apr 19, 2018

jlewi commented Apr 20, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lluunn commented Apr 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jyonn commented Apr 24, 2018

googlebot commented Apr 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lluunn commented Apr 27, 2018

pdmack commented May 2, 2018

k8s-ci-robot commented May 2, 2018

Jyonn commented Apr 19, 2018 •

edited