[mergify] backport automation to 7.x, 7.12 and 7.11 #24608

v1v · 2021-03-17T17:07:55Z

What does this PR do?

Enable backports with mergify.

Why is it important?

Mergify provides different mechanism to interact with PRs and automate certain process.

Use cases

We enabled it for the https://github.com/elastic/apm-pipeline-library/ project for three different automations:

Automatically merge to master ( if >2 approvals, CLA is ✅ , label=ready-to-merge and CI ✅ )
Automatically backport to 7.x (if origin=master, label=backport-to-7.x)
Merge backports ( if CLA is ✅ and CI ✅)

Some examples

Backports to 7.x -> elastic/apm-pipeline-library#995
Merges to master -> elastic/apm-pipeline-library#994

Pros

It does not require any credentials.
It works out of the box.
It provides GitHub comments.
Extend functionality with more automation.
infosec team approved.

Cons

It's a GitHub APP so potentially the service could be deprecated.
- [mitigation] -> Nothing we can do about it.
It does not use the .backportrc.json
- [mitigation] -> .backportrc.jsonis automatically modified when new branches are created -> https://github.com/elastic/beats/pull/24098 . Therefore.mergify.yml` could be also modified accordingly.
Backport are not assigned to the original contributor.
- [mitigation] -> To explore if there is kind of way to do so.
Only free for open repos. See comment [mergify] backport automation to 7.x, 7.12 and 7.11 #24608 (comment)
- [mitigation] -> To explore if private repos are a thing and if so whether the payment plan is sufficient.

Issues

Duplicates [Github Action] Add backport workflow #24576
There is an initiative to automate the GitHub labels creation in each obs11 repo -> [GH][LABELS] Automation apm-pipeline-library#886

Actions

Ask Infra to enable the mergify app in this project.
Agree with the Beats team
Document the process. I documented the process in the internal repositories instead of https://github.com/elastic/beats/tree/master/docs/devguide (thanks @cachedout)

v1v · 2021-03-17T17:15:46Z

.mergify.yml

+  - name: backport patches to 7.x branch
+    conditions:
+      - base=master
+      - label=backport-to-7.x


Labels to be agreed!

I think we should use the existing labels like v7.13.0 so people don't have to duplicate them.

Yes, I added a comment to remember we need to set the labels accordingly.

For the record, we have a WiP automation to apply the same labels across all the observability repositories, therefore it will be quite simple to apply a new label everywhere.

I think we should use the existing labels like v7.13.0 so people don't have to duplicate them.

We backport too often after FF, sometimes right before release time leaving us with fixes that didn't make it into the release. From past experience we've been notoriously bad at telling which exact a fix/features is in. I think I would prefer to tell the target branch instead of the exact version.

v1v · 2021-03-17T17:15:53Z

.mergify.yml

+  - name: backport patches to 7.12 branch
+    conditions:
+      - base=master
+      - label=backport-to-7.12


Labels to be agreed

v1v · 2021-03-17T17:16:01Z

.mergify.yml

+  - name: backport patches to 7.11 branch
+    conditions:
+      - base=master
+      - label=backport-to-7.11


Labels to be agreed!

elasticmachine · 2021-03-17T17:24:06Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Build Cause: Pull request #24608 updated
Start Time: 2021-04-07T15:21:08.060+0000
Duration: 19 min 0 sec
Commit: ff1eaca

Trends 🧪

❕ Flaky test report

No test was executed to be analysed.

cachedout

I know this is a backport (sorry!) and I should have commented on the original, but I didn't realize until now -- did the original come with any developer documentation?

It might be good to find a home for this somewhere in here: https://github.com/elastic/beats/tree/master/docs/devguide

ruflin · 2021-03-18T09:28:31Z

One thing that becomes more and more important from my perspective is that we have consistency across repositories. There are quite a few engineers that work on beats, kibana, integrations and other repositories. Having to use different tools for each repository is a hurdle. The same is the case for the labels, they have to be consistent.

