Update kfserving manifests for v0.4.1 by pvaneck · Pull Request #1575 · kubeflow/community-distribution

pvaneck · 2020-10-01T23:19:46Z

Which issue is resolved by this Pull Request:
Resolves kserve/kserve#1015

Description of your changes:
This follows the pattern used in pipelines/upstream where upstream contains the installation manifests from the source component repo. A script is provided to sync the upstream directory (i.e. kfserving/upstream) with the source repo target location. This should make future updates easier.

Essentially, the kfserving.yaml in this patch is the same as the one in https://github.com/kubeflow/kfserving/blob/master/install/v0.4.1/kfserving.yaml. The differences are the removal of the namespace object YAML and some post processing done by the pull script to change instances of kfserving-system to kubeflow. Also the status key from the CRD definition was removed to pass unit tests. This can probably be removed upstream.

Patches are also provided to get kfserving working in the kubeflow namespace.

Checklist:

Unit tests have been rebuilt:
1. cd manifests/tests
2. make generate-changed-only
3. make test

kubeflow-bot · 2020-10-01T23:19:57Z

This change is

k8s-ci-robot · 2020-10-01T23:19:57Z

Hi @pvaneck. 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.

Details

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.

pvaneck · 2020-10-01T23:21:36Z

/assign @adrian555

PatrickXYS · 2020-10-02T00:25:15Z

After manifest V3 changes, we now consider component's base folder as read-only legacy, can we move changes to base_v3 if there's no must-do reason to change base folder?

pvaneck · 2020-10-03T00:10:00Z

Hi @PatrickXYS. Want to double check. Are you saying that I should copy the resource YAMLs into base_v3 folders and reference those? I.e.:

kfserving/kfserving-install/base_v3
kfserving/kfserving-crds/base_v3

Looks like most other components' base_v3 folders only contain a kustomization.yaml file which reference YAMLs in the base directories. Not sure if the deviation from this pattern is intended. In Kfserving's case, looks like https://github.com/kubeflow/manifests/blob/master/kfserving/installs/generic/kustomization.yaml is the V3 manifest that references files in the relevant base directories.

adrian555 · 2020-10-03T03:21:16Z

@pvaneck let's follow the pipeline/upstream model and create a kfserving/upstream directory. And update kfserving/install/generic to take resources from kfserving/upstream.

Also remember to run make generate followed by make test commands from tests directory for unit testing and commit the updated test data/file.

Thanks.

PatrickXYS · 2020-10-08T01:01:11Z

/ok-to-test

pvaneck · 2020-10-09T20:36:52Z

Tested on a Minikube and IBM Cloud Kubernetes, and appears to work fine. Ran some inferences using Pytorch and sklearn. Currently validating on Openshift.

PatrickXYS · 2020-10-09T22:50:30Z

I'm not sure if @kubeflow/kfserving-owners is aware of this, can you also take a look?

animeshsingh · 2020-10-09T23:03:34Z

Thanks @pvaneck This looks great. Thanks @adrian555 for all the help reviewing i.

@yuzisun please take a look.
@pvaneck is going to be the focal point from IBM integrating KFServing in Kubeflow ecosystem moving forward.

One of the questions we have is that should we cut a minor release of KFServing (as there are couple of fixes we need from master for OpenShift). The next release of Kubeflow is coming soon in few weeks time-frame, so wanted to get your opinion on it.

yuzisun · 2020-10-12T23:32:04Z

@animeshsingh you might want to cherry pick the openshift fixes to release-0.4 branch at this point, we can then release 0.4.1 following this doc.

yuzisun · 2020-10-12T23:39:02Z

+
+# Replace 'kfserving-system' namespace references with 'kubeflow'.
+find kfserving/upstream -name 'kfserving.yaml' -exec \
+    sed -i.bak 's/kfserving-system/kubeflow/' {} +


@pvaneck @jazzsir recently added a kubeflow overlay in this PR to directly generate the KFServing release yaml for kubeflow installation.

Oh nice. I see that you will be potentially generating a kfserving_kubeflow.yaml in the next release? If so, then this can be removed.

@yuzisun who is testing that generated kubeflow overlay? with what versions? And in context of Kubeflow?

we would still need to test and integrate kubeflow for each release, if user can not wait for kubeflow release they can use the yaml KFServing generates but they are responsible for testing out in their setup

@pvaneck for 0.4 release we still need your change to integrate kubeflow, we can clean up next time when we integrate 0.5.

yuzisun · 2020-10-12T23:39:40Z

