Update stable branch workflows

Development RFCs
1

It’s time to revisit how we deal with cherry picks for Foreman & Katello stable branches. The primary aim is to evaluate the current processes and see if we can align both projects on the same workflow while also fixing other issues we see.

The opening post will contain the factual statements about the current flow. All proposals should be added in comments.

What is cherry picking?

A quick overview. Normally development happens in a single branch while there’s also a stable for each X.Y version. Cherry picking is the process of taking fixes from that development branch to the appropriate X.Y branches.

The process then roughly comes down to deciding if the fix is appropriate for the X.Y version. Some factors that can influence this:

  • Is the bug present in X.Y?
  • Is the bug critical enough to warrant a cherry pick?
  • Does the fix in develop work on X.Y?
  • Won’t the fix introduce other problems like regressions, behavior changes, require newer dependencies?

There can be more. Sadly this is a bit of a fuzzy area where you sometimes need experience to judge this. Because of that many maintainers err on the side of caution and avoid picking changes. Mostly because the last item (xkcd: Workflow).

Current status

Foreman and Katello have their own workflows, so they’re described separate. This is not desirable, but it is the status quo.

We are also dependent on Redmine for the processes, but Discussion on Future of Issue tracking in Foreman was opened about this.

To update Redmine we have automation to add the unreleased version for the target git branch to the Fixed in releases field when a PR is merged. How it exactly works can be seen in the source. A practical example for the Foreman repository: when merging to develop it adds 3.8.0 to the Fixed in releases field. If you cherry pick to 3.7.0-stable, it adds 3.7.1 because those are the next versions to be released.

Foreman

Applies to:

Conventions:

  • Development branch: develop
  • Stable branch: X.Y-stable

Process:

Last time we talked we reached a conclusion.

The short summary is that in a weekly meeting the core team goes over merged PRs to decide what should be cherry picked. Explicit suggestions can be done using the Needs cherrypick label.

It should be noted that attendance on the core maintainers call is low to the point that @MariaAga suggested to stop hosting it.

In addition to picking what’s already merged there is also the case that there may be a blocking bug for a release when a fix may not yet have been written (or merged). Today we can use the Target version field in Redmine. This is a single value item, so that’s not great but we can write a query to find issues where target version is x.y.z and fixed in releases field doesn’t contain x.y.z.

Katello

I’m actually not entirely familiar with this, so I’m going to ask Katello maintainers to expand on this.

Applies to:

Conventions:

  • Development branch: master
  • Stable branch: KATELLO-X.Y

Process:

3 Likes
2

How cherry picks are done

Cherry picks are done using git cherry-pick -x on a commit or a series of commits. The -x argument generates a comment mentioning the original commit. Taking Fixes #36450 - Respect old/new host details in log/vmrc · theforeman/foreman@a777b60 · GitHub as an example you can see the last line. So you know it was generated using git cherry-pick -x 051103759805fe7f952a48c9f45c4ac081c986c5.

The cherry-pick command creates a new commit for each commit and applies it on the current branch (provided no conflicts show up). So you can either create a new branch, submit that as a PR or commit it straight to the stable branch.

How we do this exactly differs: sometimes it’s pushed straight, sometimes a new PR for a single pick is created and sometimes we create a PR for a series of picks. In practice it doesn’t really matter when everything is well, but splitting can have the benefit that your CI can better catch where it exactly went wrong.

Proposal to use patchback

This tool is developed by Ansible and used by Pulp (and probably more):

The idea is that you install the app on the GH org and place a configuration in the repository. Then you apply labels to a PR and the app opens PRs each branch. For example, you add the stable-3.7 and stable-3.6 labels and it’ll open two PRs: one for each branch.

I’d fully expect that all our automation with Redmine would pick this up and not need any modification.

Steps to be taken

Today there are 2 limitations, although the first only applies to Foreman and not Katello. It can’t deal with branch suffixes. So KATELLO-X.Y works fine, X.Y-stable doesn’t:

https://github.com/sanitizers/patchback-github-app/issues/36

Then it also can’t deal with the rebase “merge” strategy that we always use:

https://github.com/sanitizers/patchback-github-app/issues/35

This is mostly a side effect of the GH webhook and API making it hard.

Both issues are probably not hard, but I’d first like to get agreement that this is a good direction.

Future improvements: detecting the target branches

