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
Yes.
- Options will be marked as "preview"
Yes.
- 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
item.
- CLIs will note the features are "preview" in their help strings
Yes.
- 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
Right.
- 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,
-Bob
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 <kuk...@noironetworks.com
<mailto:kuk...@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@lists.openstack.org
<mailto:OpenStack-dev@lists.openstack.org>
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
_______________________________________________
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
_______________________________________________
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
