#995006 dh_installsystemd should call daemon-reload in postinst if unit files changed

#995006#5
Date:
2021-09-24 16:22:00 UTC
From:
To:
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

#995006#10
Date:
2021-09-24 16:58:13 UTC
From:
To:
Am 24.09.2021 um 18:22 schrieb Vasyl Gello:

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

Thanks

#995006#15
Date:
2021-09-24 17:14:56 UTC
From:
To:
Am 24.09.2021 um 18:58 schrieb Michael Biebl:

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

#995006#20
Date:
2021-09-24 18:50:12 UTC
From:
To:
Hi Michael!

The package I encountered an issue is trunk version of Xpra: 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

Please let me know if you need more information.
-- 
Vasyl Gello
==================================================
Certified SolidWorks Expert

Mob.:+380 (98) 465 66 77

E-Mail: vasek.gello@gmail.com

Skype: vasek.gello
==================================================
호랑이는 죽어서 가죽을 남기고 사람은 죽어서 이름을 남긴다

#995006#25
Date:
2021-09-24 19:18:31 UTC
From:
To:
Am 24.09.2021 um 20:50 schrieb Vasyl Gello:

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.

#995006#32
Date:
2021-09-24 19:21:11 UTC
From:
To:
Hi Michael,
-- 
Vasyl Gello
==================================================
Certified SolidWorks Expert

Mob.:+380 (98) 465 66 77

E-Mail: vasek.gello@gmail.com

Skype: vasek.gello
==================================================
호랑이는 죽어서 가죽을 남기고 사람은 죽어서 이름을 남긴다

#995006#37
Date:
2021-09-24 19:25:06 UTC
From:
To:
Am 24.09.2021 um 21:21 schrieb Vasyl Gello:

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.

#995006#42
Date:
2021-09-24 19:36:35 UTC
From:
To:
retitle -1 dh_installsystemd should call daemon-reload in postinst if unit files changed
reassign -1 debhelper 13.5.1

Thanks for the tip on i-s-h!
-- 
Vasyl Gello
==================================================
Certified SolidWorks Expert

Mob.:+380 (98) 465 66 77

E-Mail: vasek.gello@gmail.com

Skype: vasek.gello
==================================================
호랑이는 죽어서 가죽을 남기고 사람은 죽어서 이름을 남긴다

#995006#47
Date:
2021-09-24 19:40:48 UTC
From:
To:
Oops, forgot to add Control word…  :(
-- 
Vasyl Gello
==================================================
Certified SolidWorks Expert

Mob.:+380 (98) 465 66 77

E-Mail: vasek.gello@gmail.com

Skype: vasek.gello
==================================================
호랑이는 죽어서 가죽을 남기고 사람은 죽어서 이름을 남긴다

#995006#60
Date:
2021-09-25 14:50:11 UTC
From:
To:
Am 24.09.21 um 21:36 schrieb Vasyl Gello:

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


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

#995006#65
Date:
2021-09-25 14:54:14 UTC
From:
To:
Am 25.09.21 um 16:50 schrieb Michael Biebl:

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

#995006#70
Date:
2021-09-25 16:46:18 UTC
From:
To:
Hi Michael,
too excessively.
dh_installsystemd in postinst) for packages which do not actually (re)start
a unit as part of the upgrade process.
management of the life cycle of the service entirely to the administrator.
Wouldn't you agree?
thankfully doesn't happen too often)
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.

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.

Of course I can implement the same check in postinst script of Xpra, but I decided to discuss
my idea with involved colleagues.

What do you think?
-- 
Vasyl Gello
==================================================
Certified SolidWorks Expert

Mob.:+380 (98) 465 66 77

E-Mail: vasek.gello@gmail.com

Skype: vasek.gello
==================================================
호랑이는 죽어서 가죽을 남기고 사람은 죽어서 이름을 남긴다

#995006#75
Date:
2021-09-25 16:52:24 UTC
From:
To:
Am 25.09.21 um 18:46 schrieb Vasyl Gello:

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.

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?

#995006#80
Date:
2021-09-25 16:57:39 UTC
From:
To:
Version 4.x is not in Debian yet, and I haven't seen any reports from users making use of Xpra proxy server on Debian yet.
-- 
Vasyl Gello
==================================================
Certified SolidWorks Expert

Mob.:+380 (98) 465 66 77

E-Mail: vasek.gello@gmail.com

Skype: vasek.gello
==================================================
호랑이는 죽어서 가죽을 남기고 사람은 죽어서 이름을 남긴다

#995006#83
Date:
2021-09-26 11:25:49 UTC
From:
To:
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.