Bug#995006: init-system-helpers: deb-systemd-helper does not call daemon-reload despite unit files were changed

0 views
Skip to first unread message

Vasyl Gello

unread,
Sep 24, 2021, 12:30:03 PMSep 24
to
Package: init-system-helpers
Version: 1.60
Severity: important
X-Debbugs-Cc: mat...@debian.org, mbi...@debian.org

Dear colleagues,

I noticed that systemd keeps older versions of unit files even after
I explicitly rebuilt the package with new contents of unitfiles.

I decided to track down the issue and found that '$changed_sth' is
set to 1 only when **links** to systemd units are somehow updated,
i.e:

- make_systemd_links
- remove_links
- mask_service
- unmask_service

Furthermore, 'dh_installsystemd' does place 'systemctl daemon-reload'
in postrm script, but relies on conditional invocation of the same
inside 'deb-systemd-helper' for postinst one.

To reproduce, you need a package installing systemd units.
First you install the package version with one contents of systemd unit,
then install another version with changed content of the same
systemd unit, and typing `systemctl status service` shows you old
configuration of service.

Cheers,
Vasyl

-- System Information:
Debian Release: bookworm/sid
APT prefers unstable-debug
APT policy: (500, 'unstable-debug'), (500, 'buildd-unstable'), (500, 'unstable'), (1, 'experimental')
Architecture: amd64 (x86_64)

Kernel: Linux 5.10.0-8-amd64 (SMP w/6 CPU threads)
Kernel taint flags: TAINT_OOT_MODULE, TAINT_UNSIGNED_MODULE
Locale: LANG=C, LC_CTYPE=C (charmap=UTF-8) (ignored: LC_ALL set to en_US.UTF-8), LANGUAGE not set
Shell: /bin/sh linked to /bin/dash
Init: unable to detect

Versions of packages init-system-helpers depends on:
ii perl-base 5.32.1-5

init-system-helpers recommends no packages.

init-system-helpers suggests no packages.

Versions of packages init-system-helpers is related to:
pn insserv <none>

-- no debconf information

Michael Biebl

unread,
Sep 24, 2021, 1:10:02 PMSep 24
to
Am 24.09.2021 um 18:22 schrieb Vasyl Gello:
> Package: init-system-helpers
> Version: 1.60
> Severity: important
> X-Debbugs-Cc: mat...@debian.org, mbi...@debian.org
>
> Dear colleagues,
>
> I noticed that systemd keeps older versions of unit files even after
> I explicitly rebuilt the package with new contents of unitfiles.
>
> I decided to track down the issue and found that '$changed_sth' is
> set to 1 only when **links** to systemd units are somehow updated,
> i.e:
>
> - make_systemd_links
> - remove_links
> - mask_service
> - unmask_service
>
> Furthermore, 'dh_installsystemd' does place 'systemctl daemon-reload'
> in postrm script, but relies on conditional invocation of the same
> inside 'deb-systemd-helper' for postinst one.
>
> To reproduce, you need a package installing systemd units.
> First you install the package version with one contents of systemd unit,
> then install another version with changed content of the same
> systemd unit, and typing `systemctl status service` shows you old
> configuration of service.

Please provide a minimal package/.dsc which shows the problem.

Thanks

Michael Biebl

unread,
Sep 24, 2021, 1:20:02 PMSep 24
to
Am 24.09.2021 um 18:58 schrieb Michael Biebl:

> Please provide a minimal package/.dsc which shows the problem.

I'm especially interested in debian/rules (how you call
dh_installsystemd), the unit files and the generated maintainer scripts
files.

Michael Biebl

unread,
Sep 24, 2021, 3:30:03 PMSep 24
to
Am 24.09.2021 um 21:21 schrieb Vasyl Gello:
> Hi Michael,
>
> >I think the relevant bits are
> >
> > dh_installsystemd -v --no-enable --no-start
> >
> >which means,iirc, no explicit maintainer scripts code is generated to
> reload systemd as you asked it to neither start nor enable the service.
> >Since i-s-h is not really involved at this point, I assume this issue
> is misfiled.
>
> So if we don't want to enable and/or start service in postinst, proper
> solution is to add wrapped block to postinst with proper comment
> referencing this bug?

No, my point is that i-s-h is not called as part of the upgrade process,
since you explicititly asked not to. So i-s-h can not call daemon-reload.
Whether dh_installsystemd should generate a blank "daemon-reload" in
this case is another matter.


OpenPGP_signature

Michael Biebl

unread,
Sep 24, 2021, 3:30:04 PMSep 24
to
Control: severity -1 normal

Am 24.09.2021 um 20:50 schrieb Vasyl Gello:
> Hi Michael!
>
> The package I encountered an issue is trunk version of Xpra:
> https://salsa.debian.org/basilgello/xpra/-/tree/debian/4.x
> <https://salsa.debian.org/basilgello/xpra/-/tree/debian/4.x>
>
> I have uploaded the artifacts to FEX, they will be stored for 7 days
> there: https://fex.net/s/aferrrr <https://fex.net/s/aferrrr>
OpenPGP_signature

Michael Biebl

unread,
Sep 25, 2021, 11:00:03 AMSep 25
to
Am 25.09.21 um 16:50 schrieb Michael Biebl:
> daemon-reload is a costly operation, so we should try to avoid calling
> it too excessively.

I've been trying hard to get rid of "daemon-reload" calls where not
absolutely necessary, see e.g. [1-3]

So I'd be very wary re-adding them too liberally.

Regards,
Michael