Kibana uses the backport tool today and it seems to work well. So I would like to understand why we can't do the same here as Kibana.

v1v · 2021-03-18T09:50:09Z

In order to help with the rationale about this PR, let me highlight what happened for having two different PRs for a similar problem:

[Github Action] Add backport workflow #24576 was raised.
there was a conversation regarding the implications for using GitHub actions with credentials.
productivity team gathered to discuss what to do.
a) for clarity this requirement was already tracked in our TODO list but no yet prioritised.
b) this requirement could be related to the merge-queue proposal for the beats project, that was intentionally postponed until 7.15, when the changelog could be improved.
We came up with this proposal and explained the PROS/CONS.

In a nutshell:

in both PRs the aim is exactly the same for the end user.
- add a GitHub label and the the automation will happen.
the process will remain the same from the user point of view.
the only differences are the ones related to the implementation details.

Regarding the GitHub labels:

[GH][LABELS] Automation apm-pipeline-library#886 is the initiative that was started to align the same labels across all the observability repositories.
the release beats project has got some automation to bump the version in the .backportrc.json, therefore the same automation could be used to this particular implementation.
the existing labels in this PR need to be agreed.

ruflin · 2021-03-18T09:57:13Z

in both PRs the aim is exactly the same for the end user

This sounds great. As long as the dev does not need to have specific knowledge if it is in Kibana or Beats, this sounds great. And I think short term it is more important to having A tool than none.

I'm pretty sure the devil / differences are somewhere in the details but we will find out. Lets keep moving on this in case @sqren agrees. My main ask is to make sure labels are aligned so the statement above holds true.

Long term I think we should align tooling.

sorenlouv · 2021-03-18T10:08:54Z

Going the mergify route is good with me. I agree with @ruflin that the user experience should be the same if you contribute to kibana or beats.

I'm curious to hear what happens when the mergify backport fails. How does it delegate the resolution to the end user?
The Github action we use in kibana will let the user know how to run the backport command locally (example):

Running node scripts/backport --pr 94072 will then auto-select the remaining branches and cherry-pick the changes. When the user has resolved the conflicts for each branch the tool will push them and create the backport PRs as usual.

v1v · 2021-03-18T10:21:15Z

I'm curious to hear what happens when the mergify backport fails. How does it delegate the resolution to the end user?

The backport action in mergify creates the PRs with conflicts by default and tag them with a specific label, further details in
https://docs.mergify.io/actions/backport.html

It won't add any notifications to the original PR. But create a PR with the label. See below to illustrate a real example:

elastic/apm-pipeline-library#978 is one example for instance

The Github action we use in kibana will let the user know how to run the backport command locally

mergify supports GitHub comments to interact with the mergify actions, therefore no need to run anything by command line at first. For instance, see the below snippet that I took from the above PR.

More conditions and actions can be found in the documentation.
You can also trigger Mergify actions by commenting on this pull request:
@Mergifyio refresh will re-evaluate the rules
@Mergifyio rebase will rebase this PR on its base branch
@Mergifyio update will merge the base branch into this PR
@Mergifyio backport will backport this PR on branch

Further details in https://docs.mergify.io/commands.html

sorenlouv · 2021-03-18T10:30:06Z

mergify supports GitHub comments to interact with the mergify actions, therefore no need to run anything by command line at first.

The user only need to run the backport tool locally if a conflict occurs. Similar to how the mergify approach requires the user to checkout the branch locally and resolve the conflicts. imo the mergify approach is a little more tedious, especially if there are multiple branches with conflicts (like the example above) but we'll see.

kuisathaverat · 2021-03-18T11:29:46Z

we can add a comment with the instructions to backport the PR with backport locally in case of failure https://docs.mergify.io/examples.html#request-for-action

sorenlouv · 2021-03-18T12:27:44Z

we can add a comment with the instructions to backport the PR with backport locally in case of failure docs.mergify.io/examples.html#request-for-action

That sounds like a good idea 👍

v1v · 2021-03-18T21:11:56Z

.mergify.yml

