ceph/SubmittingPatches-backports.rst

   1 Submitting Patches to Ceph - Backports
   2 ======================================
   3
   4 Most likely you're reading this because you intend to submit a GitHub pull
   5 request ("PR") targeting one of the stable branches ("nautilus", etc.) at
   6 https://github.com/ceph/ceph.
   7
   8 Before you open that PR, please read this entire document or, at the very least,
   9 the following two sections: `General principles`_ and `Cherry-picking rules`_.
  10
  11
  12 .. contents::
  13    :depth: 3
  14
  15
  16 General principles
  17 ------------------
  18
  19 To help the people who will review your backport, please state either in the
  20 backport PR, or in the backport tracker issue, or in the master tracker issue:
  21
  22 1. what bug is fixed
  23 2. why this fix is the minimal way to do it
  24 3. why does this need to be fixed in <release>
  25
  26 The above should be followed especially in cases when the backport could be seen
  27 as introducing, into a stable branch, code that is not related to a particular
  28 bug or issue.
  29
  30 Rationale: every modification of a stable branch carries a certain risk of
  31 introducing a regression. To minimize this risk, backports should be as
  32 straightforward and narrowly-targeted as possible. As a stable release series
  33 ages, the importance of following these general principles rises.
  34
  35
  36 Cherry-picking rules
  37 --------------------
  38
  39 The following rules, which have been codified from "best practices" developed
  40 over years of backporting, apply to the actual backport implementation:
  41
  42 * all fixes should land in master first
  43 * commits to stable branches should be cherry-picked from master
  44 * before starting to cherry-pick a set of commits from master, grep the master git history for the SHA1 of each master commit (using ``git log --grep``) to check for follow-up fixes. Include any follow-up fixes found in the set of commits to be cherry-picked.
  45 * when backporting a master PR to a stable branch, double-check that the backport PR contains cherry-picks of all of the master PR's commits. If any commit needs to be omitted, declare and explain this in the PR.
  46 * cherry-picks must be done using ``git cherry-pick -x``
  47 * if a cherry-pick from master is not feasible and a direct fix is being undertaken, this must be explained
  48 * the commit message generated by ``git cherry-pick -x`` must not be modified, except to add a "Conflicts" section below the "cherry picked from commit ..." line added by git
  49 * the "Conflicts" section must mention all files where changes had to be made manually (not just conflicts flagged by git)
  50 * the "Conflicts" section should also describe the manual changes that were made
  51 * if a change is to be backported to multiple stable branches, a tracker issue is needed, so the backports can be tracked (if a change is only to be backported to the most recent stable branch, a tracker issue is not strictly required)
  52
  53 For more information on tracker issues, see `Tracker workflow`_.
  54
  55 For more information on conflict resolution and writing the "Conflicts" section,
  56 see `Conflict resolution`_.
  57
  58 Adhering to these rules will make your backport easier for reviewers to
  59 understand. Not adhering to these rules creates additional work for reviewers
  60 and may cause your backport PR to be rejected.
  61
  62 Notes on the cherry-picking rules
  63 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  64
  65 What does "all fixes should land in master first" mean? What if I just need to
  66 fix the issue in <release>?
  67
  68 As the person fixing the issue, you are required to first check whether the
  69 issue exists in master. If it does, then the proper course of action is to
  70 create a master tracker (see `Tracker workflow`_) and fix the issue in master,
  71 first, and only then cherry-pick the fix to the stable branches that have the
  72 issue.
  73
  74 If the issue exists in the stable branch, but not in master, there are several
  75 possibilities:
  76
  77 1. it's a regression introduced into the stable branch by a bad backport
  78 2. the issue was fixed in master by some massive refactoring that cannot be backported
  79 3. the issue was already fixed in master by a cherry-pickable commit
  80
  81 In cases 1 and 2, it's permissible to fix the issue directly in the most recent
  82 stable branch, subject to the rule "if a commit could not be cherry-picked from
  83 master, the commit message must explain why that was not possible". Once the
  84 fix has landed in the most recent stable branch, it can be cherry-picked to
  85 older stable branches from there.
  86
  87 In case 3, the issue should be handled like any other backport - read on.
  88
  89
  90 Tracker workflow
  91 ----------------
  92
  93 Any change that is to be backported to multiple stable branches should have
  94 an associated tracker issue at https://tracker.ceph.com.
  95
  96 For fixes already merged to master, this may have already been done - see the
  97 ``Fixes:`` line in the master PR. If the master PR has already been merged and
  98 there is no associated master tracker issue, you can create a master tracker
  99 issue and fill in the fields as described below.
 100
 101 This master tracker issue should be in the "Bug" or "Feature"
 102 trackers of the relevant subproject under the "Ceph" parent project (or
 103 in the "Ceph" project itself if none of the subprojects are a good fit).
 104 The stable branches to which the master changes are to be cherry-picked should
 105 be listed in the "Backport" field. For example::
 106
 107     Backport: mimic, nautilus
 108
 109 Once the PR targeting master is open, add the PR number assigned by GitHub to
 110 the tracker issue. For example, if the PR number is 99999::
 111
 112     Pull request ID: 99999
 113
 114 Once the master PR has been merged, after checking that the change really needs
 115 to be backported and the Backport field has been populated, change the master
 116 tracker issue's ``Status`` field to "Pending Backport".
 117
 118     Status: Pending Backport
 119
 120 If you do not have sufficient permissions to modify any field of the tracker
 121 issue, just add a comment describing what changes you would like to make.
 122 Someone with permissions will make the necessary modifications on your behalf.
 123
 124 For straightforward backports, that's all that you (as the developer of the fix)
 125 need to do. Volunteers from the `Stable Releases and Backports team`_ will
 126 proceed to create Backport issues to track the necessary backports and stage the
 127 backports by opening GitHub PRs with the cherry-picks. If you don't want to
 128 wait, and provided you have sufficient permissions at https://tracker.ceph.com,
 129 you can `create Backport tracker issues` and `stage backports`_ yourself. In
 130 that case, read on.
 131
 132
 133 .. _`create backport tracker issues`:
 134 .. _`backport tracker issue`:
 135
 136 Creating Backport tracker issues
 137 --------------------------------
 138
 139 To track backporting efforts, "backport tracker issues" can be created from
 140 a parent "master tracker issue". The master tracker issue is described in the
 141 previous section, `Tracker workflow`_. This section focuses the backport tracker
 142 issue.
 143
 144 Once the entire `Tracker workflow`_ has been completed for the master issue,
 145 issues can be created in the Backport tracker for tracking the backporting work.
 146
 147 Under ordinary circumstances, the developer who merges the master PR will flag
 148 the master tracker issue for backport by changing the Status to "Pending
 149 Backport", and volunteers from the `Stable Releases and Backports team`_
 150 periodically create backport tracker issues by running the
 151 ``backport-create-issue`` script. They also do the actual backporting. But that
 152 does take time and you may not want to wait.
 153
 154 You might be tempted to forge ahead and create the backport issues yourself.
 155 Please don't do that - it is difficult (bordering on impossible) to get all the
 156 fields correct when creating backport issues manually, and why even try when
 157 there is a script that gets it right every time? Setting up the script requires
 158 a small up-front time investment. Once that is done, creating backport issues
 159 becomes trivial.
 160
 161 The backport-create-issue script
 162 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 163
 164 The script used to create backport issues is located at
 165 ``src/script/backport-create-issue`` in the master branch. Though there might be
 166 an older version of this script in a stable branch, do not use it. Only use the
 167 most recent version from master.
 168
 169 Once you have the script somewhere in your PATH, you can proceed to install the
 170 dependencies.
 171
 172 The dependencies are:
 173
 174 * python3
 175 * python-redmine
 176
 177 Python 3 should already be present on any recent Linux installation. The second
 178 dependency, `python-redmine`_, can be obtained from PyPi::
 179
 180     pip3 install --user python-redmine
 181
 182
 183 .. _`python-redmine`: https://pypi.org/project/python-redmine/
 184
 185 Then, try to run the script::
 186
 187     backport-create-issue --help
 188
 189 This should produce a usage message.
 190
 191 Finally, run the script to actually create the Backport issues.
 192 For example, if the tracker issue number is 55555::
 193
 194     backport-create-issue --user <tracker_username> --password <tracker_password> 55555
 195
 196 The script needs to know your https://tracker.ceph.com credentials in order to
 197 authenticate to Redmine. In lieu of providing your literal username and password
 198 on the command line, you could also obtain a REST API key ("My account" -> "API
 199 access key"), put it in ``~/.redmine_key`` and run the script like so::
 200
 201     backport-create-issue 55555
 202
 203
 204 .. _`stage backports`:
 205 .. _`stage the backport`:
 206 .. _`staging a backport`:
 207
 208 Opening a backport PR
 209 ---------------------
 210
 211 Once the `Tracker workflow`_ is completed and the `backport tracker issue`_ has
 212 been created, it's time to open a backport PR. One possibility is to do this
 213 manually, while taking care to follow the `cherry-picking rules`_. However, this
 214 can result in a backport that is not properly staged. For example, the PR
 215 description might not contain a link to the `backport tracker issue`_ (a common
 216 oversight). You might even forget to update the `backport tracker issue`_.
 217
 218 In the past, much time was lost, and much frustration caused, by the necessity
 219 of staging backports manually. Now, fortunately, there is a script available
 220 which automates the process and takes away most of the guesswork.
 221
 222 The ceph-backport.sh script
 223 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
 224
 225 Similar to the case of `creating backport tracker issues`_, staging the actual
 226 backport PR and updating the Backport tracker issue is difficult - if not
 227 impossible - to get right if you're doing it manually, and quickly becomes
 228 tedious if you do it more than once in a long while.
 229
 230 The ``ceph-backport.sh`` script automates the entire process of cherry-picking
 231 the commits from the master PR, opening the GitHub backport PR, and
 232 cross-linking the GitHub backport PR with the correct Backport tracker issue.
 233 The script can also be used to good effect if you have already manually prepared
 234 the backport branch with the cherry-picks in it.
 235
 236 The script is located at ``src/script/ceph-backport.sh`` in the ``master``
 237 branch. Though there might be an older version of this script in a stable
 238 branch, do not use it. Only use the most recent version from the master branch.
 239 To do this from anywhere and from any branch use the following
 240 alias that will use the most recent script in ``upstream/master`` of your
 241 local ceph clone on every call::
 242
 243     alias ceph-backport="bash <(git --git-dir=$pathToCephClone/.git --no-pager show upstream/master:src/script/ceph-backport.sh)"
 244
 245 ``ceph-backport.sh`` is just a bash script, so the only dependency is ``bash``
 246 itself, but it does need to be run in the top level of a local clone of
 247 ``ceph/ceph.git``. A small up-front time investment is required to get the
 248 script working in your environment. This is because the script needs to
 249 authenticate itself (i.e., as you) in order to use the GitHub and Redmine REST
 250 API services.
 251
 252 The script is self-documenting. Just run the script and proceed from there.
 253
 254 Once the script has been set up properly, you can validate the setup like so::
 255
 256     ceph-backport.sh --setup
 257
 258 Once you have this saying "Overall setup is OK", you have two options for
 259 staging the backport: either leave everything to the script, or prepare the
 260 backport branch yourself and use the script only for creating the PR and
 261 updating the Backport tracker issue.
 262
 263 If you prefer to leave everything to the script, just provide the Backport
 264 tracker issue number to the script::
 265
 266     ceph-backport.sh 55555
 267
 268 The script will start by creating the backport branch in your local git clone.
 269 The script always uses the following format for naming the branch::
 270
 271     wip-<backport_issue_number>-<name_of_stable_branch>
 272
 273 For example, if the Backport tracker issue number is 55555 and it's targeting
 274 the stable branch "nautilus", the backport branch would be named::
 275
 276     wip-55555-nautilus
 277
 278 If you prefer to create the backport branch yourself, just do that. Be sure to
 279 name the backport branch as described above. (It's a good idea to use this
 280 branch naming convention for all your backporting work.) Then, run the script::
 281
 282     ceph-backport.sh 55555
 283
 284 The script will see that the backport branch already exists, and use it.
 285
 286 Once the script hits the first cherry-pick conflict, it will no longer provide
 287 any cherry-picking assistance, so in that case it's up to you to resolve the conflict(s)
 288 (as described in `Conflict resolution`_) and finish cherry-picking
 289 all of the remaining commits. Once you are satisfied that the backport is complete in
 290 your local branch, `ceph-backport.sh` can finish the job of creating the pull request
 291 and updating the backport tracker issue. To make that happen, just re-run the script
 292 exactly as you did before::
 293
 294     ceph-backport.sh $BACKPORT_TRACKER_ID
 295
 296 The script will detect that it is running from a branch with the same name as the one it
 297 would normally create on the first run and continues after the cherry-picking phase.
 298
 299 For a quick reference on CLI, that contains above information, you can run::
 300
 301     ceph-backport.sh --usage
 302
 303 Conflict resolution
 304 ^^^^^^^^^^^^^^^^^^^
 305
 306 If git reports conflicts, the script will abort to allow you to resolve the
 307 conflicts manually.
 308
 309 Once the conflicts are resolved, complete the cherry-pick ::
 310
 311     git cherry-pick --continue
 312
 313 Git will present a draft commit message with a "Conflicts" section.
 314
 315 Unfortunately, in recent versions of git, the Conflicts section is commented
 316 out. Since the Conflicts section is mandatory for Ceph backports that do not
 317 apply cleanly, you will need to uncomment the entire "Conflicts" section
 318 of the commit message before committing the cherry-pick. You can also
 319 include commentary on what the conflicts were and how you resolved
 320 them. For example::
 321
 322     Conflicts:
 323             src/foo/bar.cc
 324     - mimic does not have blatz; use batlo instead
 325
 326 When editing the cherry-pick commit message, leave everything before the
 327 "cherry picked from" line unchanged. Any edits you make should be in the part
 328 following that line. Here is an example::
 329
 330     osd: check batlo before setting blatz
 331
 332     Setting blatz requires special precautions. Check batlo first.
 333
 334     Fixes: https://tracker.ceph.com/issues/99999
 335     Signed-off-by: Random J Developer <random@developer.example.com>
 336     (cherry picked from commit 01d73020da12f40ccd95ea1e49cfcf663f1a3a75)
 337
 338     Conflicts:
 339         src/osd/batlo.cc
 340     - add_batlo_check has an extra arg in newer code
 341
 342 Naturally, the ``Fixes`` line points to the master issue. You might be tempted
 343 to modify it so it points to the backport issue, but - please - don't do that.
 344 First, the master issue points to all the backport issues, and second, *any*
 345 editing of the original commit message calls the entire backport into doubt,
 346 simply because there is no good reason for such editing.
 347
 348 The part below the ``(cherry picked from commit ...)`` line is fair game for
 349 editing. If you need to add additional information to the cherry-pick commit
 350 message, append that information below this line. Once again: do not modify the
 351 original commit message.
 352
 353 If you use `ceph-backport.sh` for your backport creation (which is recommended),
 354 read up at the end of `The ceph-backport.sh script`_ on how to continue from here.
 355
 356 Labelling of backport PRs
 357 -------------------------
 358
 359 Once the backport PR is open, the first order of business is to set the
 360 Milestone tag to the stable release the backport PR is targeting. For example,
 361 if the PR is targeting "nautilus", set the Milestone tag to "nautilus".
 362
 363 If you don't have sufficient GitHub permissions to set the Milestone, don't
 364 worry. Members of the `Stable Releases and Backports team`_ periodically run
 365 a script (``ceph-backport.sh --milestones``) which scans all PRs targetting stable
 366 branches and automatically adds the correct Milestone tag if it is missing.
 367
 368 Next, check which component label was applied to the master PR corresponding to
 369 this backport, and double-check that that label is applied to the backport PR as
 370 well. For example, if the master PR carries the component label "core", the
 371 backport PR should also get that label.
 372
 373 In general, it is the responsibility of the `Stable Releases and Backports
 374 team`_ to ensure that backport PRs are properly labelled. If in doubt, just
 375 leave the labelling to them.
 376
 377 .. _`backport PR reviewing`:
 378 .. _`backport PR testing`:
 379 .. _`backport PR merging`:
 380
 381 Reviewing, testing, and merging of backport PRs
 382 -----------------------------------------------
 383
 384 Once your backport PR is open and the Milestone is set properly, the
 385 `Stable Releases and Backports team` will take care of getting the PR
 386 reviewed and tested. Once the PR is reviewed and tested, it will be merged.
 387
 388 If you would like to facilitate this process, you can solicit reviews and run
 389 integration tests on the PR. In this case, add comments to the PR describing the
 390 tests you ran and their results.
 391
 392 Once the PR has been reviewed and deemed to have undergone sufficient testing,
 393 it will be merged. Even if you have sufficient GitHub permissions to merge the
 394 PR, please do *not* merge it yourself. (Uncontrolled merging to stable branches
 395 unnecessarily complicates the release preparation process, which is done by
 396 volunteers.)
 397
 398
 399 Stable Releases and Backports team
 400 ----------------------------------
 401
 402 Ceph has a `Stable Releases and Backports`_ team, staffed by volunteers,
 403 which is charged with maintaining the stable releases and backporting bugfixes
 404 from the master branch to them. (That team maintains a wiki, accessible by
 405 clicking the `Stable Releases and Backports`_ link, which describes various
 406 workflows in the backporting lifecycle.)
 407
 408 .. _`Stable Releases and Backports`: http://tracker.ceph.com/projects/ceph-releases/wiki
 409
 410 Ordinarily, it is enough to fill out the "Backport" field in the bug (tracker
 411 issue). The volunteers from the Stable Releases and Backports team will
 412 backport the fix, run regression tests on it, and include it in one or more
 413 future point releases.
 414
 415