[katello-devel] Katello Backend Service Relationships and Architecture

[Bryan Kearney]

On 12/14/2012 08:51 AM, Petr Chalupa wrote:
>
>
> On 14.12.12 10:05, Lukas Zapletal wrote:
>> On Thu, Dec 13, 2012 at 03:03:00PM -0500, Bryan Kearney wrote:
>>> The original glue layer attempted to enforce the following practices:
>>>
>>> * The controller layer (/controllers and /controllers/api) should
>>> not be aware of the backend engine providing the implementation.
>>> * Any object which the controller would interact with should live in
>>> the model tier (/models), since that is standard rails
>>> * A glue layer (/model/glue) would enhance the model tier and handle
>>> reading/writing to the backend.
>>> * All transport to the backend systems is external (in /lib/resources)
>>
>> I know this thread is about Foreman vs others, but since you replied to
>> my mail I need to write this over and over again. With the current
>> orchestration:
>>
>> We tightly couple model and orchestration while orchestration should
>> be independent. Katello model is just one of the parties we want to
>> integrate with.
>>
>> We can only orchestrate things that have resources in our database and
>> for all the others we need to "hack" alternate models to get this
>> working.
>>
>> We tend to do long running orchestration by hacking model classes (like
>> org.task_id) which is weird - we are abusing models to do orchestration
>> work. Those things are mixed.
>>
>> Our codebase is difficult to understand - we are building chains of
>> callbacks that are deeply hidden in our model objects. Integration
>> proces is a simple function with several statements while we spread it
>> across multiple classes.
>>
>> Errors in our model lead to errors in orchestration. While data
>> inconsistency in our katello db are really bad, they are much easier to
>> fix comparing with inconsistencies across multiple backend engines.
>>
>> Our orchestration code is pretty untestable as a standalone component
>> and the whole codebase is error prone.
>>
>> Katello models are error prone because we have so much code logic in
>> there, so many hooks, callbacks and validations needed for
>> orchestration. They don't need to be there.
>>
>> Our unit tests are either testing models with orchestration turned off
>> or models and orchestration with turned it on. No separation here, tests
>> are mixed into each other.
>>
>> This approach came from foreman where it works pretty well, but
>> Foreman's orchestration is more different from Katello than we thought
>> it will be.
>>
>> I said it like twenty times and I can say it again: Let's move our
>> orchestration out of our models. Let's refactor this while we still have
>> some chance to do this. Now is the time, once we agree this is the
>> "best" approach and maybe rewrite foreman part into this pattern, there
>> is no easy way out.
>>
>
> I agree with Lukas. Our orchestration is fragile and doesn't feel good.
>
> # What I think we can do right now
>
> We could at least start cleaning up the current orchestration code
> without changing its design. I think we would benefit a lot if we just
> removed the model-tangle by replacing modules with classes.
>
> see attachment
>
> ## Image legend
>
> - blue represents gems
> - all black nodes are classes, no modules
>    - nodes with full line are present classes
>    - dashed nodes are classes which aren't implemented
> - black full edge represents oriented knowledge
> - dashed edges represents inheritance
>
> You can modify and regenerate the pdf with graphwiz:
> `dot graph_orchest.dot -Tpdf graph_orchest.dot.pdf`
>
> ## Class legend of new classes
>
> AbstractKatelloRecord: Abstract parent of all katello ActiveRecord models
>
> AbstractOrchestratedRecord: Abstract parent of all katello ActiveRecord
> models with any orchestration.
>
> Glue::Abstract, Glue::Foreman::Abstract, Glue::Pulp::Abstract:  abstract
> parent orchestration classes for all, foreman and pulp
>
> Relation between Glue and ActiveRecords objects: will be very similar to
> a observer pattern. An AbstractOrchestratedRecord descendant (e.g. User
> instance) will send events (generated by ActiveRecord callbacks) to any
> number of registered Glue::Abstract descendants (e.g.
> Glue::Foreman::User instance).
>
> ## Benefits are:
> - separated logic
> - not so fat ActiveRecord models
> - Glue object has its own namespace, no collisions inside model
> - Orchestration switching off/on is simple as un/registering glue
> objects from AbstractOrchestratedRecord descendants
>
> # And for future
>
> I think we should start exploring other solutions. I like Lukas's idea
> to build orchestration around processes (ruote [1]) very much.
>
> [1] http://ruote.rubyforge.org/
>
> Petr
>
> # Notes
>
> How to define Abstract class from ActiveRecord::Base:
>
> class AbstractKatelloRecord < ActiveRecord::Base
>    self.abstract_class = true
>    # ...
> end
>
> This class then doesn't have table, is considered abstract by
> ActiveRecord and doesn't break STI.
>
>
>
>

I assume in picture that Glue::Pulp::User and Pulp::User would be 
concrete classes, not abstract.

-- bk