From 0a54fcdfc491ce2b2bb3ded77e319a7cff785e73 Mon Sep 17 00:00:00 2001 From: Patrick Donnelly Date: Wed, 6 May 2026 16:50:42 -0400 Subject: [PATCH] doc: document the releng-audit workflow and update release examples 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 --- SubmittingPatches-backports.rst | 122 ++++++++++++++++++++----------- doc/dev/development-workflow.rst | 115 ++++++++++++++++------------- src/script/ptl-tool.py | 12 +-- 3 files changed, 152 insertions(+), 97 deletions(-) diff --git a/SubmittingPatches-backports.rst b/SubmittingPatches-backports.rst index bb55088cb5fa..45d24d1ff064 100644 --- a/SubmittingPatches-backports.rst +++ b/SubmittingPatches-backports.rst @@ -2,7 +2,7 @@ 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 +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, @@ -16,6 +16,8 @@ the following two sections: `General principles`_ and `Cherry-picking rules`_. 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: @@ -36,11 +38,14 @@ 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`` @@ -111,9 +116,16 @@ 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". +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 @@ -121,40 +133,28 @@ 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. -Authors of pull requests are responsible for creating associated backport pull -requests. As long as you have sufficient permissions at -https://tracker.ceph.com, you can `create Backport tracker issues` and `stage -backports`_ yourself. Read these linked sections to learn how to create -backport tracker issues and how to stage backports: - .. _`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". +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. -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. +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 @@ -178,21 +178,21 @@ dependency, `python-redmine`_, can be obtained from PyPi:: Then, try to run the script:: - backport-create-issue --help + 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:: - backport-create-issue --user --password 55555 + python3 -c "$(git show main:src/script/backport-create-issue)" --user --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 + python3 -c "$(git show main:src/script/backport-create-issue)" 55555 .. _`stage backports`: @@ -213,6 +213,8 @@ 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 ^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -294,13 +296,23 @@ 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 ^^^^^^^^^^^^^^^^^^^ -If git reports conflicts, the script will abort to allow you to resolve the -conflicts manually. +**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. -Once the conflicts are resolved, complete the cherry-pick :: +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 @@ -313,6 +325,7 @@ 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 @@ -347,17 +360,40 @@ 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 -------------------------- +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. -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". +* **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. -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. +**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`: @@ -369,6 +405,8 @@ 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. diff --git a/doc/dev/development-workflow.rst b/doc/dev/development-workflow.rst index 8065a69b0450..594dea901800 100644 --- a/doc/dev/development-workflow.rst +++ b/doc/dev/development-workflow.rst @@ -34,37 +34,42 @@ were not written prior to checking all integration tests, they can be committed to the stable branch and the result sent for publication without going through another run of integration tests. + Release Cycle ============= :: - Ceph hammer infernalis + Ceph squid tentacle Developer CDS CDS Summit | | | | - development | | - release | v0.88 v0.89 v0.90 ... | v9.0.0 - --v--^----^--v---^------^--v- ---v----^----^--- 2015 - | | | | - stable giant | | hammer - release v0.87 | | v0.94 - | | - point firefly dumpling - release v0.80.8 v0.67.12 - - -Four times a year, the development roadmap is discussed online during -the `Ceph Developer Summit `_. A -new stable release (hammer, infernalis, jewel ...) is published at the same -frequency. Every other release (firefly, hammer, jewel...) is a `Long Term -Stable (LTS) <../../releases>`_. See `Understanding the release cycle -<../../releases#understanding-the-release-cycle>`_ for more information. +development | | +release | +v19.0.0 ... | +v20.0.0 + --v--^----^--v---^------^--v- ---v----^----^--- 2024 + | +| | | + stable reef | +| squid + release v18.2.0 | +| v19.2.0 + | +| + point quincy reef + release v17.2.7 v18.2.1 + + +A new stable release (quincy, reef, squid ...) is published at the same +frequency. Every release (quincy, reef, squid...) is a `Long Term Stable (LTS) +<../../releases>`_. See `Understanding the release cycle + Merging bug fixes or features ============================= -The development branch is ``master`` and the workflow followed by all +The development branch is ``main`` and the workflow followed by all developers can be summarized as follows: * The developer prepares a series of commits @@ -88,7 +93,7 @@ Resolving bug reports and implementing features =============================================== All bug reports and feature requests are in the `issue tracker -`_ and the workflow can be summarized as +`_ and the workflow can be summarized as follows: * The reporter creates the issue with priority ``Normal`` @@ -110,7 +115,7 @@ status of an open issue can be: For each ``Pending Backport`` issue, there exists at least one issue in the ``Backport`` tracker to record the work done to cherry pick the necessary -commits from the master branch to the target stable branch. See `the backporter +commits from the main branch to the target stable branch. See `the backporter manual `_ for more information. @@ -119,40 +124,40 @@ Running and interpreting teuthology integration tests ===================================================== The :doc:`/dev/sepia` runs `teuthology -`_ integration tests `on a regular basis `_ and the -results are posted on `pulpito `_ and the +`_ integration tests `on a regular basis `_ and the +results are posted on `pulpito `_ and the `ceph-qa mailing list `_. * The job failures are `analyzed by quality engineers and developers - `_ + `_ * If the cause is environmental (e.g. network connectivity), an issue is created in the `sepia lab project - `_ + `_ * If the bug is known, a pulpito URL to the failed job is added to the issue * If the bug is new, an issue is created The ``quality engineer`` is either a developer or a member of the QE team. There is at least one integration test suite per project: -* `rgw `_ suite -* `CephFS `_ suite -* `rados `_ suite -* `rbd `_ suite +* `rgw `_ suite +* `CephFS `_ suite +* `rados `_ suite +* `rbd `_ suite and many others such as -* `upgrade `_ suites -* `power-cyle `_ suite +* `upgrade `_ suites +* `power-cycle `_ suite * ... Preparing a new release ======================= A release is prepared in a dedicated branch, different from the -``master`` branch. +``main`` branch. -* For a stable releases it is the branch matching the release code - name (dumpling, firefly, etc.) +* For a stable release it is the branch matching the release code + name (quincy, reef, etc.) * For a development release it is the ``next`` branch The workflow expected of all developers to stabilize the release @@ -160,7 +165,7 @@ candidate is the same as the normal development workflow with the following differences: * The pull requests must target the stable branch or next instead of - master + main * The reviewer rejects pull requests that are not bug fixes * The ``Backport`` issues matching a teuthology test failure and set with priority ``Urgent`` must be fixed before the release @@ -184,13 +189,12 @@ more. When a stable release is to be retired, it may be safer to recommend an upgrade to the next LTS release instead of proposing a new point release to fix a problem. For instance, the -``dumpling`` v0.67.11 release has bugs related to backfilling which have -been fixed in ``firefly`` v0.80.x. A backport fixing these backfilling -bugs has been tested in the draft point release ``dumpling`` v0.67.12 but -they are large enough to introduce a risk of regression. As ``dumpling`` +``quincy`` v17.2.6 release might have bugs related to backfilling which have +been fixed in ``reef`` v18.2.x. A backport fixing these backfilling +bugs might be tested in a draft point release but if +they are large enough to introduce a risk of regression and the older release is to be retired, users suffering from this bug can -upgrade to ``firefly`` to fix it. Unless users manifest themselves and ask -for ``dumpling`` v0.67.12, this draft release may never be published. +upgrade to the newer LTS to fix it. * The ``Ceph lead`` decides a new stable release must be published * The ``release master`` gets approval from all leads @@ -208,13 +212,11 @@ for ``dumpling`` v0.67.12, this draft release may never be published. The person responsible for each role is: -* Sage Weil is the ``Ceph lead`` -* Sage Weil is the ``release master`` for major stable releases - (``firefly`` 0.80, ``hammer`` 0.94 etc.) -* Loic Dachary is the ``release master`` for stable point releases - (``firefly`` 0.80.10, ``hammer`` 0.94.1 etc.) -* Yuri Weinstein is the ``quality engineer`` -* Alfredo Deza is the ``publisher`` +* The ``Ceph lead`` oversees the overall release schedule. +* The ``release master`` for major stable releases ensures releases meet criteria. +* The ``release master`` for stable point releases coordinates backports and publishes point releases. +* The ``quality engineer`` verifies integration tests. +* The ``publisher`` publishes the release artifacts. Cutting a new development release ================================= @@ -223,12 +225,27 @@ The publication workflow of a development release is the same as preparing a new release and cutting it, with the following differences: -* The ``next`` branch is reset to the tip of ``master`` after +* The ``next`` branch is reset to the tip of ``main`` after publication * The ``quality engineer`` is not required to run additional tests, the ``release master`` directly informs the ``publisher`` that the release is ready to be published. +Automated Backport Auditing +=========================== + +All backport PRs undergo automated hygiene checks by the `releng-audit` GitHub Actions CI to ensure they meet strict quality standards. These checks include: + +* **Commit Parity:** Ensuring all commits in the backport accurately map to the original ``main`` PRs. +* **Conflict Simulation:** Dry-running cherry-picks to detect undocumented conflicts or deviations. +* **Redmine Linkage:** Verifying accurate tracking and release targeting in the issue tracker. + +Passing these checks automatically applies the ``releng-audit-pass`` label, which is required for the PR to be merged. If the audit fails, the ``releng-audit-fail`` label is applied and an "anti-spam" push shield halts further automated checks. After fixing the issues, you must manually remove the ``releng-audit-fail`` label or comment ``/audit retest`` to trigger a new audit. + +If the CI correctly flags a valid deviation (e.g., a complex but necessary manual conflict resolution), you can request an override. Ping the Component Lead or the ``#ceph-upstream-releases`` Slack channel to request a review. Any user with "maintain" or "admin" rights on the repository, or a member of the ``@ceph/ceph-release-manager`` team, can bypass the check by commenting ``/audit override`` or manually applying the ``releng-audit-override`` label. + +For the granular technical requirements and rules, see the `Cherry-picking rules `_ in the backporter manual. + Publishing point releases and backporting ========================================= diff --git a/src/script/ptl-tool.py b/src/script/ptl-tool.py index 5330a92ca55d..fc7509799ae0 100755 --- a/src/script/ptl-tool.py +++ b/src/script/ptl-tool.py @@ -436,11 +436,11 @@ def format_parity_row_md(left_sha, left_msg, right_sha, right_msg, is_missing=Fa if is_extra: right_col = "**<<<< EXTRA IN BACKPORT >>>>**" else: - right_prefix_clean = right_prefix.strip() - prefix = f"**{right_prefix_clean}** " if right_prefix_clean else "" - right_col = f"{prefix}{right_sha} {right_msg}" + right_col = f"{right_sha} {right_msg}" - return f"| {left_col} | {right_col} |" + source_pr = f"**{right_prefix.strip()}**" if right_prefix.strip() else "" + + return f"| {left_col} | {source_pr} | {right_col} |" def make_pipe(content, fds_to_close, threads): """ @@ -693,8 +693,8 @@ class CommitParityCheck(BaseAuditCheck): visualizer_lines.append("COMMIT PARITY VISUALIZER") visualizer_lines.append("=" * 80) - visualizer_md_lines.append(f"| BACKPORT PR #{pr} | SOURCE PR / STATUS |") - visualizer_md_lines.append("|---|---|") + visualizer_md_lines.append(f"| BACKPORT PR #{pr} | SOURCE PR | SOURCE STATUS |") + visualizer_md_lines.append("|---|---|---|") bp_to_source = {} for pr_name, commit_list in pr_mapping.items(): -- 2.47.3