[openstack-dev] [Neutron] Simple proposal for stabilizing new features in-tree

Robert Kukura kukura at noironetworks.com
Mon Aug 11 16:58:47 UTC 2014

On 8/8/14, 6:28 PM, Salvatore Orlando wrote:
> "If we want to keep everything the way it is, we have to change 
> everything" [1]
> This is pretty much how I felt after reading this proposal, and I felt 
> that this quote, which Ivar will probably appreciate, was apt to the 
> situation.
> Recent events have spurred a discussion about the need for a change in 
> process. It is not uncommon indeed to believe that by fixing the 
> process, things will inevitably change for better. While no-one argues 
> that flaws in processes need to be fixed, no process change will ever 
> change anything, in my opinion, unless it is aimed at spurring change 
> in people as well.
> From what I understand, this proposal starts with the assumption that 
> any new feature which is committed to Neutron (ie: has a blueprint 
> approved), and is not a required neutron component should be 
> considered as a preview. This is not different from the process, 
> which, albeit more informally, has been adopted so far. Load 
> Balancing, Firewall, VPN, have all been explicitly documented as 
> experimental in their first release; I would argue that even if not 
> experimental anymore, they may not be considered stable until their 
> stability was proven by upstream QA with API and scenario tests - but 
> this is not sanctioned anywhere currently, I think.
Correct, this proposal is not so much a new process or change in process 
as a formalization of what we've already been doing, and a suggested 
adaptation to clarify the current expectations around stability of new APIs.
> According to this proposal, for preview features:
> - all the code would be moved to a "preview" package
> - Options will be marked as "preview"
> - URIs should be prefixed with "preview"
That's what I suggested, but, as several people have pointed out, this 
does seem like its worth the cost of breaking the API compatibility just 
at the point when it is being declared stable. I'd like to withdraw this 
> - CLIs will note the features are "preview" in their help strings
> - Documentation will explicitly state this feature is "preview" (I 
> think we already mark them as experimental, frankly I don't think 
> there are a lot of differences in terminology here)
Yes. Again to me, failure is one likely outcome of an "experiment". The 
term "preview" is intended to imply more of a commitment to quickly 
reach stability.
> - Database migrations will be in the main alembic path as usual
> - CLI, Devstack and Heat support will be available
Right, as appropriate for the feature.
> - Can be used by non-preview neutron code
No, I suggested "No non-preview Neutron code should import code from 
anywhere under the neutron.preview module, ...".
> - Will undergo the usual review process
Right. This is key for the code to not have to jump through a new major 
upheaval at right as it becomes stable.
> - QA will be desirable, but will done either with "WIP" tempest 
> patches or merging the relevant scenario tests in the preview feature 
> iself
More than "desirable". We need a way to maintain and run the 
tempest-like API and scenario tests during the stabilization process, 
but to let then evolve with the feature.
> - The feature might be promoted or removed, but the process for this 
> is not yet defined.
Any suggestions? I did try to address preventing long-term stagnation of 
preview features. As a starting point, reviewing and merging a patch 
that moves the code from the preview sub-tree to its intended location 
could be a lightweight promotion process.
> I don't think this change in process will actually encourage better 
> behaviour both by contributors and core reviewers.
Encouraging better behavior might be necessary, but wasn't the main 
intent of this proposal. This proposal was intended to clarify and 
formalize the stability expectations around the initial releases of new 
features. It was specifically intended to address the conundrum 
currently faced by reviewers regarding patches that meet all applicable 
quality standards, but may not yet have (somehow, miraculously) achieved 
the maturity associated with stable APIs and features fully supported 
for widespread deployment.
> I reckon that better behaviour might be encouraged by forcing 
> developer and reviewers to merge in the neutron source code tree only 
> code which meets the highest quality standards. A change in process 
> should enforce this - and when I think about the criteria, I think at 
> the same kind of criteria we're being imposed to declare parity with 
> nova. Proven reliability, and scalability should be a must. Proven 
> usability should be a requirement for all new APIs.
I agree regarding the quality standards for merging of code, and am not 
suggesting relaxing those one bit. But proving all of the desirable 
system-level attributes of a complex new feature before merging anything 
precludes any kind of iterative development process. I think we should 
consider enforcing things like proven reliability, scalability, and 
usability at the point where the feature is promoted to stable rather 
than before merging the initial patch.
> On the other hand we also need to avoid to over bureaucratise Neutron 
> - nobody loves that - and therefore ensure this process is enforced 
> only when really needed.
> Looking at this proposal I see a few thing I'm not comfortable with:
> - having no clear criterion for exclusion a feature might imply that 
> will be silently bit-rotting code in the preview package. Which what 
> would happen for instance if we end up with a badly maintained feature 
> , but since one or two core devs care about it, they'll keep vetoing 
> the removal
First, the feature will never be considered inclusion in the preview 
sub-tree without an initial approved blueprint and specification. 
Second, I suggest we automatically remove a feature from the preview 
tree after some small number of cycles, unless a new blueprint detailing 
what needs to be done to complete stabilization is approved.
> - using the normal review process will still not solve the problem of 
> slow review cycles, pointless downvotes for reviewers which actually 
> just do not understand the subject matter, and other pains associated 
> with lack of interest from small or large parts of the core team. For 
> instance, I think there is a line of pretty annoyed contributors as we 
> did not even bother reviewing their specs.
Agreed. But I'm hoping a clarified set of expectations for new features 
will allow implementation, review, and merging of code for approved 
blueprints to proceed incrementally, as is intended in our current 
process, which will build up the familiarization of the team with the 
new features as they are being developed.
> - The current provision about QA seems to state that it's ok to keep 
> code in the main repo that does not adhere to appropriate quality 
> standards. Which is the mistake we did with lbaas and other features, 
> and I would like to avoid. And to me it is not sufficient that the 
> code is buried in the 'preview' package.
Lowering code quality standards is definitely no part of the intent. The 
preview code must be production-ready in order to be merged. Its API and 
data model are just not yet declared stable.
> - Mostly important, this process provides a justification for 
> contributors to push features which do not meet the same standards as 
> other neutron parts and expect them to be merged and eventually 
> promoted, and on the other hand provides the core team with the 
> entitlement for merging them - therefore my main concern that it does 
> not encourages better behaviour in people, which should be the 
> ultimate aim of a process change.
I'm really confused by this interpretation of my proposal. Preview code 
patches must go through the normal review process. Each individual patch 
must meet all our normal standards. And the system-level quality 
attributes of the feature must be proven as well for the feature to be 
declared stable. But you can't prove these system level attributes until 
you get the code into the hands of early adopters and incorporate their 
feedback. Our current process can facilitate this, as long as we set 
expectations properly during this stabilization phase.
> If you managed to read through all of this, and tolerated my dorky 
> literature references, I really appreciate your patience, and would 
> like to conclude that here we're discussing proposals for a process 
> change, whereas I expect to discuss in the next neutron meeting the 
> following:
> - whether is acceptable to change the process now
> - what did go wrong in our spec review process, as we ended up with at 
> least an approved spec which is actually fiercely opposed by other 
> core team members.
These discussions need to happen. I don't think my proposal should be 
looked at as a major process change, but rather as a clarification of 
how our current process explicitly supports iterative development and 
stabilization of new features. It can be applied to several of the new 
features targeted for Juno. Whether there is actual opposition to the 
inclusion of any of these is a separate matter, but some clarity about 
exactly what inclusion would mean can't hurt that discussion.

