> You may not realize you do a disservice to those reading this post and
> those reviewing future patches if you set unreasonable expectations.

> Telling others that they can expect a patch merged in the same day is
> not reasonable, even if that has been your experience. While we do our
> best to keep current, we all are very busy and requests for repos are
> increasing. If folks want a repo they can submit a patch to create one,
> here is a good guide:
> http://docs.openstack.org/infra/manual/creators.html and it will be
> reviewed along with all other patches to project-config.

Anita,

Thanks for correcting me.  Yeah, I just quoted *my experience with
openstack-infra *blindly.  Sorry for that.

Rather I also wanted to point out to our folks, things in infra are so
automated that putting an openstack-related module into stackforge has been
become fully automatic and easy *(easy for the requestor, of course keeping
in mind that the request has to be correct and get's reviewed and approved
by  infra guys)*.  Kudos to you guys :-)

Regards,
Ramesh


On Sun, Mar 1, 2015 at 12:49 AM, Anita Kuno <ante...@anteaya.info> wrote:

> On 02/28/2015 01:28 AM, Ramakrishnan G wrote:
> > Hello All,
> >
> > This is about adding vendor drivers in Ironic.
> >
> > In Kilo, we have many vendor drivers getting added in Ironic which is a
> > very good thing.  But something I noticed is that, most of these reviews
> > have lots of hardware-specific code in them.  This is something most of
> the
> > other Ironic folks cannot understand unless they go and read the hardware
> > manuals of the vendor hardware about what is being done.  Otherwise we
> just
> > need to blindly mark the file as reviewed.
> >
> > Now let me pitch in with our story about this.  We added a vendor driver
> > for HP Proliant hardware (the *ilo drivers in Ironic).  Initially we
> > proposed this same thing in Ironic that we will add all the hardware
> > specific code in Ironic itself under the directory drivers/modules/ilo.
> > But few of the Ironic folks didn't agree on this (Devananda especially
> who
> > is from my company :)). So we created a new module proliantutils, hosted
> in
> > our own github and recently moved it to stackforge.  We gave a limited
> set
> > of APIs for Ironic to use - like get_host_power_status(),
> set_host_power(),
> > get_one_time_boot(), set_one_time_boot(), etc. (Entire list is here
> >
> https://github.com/stackforge/proliantutils/blob/master/proliantutils/ilo/operations.py
> > ).
> >
> > We have only seen benefits in doing it.  Let me bring in some examples:
> >
> > 1) We tried to add support for some lower version of servers.  We could
> do
> > this without making any changes in Ironic (Review in proliantutils
> > https://review.openstack.org/#/c/153945/)
> > 2) We are adding support for newer models of servers (earlier we use to
> > talk to servers in protocol called RIBCL, newer servers we will use a
> > protocol called RIS) - We could do this with just 14 lines of actual code
> > change in Ironic (this was needed mainly because we didn't think we will
> > have to use a new protocol itself when we started) -
> > https://review.openstack.org/#/c/154403/
> >
> > Now talking about the advantages of putting hardware-specific code in
> > Ironic:
> >
> > *1) It's reviewed by Openstack community and tested:*
> > No. I doubt if I throw in 600 lines of new iLO specific code that is
> here (
> >
> https://github.com/stackforge/proliantutils/blob/master/proliantutils/ilo/ris.py
> )
> > for Ironic folks, they will hardly take a look at it.  And regarding
> > testing, it's not tested in the gate unless we have a 3rd party CI for
> it.
> >  [We (iLO drivers) also don't have 3rd party CI right now, but we are
> > working on it.]
> >
> > *2) Everything gets packaged into distributions automatically:*
> > Now the hardware-specific code that we add in Ironic under
> > drivers/modules/<vendor>/ will get packaged into distributions, but this
> > code in turn will have dependencies  which needs to be installed manually
> > by the operator (I assume vendor specific dependencies are not considered
> > by Linux distributions while packaging Openstack Ironic). Anyone
> installing
> > Ironic and wanting to manage my company's servers will again need to
> > install these dependencies manually.  Why not install the wrapper if
> there
> > is one too.
> >
> > I assume we only get these advantages by moving all of hardware-specific
> > code to a wrapper module in stackforge and just exposing some APIs for
> > Ironic to use:
> > * Ironic code would be much cleaner and easier to maintain
> > * Any changes related to your hardware - support for newer hardware, bug
> > fixes in particular models of hardware, would be very easy. You don't
> need
> > to change Ironic code for that. You could just fix the bug in your
> module,
> > release a new version and ask your users to install a newer version of
> the
> > module.
> > * python-fooclient could be used outside Ironic to easily manage foo
> > servers.
> > * Openstack CI for free if you are in stackforge - unit tests, flake
> tests,
> > doc generation, merge, pypi release everything handled automatically.
> >
> > I don't see any disadvantages.
> >
> > Now regarding the time taken to do this, if you have all the code ready
> now
> > in Ironic (which assume you will already have), perhaps it will take a
> day
> > to do this - half a day for putting into a separate module in
> python/github
> > and half a day for stackforge. The request to add stackforge should get
> > approved in the same day (if everything is all-right).
> You may not realize you do a disservice to those reading this post and
> those reviewing future patches if you set unreasonable expectations.
>
> Telling others that they can expect a patch merged in the same day is
> not reasonable, even if that has been your experience. While we do our
> best to keep current, we all are very busy and requests for repos are
> increasing. If folks want a repo they can submit a patch to create one,
> here is a good guide:
> http://docs.openstack.org/infra/manual/creators.html and it will be
> reviewed along with all other patches to project-config.
>
> Thank you,
> Anita.
> >
> > Let me know all of your thoughts on this.  If we agree, I feel we should
> > have some documentation on it in our Ironic docs directory.
> >
> > Thanks for reading :)
> >
> > Regards,
> > Ramesh
> >
> >
> >
> >
> __________________________________________________________________________
> > OpenStack Development Mailing List (not for usage questions)
> > Unsubscribe:
> openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> >
>
>
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Reply via email to