Re: [openstack-dev] [Ironic] Adding vendor drivers in Ironic
On Sun, Mar 1, 2015 at 8:45 AM, Clint Byrum wrote: > Excerpts from Gary Kotton's message of 2015-03-01 02:32:37 -0800: >> Hi, >> I am just relaying pain-points that we encountered in neutron. As I have >> said below it makes the development process a lot quicker for people >> working on external drivers. I personally believe that it fragments the >> community and feel that the external drivers loose the community >> contributions and inputs. > > I think you're right that this does change the dynamic in the > community. One way to lower the barrier is to go ahead and define the > plugin API very strongly, but then delegate control of drivers in-tree > to active maintainers, rather than in external repositories. If a driver > falls below the line in terms of maintenance, then it can be deprecated. > And if a maintainer feels strongly that they cannot include the driver > with Ironic for whatever reason, the plugin API being strongly defined > will allow them to do so. > ++ on all counts. Even with delegation of existing drivers to an active driver maintainer(s), there is still a cost to the core review team: new driver submissions have generally not come from existing core reviewers. That could be mitigated if we were to encourage new drivers to be developed and proven out of tree while the author becomes active in the "parent" project, then when the core team feels ready, allow the driver in-tree and delegate its maintenance. -D __ 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
Re: [openstack-dev] [Ironic] Adding vendor drivers in Ironic
I know I'm arriving late to this thread, but I want to chime in with a resounding "yes!" to this approach. We've held to a fairly strict policy around maintaining compatibility within the driver API since early on, and we'll continue to do that as we add new interfaces (see the new ManagementInterface) or enhance existing ones (like the new @clean_step decorator). I believe that, as a hardware abstraction layer, we need to enable authors of vendor-specific plugins/drivers to innovate out of tree -- whether that is through a driver + library, such as stackforge/proliantutils, or in a completely out of tree driver (I'm not aware of any open source examples of this yet, but I have heard of private ones). As far as the two (dis)advantages you list: 1) vendor drivers reviewed by the community On the one hand, I believe there is some benefit from the broader OpenStack community reviewing vendor code. Some... but not much. Architecture and general principles (is it scalable? are you doing something that operators will hate?) is valuable, but that's where it stops. As you pointed out, it is impractical to expect open source developers (or developers at other hardware companies) to have the knowledge and expertise necessary (or hardware available to them) to determine whether or not vendor-specific code will work with specific vendor hardware. As an interesting data point, we now have support for 8 different types of hardware drivers in tree (10 if you count the ssh and vbox drivers that we use in testing). 2) distros (not) packaging dependencies Because we can not actually test every driver's dependencies in the upstream gate (there's no hardware to exercise against), we do not install those packages in devstack-gate, and so we do not list driver's python dependencies in requirements.txt. We mock those libraries and test the in-tree driver code, and rely on the library and driver authors to ensure their fitness for use against real hardware. So, yes, third-party-CI is essential to this -- and I wish more vendors would step up and run CI on their drivers. But I digress... After talking with Thomas Goirand about debian packaging of Ironic, we realized that there wasn't a clear list of our driver's dependencies. So to provide distros with a single location for this information, we are now maintaining a "driver-requirements.txt" file. It's fairly new -- I added it about a month ago. See https://github.com/openstack/ironic/blob/master/driver-requirements.txt Thanks for writing up your experiences with this, Ramakrish. I believe it is a great example to other driver authors -- and it would be great to have a wiki or doc page describing the how's and why's of this approach. Cheers, Devananda On Fri, Feb 27, 2015 at 10:28 PM, 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/proli
Re: [openstack-dev] [Ironic] Adding vendor drivers in Ironic
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 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// 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 n
Re: [openstack-dev] [Ironic] Adding vendor drivers in Ironic
On Sun, Mar 1, 2015 at 4:32 AM, Gary Kotton wrote: > Hi, > I am just relaying pain-points that we encountered in neutron. As I have > said below it makes the development process a lot quicker for people > working on external drivers. I personally believe that it fragments the > community and feel that the external drivers loose the community > contributions and inputs. > Thanks > Gary > > I find it unfair to say that the decomposition in Neutron caused fragmentation. As of Kilo-2, we had 48 drivers/plugins in-tree in Neutron, with another 10 or so proposed for Kilo. It's not scalable to continue down that path! Further, most of those drivers/plugins were upstream but the contributors were not really contributing to Neutron outside of their own plugins/drivers. The decomposition lets them contribute and merge where they are already contributing: In their own plugins/drivers. It also removes a lot of code from Neutron which most core reviewers could never test or run due to lacking proprietary hardware or software. All of these reasons were in the decomposition spec [1]. I've not heard from anyone else who thinks the decomposition is a bad idea or is not going well in practice. The opposite has actually been true: Everyone has been happy with it's execution and the results it's allowing. I credit Armando for his great work leading this effort. It's been a huge effort but the results have been pretty amazing. Thanks, Kyle [1] http://specs.openstack.org/openstack/neutron-specs/specs/kilo/core-vendor-decomposition.html On 2/28/15, 7:58 PM, "Clint Byrum" wrote: > > >I'm not sure I understand your statement Gary. If Ironic defines > >what is effectively a plugin API, and the vendor drivers are careful > >to utilize that API properly, the two sets of code can be released > >entirely independent of one another. This is how modules work in the > >kernel, X.org drivers work, and etc. etc. Of course, vendors could be > >irresponsible and break compatibility with older releases of Ironic, > >but that is not in their best interest, so I don't see why anybody would > >need to tightly couple. > > > >As far as where generic code goes, that seems obvious: it all has to go > >into Ironic and be hidden behind the plugin API. > > > >Excerpts from Gary Kotton's message of 2015-02-28 09:28:55 -0800: > >> Hi, > >> There are pros and cons for what you have mentioned. My concern, and I > >>mentioned them with the neutron driver decomposition, is that we are are > >>loosing the community inputs and contributions. Yes, one can certainly > >>move faster and freer (which is a huge pain point in the community). How > >>are generic code changes percolated to your repo? Do you have an > >>automatic CI that detects this? Please note that when itonic release you > >>will need to release your repo so that the relationship is 1:1... > >> Thanks > >> Gary > >> > >> From: Ramakrishnan G > >>mailto:rameshg87.openst...@gmail.com>> > >> Reply-To: OpenStack List > >> openstack-dev@lists.openstack.o > >>rg>> > >> Date: Saturday, February 28, 2015 at 8:28 AM > >> To: OpenStack List > >> openstack-dev@lists.openstack.o > >>rg>> > >> Subject: [openstack-dev] [Ironic] Adding vendor drivers in Ironic > >> > >> > >> 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. (
Re: [openstack-dev] [Ironic] Adding vendor drivers in Ironic
On 02/28/2015 07:28 AM, Ramakrishnan G wrote: Hello All, Hi! 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// 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). 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 writing this out! I understand the concern about splitting this community effort, however, I tend to agree with you. Reviewing vendor-specific code does make me feel weird: on one hand I do my best to check it, on another - I realize that I can approve code that has no chances of working just because I can and will miss some hardware-specific detail. My biggest concern in making it a rule is support level for this external code. I'm not questioning quality of proliantutils :) but I can envision people not putting too much effort into maintaining their 3rd part stuff. E.g. I already saw 3rd party module that is not even on PyPI (and does not have any tests). I don't know what to do in this cas
Re: [openstack-dev] [Ironic] Adding vendor drivers in Ironic
Excerpts from Gary Kotton's message of 2015-03-01 02:32:37 -0800: > Hi, > I am just relaying pain-points that we encountered in neutron. As I have > said below it makes the development process a lot quicker for people > working on external drivers. I personally believe that it fragments the > community and feel that the external drivers loose the community > contributions and inputs. I think you're right that this does change the dynamic in the community. One way to lower the barrier is to go ahead and define the plugin API very strongly, but then delegate control of drivers in-tree to active maintainers, rather than in external repositories. If a driver falls below the line in terms of maintenance, then it can be deprecated. And if a maintainer feels strongly that they cannot include the driver with Ironic for whatever reason, the plugin API being strongly defined will allow them to do so. __ 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
Re: [openstack-dev] [Ironic] Adding vendor drivers in Ironic
Hi, I am just relaying pain-points that we encountered in neutron. As I have said below it makes the development process a lot quicker for people working on external drivers. I personally believe that it fragments the community and feel that the external drivers loose the community contributions and inputs. Thanks Gary On 2/28/15, 7:58 PM, "Clint Byrum" wrote: >I'm not sure I understand your statement Gary. If Ironic defines >what is effectively a plugin API, and the vendor drivers are careful >to utilize that API properly, the two sets of code can be released >entirely independent of one another. This is how modules work in the >kernel, X.org drivers work, and etc. etc. Of course, vendors could be >irresponsible and break compatibility with older releases of Ironic, >but that is not in their best interest, so I don't see why anybody would >need to tightly couple. > >As far as where generic code goes, that seems obvious: it all has to go >into Ironic and be hidden behind the plugin API. > >Excerpts from Gary Kotton's message of 2015-02-28 09:28:55 -0800: >> Hi, >> There are pros and cons for what you have mentioned. My concern, and I >>mentioned them with the neutron driver decomposition, is that we are are >>loosing the community inputs and contributions. Yes, one can certainly >>move faster and freer (which is a huge pain point in the community). How >>are generic code changes percolated to your repo? Do you have an >>automatic CI that detects this? Please note that when itonic release you >>will need to release your repo so that the relationship is 1:1... >> Thanks >> Gary >> >> From: Ramakrishnan G >>mailto:rameshg87.openst...@gmail.com>> >> Reply-To: OpenStack List >>mailto:openstack-dev@lists.openstack.o >>rg>> >> Date: Saturday, February 28, 2015 at 8:28 AM >> To: OpenStack List >>mailto:openstack-dev@lists.openstack.o >>rg>> >> Subject: [openstack-dev] [Ironic] Adding vendor drivers in Ironic >> >> >> 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://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_stackforg >>e_proliantutils_blob_master_proliantutils_ilo_operations.py&d=AwICAg&c=Sq >>cl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=VlZxHpZBmzzkWT5jqz9JYBk8YTeq9 >>N3-diTlNj4GyNc&m=QRyrevJwoHB6GFxiTDorDRShZ79rnf-SwtdVwGiYfcc&s=e9_q3eOLqT >>eI3oNwT_0fur3qzpFLUy9wxVPEjujfAMs&e= >><https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_stackfor >>ge_proliantutils_blob_master_proliantutils_ilo_operations.py&d=AwMFaQ&c=S >>qcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=VlZxHpZBmzzkWT5jqz9JYBk8YTeq >>9N3-diTlNj4GyNc&m=m5_FxZnmz3 > cyIvavSV > >DImH6xLR79L-svbcYKkjdcnb8&s=fjlOB2ORYcne-cyYnZJO8bdpi4J8rbfCAbmciPllmFI&e= >>). >> >> 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)
Re: [openstack-dev] [Ironic] Adding vendor drivers in Ironic
> 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 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// 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
Re: [openstack-dev] [Ironic] Adding vendor drivers in Ironic
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// 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 Ironi
Re: [openstack-dev] [Ironic] Adding vendor drivers in Ironic
I'm not sure I understand your statement Gary. If Ironic defines what is effectively a plugin API, and the vendor drivers are careful to utilize that API properly, the two sets of code can be released entirely independent of one another. This is how modules work in the kernel, X.org drivers work, and etc. etc. Of course, vendors could be irresponsible and break compatibility with older releases of Ironic, but that is not in their best interest, so I don't see why anybody would need to tightly couple. As far as where generic code goes, that seems obvious: it all has to go into Ironic and be hidden behind the plugin API. Excerpts from Gary Kotton's message of 2015-02-28 09:28:55 -0800: > Hi, > There are pros and cons for what you have mentioned. My concern, and I > mentioned them with the neutron driver decomposition, is that we are are > loosing the community inputs and contributions. Yes, one can certainly move > faster and freer (which is a huge pain point in the community). How are > generic code changes percolated to your repo? Do you have an automatic CI > that detects this? Please note that when itonic release you will need to > release your repo so that the relationship is 1:1... > Thanks > Gary > > From: Ramakrishnan G > mailto:rameshg87.openst...@gmail.com>> > Reply-To: OpenStack List > mailto:openstack-dev@lists.openstack.org>> > Date: Saturday, February 28, 2015 at 8:28 AM > To: OpenStack List > mailto:openstack-dev@lists.openstack.org>> > Subject: [openstack-dev] [Ironic] Adding vendor drivers in Ironic > > > 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<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_stackforge_proliantutils_blob_master_proliantutils_ilo_operations.py&d=AwMFaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=VlZxHpZBmzzkWT5jqz9JYBk8YTeq9N3-diTlNj4GyNc&m=m5_FxZnmz3cyIvavSV DImH6xLR79L-svbcYKkjdcnb8&s=fjlOB2ORYcne-cyYnZJO8bdpi4J8rbfCAbmciPllmFI&e=>). > > 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<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_stackforge_proliantutils_blob_master_proliantutils_ilo_ris.py&d=AwMFaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=VlZxHpZBmzzkWT5jqz9JYBk8YTeq9N3-diTlNj4GyNc&m=m5_FxZnmz3cyIvavSVDImH6xLR79L-svbcYKkjdcnb8&s=vYNQ8MopljQOqje3T_aIhtw0oZPK4tFHGnlcbBH6wac&e=>) > 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 workin
Re: [openstack-dev] [Ironic] Adding vendor drivers in Ironic
Hi, There are pros and cons for what you have mentioned. My concern, and I mentioned them with the neutron driver decomposition, is that we are are loosing the community inputs and contributions. Yes, one can certainly move faster and freer (which is a huge pain point in the community). How are generic code changes percolated to your repo? Do you have an automatic CI that detects this? Please note that when itonic release you will need to release your repo so that the relationship is 1:1... Thanks Gary From: Ramakrishnan G mailto:rameshg87.openst...@gmail.com>> Reply-To: OpenStack List mailto:openstack-dev@lists.openstack.org>> Date: Saturday, February 28, 2015 at 8:28 AM To: OpenStack List mailto:openstack-dev@lists.openstack.org>> Subject: [openstack-dev] [Ironic] Adding vendor drivers in Ironic 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<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_stackforge_proliantutils_blob_master_proliantutils_ilo_operations.py&d=AwMFaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=VlZxHpZBmzzkWT5jqz9JYBk8YTeq9N3-diTlNj4GyNc&m=m5_FxZnmz3cyIvavSVDImH6xLR79L-svbcYKkjdcnb8&s=fjlOB2ORYcne-cyYnZJO8bdpi4J8rbfCAbmciPllmFI&e=>). 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<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_stackforge_proliantutils_blob_master_proliantutils_ilo_ris.py&d=AwMFaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=VlZxHpZBmzzkWT5jqz9JYBk8YTeq9N3-diTlNj4GyNc&m=m5_FxZnmz3cyIvavSVDImH6xLR79L-svbcYKkjdcnb8&s=vYNQ8MopljQOqje3T_aIhtw0oZPK4tFHGnlcbBH6wac&e=>) 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// 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 yo
[openstack-dev] [Ironic] Adding vendor drivers in Ironic
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// 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). 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
