-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add instruction in how to install Knative with Knative operators #2261
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
Add instruction in how to install Knative with Knative operators #2261
Conversation
abrennan89
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some review comments.
Overall, great work! I would say though that it's worth reviewing the dev documentation style guide sections on style and tone to see where improvements can be made to remove 'fluff' or make the content more readable, e.g. https://developers.google.com/style/tone and https://developers.google.com/style/voice
Thanks for this contribution, Vincent!
|
|
||
| # Knative installations with Knative Operators | ||
|
|
||
| As of v0.10.0, Knative started to release Knative operators as tools to install, configure and manage Knative. This |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would remove the sentence:
"As of v0.10.0, Knative started to release Knative operators as tools to install, configure and manage Knative."
or rewrite it to remove specifying the version, e.g.
"Knative provides operators...[]"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(These docs are all versioned, so we shouldn't need to reference the version within the docs.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
| # Knative installations with Knative Operators | ||
|
|
||
| As of v0.10.0, Knative started to release Knative operators as tools to install, configure and manage Knative. This | ||
| guide will explain how to install and uninstall Knative with Knative operators. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change with -> using maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
|
|
||
| There are two major components in Knative: Serving and Eventing. There are two operators: [Knative Serving Operator](https://github.com/knative/serving-operator) | ||
| and [Knative Eventing Operator](https://github.com/knative/eventing-operator), respectively dedicated to each of them. Please make sure you have set up a Kubernetes | ||
| cluster accessible to your local workstation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if it's clear what this means - maybe instead add a pre-requisites or before you begin section that calls out any specific requirements for the cluster?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to a "prerequisites" section. You probably want to include:
- Have a cluster
- Cluster has enough space
- You have
cluster-admin(or at least the ability to create CRDs, Roles, and RoleBindings, which is effectivelycluster-admin) - Ability to fetch images from the internet (?)
| Knative operators are still in Alpha phase. They only focus on Knative installation for the generic Kubernetes platform, | ||
| which means Docker-Desktop Kubernetes on Mac and Minikube can be directly used as the Kubernetes clusters for you to install | ||
| Knative with Knative operators, without any additional configuration out of the scope of the operator custom resource, | ||
| but vendor-specific Kubernetes services, like Google Kubernetes Engine, IBM Kubernetes Service, OpenShift, etc, may need |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make this a new sentence for better readability?
e.g.
"Vendor specific...[]"
change like -> such as? Maybe remove the "etc"?
e.g.
"...such as GKE, IBM Kubernetes Service or Red Hat OpenShift"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is a really long sentence. Maybe split it as:
The focus has been on installation on a generic Kubernetes platform (such as Docker Desktop, Minikube, or kubeadm clusters), and the operator does not perform any platform specific customization.
I don't think we're suggesting that the operator won't work on GKE, IKS, or OpenShift, just that it won't be integrated and you might to do some additional work.
| * Docker-Desktop Kubernetes on Mac | ||
| * Minikube | ||
|
|
||
| The same to any other Kubernetes operator, Knative operators use custom resources as the sources of truth to configure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove "The same to any other Kubernetes operator"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
| cluster for your production environment, helping us find out what we can potentially improve in Knative operators by | ||
| reporting your issues. | ||
|
|
||
| ## Prerequisites: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move this to earlier in the doc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
| ## Prerequisites: | ||
|
|
||
| Knative Serving needs an ingress or gateway to route inbound network traffic to the services. There are multiple options | ||
| that can be used as the ingress candidates: Istio, Ambassador, Contour, Gloo, Kourier, etc. However, to install Knative |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
again remove "etc"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
|
|
||
| Knative Serving needs an ingress or gateway to route inbound network traffic to the services. There are multiple options | ||
| that can be used as the ingress candidates: Istio, Ambassador, Contour, Gloo, Kourier, etc. However, to install Knative | ||
| with operators, we only support Istio as the ingress for now. Knative Operators are currently working on enabling the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could actually remove this section altogether if only Istio is supported and just mention this requirement instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I consolidate this section with prerequisites.
| If everything goes fine, it is time to install Knative Serving by installing the custom resource of Knative Serving | ||
| operator. Operator supports to install Knative Serving under any namespace, which needs to be created as well. Then, | ||
| we can create a custom resource with empty spec section. Technically, you can use any namespace, but we recommend you | ||
| to use knative-serving. In the rest of this tutorial, we keep on using knative-servingas the namespace to create the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: change knative-servingas to knative-serving as
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
|
|
||
| If deployment of Knative Eventing operator is ready, it is time to install Knative Evenintg by installing the custom | ||
| resource of Knative Eventing operator. The same to Knative Serving operator, you can use any namespace, but we recommend | ||
| you to use knative-eventing for Knative eventing. In the rest of this tutorial, we keep on using knative-eventingas the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same issue for knative-eventingas ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
docs/install/README.md
Outdated
|
|
||
| To install the Knative CLI, follow our [installation guide](./install-kn.md). | ||
|
|
||
| To install Knative components with Knative Operators, follow our [installation guide with Knative Operators](./knative-with-operators.md). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this might be clearer if you make this an alternative on line 2, rather than a separate item. Right now, this reads:
- Follow these instructions to install Knative
- Then install the client side tools
- Then install the operator.
I think you want:
- Install Knative manually, or use the operator
- Then install the CLI tools.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
|
|
||
| # Knative installations with Knative Operators | ||
|
|
||
| As of v0.10.0, Knative started to release Knative operators as tools to install, configure and manage Knative. This |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(These docs are all versioned, so we shouldn't need to reference the version within the docs.)
| # Knative installations with Knative Operators | ||
|
|
||
| As of v0.10.0, Knative started to release Knative operators as tools to install, configure and manage Knative. This | ||
| guide will explain how to install and uninstall Knative with Knative operators. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I'd s/will explain/explains/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
| guide will explain how to install and uninstall Knative with Knative operators. | ||
|
|
||
| There are two major components in Knative: Serving and Eventing. There are two operators: [Knative Serving Operator](https://github.com/knative/serving-operator) | ||
| and [Knative Eventing Operator](https://github.com/knative/eventing-operator), respectively dedicated to each of them. Please make sure you have set up a Kubernetes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These sentences read a little bit choppy (probably because you've got 4 things that are related to each other in a matrix-y way, but text is linear).
What about turning it around and emphasizing the operator first?
Each component in Knative has a separate operator to install and configure the components. This means that there is a [Serving operator] and an [Eventing operator], and you can choose to install one or both independently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
|
|
||
| There are two major components in Knative: Serving and Eventing. There are two operators: [Knative Serving Operator](https://github.com/knative/serving-operator) | ||
| and [Knative Eventing Operator](https://github.com/knative/eventing-operator), respectively dedicated to each of them. Please make sure you have set up a Kubernetes | ||
| cluster accessible to your local workstation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to a "prerequisites" section. You probably want to include:
- Have a cluster
- Cluster has enough space
- You have
cluster-admin(or at least the ability to create CRDs, Roles, and RoleBindings, which is effectivelycluster-admin) - Ability to fetch images from the internet (?)
| Knative operators are still in Alpha phase. They only focus on Knative installation for the generic Kubernetes platform, | ||
| which means Docker-Desktop Kubernetes on Mac and Minikube can be directly used as the Kubernetes clusters for you to install | ||
| Knative with Knative operators, without any additional configuration out of the scope of the operator custom resource, | ||
| but vendor-specific Kubernetes services, like Google Kubernetes Engine, IBM Kubernetes Service, OpenShift, etc, may need |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is a really long sentence. Maybe split it as:
The focus has been on installation on a generic Kubernetes platform (such as Docker Desktop, Minikube, or kubeadm clusters), and the operator does not perform any platform specific customization.
I don't think we're suggesting that the operator won't work on GKE, IKS, or OpenShift, just that it won't be integrated and you might to do some additional work.
| the open source version only supports the following Kubernetes services, without additional configurations: | ||
|
|
||
| * Docker-Desktop Kubernetes on Mac | ||
| * Minikube |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about kubeadm-created clusters?
More generally, why aren't other platforms supported?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like say if we install operator directly on vendor-specific platforms, we need to do some other customization to make it work. The customization/additional configuration vary from platform to platform.
houshengbo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@evankanderson @abrennan89 I have pushed a new commit based on the comments. Thanks for the review.
| @@ -0,0 +1,244 @@ | |||
| --- | |||
| title: "Installing Knative with Knative Operators" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change to "Installing Knative components using Operators"
| Knative operators are still in Alpha phase. They focus on installation on a generic Kubernetes platform, such as | ||
| Docker Desktop, Minikube, or kubeadm clusters, and the operators do not perform any platform specific customization. | ||
| If you are not sure the customization of the Knative operators for vendor-specific platforms, use the generic Kubernetes | ||
| platform. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd remove this section entirely since the only installation docs we offer now are the 'generic' ones: https://knative.dev/docs/install/any-kubernetes-cluster/
Maybe you could just re-use the Before you begin section of those docs here instead? cc @mattmoor to check the accuracy of this suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@houshengbo idk if you missed this comment or you're just not doing this?
I don't think there's any need to mention specific vendors here. I'd remove lines 28-31 but it's your call.
abrennan89
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some additional comments @houshengbo
| type: "docs" | ||
| --- | ||
|
|
||
| # Knative installations with Knative Operators |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Installing Knative components using Operators"
abrennan89
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think just one or two minor comments, and Matt has mentioned a couple of spelling errors. Otherwise, looks good to me! Thanks @houshengbo !
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: abrennan89, houshengbo 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 |
|
Thanks to everyone for their work on this! |
…) (#2304) * Add instruction in how to install Knative with Knative operators * Fix the operators's doc based on comments * Modify the docs of Operators to outline better * Fixes the typos
Fixes #2260
The outline of this page can be found here: https://github.com/houshengbo/docs/blob/add-doc-operator/docs/install/knative-with-operators.md
Proposed Changes