Merge ~sergiodj/ubuntu/+source/nfs-utils:bug1918141-service-file-adjustment-groovy into ubuntu/+source/nfs-utils:ubuntu/groovy-devel

[Sergio Durigan Junior]

This is a fix for bug 1918141 on groovy.

The issue here is that nfs-utils' systemd service file doesn't properly depend on network-online.target, which can cause problems when the host(s) listed on /etc/exports need to be resolved (i.e., are hostnames, not IP addresses).

The bug is a bit tricky to reproduce, but I wrote an SRU template which contains detailed instructions on how to do that.

nfs-utils is really old on Ubuntu/Debian, but I was able to backport the proper fixes from the upstream git. You will notice that, although the changes made to the service file are simple, I opted to keep them separated in two patches. This is done to reflect the fact that the change was made in two upstream commits.

I've already uploaded the fix to hirsute:

https://code.launchpad.net/~sergiodj/ubuntu/+source/nfs-utils/+git/nfs-utils/+merge/399688

You can find a PPA with the proposed fix here:

https://launchpad.net/~sergiodj/+archive/ubuntu/nfs-utils-bug1918141/+packages

autopkgtest is still happy:

autopkgtest [17:52:10]: @@@@@@@@@@@@@@@@@@@@ summary
local-server-client PASS

[Utkarsh Gupta (utkarsh)]

Hello,

Since I and Christian pair-reviewed this earlier today, I'm going to drop my review here as well!

* Changelog:
  - [✓] old content and logical tag match as expected
  - [✓] changelog entry correct version and targeted codename
  - [✓] changelog entries correct
  - [✓] update-maintainer has been run

* New Delta:
  - [✓] patches match what was proposed upstream
  - [✓] patches correctly included in debian/patches/series
  - [?] patches have correct DEP3 metadata

Minor comment here:
The first patch in the header (line 46 of the diff) has "Applied-Upstream: 2.1.2-rc2", which seems to be missing in the second patch. Whilst a trivial nitpick, but since it was there in the first patch, it looks like it wasn't included in the second one intentionally, which would mean that it wasn't applied upstream - which is not the case. So for this + consistency reasons, could you add the same line in the second patch as well?

* Build/Test:
  - [✓] build is ok
  - [✓] verified PPA package installs/uninstalls
  - [✓] autopkgtest against the PPA package passes

----------------------------------------------------

All looks good, no blocker, so I'm going to approve this.
Besides, is it a Ubuntu-specific problem? Is not not affecting Debian? If it is, do you intend to forward this upstream? If it isn't, then ignore this part! :)

review: Approve

[Sergio Durigan Junior (sergiodj)]

Thanks, I've added the header to the patch (the reply to the Debian bug question can be found in the Focal MP).

Pushed & uploaded:

$ git push pkg upload/1%1.3.4-2.5ubuntu6.1
Enumerating objects: 17, done.
Counting objects: 100% (17/17), done.
Delta compression using up to 8 threads
Compressing objects: 100% (12/12), done.
Writing objects: 100% (12/12), 3.22 KiB | 548.00 KiB/s, done.
Total 12 (delta 8), reused 0 (delta 0)
To ssh://git.launchpad.net/ubuntu/+source/nfs-utils
 * [new tag] upload/1%1.3.4-2.5ubuntu6.1 -> upload/1%1.3.4-2.5ubuntu6.1
$ dput nfs-utils_1.3.4-2.5ubuntu6.1_source.changes
Trying to upload package to ubuntu
Checking signature on .changes
gpg: /home/sergio/work/nfs-utils/nfs-utils_1.3.4-2.5ubuntu6.1_source.changes: Valid signature from 106DA1C8C3CBBF14
Checking signature on .dsc
gpg: /home/sergio/work/nfs-utils/nfs-utils_1.3.4-2.5ubuntu6.1.dsc: Valid signature from 106DA1C8C3CBBF14
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading nfs-utils_1.3.4-2.5ubuntu6.1.dsc: done.
  Uploading nfs-utils_1.3.4-2.5ubuntu6.1.debian.tar.xz: done.
  Uploading nfs-utils_1.3.4-2.5ubuntu6.1_source.buildinfo: done.
  Uploading nfs-utils_1.3.4-2.5ubuntu6.1_source.changes: done.
Successfully uploaded packages.

Unmerged commits

d732e28... by Sergio Durigan Junior on 2021-03-16

changelog for 1:1.3.4-2.5ubuntu6.1

