debhelper restarts services marked --no-restart-on-upgrade

Hi,
recent tests with libvirt 8.0 (in PPA, but even rebuilding our current libvirt in Jammy has the same result) shows that we are affected as expected. On reinstall or upgrade guests are shut down due to this.

Trying to work around that until resolved in debhelper:
=> https://salsa.debian.org/libvirt-team/libvirt/-/merge_requests/132

FYI: Keeping things up is even harder, while restarting sockets in the past complained about services already being up they now restart the service.
So if anyone is looking at this for some package, be careful that you'd also need to handle .socket files special now as they are affected by this issue here as well and when restarted will carry the problem to the service.

I'd like to know if there already is an Ubuntu position on this, will we revert [1] to avoid a yet unknown amount of breakage in Jammy?
I'd like to know to adapt my packages accordingly ...

[1]: https://salsa.debian.org/debian/debhelper/-/commit/6067bc2

I'm working on patching debhelper (brushing up on some *very* rusty perl knowledge!) as some of the comments upstream indicate that as many as 40+ packages may be affected and I don't see it as viable to work around every one of them. However, first I want to understand exactly what the patch that caused the breakage was doing to make sure that in reverting it I don't cause more breakage.

My current thinking (subject to change!) is that the patch was solving a legitimate, and quite important issue (https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=989155) but that the method of the solution is incorrect (or at least problematic). I *think* (not implemented / tested yet) that it's preferable to solve the *original* problem (which led to the problematic patch) with the introduction of a preinst hook to handle the restart, which would avoid having to fiddle with the start actions introduced by dh_installinit and dh_installsystemd. Anyway, that's where I am at the moment, but I've got some Pi bits to sort out before I can get back to digging further and testing some tentative solutions.

docker.io is also affected: https://git.launchpad.net/ubuntu/+source/docker.io/tree/debian/rules?h=ubuntu/jammy&id=18b9d33de2adcb7ec361e4789f1f96223e80d607#n109

Status changed to 'Confirmed' because the bug affects multiple users.

I've now got a branch on salsa [1] with a proposal for a fix which I've tested against a few affected packages in a few different environments. The debhelper test suite still passes, and overall I'm *reasonably* happy with it, but I still need to write up a proper explanation of why I've changed what I've changed.

If anyone wants to play with a test build of the patched debian version of the package (note: not one with the ubuntu delta; I need to do another rebase for that), you can find some packages in this PPA [2]. I'll try and get this written up and proposed (both upstream and here) shortly.

[1]: https://salsa.debian.org/waveform/debhelper/-/commits/fix-restart

[2]: https://launchpad.net/~waveform/+archive/ubuntu/debhelper

I've now opened a merge request with debhelper upstream incorporating my proposed fix [1] and I've added the fix to our proposed merge of debhelper in LP: #1960248. Test packages are built in [2] -- anybody affected by this is encouraged to test by build their packages with this PPA as an "extra" repo, e.g.:

  $ sbuild --extra-repository="deb [trusted=yes] https://ppa.launchpadcontent.net/waveform/dh-merge/ubuntu jammy main" ...

Then test that upgrades either stop & start (or don't!) as expected. I've checked the generated maintscripts *look* fine in several cases, but I've only managed to test actual upgrade behaviour of one case so much more is needed, so a lot more scrutiny is definitely warranted!

[1]: https://salsa.debian.org/debian/debhelper/-/merge_requests/61

[2]: https://launchpad.net/~waveform/+archive/ubuntu/dh-merge

One extra note: once this fix is landed we also need to figure out which packages need a no-change re-build. Is it possible to "grep" the archive for any package which mentions --no-restart-after-upgrade, --no-restart-on-upgrade, or --no-stop-on-upgrade in its d/rules ?

@Dave - you can do things like:
  https://codesearch.debian.net/search?q=dh_installsystemd.*--no-start&literal=0

FYI - For libvirt I'm uploading now with the mitigation for the debhelper issue to unblock others waiting on libvirt.

But the plan is to drop this mitigation before jammy release once debhelper is fixed.

> @Dave - you can do things like:
> https://codesearch.debian.net/search?q=dh_installsystemd.*--no-start&literal=0

Ah, very useful. sarnold has kindly run a similar search on all components of the Ubuntu archive for jammy. I'm currently going through this, and comparing it to a similar run on codesearch.debian.net (incidentally, this issue affects both dh_installsystemd *and* dh_installinit usage). I'll post a full list of affected packages to this ticket, and the upstream Debian ticket once I've weeded it out (there's a few false positives in there with changelog entries, commented out bits, etc.)

The fix has now been merged upstream in debhelper, so we're just waiting on sponsorship of LP: #1960248 to get the fix into our version as well. In the meantime, I've gone through the list sarnold provided; here's the list of source packages which will need a no-change rebuild once the debhelper fix has landed:

