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

Make note of knative-eventing being optional in README #1826

Merged
merged 1 commit into from
Apr 12, 2021

Conversation

pvaneck
Copy link
Member

@pvaneck pvaneck commented Apr 9, 2021

Only Knative Serving is required for KFServing to work. Knative Eventing is used for specific logger/metrics tracking use cases.

@yanniszark
Copy link
Contributor

Thanks a lot @pvaneck. I have some questions about the PR.

First of all, since the example kustomization is about installing all Kubeflow components and common services, should we leave eventing inside?

Then, we can use the detailed component-by-component guide to describe its optional nature. In the docs, instead of commenting out the optional part, I would suggest making them two individual code blocks. Something like the following:

Install Knative Serving:

kustomize build --load_restrictor=none common/knative/knative-serving-crds/base | kubectl apply -f -
kustomize build --load_restrictor=none common/knative/knative-serving-install/base | kubectl apply -f -
kustomize build --load_restrictor=none common/istio-1-9-0/cluster-local-gateway/base | kubectl apply -f -

Optionally, you can install Knative Eventing which is needed for inference request logging:

kustomize build --load_restrictor=none common/knative/knative-eventing-crds/base | kubectl apply -f -
kustomize build --load_restrictor=none common/knative/knative-eventing-install/base | kubectl apply -f -

What do you think?

@pvaneck
Copy link
Member Author

pvaneck commented Apr 9, 2021

@yanniszark That sound reasonable. I will follow your suggestion about the code blocks and will leave eventing inside the example reference deployment.

Only Knative Serving is required for KFServing to work.
Knative Eventing is used for specific logger/metrics tracking
use cases, so a note is made in the README about eventing
being optional.
@pvaneck pvaneck force-pushed the eventing-default branch from 133aa6a to 323911a Compare April 9, 2021 20:57
@pvaneck pvaneck changed the title Make knative-eventing optional Make note of knative-eventing being optional in README Apr 9, 2021
@yanniszark
Copy link
Contributor

/lgtm
/approve

@google-oss-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

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

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:

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

@google-oss-robot google-oss-robot merged commit 5cae069 into kubeflow:master Apr 12, 2021
yanniszark pushed a commit to arrikto/kubeflow-manifests that referenced this pull request Apr 16, 2021
Only Knative Serving is required for KFServing to work.
Knative Eventing is used for specific logger/metrics tracking
use cases, so a note is made in the README about eventing
being optional.
google-oss-robot pushed a commit that referenced this pull request Apr 17, 2021
* chore: remove argo manifests from contrib (#1813)

* Make note of knative-eventing being optional in README  (#1826)

Only Knative Serving is required for KFServing to work.
Knative Eventing is used for specific logger/metrics tracking
use cases, so a note is made in the README about eventing
being optional.

* README: Expand section for connecting to a Kubeflow cluster (#1841)

The current section for connecting to a Kubeflow cluster only covers
connecting with port-forward. Expand the section with instructions for
NodePort / LoadBalancer / Ingress.

Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com>

* Sync Katib manifests with Kustomize v4 support (#1840)

* Kubeflow Tekton Pipelines: Sync manifests (#1843)

Sync manifests for application "Kubeflow Tekton Pipelines".
Upstream manifests are copied from:
- Repo: https://github.com/kubeflow/kfp-tekton
- Path: manifests/kustomize
- Revision: v0.8.0-rc0

Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com>

* Sync wg-notebooks apps to revision 1.3.0-rc.1 (#1846)

* Notebook Controller: Sync manifests

Sync manifests for application "Notebook Controller".
Upstream manifests are copied from:
- Repo: https://github.com/kubeflow/kubeflow
- Path: components/notebook-controller/config
- Revision: 8901e3af3b83f599b89562162cdb2854f57082d3

Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com>

* Tensorboard Controller: Sync manifests

Sync manifests for application "Tensorboard Controller".
Upstream manifests are copied from:
- Repo: https://github.com/kubeflow/kubeflow
- Path: components/tensorboard-controller/config
- Revision: 8901e3af3b83f599b89562162cdb2854f57082d3

Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com>

* Central Dashboard: Sync manifests

Sync manifests for application "Central Dashboard".
Upstream manifests are copied from:
- Repo: https://github.com/kubeflow/kubeflow
- Path: components/centraldashboard/manifests
- Revision: 8901e3af3b83f599b89562162cdb2854f57082d3

Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com>

* Profiles + KFAM: Sync manifests

Sync manifests for application "Profiles + KFAM".
Upstream manifests are copied from:
- Repo: https://github.com/kubeflow/kubeflow
- Path: components/profile-controller/config
- Revision: 8901e3af3b83f599b89562162cdb2854f57082d3

Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com>

* PodDefaults Webhook: Sync manifests

Sync manifests for application "PodDefaults Webhook".
Upstream manifests are copied from:
- Repo: https://github.com/kubeflow/kubeflow
- Path: components/admission-webhook/manifests
- Revision: 8901e3af3b83f599b89562162cdb2854f57082d3

Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com>

* Jupyter Web App: Sync manifests

Sync manifests for application "Jupyter Web App".
Upstream manifests are copied from:
- Repo: https://github.com/kubeflow/kubeflow
- Path: components/crud-web-apps/jupyter/manifests
- Revision: 8901e3af3b83f599b89562162cdb2854f57082d3

Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com>

* Tensorboards Web App: Sync manifests

Sync manifests for application "Tensorboards Web App".
Upstream manifests are copied from:
- Repo: https://github.com/kubeflow/kubeflow
- Path: components/crud-web-apps/tensorboards/manifests
- Revision: 8901e3af3b83f599b89562162cdb2854f57082d3

Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com>

* Volumes Web App: Sync manifests

Sync manifests for application "Volumes Web App".
Upstream manifests are copied from:
- Repo: https://github.com/kubeflow/kubeflow
- Path: components/crud-web-apps/volumes/manifests
- Revision: 8901e3af3b83f599b89562162cdb2854f57082d3

Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com>

* Remove load_restrictor kustomize flag and lint README markdown (#1844)

* remove load-restrictor kustomize flag and lint README markdown

* Set kustomize loader to RestrictionRootOnly in test_util.go

* review: undo changes to test scripts

* review: add note to reusing patches best practices

* README: Add component version matrix (#1847)

Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com>

* README: Add FAQ, elaborate kustomize version, update version matrix (#1848)

* README: Add FAQ section and justify kustomize version

Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com>

* README: Update version matrix

Signed-off-by: Yannis Zarkadas <yanniszark@arrikto.com>

Co-authored-by: Yuan (Bob) Gong <4957653+Bobgy@users.noreply.github.com>
Co-authored-by: Paul Van Eck <pvaneck@us.ibm.com>
Co-authored-by: Andrey Velichkevich <andrey.velichkevich@gmail.com>
Co-authored-by: DavidSpek <vanderspek.david@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants