readme: lots more documentation #50
Conversation
openshift-ci-robot
added
the
size/L
openshift-ci-robot
added
the
approved
README.md
Outdated
| @@ -1,52 +1,202 @@ | |||
| # Cluster Network Operator | |||
|
|
|||
| This is an operator that manages the networking components for an openshift cluster. | |||
| The Cluster Network Operator installs and upgrades the networking components on a Kubernetes cluster. | |||
There was a problem hiding this comment.
Given that we only plan to support the network plugins that OCP supports, I think "an OpenShift cluster" is more correct than "a Kubernetes cluster"? Or maybe just explicitly explain that it can run on plain k8s but only supports a subset of plugins and configuration options.
README.md
Outdated
|
|
||
| ## Building | ||
| When the controller has reconciled and all its dependent resources have converged, the cluster should have an installed SDN plugin and a working service network. It normally runs very early in the install process - before the permanent API server has been started. |
There was a problem hiding this comment.
"permanent API server" is assuming OpenShift again.
Maybe something like "In OpenShift, the Cluster Network Operator runs very early in the install process, to set up the pod network and service proxying before starting the aggregated apiserver." (AIUI the main k8s apiserver is started hostNetwork before CNO, and keeps running afterward, and it's only the OpenShift apiserver that runs on the pod network? Or maybe that changed at some point, or maybe I misunderstood...)
There was a problem hiding this comment.
That is a better way of wording it. FYI, the network operator does run using the bootstrap api server (the apiserver running on the bootstrap node). The switch over to the real apiserver happens afterwards, even though the apiserver is host network.
README.md
Outdated
| ``` | ||
|
|
||
| ## Running | ||
| ## Configuring IP address pools | ||
| Users must supply at least two address pools - one for pods, and one for services. This is the ClusterNetworks and ServiceNetwork parameter. Some network plugins, such as OpenShiftSDN, support multiple ClusterNetworks. |
There was a problem hiding this comment.
"These are" not "This is"
|
|
||
| It follows the [Controller pattern](https://godoc.org/github.com/kubernetes-sigs/controller-runtime/pkg#hdr-Controller): it reconciles the state of the cluster against a desired configuration. The configuration specified by a CustomResourceDefinition called `networkoperator.openshift.io/NetworkConfig/v1`, which has a corresponding [type](/openshift/cluster-network-operator/blob/master/pkg/apis/networkoperator/v1/networkconfig_types.go). |
There was a problem hiding this comment.
so this is all going to change post-openshift/api#141 right?
README.md
Outdated
|
|
||
| * `iptablesSyncPeriod`: The interval between periodic iptables refreshes. Default: 30 seconds | ||
| * `bindAddress`: The address to "bind" to - the address for which traffic will be redirected | ||
| * `proxyArguments`: additional command-line flags to pass to kube-proxy - see |
There was a problem hiding this comment.
missing something at the end
There was a problem hiding this comment.
Bah, we don't actually have a list of all of the possible iptables configuration flags. The only existing one is upstream: https://kubernetes.io/docs/reference/command-line-tools-reference/kube-proxy/
There was a problem hiding this comment.
Referencing upstream docs is fine, I think
|
@danwinship thanks a ton for the thorough review. |
There was a problem hiding this comment.
Yea Docs! Thanks for writing this. I have a bunch of comments spread around. Two main points: the high level discussion should also motivate the choices. A lot of getting the cluster networking is done once when it is first set up. We need to let people know the ramifications of their choices. Second, we need to articulate valid ranges and any special cases for each option and how the choices impact operations.
We have tuning knobs and we need to discuss how to use them and what to expect as they are adjusted.
Maybe we should eliminate defaults and force the user to fill in all parameters.
README.md
Outdated
|
|
||
| You can build an image using podman with | ||
| # Configuring | ||
| The network operator has a complex configuration, but most parameters have a sensible default. |
There was a problem hiding this comment.
The following does not appear all that complex...
|
@pecameron @danwinship thanks for the thorough review. Updated, PTAL. |
| Currently, the only supported value for network Type is `OpenShiftSDN`. | ||
|
|
||
| ### Configuring OpenShiftSDN | ||
| OpenShiftSDN supports the following configuration options, all of which are optional: |
There was a problem hiding this comment.
should clarify that these are under defaultNetwork.openshiftSDNConfig
|
|
||
| * `iptablesSyncPeriod`: The interval between periodic iptables refreshes. Default: 30 seconds. Increasing this can reduce the number of iptables invocations. | ||
| * `bindAddress`: The address to "bind" to - the address for which traffic will be redirected. | ||
| * `proxyArguments`: additional command-line flags to pass to kube-proxy - see the [documentation](https://kubernetes.io/docs/reference/command-line-tools-reference/kube-proxy/). |
There was a problem hiding this comment.
should clarify how this works; proxyArguments is a map[string][]string, so the value has to be specified as an array, even though most arguments only accept a single value. (This might make more sense as a footnote to the example below.)
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danwinship, squeed 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 |
No description provided.