Bug 1112991 - systemd-rpm-macros: missing method for reload instead of restart on update
systemd-rpm-macros: missing method for reload instead of restart on update
Status: RESOLVED WONTFIX
Classification: openSUSE
Product: openSUSE Tumbleweed
Classification: openSUSE
Component: Basesystem
Current
Other Other
: P5 - None : Normal (vote)
: ---
Assigned To: systemd maintainers
E-mail List
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2018-10-23 23:12 UTC by Ruediger Oertel
Modified: 2018-11-09 12:48 UTC (History)
4 users (show)

See Also:
Found By: ---
Services Priority:
Business Priority:
Blocker: ---
Marketing QA Status: ---
IT Deployment: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ruediger Oertel 2018-10-23 23:12:19 UTC
see also home:oertel:branches:Base:System/systemd-rpm-macros

+Wed Oct 24 01:08:16 CEST 2018 - ro@suse.de
+
+- add -r option for service_del_postun to allow
+  for reload instead of restart on update 


--- macros.systemd      (revision 1)
+++ macros.systemd      (working copy)
@@ -63,6 +63,14 @@
                             "$DISABLE_RESTART_ON_UPDATE" = 1 && exit 0\
                        %{?*:/usr/bin/systemctl try-restart %{*}}\
                ) || : %{nil}
