Status

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: kgrandon, Assigned: kgrandon)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

User Story

Integration branch process
--------------------------------------
First steps for all cases:
1. User submits pull request.
2. Bug receives R+ and checkin-needed keyword.
3. Create the integration branch, based on target branch, if it's not there. 
4. Cherry-pick the commits into the integration branch.

If the cherry-pick fails:
5. Notify the user that we could not merge the commit.
6. Remove the checkin-needed keyword.

Test passes CI:
5. Submit a TC graph for testing.
6. Receive a pulse update that this passes CI.
7. Fast-forward master to the new commit.
8. Close the pull request, and remove checkin-needed.

Test fails CI:
5. Submit a TC graph for testing.
6. Receive a pulse update that this failed CI.
7. Delete the integration branch.
8. Restore the integration branch via brute force if needed.
8.a. If the failed commit matches the tip of the integration branch, no action is necessary.
8.b. Otherwise, create the integration branch.
8.c. Cherry-pick all other commits from pull requests that were in the previous integration branch, running tests for each one.
9. Note that CI failed in the bug, and remve checkin-needed.


Reconstructing the integration branch
--------------------------------------
If autolander goes down for any amount of time we need to be able to reconstruct the integration branch. We need to see if we have enough information from the bugzilla exchange and our azure bug store to reconstruct this. This would mean that we need to query the bugzilla and github API quite a bit on startup. We may be able to reduce this by storing the currently tested pull requests on azure.

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
We should use an integration branch for landing in gaia. The integration branch will ensure that things are tested in order, and landed in the expected order.
(Assignee)

Updated

4 years ago
User Story: (updated)
(Assignee)

Comment 1

4 years ago
Created attachment 8513000 [details] [review]
Pull request

Hey guys - wondering if you could give me some feedback on this patch if you have time. You may want to double check that I'm constructing/scheduling the taskgrah properly. Thanks!
Attachment #8513000 - Flags: feedback?(jopsen)
Attachment #8513000 - Flags: feedback?(jlal)
Attachment #8513000 - Flags: feedback?(garndt)
I added a bunch of comments to the taskcluster specific elements on github.

---
Additionally I would strongly recommend that you replace: lib/pulse.js
with PulseListener from taskcluster-client.
In fact you can do with a single AMQP connection the utilities we have in taskcluster-client is already pretty good for this:

var taskcluster = require('taskcluster-client');
var pulseConn   = new taskcluster.PulseConnection({username: .., password: ...});
var listener1   = new taskcluster.PulseListener({connection: pulseConn, queueName: config.. || undefined});
var listener2   = new taskcluster.PulseListener({connection: pulseConn, queueName: config.. || undefined});
listener2.bind({exchange: ..., routingKeyPattern: ...}); // if you want to bind to a non-taskcluster exchange

Note, if you specify a queueName for PulseListener it'll create an non-exclusive durable queue.
So for testing and local development you don't want to do that, but in your deployment you'll want do this.
Then it's also trivial to scale up and launch two nodes running your code, if one can't keep up.
Comment on attachment 8513000 [details] [review]
Pull request

Definitely address some of the malplaced scopes you added to tasks, and remove your custom route on tasks that you only want to add on the task-graph level.
The treeherder route should still exist on task-level, but should perhaps be configured by parameter substitution.
Attachment #8513000 - Flags: feedback?(jopsen) → feedback-
(Assignee)

Updated

4 years ago
Assignee: nobody → kgrandon
Status: NEW → ASSIGNED
Comment on attachment 8513000 [details] [review]
Pull request

gave it a skim the git logic seems pretty sane looking forward to trying this more.
Attachment #8513000 - Flags: feedback?(jlal) → feedback+

Comment 5

4 years ago
Comment on attachment 8513000 [details] [review]
Pull request

Made a few comments in the PR.  Looks good.
Attachment #8513000 - Flags: feedback?(garndt) → feedback+
(Assignee)

Comment 6

4 years ago
Comment on attachment 8513000 [details] [review]
Pull request

Spoke to Jonas over IRC and will follow-up with the scoping clamp-down in bug 1091212.

I think that enough people have looked at this now that I will go ahead and be landing this to master, and will iterate. Thanks for taking a look at this everyone!
Attachment #8513000 - Flags: feedback-
(Assignee)

Comment 7

4 years ago
Landed in master: https://github.com/KevinGrandon/autolander/commit/cf669bb25286c88d2f75bf836d838b4f07c1f255
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.