Closed Bug 1179458 Opened 9 years ago Closed 9 years ago

taskcluster-github: Generic Github integration w. organization hooks

Categories

(Taskcluster :: Services, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jonasfj, Assigned: mrrrgn)

References

Details

We should create a service that uses github organization hooks to:
 A) Publish a pulse messages for pushes and PRs
    (remember to make useful routing keys)
 B) Create a task-graph with the task from ".taskcluster.yml" (see bug 1158272)
    (if the ".taskcluster.yml" exists)

Reporting Status:
  When task-graph created in step (B) is running/resolved we should update
  the commit status on github using github API.
  For now showing status + link to task-graph inspector seems solid,
  we can do fancier stuff later.
  (Do this using background worker listening on pulse exchanges)

Auto-landing:
  We'll probably want to add some auto-lander feature. Not sure how to
  add configuration for this in-tree, that might be sketchy, as it probably
  has to specify bugzilla component. We should chat with:
  - kgrandon who maintains auto-lander for gaia.
  - edunham who knows a bit about what research does for auto-landing.
  (implemented with background worker listening on pulse exchanges)

Scopes:
  We need to two sets of scopes:
   i)  Scopes issued to task-graphs created for a push, branch, internal PR
   ii) Scopes issued to task-graphs created for a PR from external repository
  Short term, we can hardcode these in config.
  Long term, we grant "assume:github-repository:<org>/<repo>" and
  auth.taskcluster.net will know what set of scopes that maps to.
  (at least that is the future concept).

Configuration:
  I suggest we specify configured github organization using environment
  variables on heroku. Or if we're crazy add API for managing github orgs,
  and store the data in azure table storage (with base.Entity).

-------------------
Implementation-wise:
  I strongly suggest using the taskcluster technology stack.
  
  babeljs:         (stage: 1, so we have async/await)
  taskcluster-base:
    base.API       (for declaring API methods)
    base.Exchanges (for declaring pulse exchanges)
    base.stats     (for reporting influxdb stats)
    base.validator (for JSON schema validation, and loading of schemas)
    base.config    (for loading config)
    etc...
    base.Entity    (if we need persist data to azure table storage)

  This way, API and pulse exchange docs and client libraries can be
  automatically generated using our existing tooling.

  See taskcluster-queue, taskcluster-index and other TC components for how
  to set this up. Note: taskcluster-purge-cache is a very good and short example
  that has both API and exchanges.
  Ask jonasfj, if there is any questions and file for review once the skeleton
  is up. Our tech stack (skeleton) is a moving target, but we try to move in
  the same direction.

I propose we name this component: taskcluster-github
@mrrrgn,
You expressed interest in this, I think we should discuss auto-landing with customers for this.
I'm not quite sure how this should work, and how it could be configure safely.

For getting started take a look at: "taskcluster-purge-cache" it's a good very very small skeleton to
start from. But in general don't be scared to look at other components to see what they do or ask me.
The purge-cache and queue components are probably amongst most up to date, some of the other suffers
legacy code with a lot of ugly promises.

Anyways, what do you think about the outline above?
Flags: needinfo?(winter2718)
(In reply to Jonas Finnemann Jensen (:jonasfj) from comment #1)
> @mrrrgn,
> You expressed interest in this, I think we should discuss auto-landing with
> customers for this.
> I'm not quite sure how this should work, and how it could be configure
> safely.
> 
> For getting started take a look at: "taskcluster-purge-cache" it's a good
> very very small skeleton to
> start from. But in general don't be scared to look at other components to
> see what they do or ask me.
> The purge-cache and queue components are probably amongst most up to date,
> some of the other suffers
> legacy code with a lot of ugly promises.
> 
> Anyways, what do you think about the outline above?

This looks like a great plan, and I'd agree that using a skeleton project is a smart move for me here. Thanks!

The only thing I'm stuck on is that docker-worker-ci and autolander both implement part B, almost to a T. I realize the value of having our own "taskcluster" official way of doing things; but I'd hate to see taskcluster being invoked twice from the same PR (via autolander and taskcluster-github). Aside from that, I'll pick through those two projects for ideas about moving this forward.

One idea with autolanding: We should only do it for PRs from people who are already a part of the organization to start. If a whitelist is needed beyond that, why not make it an entry in the .taskcluster.yml file?
Flags: needinfo?(winter2718)
Assignee: nobody → winter2718
Then again, I agree, having a service that's able to land prs for the whole Mozilla org may be in bad taste. The only other option would be to have individual repos opt in by manually giving us permissions. It's still a tough call IMO. It would be great to have a one-step setup: just drop in .taskcluster.yml and go.
Eh, second thought, just having users grant permissions to an account we control is the only sane way I can imagine approaching it.
Depends on: 1179487
No longer depends on: 1179487
Depends on: 1179487, 1158272
@mrrrgn,
Yeah, people will have to add an "auto-lander" user to the organization. For us to be able to auto-land things.
The config options I'm thinking of is:
 - Is auto-landing activated for the repository: yes/no
 - Where does the `r+` come from?
   - Does the assignee write `r+` in a comment?
   - Is there a linked bugzilla bug? Or a linked bugzilla attachment? Which may have an r+
(I'm not sure if github has a way to hide the "merge" button, or make it auto-land)
kgrandons autolander: https://github.com/mozilla/autolander
I think it makes more sense to talk to him than trying to decipher the code.

But my guess is for each PR:
 * Create a task-graph for the PR
 * Sync task-graph state with github status API
 * Determine if we should land it:
   1) Read "Bug <number> - ..." from PR title
   2) Find bugzilla attachment, by finding the attachment consisting of the PR URL
   3) When the bugzilla attachment has r+, then either:
      A) Wait for bug to have [checkin-needed] then auto-land
      B) Offer a button the user can click to auto-land

Note:
 - I don't think we can hide the big MERGE button on github, unless we revoke commit access.
   (probably fine since people shouldn't have direct commit access anyways)
 - There probably has to be some sanity about:
     - which bugzilla components a bug can belong to,
     - who is allowed to set the r+
Component: TaskCluster → General
Product: Testing → Taskcluster
Component: General → Integration
No longer depends on: 1158272
https://github.com/taskcluster/taskcluster-github
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Component: Integration → Services
You need to log in before you can comment on or make changes to this bug.