+%_restart_on_update_reload() (\
+                       test "$YAST_IS_RUNNING" = instsys && exit 0\
+                       test -f /etc/sysconfig/services -a \\\
+                            -z "$DISABLE_RESTART_ON_UPDATE" && . /etc/sysconfig/services\
+                       test "$DISABLE_RESTART_ON_UPDATE" = yes -o \\\
+                            "$DISABLE_RESTART_ON_UPDATE" = 1 && exit 0\
+                       %{?*:/usr/bin/systemctl try-reload-or-restart %{*}}\
+               ) || : %{nil}
 
 %_stop_on_removal_force() ( \
                test "$YAST_IS_RUNNING" = instsys && exit 0\
@@ -173,13 +181,13 @@
 # variable if not found use the value read from /etc/sysconfig/services
 #
 
-%service_del_postun(fn)                                                                        \
+%service_del_postun(fnr)                                                               \
 test -n "$FIRST_ARG" || FIRST_ARG="$1"                                                 \
 if [ "$FIRST_ARG" -ge 1 ]; then                                                                \
        # Package upgrade, not uninstall                                                \
        if [ -x /usr/bin/systemctl ]; then                                              \
                /usr/bin/systemctl daemon-reload || :                                   \
-               %{expand:%%_restart_on_update%{-f:_force}%{!-f:%{-n:_never}} %{?*}}     \
+               %{expand:%%_restart_on_update%{-f:_force}%{!-f:%{-n:_never}}%{!-f:%{!-n:%{-r:_reload}}} %{?*}}  \
        fi                                                                              \
 else # package uninstall                                                               \
        for service in %{?*} ; do                                                       \
Comment 1 Franck Bui 2018-10-27 16:22:14 UTC
(In reply to Ruediger Oertel from comment #0)
> see also home:oertel:branches:Base:System/systemd-rpm-macros
> 

Could you describe the problem you're trying to solve ?

Why would you want to avoid restarting a service on a package update ?
Comment 2 Ruediger Oertel 2018-10-28 23:34:46 UTC
it's basically the old "ExecRestart" discussion.
Some services want to implement the Restart action themselves,
in this case because they want to pass on file descriptors to
their successor. just killing the old process and starting a
new one is disruptive and absolutely not wanted here.
so instead of calling "restart" during update, we want our own
implementation, (which is then a ExecReload action that does
what we need without havinng to loose data during a uncoordinated
restart).
Comment 3 Franck Bui 2018-10-29 08:19:35 UTC
(In reply to Ruediger Oertel from comment #2)
> it's basically the old "ExecRestart" discussion.

Any links to share ?

> Some services want to implement the Restart action themselves,
> in this case because they want to pass on file descriptors to
> their successor.

What do you mean by "themselves" ?

PID1 offers an API to services/daemons for sending and storing their fds for this case. See logind and journald for examples.

Do you mean that in your case this API can't be used ? if so can you explain why ?
Comment 4 Ruediger Oertel 2018-10-29 09:01:01 UTC
"themselves" reading: the process want's to start it's successor itself,
pass on whatever info/data/... it wants and then terminates.

but it's actually micha's code in this case, micha can you elaborate ?
Comment 5 Michael Schröder 2018-10-29 11:00:31 UTC
As Rudi said: we wan't a graceful restart with no downtime at all.

Regarding #3: what interface? Currently a restart is hardcoded as first stop then start, so how would such an interface be useful at all? The service doesn't know that it's going to be started again.
Comment 6 Franck Bui 2018-10-29 11:04:49 UTC
(In reply to Michael Schröder from comment #5)
> As Rudi said: we wan't a graceful restart with no downtime at all.

Well that's what most other services want to implement so this is not new and as I said journald, logind... already implement that.

> Regarding #3: what interface? 

Please read https://www.freedesktop.org/software/systemd/man/sd_notify.html
Comment 7 Michael Schröder 2018-10-29 11:57:31 UTC
And how would this work from a perl program?
Comment 8 Michael Schröder 2018-10-29 11:59:12 UTC
Also note that this is *way* more complicated than it needs to be.
Comment 9 Michael Schröder 2018-10-29 12:26:24 UTC
(It's also not clear to me that stopping the service will make systemd close all stored fds. Is that the case?)
Comment 10 Franck Bui 2018-10-29 13:46:57 UTC
(In reply to Michael Schröder from comment #7)
> And how would this work from a perl program?

The daemon is implemented in perl ? that's not really common...

(In reply to Michael Schröder from comment #8)
> Also note that this is *way* more complicated than it needs to be.

Well, how do you plan to implement that exactly otherwise ?

(In reply to Michael Schröder from comment #9)
> (It's also not clear to me that stopping the service will make systemd close
> all stored fds. Is that the case?)

Sure.
Comment 11 Michael Schröder 2018-10-29 14:38:28 UTC
> The daemon is implemented in perl ? that's not really common...

Now that's a helpful comment...

> Well, how do you plan to implement that exactly otherwise ?

Exactly what Rudi proposed.

> Sure.

(I couldn't find that in the systemd code, that's why I was asking.)
Comment 12 Franck Bui 2018-10-29 15:25:31 UTC
(In reply to Michael Schröder from comment #11)
> > Well, how do you plan to implement that exactly otherwise ?
> 
> Exactly what Rudi proposed.

That's not clear.

My current understanding of your needs is that you want a new (hackish) option (BTW the existing options are already hacks) because your service don't support "true" restarting.

So instead you just ask your daemon to reload its configuration during package update while the daemon is supposed to be re-executed so the new binary is in-use.

If so I would suggest to either implement a proper way to send the fds to systemd or to implement your own logic in your package. 

So if I'm getting this right, creating new macros or adding new options for encouraging this odd case is not an option, sorry.
Comment 13 Michael Schröder 2018-10-29 16:05:06 UTC
Well, "true" restarting is in the eye of the beholder.

It's not "reloading its configuration", it's actually restarting, aka re-executing the new binary. It's a mis-use of "reload" because systemd's restart means a hard stop/start.

Your suggestions are not really helping. Please show me how to do that filedescriptor sending/receiving in perl.

I'm fine with simply overwriting your systemd macros in our packages, so don't worry. I find your attitude a bit strange, though. Have a nice day!
Comment 14 Franck Bui 2018-10-29 16:50:10 UTC
(In reply to Michael Schröder from comment #13)
> Your suggestions are not really helping. Please show me how to do that
> filedescriptor sending/receiving in perl.

A perl module wrapping parts of libsystemd would be needed I guess like python-systemd does for python.

But I guess you already looked for search module.

BTW which package are talking about ?
Comment 15 Ruediger Oertel 2018-10-29 23:44:46 UTC
yes, a perl module exists at https://metacpan.org/pod/Systemd::Daemon
but the FD passing is not implemented.

actually perl services are not really uncommon, once you look beyond basic
systme services and more into the services actually implemented on top of
the basic operating system.

which package ... "obs-server", the package containing the buildservice ;)
Comment 16 Michael Schröder 2018-10-30 08:53:48 UTC
Maybe I got carried away a bit yesterday, sorry. Anyway, here's some more information from me:

The package we're talking about is the OBS backend server, serving requests on a socket. Our goal is to do a graceful restart on package update. This means:

1) we do not want to close the server socket
2) we want the current jobs to finish (this might take a couple of minutes for some types, the server has special code to deal with longrunning jobs)

So we can't use systemd's restart mechanism which does a hard start/stop. Instead, we opted to 'hijack' the reload mechanism. (A graceful restart is also the only way to reload the server config, so it's not that far-fetched.)

We do this since quite some time with sysv-init style scripts, but now we want to migrate to systemd service files. That's why Rudi is asking for some way to make the package update to a reload instead of a restart.

I understand that the current proposed change is somewhat invasive for this special case, but maybe we can do something simpler: How about adding

%_systemd_try_restart try-restart

and adapting the restart_on_update macros? Then we could simply change this in our package.
Comment 17 Franck Bui 2018-11-05 07:21:27 UTC
(In reply to Michael Schröder from comment #16)
> Anyway, here's some more information from me:

Thanks, please next time include such description earlier.

> 
> The package we're talking about is the OBS backend server, serving requests
> on a socket. Our goal is to do a graceful restart on package update. This
> means:
> 
> 1) we do not want to close the server socket
> 2) we want the current jobs to finish (this might take a couple of minutes
> for some types, the server has special code to deal with longrunning jobs)
> 
> So we can't use systemd's restart mechanism which does a hard start/stop.

Well as discussed previously, you're supposed to use sd_notify() in this case. But it appears that no perl bindings exists currently so they should implemented like python-systemd does for python.

> Instead, we opted to 'hijack' the reload mechanism. (A graceful restart is
> also the only way to reload the server config, so it's not that far-fetched.)
> 
> We do this since quite some time with sysv-init style scripts, but now we
> want to migrate to systemd service files. That's why Rudi is asking for some
> way to make the package update to a reload instead of a restart.
> 
> I understand that the current proposed change is somewhat invasive for this
> special case, but maybe we can do something simpler: How about adding

The problem is not about how invasive the changes would be but making a somehow 'special' case officially available. Again the official way for a service to preserve its file-descriptors is to send them to PID1.

IMHO until such API becomes available for perl, you should just call 'try-reload-or-restart' command in the %postun section of your package.

Hope that makes sense.
Comment 18 Michael Schröder 2018-11-05 12:39:12 UTC
You ignored the "must not kill running jobs" part.
Comment 19 Franck Bui 2018-11-06 07:42:07 UTC
How does this relate to systemd rpm macros ? I mean the jobs part is not something that systemd is supposed to provide support for you.

This actually suggests that the restart of your service is not properly implemented or designed and doesn't support re-executing.

PID1 or other services have the same problems to deal with. Nevertheless PID1, for instance, is not doing "reload instead of restart" even though it manages a lot of long standing services which must not be stopped while PID1 is restarted.
Comment 20 Michael Schröder 2018-11-06 10:51:17 UTC
The point is that we do not want a "restart on update" in the systemd sense. We want something more graceful on package update.

This is not about how systemd works, but about the package scriptlet macros.

(Btw, who else besides the systemd internal login and journal uses that fdstore mechanism? I haven't found anything searching the web. Even apache does not seem to use it.)
Comment 21 Franck Bui 2018-11-09 10:01:48 UTC
(In reply to Michael Schröder from comment #20)
> The point is that we do not want a "restart on update" in the systemd sense.
> We want something more graceful on package update.
> 

The problem here is that different packages will need different custom actions. For example one will need to be re-executed with a command of its own (such as fooctl reexecute) another one might want to receive a signal, in your case you want to reload and so on..

I don't see how all of these different cases could be hidden via a rpm macro and the point of doing it actually. The rpm macros have been created to deal with the generic cases only where stop/start is enough. If you need to do more involved stuff, then it's better to write the commands explicitely instead of trying to hide them behind macros.
Comment 22 Michael Schröder 2018-11-09 12:23:22 UTC
Yes, that's what we're doing right now. Closing.
Comment 23 Franck Bui 2018-11-09 12:48:59 UTC
Well I hope the arguments made sense.