[Fedora-packaging] Systemd scriptlet comments

[Toshio Kuratomi]

On Tue, Jul 19, 2011 at 3:24 PM, Ville Skyttä <ville.skytta at iki.fi> wrote:
> On 06/22/2011 07:24 PM, Toshio Kuratomi wrote:
>
>> We discussed this when we came up with the guidelines.  IIRC, we finally
>> decided this wasn't workable because we don't prevent people from packaging
>> systemVinit scripts (either in subpackages or in a wholly separate package.
>>
>> I agree with your points about fragility, though.  If you can think of a way
>> that handles both I'd be happy to hear it.
>
> Regarding the above, I don't think trying to chase "someone might do
> something" at the expense of making regular package maintenance harder
> or knowingly break at least to some extent is a good approach.
>
I agree about breakage.  But I guess we are drawing different lines
regarding what's harder for regular package maintenance.  One solution
might be to have two separate scriptlets, one if you're retaining the
sysv scripts and one if you aren't.  OTOH, the systemd section is
already pretty long and conditionalized which doesn't make for easy
reading (perhaps it's inevitable when dealing with init; the SysV
section was also pretty long and involved).

> Anyway, here's a couple of new thoughts:
>
> First, daemons are usually restarted on upgrade after the old package
> has been removed, so if this is desirable and the trigger approach is
> used, the try-restart should be done in %triggerpostun instead of
> %triggerun, no?
>
Is the important part that the old package has been removed or that
the new package has been installed?  If %triggerun is happening
sufficiently late, then using it allows us to only have on trigger
script rather than two.  If it does make a difference, then you're
right, we should move the try-restart portion to its own
%triggerpostun.

> Second, one way to ensure that the EVR of the package in the distro
> version that had the sysv scripts stays older than the one in which
> systemd migration happens even if the old distro version gets version
> updates is to bump Epoch when doing the migration, and take advantage of
> that in the versioned trigger.
>
I thought of this but didn't think that anyone would go for it.  I
think the argument against epoch has traditionally been that it's not
user-visible and it's easy for packagers to forget to update
dependencies when an epoch is in use.

> Third, for my test case where the old package does its try-restart with
> "service" instead of invoking the init script directly, this appears to
> work fine:
>
>    %pre
>    if [ $1 -gt 1 ] && [ ! -e %{_unitdir}/FOO.service ] && \
>       [ -e %{_initddir}/FOO ] ; then
>        systemd-sysv-convert --save FOO &>/dev/null
>        chkconfig --del FOO &>/dev/null || :
>    fi
>
>    %post
>    systemctl daemon-reload &>/dev/null || :
>
>    %preun
>    if [ $1 -eq 0 ] ; then
>        systemctl --no-reload disable FOO.service &>/dev/null
>        systemctl stop FOO.service &>/dev/null || :
>    fi
>
>    %postun
>    systemctl daemon-reload &>/dev/null
>    [ $1 -gt 0 ] && systemctl try-restart FOO.service &>/dev/null || :
>
> I believe the conditions in %pre should prevent it from doing bad things
> (only when upgrading the package, and only if the sysv script is around
> but the unit file isn't yet).  Note that daemon-reload is done in %post
> also for upgrades so that the "service FOO try-restart" in the old
> package's %postun will do the right thing.  Note also no need for
> triggers in this scenario.
>
> If the old package's %postun invokes the sysv init script directly,
> getting the daemon restarted gets uglier but using a temp file (for
> example
> %{_var}/run/%{name}-%{version}-%{release}-%{arch}.systemd-migration,
> referred to as %{migrfile} below), something like this works in my tests:
>
>    %pre
>    rm -f %{migrfile} &>/dev/null
>    if [ $1 -gt 1 ] && [ ! -e %{_unitdir}/FOO.service ] && \
>       [ -e %{_initddir}/FOO ] ; then
>        systemd-sysv-convert --save FOO &>/dev/null
>        chkconfig --del FOO &>/dev/null
>        touch %{migrfile} &>/dev/null
>    fi
>    exit 0
>
>    %post
>    [ $1 -eq 1 ] && systemctl daemon-reload &>/dev/null || :
>
>    %preun
>    if [ $1 -eq 0 ] ; then
>        systemctl --no-reload disable FOO.service &>/dev/null
>        systemctl stop FOO.service &>/dev/null || :
>    fi
>
>    %postun
>    systemctl daemon-reload &>/dev/null
>    [ $1 -gt 0 ] && systemctl try-restart FOO.service &>/dev/null || :
>
>    %triggerpostun -- %{name}
>    if [ $1 -gt 0 ] && [ -e %{migrfile} ] ; then
>        systemctl daemon-reload &>/dev/null
>        systemctl try-restart FOO.service &>/dev/null
>    fi
>    rm -f %{migrfile} &>/dev/null || :

If you'd like to write this up as a draft I know that we beat
ourselves up trying to remove the need for triggers before giving up.
So we'd probably like to have something that does without them.
Documenting how and what you've tested would also be good as testing
these is one of the pain points when working on this.
(For instance, this set of tests [which even after doing all that,
still wasn't a complete set of tests] that I did prior to our
discarding migration of boot-on/off status:
https://fedoraproject.org/wiki/User:Toshio/Testing_systemd )

-Toshio