aoetools
apcupsd
apparmor
apt
apt-cacher-ng
aumix
bip
bootmail
buildbot
ceph
cloud-init
cpufrequtils
dbus
dbus-broker
docker.io
dpdk
drbd
entropybroker
finalrd
g180-led
ganeti
google-guest-agent
google-osconfig-agent
gpm
h2o
haproxy
ifupdown
ifupdown-ng
iipimage
inetsim
inspircd
iwd
libvirt
live-config
live-tools
lvm2
lxc
lxcfs
moosefs
neutron
nghttp
nginx
ngtcp
nodm
ntp
nullmailer
ocfs2-tools
open-infrastructure-compute-tools
open-iscsi
openldap
open-vm-tools
openvpn
pacemaker
packagekit
pdupdaemon
phosh
pi-bluetooth
policycoreutils
python-certbot
python-rtslib-fb
qcontrol
qemu
quassel
robustirc-bridge
rpcbind
runit
samba
sanlock
sbd
sbuild
slim
sysstat
tarantool
targetcli-fb
tgt
tlp
ufw
umtp-responder
unscd
walinuxagent
wdm
whereami
xrdp
yubikey-luks
zfs-linux
zram-config

Please note: this list is probably *not* comprehensive. We've only searched for --no-restart-on-upgrade, --no-stop-on-upgrade, and --no-restart-after-upgrade but it's possible packages may rely on the short-form of these options as well (-r and -R). However, these are rather impractical to search for, and I would *hope* that the vast majority of d/rules implementations use the long form so if we are missing any packages, the impact is (hopefully!) minimal.

I hit a problem in a dep8 test that relies on slapd (from openldap) to be restarted after a "dpkg-reconfigure slapd". This restart stopped happening in 2.5.11+dfsg-1~exp1ubuntu3, which was built with 13.6ubuntu1.

See how the authentication only works after I restart slapd manually:
root@j-slapd:~# pidof slapd
3570
root@j-slapd:~# /home/ubuntu/set-selections.sh
info: Trying to set 'slapd/domain' [string] to 'example.com'
info: Loading answer for 'slapd/domain'
info: Trying to set 'shared/organization' [string] to 'example'
info: Loading answer for 'shared/organization'
info: Trying to set 'slapd/password1' [password] to 'secret'
info: Loading answer for 'slapd/password1'
info: Trying to set 'slapd/password2' [password] to 'secret'
info: Loading answer for 'slapd/password2'
root@j-slapd:~# rm -rf /var/backups/*slapd* /var/backups/unknown*ldapdb
root@j-slapd:~# dpkg-reconfigure -fnoninteractive -pcritical slapd
  Backing up /etc/ldap/slapd.d in /var/backups/slapd-2.5.11+dfsg-1~exp1ubuntu3... done.
  Moving old database directory to /var/backups:
  - directory unknown... done.
  Creating initial configuration... done.
  Creating LDAP directory... done.
root@j-slapd:~# pidof slapd
3570
root@j-slapd:~# ldapwhoami -x -D cn=admin,dc=example,dc=com -w secret
ldap_bind: Invalid credentials (49)
root@j-slapd:~# systemctl restart slapd
root@j-slapd:~# ldapwhoami -x -D cn=admin,dc=example,dc=com -w secret
dn:cn=admin,dc=example,dc=com
root@j-slapd:~#

With the packages from the release pocket, slapd is restarted after the dpkg-reconfigure and the test works.

I'm attaching a diff between the prelease and proposed maintainerscripts for slapd.

Here is a little reproducer I'm using.

apt update
DEBIAN_FRONTEND=noninteractive apt-get -y --allow-unauthenticated -o Dpkg::Options::='--force-confold' install slapd ldap-utils

debconf-set-selections -v << EOF
slapd slapd/domain string example.com
slapd shared/organization string example
slapd slapd/password1 password secret
slapd slapd/password2 password secret
EOF

