Open Stack

On Aug 20, 2014, at 6:28 PM, Salvatore Orlando <sorlando at nicira.com> wrote:

> Some comments inline.
> 
> Salvatore
> 
> On 20 August 2014 17:38, Ihar Hrachyshka <ihrachys at redhat.com> wrote:
> 
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA512
>> 
>> Hi all,
>> 
>> I've read the proposal for incubator as described at [1], and I have
>> several comments/concerns/suggestions to this.
>> 
>> Overall, the idea of giving some space for experimentation that does
>> not alienate parts of community from Neutron is good. In that way, we
>> may relax review rules and quicken turnaround for preview features
>> without loosing control on those features too much.
>> 
>> Though the way it's to be implemented leaves several concerns, as follows:
>> 
>> 1. From packaging perspective, having a separate repository and
>> tarballs seems not optimal. As a packager, I would better deal with a
>> single tarball instead of two. Meaning, it would be better to keep the
>> code in the same tree.
>> 
>> I know that we're afraid of shipping the code for which some users may
>> expect the usual level of support and stability and compatibility.
>> This can be solved by making it explicit that the incubated code is
>> unsupported and used on your user's risk. 1) The experimental code
>> wouldn't probably be installed unless explicitly requested, and 2) it
>> would be put in a separate namespace (like 'preview', 'experimental',
>> or 'staging', as the call it in Linux kernel world [2]).
>> 
>> This would facilitate keeping commit history instead of loosing it
>> during graduation.
>> 
>> Yes, I know that people don't like to be called experimental or
>> preview or incubator... And maybe neutron-labs repo sounds more
>> appealing than an 'experimental' subtree in the core project. Well,
>> there are lots of EXPERIMENTAL features in Linux kernel that we
>> actively use (for example, btrfs is still considered experimental by
>> Linux kernel devs, while being exposed as a supported option to RHEL7
>> users), so I don't see how that naming concern is significant.
>> 
> 
> I think this is the whole point of the discussion around the incubator and
> the reason for which, to the best of my knowledge, no proposal has been
> accepted yet.
> 
>> 
>> 2. If those 'extras' are really moved into a separate repository and
>> tarballs, this will raise questions on whether packagers even want to
>> cope with it before graduation. When it comes to supporting another
>> build manifest for a piece of code of unknown quality, this is not the
>> same as just cutting part of the code into a separate
>> experimental/labs package. So unless I'm explicitly asked to package
>> the incubator, I wouldn't probably touch it myself. This is just too
>> much effort (btw the same applies to moving plugins out of the tree -
>> once it's done, distros will probably need to reconsider which plugins
>> they really want to package; at the moment, those plugins do not
>> require lots of time to ship them, but having ~20 separate build
>> manifests for each of them is just too hard to handle without clear
>> incentive).
>> 
> 
> One reason instead for moving plugins out of the main tree is allowing
> their maintainers to have full control over them.
> If there was a way with gerrit or similars to give somebody rights to merge
> code only on a subtree I probably would not even consider the option of
> moving plugin and drivers away. From my perspective it's not that I don't
> want them in the main tree, it's that I don't think it's fair for core team
> reviewers to take responsibility of approving code that they can't fully
> tests (3rd partt CI helps, but is still far from having a decent level of
> coverage).
> 
> 
>> 
>> 3. The fact that neutron-incubator is not going to maintain any stable
>> branches for security fixes and major failures concerns me too. In
>> downstream, we don't generally ship the latest and greatest from PyPI.
>> Meaning, we'll need to maintain our own downstream stable branches for
>> major fixes. [BTW we already do that for python clients.]
>> 
>> 
> This is a valid point. We need to find an appropriate trade off. My
> thinking was that incubated projects could be treated just like client
> libraries from a branch perspective.
> 
> 
> 
>> 4. Another unclear part of the proposal is that notion of keeping
>> Horizon and client changes required for incubator features in
>> neutron-incubator. AFAIK the repo will be governed by Neutron Core
>> team, and I doubt the team is ready to review Horizon changes (?). I
>> think I don't understand how we're going to handle that. Can we just
>> postpone Horizon work till graduation?
>> 
>> 
> I too do not think it's a great idea, mostly because there will be horizon
> bits not shipped with horizon, and not verified by horizon core team.
> I think it would be ok to have horizon support for neutron incubator. It
> won't be the first time that support for experimental features is added in
> horizon.
> 
> 
> 5. The wiki page says that graduation will require full test coverage.
>> Does it mean 100% coverage in 'coverage' report? I don't think our
>> existing code is even near that point, so maybe it's not fair to
>> require that from graduated code.
>> 
> 
> I agree that by these standards we should take the whole neutron and return
> it to incubation, or probably just chuck it in the bin.
> It's not a mystery that Neutron quality is well below integrated level but
> let's not diverge.
> 
> On the other hand, the fact that Neutron's code is rubbish does not
> authorise the addition of further rubbish.
> I see this requirement for graduation as a measure to ensure new additions
> to neutron have proper quality.
> In the meanwhile it will be mandatory for the neutron community to keep
> working on quality, scalability and improve testing coverage.
> Otherwise there will be no talk about neutron-incubator, mostly because
> probably there will be no neutron.