+if [ -d kfserving/upstream ]; then
+    rm -rf kfserving/upstream
+fi
+kpt pkg get $KFSERVING_SRC_REPO/install/$KFSERVING_VERSION kfserving/upstream


Just for my knowledge, what does kpt pkg command do ?

Pretty much just syncs what is in the targeted kfserving repo directory with the local manifests directory. In this case, it just syncs whatever is in kfserving/install/v0.4.0. I am not particularly tied to this method as I was namely just following what was being done for pipelines. The idea here is that you just increment the value in the script then run it when a new version of kfserving is out, so it's more of a convenience method that can help get the latest kfserving yaml file. Ideally whatever is in the kfserving/upstream folder in this repo matches closely to what you have in the kfserving repo.

yuzisun · 2020-10-12T23:48:11Z

    matchExpressions:
    - key: control-plane
      operator: DoesNotExist
+  objectSelector:


just note that this moves the minimal kube requirement to 1.15, not exactly sure what's the min requirement for kubeflow now. It can still work with kube 1.14 but you would need the --validate=false to ignore the error.

Yea, not quite sure either as the compatibility matrix here https://www.kubeflow.org/docs/started/k8s/overview/ doesn't seem to have been updated for 1.1.

adrian555 · 2020-10-13T16:37:07Z

/lgtm

animeshsingh · 2020-10-21T00:38:34Z

@pvaneck can we move this 0.4.1?
https://github.com/kubeflow/kfserving/blob/master/install/v0.4.1/kfserving.yaml

pvaneck · 2020-10-21T01:31:40Z

@animeshsingh Updated to v0.4.1

Jeffwan · 2020-11-04T03:09:10Z

@PatrickXYS Can you check test case? Seems it's pending and not abled to be scheduled.

PatrickXYS · 2020-11-04T03:38:46Z

Let me check the error

PatrickXYS · 2020-11-04T03:39:58Z

Ah wait, the manifest PR is not yet merged....

I won't expect test passes

Jeffwan · 2020-11-04T18:04:15Z

@PatrickXYS If not, AWS CI should not add prow job for this repo, right? This will block PR merge.

PatrickXYS · 2020-11-04T18:06:51Z

@Jeffwan Let me solve this

PatrickXYS · 2020-11-04T20:52:53Z

/test kubeflow-manifests-presubmit-e2e

k8s-ci-robot · 2020-11-04T20:52:58Z

@PatrickXYS: No presubmit jobs available for kubeflow/manifests@master

Details

In response to this:

/test kubeflow-manifests-presubmit-e2e

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.

PatrickXYS · 2020-11-04T21:12:57Z

Can you rebase and run test again?

This follows the pattern used in pipelines/upstream where 'upstream' contains the installation manifests from the source component repo. A script is provided to sync the upstream directory with the source repo. Patches are provided to get kfserving working in the kubeflow namespace.

PatrickXYS · 2020-11-05T04:07:59Z

@yuzisun @pvaneck Are we good to go?

pvaneck · 2020-11-05T04:13:12Z

@PatrickXYS I believe so. /cc @animeshsingh

animeshsingh · 2020-11-05T16:22:13Z

We are good to go

/lgtm
/approve

k8s-ci-robot · 2020-11-05T16:22:20Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: animeshsingh, pvaneck

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [animeshsingh]

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

Jeffwan · 2020-11-05T18:09:27Z

Thanks everyone. I assume kfserving side has all changes for 1.2.

animeshsingh · 2020-11-05T22:50:02Z

@Jeffwan we are experimenting with KNative 0.14.3 to see if we can position it as a minimum req - if we get it in time, that's the only change you are looking at. What's the final de cut date for you?

Jeffwan · 2020-11-06T02:50:35Z

@animeshsingh end of this week. cherry-pick is acceptable until Nov 13.

Will be responsible for maintaining manifests for KFServing, OpenShift, IKS etc. PRs merged: kubeflow#1515 kubeflow#1575 kubeflow#1580 kubeflow#1617 Reviews: kubeflow#1582 kubeflow#1621 kubeflow#1625 kubeflow#1627 kubeflow#1628 kubeflow/kfctl#419

* Add Paul as a reviewer Will be responsible for maintaining manifests for KFServing, OpenShift, IKS etc. PRs merged: #1515 #1575 #1580 #1617 Reviews: #1582 #1621 #1625 #1627 #1628 kubeflow/kfctl#419 * Update OWNERS

k8s-ci-robot requested review from PatrickXYS and krishnadurai October 1, 2020 23:19