+    actions:
+        comment:
+          message: |
+            This pull request is now in conflicts. Could you fix it @{{author}}? 🙏


author won't be resolved correctly -> Mergifyio/mergify#1096

See v1v/apm-pipeline-library#19 (comment)

cachedout · 2021-03-22T08:38:22Z

Long term I think we should align tooling.

Just want to mention here that while Mergify is free for public repos (and very good IMHO!), its per-organization pricing may be a barrier if we ever want to roll it out to private repos. If tooling consistency across repos/teams is a concern, we should take this into account.

urso · 2021-03-23T13:26:59Z

The user only need to run the backport tool locally if a conflict occurs. Similar to how the mergify approach requires the user to checkout the branch locally and resolve the conflicts. imo the mergify approach is a little more tedious, especially if there are multiple branches with conflicts (like the example above) but we'll see.

Master and 7.x are mostly in sync. We seldomly have conflicts on the code and often it is a sign of a missing backport. I don't consider this a big problem. Smaller conflicts (e.g. on the changelog) can be fixed via the github UI.
Often git gets the changelog wrong (adding too many entries). This is not a conflicts, but still require the original author to review and fix the backport.

With mergify doing the backport, will the PR be assigned to the original author?

v1v · 2021-03-23T14:22:35Z

With mergify doing the backport, will the PR be assigned to the original author?

No, Backport are not assigned to the original contributor. See my comment in #24608 (comment)

Off the top of my head, an easy workaround could be the GH owners, it's not 100% effective but at leats could help with.

Meanwhile, I'll look it at if we can contribute to the mergify source code to provide that feature

urso · 2021-03-25T16:59:13Z

No, Backport are not assigned to the original contributor. See my comment in #24608 (comment)

Oh, I thaught this is about the author only. As backports still might need manual intervention (changelog), I was just wondering if we can as a fallback just add the original author as 'reviewer' or 'assignee'.
Anyways, the backport should be linked (via the title) to the original PR, so it shouldn't be too hard to find the backport PRs. Don't think it is a blocker.

What is the title of the backport PR? The changelog scripts extract the original PR of a backport based on the PR title.

v1v · 2021-03-25T20:28:58Z

What is the title of the backport PR? The changelog scripts extract the original PR of a backport based on the PR title.

The tile is the original backport's title in addition to (bp#<number>)

A real example, elastic/e2e-testing#935 was the original PR and created:

