[Pki-devel] [PATCH] pki-tomcatd fails to start on system boot

[Matthew Harmsen]

On 04/13/15 10:37, Endi Sukma Dewata wrote:
> Just one thing:
>
> 1. We still have this message in pkispawn man page:
>
>   To  enable  a  PKI  instance  named <pki_instance_name> to be started
>   automatically upon system reboot:
>
>     systemctl enable pki-tomcatd@<pki_instance_name>.service
>
DONE
> Once it's fixed, ACK.
>
Checked into master:
* 18b24a990ff9b97cf58aa630af0084975fe4c130
> Please also see comments below.
>
> On 4/10/2015 7:07 PM, Matthew Harmsen wrote:
>> Please review the attached patch which addresses the following issues:
>>
>>   * PKI TRAC Ticket #1315 - pki-tomcatd fails to start on system boot
>>     <https://fedorahosted.org/pki/ticket/1315>
>>   * PKI TRAC Ticket #1340 - pkidestroy should not remove /var/lib/pki
>>     <https://fedorahosted.org/pki/ticket/1340>
>>
>> Note that this was tested successfully on my Fedora 21 laptop.
>>
>> After numerous re-writes in which I attempted to make it work on an
>> individual PKI instance (but not subsystems within a shared PKI
>> instance), I finally gave in and made it work as explained in the
>> 'pki_default.cfg' man page.
>>
>> The issue was that 'systemctl disable <instance>' not only removed the
>> desired symbolic link from
>> '/etc/systemd/system/multi-user.target.wants', but also caused the
>> deletion of the entire '/etc/systemd/system/pki-targetd.target.wants'
>> directory (which is owned by the pki-server package).  Within PKI, this
>> directory and its internal symbolic link are always required for proper
>> operation, and it confused the system so badly, I was not able to
>> restore it by simply re-running 'systemctl enable <instance>'.
>
> Is it possible to remove/reduce the dependency on systemd (in case 
> systemd is not available on certain distributions)?
>
Theoretically, this already exists for Debian using SysV init stuff in 
the new enable/disable routines - this was never tested, however.

I don't think that generic reliance on SysV init scripts at this point 
in time is very practical -- are there other distributions that you are 
referring to which utilize Dogtag but have not switched over to systemd?
>> As the revised man page states, to manually disable PKI instances from
>> starting upon reboot, run 'systemctl disable pki-tomcatd.target', to
>> manually enable them, run 'systemctl enable pki-tomcatd.target'; no one
>> should ever run 'systemctl enable/disable <pki instance>' (nor for that
>> matter 'systemctl enable/disable <389 instance>') as this confuses the
>> system.
>
> If someone accidentally runs 'systemctl enable/disable <pki instance>' 
> (I don't think that's preventable since other systemctl commands for 
> specific instance works), how can that be fixed? Remove the instance 
> and reinstall the packages?
Unfortunately, I know of no way to protect against this (restoration 
could be done manually by recreating the directory and symlink, but the 
admin would need to know what the original names and symlink targets were).
>
>> Additionally, this patch makes the change to 'infrastructure_layout.py'
>> to only create/remove the '/var/lib/pki' directory (owned by the
>> 'pki-server' package) when it has been relocated using pkispawn's '-p
>> <prefix>' test parameter.
>
> I don't think we should remove (or maybe even create) the folder even 
> if it's been relocated. Someone could probably setup the folder in 
> such a way, and removing the directory could be undesirable. We can 
> discuss this separately.
>
Since the "relocated" instances top-level directory is only specified 
via the "-p" test flag for pkispawn, I don't think that it is 
catastrophic to  automaticlaly create/remove it.  Although I have not 
used this feature since early days of development, I found it very 
useful for testing, so I will include this in my checked-in code, and a 
separate ticket will need to be filed should it be determined to be 
removed in the future.
>> Finally, since another line was added to the final status report
>> produced at the end of 'pkispawn', I streamlined the spacing a bit in
>> this patch.
>