Bug #51224
openceph-backport.sh: a PR for N needs a staged PR for N+1
0%
Description
ceph-backports.sh must refuse to create a backport for octopus if the matching backport for pacific is not merged staged yet (with a --force option because there presumably are exceptions).
It is always better to stage, test , merge and release a backport in release N+1 before doing the same for release N. However, people and processes in place for each stage are different. Someone willing to help with the backports by reading the backport submission documentation may not fully understand the need for this ordering.
If such a person tries to stage a backport for N and is shown the message: "this was not merged staged in N+1, please work on that before (or use --force if you know what you're doing)", they will hopefully redirect their efforts to stage a backport for N+1, therefore helping with the ordering of backports.
Files
Updated by Loïc Dachary almost 3 years ago
- Blocked by Feature #51222: ceph-backport.sh: minimal automated test added
Updated by Loïc Dachary almost 3 years ago
It will be easier on the person doing the backport if the check is done on the existence of a matching pull request for N+1 instead of the fact that it is merged. Neha suggested that. And Yuri has no objection at this point in time.
Updated by Ilya Dryomov over 2 years ago
- Subject changed from ceph-backport.sh: a PR for N needs a merged PR for N+1 to ceph-backport.sh: a PR for N needs a staged PR for N+1
Updating the title as requiring that N+1 PR is merged rather than staged is definitely not something we want to do.
Updated by Loïc Dachary over 2 years ago
- File 0001-scripts-ceph-backport.sh-bump-version-to-18.patch 0001-scripts-ceph-backport.sh-bump-version-to-18.patch added
- File 0002-scripts-ceph-backport.sh-init-active_milestones-in-a.patch 0002-scripts-ceph-backport.sh-init-active_milestones-in-a.patch added
- File 0003-scripts-ceph-backport.sh-allow-override-of-milestone.patch 0003-scripts-ceph-backport.sh-allow-override-of-milestone.patch added
- File 0004-scripts-ceph-backport.sh-verify-backup-ordering.patch added
- Status changed from New to Fix Under Review
The patches attached are tested (see the corresponding CI job) with the most meaningful test at test_vet_backport_ordering.
It should be tested manually with GitHub credentials by trying to submit a backport for octopus when the corresponding backport for pacific does not exist yet and verifying it fails with a sensible error message.
Note that it requires https://tracker.ceph.com/issues/51222 to be merged first.
Updated by Loïc Dachary over 2 years ago
@Nathan Cutler I'd appreciate if you have the opportunity to manually test this with the real GitHub before a PR is created. The CI against Gitea is reassuring but the devil is in the details :-)
Updated by Loïc Dachary over 2 years ago
- File deleted (
0004-scripts-ceph-backport.sh-verify-backup-ordering.patch)
Updated by Loïc Dachary over 2 years ago
The commit for the version number bump is incorrect. As explained by @Nathan Cutler on IRC:
The ceph-backport.sh version number is incremented by running "src/script/ceph-backport.sh --update-version" and including the results of that as a commit in the PR (as I am doing in the PR you mentioned). The version number is x.y.z.c where x.y.z is the Ceph version number and c is the number of commits that have been added since the Ceph version number was last incremented. The code for --update-version can be viewed in the update_version_number_and_exit function of ceph-backport.sh. While the ceph-backport.sh version number is based on the Ceph version number, there is no effort to keep it up-to-date with the Ceph version number: it is only only updated when the ceph-backport.sh code is changed. The beauty of the ceph-backport.sh version number becomes apparent when someone reports problems with ceph-backoprt.sh. I ask them to run "ceph-backport.sh --version" and tell me what it says, and at one glance I know how old the ceph-backport.sh code is that they are running.
Updated by Nathan Cutler over 2 years ago
@Loïc Dachary: You don't necessarily need to include a version number bump patch in this patchset. It will probably be easier, assuming I'll be the one who opens the GitHub PR, for me to create that commit.
Updated by Loïc Dachary over 2 years ago
I'll send them to you via email without the version commit then!