development.md: rewrite for clarity, grammar, and spelling
This commit is contained in:
parent
2cec7e44a7
commit
6b42e8126a
|
|
@ -1,115 +1,119 @@
|
|||
# Development Guide
|
||||
|
||||
This document is the canonical source of truth for things like
|
||||
supported toolchain versions for building Kubernetes.
|
||||
This document is the canonical source of truth for things like supported
|
||||
toolchain versions for building Kubernetes.
|
||||
|
||||
Please submit an [issue] on github if you
|
||||
* find a requirement that this doc does not capture,
|
||||
* find other docs with references to requirements that
|
||||
are not simply links to this doc.
|
||||
|
||||
This document is intended to be relative to the branch in which it is found.
|
||||
Please submit an [issue] on Github if you
|
||||
* Notice a requirement that this doc does not capture.
|
||||
* Find a different doc that specifies requirements (the doc should instead link
|
||||
here).
|
||||
|
||||
Development branch requirements will change over time, but release branch
|
||||
requirements are frozen.
|
||||
|
||||
## Pre submit flight checks
|
||||
|
||||
Make sure you decide whether your issue and/or pull request is improving kubernetes architecture or whether its simply fixing a bug.
|
||||
Determine whether your issue or pull request is improving Kubernetes'
|
||||
architecture or whether it's simply fixing a bug.
|
||||
|
||||
Make sure there are no typos, if you need a diagram, add it. Make sure you SEPARATE the description of the problem
|
||||
(i.e. Y is a critical component that is too slow for an SLA that we care about) from the solution (i.e. make X faster).
|
||||
If you need a diagram, add it. SEPARATE the description of the problem (e.g. Y
|
||||
is a critical component that is too slow for an SLA that we care about) from the
|
||||
solution (e.g. make X faster).
|
||||
|
||||
Some of these checks were less common in Kubernetes earlier days, but now having over 1000 contributors, each issue should be
|
||||
filed with care, and should be sanity-checkable in under 5 minutes (even the busiest of reviewers can spare up to 5 minutes to
|
||||
review a patch that is thoughtfully justified).
|
||||
Some of these checks were less common in Kubernetes' earlier days. Now that we
|
||||
have over 1000 contributors, each issue should be filed with care. No issue
|
||||
should take more than 5 minutes to check for sanity (even the busiest of
|
||||
reviewers can spare 5 minutes to review a patch that is thoughtfully justified).
|
||||
|
||||
### Is this just a simple bug fix?
|
||||
|
||||
These patches can be easy to review since test coverage is submitted with the patch. Bug fixes don't usually require alot
|
||||
of extra testing: But please update the unit tests so that they catch the bug !
|
||||
Simple bug patches are easy to review since test coverage is submitted with the
|
||||
patch. Bug fixes don't usually require a lot of extra testing, but please
|
||||
update the unit tests so they catch the bug!
|
||||
|
||||
### Is this an architecture improvement?
|
||||
|
||||
Some examples of "Architecture" improvements:
|
||||
Some examples of "Architecture" improvements include:
|
||||
|
||||
- Adding a new feature or making a feature more configurable/modular.
|
||||
- Converting structs to interfaces.
|
||||
- Adding a new feature or making a feature more configurable or modular.
|
||||
- Improving test coverage.
|
||||
- Decoupling logic or creation of new utilities.
|
||||
- Making code more resilient (sleeps, backoffs, reducing flakiness, etc).
|
||||
- Making code more resilient (sleeps, backoffs, reducing flakiness, etc.).
|
||||
|
||||
These sorts of improvements are easily evaluated if they decrease lines of code without breaking functionality.
|
||||
These sorts of improvements are easily evaluated, especially when they decrease
|
||||
lines of code without breaking functionality. That said, please explain exactly
|
||||
what you are 'cleaning up' in your Pull Request so as not to waste a reviewer's
|
||||
time.
|
||||
|
||||
If you are improving the quality of code, then justify/state exactly what you 'cleaning up' in your Pull Request so as
|
||||
not to waste reviewer time.
|
||||
|
||||
If you're making code more resilient, test it with a local cluster to demonstrate how exactly your patch changes
|
||||
things.
|
||||
|
||||
Example: If you made a controller more robust to inconsistent data, make a mock object which returns incorrect data a
|
||||
few times and verify the controllers behaviour accordingly.
|
||||
If you're making code more resilient, include tests that demonstrate the new
|
||||
resilient behavior. For example: if your patch causes a controller to better
|
||||
handle inconsistent data, make a mock object which returns incorrect data a few
|
||||
times and verify the controller's new behaviour.
|
||||
|
||||
### Is this a performance improvement ?
|
||||
|
||||
If you are submitting a performance bug, you MUST ALSO submit data that demonstrates your problem if you want the issue to
|
||||
remain open. This can be done locally using kubemark, scheduler_perf, unit tests, go benchmark tests, or e2e tests on
|
||||
a real cluster with metrics plots.
|
||||
Performance bug reports MUST include data that demonstrates the bug. Without
|
||||
data, the issue will be closed. You can measure performance using kubemark,
|
||||
scheduler_perf, go benchmark tests, or e2e tests on a real cluster with metric
|
||||
plots.
|
||||
|
||||
Examples of how NOT to suggest a performance bug (these can really lead to a long review process and waste cycles):
|
||||
Examples of how NOT to suggest a performance bug (these lead to a long review
|
||||
process and waste cycles):
|
||||
|
||||
- We *should* be doing X instead of Y because it *might* lead to better performance.
|
||||
- We *should* be doing X instead of Y because it *might* lead to better
|
||||
performance.
|
||||
- Doing X instead of Y would reduce calls to Z.
|
||||
|
||||
The above statements have basically no value to a reviewer, because neither is a strong, testable, assertive statement.
|
||||
This will land your PR in a no-man's-land zone (at best), or waste tons of time for a busy reviewer (at worst).
|
||||
The above statements have no value to a reviewer because neither is backed by
|
||||
data. Writing issues like this lands your PR in a no-man's-land and waste your
|
||||
reviewers' time.
|
||||
|
||||
Of course any improvement is welcome, but performance improvements are the hardest to review. They often make code more
|
||||
complex, and to-often are not easily evaluated at review time due to lack of sufficient data submitted by the author
|
||||
of a performance improvement patch.
|
||||
|
||||
Some examples of "Performance" improvements:
|
||||
Examples of possible performance improvements include (remember, you MUST
|
||||
document the improvement with data):
|
||||
|
||||
- Improving a caching implementation.
|
||||
- Reducing calls to functions which are O(n^2), or reducing dependence on API server requests.
|
||||
- Changing the value of default parameters for proceeses, or making those values 'smarter'.
|
||||
- Parallelizing a calculation that needs to run on a large set of node/pod objects.
|
||||
- Reducing calls to functions which are O(n^2)
|
||||
- Reducing dependence on API server requests.
|
||||
- Changing the value of default parameters for processes, or making those values
|
||||
'smarter'.
|
||||
- Parallelizing a calculation that needs to run on a large set of node/pod
|
||||
objects.
|
||||
|
||||
These issues should always be submitted with (in decreasing order or value):
|
||||
|
||||
- A golang Benchmark test.
|
||||
- A visual depiction of reduced metric load on a cluster (measurable using metrics/ endpoints and grafana).
|
||||
- A hand-instrumented timing test (i.e. adding some logs into the controller manager).
|
||||
- A visual depiction of reduced metric load on a cluster (measurable using
|
||||
metrics/ endpoints and grafana).
|
||||
- A hand-instrumented timing test (i.e. adding some logs into the controller
|
||||
manager).
|
||||
|
||||
Without submitting data and results for your suggested performance improvements, its very possible that bikeshedding
|
||||
about meaningless possible performance optimizations could waste both reviewer time as well as your own.
|
||||
|
||||
Some examples of properly submitted performance issues, from different parts of the codebase. They all have one thing
|
||||
in common: Lots of data in the issue definition. If you are new to kubernetes and thinking about filing a performance
|
||||
optimization, re-read one or all of these before you get started.
|
||||
Here are some examples of properly submitted performance issues. If you are new
|
||||
to kubernetes and thinking about filing a performance optimization, re-read one
|
||||
or all of these before you get started.
|
||||
|
||||
- https://github.com/kubernetes/kubernetes/issues/18266 (apiserver)
|
||||
- https://github.com/kubernetes/kubernetes/issues/32833 (node)
|
||||
- https://github.com/kubernetes/kubernetes/issues/31795 (scheduler)
|
||||
|
||||
Since performance improvements deal with empirical systems, one playing in this space should be intimately familiar with
|
||||
the "scientific method" of creating a hypothesis, collecting data, and then revising your hypothesis. The above issues
|
||||
tend to do this transparently, using figures and data rather then theoretical postulations, as a first pass before a
|
||||
single line of code is reviewed.
|
||||
Since performance improvements can be empirically measured, you should follow
|
||||
the "scientific method" of creating a hypothesis, collecting data, and then
|
||||
revising your hypothesis. The above issues do this transparently, using figures
|
||||
and data rather then conjecture. Notice that the problem is analyzed and a
|
||||
correct solution is created before a single line of code is reviewed.
|
||||
|
||||
## Building Kubernetes with Docker
|
||||
|
||||
Official releases are built using Docker containers. To build Kubernetes using Docker please follow
|
||||
[these instructions](http://releases.k8s.io/HEAD/build/README.md).
|
||||
Official releases are built using Docker containers. To build Kubernetes using
|
||||
Docker please follow [these
|
||||
instructions](http://releases.k8s.io/HEAD/build/README.md).
|
||||
|
||||
## Building Kubernetes on a local OS/shell environment
|
||||
|
||||
Kubernetes development helper scripts assume an up-to-date
|
||||
GNU tools environment. Most recent Linux distros should work
|
||||
out-of-the-box.
|
||||
Kubernetes development helper scripts assume an up-to-date GNU tools
|
||||
environment. Recent Linux distros should work out-of-the-box.
|
||||
|
||||
Mac OS X ships with outdated BSD-based tools.
|
||||
We recommend installing [Os X GNU tools].
|
||||
Mac OS X ships with outdated BSD-based tools. We recommend installing [OS X GNU
|
||||
tools].
|
||||
|
||||
### etcd
|
||||
|
||||
|
|
@ -119,36 +123,30 @@ Please [install it locally][etcd-install] to run local integration tests.
|
|||
|
||||
### Go
|
||||
|
||||
Kubernetes is written in [Go](http://golang.org).
|
||||
If you don't have a Go development environment,
|
||||
please [set one up](http://golang.org/doc/code.html).
|
||||
Kubernetes is written in [Go](http://golang.org). If you don't have a Go
|
||||
development environment, please [set one up](http://golang.org/doc/code.html).
|
||||
|
||||
|
||||
| Kubernetes | requires Go |
|
||||
|----------------|--------------|
|
||||
| 1.0 - 1.2 | 1.4.2 |
|
||||
| 1.3, 1.4 | 1.6 |
|
||||
| 1.5 and higher | 1.7 - 1.7.5 |
|
||||
| | [1.8][go-1.8] not verified as of Feb 2017 |
|
||||
| Kubernetes | requires Go |
|
||||
|----------------|-------------------------------------------|
|
||||
| 1.0 - 1.2 | 1.4.2 |
|
||||
| 1.3, 1.4 | 1.6 |
|
||||
| 1.5 and higher | 1.7 - 1.7.5 |
|
||||
| | [1.8][go-1.8] not verified as of Feb 2017 |
|
||||
|
||||
After installation, you'll need `GOPATH` defined,
|
||||
and `PATH` modified to access your Go binaries.
|
||||
Ensure your GOPATH and PATH have been configured in accordance with the Go
|
||||
environment instructions.
|
||||
|
||||
A common setup is
|
||||
```sh
|
||||
export GOPATH=$HOME/go
|
||||
export PATH=$PATH:$GOPATH/bin
|
||||
```
|
||||
|
||||
#### Upgrading Go
|
||||
#### Upgrading Go
|
||||
|
||||
Upgrading Go requires specific modification of some scripts and container
|
||||
images.
|
||||
|
||||
- The image for cross compiling in [build/build-image/cross].
|
||||
The `VERSION` file and `Dockerfile`.
|
||||
- Update [dockerized-e2e-runner.sh] to run a kubekins-e2e with the desired Go version.
|
||||
This requires pushing the [e2e][e2e-image] and [test][test-image] images that are `FROM` the desired Go version.
|
||||
- Update [dockerized-e2e-runner.sh] to run a kubekins-e2e with the desired Go
|
||||
version. This requires pushing the [e2e][e2e-image] and [test][test-image]
|
||||
images that are `FROM` the desired Go version.
|
||||
- The cross tag `KUBE_BUILD_IMAGE_CROSS_TAG` in [build/common.sh].
|
||||
|
||||
|
||||
|
|
@ -161,13 +159,16 @@ manage dependencies.
|
|||
go get -u github.com/tools/godep
|
||||
```
|
||||
|
||||
Check your version; `v63` or higher is known to work for Kubernetes.
|
||||
The Godep version that Kubernetes is using is listed in `Godep/Godep.json` (in
|
||||
the kubernetes repo root). See what version you are running with this command:
|
||||
|
||||
```sh
|
||||
godep version
|
||||
```
|
||||
|
||||
Developers planning to manage dependencies in the `vendor/` tree may want to
|
||||
explore alternative environment setups. See [using godep to manage dependencies](godep.md).
|
||||
explore alternative environment setups. See [using godep to manage
|
||||
dependencies](godep.md).
|
||||
|
||||
|
||||
|
||||
|
|
@ -223,18 +224,6 @@ git remote set-url --push upstream no_push
|
|||
git remote -v
|
||||
```
|
||||
|
||||
#### Define a pre-commit hook
|
||||
|
||||
Please link the Kubernetes pre-commit hook into your `.git` directory.
|
||||
|
||||
This hook checks your commits for formatting, building, doc generation, etc.
|
||||
It requires both `godep` and `etcd` on your `PATH`.
|
||||
|
||||
```sh
|
||||
cd $working_dir/kubernetes/.git/hooks
|
||||
ln -s ../../hooks/pre-commit .
|
||||
```
|
||||
|
||||
### 3 Branch
|
||||
|
||||
Get your local master up to date:
|
||||
|
|
@ -260,12 +249,13 @@ cd $working_dir/kubernetes
|
|||
make
|
||||
```
|
||||
|
||||
To remove the limit on the number of errors the Go compiler reports (default limit is 10 errors):
|
||||
To remove the limit on the number of errors the Go compiler reports (default
|
||||
limit is 10 errors):
|
||||
```sh
|
||||
make GOGCFLAGS="-e"
|
||||
```
|
||||
|
||||
To build with optimizations disabled for enabling use of source debug tools:
|
||||
To build with optimizations disabled (enables use of source debug tools):
|
||||
|
||||
```sh
|
||||
make GOGCFLAGS="-N -l"
|
||||
|
|
@ -283,7 +273,7 @@ make cross
|
|||
cd $working_dir/kubernetes
|
||||
|
||||
# Run every unit test
|
||||
make test
|
||||
make test
|
||||
|
||||
# Run package tests verbosely
|
||||
make test WHAT=pkg/util/cache GOFLAGS=-v
|
||||
|
|
@ -322,13 +312,13 @@ When ready to review (or just to establish an offsite backup or your work),
|
|||
push your branch to your fork on `github.com`:
|
||||
|
||||
```sh
|
||||
git push -f origin myfeature
|
||||
git push -f ${your_remote_name} myfeature
|
||||
```
|
||||
|
||||
### 7 Create a pull request
|
||||
|
||||
1. Visit your fork at https://github.com/$user/kubernetes (replace `$user` obviously).
|
||||
2. Click the `Compare & pull request` button next to your `myfeature` branch.
|
||||
1. Visit your fork at https://github.com/$user/kubernetes
|
||||
2. Click the `Compare & Pull Request` button next to your `myfeature` branch.
|
||||
3. Check out the pull request [process](pull-requests.md) for more details.
|
||||
|
||||
_If you have upstream write access_, please refrain from using the GitHub UI for
|
||||
|
|
@ -345,12 +335,10 @@ and style.
|
|||
Commit changes made in response to review comments to the same branch on your
|
||||
fork.
|
||||
|
||||
Very small PRs are easy to review. Very large PRs are very difficult to
|
||||
review.
|
||||
|
||||
Very small PRs are easy to review. Very large PRs are very difficult to review.
|
||||
At the assigned reviewer's discretion, a PR may be switched to use
|
||||
[Reviewable](https://reviewable.k8s.io) instead. Once a PR is switched to
|
||||
Reviewable, please ONLY send or reply to comments through reviewable. Mixing
|
||||
Reviewable, please ONLY send or reply to comments through Reviewable. Mixing
|
||||
code review tools can be very confusing.
|
||||
|
||||
See [Faster Reviews](faster_reviews.md) for some thoughts on how to streamline
|
||||
|
|
@ -364,7 +352,7 @@ branch should represent meaningful milestones or units of work. Use commits to
|
|||
add clarity to the development and review process.
|
||||
|
||||
Before merging a PR, squash any _fix review feedback_, _typo_, and _rebased_
|
||||
sorts of commits.
|
||||
sorts of commits.
|
||||
|
||||
It is not imperative that every commit in a PR compile and pass tests
|
||||
independently, but it is worth striving for.
|
||||
|
|
@ -377,7 +365,7 @@ masse. This makes reviews easier.
|
|||
[]()
|
||||
<!-- END MUNGE: GENERATED_ANALYTICS -->
|
||||
|
||||
[Os X GNU tools]: https://www.topbug.net/blog/2013/04/14/install-and-use-gnu-command-line-tools-in-mac-os-x
|
||||
[OS X GNU tools]: https://www.topbug.net/blog/2013/04/14/install-and-use-gnu-command-line-tools-in-mac-os-x
|
||||
[build/build-image/cross]: https://github.com/kubernetes/kubernetes/blob/master/build/build-image/cross
|
||||
[build/common.sh]: https://github.com/kubernetes/kubernetes/blob/master/build/common.sh
|
||||
[dockerized-e2e-runner.sh]: https://github.com/kubernetes/test-infra/blob/master/jenkins/dockerized-e2e-runner.sh
|
||||
|
|
|
|||
Loading…
Reference in New Issue