> 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 <[email protected]> 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: > [email protected]?subject:unsubscribe > > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > > > > > __________________________________________________________________________ > OpenStack Development Mailing List (not for usage questions) > Unsubscribe: [email protected]?subject:unsubscribe > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >
__________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: [email protected]?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