1a06dc9... by Sergio Durigan Junior on 2021-03-15

  * Depend on network-online.target when starting services. (LP: #1918141)
    - d/p/lp1918141-use-network-online-target-01.patch: Declare a
      Wants=network-online.target on all NFS server services.
    - d/p/lp1918141-use-network-online-target-02.patch: Declare a
      After=network-online.target on all NFS server services.
    Thanks to Niklas Edmundsson for helping with the reproducer.

diff --git a/debian/changelog b/debian/changelog
index 49e85c3..5685bfe 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,14 @@
+nfs-utils (1:1.3.4-2.5ubuntu6.1) groovy; urgency=medium
+
+  * Depend on network-online.target when starting services. (LP: #1918141)
+    - d/p/lp1918141-use-network-online-target-01.patch: Declare a
+      Wants=network-online.target on all NFS server services.
+    - d/p/lp1918141-use-network-online-target-02.patch: Declare a
+      After=network-online.target on all NFS server services.
+    Thanks to Niklas Edmundsson for helping with the reproducer.
+
+ -- Sergio Durigan Junior <[e-mail]>  Tue, 16 Mar 2021 17:12:01 -0400
+
 nfs-utils (1:1.3.4-2.5ubuntu6) groovy; urgency=medium
 
   * Allow compilation to succeed with -fno-common (Closes: #957608)
diff --git a/debian/patches/lp1918141-use-network-online-target-01.patch b/debian/patches/lp1918141-use-network-online-target-01.patch
new file mode 100644
index 0000000..680bb2f
--- /dev/null
+++ b/debian/patches/lp1918141-use-network-online-target-01.patch
@@ -0,0 +1,82 @@
+From: Steve Dickson <[e-mail]>
+Date: Mon, 10 Apr 2017 07:16:58 -0400
+Subject: systemd: NFS server services should use network-online
+
+There has been an number startup problems where parts of
+the NFS server fails to start due to DNS and other
+parts of the network not be up.
+
+Reading the systemd.special it seems network.target is
+a passive unit which does not wait for the entire
+network to come up and network-online.target is an
+active unit which does wait.
+
+So this adds Wants=network-online.target to all of
+the NFS server services
+
+Signed-off-by: Steve Dickson <[e-mail]>
+
+Origin: backport, http://git.linux-nfs.org/?p=steved/nfs-utils.git;a=commit;h=9d4fc3fb
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/nfs-utils/+bug/1918141
+Applied-Upstream: 2.1.2-rc2
+---
+ systemd/nfs-mountd.service       | 2 +-
+ systemd/nfs-server.service       | 2 +-
+ systemd/rpc-statd-notify.service | 2 +-
+ systemd/rpc-statd.service        | 3 ++-
+ 4 files changed, 5 insertions(+), 4 deletions(-)
+
+diff --git a/systemd/nfs-mountd.service b/systemd/nfs-mountd.service
+index 5f5f0a9..e4ac50d 100644
+--- a/systemd/nfs-mountd.service
++++ b/systemd/nfs-mountd.service
+@@ -2,8 +2,8 @@
+ Description=NFS Mount Daemon
+ DefaultDependencies=no
+ Requires=proc-fs-nfsd.mount
++Wants=network-online.target
+ After=proc-fs-nfsd.mount
+-After=network.target local-fs.target
+ After=rpcbind.socket
+ BindsTo=nfs-server.service
+ 
+diff --git a/systemd/nfs-server.service b/systemd/nfs-server.service
+index 23e67f8..bbcd967 100644
+--- a/systemd/nfs-server.service
++++ b/systemd/nfs-server.service
+@@ -3,7 +3,7 @@ Description=NFS server and services
+ DefaultDependencies=no
+ Requires=network.target proc-fs-nfsd.mount
+ Requires=nfs-mountd.service
+-Wants=rpcbind.socket
++Wants=rpcbind.socket network-online.target
+ Wants=nfs-idmapd.service
+ 
+ After=local-fs.target
+diff --git a/systemd/rpc-statd-notify.service b/systemd/rpc-statd-notify.service
+index 05c54ef..29a4821 100644
+--- a/systemd/rpc-statd-notify.service
++++ b/systemd/rpc-statd-notify.service
+@@ -1,7 +1,7 @@
+ [Unit]
+ Description=Notify NFS peers of a restart
+ DefaultDependencies=no
+-Requires=network.target
++Wants=network-online.target
+ After=local-fs.target network.target nss-lookup.target
+ 
+ # if we run an nfs server, it needs to be running before we
+diff --git a/systemd/rpc-statd.service b/systemd/rpc-statd.service
+index 2138f74..ee6e05c 100644
+--- a/systemd/rpc-statd.service
++++ b/systemd/rpc-statd.service
+@@ -3,7 +3,8 @@ Description=NFS status monitor for NFSv2/3 locking.
+ DefaultDependencies=no
+ Conflicts=umount.target
+ Requires=nss-lookup.target rpcbind.socket
+-After=network.target nss-lookup.target rpcbind.socket
++Wants=network-online.target
++After=nss-lookup.target rpcbind.socket
+ 
+ PartOf=nfs-utils.service
+ 
diff --git a/debian/patches/lp1918141-use-network-online-target-02.patch b/debian/patches/lp1918141-use-network-online-target-02.patch
new file mode 100644
index 0000000..52715e3
--- /dev/null
+++ b/debian/patches/lp1918141-use-network-online-target-02.patch
@@ -0,0 +1,75 @@
+From: Steve Dickson <[e-mail]>
+Date: Mon, 24 Apr 2017 11:25:39 -0400
+Subject: systemd: Afters are also needed for the Wants=network-online.target
+
+Commit 9d4fc3fb added Wants=network-online.target which
+is not enough to ensure the network is completely up
+before the NFS server is started. After=network-online.target
+is also needed.
+
+Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1419351
+
+Signed-off-by: Steve Dickson <[e-mail]>
+
+Origin: backport, http://git.linux-nfs.org/?p=steved/nfs-utils.git;a=commit;h=09e5c6c2a3f8eac91d5353e6d4ff6aee7757ab08
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/nfs-utils/+bug/1918141
+---
+ systemd/nfs-mountd.service       | 1 +
+ systemd/nfs-server.service       | 4 ++--
+ systemd/rpc-statd-notify.service | 2 +-
+ systemd/rpc-statd.service        | 2 +-
+ 4 files changed, 5 insertions(+), 4 deletions(-)
+
+diff --git a/systemd/nfs-mountd.service b/systemd/nfs-mountd.service
+index e4ac50d..ed357a2 100644
+--- a/systemd/nfs-mountd.service
++++ b/systemd/nfs-mountd.service
+@@ -4,6 +4,7 @@ DefaultDependencies=no
+ Requires=proc-fs-nfsd.mount
+ Wants=network-online.target
+ After=proc-fs-nfsd.mount
++After=network-online.target local-fs.target
+ After=rpcbind.socket
+ BindsTo=nfs-server.service
+ 
+diff --git a/systemd/nfs-server.service b/systemd/nfs-server.service
+index bbcd967..6cc7a77 100644
+--- a/systemd/nfs-server.service
++++ b/systemd/nfs-server.service
+@@ -6,8 +6,8 @@ Requires=nfs-mountd.service
+ Wants=rpcbind.socket network-online.target
+ Wants=nfs-idmapd.service
+ 
+-After=local-fs.target
+-After=network.target proc-fs-nfsd.mount rpcbind.socket nfs-mountd.service
++After=network-online.target local-fs.target
++After=proc-fs-nfsd.mount rpcbind.socket nfs-mountd.service
+ After=nfs-idmapd.service rpc-statd.service
+ Before=rpc-statd-notify.service
+ 
+diff --git a/systemd/rpc-statd-notify.service b/systemd/rpc-statd-notify.service
+index 29a4821..c1cf45c 100644
+--- a/systemd/rpc-statd-notify.service
++++ b/systemd/rpc-statd-notify.service
+@@ -2,7 +2,7 @@
+ Description=Notify NFS peers of a restart
+ DefaultDependencies=no
+ Wants=network-online.target
+-After=local-fs.target network.target nss-lookup.target
++After=local-fs.target network-online.target nss-lookup.target
+ 
+ # if we run an nfs server, it needs to be running before we
+ # tell clients that it has restarted.
+diff --git a/systemd/rpc-statd.service b/systemd/rpc-statd.service
+index ee6e05c..8961f64 100644
+--- a/systemd/rpc-statd.service
++++ b/systemd/rpc-statd.service
+@@ -4,7 +4,7 @@ DefaultDependencies=no
+ Conflicts=umount.target
+ Requires=nss-lookup.target rpcbind.socket
+ Wants=network-online.target
+-After=nss-lookup.target rpcbind.socket
++After=network-online.target nss-lookup.target rpcbind.socket
+ 
+ PartOf=nfs-utils.service
+ 
diff --git a/debian/patches/series b/debian/patches/series
index 9fac564..eac2e58 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -27,3 +27,5 @@ fix-start-ordering-2.patch
 fix-start-ordering-3.patch
 CVE-2019-3689.patch
 Allow-compilation-to-succeed-with-fno-common.patch
+lp1918141-use-network-online-target-01.patch
+lp1918141-use-network-online-target-02.patch