Open Stack

On Wed, Jul 16, 2014 at 08:29:26AM -0700, Dan Smith wrote:
> >> Based on these experiences, libvirt version differences seem to be as
> >> substantial as major hypervisor differences.
> > 
> > I think that is a pretty dubious conclusion to draw from just a
> > couple of bugs. The reason they really caused pain is that because
> > the CI test system was based on old version for too long.
> 
> I think the conclusion being made is that "libvirt versions two years
> apart are effectively like different major versions of a hypervisor." I
> don't think that's wrong.
> 
> > That is rather misleading statement you're making there. Libvirt is
> > in fact held to *higher* standards than xen/vmware/hypver because it
> > is actually gating all commits. The 3rd party CI systems can be
> > broken for days, weeks and we still happily accept code for those
> > virt. drivers.
> 
> Right, and we've talked about raising that bar as well, by tracking
> their status more closely, automatically -2'ing patches that touch the
> subdirectory but don't get a passing vote from the associated CI system,
> etc.
> 
> You're definitely right that libvirt is held to a higher bar in terms of
> it being required to pass tests before we can even mechanically land a
> patch. However, there is a lot of function in the driver that we don't
> test right now because of the version we're tied to in the gate nodes.
> It's actually *easier* for a 3rd party system like vmware to roll their
> environment and enable tests of newer features, so I don't think that
> this requirement would cause existing 3rd party CI systems any trouble.
> 
> > AFAIK there has never been any statement that every feature added
> > to xen/vmware/hyperv must be tested by the 3rd party CI system.
> 
> On almost every spec that doesn't already call it out, a reviewer asks
> "how are you going to test this beyond just unit tests?" I think the
> assumption and feeling among most reviewers is that new features,
> especially that depend on new things (be it storage drivers, hypervisor
> versions, etc) are concerned about approving without testing.

Expecting new functionality to have testing coverage in the common
case is entirely reasonable. What I disagree with is the proposal
to say it is mandatory, when the current CI system is not able to
test it for any given reason. In some cases it might be reasonable
to expect the contributor to setup 3rd party CI, but we absolutely
cannot make that a fixed rule or we'll kill contributions from
people who are not backed by vendors in a position to spend the
significant resource it takes to setup & maintain CI.  IMHO the
burden is on the maintainer of the CI to ensure it is able to
follow the needs of the contributors. ie if the feature needs a
newer libvirt version in order to test with, the CI maintainer(s)
should deal with that. We should not turn away the contributor
for a problem that is outside their control.

> > AFAIK the requirement for 3rd party CI is merely that it has to exist,
> > running some arbitrary version of the hypervisor in question. We've
> > not said that 3rd party CI has to be covering every version or every
> > feature, as is trying to be pushed on libvirt here.
> 
> The requirement in the past has been that it has to exist. At the last
> summit, we had a discussion about how to raise the bar on what we
> currently have. We made a lot of progress getting those systems
> established (only because we had a requirement, by the way) in the last
> cycle. Going forward, we need to have new levels of expectations in
> terms of coverage and reliability of those things, IMHO.

IMHO we need to maintain a balance between ensuring code quality
and being welcoming & accepting to new contributors. 

New features have a certain value $NNN to the project & our users.
The lack of CI testing does not automatically imply that the value
of that work is erased to $0 or negative $MMM. Of course the lack
of CI will create uncertainty in how valuable it is, and potentially
imply costs for us if we have to deal with resolving bugs later.
We must be careful not to overly obsess on the problems of work
that might have bugs, to the detriment of all the many submissions
that work well.

We need to take a pragmatic view of this tradeoff based on the risk
implied by the new feature. If the new work is impacting existing
functional codepaths then this clearly exposes existing users to
risk of regressions, so if that codepath is not tested this is
something to be very wary of. If the new work is adding new code
paths that existing deployments wouldn't exercise unless they 
explicitly opt in to the feature, the risk is significantly lower.
The existence of unit tests will also serve to limit the risk in
many, but not all, situations. If something is not CI tested then
I'd also expect it to get greater attention during review, with
the reviewers actually testing it functionally themselves as well
as code inspection. Finally we should also have some good faith in
our contributors that they are not in fact just submitting total
untested junk to us.

Now the need to do greater detailed review is certainly a cost
and so it is reasonable to say that we'd rate limit the amount
of code we're prepared to accept in a cycle without CI testing.
This is ok, because again it is a case of making a pragmatic
decision weighing the pluses & minuses of the work, not automatically
assuming the worst.

> > As above, aside from the question of gating vs non-gating, the bar is
> > already set at the same level of everyone. There has to be a CI system
> > somewhere testing some arbitrary version of the software. Everyone meets
> > that requirement.
> 
> Wording our current requirement as you have here makes it sound like an
> "arbitrary" ticky mark, which saddens and kind of offends me. What we
> currently have was a step in the right direction. It was a lot of work,
> but it's by no means arbitrary nor sufficient, IMHO.

I just meant it is arbitrary in the sense that we did not place any
requirements around what level of feature coverage or what version
coverage 3rd party CI had to reach. We left the details entirely upto
the maintainers to decide upon as they decided was appropriate.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|