Merge ~sergiodj/ubuntu/+source/drbd-utils:fix-timeout-jammy into ubuntu/+source/drbd-utils:ubuntu/jammy-devel

[Sergio Durigan Junior]

This MP fixes bug #2043817.

The problem and the solution are described in the SRU text.

PPA: https://launchpad.net/~sergiodj/+archive/ubuntu/drbd-utils

dep8 results (superficial) to be posted later.

[Andreas Hasenack (ahasenack)]

+1, just a nit in the version number for the SRU.

Agreed only jammy is affected, and the second patch on top is much better, as parsing --help is really brittle.

review: Approve

[git-ubuntu bot (git-ubuntu-bot)]

Approvers: sergiodj, ahasenack
Uploaders: sergiodj, ahasenack
MP auto-approved

review: Approve

[Sergio Durigan Junior (sergiodj)]

Thanks, Andreas.

Uploaded:

$ dput drbd-utils_9.15.0-1ubuntu0.1_source.changes
Trying to upload package to ubuntu
Checking signature on .changes
gpg: /home/sergio/work/drbd-utils/drbd-utils_9.15.0-1ubuntu0.1_source.changes: Valid signature from 106DA1C8C3CBBF14
Checking signature on .dsc
gpg: /home/sergio/work/drbd-utils/drbd-utils_9.15.0-1ubuntu0.1.dsc: Valid signature from 106DA1C8C3CBBF14
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading drbd-utils_9.15.0-1ubuntu0.1.dsc: done.
  Uploading drbd-utils_9.15.0-1ubuntu0.1.debian.tar.xz: done.
  Uploading drbd-utils_9.15.0-1ubuntu0.1_source.buildinfo: done.
  Uploading drbd-utils_9.15.0-1ubuntu0.1_source.changes: done.
Successfully uploaded packages.