Update readme with correct service versions and profile (bp #935) e2e-testing#936
Update readme with correct service versions and profile (bp #935) e2e-testing#937
Update readme with correct service versions and profile (bp #935) e2e-testing#938

urso · 2021-03-25T20:52:30Z

The tile is the original backport's title in addition to (bp#)

Thanks. This format will not match our regex, but it should just be a one liner here.

urso · 2021-03-25T21:07:06Z

.mergify.yml

+  - name: backport patches to 7.12 branch
+    conditions:
+      - base=master
+      - label=v7.12.0


How exactly will the labels work?

a. If we are about to release 7.12.0, I might want to use the label v7.12.1, in order to postpone the backport for after the release is out. Will we update the version filter here after a release? If so, will my PR automatically backported when we update the label filter here?

b. Will my PR be backported if I add v7.12.0 even after lets say 7.12.1 was released? (Although it might be a bit confusing to users looking at the PR labels to figure out if a bug fix is available)

I'll answer your questions in a minute, for clarity, the initial proposal was to use the existing labels vmajor.minor.0 (see #24576 (comment))

a. If we are about to release 7.12.0, I might want to use the label v7.12.1, in order to postpone the backport for after the release is out. Will we update the version filter here after a release? If so, will my PR automatically backported when we update the label filter here?

Yes, mergify works for even PRs which were not backported after the merge, as long as the label is added. In addition you can use the github comment = @Mergifyio backport <destination> will backport this PR on <destination> branch

To illustrate the above, I just tagged elastic/apm-pipeline-library#1053 with the explicit label and elastic/apm-pipeline-library#1056 was created. NOTE: for some reason the timestamp for the label does not match with when I did it, (about 7 minutes ago instead of 2 days...)

b. Will my PR be backported if I add v7.12.0 even after lets say 7.12.1 was released? (Although it might be a bit confusing to users looking at the PR labels to figure out if a bug fix is available)

Yes, backports with mergify don't know about releases but branches and labels.

a. If we are about to release 7.12.0, I might want to use the label v7.12.1, in order to postpone the backport for after the release is out. Will we update the version filter here after a release? If so, will my PR automatically backported when we update the label filter here?

if the mergify config is updated with that branch before you merge the PR, the PR would backported.

b. Will my PR be backported if I add v7.12.0 even after lets say 7.12.1 was released? (Although it might be a bit confusing to users looking at the PR labels to figure out if a bug fix is available)

the backport is made on a new PR when you merge the fixed PR, if the labels and mergify configuration are in place when you merge, a new PR is made to every branch that you requested backport with the labels.

There is an alternative way to make the backports manually using a GitHub comment (see https://docs.mergify.io/commands.html#backport)

https://mergify.io/features/backports

Maybe to clarify case a:

mergify only backports if label v7.12.0 is set

I create a PR and merge it

I add the label v7.12.1 to my PR (because of FF I don't want the change be backported yet)

Version 7.12.0 gets released

After 7.12.0 is released we update the mergify configuration to backport if v7.12.0 or v7.12.1 is set.

After step 5, will my old PR be picked up automatically or am I required to remember that I did have a 2 weeks old PR that I must now 'manually' backport by adding a comment to the PR (or remove + add label again).

Case b is a little unfortunate as labels do not necesarrily reflect reality, but well, it's the way it is.

After 7.12.0 is released we update the mergify configuration to backport if v7.12.0 or v7.12.1 is set.

this happens after you merge the PR, mergify does not review old merged PRs, so the backport is not made. In this case, you have to manually backport the PR, either from the PR with a GiHub Comment or as you make now from your laptop.
However, if we detect changes on the mergify configfile can trigger something that reviews all PRs with the new label and make backport mergify comments on those PRs, but this is something not out of the box. We have to see if this use case exists and how many times it happens, then we can evaluate the effort and the value of this process.

AFAIK if you fix something in branch 7.12 that has the version 7.12.0 this will be in the 7.12 branch (the same major.minor) that will contain the version 7.12.1 (different patch), so in the case you put as an example there is no backport.
I can see a case similar of what you explain between minor versions, but in those case you use the branch 7.x and then you backport to 7.12, 7.11, ... in those cases the branch already exists so the backport is made.

To help to unblock this 🧵

Since there is no backport automation in place for this project I see the benefits of enabling it for facilitating the backporting process, but I don't know if it could cause any issues. Although, mergify requires certain discipline regarding what labels should be added.

Do you think if we enable mergify it will help with the backporting? Or is it required to solve all of the above-mentioned use cases?

I don't think either behavior is a blocker and adding mergify is an improvement either way. As we have to document the backport process in our how to guides. I'm trying to figure out how it works and where we have potential gotchas that we should document as well. I did just bring up the two cases as we've seen them from time to time in the past: label does not match actual release + us holding of with backport in order to target the next patch release... cough... kafka ... cough.

v1v · 2021-03-30T14:33:25Z

With mergify doing the backport, will the PR be assigned to the original author?

I contributed with a new feat -> Mergifyio/mergify#2336 (wait for their feedback)

urso · 2021-03-31T11:30:39Z

I contributed with a new feat -> Mergifyio/mergify#2336 (wait for their feedback)

Nice. Let's push this PR here anyways. We can still enhance the experience once the mergify feature is merged.

* upstream/master: (91 commits) [Filebeat] Change okta.target to nested field (elastic#24636) Add RFC5424 format support for syslog input (elastic#23954) Fix links to Beats product pages (elastic#24821) [DOCS] Fix 'make setup' instructions for a new beat (elastic#24944) Remove duplicate decode_xml entry (elastic#24941) [libbeat] Add wineventlog schema to decode_xml processor (elastic#24726) [Elastic Agent] Add check for URL set when cert and cert key. (elastic#24904) feat: stage execution cache (elastic#24780) Fix error in Journalbeat commands (elastic#24880) Add baseline ECS 1.9.0 upgrade (elastic#24909) [Elastic Agent] Cloud container legacy apm files. (elastic#24896) [Elastic Agent]: Reduce allowed socket path length (elastic#24914) Add ability to destroy indices with wildcards in testing (elastic#24915) Add status subcommand to report status of running daemon. (elastic#24856) Fix types of fields GetHits and Ops in Metricbeat module for Couchbase (elastic#23287) Add support for Filestream input in elastic agent. (elastic#24820) Implement k8s secrets provider for Agent (elastic#24789) Sort processor list in docs (elastic#24874) Add support for SCRAM authentication in kafka metricbeat module (elastic#24810) Properly update offset in case of unparasable line (elastic#22685) ...

Mergify commit status checks won't be waiting for but the backport will happen after the PR has been merged as expected but the checks won't be enabled

v1v · 2021-04-07T09:49:59Z

If we agree in our monthly meeting today, then I'll proceed with merging this PR.

sorenlouv · 2021-04-07T10:12:23Z

.mergify.yml

@@ -29,7 +32,7 @@ pull_request_rules:
    actions:
        comment:
          message: |
-            This pull request is now in conflicts. Could you fix it @{{author}}? 🙏
+            This pull request is now in conflicts. Could you fix it? 🙏
            To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/
            ```
            git fetch upstream


Wouldn't it be easier to ask the user to use a tool for this? Either running mergify locally or backport.
In Kibana we help the user like this:

good point. I don't know what's the tool the team have in place for that, in order to be be tool-agnostic the message provides the git commands to solve it locally.

I'll keep a note to iterate on this and make the user experience smooth. Thanks for the feedback :)

Most people use the dev-tools/cherry-pick-pr tool in the Beats repo it seems. But a few contributors use the nodejs backport tool. Both are supported. For the changelog we have a 'requirement' on the PR title to properly link to the original PR, so we can find unique PRs and double check if these features are really released (github tags can be missleading). Both tools are supported by our changelog scripts. Using custom git commands might be a problem if the title is not properly formatted (unless we add another lint check on the PR title for backports).

We can then iterate with the existing tools, the current comment intends to help with fixing the just created backport PR that has got some conflicts, so we might need to think about that particular use case. I'll keep a note for the upcoming iterations

v1v

As agreed we will roll out this automation in two phases:

experimental one by using the backport-vMajor.Minor.0 labels. Therefore, users should use those new tags.
production, so remove the backport-vMajor.Minor.0 labels and use the usual ones.

I'll proceed with an email in the mailing list to explain how this will work.

.mergify.yml

…eline * upstream/master: CI: curl seems to be available but where cannot find it (elastic#24965) [mergify] backport automation to 7.x, 7.12 and 7.11 (elastic#24608) Only show deprecation warnings for CN-based verification once (elastic#24948) CI: use ubuntu-20 (elastic#24963) Debug empty root field instead of error (elastic#24966) Kubernetes_secrets provider improvements (elastic#24912)

[mergify] backport automation to 7.x, 7.12 and 7.11

da361cc

v1v added the automation label Mar 17, 2021

v1v requested review from a team March 17, 2021 17:07

v1v self-assigned this Mar 17, 2021

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Mar 17, 2021

v1v mentioned this pull request Mar 17, 2021

[Github Action] Add backport workflow #24576

Closed

v1v commented Mar 17, 2021

View reviewed changes

mdelapenya added the Team:Automation Label for the Observability productivity team label Mar 17, 2021

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Mar 17, 2021

kuisathaverat approved these changes Mar 18, 2021

View reviewed changes

cachedout approved these changes Mar 18, 2021

View reviewed changes

ruflin mentioned this pull request Mar 18, 2021

Add autobackports with labels to fleet-server repository elastic/fleet-server#140

Closed

v1v added 2 commits March 18, 2021 21:01

Use current labels

edc1caa

Add ask to resolve conflict with the how to

bab54e9

v1v mentioned this pull request Mar 18, 2021

Enable backport automation with mergify elastic/fleet-server#143

Merged

2 tasks

v1v commented Mar 18, 2021

View reviewed changes

v1v added 2 commits March 23, 2021 13:33

cosmetic change to validate the mergify

254b2dd

quote the branches

840d99a

kuisathaverat mentioned this pull request Mar 24, 2021

feat: add support for auto-backports elastic/apm-server#4990

Merged

2 tasks

urso reviewed Mar 25, 2021

View reviewed changes

urso approved these changes Mar 29, 2021

View reviewed changes

v1v added 2 commits April 7, 2021 10:44

[mergify] use the merged condition

bf22c7e

Mergify commit status checks won't be waiting for but the backport will happen after the PR has been merged as expected but the checks won't be enabled

sorenlouv reviewed Apr 7, 2021

View reviewed changes

v1v commented Apr 7, 2021

View reviewed changes

.mergify.yml Outdated Show resolved Hide resolved

.mergify.yml Outdated Show resolved Hide resolved

.mergify.yml Outdated Show resolved Hide resolved

Use backport-vMajor.Minor.0 labels for the experimental phase

ff1eaca

v1v merged commit a2440a6 into elastic:master Apr 7, 2021

v1v deleted the feature/mergify branch April 7, 2021 16:26

v1v mentioned this pull request Jun 25, 2021

[CI] Idea: Merge gating elastic/kibana#103180

Closed

[mergify] backport automation to 7.x, 7.12 and 7.11 #24608

[mergify] backport automation to 7.x, 7.12 and 7.11 #24608

Conversation

v1v commented Mar 17, 2021 • edited Loading

What does this PR do?

Why is it important?

Use cases

Some examples

Pros

Cons

Issues

Actions

Choose a reason for hiding this comment

sorenlouv Mar 18, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Mar 17, 2021 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

Trends 🧪

❕ Flaky test report

cachedout left a comment

Choose a reason for hiding this comment

ruflin commented Mar 18, 2021

v1v commented Mar 18, 2021 • edited Loading

ruflin commented Mar 18, 2021

sorenlouv commented Mar 18, 2021 • edited Loading

v1v commented Mar 18, 2021

sorenlouv commented Mar 18, 2021 • edited Loading

kuisathaverat commented Mar 18, 2021 • edited Loading

sorenlouv commented Mar 18, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cachedout commented Mar 22, 2021

urso commented Mar 23, 2021

v1v commented Mar 23, 2021

urso commented Mar 25, 2021

v1v commented Mar 25, 2021

urso commented Mar 25, 2021

Choose a reason for hiding this comment

v1v Mar 26, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kuisathaverat Mar 26, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

urso Mar 26, 2021 • edited Loading

Choose a reason for hiding this comment

v1v commented Mar 30, 2021

urso commented Mar 31, 2021

v1v commented Apr 7, 2021

sorenlouv Apr 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

v1v left a comment

Choose a reason for hiding this comment

v1v commented Mar 17, 2021 •

edited

Loading

sorenlouv Mar 18, 2021 •

edited

Loading

elasticmachine commented Mar 17, 2021 •

edited by jenkins-beats-ci bot

Loading

v1v commented Mar 18, 2021 •

edited

Loading

sorenlouv commented Mar 18, 2021 •

edited

Loading

sorenlouv commented Mar 18, 2021 •

edited

Loading

kuisathaverat commented Mar 18, 2021 •

edited

Loading

v1v Mar 26, 2021 •

edited

Loading

kuisathaverat Mar 26, 2021 •

edited

Loading

urso Mar 26, 2021 •

edited

Loading

sorenlouv Apr 7, 2021 •

edited

Loading