[Freeipa-devel] [PATCH 0083] Make CS.cfg edits with CA instance stopped

[Petr Viktorin]

On 08/23/2013 02:23 PM, Tomas Babej wrote:
> On 08/05/2013 05:43 PM, Martin Kosek wrote:
>> On 08/02/2013 03:32 PM, Tomas Babej wrote:
>>> Hi,
>>>
>>> This patch makes sure that all edits to CS.cfg configuration file
>>> are performed while pki-tomcatd service is stopped.
>>>
>>> Introduces a new contextmanager stopped_service for handling
>>> a general problem of performing a task that needs certain service
>>> being stopped.
>>>
>>> https://fedorahosted.org/freeipa/ticket/3804
>>>
>>> Tomas
>>>
>> 1) I think it would make sense to ideally run the steps updating
>> CS.cfg close
>> together, stop PKI before this group and start it after it finishes.
>> Otherwise,
>> installer runs many service stops and starts which may be error prone,
>> especially given the fragile (and sometimes slow) java server handling.
>>
>> 2) I am thinking that stopped_service context manager could be as well
>> defined
>> in ipaserver/install/service.py, as a context manager of the class.
>> That way,
>> every installer class could use that like:
>>
>> class CAInstance():
>>     ...
>>     def __some_step(self):
>>        with self.stopped_service(start_when_finished=True):
>>            # do something
>
> I considered this approach, but this might introduce unnecessary errors if
> we ever reorder the install steps in cainstance.py. I rather added two
> explicit steps to stop and start the CA instance.
>
>> That way, context manager could just use self.name to avoid numerous
>> hardcoded
>> service names like:
>>
>> ...
>>          with stopped_service('pki_tomcatd',
>>
>> instance_name=self.dogtag_constants.PKI_INSTANCE_NAME):
>> ...
>>
>
> Yes, but there are functions outside CAInstance class that leverage this
> context.
>
>> 3) After I installed pki-ca, I saw no published CRL files:
>> # ls -la /var/lib/ipa/pki-ca/publish/
>>
>> I am not sure what is the root cause, maybe some of the numerous
>> start/restarts
>> broke the publisher process.
> I'm not seeing this with the updated version of the patch anymore.
>
>> Martin
>
> Updated patch attached.

ACK for master & ipa-3-3. For ipa-3-2, the patch needs a rebase.

Pushed:
master: ab6a6e27d88b44b8c3f07290ae753558705363ee
ipa-3-3: 12cb45c767d097a39d082ebad0f846bdb94ed9ca


-- 
Petr³