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