Implement pull request close events
There is currently a feature request associated with being able to run some sort of pipeline at the termination of some git event; specifically in this case a "closure" of a pull request.
This allows the consumers of drone to use drone to spin up ephemeral environments for the purpose of testing, and then later have them torn down automatically by Drone once the pull request has been closed.
This commit implements such a solution after reading the guidance on:
#2884
The commits describe the design path in further detail.
This worked :D I've patched our drone server with it for now to see if it breaks things
trigger:
event:
include:
- pull_request
action:
include:
- closed
Which worked with
- Creating the PR
- Updating the PR
- Closing the PR
- Merging the PR
I would expect the above yaml to only work with pull request closed actions, and to ignore sync and create actions. If you want to trigger on create, sync and closed events you need to enumerate all action types.
trigger:
event:
- pull_request
action:
- closed
- open
- sync
I was unsure from the comment exactly how this behaved, but just wanted to make sure we were on the same page.
I would expect the above yaml to only work with pull request closed actions, and to ignore sync and create actions. If you want to trigger on create, sync and closed events you need to enumerate all action types.
Sorry yes, I should have made that clear -- that's how it works presently. Rather, the initial comment:
Creating the PR Updating the PR Closing the PR Merging the PR
Meant that those were the behaviors that I tested across both pipelines in that yaml.
There's a regression in this patch -- it somehow blocks pipelines that only include:
trigger:
ref:
include:
- refs/tags/**
- refs/heads/development
I do not know why yet; I dropped the commit off our server.
(Edited as it was large) Testing was successfully completed based on: https://gist.github.com/andrewhowdencom/4da6514c2d01ada0244b2aca9949dd17
Pull request is also now up to date with revised solution (See a6effe3). Will leave patched on our server for now to see how it goes, but beyond that it should work fine.
thanks for providing the examples. the logic needs to be slightly adjusted to avoid breaking the current excludes behavior. Basically if the action is closed, the pipeline should only execute when the following evaluates to true:
if len(when.actions.includes) > 0 && when.actions.Includes("closed")
finally, we should add some unit tests to confirm the above logic: https://github.com/drone/drone/blob/master/trigger/skip_test.go
Re. Unit tests I agree but also have no time at the minute;
Re. Logic I'm not sure I 100% understand. Do you mean, given a pipeline that has:
trigger:
event:
- pull_request
action:
exclude:
- opened
It should not listen for the "closed" event? That is, practically it would mean:
trigger:
event:
- pull_request
action:
exclude:
- opened
- closed
(Both of these changes are fine but will need to wait for another day)
the below example should not create a pipeline for a pipeline closed event. There may be teams already using the below syntax (for synchronization actions) and they will not expect this syntax to process a pull request closed event. We need to ensure this change does not break existing configurations.
trigger:
event:
- pull_request
action:
exclude:
- opened
Sounds good! That makes sense. I will tackle that as soon as I can.
I had another go, but haven't tested it by actually releasing it yet. I wrote some tests for it based on the yaml I had before.
For now, I need sleep :sleeping: but will check this in a bit.
Edit: I Feel like there's a bug with excludes there. I can't see it though -- too tired I guess.
It's here somewhere:
// If there are still conditions but they do not match this action, the pipeline should be skipped. return true
I.e. implicit allow by lack of exclude. If you only exclude synchronize, then opened should still work.
More edit:
Found it. Probably a way to rework that logic, but that's another day's problem.
status update: This sat on the build server for a week and didn't break anything!
This would be a very helpful feature to have!
We desperately need this.
The conflicts on this are non-trivial iirc, and I have limited time to resolve them. When we next upgrade our build server we'd look at it, but there's no massive reason to do that at the minute so it might be a while.
If someone wants to carry this, please feel free ♥
Revised; not really tested yet though. Someone should consider carrying this. For now, I deployed it on an internal CI server.