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