Support for gcs access without credentials and multi-model serving e2e test with sklearn/xgboost examples + docs #1306

abchoo · 2021-01-18T18:47:15Z

What this PR does / why we need it: Added support for gcs access without specifying credentials. Included test and support for multi-model serving e2e test with sklearn and xgboost examples. Also added a example of how to run multi model serving with public gcs bucket.

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 #

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

Release note:

None

aws-kf-ci-bot · 2021-01-18T18:47:25Z

Hi @abchoo. Thanks for your PR.

I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

google-cla · 2021-01-18T18:47:27Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla · 2021-01-18T19:01:36Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla · 2021-01-18T19:12:50Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

yuzisun · 2021-01-18T19:17:14Z

python/kfserving/kfserving/api/kf_serving_client.py

@@ -84,36 +87,41 @@ def set_credentials(self, storage_type, namespace=None, credentials_file=None,
            raise RuntimeError("Invalid storage_type: %s, only support GCS, S3 and Azure\
                currently.\n" % storage_type)

-    def create(self, inferenceservice, namespace=None, watch=False, timeout_seconds=600):  # pylint:disable=inconsistent-return-statements
+    def create(self, custom_object, namespace=None, watch=False, timeout_seconds=600, # pylint:disable=inconsistent-return-statements


Can we have a separate API with create_trained_model ?

Within the same class or a whole new class altogether?

yuzisun · 2021-01-18T19:50:58Z

cmd/agent/main.go

-		downloader.Providers[storage.GCS] = &storage.GCSProvider{
-			Client: stiface.AdaptClient(client),
-		}
+		gcs_client, err = gstorage.NewClient(ctx)


I think it is better to initialize the storage client here, when protocol is extracted we know what storage client to initialize, same applies to S3.

yuzisun · 2021-01-18T19:51:42Z

test/e2e/common/utils.py

    if protocol_version == "v2":
-        url = f"http://{cluster_ip}/v2/models/{service_name}/infer"
+        url = f"http://{cluster_ip}/v2/models/{model_name}/infer"


Great change!

yuzisun · 2021-01-18T19:52:49Z

test/e2e/predictor/test_multi_model_serving.py

+        )
+    )
+
+    service_name = "isvc-sklearn"


probably rename to isvc-sklearn-mms otherwise this may cause conflicts with the single model test.

google-cla · 2021-01-18T20:05:06Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla · 2021-01-18T23:24:45Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

google-cla · 2021-01-19T02:56:10Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

yuzisun · 2021-01-19T14:36:20Z

/ok-to-test

abchoo · 2021-01-21T22:15:23Z

/retest

yuzisun · 2021-01-22T22:38:42Z

@abchoo Awesome work!

/lgtm
/approve

k8s-ci-robot · 2021-01-22T22:39:07Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abchoo, yuzisun

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~~ [yuzisun]

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

k8s-ci-robot requested review from adriangonz and Tomcli January 18, 2021 18:47

aws-kf-ci-bot added the needs-ok-to-test label Jan 18, 2021

k8s-ci-robot added the size/L label Jan 18, 2021

yuzisun reviewed Jan 18, 2021

View reviewed changes

abchoo force-pushed the gcs-support branch from 0894b4a to a75d908 Compare January 18, 2021 20:04

k8s-ci-robot added size/XXL and removed size/L labels Jan 18, 2021

k8s-ci-robot added size/L and removed size/XXL labels Jan 18, 2021

abchoo force-pushed the gcs-support branch from 2c87cd4 to b58d1de Compare January 19, 2021 02:55

k8s-ci-robot added size/XXL and removed size/L labels Jan 19, 2021

abchoo force-pushed the gcs-support branch from b58d1de to a909c21 Compare January 19, 2021 03:16

k8s-ci-robot added size/L and removed size/XXL labels Jan 19, 2021

abchoo requested a review from yuzisun January 19, 2021 06:54

