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