summaryrefslogtreecommitdiffstats
path: root/SubmittingPatches-backports.rst
diff options
context:
space:
mode:
authorDaniel Baumann <daniel.baumann@progress-linux.org>2024-04-07 18:45:59 +0000
committerDaniel Baumann <daniel.baumann@progress-linux.org>2024-04-07 18:45:59 +0000
commit19fcec84d8d7d21e796c7624e521b60d28ee21ed (patch)
tree42d26aa27d1e3f7c0b8bd3fd14e7d7082f5008dc /SubmittingPatches-backports.rst
parentInitial commit. (diff)
downloadceph-19fcec84d8d7d21e796c7624e521b60d28ee21ed.tar.xz
ceph-19fcec84d8d7d21e796c7624e521b60d28ee21ed.zip
Adding upstream version 16.2.11+ds.upstream/16.2.11+dsupstream
Signed-off-by: Daniel Baumann <daniel.baumann@progress-linux.org>
Diffstat (limited to 'SubmittingPatches-backports.rst')
-rw-r--r--SubmittingPatches-backports.rst415
1 files changed, 415 insertions, 0 deletions
diff --git a/SubmittingPatches-backports.rst b/SubmittingPatches-backports.rst
new file mode 100644
index 000000000..e54816b7f
--- /dev/null
+++ b/SubmittingPatches-backports.rst
@@ -0,0 +1,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 master 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 master first
+* commits to stable branches should be cherry-picked from master
+* 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.
+* 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.
+* cherry-picks must be done using ``git cherry-pick -x``
+* if a cherry-pick from master 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 master 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 master. If it does, then the proper course of action is to
+create a master tracker (see `Tracker workflow`_) and fix the issue in master,
+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 master, there are several
+possibilities:
+
+1. it's a regression introduced into the stable branch by a bad backport
+2. the issue was fixed in master by some massive refactoring that cannot be backported
+3. the issue was already fixed in master 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
+master, 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 master, this may have already been done - see the
+``Fixes:`` line in the master PR. If the master PR has already been merged and
+there is no associated master tracker issue, you can create a master tracker
+issue and fill in the fields as described below.
+
+This master 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 master changes are to be cherry-picked should
+be listed in the "Backport" field. For example::
+
+ Backport: mimic, nautilus
+
+Once the PR targeting master 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 master PR has been merged, after checking that the change really needs
+to be backported and the Backport field has been populated, change the master
+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 "master tracker issue". The master 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 master issue,
+issues can be created in the Backport tracker for tracking the backporting work.
+
+Under ordinary circumstances, the developer who merges the master PR will flag
+the master 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 master 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 master.
+
+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 master 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 ``master``
+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 master branch.
+To do this from anywhere and from any branch use the following
+alias that will use the most recent script in ``upstream/master`` of your
+local ceph clone on every call::
+
+ alias ceph-backport="bash <(git --git-dir=$pathToCephClone/.git --no-pager show upstream/master: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 master issue. You might be tempted
+to modify it so it points to the backport issue, but - please - don't do that.
+First, the master 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 master PR corresponding to
+this backport, and double-check that that label is applied to the backport PR as
+well. For example, if the master 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 master 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.
+
+