aws-kf-ci-bot added ok-to-test and removed needs-ok-to-test labels Jan 19, 2021

abchoo added 17 commits January 21, 2021 14:57

New method to create/deploy trained model object

8056ce8

Example on running multi model serving

3c39c54

Snake case variable -> Camel case

a174505

Using new version constants

c125a2f

CreateProviderIfNotExists will return provider and error

154424a

Updated to use GetProvider

9b6d50b

Corrected the file path

b736afe

Removed object file path when creating fileName

a626c22

Added overview of multi-model serving

187fb7a

Overview of inferenceservice, trainedmodel, and model agent

054c1b3

Removed check for version in create_trained_model

ad717d3

Multi-model serving example for sklearn

095e1c3

Moved provider creation to package agent storage

7349a1b

Fixed up confusing wording

c53961e

Fixed up typo

f51ca06

Included detailed diagram

8268464

Added general overview

96f65ee

abchoo force-pushed the gcs-support branch from d0f3432 to 96f65ee Compare January 21, 2021 20:12

abchoo requested review from yuzisun, yuzliu and ifilonenko January 21, 2021 23:21

k8s-ci-robot assigned yuzisun Jan 22, 2021

k8s-ci-robot added the lgtm label Jan 22, 2021

k8s-ci-robot added the approved label Jan 22, 2021

k8s-ci-robot merged commit 6d3d744 into kserve:master Jan 22, 2021

yuzisun mentioned this pull request Jan 31, 2021

Write e2e test for MMS InferenceService #1187

Closed

abchoo mentioned this pull request Feb 17, 2021

Add abchoo as Kubeflow member kubeflow/internal-acls#433

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for gcs access without credentials and multi-model serving e2e test with sklearn/xgboost examples + docs #1306

Support for gcs access without credentials and multi-model serving e2e test with sklearn/xgboost examples + docs #1306

abchoo commented Jan 18, 2021 •

edited

aws-kf-ci-bot commented Jan 18, 2021

google-cla bot commented Jan 18, 2021

google-cla bot commented Jan 18, 2021

google-cla bot commented Jan 18, 2021

yuzisun Jan 18, 2021

abchoo Jan 18, 2021

yuzisun Jan 18, 2021

yuzisun Jan 18, 2021

yuzisun Jan 18, 2021 •

edited

google-cla bot commented Jan 18, 2021

google-cla bot commented Jan 18, 2021

google-cla bot commented Jan 19, 2021

yuzisun commented Jan 19, 2021

abchoo commented Jan 21, 2021

yuzisun commented Jan 22, 2021

k8s-ci-robot commented Jan 22, 2021

Support for gcs access without credentials and multi-model serving e2e test with sklearn/xgboost examples + docs #1306

Support for gcs access without credentials and multi-model serving e2e test with sklearn/xgboost examples + docs #1306

Conversation

abchoo commented Jan 18, 2021 • edited

aws-kf-ci-bot commented Jan 18, 2021

google-cla bot commented Jan 18, 2021

google-cla bot commented Jan 18, 2021

google-cla bot commented Jan 18, 2021

yuzisun Jan 18, 2021

Choose a reason for hiding this comment

abchoo Jan 18, 2021

Choose a reason for hiding this comment

yuzisun Jan 18, 2021

Choose a reason for hiding this comment

yuzisun Jan 18, 2021

Choose a reason for hiding this comment

yuzisun Jan 18, 2021 • edited

Choose a reason for hiding this comment

google-cla bot commented Jan 18, 2021

google-cla bot commented Jan 18, 2021

google-cla bot commented Jan 19, 2021

yuzisun commented Jan 19, 2021

abchoo commented Jan 21, 2021

yuzisun commented Jan 22, 2021

k8s-ci-robot commented Jan 22, 2021

abchoo commented Jan 18, 2021 •

edited

yuzisun Jan 18, 2021 •

edited