Merge ~mitchdz/ubuntu/+source/multipath-tools:mitch/update_multipathd_service into ubuntu/+source/multipath-tools:ubuntu/devel

[Mitchell Dzurick]

PPA - https://launchpad.net/~mitchdz/+archive/ubuntu/multipath-tools-change-service-requires

[Mitchell Dzurick (mitchdz)]

Note: Attempting to upstream this change

https://github.com/opensvc/multipath-tools/pull/72

Upstream is responsive so we can wait to see if they accept it

[Mitchell Dzurick (mitchdz)]

upstream rejected this change, and for good reason. Users could want to activate the service, without activating the socket. Adding `Wants=multipathd.socket` will start the socket whenever we start the service.

However, with that said, the current maintainer scripts enable _both_ the service/socket so it seems we are going into the direction of having both active by default on boot.

This current behavior is honestly debatable, and in the future we should consider moving to a socket-based activation approach so users can leave the service inactive until used. I believe this is the direction a lot of packages are heading towards.

I would still like to propose this change as it provides robust handling for _our_ configuration, and users can disable the service later on if they want.

I am still looking for good alternatives, there is one I have identified, which is to add --no-start to dh_installsystemd and then handle restarting/starting the service/socket in postinst after the #DEBHELPER#. I don't particularly like this one, I feel like there should be a way in the debhelper scripts to force the order to be start the socket and then the service.

[Andreas Hasenack (ahasenack)]

Looking at this.

In the meantime, have you dug into the debhelper script to try to understand why they use the ordering the use? Is it deterministic, specifically chosen to be the way it is?

[Andreas Hasenack (ahasenack)]

I keep going back to: why are we shipping a service and socket and enabling both? And with this "Wants=multipathd.socket" added, we are essentially linking them together? Rethorical question, mind you :)

I'll investigate a bit more what debhelper is doing. But in the back of my mind, thinking that we are in feature freeze, maybe the best approach will be to just manually handle these bits in postinst, without debhelper, so we can specify the ordering, and take a step back in the next cycle and evaluate how we want to ship multipath-tools.

To be continued.

[Mitchell Dzurick (mitchdz)]

Thanks for looking Andreas! I definitely agree with your assessment.

The reason I like adding the dependency is that it just makes handling the service/socket more robust, and that's a quality I really appreciate when it comes to the maintainer scripts. Also helps the users not get the socket inactive as well.

I'm also cautious that maybe doing systemctl start/restart multipathd.socket multipathd.service could cause issues. I have been experimenting with doing that, and I was able to get the system into a state where I couldn't restart the socket. I'll see if I can reproduce that state, because I definitely don't want to lock up either the service/socket on upgrade. Potentially it was caused by other things I was doing so I'll attempt it in a fresh vm.

[Andreas Hasenack (ahasenack)]

Continuing to look at this and comparing to other packages, it's now looking more and more like your initial solution (Wants=) is the best indeed.

Continuing.

[Mitchell Dzurick (mitchdz)]

I'm going to write up a quick bash script to more thoroughly test how the service/socket responds when it's in different states. I want to ensure that on upgrade, the service/socket don't go from active -> inactive on the user.

For now, I will assume that going from Inactive -> Active is okay on upgrade. e.g. If the user has the service active but socket inactive, on upgrade it is fine for both the service/socket to be active, but not okay for the service to be inactive.

I'll come back with the results soon.

[Andreas Hasenack (ahasenack)]

One thing I tried was disabling the service, and then seeing how things behaved. Because of the "Also=multipathd.socket" line in the service's [Install] section, that will at first also disable the socket (not something new in this branch). You can then enable just the socket, and you will have things working again and the service started on-demand. So we didn't lose this behavior, good.

Your scripts could become a new DEP8 test, btw :)

But one thing to keep in mind: today is beta freeze (in a few hours at most). If we don't get this in now, maybe we will only have a chance again after mantic is released.

[Andreas Hasenack (ahasenack)]

I didn't find a multipathc (client) command that would not require multipathd running, so I didn't quite understand the scenario in [1]. But I didn't try all of them.

1. https://github.com/opensvc/multipath-tools/pull/72#issuecomment-1720156274

[Mitchell Dzurick (mitchdz)]

Apologies in advance for the long wall of text. It seems I can't add text files to a MP.

After further testing, I'd like to recommend NOT going with the route of only adding Wants=.

Instead I'd like to start/restart multipathd like so:

systemctl stop multipathd.service multipathd.socket
systemctl start multipathd.socket multipathd.service

This provides the most robust solution out of my testing, where the service/socket are in various states.

You can reproduce my testing with the following steps:

$ lxc launch ubuntu-daily:mantic test-multipathd --vm
$ lxc shell test-multipathd
# git clone https://github.com/mitchdz/test_multipath-tools-systemctl.git
# echo "No Wants="
# test_multipath-tools-systemctl/test_multipathd.sh
# echo ""
# sed -i '8i Wants=multipathd.socket' /usr/lib/systemd/system/multipathd.service
# echo "With Wants="
# test_multipath-tools-systemctl/test_multipathd.sh

An 'O' indicates $? == 0
an 'X' indicates $? != 0

