Treeherder support for Autoland

RESOLVED WONTFIX

Status

MozReview
Autoland
RESOLVED WONTFIX
4 years ago
2 years ago

People

(Reporter: jgriffin, Unassigned)

Tracking

Details

(Whiteboard: [autoland M3])

(Reporter)

Description

4 years ago
Will is going to take a look at the scope of work needed for Treeherder support for Autoland.  Dan, can you comment on what's needed specifically?  From the wiki:

5 - When Autoland service gets results for the retriggered jobs:
5a - If all retriggered jobs pass, it lands the patch in-tree (m-i by default, by developer can request another tree using some TBD try syntax)
5b - If patch no longer cleanly applies, Autoland rejects with a message on the bug
5c - If the retriggered jobs fail, Autoland will annotate the push in Treeherder

6 - Sheriffs periodically monitor the Autoland queue in Treeherder for jobs that need human inspection, and flag them as ok to land or not ok to land, possibly after additional manual retriggers.
6a - If ok to land, autoland lands the patch in-tree. We may coalesce pending autoland commits and land them in batches periodically, like once per hour, ala Bumper Bot.
6b - If not ok to land, Autoland notifies the developer of the problem via bugzilla.

It looks like we need:
* a way to annotate changesets in Treeherder
* a separate view for Autoland that displays only pushes that are annotated with a specific flag
* some UI buttons for 'approve' and 'reject' on the Autoland view, restricted to users with elevated privileges

Please confirm or correct.
(Reporter)

Updated

4 years ago
Flags: needinfo?(dminor)

Comment 1

4 years ago
Is the plan to have the autoland UI integrated as part of treeherder, or separate? I would have thought the latter, but comment 0 implies the former. Using treeherder to store the push pass/fail/... states sounds sensible - when implementing that it would be good to bear in mind other possible (semi-automatic) annotations that may be used in the future for !try repos - eg the "ready to merge" or "push is busted since contains failures classified with 'fixed-by-commit'" we've talked about for making it easier to determine last green changeset to merge (which becomes more important once we start increasing coalescing).

Comment 2

4 years ago
(In reply to Ed Morley [:edmorley] from comment #1)
> Is the plan to have the autoland UI integrated as part of treeherder, or
> separate? I would have thought the latter, but comment 0 implies the former.
> Using treeherder to store the push pass/fail/... states sounds sensible -
> when implementing that it would be good to bear in mind other possible
> (semi-automatic) annotations that may be used in the future for !try repos -
> eg the "ready to merge" or "push is busted since contains failures
> classified with 'fixed-by-commit'" we've talked about for making it easier
> to determine last green changeset to merge (which becomes more important
> once we start increasing coalescing).

Currently I'm using the buildapi to get pass/fail states for autoland test jobs, mostly because I'm already using that to retrigger failed jobs, so it ties nicely together.

Based upon our meeting the other day, it sounds like we also want a UI for sheriffs to monitor autoland jobs for cases that require manual intervention. I think we need to determine the use cases for sheriff intervention in an autoland job before we go any further here.

Based on the above:
* show list of pending autoland requests (i.e. have --autoland in their try syntax)
* cancel autoland request
** I think our policy will be that if any jobs are cancelled for an autoland request, the autoland request itself will be cancelled
* approve autoland request
** What would be the usecase here -- land an autoland request even though there are test failures?

Ed, Ryan, anything to add?
Flags: needinfo?(dminor)

Updated

4 years ago
Flags: needinfo?(ryanvm)
(Reporter)

Comment 3

4 years ago
The UI that I've discussed with Will and Jeads would provide a separate "Autoland" checkbox under the "try" category in repos.  Selecting this would show a view exactly like the regular try repo, except that it will filter on pushes that have been annotated by the Autoland service as "autoland-sheriff", or something similar.

This view will contain "Approve" and "Reject" buttons, available only to users with elevated privileges.  Clicking either button will cause that push to be de-annotated, and thus disappear from the Autoland view.  (It will also cause the Autoland service to take the selected action on a mechanism TBD.)
(In reply to Jonathan Griffin (:jgriffin) from comment #3)
> The UI that I've discussed with Will and Jeads would provide a separate
> "Autoland" checkbox under the "try" category in repos.  Selecting this would
> show a view exactly like the regular try repo, except that it will filter on
> pushes that have been annotated by the Autoland service as
> "autoland-sheriff", or something similar.
> 
> This view will contain "Approve" and "Reject" buttons, available only to
> users with elevated privileges.  Clicking either button will cause that push
> to be de-annotated, and thus disappear from the Autoland view.  (It will
> also cause the Autoland service to take the selected action on a mechanism
> TBD.)

Clarification: The current intention, is that pushes will only appear in this view if retriggered jobs fail (i.e. two oranges in a row for a job in a push), which implies that some human judgement is needed. If a job is green (or all retriggered jobs pass), then the autoland service will just land the push directly.

I don't think it should be *too* difficult to change this criteria if so desired.
Comments 3 and 4 are in line with my recollection as well. In the end, it's basically the same process as how we currently manually verify Try pushes before landing a checkin-needed, just with a substantially better UI and workflow attached to it :)
Flags: needinfo?(ryanvm)
(Reporter)

Updated

4 years ago
Whiteboard: [treeherder M2]
(Reporter)

Updated

4 years ago
Whiteboard: [treeherder M2] → [autoland M2]

Comment 6

3 years ago
Moving to M3, as far as I know, we're not planning to do anything treeherder related for autoland in 2015Q1.
Whiteboard: [autoland M2] → [autoland M3]

Comment 7

3 years ago
Now that we are building an UI for Autoland as part of MozReview I'm not certain it makes sense to have a separate UI inside of Treeherder.

For the current workflow (mozreview -> try) I think it is reasonable to have whomever requested the landing monitor the results inside of MozReview and retrigger things if something went wrong rather than further burdening the sheriffs with this.

A future workflow would be mozreview -> inbound, but in this case the job will be visible to the sheriffs as part of their current workflow, so again I don't see a need for a separate UI.

Any thoughts on this?
(In reply to Dan Minor [:dminor] from comment #7)
> Now that we are building an UI for Autoland as part of MozReview I'm not
> certain it makes sense to have a separate UI inside of Treeherder.
> 
> For the current workflow (mozreview -> try) I think it is reasonable to have
> whomever requested the landing monitor the results inside of MozReview and
> retrigger things if something went wrong rather than further burdening the
> sheriffs with this.
> 
> A future workflow would be mozreview -> inbound, but in this case the job
> will be visible to the sheriffs as part of their current workflow, so again
> I don't see a need for a separate UI.
> 
> Any thoughts on this?

My understanding was that the future workflow would be mozreview -> try -> inbound if that matters.
Can we wontfix this given the mozreview->autoland integration?
Flags: needinfo?(jgriffin)
(Reporter)

Comment 10

3 years ago
(In reply to Mauro Doglio [:mdoglio] from comment #9)
> Can we wontfix this given the mozreview->autoland integration?

Yep.  If we need something in the future, we can file a separate bug.
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Flags: needinfo?(jgriffin)
Resolution: --- → WONTFIX
(Assignee)

Updated

2 years ago
Product: Tree Management → MozReview
You need to log in before you can comment on or make changes to this bug.