<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Feb 17, 2016 at 10:23 AM, Bogdan Dobrelya <span dir="ltr"><<a href="mailto:bdobrelia@mirantis.com" target="_blank">bdobrelia@mirantis.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">> So we'll have tons of conditionals in composition layer, right? Even if<br>
> some puppet-openstack class have just one new parameter in new release,<br>
> then we'll have to write a conditional and duplicate class declaration. Or<br>
> write complex parameters hash definitions/merges and use<br>
> create_resources(). The more releases we want to support the more<br>
> complicated composition layer will become. That won't make contribution to<br>
> fuel-library easier and even can greatly reduce development speed. Also are<br>
> we going to add new features to stable releases using this workflow with<br>
> single composition layer?<br>
<br>
As I can see from an example composition [0], such code would be an<br>
unmaintainable burden for development and QA process. Next imagine a<br>
case for incompatible *providers* like network transformations - shall<br>
we put multiple if/case to the ruby providers as well?..<br>
<br>
That is not a way to go for a composition, sorry. While the idea may be<br>
doable, I agree, but perhaps another way.<br></blockquote><div><br></div><div>So I agree that the provided example isn't exactly the cleanest implementation but I think the point is that we need to start considering supporting multiple versions of OpenStack for a given set of fuel-library manifests. I think the provided example begins to show some issues with the way we currently structure our modular tasks.  We basically don't have any organized structure which leads to having to sprinkle random if/elses throughout the code to try and provide some semblance of backwards compatibility or deprecation for tasks.  As I see it today in fuel-library we are too closely tied to the current development version of OpenStack and with every release we are not providing proper deprecation for configuration tasks between each release of fuel. I'm sure fuel plugin developers could probably attest to the amount of pain each release brings trying to re-reverse engineer the deployment process to see what has changed. </div><div><br></div><div>I think it's unrealistic to expect that all users are going to want the latest version of OpenStack as soon as it's released. Fuel is an OpenStack deployment tool, so why can't we deploy different versions with a single version of fuel?  Technically we should have a way to do with with releases in Fuel but it seems that we've handicapped any ability to try and leverage this information to support a different version of OpenStack.  Much like the UCA work where we started to extract out the package/repo information, we need to extract out the OpenStack release information (version/repos/etc). We should allow a user to specify that they want a build a cloud using X fuel release to deploy Y os with Z OpenStack release.  If we can't provide this type of flexibility I'm not seeing why someone would want to use Fuel over their own OS provisioning+puppet/ansible/chef deployment method.</div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
(tl;dr)<br>
By the way, this reminded me "The wrong abstraction" [1] article and<br>
discussion. I agree with the author and believe one should not group<br>
code (here it is versioned puppet modules & compositions) in a way which<br>
introduces abstractions (here a super-composition) with multiple<br>
if/else/case and hardcoded things to switch the execution flow based on<br>
version of things. Just keep code as is - partially duplicated by<br>
different releases in separate directories with separate modules and<br>
composition layers and think of better solutions please.<br></blockquote><div><br></div><div>Completely duplicating the fuel-library for each OpenStack release is probably even more unmanageable then beginning to structure our composition layer into something that supports multiple OpenStack resources but I guess that could be an option...</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
There is also a nice comment: "...try to optimize my code around<br>
reducing state, coupling, complexity and code, in that order". I<br>
understood that like a set of "golden rules":<br>
- Make it coupled more tight to decrease (shared) state<br>
- Make it more complex to decrease coupling<br>
- Make it duplicated to decrease complexity (e.g. abstractions)<br>
<br>
(tl;dr, I mean it)<br>
So, bringing those here.<br>
- The shared state is perhaps the Nailgun's world view of all data and<br>
versioned serializers for supported releases, which know how to convert<br>
the only latest existing data to any of its supported previous versions.<br>
- Decoupling we do by putting modules with its compositions to different<br>
versioned /etc/puppet subdirectories. I'm not sure how do we decouple<br>
Nailgun serializers though.<br>
- Complexity is how we compose those modules / write logic of serializers.<br>
- Duplication is puppet classes (and providers) with slightly different<br>
call parameters from a version to version. Sometimes even not backwards<br>
compatible. Probably same to the serializers?<br>
<br>
So, we're going to *increase complexity* by introducing<br>
super-compositions for multi OpenStack releases. Not sure about what to<br>
happen to the serializers, any volunteers to clarify an impact?. And the<br>
Rules "allow" us to do so only in order to decrease either coupling or<br>
shared state, which is not the case, AFAICT. Modules with compositions<br>
are separated well by OpenStack versions, nothing to decrease. Might<br>
that change to decrease a shared state? I'm not sure if it even applies<br>
here. Puppet versioning shares nothing. Only Nailgun folks may know the<br>
answer.<br>
<br></blockquote><div><br></div><div>I don't think we have to increase complexity if we properly structure fuel-library. I think we could also structure it in a way that can be tested in an automated fashion. I think we can come up with a set of rules for tasks and how to implement a new task that can reduce complexity and actually allow for code reuse.  If you take a look at our tasks today there is actually an excessive amount of duplication between tasks[0][1] around variable and configuration gathering before we even call the upstream OpenStack modules.  The tasks we have today are basically just undocumented freeform puppet that can only be fully tested via multiple deployment tests because of the lack of decent test coverage.  Additionally, we have no real way of testing deprecation between releases today because there's no formal api contract for modular tasks themselves.</div><div><br></div><div>I would be interested in investigating a restructure of the tasks that might go something like....</div><div>1) Restructure fuel-library tasks into a configuration generation/formatting method (extracting data from hiera/globals/etc) and the actual configuration application logic</div><div>2) For the configuration gathering items that we traditionally load up at the top of our tasks, we could investigate leveraging hiera for providing data to classes or structure into an osnailyfacter::config::<thing> class.</div><div>2) Move the actual configuration application logic into an osnailyfacter::task::<thing> method with a proper documented api contract with deprecation policy. These classes could use something like create_resources(...) to dynamically load the classes with the provided parameters to support the class api changes between upstream module versions.</div><div>3) Add release specific overrides into osnailyfacter::task::<thing>::<release> where ::<release> could be automatically included based on something like hiera('openstack_version')</div><div><br></div><div>There are a few added benefits of restructuring the tasks into traditional puppet classes.</div><div>1) If we need to support any sort of traditional puppet master LCM for nodes, by moving the tasks into specific class we can actually just leverage our already written task code to ensure configurations.</div><div>2) Testability around idempotency as we can write beaker tests to be able to test deployment tasks for idempotency and deployment without having to stand up all the other pieces of fuel.  Similar to the Puppet OpenStack's puppet-openstack-integration[2] module.</div><div>3) Better defined separation and containment of release specific items so when we are moving forward.  It's much easier to remove n-X release as we could just 'find deployment/puppet/osnailyfacter/manifests/ -name 'kilo.pp' -delete'</div><div><br></div><div>That being said, this would be a lot of work but it's mostly taking what we have today and reorganizing it rather than having to write things from scratch and providing some policy rules around structure of tasks.  I think in the long run we could benefit by being able to test fuel-library tasks with existing tools rather than continually having to rely on noop or actual deployment tests with nailgun/astute/isos/etc.  This is just a starting point for a conversation and I think we should have a serious discussion about this topic and not just dismiss it because it's hard or might add complexity. </div><div><br></div><div><br></div><div>Thanks,</div><div>-Alex</div><div> </div><div>[0] <a href="https://github.com/openstack/fuel-library/blob/master/deployment/puppet/osnailyfacter/modular/glance/glance.pp#L3-L99" target="_blank">https://github.com/openstack/fuel-library/blob/master/deployment/puppet/osnailyfacter/modular/glance/glance.pp#L3-L99</a></div><div>[1] <a href="https://github.com/openstack/fuel-library/blob/master/deployment/puppet/osnailyfacter/modular/keystone/keystone.pp#L3-L115" target="_blank">https://github.com/openstack/fuel-library/blob/master/deployment/puppet/osnailyfacter/modular/keystone/keystone.pp#L3-L115</a></div><div>[2] <a href="https://github.com/openstack/puppet-openstack-integration/" target="_blank">https://github.com/openstack/puppet-openstack-integration/</a></div></div></div></div>