rm -rf /var/backups/*slapd* /var/backups/unknown*ldapdb

# note slapd pid

pidof slapd

dpkg-reconfigure -fnoninteractive -pcritical slapd

# check if slapd was restarted
pidof slapd

# also, this command should work
# ldapwhoami -x -D cn=admin,dc=example,dc=com -w secret
dn:cn=admin,dc=example,dc=com

I'm attaching set -x outputs from all slapd maintainer scripts during the reconfigure, in each case (proposed and release)

Many thanks to Andreas for the report, and the logs. I suppose it was too much to hope that this went through entirely smoothly, and so it has come to pass! After digging into the logs above, and then dpkg-reconfigure's code it appears that it does the following (see [1] for details):

* Run the package's prerm script (if it exists)
* Run the package's config script (ditto)
* Run the package's postinst script (ditto)

This may seem a little odd; why run the prerm script if you don't intend on removing the package? Presumably because, prior to the change made above, that's where the "stop services" action went if --no-restart-after-upgrade was specified in d/rules. However, as discussed above, that leaves the restart behaviour of a package spread across two distinct versions in the case of an upgrade (because it's actually the *old* version's prerm that runs, followed by the *new* versions's postinst to start them up again). Now, it's the package's preinst script that handles stopping services.

Therefore, if we were being strict, dpkg-reconfigure ought to run just the preinst, config, and postinst scripts, avoiding the prerm entirely. Unfortunately, I suspect that's likely to break anything that relies upon it running the prerm script, so that's probably not a wise idea. I'll propose a change upstream to add preinst to the running order, leaving prerm alone for now, and I'll post a debdiff with a similar change here.

[1]: https://salsa.debian.org/pkg-debconf/debconf/-/blob/master/dpkg-reconfigure#L196-198

This bug was fixed in the package debhelper - 13.6ubuntu1

---------------
debhelper (13.6ubuntu1) jammy; urgency=medium

  * Merge from Debian unstable (LP: #1960248). Remaining changes:
    - Generate ddebs from debhelper instead of pkg-create-dbgsym:
      - Set DBGSYM_PACKAGE_TYPE to ddeb to get correct debian/files output.
    - dh_installchangelogs: Do not install upstream changelog in compat level
      7 and higher to avoid pointlessly bloating installed packages.
    - dh_strip: Strip LTO sections unless --keep-lto is given.
    - dh_strip: For a static archive, test if any .text sections are non-
      empty. Warn on empty archives.
    - objcopy/strip changed in 2.36.1, not keeping file attributes of the
      original file. Work around that in dh_strip to write to a temporary file
      and cat'ing this to the original file to keep the original attributes.
    - Imply '<!noudeb>' profile if not set on package type udeb.
    - dh_strip: Set a unique build-id before stripping files.
    - Allow dh_strip to be larger for the tests
  * Add awk dependency implied by empty .text section check
  * Fix restart behaviour of packages marked either
    - no-stop-on-upgrade or --no-restart-after-upgrade (LP: #1959054)

debhelper (13.6) unstable; urgency=medium

  [ Niels Thykier ]
  * dh_assistant: Avoid creating `debian/.debhelper` when the
    which-build-system sub command is invoked. Thanks to
    Jelmer Vernooij for spotting that bug.
  * dh_assistant: Add new active-compat-level command, which
    outputs information about which compat level is declared
    and active. It also tells how the compat level was
    declared.
  * Dh_Lib.pm: Add new function, get_non_binnmu_date_epoch,
    only needed for dh_strip_nondeterminism.
  * dh_installcron: Add support for `cron.yearly` packaging
    file. Thanks to Martin-Éric Racine for the suggestion.
    (Closes: #1000363)
  * Dh_Lib.pm: Remove support for compat 5 and 6.
  * debhelper.pod,debhelper-obsolete-compat.pod: Update to
    reflect the new status for compat 5 and 6.

  [ Guillem Jover ]
  * dh: Add missing _ in execute_after example in POD.

  [ Sandro Tosi ]
  * dh_compress: Exclude .woff and .woff2 by default.

  [ Translations ]
  * Update Portuguese translation (Américo Monteiro)
    (Closes: #1000719)

 -- Dave Jones <email address hidden> Mon, 07 Feb 2022 15:59:07 +0000

@ahasenack Could you try the debconf version from [1]? I've patched that with the same fix that I've proposed upstream [2]. I'll add the relevant debian bug here too. I realize this is *technically* a separate issue, but it's all so interconnected and related that I'd prefer to keep it all in one place.

[1]: https://launchpad.net/~waveform/+archive/ubuntu/dh-merge

[2]: https://salsa.debian.org/pkg-debconf/debconf/-/merge_requests/10

Gladly!

It worked

ii debconf 1.5.79ubuntu1 all Debian configuration management system

root@j-slapd-reconfigure:~# pidof slapd
105004
root@j-slapd-reconfigure:~# dpkg-reconfigure -fnoninteractive -pcritical slapd
  Backing up /etc/ldap/slapd.d in /var/backups/slapd-2.5.11+dfsg-1~exp1ubuntu3... done.
  Moving old database directory to /var/backups:
  - directory unknown... done.
  Creating initial configuration... done.
  Creating LDAP directory... done.

root@j-slapd-reconfigure:~# pidof slapd
105415
root@j-slapd-reconfigure:~#

Excellent, happy with that. I'm attaching a debdiff for the debconf change, and will see if I can rustle up a sponsor on Monday to finally put all this to bed!

This bug was fixed in the package debconf - 1.5.79ubuntu1

---------------
debconf (1.5.79ubuntu1) jammy; urgency=medium

  * dpkg-reconfigure: fix restart of services declared with
    --no-restart-after-upgrade (LP: #1959054)

 -- Dave Jones <email address hidden> Sun, 20 Feb 2022 14:42:49 +0000

See bug 1965758 this also breaks focal-backports - please upload the fix there as well.

Note related bug if a package were to use debhelper-compat 10 vs 9 on focal.

In Focal, debhelper-compat 10 will incorrectly introduce ignore the params

        dh_systemd_start --no-restart-on-upgrade --no-start

And will still inject which will restart all services across package upgrade with something like the following pseudo script:

deb-systemd-invoke try-restart <all services>

Moral of the story, don't shift your debian/compat version on stable releases or bad things could happen.

See: LP: #1999159