+1

If we judge ourselves by our past standards we have no hope of improving.  

I would agree, though, that the wiki requirement of ‘full test coverage’ is very much ambiguous.  I would hope that a new feature would have sufficient testing to validate its target use cases and guard against regression, but that determination is unlikely to be as mechanical as ‘100% coverage achieved’.  A more useful metric might be whether a quorum of cores believe that the value of a new feature exceeds the costs imposed by its ongoing maintenance.  If a feature has insufficient or poor quality testing, the value of the feature could be seen to be not justified by the cost it would impose.


> 
> 
>> A separate tree would probably be reasonable if it would be governed
>> by a separate team. But as it looks now, it's still Neutron Cores who
>> will do the review heavy-lifting. So I wonder why not just applying
>> different review rules for patches for core and the staging subtree.
>> 
> 
> This is a good point. As a neutron core I don't want to do the heavy
> lifting there. I think we should define rules which allow teams to iterate
> quickly while enabling the neutron core team to retain some form of control
> on what goes in the incubator.

I think divesting the current cores of responsibility of reviewing a separate plugin tree makes good sense - the impact of a given plugin on everything else is likely to be minimal.  

I think incubator is different, though.  Why would we be incubating something if we didn’t anticipate that it could be merged into the main tree at some point - to the community's benefit - and wouldn’t we want it to be of sufficiently high quality so as to avoid a costly rewrite?  I’m not sure how siloing incubation review efforts would support that outcome.

I think cores should be responsible for both Neutron and incubator, but with a caveat.  There should be different standards for incubator code.  Patches should be allowed to land early and often - none of this ‘I want to see the whole feature before I merge anything’ that have proven so costly this cycle and forced any non-trivial feature to be developed monolithically.  Let’s try iterating quickly in the incubator repo with the understanding that, unlike in the main tree, landing a patch doesn’t mean we are committing to support it indefinitely.  With the understanding that periodic or final feature review are a necessary part of the process - and selectivity around what we accept for incubation - I would hope that we could balance the cost of our oversight with the benefits it promises.


m.

> 
> 
>> [1]: https://wiki.openstack.org/wiki/Network/Incubator
>> [2]:
>> 
>> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/staging
>> 
>> /Ihar
>> -----BEGIN PGP SIGNATURE-----
>> Version: GnuPG/MacGPG2 v2.0.22 (Darwin)
>> 
>> iQEcBAEBCgAGBQJT9MDpAAoJEC5aWaUY1u57bYAH/0LsZonj3zVmWomUBBPriUOm
>> GRoNBHq6C7BCfO7gRnQQyRd/N4jCL4Y1Dfbfv2Ypulsgf0x+ugvmzOrWm2Sa7KiS
>> F3adumx+0OjJSMb5SSOxZQHpsZFjJmwtJjat9vwOYFXcCXhn8r9AgN3TPm5GyZ29
>> NPY+SQdqu+G/ZgXd94sE2+gGbx0H5nLZusJD0yiUpoNExhv4qvjHSZW1rwssb+Ac
>> 3dU3LU1FqhM7UxkgnWk6AGYHfLjr5CfxXBrmikQsxXljl8Sko9DBTpKa3YtVcBX1
>> FdMWLGn13nFNasGAKHot/aRfmdfPIzN0TsjjfRstm0W1VLvvbQjLxGTQDEyey/U=
>> =vdaC
>> -----END PGP SIGNATURE-----
>> 
>> _______________________________________________
>> OpenStack-dev mailing list
>> OpenStack-dev at lists.openstack.org
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>> 
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev