[Freeipa-devel] kra an ca instance installation

[Simo Sorce]

On Wed, 2015-08-26 at 09:06 +0200, Jan Cholasta wrote:
> 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>.

I had not seen this, thanks.

> >
> > 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.

Ok, I'll ask more questions as they come and if needed, thanks for
explaining.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York