Open Stack

On Mon, 2014-08-11 at 15:25 -0700, Joe Gordon wrote:
> 
> 
> 
> On Sun, Aug 10, 2014 at 11:59 PM, Mark McLoughlin <markmc at redhat.com>
> wrote:
>         On Fri, 2014-08-08 at 09:06 -0400, Russell Bryant wrote:
>         > On 08/07/2014 08:06 PM, Michael Still wrote:
>         > > It seems to me that the tension here is that there are
>         groups who
>         > > would really like to use features in newer libvirts that
>         we don't CI
>         > > on in the gate. Is it naive to think that a possible
>         solution here is
>         > > to do the following:
>         > >
>         > >  - revert the libvirt version_cap flag
>         >
>         > I don't feel strongly either way on this.  It seemed useful
>         at the time
>         > for being able to decouple upgrading libvirt and enabling
>         features that
>         > come with that.
>         
>         
>         Right, I suggested the flag as a more deliberate way of
>         avoiding the
>         issue that was previously seen in the gate with live
>         snapshots. I still
>         think it's a pretty elegant and useful little feature, and
>         don't think
>         we need to use it as proxy battle over testing requirements
>         for new
>         libvirt features.
> 
> 
> Mark,
> 
> 
> I am not sure if I follow.  The gate issue with live snapshots has
> been worked around by turning it off [0], so presumably this patch is
> forward facing.  I fail to see how this patch is needed to help the
> gate in the future.

On the live snapshot issue specifically, we disabled it by requiring
1.3.0 for the feature. With the version cap set to 1.2.2, we won't
automatically enable this code path again if we update to 1.3.0. No
question that's a bit of a mess, though.

The point was a more general one - we learned from the live snapshot
issue that having a libvirt upgrade immediately enable new code paths
was a bad idea. The patch is a simple, elegant way of avoiding that.

>  Wouldn't it just delay the issues until we change the version_cap?

Yes, that's the idea. Rather than having to scramble when the new
devstack-gate image shows up, we'd be able to work on any issues in the
context of a patch series to bump the version_cap.

> The issue I see with the libvirt version_cap [1] is best captured in
> its commit message: "The end user can override the limit if they wish
> to opt-in to use of untested features via the 'version_cap' setting in
> the 'libvirt' group." This goes against the very direction nova has
> been moving in for some time now. We have been moving away from
> merging untested (re: no integration testing) features.  This patch
> changes the very direction the project is going in over testing
> without so much as a discussion. While I think it may be time that we
> revisited this discussion, the discussion needs to happen before any
> patches are merged.

You put it well - some apparently see us moving towards a zero-tolerance
policy of not having any code which isn't functionally tested in the
gate. That obviously is not the case right now.

The sentiment is great, but any zero-tolerance policy is dangerous. I'm
very much in favor of discussing this further. We should have some
principles and goals around this, but rather than argue this in the
abstract we should be open to discussing the tradeoffs involved with
individual patches.

> I am less concerned about the contents of this patch, and more
> concerned with how such a big de facto change in nova policy (we
> accept untested code sometimes) without any discussion or consensus.
> In your comment on the revert [2], you say the 'whether not-CI-tested
> features should be allowed to be merged' debate is 'clearly
> unresolved.' How did you get to that conclusion? This was never
> brought up in the mid-cycles as a unresolved topic to be discussed. In
> our specs template we say "Is this untestable in gate given current
> limitations (specific hardware / software configurations available)?
> If so, are there mitigation plans (3rd party testing, gate
> enhancements, etc)" [3].  We have been blocking untested features for
> some time now.

Asking "is this tested" in a spec template makes a tonne of sense.
Requiring some thought to be put into mitigation where a feature is
untestable in the gate makes sense. Requiring that the code is tested
where possible makes sense. It's a zero-tolerance "get your code
functionally tested or GTFO" policy that I'm concerned about.

> I am further perplexed by what Daniel Berrange, the patch author,
> meant when he commented [2] "Regardless of the outcome of the testing
> discussion we believe this is a useful feature to have." Who is 'we'?
> Because I don't see how that can be nova-core or even nova-specs-core,
> especially considering how many members of those groups are +2 on the
> revert. So if 'we' is neither of those groups then who is 'we'?

That's for Dan to answer, but I think you're either nitpicking or have a
very serious concern.

If nitpicking, Dan could just be using the "Royal 'We'" :) Or he could
just mean the nova-core reviewers who +2ed the patch.

On the more serious front, I'm conscious that Dan, Russell and I all
work for Red Hat and maybe you're really asking whether "we" means "Red
Hat" - i.e. some sort of corporate agenda. Absolutely not. The debate
kicked off here:

  https://review.openstack.org/103923

I think it's pretty clear everyone is coming at this with good-faith,
genuine personal opinions and concerns.

Mark.