On 02/28/2015 09:36 PM, Ramakrishnan G wrote: >> 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 You are welcome Ramesh.
I am glad you are having a good experience dealing with the infra team. Going forward please be informed that I am a woman, I am not a guy. The infra team has some members which are female. Thank you, Anita. > > > 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 > __________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: [email protected]?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
