Merge ~ahasenack/ubuntu/+source/nfs-utils:impish-nfs-utils-restart-1928259 into ubuntu/+source/nfs-utils:ubuntu/devel

[Andreas Hasenack]

PPA: https://launchpad.net/~ahasenack/+archive/ubuntu/nfs-utils-restart-1928259

sudo add-apt-repository ppa:ahasenack/nfs-utils-restart-1928259

To test, follow the testing procedure in the SRU bug #1927745 and confirm that rpc.gssd was restarted after the package upgrade (different pid).

This is an attempt to fix the linked bug in the less intrusive way possible, because I want to use the same fix for an SRU all the way back to bionic.

The goal of this fix is to mark nfs-utils.service as "started", so that the debhelper postinst section will correctly restart it on upgrade.

Key aspects of this fix:
- nfs-utils.service is a "fake" service meant to control other units that declare PartOf=nfs-utils.service. It's ExecStart is simply /bin/true. There is also no [Install] section, so it can't be enabled as-is
- PartOf only has effect on "stop" and "restart":
"""
PartOf=
  Configures dependencies similar to Requires=, but limited to stopping and restarting of units. When systemd stops or restarts the units listed here, the action is propagated to this unit. Note that this is a one-way dependency — changes to this unit do not affect the listed units.
"""

This last bit is kind of crucial: it means that the "start" action has no effect on units linked via PartOf. It will just mark nfs-utils.service itself as being started (and it only runs /bin/true).

I tested this in some scenarios to make sure that the "start" action isn't starting anything by itself, and that remains true as far as I can see. The only thing that is flagged as started is nfs-utils.service itself, but just because of its RemainAfterExit=yes.

I also tested this when one of the services declaring PartOf=nfs-utils.service was masked or disabled, and it worked as expected. In both cases (masked or disabled) the service didn't start.

Another test I did was if nfs-utils.service was masked. In that case, the postinst complained about it, didn't error, and no dependent services were restarted at the end of the package upgrade, which is what I expected.

Still, this is probably not the way upstream intended for this to work (these systemd service files come from upstream, and Debian and Fedora are also affected by this bug). But a bigger surgery on all these systemd service files is out of scope for the sru, and therefore out of scope for this impish branch as well for now. But of course, if someone has a better suggestion, I'm all years :)

[Andreas Hasenack (ahasenack)]

And I just saw my hilarious commit author

[Andreas Hasenack (ahasenack)]

Fixed author and force pushed

[Dan Streetman (ddstreet)]

The partof configuration that upstream has decided to use is certainly a strange way to handle the various services they provide, and I feel like it invites fragility just like this bug, but an upstream discussion certainly is a different matter from fixing this in sru releases now.

The conflict here appears to be that upstream chose to make nfs-utils.service a static unit, while deb-systemd-invoke chose not to (re)start static units. Unfortunately, the reasoning for not (re)starting static units appears to be quite arbitrary:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=768456#35

My opinion is, if the package maintainer specifically chose to call 'dh_systemd_start' for a service that is static, then the package maintainer clearly knows (or should know) what they are talking about and deb-systemd-invoke should (re)start the service according to the package maintainer's instruction. However changing the script to (re)start static units isn't appropriate for srus (but IMHO should be fixed in Debian).

Of course the "appropriate" way to handle service restarts is for the services to be real, and to use try-restart. That would also handle the many problematic packages that start their services when the package is installed, and frequently those services fail to start due to missing or incorrect 'default' configuration (e.g.: tinyssh, dnsmasq, etc), but that's quite offtopic for this.

This seems fine as a 'least bad' way to handle things for nfs-utils for sru.

review: Approve

[Andreas Hasenack (ahasenack)]

> However changing the script to (re)start static units isn't appropriate for srus (but IMHO should be fixed in Debian).

Here you mean changing deb-systemd-invoke?

[Dan Streetman (ddstreet)]

> > However changing the script to (re)start static units isn't appropriate for srus (but IMHO should be fixed in Debian).
>
> Here you mean changing deb-systemd-invoke?

sorry, yes i meant changing deb-systemd-invoke to start static units would not be appropriate for sru; changing nfs-utils does seem correct.

[Andreas Hasenack (ahasenack)]

Thanks!

Pushing upload tag:

$ git push pkg upload/1%1.3.4-4ubuntu3
Enumerating objects: 13, done.
Counting objects: 100% (13/13), done.
Delta compression using up to 4 threads
Compressing objects: 100% (9/9), done.
Writing objects: 100% (9/9), 1.29 KiB | 42.00 KiB/s, done.
Total 9 (delta 6), reused 0 (delta 0), pack-reused 0
To ssh://git.launchpad.net/~usd-import-team/ubuntu/+source/nfs-utils
 * [new tag] upload/1%1.3.4-4ubuntu3 -> upload/1%1.3.4-4ubuntu3

Uploading to impish:
$ dput ubuntu ../nfs-utils_1.3.4-4ubuntu3_source.changes
Checking signature on .changes
gpg: ../nfs-utils_1.3.4-4ubuntu3_source.changes: Valid signature from AC983EB5BF6BCBA9
Checking signature on .dsc
gpg: ../nfs-utils_1.3.4-4ubuntu3.dsc: Valid signature from AC983EB5BF6BCBA9
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading nfs-utils_1.3.4-4ubuntu3.dsc: done.
  Uploading nfs-utils_1.3.4-4ubuntu3.debian.tar.xz: done.
  Uploading nfs-utils_1.3.4-4ubuntu3_source.buildinfo: done.
  Uploading nfs-utils_1.3.4-4ubuntu3_source.changes: done.
Successfully uploaded packages.

diff --git a/debian/changelog b/debian/changelog
index a240872..802d834 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+nfs-utils (1:1.3.4-4ubuntu3) impish; urgency=medium
+
+  * d/nfs-common.postinst: always start nfs-utils.service, so the restart in
+    the #DEBHELPER# section can do its job if needed (LP: #1928259)
+
+ -- Andreas Hasenack <[e-mail]>  Mon, 24 May 2021 17:59:39 -0300
+
 nfs-utils (1:1.3.4-4ubuntu2) hirsute; urgency=medium
 
   * Depend on network-online.target when starting services. (LP: #1918141)
diff --git a/debian/nfs-common.postinst b/debian/nfs-common.postinst
index da6ffcb..28e9d31 100644
--- a/debian/nfs-common.postinst
+++ b/debian/nfs-common.postinst
@@ -43,6 +43,10 @@ case "$1" in
 	if [ -f /lib/init/rw/sendsigs.omit.d/statd ]; then 
 	    mv /lib/init/rw/sendsigs.omit.d/statd /run/sendsigs.omit.d/statd
 	fi
+
+        # always "start" nfs-utils.service, so package upgrades will restart it,
+        # see LP: #1928259
+        systemctl start nfs-utils.service > /dev/null || true
     ;;
 esac