<div dir="ltr">Hi, <div><br></div><div>I've been thinking a good bit on this on the right way to move forward with this and in general the right way new services should be added. Yesterday I was working on a bug that was causing some problems in the openstack infra. We tracked down the issue then I uploaded a patch for it. A little while later after jenkins voted back a -1 so I started looking through the logs to see what the source of the failure was (which was actually unrelated to my patch). The random failure in the fwaas/vpn/l3-agent code which all outputs to the same log file that contains many traces for every run even successful ones. In one skim of this log file I was able to spot 4 [1]bugs which shows  these new "experimental" services that we've added to neutron have underlying problems (even though they've been in the tree for 2 releases+ now). This puts a huge strain on the whole openstack development community as they are always recheck no bug'ing due to neutron failures. </div>
<div><br></div><div>If you look at the fwaas work that was done. This merged over two releases ago and still does not have a complete API as there is no concept of where enforcement should be done. Today, enforcement is done across all of a tenant's routers making it more or less useless imho and we're just carrying it along in the tree (and it's causing us problems)!</div>
<div><br></div><div>I think Mark's idea of neutron-incubator[2] is a great step forward to improving neutron. </div><div><br></div><div>We can easily move these things out of the neutron source tree and we can plug these things in here: </div>
<div><a href="https://github.com/openstack/neutron/blob/master/etc/neutron.conf#L52">https://github.com/openstack/neutron/blob/master/etc/neutron.conf#L52</a><br></div><div><a href="https://github.com/openstack/neutron/blob/master/etc/neutron.conf#L72">https://github.com/openstack/neutron/blob/master/etc/neutron.conf#L72</a><br>
</div><div>(GASP: We have seen shipped our own API's here to customers before we were able to upstream them).</div><div><br></div><div>This allows us to decouple these experimental things from the neutron core and allows us to release these components on their own making things more modular/maintainable and stable (I think these things might even be better long term living out of the tree).  Most importantly though it doesn't put a burden on everyone else.  </div>
<div><br></div><div><br></div><div>Best, </div><div><br></div><div>Aaron</div><div><br></div><div><br></div><div>[1]</div><div><a href="http://paste.openstack.org/show/94664/">http://paste.openstack.org/show/94664/</a><br>
</div><div><a href="http://paste.openstack.org/show/94670/">http://paste.openstack.org/show/94670/</a><br></div><div><a href="http://paste.openstack.org/show/94663/">http://paste.openstack.org/show/94663/</a><br></div><div>
<a href="http://paste.openstack.org/show/94662/">http://paste.openstack.org/show/94662/</a><br></div><div><br></div><div>[2] - <a href="https://etherpad.openstack.org/p/neutron-incubator">https://etherpad.openstack.org/p/neutron-incubator</a><br>
</div><div><br></div><div><br></div><div><br></div><div><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Aug 11, 2014 at 9:58 AM, Robert Kukura <span dir="ltr"><<a href="mailto:kukura@noironetworks.com" target="_blank">kukura@noironetworks.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
  
    
  
  <div bgcolor="#FFFFFF" text="#000000"><div class="">
    <br>
    <div>On 8/8/14, 6:28 PM, Salvatore Orlando
      wrote:<br>
    </div>
    <blockquote type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div>"If we want to keep everything the way it is, we have to
            change everything" [1]<br>
          </div>
          <div><br>
          </div>
          <div>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.</div>
          <div>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. </div>
          <div><br>
          </div>
          <div>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.</div>
        </div>
      </div>
    </blockquote></div>
    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.<div class=""><br>
    <blockquote type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div><br>
          </div>
          <div>According to this proposal, for preview features:</div>
          <div>- all the code would be moved to a "preview" package</div>
        </div>
      </div>
    </blockquote></div>
    Yes.<div class=""><br>
    <blockquote type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div>- Options will be marked as "preview"</div>
        </div>
      </div>
    </blockquote></div>
    Yes.<div class=""><br>
    <blockquote type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div>- URIs should be prefixed with "preview"</div>
        </div>
      </div>
    </blockquote></div>
    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.<div class=""><br>
    <blockquote type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div>- CLIs will note the features are "preview" in their help
            strings</div>
        </div>
      </div>
    </blockquote></div>
    Yes.<div class=""><br>
    <blockquote type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div>- 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)</div>
        </div>
      </div>
    </blockquote></div>
    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.<div class=""><br>
    <blockquote type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div>- Database migrations will be in the main alembic path as
            usual</div>
        </div>
      </div>
    </blockquote></div>
    Right.<div class=""><br>
    <blockquote type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div>- CLI, Devstack and Heat support will be available</div>
        </div>
      </div>
    </blockquote></div>
    Right, as appropriate for the feature.<div class=""><br>
    <blockquote type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div>- Can be used by non-preview neutron code</div>
        </div>
      </div>
    </blockquote></div>
    No, I suggested "No non-preview Neutron code should import code from
    anywhere under the neutron.preview module, ...".
    <div class=""><blockquote type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div>- Will undergo the usual review process</div>
        </div>
      </div>
    </blockquote></div>
    Right. This is key for the code to not have to jump through a new
    major upheaval at right as it becomes stable.<div class=""><br>
    <blockquote type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div>- QA will be desirable, but will done either with "WIP"
            tempest patches or merging the relevant scenario tests in
            the preview feature iself</div>
        </div>
      </div>
    </blockquote></div>
    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.<div class=""><br>
    <blockquote type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div>- The feature might be promoted or removed, but the
            process for this is not yet defined.</div>
        </div>
      </div>
    </blockquote></div>
    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.<div class=""><br>
    <blockquote type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div><br>
          </div>
          <div>I don't think this change in process will actually
            encourage better behaviour both by contributors and core
            reviewers.</div>
        </div>
      </div>
    </blockquote></div>
    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.<div class=""><br>
    <blockquote type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div>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.</div>
        </div>
      </div>
    </blockquote></div>
    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.<div class=""><br>
    <blockquote type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div>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.</div>
          <div><br>
          </div>
          <div>Looking at this proposal I see a few thing I'm not
            comfortable with:</div>
          <div>- 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</div>
        </div>
      </div>
    </blockquote></div>
    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.<div class=""><br>
    <blockquote type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div>- 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.</div>
        </div>
      </div>
    </blockquote></div>
    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.<div class=""><br>
    <blockquote type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div>- 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.</div>
        </div>
      </div>
    </blockquote></div>
    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.<div class=""><br>
    <blockquote type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div>- 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.</div>
        </div>
      </div>
    </blockquote></div>
    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.<div class=""><br>
    <blockquote type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div><br>
          </div>
          <div>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:</div>
          <div>- whether is acceptable to change the process now</div>
          <div>- 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.</div>
        </div>
      </div>
    </blockquote></div>
    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.<br>
    <br>
    Thanks for your indulgence as well,<br>
    <br>
    -Bob<div><div class="h5"><br>
    <blockquote type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div><br>
          </div>
          <div>Have a good weekend,<br>
          </div>
          <div>Salvatore</div>
          <div><br>
          </div>
          <div>[1] Quote from "Il Gattopardo" by Giuseppe Tomasi di
            Lampedusa (english name: The Leopard)</div>
          <div><br>
          </div>
          <br>
          <div class="gmail_quote">
            On 8 August 2014 22:21, Robert Kukura <span dir="ltr"><<a href="mailto:kukura@noironetworks.com" target="_blank">kukura@noironetworks.com</a>></span>
            wrote:</div>
          <div class="gmail_quote"><br>
          </div>
          <div class="gmail_quote">
            <br>
            <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">[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.]<br>
              <br>
              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.<br>
              <br>
              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.<br>
              <br>
              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.<br>
              <br>
              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.<br>
              <br>
              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.<br>
              <br>
              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:<br>
              <br>
              * 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.<br>
              <br>
              * 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.<br>
              <br>
              * 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.<br>
              <br>
              * 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.<br>
              <br>
              * 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.<br>
              <br>
              * 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.<br>
              <br>
              * 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.<br>
              <br>
              * 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.<br>
              <br>
              * No non-preview Neutron code should import code from
              anywhere under the neutron.preview module, unless
              necessary for special cases like DB migrations.<br>
              <br>
              * URIs for the resources provided by preview features
              should contain the string "preview".<br>
              <br>
              * Configuration file content related to preview features
              should be clearly labeled as "preview".<br>
              <br>
              * 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".<br>
              <br>
              * 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.<br>
              <br>
              * 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).<br>
              <br>
              <br>
              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 <a href="https://etherpad.openstack.org/p/neutron-preview-features" target="_blank">https://etherpad.openstack.org/p/neutron-preview-features</a>
              for those interested in helping refine the proposal.<br>
              <br>
              Thanks,<br>
              <br>
              -Bob<br>
              <br>
              <br>
              _______________________________________________<br>
              OpenStack-dev mailing list<br>
              <a href="mailto:OpenStack-dev@lists.openstack.org" target="_blank">OpenStack-dev@lists.openstack.org</a><br>
              <a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
            </blockquote>
          </div>
          <br>
        </div>
      </div>
      <br>
      <fieldset></fieldset>
      <br>
      <pre>_______________________________________________
OpenStack-dev mailing list
<a href="mailto:OpenStack-dev@lists.openstack.org" target="_blank">OpenStack-dev@lists.openstack.org</a>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a>
</pre>
    </blockquote>
    <br>
  </div></div></div>

<br>_______________________________________________<br>
OpenStack-dev mailing list<br>
<a href="mailto:OpenStack-dev@lists.openstack.org">OpenStack-dev@lists.openstack.org</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
<br></blockquote></div><br></div>