k8s-ci-robot added the needs-ok-to-test label Oct 1, 2020

google-cla Bot added the cla: yes label Oct 1, 2020

k8s-ci-robot added the size/XXL label Oct 1, 2020

k8s-ci-robot assigned adrian555 Oct 1, 2020

pvaneck force-pushed the update-kfserving branch from c940793 to 4cf3ceb Compare October 7, 2020 19:16

k8s-ci-robot added ok-to-test and removed needs-ok-to-test labels Oct 8, 2020

yuzisun reviewed Oct 12, 2020

View reviewed changes

k8s-ci-robot added the lgtm label Oct 13, 2020

k8s-ci-robot removed the lgtm label Oct 21, 2020

pvaneck force-pushed the update-kfserving branch from a4ab6f1 to bc5dd1c Compare October 21, 2020 01:00

pvaneck changed the title ~~Update kfserving manifests for v0.4.0~~ Update kfserving manifests for v0.4.1 Oct 21, 2020

Jeffwan mentioned this pull request Nov 4, 2020

Kubeflow v1.2 release kubeflow/kubeflow#5371

Closed

k8s-ci-robot removed the lgtm label Nov 4, 2020

pvaneck force-pushed the update-kfserving branch from 2184cab to 2358e7d Compare November 4, 2020 02:58

pvaneck added 4 commits November 4, 2020 13:15

Generate tests

4b97d38

Update to use 0.4.1

c6cd41c

Add updated 0.4.1 yaml

f749e0f

pvaneck force-pushed the update-kfserving branch from 2358e7d to f749e0f Compare November 4, 2020 21:16

k8s-ci-robot added the lgtm label Nov 5, 2020

k8s-ci-robot added the approved label Nov 5, 2020

k8s-ci-robot merged commit 4b9d01c into kubeflow:master Nov 5, 2020

animeshsingh mentioned this pull request Nov 11, 2020

Add Paul as a reviewer #1632

Merged

yuzisun mentioned this pull request Dec 3, 2020

sklearn-iris InferenceService fails to reach ready stage in KF 1.2: "waiting for loadbalancer" kserve/kserve#1234

Closed

Conversation

pvaneck commented Oct 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kubeflow-bot commented Oct 1, 2020

Uh oh!

k8s-ci-robot commented Oct 1, 2020

Uh oh!

pvaneck commented Oct 1, 2020

Uh oh!

PatrickXYS commented Oct 2, 2020

Uh oh!

pvaneck commented Oct 3, 2020

Uh oh!

adrian555 commented Oct 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PatrickXYS commented Oct 8, 2020

Uh oh!

pvaneck commented Oct 9, 2020

Uh oh!

PatrickXYS commented Oct 9, 2020

Uh oh!

animeshsingh commented Oct 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yuzisun commented Oct 12, 2020

Uh oh!

yuzisun Oct 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pvaneck Oct 13, 2020

Choose a reason for hiding this comment

Uh oh!

animeshsingh Oct 13, 2020

Choose a reason for hiding this comment

Uh oh!

yuzisun Oct 13, 2020

Choose a reason for hiding this comment

Uh oh!

yuzisun Oct 13, 2020

Choose a reason for hiding this comment

Uh oh!

yuzisun Oct 12, 2020

Choose a reason for hiding this comment

Uh oh!

pvaneck Oct 13, 2020

Choose a reason for hiding this comment

Uh oh!

yuzisun Oct 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pvaneck Oct 13, 2020

Choose a reason for hiding this comment

Uh oh!

adrian555 commented Oct 13, 2020

Uh oh!

animeshsingh commented Oct 21, 2020

Uh oh!

pvaneck commented Oct 21, 2020

Uh oh!

Jeffwan commented Nov 4, 2020

Uh oh!

PatrickXYS commented Nov 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PatrickXYS commented Nov 4, 2020

Uh oh!

Jeffwan commented Nov 4, 2020

Uh oh!

PatrickXYS commented Nov 4, 2020

Uh oh!

PatrickXYS commented Nov 4, 2020

Uh oh!

k8s-ci-robot commented Nov 4, 2020

Uh oh!

PatrickXYS commented Nov 4, 2020

Uh oh!

pvaneck commented Oct 1, 2020 •

edited

Loading

adrian555 commented Oct 3, 2020 •

edited

Loading

animeshsingh commented Oct 9, 2020 •

edited

Loading

yuzisun Oct 12, 2020 •

edited

Loading

yuzisun Oct 12, 2020 •

edited

Loading

PatrickXYS commented Nov 4, 2020 •

edited

Loading