The results are:
No Wants=
Testing systemctl restart multipathd.socket multipathd.service
X | service active && socket active
X | service active && socket inactive
O | service inactive && socket active
O | service inactive && socket inactive
Testing systemctl restart multipathd.service multipathd.socket
O | service active && socket active
O | service active && socket inactive
X | service inactive && socket active
X | service inactive && socket inactive
Testing systemctl start multipathd.socket multipathd.service
O | service active && socket active
X | service active && socket inactive
O | service inactive && socket active
O | service inactive && socket inactive
Testing systemctl start multipathd.service multipathd.socket
O | service active && socket active
X | service active && socket inactive
O | service inactive && socket active
X | service inactive && socket inactive
Testing systemctl stop multipathd.service multipathd.socket; systemctl start multipathd.socket multipathd.service
O | service active && socket active
O | service active && socket inactive
O | service inactive && socket active
O | service inactive && socket inactive

With Wants=
Testing systemctl restart multipathd.socket multipathd.service
X | service active && socket active
X | service active && socket inactive
O | service inactive && socket active
O | service inactive && socket inactive
Testing systemctl restart multipathd.service multipathd.socket
O | service active && socket active
O | service active && socket inactive
X | service inactive && socket active
X | service inactive && socket inactive
Testing systemctl start multipathd.socket multipathd.service
O | service active && socket active
X | service active && socket inactive
O | service inactive && socket active
O | service inactive && socket inactive
Testing systemctl start multipathd.service multipathd.socket
O | service active && socket active
X | service active && socket inactive
O | service inactive && socket active
O | service inactive && socket inactive
Testing systemctl stop multipathd.service multipathd.socket; systemctl start multipathd.socket multipathd.service
O | service active && socket active
O | service active && socket inactive
O | service inactive && socket active
O | ...

[Mitchell Dzurick (mitchdz)]

This MP is what I believe is a more robust solution to this issue - https://code.launchpad.net/~mitchdz/ubuntu/+source/multipath-tools/+git/multipath-tools/+merge/451558

[Andreas Hasenack (ahasenack)]

Which one would you like to propose, this one here, or the one you linked in the comment above? Then please reject/delete one, and have only one up for review.

[Andreas Hasenack (ahasenack)]

review: Needs Information

[Mitchell Dzurick (mitchdz)]

Hi Andreas, I think the merge proposal I linked above is a better solution.

However, there's no real rush in fixing this as it's only a bug on upgrade, and I would like to see what eventually happens in Debian with regards to socket based activation, as whatever approach Debian chooses may be better to bring backwards.

Unmerged commits

0b39ea5... by Mitchell Dzurick on 2023-09-13

changelog

3fdcffb... by Mitchell Dzurick on 2023-09-13

d/p/lp2034471-multipathd-add-Wants-socket.patch: Add Wants=multipathd.socket (LP: #2034471)

in multipathd.service. This solves the issue where systemd knows the start
order of the service/socket, but there is no dependency explicitly mentioned.
This causes an issue if you try to start the socket after the service by
doing something such as

    systemctl stop multipathd.service multipathd.socket
    systemctl start multipathd.service multipathd.socket

The socket would see that it is already active, thus causing an error. This
results in the socket ending up in an inactive state.

diff --git a/debian/changelog b/debian/changelog
index 032e3d0..f31ac22 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+multipath-tools (0.9.4-5ubuntu4) mantic; urgency=medium
+
+  * d/p/lp2034471-multipathd-add-Wants-socket.patch: add
+    Wants=multipathd.socket (LP: #2034471)
+
+ -- Mitchell Dzurick <[e-mail]>  Wed, 13 Sep 2023 12:20:16 -0400
+
 multipath-tools (0.9.4-5ubuntu3) mantic; urgency=medium
 
   * d/initramfs/scripts/{local-bottom,local-top}/multipath: Make initramfs
diff --git a/debian/patches/lp2034471-multipathd-add-Wants-socket.patch b/debian/patches/lp2034471-multipathd-add-Wants-socket.patch
new file mode 100644
index 0000000..ef16218
--- /dev/null
+++ b/debian/patches/lp2034471-multipathd-add-Wants-socket.patch
@@ -0,0 +1,27 @@
+Description: Add Wants=multipathd.socket to multipathd.service
+    This solves the issue where systemd knows the start order of the 
+    service/socket, but there is no dependency explicitly mentioned. This 
+    causes an issue if you try to start the socket after the service by doing
+    something such as
+    
+        systemctl stop multipathd.service multipathd.socket
+        systemctl start multipathd.service multipathd.socket
+    
+    The socket would see that it is already active, thus causing an error. This
+    results in the socket ending up in an inactive state.
+Author: Mitchell Dzurick <[e-mail]>
+Forwarded: https://github.com/opensvc/multipath-tools/pull/72
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/multipath-tools/+bug/2034471
+Last-Update: 2023-09-13
+---
+This patch header follows DEP-3: http://dep.debian.net/deps/dep3/
+--- a/multipathd/multipathd.service
++++ b/multipathd/multipathd.service
+@@ -5,6 +5,7 @@
+ Wants=systemd-udevd-kernel.socket
+ After=systemd-udevd-kernel.socket
+ After=multipathd.socket systemd-remount-fs.service
++Wants=multipathd.socket
+ Before=initrd-cleanup.service
+ DefaultDependencies=no
+ Conflicts=shutdown.target
diff --git a/debian/patches/series b/debian/patches/series
index 1a0661f..dac4dea 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -5,3 +5,4 @@ enable-find-multipaths.patch
 0009-kpartx-rules-use-Debian-specific-partx-path.patch
 0012-Reproducible-build.patch
 0006-multipathd.service-re-add-ExecStartPre.patch
+lp2034471-multipathd-add-Wants-socket.patch