diff --git a/debian/changelog b/debian/changelog
index 53517a4..24ec5f6 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+drbd-utils (9.15.0-1ubuntu0.1) jammy; urgency=medium
+
+  * d/p/lp2043817-fix-timeout-pacemaker-jammy-*.patch: Fix timeout
+    issue with Pacemaker. (LP: #2043817)
+
+ -- Sergio Durigan Junior <[e-mail]>  Tue, 21 Nov 2023 14:16:59 -0500
+
 drbd-utils (9.15.0-1build2) jammy; urgency=medium
 
   * No-change rebuild for ppc64el baseline bump.
diff --git a/debian/control b/debian/control
index f0ee3e6..2cd9978 100644
--- a/debian/control
+++ b/debian/control
@@ -1,7 +1,8 @@
 Source: drbd-utils
 Section: admin
 Priority: optional
-Maintainer: Debian DRBD Maintainers <[e-mail]>
+Maintainer: Ubuntu Developers <[e-mail]>
+XSBC-Original-Maintainer: Debian DRBD Maintainers <[e-mail]>
 Uploaders: Norbert Tretkowski <[e-mail]>,
  Martin Loschwitz <[e-mail]>,
  Philipp Hug <[e-mail]>,
diff --git a/debian/patches/lp2043817-fix-timeout-pacemaker-jammy-01.patch b/debian/patches/lp2043817-fix-timeout-pacemaker-jammy-01.patch
new file mode 100644
index 0000000..6436be4
--- /dev/null
+++ b/debian/patches/lp2043817-fix-timeout-pacemaker-jammy-01.patch
@@ -0,0 +1,138 @@
+From: Lars Ellenberg <[e-mail]>
+Date: Wed, 12 Jan 2022 13:50:35 +0100
+Subject: crm-fence-peer: fix timeout with Pacemaker 2.1: milli seconds vs
+ seconds
+MIME-Version: 1.0
+Content-Type: text/plain; charset="utf-8"
+Content-Transfer-Encoding: 8bit
+
+crmadmin timeout was in milli seconds for <= 2.0.x,
+but became a TIMESPEC with default seconds in >= 2.1.
+
+Up to 2.0.4, atoi() was used, which effectively ignores "trailing garbage",
+so we could get away with always appending "ms".
+But with 2.0.5, it became g_option_context_parse G_OPTION_ARG_INT, which
+"Cannot parse integer value “200ms” for --timeout" :-|
+
+So grep the help message for "timeout.*milliseconds",
+and if not present, append an explicit "ms" unit.
+
+Also tolerate both ": ok" (2.1) and " (ok)" (older)
+when matching the output string of crmadmin -S.
+
+
+Origin: upstream, https://github.com/LINBIT/drbd-utils/commit/8a28be74bc6efa93931c957e54c01abb18b984fe
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/drbd-utils/+bug/2043817
+---
+ scripts/crm-fence-peer.9.sh | 24 +++++++++++++++++++++---
+ scripts/crm-fence-peer.sh   | 24 +++++++++++++++++++++---
+ 2 files changed, 42 insertions(+), 6 deletions(-)
+
+diff --git a/scripts/crm-fence-peer.9.sh b/scripts/crm-fence-peer.9.sh
+index 36590bd..c943bf9 100755
+--- a/scripts/crm-fence-peer.9.sh
++++ b/scripts/crm-fence-peer.9.sh
+@@ -392,6 +392,20 @@ check_cluster_properties()
+ 	crm_is_not_false ${stonith_enabled:-} && stonith_enabled=true || stonith_enabled=false
+ }
+ 
++setup_crm_timeout_unit_ms()
++{
++	# crmadmin timeout was in ms for <= 2.0.x,
++	# but became a TIMESPEC in >= 2.1.
++	# Up to 2.0.4, atoi() was used, which effectively ignores "trailing
++	# garbage", so we could get away with always appending "ms", but with
++	# 2.0.5, it became g_option_context_parse G_OPTION_ARG_INT, which
++	# "Cannot parse integer value “200ms” for --timeout" :-|
++	if crmadmin --help 2>&1 | grep -q -e "--timeout=.*in milliseconds"; then
++		crm_timeout_unit_ms=""
++	else
++		crm_timeout_unit_ms="ms"
++	fi
++}
+ 
+ #
+ # In case this is a two-node cluster (still common with
+@@ -737,6 +751,7 @@ drbd_peer_fencing()
+ 
+ 		local startup_fencing stonith_enabled
+ 		check_cluster_properties
++		setup_crm_timeout_unit_ms
+ 
+ 		if ! $had_constraint_on_entry ; then
+ 
+@@ -1075,14 +1090,17 @@ _check_peer_node_reachable()
+ 		# it is obviously reachable.
+ 		#
+ 		# Do this only after we have been able to reach a DC above.
+-		# Note: crmadmin timeout is in milli-seconds, and defaults to 30000 (30 seconds).
++		# Note: crmadmin timeout defaults to 30 seconds.
++		#
+ 		# Our variable $cibtimeout should be in deci-seconds (see above)
+ 		# (unless you use a very old version of pacemaker, so don't do that).
+ 		# Convert deci-seconds to milli-seconds, and double it.
++		# See also setup_crm_timeout_unit_ms() above.
++		#
+ 		if [[ $crmd = "online" ]] ; then
+ 			local out
+-			if out=$( crmadmin -t $(( cibtimeout * 200 )) -S $DRBD_PEER ) \
+-			&& [[ $out = *"(ok)" ]]; then
++			if out=$( crmadmin -t $(( cibtimeout * 200 ))$crm_timeout_unit_ms -S $DRBD_PEER ) \
++			&& [[ $out = *@(": ok"|" (ok)") ]]; then
+ 				peer_state="reachable"
+ 				return
+ 			fi
+diff --git a/scripts/crm-fence-peer.sh b/scripts/crm-fence-peer.sh
+index cb5dede..9678673 100755
+--- a/scripts/crm-fence-peer.sh
++++ b/scripts/crm-fence-peer.sh
+@@ -244,6 +244,20 @@ check_cluster_properties()
+ 	crm_is_not_false $stonith_enabled && stonith_enabled=true || stonith_enabled=false
+ }
+ 
++setup_crm_timeout_unit_ms()
++{
++	# crmadmin timeout was in ms for <= 2.0.x,
++	# but became a TIMESPEC in >= 2.1.
++	# Up to 2.0.4, atoi() was used, which effectively ignores "trailing
++	# garbage", so we could get away with always appending "ms", but with
++	# 2.0.5, it became g_option_context_parse G_OPTION_ARG_INT, which
++	# "Cannot parse integer value “200ms” for --timeout" :-|
++	if crmadmin --help 2>&1 | grep -q -e "--timeout=.*in milliseconds"; then
++		crm_timeout_unit_ms=""
++	else
++		crm_timeout_unit_ms="ms"
++	fi
++}
+ 
+ #
+ # In case this is a two-node cluster (still common with
+@@ -426,6 +440,7 @@ drbd_peer_fencing()
+ 
+ 		local startup_fencing stonith_enabled
+ 		check_cluster_properties
++		setup_crm_timeout_unit_ms
+ 
+ 		if [[ -z $have_constraint ]] ; then
+ 			# try to place it.
+@@ -718,14 +733,17 @@ check_peer_node_reachable()
+ 		# it is obviously reachable.
+ 		#
+ 		# Do this only after we have been able to reach a DC above.
+-		# Note: crmadmin timeout is in milli-seconds, and defaults to 30000 (30 seconds).
++		# Note: crmadmin timeout defaults to 30 seconds.
++		#
+ 		# Our variable $cibtimeout should be in deci-seconds (see above)
+ 		# (unless you use a very old version of pacemaker, so don't do that).
+ 		# Convert deci-seconds to milli-seconds, and double it.
++		# See also setup_crm_timeout_unit_ms() above.
++		#
+ 		if [[ $crmd = "online" ]] ; then
+ 			local out
+-			if out=$( crmadmin -t $(( cibtimeout * 200 )) -S $DRBD_PEER ) \
+-			&& [[ $out = *"(ok)" ]]; then
++			if out=$( crmadmin -t $(( cibtimeout * 200 ))$crm_timeout_unit_ms -S $DRBD_PEER ) \
++			&& [[ $out = *@(": ok"|" (ok)") ]]; then
+ 				peer_state="reachable"
+ 				return
+ 			fi
diff --git a/debian/patches/lp2043817-fix-timeout-pacemaker-jammy-02.patch b/debian/patches/lp2043817-fix-timeout-pacemaker-jammy-02.patch
new file mode 100644
index 0000000..f7a5038
--- /dev/null
+++ b/debian/patches/lp2043817-fix-timeout-pacemaker-jammy-02.patch
@@ -0,0 +1,90 @@
+From: Lars Ellenberg <[e-mail]>
+Date: Wed, 12 Jan 2022 13:50:35 +0100
+Subject: crm-fence-peer: fix timeout with Pacemaker 2.0.5: milli seconds vs
+ seconds
+MIME-Version: 1.0
+Content-Type: text/plain; charset="utf-8"
+Content-Transfer-Encoding: 8bit
+
+Addendum to 8a28be74bc6efa93931c957e54c01abb18b984fe
+Commit message of the above cited here:
+
+> crmadmin timeout was in milli seconds for <= 2.0.x,
+> but became a TIMESPEC with default seconds in >= 2.1.
+>
+> Up to 2.0.4, atoi() was used, which effectively ignores "trailing garbage",
+> so we could get away with always appending "ms".
+> But with 2.0.5, it became g_option_context_parse G_OPTION_ARG_INT, which
+> "Cannot parse integer value “200ms” for --timeout" :-|
+>
+> So grep the help message for "timeout.*milliseconds",
+> and if not present, append an explicit "ms" unit.
+
+And this is where I got it wrong :-(
+somewhere later they re-organised the help text
+so now I would need to parse --help-all.
+
+Instead try to actually call "crmadmin -t 100ms --version".
+If that works, it apparently understands (or ignores)
+the "ms" unit.
+
+
+Origin: upstream, https://github.com/LINBIT/drbd-utils/commit/3eec04bc65b39b04be21d2689568892e7788abb3
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/drbd-utils/+bug/2043817
+---
+ scripts/crm-fence-peer.9.sh | 14 +++++++++++---
+ scripts/crm-fence-peer.sh   | 14 +++++++++++---
+ 2 files changed, 22 insertions(+), 6 deletions(-)
+
+diff --git a/scripts/crm-fence-peer.9.sh b/scripts/crm-fence-peer.9.sh
+index c943bf9..fc8d2bc 100755
+--- a/scripts/crm-fence-peer.9.sh
++++ b/scripts/crm-fence-peer.9.sh
+@@ -400,10 +400,18 @@ setup_crm_timeout_unit_ms()
+ 	# garbage", so we could get away with always appending "ms", but with
+ 	# 2.0.5, it became g_option_context_parse G_OPTION_ARG_INT, which
+ 	# "Cannot parse integer value “200ms” for --timeout" :-|
+-	if crmadmin --help 2>&1 | grep -q -e "--timeout=.*in milliseconds"; then
+-		crm_timeout_unit_ms=""
+-	else
++	# Can not parse the help text reliably, because they changed content
++	# and organisation of the help text between 2.0.4 and 2.0.5.
++	# Just try using ms unit, and see if it fails.
++	if crmadmin -t 100ms --version &> /dev/null; then
++		# this is either a recent version that actually understands ms
++		# as part of the TIMESPEC, or a version that still uses atoi().
+ 		crm_timeout_unit_ms="ms"
++	else
++		# this one likely failed with
++		# crmadmin: Cannot parse integer value “100ms” for -t
++		# (>= 2.0.5, < 2.1)
++		crm_timeout_unit_ms=""
+ 	fi
+ }
+ 
+diff --git a/scripts/crm-fence-peer.sh b/scripts/crm-fence-peer.sh
+index 9678673..b0e4e0f 100755
+--- a/scripts/crm-fence-peer.sh
++++ b/scripts/crm-fence-peer.sh
+@@ -252,10 +252,18 @@ setup_crm_timeout_unit_ms()
+ 	# garbage", so we could get away with always appending "ms", but with
+ 	# 2.0.5, it became g_option_context_parse G_OPTION_ARG_INT, which
+ 	# "Cannot parse integer value “200ms” for --timeout" :-|
+-	if crmadmin --help 2>&1 | grep -q -e "--timeout=.*in milliseconds"; then
+-		crm_timeout_unit_ms=""
+-	else
++	# Can not parse the help text reliably, because they changed content
++	# and organisation of the help text between 2.0.4 and 2.0.5.
++	# Just try using ms unit, and see if it fails.
++	if crmadmin -t 100ms --version &> /dev/null; then
++		# this is either a recent version that actually understands ms
++		# as part of the TIMESPEC, or a version that still uses atoi().
+ 		crm_timeout_unit_ms="ms"
++	else
++		# this one likely failed with
++		# crmadmin: Cannot parse integer value “100ms” for -t
++		# (>= 2.0.5, < 2.1)
++		crm_timeout_unit_ms=""
+ 	fi
+ }
+ 
diff --git a/debian/patches/series b/debian/patches/series
index d247244..65f9334 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -6,3 +6,5 @@
 0006-initscript-add-start-runlevels.patch
 0007-drbdmon-buildflags.patch
 0008-drbdmon-pass-LDFLAGS-to-g.patch
+lp2043817-fix-timeout-pacemaker-jammy-01.patch
+lp2043817-fix-timeout-pacemaker-jammy-02.patch