|
| 1 | +# Versioning and Branching in controller-runtime |
| 2 | + |
| 3 | +*NB*: this also applies to controller-tools. |
| 4 | + |
| 5 | +## TL;DR: |
| 6 | + |
| 7 | +### Users |
| 8 | + |
| 9 | +- We follow [Semantic Versioning (semver)](https://semver.org) |
| 10 | +- Use releases with your dependency management to ensure that you get |
| 11 | + compatible code |
| 12 | +- The master branch contains all the latest code, some of which may break |
| 13 | + compatibility (so "normal" `go get` is not reccomended) |
| 14 | + |
| 15 | +### Contributors |
| 16 | + |
| 17 | +- All code PR must be labeled with :bug: (patch fixes), :sparkles: |
| 18 | + (backwards-compatible features), or :warning: (breaking changes) |
| 19 | + |
| 20 | +- Breaking changes will find their way into the next major release, other |
| 21 | + changes will go into an semi-immediate patch or minor release |
| 22 | + |
| 23 | +- Please *try* to avoid breaking changes when you can. They make users |
| 24 | + face difficult decisions (when do I go through the pain of upgrading), |
| 25 | + and make life hard for maintainers and contributors (dealing with |
| 26 | + differences on stable branches). |
| 27 | + |
| 28 | +### Mantainers |
| 29 | + |
| 30 | +Don't be lazy, read the rest of this doc :-) |
| 31 | + |
| 32 | +## Overview |
| 33 | + |
| 34 | +controller-runtime (and friends) follow [Semantic |
| 35 | +Versioning](https://semver.org). I'd reccomend reading the aforementioned |
| 36 | +link if you're not familiar, but essentially, for any given release X.Y.Z: |
| 37 | + |
| 38 | +- an X (*major*) release indicates a set of backwards-compatible code. |
| 39 | + Changing X means there's a breaking change. |
| 40 | + |
| 41 | +- a Y (*minor*) release indicates a minimum feature set. Changing Y means |
| 42 | + the addition of a backwards-compatible feature. |
| 43 | + |
| 44 | +- a Z (*patch*) release indicates minimum set of bugfixes. Changing |
| 45 | + Z means a backwards-compatible change that doesn't add functionality. |
| 46 | + |
| 47 | +*NB*: If the major release is `0`, any minor release may contain breaking |
| 48 | +changes. |
| 49 | + |
| 50 | +These guarantees extend to all code exposed in public APIs of |
| 51 | +controller-runtime. This includes code both in controller-runtime itself, |
| 52 | +*plus types from dependencies in public APIs*. Types and functions not in |
| 53 | +public APIs are not considered part of the guarantee. |
| 54 | + |
| 55 | +In order to easily maintain the guarantees, we have a couple of processes |
| 56 | +that we follow. |
| 57 | + |
| 58 | +## Branches |
| 59 | + |
| 60 | +controller-runtime contains two types of branches: the *master* branch and |
| 61 | +*release-X* branches. |
| 62 | + |
| 63 | +The *master* branch is where development happens. All the latest and |
| 64 | +greatest code, including breaking changes, happens on master. |
| 65 | + |
| 66 | +The *release-X* branches contain stable, backwards compatible code. Every |
| 67 | +major (X) release, a new such branch is created. It is from these |
| 68 | +branches that minor and patch releases are tagged. |
| 69 | + |
| 70 | +The maintainers are responsible for updating the contents of this branch; |
| 71 | +generally, this is done just before a release using release tooling that |
| 72 | +filters and checks for changes tagged as breaking (see below). |
| 73 | + |
| 74 | +### Tooling |
| 75 | + |
| 76 | +* [gen-release-notes.sh](hack/gen-release-notes.sh): generate release |
| 77 | + notes for a range of commits, and check for next version type |
| 78 | + (***TODO***) |
| 79 | + |
| 80 | +* [cherrypick-minor-version.sh](hack/cherrypick-minor-version.sh): update |
| 81 | + a stable branch with appropriate commits from the master (***TODO***). |
| 82 | + |
| 83 | +* [verify-commit-messages.sh](hack/verify-commit-messages.sh): check that |
| 84 | + your PR and/or commit messages have the right versioning icon |
| 85 | + (***TODO***). |
| 86 | + |
| 87 | +## PR Process |
| 88 | + |
| 89 | +Every PR should be annotated with an icon indicating whether it's |
| 90 | +a: |
| 91 | + |
| 92 | +- Breaking change: :warning: (`:warning:`) |
| 93 | +- Non-breaking feature: :sparkles: (`:sparkles:`) |
| 94 | +- Patch fix: :bug: (`:bug:`) |
| 95 | +- Docs: :book: (`:book:`) |
| 96 | +- Infra/Tests/Other: :running: (`:running:`) |
| 97 | + |
| 98 | +Individual commits may be tagged separately, but will generally be assumed |
| 99 | +to match the PR. For instance, if you have a bugfix in with a breaking |
| 100 | +change, it's generally encouraged to submit the bugfix separately, but if |
| 101 | +you must put them in one PR, mark the commit separately. |
| 102 | + |
| 103 | +### Commands and Workflow |
| 104 | + |
| 105 | +controller-runtime follows the standard Kubernetes workflow: any PR needs |
| 106 | +`lgtm` and `approved` labels, PRs authors must have signed the CNCF CLA, |
| 107 | +and PRs must pass the tests before being merged. See [the contributor |
| 108 | +docs](https://github.com/kubernetes/community/blob/master/contributors/guide/pull-requests.md#the-testing-and-merge-workflow) |
| 109 | +for more info. |
| 110 | + |
| 111 | +We use the same priority and kind labels as Kubernetes. See the labels |
| 112 | +tab in GitHub for the full list. |
| 113 | + |
| 114 | +The standard Kubernetes comment commands should work in |
| 115 | +controller-runtime. See [Prow](https://prow.k8s.io/command-help) for |
| 116 | +a command reference. |
| 117 | + |
| 118 | +## Release Process |
| 119 | + |
| 120 | +Minor and patch releases are generally done immediately after a feature or |
| 121 | +bugfix is landed, or sometimes a series of features tied together. |
| 122 | + |
| 123 | +Major releases are done once a sufficient amount of breaking changes are |
| 124 | +accrued. Since we don't intend to have a ton of these, the maintainers |
| 125 | +will evaluate when to do a major release as it comes up. |
| 126 | + |
| 127 | +### Exact Steps |
| 128 | + |
| 129 | +1. Generate release notes using the release note tooling (***TODO***) |
| 130 | + |
| 131 | +2. Add a release for controller-runtime on GitHub, using those release |
| 132 | + notes |
| 133 | + |
| 134 | +3. Add a release for controller-tools on GitHub (folowing a similar |
| 135 | + process for controller-tools release notes). |
| 136 | + |
| 137 | +4. Publish associated binaries and release tarballs (***TODO***) |
| 138 | + |
| 139 | +5. Announce the release in `#kubebuilder` on Slack with a pinned message. |
| 140 | + |
| 141 | +### Breaking Changes |
| 142 | + |
| 143 | +Try to avoid breaking changes. They make life difficult for users, who |
| 144 | +have to rewrite their code when they eventually upgrade, and for |
| 145 | +maintainers/contributors, who have to deal with differences between master |
| 146 | +and stable branches. |
| 147 | + |
| 148 | +That being said, we'll occaisonally want to make breaking changes. They'll |
| 149 | +be merged onto master, but won't make it into a release immediately (see |
| 150 | +[Release Proccess](#release-process)). |
| 151 | + |
| 152 | +If you're going to make a breaking change, please make sure to explain in |
| 153 | +detail why it's helpful. Is it necessary to cleanly resolve an issue? |
| 154 | +Does it improve API ergonomics? |
| 155 | + |
| 156 | +Maintainers should treat breaking changes with caution, and evaluate |
| 157 | +potential non-breaking solutions (see below). |
| 158 | + |
| 159 | +*NB*: Pre-1.0 releases treat breaking changes a bit more lightly. We'll |
| 160 | +still consider carefully, but the pre-1.0 timeframe is useful for |
| 161 | +converging on a ergonomic API. |
| 162 | + |
| 163 | +#### Avoiding breaking changes |
| 164 | + |
| 165 | +##### Solutions to avoid |
| 166 | + |
| 167 | +- **Confusingly duplicate methods, functions, or variables.** |
| 168 | + |
| 169 | + For instance, suppose we have an interface method `List(ctx |
| 170 | + context.Context, options *ListOptions, obj runtime.Object) error`, and |
| 171 | + we decide to switch it so that options come at the end, parametrically. |
| 172 | + Adding a new interface method `ListParametric(ctx context.Context, obj |
| 173 | + runtime.Object, options... ListOption)` is probably not the right |
| 174 | + solution: |
| 175 | + |
| 176 | + - Users will intuitively see `List`, and use that in new projects, even |
| 177 | + if it's marked as deprecated. |
| 178 | + |
| 179 | + - Users who don't notice the deprecation may be confused as to the |
| 180 | + difference between `List` and `ListParametric`. |
| 181 | + |
| 182 | + - It's not immediately obvious in isolation (e.g. in surrounding code) |
| 183 | + why the method is called `ListParametric`, and may cause confusion |
| 184 | + when reading code that makes use of that method. |
| 185 | + |
| 186 | + In this case, it may be better to make the breaking change, and then |
| 187 | + eventually do a major release. |
| 188 | + |
| 189 | +## Why don't we... |
| 190 | + |
| 191 | +### Use "next"-style branches |
| 192 | + |
| 193 | +Development branches: |
| 194 | + |
| 195 | +- don't win us much in terms of maintenance in the case of breaking |
| 196 | + changes (we still have to merge/manage multiple branches for development |
| 197 | + and stable) |
| 198 | + |
| 199 | +- can be confusing to contributors, who often expect master to have the |
| 200 | + latest changes. |
| 201 | + |
| 202 | +### Never break compatibility |
| 203 | + |
| 204 | +Never doing a new major release could be an admirable goal, but gradually |
| 205 | +leads to API cruft. |
| 206 | + |
| 207 | +Since one of the goals of controller-runtime is to be a friendly and |
| 208 | +intuitive API, we want to avoid too much API cruft over time, and |
| 209 | +occaisonal breaking changes in major releases help accomplish that goal. |
| 210 | + |
| 211 | +Furthermore, our dependency on Kubernetes libraries makes this difficult |
| 212 | +(see below) |
| 213 | + |
| 214 | +### Always assume we've broken compatibility |
| 215 | + |
| 216 | +*a.k.a. k8s.io/client-go style* |
| 217 | + |
| 218 | +While this makes life easier (a bit) for maintainers, it's problematic for |
| 219 | +users. While breaking changes arrive sooner, upgrading becomes very |
| 220 | +painful. |
| 221 | + |
| 222 | +Furthermore, we still have to maintain stable branches for bugfixes, so |
| 223 | +the maintenance burden isn't lessened by a ton. |
| 224 | + |
| 225 | +### Extend compatibility guarantees to all dependencies |
| 226 | + |
| 227 | +This is very difficult with the number of Kubernetes dependencies we have. |
| 228 | +Kubernetes dependencies tend to either break compatibility every major |
| 229 | +release (e.g. k8s.io/client-go, which loosely follows semver), or at |
| 230 | +a whim (many other Kubernetes libraries). |
| 231 | + |
| 232 | +If we limit to the few objects we expose, we can better inform users about |
| 233 | +how *controller-runtime itself* has changed in a given release. Then, |
| 234 | +users can make informed decisions about how to proceed with any direct |
| 235 | +uses of Kubernetes dependencies their controller-runtime-based application |
| 236 | +may have. |
0 commit comments