wptsync landing direct to autoland doesn't work well
Categories
(Developer Infrastructure :: General, defect)
Tracking
(Not tracked)
People
(Reporter: jgraham, Assigned: jgraham)
References
(Depends on 1 open bug)
Details
Attachments
(1 file)
wptsync recently changed from landing to mozilla-inbound to landing to autoland. However it seems like the way autoland is used makes it hard to have multiple tools pushing there. Once there's a backlog of patches in the queue, it seems like a few patches are allowed to land and then the tree is closed again to give CI time to catch up. This is totally reasonable, but means that any non-lando tool has to be lucky to find a window in which landing is allowed (basically there can't be any existing lando queue and CI must be mostly up to date). With the wpt sync in particular if we don't land changes for a few days a backlog builds up and we can end up with 100 commits in a single push. The time taken to fetch, rebase, and push such a large number of commits can be significant and if it happens to be above the average interval between lando-mediated landings it can be almost impossible to actually land a change (this was also a problem on inbound before developers switched to autoland).
There are several possible approaches to fix this problem.
- Keep landing to out "own" tree and have sheriffs merge that. That's basically the situation we had with mozilla-inbound, and I assume all the reasons that people want inbound killed still apply, so I expect this isn't an option that works for others.
- Push wptsync commits to lando somehow. This means we have a single queue for all inbound commits and don't have to have wptsync worry about the tree being open or rebase races. There are several possible ways I imagine this could work
- Push the commits to a VCS somewhere (e.g. try) and have some way to tell lando "grab all the commits from this revision and add them to the queue'
- Provide the relevant extended diffs directly over HTTP somehow (presumably like a push version of whatever pull mechanism is used for getting commits from Phabricator)
- Something involving pushing to Phabricator and auto-marking all the changesets as landable and then telling lando to fetch that stack from Phabricator.
I'm not very familiar with the way that the conduit pipeline works, so it's possible that there are other possible approaches. One constraint is that we want the system to enforce the invariant that we only touch wpt directories; this is currently enforced by a hook activated on push, so that piece would need to move elsewhere if wptsync was no longer pushing directly.
Ah, I see how this would be frustrating.
Your best option for today is to submit your revisions to Phabricator (this can be automated) and then request landing with Lando (this cannot be automated).
Lando currently does not have a public API which you can use to automate a landing request. We've spoken about this as a team several times, however there hasn't been a project that's required it until now. I'll rekindle those discussions with a focus on providing the bare minimum functionality to make things easier for you.
|
Assignee | |
Comment 2•6 years ago
|
||
OK, if the human step here is "push a single button on lando" that's not too bad as an interim solution.
In terms of submitting to Phabricator, are you just envisioning using the normal tooling here (moz-phab, etc.) or direct API access? I assume the former. Is there some way to auto-mark the commits as approved? iirc Phabricator doesn't handle a=testonly or similar, but maybe there's some way to write a rule that says commits from the bot user don't require additional approval? (if that isn't possible and a human has to manually accept each revision that's a lot more painful).
(In reply to James Graham [:jgraham] from comment #2)
In terms of submitting to Phabricator, are you just envisioning using the normal tooling here (moz-phab, etc.) or direct API access? I assume the former.
Wrapping moz-phab would be easier; the work required to hit the API directly is larger than you'd expect (spoiler: we need to parse and create our own diffs). While moz-phab wasn't designed to be driven by automation, we're happy to tweak things to make that viable if required.
Is there some way to auto-mark the commits as approved?
This shouldn't be required.
Lando is the gatekeeper when it comes to determining if a revision is good to land. Right now Lando lets L3 users land revisions which have not been approved; you'll need to click through a "land this thing that hasn't been approved" confirmation.
We're working on making the review-not-required experience more explicit in Lando, I'll make sure to add "test-only" to the mix (the other big use-case for this is documentation-only changes).
|
|
Assignee | |
Comment 4•6 years ago
•
|
||
So I tried this "by hand" using moz-phab and immediately ran into bug 1597264. Possibly with arc it all works fine, but I'm quite keen not to install PHP on the sync host.
I am also concerned that this removes the hg check that the sync bot is only pushing to the wpt directory; this was considered an important part of allowing the bot to land changes so I'd like to find some way to enforce the same kind of access control even though there isn't a direct push any more.
|
|
Assignee | |
Comment 5•6 years ago
|
||
Oh and now I have "Lando API returned an unexpected error" at https://lando.services.mozilla.com/D53624/
(In reply to James Graham [:jgraham] from comment #4)
I am also concerned that this removes the hg check that the sync bot is only pushing to the wpt directory; this was considered an important part of allowing the bot to land changes so I'd like to find some way to enforce the same kind of access control even though there isn't a direct push any more.
Lando performs a hg push like any other user; all the commit hooks are run.
You'll need to log into Lando as the sync bot in order to ensure the push user is the same as your direct push.
Depending on how that account is setup it might be problematic as this means logging in to auth0 as the sync bot.
|
|
Assignee | |
Comment 7•6 years ago
|
||
Ah I didn't think about the user lando pushes as. Having said that, if it's possible to log into auth0 as the sync bot I don't know how. And in practice "remember to log in as a special user" doesn't seem like it's going to work in the face of human fallibility.
(In reply to James Graham [:jgraham] from comment #5)
Oh and now I have "Lando API returned an unexpected error" at https://lando.services.mozilla.com/D53624/
Filed as bug 1597306
|
|
Assignee | |
Comment 9•6 years ago
|
||
|
|
Assignee | |
Comment 11•3 years ago
|
||
We don't seem to have the same number of tree closures anymore, so pushing to autoland isn't so problematic. We also explored the option of using Lando and it didn't work out because of commit limits.
Given this doesn't seem to actually be such a problem anymore I'll close it.
Description
•