Thanks for your indulgence as well,

> Have a good weekend,
> Salvatore
> [1] Quote from "Il Gattopardo" by Giuseppe Tomasi di Lampedusa 
> (english name: The Leopard)
> On 8 August 2014 22:21, Robert Kukura <kukura at noironetworks.com 
> <mailto:kukura at noironetworks.com>> wrote:
>     [Note - I understand there are ongoing discussion that may lead to
>     a proposal for an out-of-tree incubation process for new Neutron
>     features. This is a complementary proposal that describes how our
>     existing development process can be used to stabilize new features
>     in-tree over the time frame of a release cycle or two. We should
>     fully consider both proposals, and where each might apply. I hope
>     something like the approach I propose here will allow the
>     implementations of Neutron BPs with non-trivial APIs that have
>     been targeted for the Juno release to be included in that release,
>     used by early adopters, and stabilized as quickly as possible for
>     general consumption.]
>     According to our existing development process, once a blueprint
>     and associated specification for a new Neutron feature have been
>     reviewed, approved, and targeted to a release, development
>     proceeds, resulting in a series of patches to be reviewed and
>     merged to the Neutron source tree. This source tree is then the
>     basis for milestone releases and the final release for the cycle.
>     Ideally, this development process would conclude successfully,
>     well in advance of the cycle's final release, and the resulting
>     feature and its API would be considered fully "stable" in that
>     release. Stable features are ready for widespread general
>     deployment. Going forward, any further modifications to a stable
>     API must be backwards-compatible with previously released
>     versions. Upgrades must not lose any persistent state associated
>     with stable features. Upgrade processes and their impact on a
>     deployments (downtime, etc.) should be consistent for all stable
>     features.
>     In reality, we developers are not perfect, and minor (or more
>     significant) changes may be identified as necessary or highly
>     desirable once early adopters of the new feature have had a chance
>     to use it. These changes may be difficult or impossible to do in a
>     way that honors the guarantees associated with stable features.
>     For new features that effect the "core" Neutron API and therefore
>     impact all Neutron deployments, the stability requirement is
>     strict. But for features that do not effect the core API, such as
>     services whose deployment is optional, the stability requirement
>     can be relaxed initially, allowing time for feedback from early
>     adopters to be incorporated before declaring these APIs stable.
>     The key in doing this is to manage the expectations of developers,
>     packagers, operators, and end users regarding these new optional
>     features while they stabilize.
>     I therefore propose that we manage these expectations, while new
>     optional features in the source tree stabilize, by clearly
>     labeling these features with the term "preview" until they are
>     declared stable, and sufficiently isolating them so that they are
>     not confused with stable features. The proposed guidelines would
>     apply during development as the patches implementing the feature
>     are first merged, in the initial release containing the feature,
>     and in any subsequent releases that are necessary to fully
>     stabilize the feature.
>     Here are my initial not-fully-baked ideas for how our current
>     process can be adapted with a "preview feature" concept supporting
>     in-tree stabilization of optional features:
>     * Preview features are implementations of blueprints that have
>     been reviewed, approved, and targeted for a Neutron release. The
>     process is intended for features for which there is a commitment
>     to add the feature to Neutron, not for experimentation where
>     "failing fast" is an acceptable outcome.
>     * Preview features must be optional to deploy, such as by
>     configuring a service plugin or some set of drivers. Blueprint
>     implementations whose deployment is not optional are not eligible
>     to be treated as preview features.
>     * Patches implementing a preview feature are merged to the the
>     master branch of the Neutron source tree. This makes them
>     immediately available to all direct consumers of the source tree,
>     such as developers, trunk-chasing operators, packagers, and
>     evaluators or end-users that use DevStack, maximizing the
>     opportunity to get the feedback that is essential to quickly
>     stabilize the feature.
>     * The process for reviewing, approving and merging patches
>     implementing preview features is exactly the same as for all other
>     Neutron patches. The patches must meet HACKING standards, be
>     production-quality code, have adequate test coverage, have DB
>     migration scripts, etc., and require two +2s and a +A from Neutron
>     core developers to merge.
>     * DB migrations for preview features are treated similarly to
>     other DB migrations, forming a single ordered list that results in
>     the current overall DB schema, including the schema for the
>     preview feature. But DB migrations for a preview feature are not
>     yet required to preserve existing persistent state in a
>     deployment, as would be required for a stable feature.
>     * All code that is part of a preview feature is located under
>     neutron/preview/<feature>/. Associated unit tests are located
>     under neutron/tests/unit/preview/<feature>/, and similarly for
>     other test categories. This makes the feature's status clear to
>     developers and other direct consumers of the source tree, and also
>     allows packagers to easily partition all preview features or
>     individual preview features into separate optionally installable
>     packages.
>     * The tree structures underneath these locations should make it
>     straightforward to move the preview feature code to its proper
>     tree location once it is considered stable.
>     * Tempest API and scenario tests for preview features are highly
>     desirable. We need to agree on how to accomplish this without
>     preventing necessary API changes. Posting WIP patches to the
>     Tempest project may be sufficient initially. Putting Tempest-like
>     tests in the Neutron tree until preview features stabilize, then
>     moving them to Tempest when stabilization is complete, might be a
>     better long term solution.
>     * No non-preview Neutron code should import code from anywhere
>     under the neutron.preview module, unless necessary for special
>     cases like DB migrations.
>     * URIs for the resources provided by preview features should
>     contain the string "preview".
>     * Configuration file content related to preview features should be
>     clearly labeled as "preview".
>     * Preview features should be documented similarly to any stable
>     Neutron feature, but documents or sections of documents related to
>     preview features should have an easily recognizable label that
>     clearly identifies the feature as a "preview".
>     * Support for preview features in client libraries, and in other
>     projects such as Horizon, Heat, and DevStack, are essential to get
>     the feedback needed from early adopters during feature
>     stabilization. They are implemented normally, but should be
>     labeled "preview" appropriately, such as in GUIs, in CLI help
>     strings and in documentation so that end user expectations
>     regarding stability are managed.
>     * A process is needed to prevent long-term stagnation of features
>     in the preview sub-tree. It is reasonable to expect a new feature
>     to remain for one or two cycles, possibly with little change
>     (other than bug fixes), before stabilizing. A suggested rule is
>     that a new approved BP is required after two cycles, or the
>     feature gets removed from the Neutron source tree (maybe moved
>     (back) to an incubation repository).
>     I would appreciate feedback via this email thread on whether this
>     "preview feature" concept is worth further consideration,
>     refinement and potential usage for approved feature blueprints,
>     especially during the Juno cycle. I've also posted the proposal
>     text at https://etherpad.openstack.org/p/neutron-preview-features
>     for those interested in helping refine the proposal.
>     Thanks,
>     -Bob
>     _______________________________________________
>     OpenStack-dev mailing list
>     OpenStack-dev at lists.openstack.org
>     <mailto: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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20140811/a8d5cab5/attachment-0001.html>

More information about the OpenStack-dev mailing list