[1] https://salsa.debian.org/debian/debhelper/-/merge_requests/43
[2]
https://salsa.debian.org/systemd-team/systemd/commit/54ebb5500e75562d77fc5668ef7194af579f85bd
[3]
https://salsa.debian.org/debian/init-system-helpers/commit/f0cac594ab79ebe72c53529046304037cf5c09b8

OpenPGP_signature

Michael Biebl

unread,
Sep 25, 2021, 11:00:03 AMSep 25
to
Am 24.09.21 um 21:36 schrieb Vasyl Gello:
> retitle -1 dh_installsystemd should call daemon-reload in postinst if
> unit files changed
> reassign -1 debhelper 13.5.1
>
> >No, my point is that i-s-h is not called as part of the upgrade
> process, since you explicititly asked not to. So i-s-h can not call
> daemon-reload.
> >Whether dh_installsystemd should generate a blank "daemon-reload" in
> this case is another matter.
>
> OK, so I am re-assigning the bug to debhelper and temporary add the
> daemon-reload step into xpra postinst.

So far, we call daemon-reload in the following cases:


> autoscripts/postrm-systemd-reload-only: systemctl --system daemon-reload >/dev/null || true
> autoscripts/postinst-systemd-restartnostart: systemctl --system daemon-reload >/dev/null || true
> autoscripts/postinst-systemd-restart: systemctl --system daemon-reload >/dev/null || true
> autoscripts/postinst-systemd-start: systemctl --system daemon-reload >/dev/null || true


So, it is currently tied to start/restart requests, i.e. we only call
daemon-reload at the exact point when it's actually needed before we
(re)start a service and need the updated configuration, i.e. in cases
where dh_installsystemd controls the life cycle of the service.

daemon-reload is a costly operation, so we should try to avoid calling
it too excessively.

So I'm not convinced it is a good idea to generate a daemon-reload (via
dh_installsystemd in postinst) for packages which do not actually
(re)start a unit as part of the upgrade process.

By using "--no-enable --no-start" you are basically leaving the
management of the life cycle of the service entirely to the
administrator. Wouldn't you agree?

Shouldn't the administrator then call "systemctl daemon-reload" as well?
systemd will even warn him in cases a .service file has changed (which
thankfully doesn't happen too often)

Is this actually an issue in practice? Do you have bug reports where
users of your xpra package have asked for this?


Regards,
Michael


OpenPGP_signature

Michael Biebl

unread,
Sep 25, 2021, 1:00:04 PMSep 25
to
Am 25.09.21 um 18:46 schrieb Vasyl Gello:
> Hi Michael,
>
> >daemon-reload is a costly operation, so we should try to avoid calling it
> too excessively.
> >
> >So I'm not convinced it is a good idea to generate a daemon-reload (via
> dh_installsystemd in postinst) for packages which do not actually (re)start
> a unit as part of the upgrade process.
> >
> >By using "--no-enable --no-start" you are basically leaving the
> management of the life cycle of the service entirely to the administrator.
> Wouldn't you agree?
> >
> >Shouldn't the administrator then call "systemctl daemon-reload" as well?
> >systemd will even warn him in cases a .service file has changed (which
> thankfully doesn't happen too often)
> >
> >Is this actually an issue in practice? Do you have bug reports where
> users of your xpra package have asked for this?
>
> I realize daemon-reload is a costly operation, and I also realize the unit
> in question is used by proxy module of Xpra, which is optional.
>
> That's why Dmitry intentionally left it for system administrators to
> configure.
>
> Your point that typically units are not changed daily is also perfectly
> valid.
>
> However, speaking of "Shouldn't the administrator then call "systemctl
> daemon-reload" as well?"…
> No, if one uses unattended-upgrades or any other automation like auter
> or puppet etc.
>
> Server gets rebooted or service gets restarted and start-up fails due to
> requirement of
> "daemon-reload" or worse, starts with previous configuration which fails
> too.

A reboot is not relevant here.
And if you use an automation framework like puppet, you can just as well
add a daemon-reload there if you manage the service via puppet.

> What I suggest is slightly different Debian-wise. We do have list of
> conffiles and list of files
> installable by the package. What prevents us from scanning the list for
> known locations
> of systemd units ({/usr}/lib/systemd/{system,user}?) and compare them by
> hash or by diff
> just like we do it with conffiles? If the units, timers, etc differ, we
> call daemon-reload and
> issue a warning to stderr so both user or script catches it. Of course
> it may need some additions
> to dpkg and debhelper (or to plain debhelper only?) but this issue will
> be eradicated for all packages
> rebuilt after the update.

systemd already warns you, if you (manually) try to restart a service
that has been modified on disk.
So I don't see the use case here.

Again, you you have any bug reports where users actually ask for this?


OpenPGP_signature

Mattia Rizzolo

unread,
Sep 26, 2021, 7:30:03 AMSep 26
to
On Sat, Sep 25, 2021 at 04:50:11PM +0200, Michael Biebl wrote:
> daemon-reload is a costly operation, so we should try to avoid calling it
> too excessively.
>
> So I'm not convinced it is a good idea to generate a daemon-reload (via
> dh_installsystemd in postinst) for packages which do not actually (re)start
> a unit as part of the upgrade process.

I wonder, do you already thought about using a dpkg trigger?
I don't think that running daemon-reload once at the end of a set of
package upgrade is too much.

--
regards,
Mattia Rizzolo

GPG Key: 66AE 2B4A FCCF 3F52 DA18 4D18 4B04 3FCD B944 4540 .''`.
More about me: https://mapreri.org : :' :
Launchpad user: https://launchpad.net/~mapreri `. `'`
Debian QA page: https://qa.debian.org/developer.php?login=mattia `-
signature.asc
Reply all
Reply to author
Forward
0 new messages