[Freeipa-devel] kra an ca instance installation

[Jan Cholasta]

On 25.8.2015 17:50, Simo Sorce wrote:
> On Tue, 2015-08-25 at 16:22 +0200, Jan Cholasta wrote:
>> On 25.8.2015 16:09, Simo Sorce wrote:
>>> On Tue, 2015-08-25 at 15:52 +0200, Jan Cholasta wrote:
>>>> On 25.8.2015 15:37, Simo Sorce wrote:
>>>>> On Tue, 2015-08-25 at 11:34 +0200, Jan Cholasta wrote:
>>>>>> On 25.8.2015 11:25, Jan Cholasta wrote:
>>>>>>> On 24.8.2015 19:51, Simo Sorce wrote:
>>>>>>>> Why do we have cainstance.py and ca.py and krainstance.py and kra.py in
>>>>>>>> ipaserver/install when you always need both files to do anything around
>>>>>>>> installation of the ca ?
>>>>>>>>
>>>>>>>> Is there a motivation ?
>>>>>>>> Or can I simply provide a patch to remove the ca.py and kra.py files an
>>>>>>>> unify all code in the proper *instance.py file ?
>>>>>>>
>>>>>>> ca.py and kra.py are the proper files ready to be migrated to the new
>>>>>>> install framework, cainstance.py and krainstance.py will be removed.
>>>>>>
>>>>>> ... once the migration is done. (Hit send button too fast.)
>>>>>>
>>>>>> The motivation is that *instance.py do not provide a uniform interface,
>>>>>> have a lot of redundant and duplicate stuff and are generally unfit for
>>>>>> any further extension.
>>>>>
>>>>> I have been changing only the instance files, so we are going in
>>>>> different directions.
>>>>
>>>> I don't see how, {ca,kra}instance.py code is called from {ca,kra}.py.
>>>> {ca,kra}.py contains all the code that is actually needed to install
>>>> CA/KRA that is not in {ca,kra}instance.py and was previously scattered
>>>> around ipa-{server,replica,ca,kra}-install.
>>>>
>>>>>
>>>>> I do not really care what file we are going into, but there is a lot of
>>>>> code in the installer now that does not tell the user a step is being
>>>>> done, while instances do that through the step interface.
>>>>
>>>> This code was always there, we only moved it to a single location. See
>>>> git history.
>>>>
>>>>>
>>>>> The step interface is also a very good way to let someone that read the
>>>>> code see what is going on and follow each step.
>>>>>
>>>>> Are you proposing to stop going through the instances steps ? I found
>>>>> the current way kra and ca installation is setup basically a regression,
>>>>> it took me a *lot* longer that it should be needed to follow through all
>>>>> the steps that are really taken.
>>>>
>>>> Again, we only moved the code around, and I don't think we created any
>>>> regressions.
>>>
>>> Ok, so the plan is just to move the CAInstance class and code *as is*
>>> from cainstance.py to ca.py ?I guess I am confused about what is your
>>> plan around this exactly.
>>
>> The code in CAInstance will be gradually migrated to a new class in
>> ca.py which uses the new install framework. We started with the
>> top-level ipa-server-install and ipa-replica-install code, which is
>> still not done, but you can see it for reference on how it will look
>> like (ipaserver/install/server/*.py, especially the classes at the
>> bottom of the files).
>
> Well I had to modify server/replicainstall.py quite radically, I had to
> create a new set of promote_check and promote functions there. So we are
> back to duplicated code, sorry (no way around it).

It's OK, there is still a lot of duplicate code in server/replica 
install anyway, we deduplicated only the CA, KRA and DNS install code so 
far.

>
> However the functions in server/replicainstall.py still use the
> instances mostly for all components *except* for kra and ca where there
> is really confusing code calling unnecessarily instances multiple times
> and fooling around with stuff.

If you think there are unnecessary calls I think you are reading the 
code wrong.

> I do not really understand what you mean
> by migrating from CAInstance to what's in server/*install.py because in
> there all instances are used for the individual components, so I am now
> scratching my head.

What I mean is that the code to install a component will be wrapped in a 
class similar to Server or Replica classes in these files. There will 
still be steps like in CAInstance, but the rest will be different. I 
don't think you have seen the PoC from 5 months ago: 
<https://www.redhat.com/archives/freeipa-devel/2015-March/msg00374.html>.

>
> The code in ca.py especially is really confusing, I rewrote it once to
> eliminate duplications then decided to simply not touch it in my branch
> (and threw away the modifications) because it is too confusing and I did
> not want to risk regressions. So I created a simple create_replica()
> function in the CA instance that does all that is needed.

Before ca.py, you would have to rewrite the same confusing code in 
ipa-server-install, ipa-replica-install and ipa-ca-install. I don't see 
how that's better.

>
> I guess I will just keep ignoring the confusion and try to come up with
> something that "works" but I'd really like to understand what is the
> endgame there. If you want to replace instances (why?), what will you
> replace it with ?

With something that does not mix install with service management, 
maximizes code reuse and makes related code colocated, provides 
introspection on configuration options, allows componentization and 
idempotent execution. See the PoC linked above.

>
> Simo.
>


-- 
Jan Cholasta