mirror of
https://github.com/ceph/ceph
synced 2026-08-01 22:45:39 +00:00
Updates SubmittingPatches-backports.rst and development-workflow.rst to capture the new automated backport auditing system. Changes include: - Documenting the CI checks for commit parity, conflict simulation, and Redmine linkage. - Explaining how PR authors can trigger a retest (`/audit retest`) and how leads can bypass the audit (`/audit override`). - Replacing outdated `master` branch references with `main`. - Updating the release cycle timeline to reflect current releases (Quincy, Reef, Squid, Tentacle). - Removing obsolete instructions for manual PR labeling. Signed-off-by: Patrick Donnelly <pdonnell@ibm.com>
419 lines
20 KiB
ReStructuredText
419 lines
20 KiB
ReStructuredText
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 ("tentacle", 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
|
|
------------------
|
|
|
|
.. note:: There are many automations that exist for backports. If you read nothing else in this document, please read `ceph-backport`_ section for automating backport creation.
|
|
|
|
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
|
|
--------------------
|
|
|
|
**Note: These rules are strictly enforced by the `releng-audit` GitHub Actions CI. Failure to adhere to them will block your PR.**
|
|
|
|
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``
|
|
* if a commit is entirely specific to the backport and *not* cherry-picked from ``main``, its commit message summary must begin with the target branch name (e.g., ``squid: fix compilation issue``).
|
|
* 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
|
|
|
|
In general, you should stop here with manually managing the state of the
|
|
tracker tickets. If the ``Pull request ID`` field of the ``main`` tracker is
|
|
up-to-date, the ``redmine-upkeep`` Github CI will automatically detect a merge
|
|
and move your tracker to the ``Pending backport`` state as long as the
|
|
``Backport`` field has release branches in it. Otherwise the ticket will be
|
|
moved to ``Resolved``.
|
|
|
|
However, if that CI fails to run for whatever reason, you may manually correct
|
|
the ticket by setting 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.
|
|
|
|
.. _`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.
|
|
|
|
When a ``main`` branch tracker issue is in the ``Pending Backport`` state,
|
|
another Github CI running ``backport-create-issue`` will trigger and create the
|
|
backport tickets to be associated with the ``main`` tracker. This workflow will
|
|
auto-populate the required metadata for the tracker tickets to track the
|
|
backports.
|
|
|
|
The backport-create-issue script
|
|
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
|
|
|
.. warning:: It is discouraged to run this script yourself. However, sometimes CI checks fail to run, so please review for those circumstances.
|
|
|
|
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::
|
|
|
|
python3 -c "$(git show main:src/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::
|
|
|
|
python3 -c "$(git show main:src/script/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::
|
|
|
|
python3 -c "$(git show main:src/script/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.
|
|
|
|
.. _`ceph-backport`:
|
|
|
|
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
|
|
|
|
Milestones and Labels
|
|
"""""""""""""""""""""
|
|
|
|
The ``ceph-backport.sh`` script automates the process of setting the Milestone
|
|
tag to the stable release the backport PR is targeting.
|
|
|
|
PR labels (such as component labels) are added automatically by automations;
|
|
the backport author does not need to do anything manually.
|
|
|
|
|
|
Conflict resolution
|
|
^^^^^^^^^^^^^^^^^^^
|
|
|
|
**Automated Conflict Simulation:** The `releng-audit` CI will perform a dry-run of the cherry-pick and compare the resulting tree to your PR. If you make *any* manual changes (even to fix a bug introduced by the backport), you **must** document it in the ``Conflicts:`` block. If a legitimate manual resolution is flagged as an unapproved deviation, any user with "maintain" or "admin" rights on the repository, or a member of the ``@ceph/ceph-release-manager`` team, must comment ``/audit override`` or manually apply the ``releng-audit-override`` label on the PR to bypass the simulation check.
|
|
|
|
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.
|
|
|
|
Automated Audit Workflow
|
|
------------------------
|
|
|
|
Once your backport PR is open, it will be automatically audited by the `releng-audit` GitHub Actions CI. This workflow enforces backport rules and ensures consistency between GitHub and Redmine.
|
|
|
|
The audit performs the following checks:
|
|
|
|
* **Merge Conflict:** Verifies that the PR can merge cleanly into the target base branch without git conflicts.
|
|
* **Commit Parity:** Ensures all commits from the original ``main`` PR(s) are present in the backport. It flags missing commits, unmerged cherry-picks, or invalid commit message formats.
|
|
* **Conflict Simulation:** Dry-runs the cherry-pick sequence to dynamically verify conflict resolutions. It will fail if there are undocumented or unapproved deviations from a clean cherry-pick.
|
|
* **Redmine Linkage:** Checks that the backport PR is properly linked to its Redmine backport tracker, and that the original ``main`` PR is correctly linked to the parent tracker ticket.
|
|
|
|
**Addressing Failures (PR Author Workflow):**
|
|
|
|
If the audit fails, the CI bot will post a review detailing the issues and apply the ``releng-audit-fail`` label.
|
|
|
|
* **Fix the issues:** Address the failures as guided by the bot's review (e.g., fix Redmine tracker links, rebase your branch, fix commit messages, or adjust the code to match upstream patches).
|
|
* **Retest:** When you are ready for a new audit, **remove the ``releng-audit-fail`` label** or comment ``/audit retest`` on the PR.
|
|
* **Note:** You cannot manually apply the ``releng-audit-pass`` or ``releng-audit-fail`` labels; they are strictly managed by the CI bot and your manual changes to them will be rejected.
|
|
|
|
**Bypassing the Audit (Component Lead / Release Manager Workflow):**
|
|
|
|
In some cases, the audit may flag a legitimate manual conflict resolution or an intentional deviation in the code.
|
|
|
|
* If a deviation is intentional, documented in the commit message, and approved, authorized users (repository admins/maintainers or members of the ``@ceph/ceph-release-manager`` team) can bypass the check.
|
|
* To apply the bypass, comment ``/audit override`` on the PR or manually apply the ``releng-audit-override`` label.
|
|
|
|
**Audit Labels Explained:**
|
|
|
|
* ``releng-audit-pass``: Applied automatically by the bot when all checks pass.
|
|
* ``releng-audit-fail``: Applied automatically by the bot when checks fail. Removing this label triggers a new audit.
|
|
* ``releng-audit-override``: Applied manually by authorized leads/managers to override an audit failure.
|
|
|
|
The bot will leave a failing CI check if a backport PR does not have either ``releng-audit-pass`` or ``releng-audit-override``.
|
|
|
|
.. _`backport PR reviewing`:
|
|
.. _`backport PR testing`:
|
|
.. _`backport PR merging`:
|
|
|
|
Reviewing, testing, and merging of backport PRs
|
|
-----------------------------------------------
|
|
|
|
Once your backport PR is open, it will be reviewed and tested. When the PR has
|
|
been reviewed and tested, it will be merged.
|
|
|
|
* **Prerequisite:** The PR must have the ``releng-audit-pass`` or ``releng-audit-override`` label applied before it is eligible for merging.
|
|
|
|
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.)
|