openstack team mailing list archive

[Mark McLoughlin <markmc@xxxxxxxxxx>]

Hey,

On Thu, 2012-05-03 at 14:24 +0200, Thierry Carrez wrote:
> Mark McLoughlin wrote:

> > Ok, what are "subsystem branches" and how would they work?
> > [...]
> >   - It would be up to the project dictators to help drive patches 
> >     through the right subsystem branches - e.g. they might object if 
> >     one subsystem maintainer merged a patch that inappropriately cut 
> >     into another subsystem or they might refuse to merge a given patch
> >     into the main branch unless it went through the appropriate 
> >     subsystem branch.
> > 
> >     (In gerrit, this would mean a small team or single person who can 
> >     +1 approve merge proposals on master. They would -1 proposals
> >     submitted against master which should have been submitted against a 
> >     subsystem branch.)
> 
> IIUC under this model we would still accept changes to be directly
> proposed to master (think bug fixes, or features that don't belong into
> a specific subsystem) ? For those we would still need a review team
> (separate from the "project dictators" who do the second-stage review)
> with wide expertise on all things Nova.

Yes. Until we get to a point where every change should obviously go
through a particular subsystem branch, you need this. And we may never
need to get there.

Even if you had subsystem branches for everything, allowing people to go
direct to master is a nice way to be sure that people don't need to be
blocked by a subsystem maintainer. If a subsystem maintainer is
non-responsive or totally non-cooperative, then just go directly to
master. This only works if the project dictator flames/ignores anyone
abusing this route.

On the practical point about "project dictators" vs "review team", you
can give +1 approve rights to the former and give +2 review rights to
both.

That means the dictators need to approve everything coming into master,
but that should mostly just be (1) signing off on a submission from a
subsystem maintainer and (2) signing off on a review which has 2 +2s
from the review team.

> > [...]
> >   - Plausible subsystem branches are e.g.:
> > 
> >       - OpenStack APIs
> >       - EC2 API
> >       - virt
> >          - libvirt driver
> >          - xenapi driver
> >          - vmware driver
> >       - networking
> >       - volumes
> >       - scheduler
> >       - RPC
> > 
> >     Deciding which areas make sense as a subsystem branch is 
> >     non-trivial.
> > 
> >     Should there be a "DB" subsystem? Probably not, because that would 
> >     mean every new feature needs to come through this branch or, 
> >     alternatively, the DB maintainer would need to accept DB schema 
> >     additions without the context of how it's being used higher up the 
> >     stack.
> > 
> >     Ok, so why does it make sense to have an "OpenStack APIs" 
> >     subsystem? Don't all features affect that branch too? Well, maybe, 
> >     but the APIs really do need strong oversight. Perhaps we can be 
> >     confident that we can add e.g. a new scheduler feature through the
> >     scheduler branch and then later merge any API additions through the 
> >     APIs branch.
> 
> I think there are two types of subsystems making sense: those areas of
> code that are already separated into a self-contained module, and those
> areas of the code where editorial control / extra expertise is needed.
> 
> As an example, sometimes I wish that all proposed run-as-root commands
> would be through a security review before being accepted in master
> (because sometimes they are overkill and needlessly increase the
> run-as-root surface, and some other times they annihilate our efforts at
> filtering nova->root escalation). This is difficult to do with a common
> team of 25+ reviewers.

Yes, but it would be difficult to co-ordinate this stuff as a separate
subsystem tree. You'd have two options:

  - Any rootwrap changes must be incorporated separately into the
    rootwrap subsystem tree before they can be merged into another 
    subsystem tree. But how can you review rootwrap changes without 
    seeing the context of why they're needed?

  - Any rootwrap changes get merged into the various subsystem trees 
    first, but they must then go through the rootwrap subsystem tree 
    before being merged into master.

In reality, I think you just need to trust the combination of the
subsystem maintainers and project dictators to be on top of this kind of
stuff. Or at least that they would be sensible to pull in an appropriate
reviewer to double check it before merging it.

> > And how about feature branches?
> > 
> >   - Feature branches are relatively short-lived (i.e. weeks or months
> >     rather than years) branches for a specific feature. They are a
> >     mechanism for developers to work on a patch series in the open until
> >     the feature is complete enough to be merged into a subsystem branch
> >     or master.
> > 
> >     (I'm not sure gerrit is right for this. Why not just do it in 
> >     folk's github forks? I think all people are looking for is for 
> >     people to be more aware of feature branches. How about if you put 
> >     details of your feature branch in the blueprint for the feature?)
> > 
> >     (If not using gerrit, can developers configure Jenkins to CI their 
> >     branch? Or is Smokestack the right tool?)
> 
> I think preserving the ability to run your branch through integration
> testing is a necessary prerequisite of the new model.

Yes, it's only really needed at the point where the feature branch is
merged into the subsystem tree. The developer should be able to break
stuff on their WIP feature branch if they wish, since they'll be expect
to rebase/fix before proposing it to be merged into the subsystem tree.

So Jenkins/Smokestack would be nice tools to provide to folks working on
feature branches, but they don't need to be gating.

Cheers,
Mark.