Today we rely on humans to decide where to pick to. Now it means a human needs to make the right PRs. With Patchback it would be reduced to applying the correct labels. We have more information that we could use.

For example, in Redmine we already know if something is a bug or a feature. Generally we only pick bugs. If the Found in releases field is filled in we can automatically suggest the branch labels based on Redmine information.

Another more automated approach would be to use Fixes. In Submitting patches: the essential guide to getting your code into the kernel — The Linux Kernel documentation the process for the Linux kernel is used. It is trivial to parse the Fixes: field from a commit message. For example, from 557b9c795101b858aab3877af6c30a23f97f963b:

$ git log --format="%(trailers:key=Fixes,valueonly,separator=' ')" 557b9c795101b858aab3877af6c30a23f97f963b -1 | cut -d' ' -f1
6fcadb288d2ac6b81207b42facc4877572bac61f

Now finding out which releases contained that is quite easy: git tag --contains 6fcadb288d2ac6b81207b42facc4877572bac61f will tell you it was introduced in 2.2.0.

It does require commit authors to include the metadata, but it can be very useful for these cases.

Automation could use this information (both Redmine and commit metadata) to suggest labels (taking supported branches into account).

2 Likes
3

Actually I was going to propose similar stuff. Although, I wasn’t aware of the tool you’re suggesting. I was thinking about improving our GH bot or write another one to create a CP from a commit. The workflow for changes from a PR which was merged are needed in a stable branch would be:

  1. Write a comment on merged PR, something like: [CP 3.6-stable][CP 3.5-stable].
  2. The bot would create a PR for each branch with the CP, possibly applying labels, so one doesn’t need to scroll to the bottom of PR’s discussion.
  3. Someone (maybe author of the PR or assigned reviewer) checks if the CP is okay and merges it.

Essentially it’s the same as your suggestion, but we would not rely on a third-party stuff we would need to update per our needs. Downside is to find a person to update/write the bot.

So, anyway, big +1 for anything which is automated around cherry-picking.

4

Notes about Katello:

We have a weekly triage meeting where we set the target version for every issue in a release.

We don’t pay much attention to the “Fixed In Version” value. In the past we had a TODO column like Foreman, but none of us Katello devs use it so it was neglected. We rely on tool_belt to tell us what needs cherry-picking (and for changelog creation+amendment too).

For cherry-picking, we cherry-pick at release time only. When it comes time to cherry pick, we use tool_belt. Assuming that the Katello version configuration has the proper “prior releases”, cherry-picks for a version or any older version that are in the prior releases list should show up with the ./tools.rb cherry-picks configs/katello/x.y.yaml command.

If the cherry-picks are clean, as in no conflicts, then the command output from tool_belt can be copy and pasted to perform all the necessary cherry-picks at once.

5

The bot should set this automatically when it’s merged. However, since the process is to do the picking in bulk you won’t see the field updated until the PR is actually merged.

So I went and looked. If we take [CP] 4.9.0 ga by wbclark · Pull Request #10623 · Katello/katello · GitHub as an example then we can get an overview of the issues in that PR by going to the PR Processor checks: [CP] 4.9.0 ga by wbclark · Pull Request #10623 · Katello/katello · GitHub

If I open the issues then none of them have the correct Fixed in releases field, but they have the correct development version. Something else must be wrong.

The first thing that comes to my mind is that bot can’t find an open Redmine version for the KATELLO-4.9 branch. One possibility is that it doesn’t map the branch, the other is that the Redmine version 4.9.0 (Katello 4.9.0 - Katello - Foreman) was closed before the PR was merged (because the release must be open to set Fixed in releases). I suspect in this case it’s actually the former:

There’s no code to handle a KATELLO- prefix. Untested, but I think this should work:

https://github.com/theforeman/prprocessor/pull/176

Then as long as the Redmine version is open before you merge, it should give you correct Fixed in releases for free with your current workflow.

This is why it’s important to write down processes: you can find out something you assumed worked automatically for ages has never been implemented.

6

And I just realized: the release notes will then also be incomplete. This part here filters on the Fixed in releases field, which I assumed to be correct.

We probably haven’t noticed this because there are no patch releases on Foreman 3.6 and Katello 4.8 Release Notes and Foreman 3.5 and Katello 4.7 Release Notes which was absolutely the goal for those pages. And nobody is going to notice a list is not complete, if it’s already large.