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

Proposed by Sergio Durigan Junior
Status: Merged
Approved by: git-ubuntu bot
Approved revision: not available
Merged at revision: 1f478da3901e355b6a283636af071f2e175de3cd
Proposed branch: ~sergiodj/ubuntu/+source/drbd-utils:fix-timeout-jammy
Merge into: ubuntu/+source/drbd-utils:ubuntu/jammy-devel
Diff against target: 279 lines (+239/-1)
5 files modified
debian/changelog (+7/-0)
debian/control (+2/-1)
debian/patches/lp2043817-fix-timeout-pacemaker-jammy-01.patch (+138/-0)
debian/patches/lp2043817-fix-timeout-pacemaker-jammy-02.patch (+90/-0)
debian/patches/series (+2/-0)
Reviewer Review Type Date Requested Status
git-ubuntu bot Approve
Andreas Hasenack Approve
Canonical Server Reporter Pending
Review via email: mp+457270@code.launchpad.net

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Andreas Hasenack (ahasenack) wrote :

+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
Revision history for this message
git-ubuntu bot (git-ubuntu-bot) wrote :

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

review: Approve
Revision history for this message
Sergio Durigan Junior (sergiodj) wrote :

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.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/debian/changelog b/debian/changelog
index 53517a4..24ec5f6 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
1drbd-utils (9.15.0-1ubuntu0.1) jammy; urgency=medium
2
3 * d/p/lp2043817-fix-timeout-pacemaker-jammy-*.patch: Fix timeout
4 issue with Pacemaker. (LP: #2043817)
5
6 -- Sergio Durigan Junior <sergio.durigan@canonical.com> Tue, 21 Nov 2023 14:16:59 -0500
7
1drbd-utils (9.15.0-1build2) jammy; urgency=medium8drbd-utils (9.15.0-1build2) jammy; urgency=medium
29
3 * No-change rebuild for ppc64el baseline bump.10 * 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 @@
1Source: drbd-utils1Source: drbd-utils
2Section: admin2Section: admin
3Priority: optional3Priority: optional
4Maintainer: Debian DRBD Maintainers <debian-ha-maintainers@lists.alioth.debian.org>4Maintainer: Ubuntu Developers <ubuntu-devel-discuss@lists.ubuntu.com>
5XSBC-Original-Maintainer: Debian DRBD Maintainers <debian-ha-maintainers@lists.alioth.debian.org>
5Uploaders: Norbert Tretkowski <norbert@tretkowski.de>,6Uploaders: Norbert Tretkowski <norbert@tretkowski.de>,
6 Martin Loschwitz <madkiss@debian.org>,7 Martin Loschwitz <madkiss@debian.org>,
7 Philipp Hug <debian@hug.cx>,8 Philipp Hug <debian@hug.cx>,
diff --git a/debian/patches/lp2043817-fix-timeout-pacemaker-jammy-01.patch b/debian/patches/lp2043817-fix-timeout-pacemaker-jammy-01.patch
8new file mode 1006449new file mode 100644
index 0000000..6436be4
--- /dev/null
+++ b/debian/patches/lp2043817-fix-timeout-pacemaker-jammy-01.patch
@@ -0,0 +1,138 @@
1From: Lars Ellenberg <lars.ellenberg@linbit.com>
2Date: Wed, 12 Jan 2022 13:50:35 +0100
3Subject: crm-fence-peer: fix timeout with Pacemaker 2.1: milli seconds vs
4 seconds
5MIME-Version: 1.0
6Content-Type: text/plain; charset="utf-8"
7Content-Transfer-Encoding: 8bit
8
9crmadmin timeout was in milli seconds for <= 2.0.x,
10but became a TIMESPEC with default seconds in >= 2.1.
11
12Up to 2.0.4, atoi() was used, which effectively ignores "trailing garbage",
13so we could get away with always appending "ms".
14But with 2.0.5, it became g_option_context_parse G_OPTION_ARG_INT, which
15"Cannot parse integer value “200ms” for --timeout" :-|
16
17So grep the help message for "timeout.*milliseconds",
18and if not present, append an explicit "ms" unit.
19
20Also tolerate both ": ok" (2.1) and " (ok)" (older)
21when matching the output string of crmadmin -S.
22
23
24Origin: upstream, https://github.com/LINBIT/drbd-utils/commit/8a28be74bc6efa93931c957e54c01abb18b984fe
25Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/drbd-utils/+bug/2043817
26---
27 scripts/crm-fence-peer.9.sh | 24 +++++++++++++++++++++---
28 scripts/crm-fence-peer.sh | 24 +++++++++++++++++++++---
29 2 files changed, 42 insertions(+), 6 deletions(-)
30
31diff --git a/scripts/crm-fence-peer.9.sh b/scripts/crm-fence-peer.9.sh
32index 36590bd..c943bf9 100755
33--- a/scripts/crm-fence-peer.9.sh
34+++ b/scripts/crm-fence-peer.9.sh
35@@ -392,6 +392,20 @@ check_cluster_properties()
36 crm_is_not_false ${stonith_enabled:-} && stonith_enabled=true || stonith_enabled=false
37 }
38
39+setup_crm_timeout_unit_ms()
40+{
41+ # crmadmin timeout was in ms for <= 2.0.x,
42+ # but became a TIMESPEC in >= 2.1.
43+ # Up to 2.0.4, atoi() was used, which effectively ignores "trailing
44+ # garbage", so we could get away with always appending "ms", but with
45+ # 2.0.5, it became g_option_context_parse G_OPTION_ARG_INT, which
46+ # "Cannot parse integer value “200ms” for --timeout" :-|
47+ if crmadmin --help 2>&1 | grep -q -e "--timeout=.*in milliseconds"; then
48+ crm_timeout_unit_ms=""
49+ else
50+ crm_timeout_unit_ms="ms"
51+ fi
52+}
53
54 #
55 # In case this is a two-node cluster (still common with
56@@ -737,6 +751,7 @@ drbd_peer_fencing()
57
58 local startup_fencing stonith_enabled
59 check_cluster_properties
60+ setup_crm_timeout_unit_ms
61
62 if ! $had_constraint_on_entry ; then
63
64@@ -1075,14 +1090,17 @@ _check_peer_node_reachable()
65 # it is obviously reachable.
66 #
67 # Do this only after we have been able to reach a DC above.
68- # Note: crmadmin timeout is in milli-seconds, and defaults to 30000 (30 seconds).
69+ # Note: crmadmin timeout defaults to 30 seconds.
70+ #
71 # Our variable $cibtimeout should be in deci-seconds (see above)
72 # (unless you use a very old version of pacemaker, so don't do that).
73 # Convert deci-seconds to milli-seconds, and double it.
74+ # See also setup_crm_timeout_unit_ms() above.
75+ #
76 if [[ $crmd = "online" ]] ; then
77 local out
78- if out=$( crmadmin -t $(( cibtimeout * 200 )) -S $DRBD_PEER ) \
79- && [[ $out = *"(ok)" ]]; then
80+ if out=$( crmadmin -t $(( cibtimeout * 200 ))$crm_timeout_unit_ms -S $DRBD_PEER ) \
81+ && [[ $out = *@(": ok"|" (ok)") ]]; then
82 peer_state="reachable"
83 return
84 fi
85diff --git a/scripts/crm-fence-peer.sh b/scripts/crm-fence-peer.sh
86index cb5dede..9678673 100755
87--- a/scripts/crm-fence-peer.sh
88+++ b/scripts/crm-fence-peer.sh
89@@ -244,6 +244,20 @@ check_cluster_properties()
90 crm_is_not_false $stonith_enabled && stonith_enabled=true || stonith_enabled=false
91 }
92
93+setup_crm_timeout_unit_ms()
94+{
95+ # crmadmin timeout was in ms for <= 2.0.x,
96+ # but became a TIMESPEC in >= 2.1.
97+ # Up to 2.0.4, atoi() was used, which effectively ignores "trailing
98+ # garbage", so we could get away with always appending "ms", but with
99+ # 2.0.5, it became g_option_context_parse G_OPTION_ARG_INT, which
100+ # "Cannot parse integer value “200ms” for --timeout" :-|
101+ if crmadmin --help 2>&1 | grep -q -e "--timeout=.*in milliseconds"; then
102+ crm_timeout_unit_ms=""
103+ else
104+ crm_timeout_unit_ms="ms"
105+ fi
106+}
107
108 #
109 # In case this is a two-node cluster (still common with
110@@ -426,6 +440,7 @@ drbd_peer_fencing()
111
112 local startup_fencing stonith_enabled
113 check_cluster_properties
114+ setup_crm_timeout_unit_ms
115
116 if [[ -z $have_constraint ]] ; then
117 # try to place it.
118@@ -718,14 +733,17 @@ check_peer_node_reachable()
119 # it is obviously reachable.
120 #
121 # Do this only after we have been able to reach a DC above.
122- # Note: crmadmin timeout is in milli-seconds, and defaults to 30000 (30 seconds).
123+ # Note: crmadmin timeout defaults to 30 seconds.
124+ #
125 # Our variable $cibtimeout should be in deci-seconds (see above)
126 # (unless you use a very old version of pacemaker, so don't do that).
127 # Convert deci-seconds to milli-seconds, and double it.
128+ # See also setup_crm_timeout_unit_ms() above.
129+ #
130 if [[ $crmd = "online" ]] ; then
131 local out
132- if out=$( crmadmin -t $(( cibtimeout * 200 )) -S $DRBD_PEER ) \
133- && [[ $out = *"(ok)" ]]; then
134+ if out=$( crmadmin -t $(( cibtimeout * 200 ))$crm_timeout_unit_ms -S $DRBD_PEER ) \
135+ && [[ $out = *@(": ok"|" (ok)") ]]; then
136 peer_state="reachable"
137 return
138 fi
diff --git a/debian/patches/lp2043817-fix-timeout-pacemaker-jammy-02.patch b/debian/patches/lp2043817-fix-timeout-pacemaker-jammy-02.patch
0new file mode 100644139new file mode 100644
index 0000000..f7a5038
--- /dev/null
+++ b/debian/patches/lp2043817-fix-timeout-pacemaker-jammy-02.patch
@@ -0,0 +1,90 @@
1From: Lars Ellenberg <lars.ellenberg@linbit.com>
2Date: Wed, 12 Jan 2022 13:50:35 +0100
3Subject: crm-fence-peer: fix timeout with Pacemaker 2.0.5: milli seconds vs
4 seconds
5MIME-Version: 1.0
6Content-Type: text/plain; charset="utf-8"
7Content-Transfer-Encoding: 8bit
8
9Addendum to 8a28be74bc6efa93931c957e54c01abb18b984fe
10Commit message of the above cited here:
11
12> crmadmin timeout was in milli seconds for <= 2.0.x,
13> but became a TIMESPEC with default seconds in >= 2.1.
14>
15> Up to 2.0.4, atoi() was used, which effectively ignores "trailing garbage",
16> so we could get away with always appending "ms".
17> But with 2.0.5, it became g_option_context_parse G_OPTION_ARG_INT, which
18> "Cannot parse integer value “200ms” for --timeout" :-|
19>
20> So grep the help message for "timeout.*milliseconds",
21> and if not present, append an explicit "ms" unit.
22
23And this is where I got it wrong :-(
24somewhere later they re-organised the help text
25so now I would need to parse --help-all.
26
27Instead try to actually call "crmadmin -t 100ms --version".
28If that works, it apparently understands (or ignores)
29the "ms" unit.
30
31
32Origin: upstream, https://github.com/LINBIT/drbd-utils/commit/3eec04bc65b39b04be21d2689568892e7788abb3
33Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/drbd-utils/+bug/2043817
34---
35 scripts/crm-fence-peer.9.sh | 14 +++++++++++---
36 scripts/crm-fence-peer.sh | 14 +++++++++++---
37 2 files changed, 22 insertions(+), 6 deletions(-)
38
39diff --git a/scripts/crm-fence-peer.9.sh b/scripts/crm-fence-peer.9.sh
40index c943bf9..fc8d2bc 100755
41--- a/scripts/crm-fence-peer.9.sh
42+++ b/scripts/crm-fence-peer.9.sh
43@@ -400,10 +400,18 @@ setup_crm_timeout_unit_ms()
44 # garbage", so we could get away with always appending "ms", but with
45 # 2.0.5, it became g_option_context_parse G_OPTION_ARG_INT, which
46 # "Cannot parse integer value “200ms” for --timeout" :-|
47- if crmadmin --help 2>&1 | grep -q -e "--timeout=.*in milliseconds"; then
48- crm_timeout_unit_ms=""
49- else
50+ # Can not parse the help text reliably, because they changed content
51+ # and organisation of the help text between 2.0.4 and 2.0.5.
52+ # Just try using ms unit, and see if it fails.
53+ if crmadmin -t 100ms --version &> /dev/null; then
54+ # this is either a recent version that actually understands ms
55+ # as part of the TIMESPEC, or a version that still uses atoi().
56 crm_timeout_unit_ms="ms"
57+ else
58+ # this one likely failed with
59+ # crmadmin: Cannot parse integer value “100ms” for -t
60+ # (>= 2.0.5, < 2.1)
61+ crm_timeout_unit_ms=""
62 fi
63 }
64
65diff --git a/scripts/crm-fence-peer.sh b/scripts/crm-fence-peer.sh
66index 9678673..b0e4e0f 100755
67--- a/scripts/crm-fence-peer.sh
68+++ b/scripts/crm-fence-peer.sh
69@@ -252,10 +252,18 @@ setup_crm_timeout_unit_ms()
70 # garbage", so we could get away with always appending "ms", but with
71 # 2.0.5, it became g_option_context_parse G_OPTION_ARG_INT, which
72 # "Cannot parse integer value “200ms” for --timeout" :-|
73- if crmadmin --help 2>&1 | grep -q -e "--timeout=.*in milliseconds"; then
74- crm_timeout_unit_ms=""
75- else
76+ # Can not parse the help text reliably, because they changed content
77+ # and organisation of the help text between 2.0.4 and 2.0.5.
78+ # Just try using ms unit, and see if it fails.
79+ if crmadmin -t 100ms --version &> /dev/null; then
80+ # this is either a recent version that actually understands ms
81+ # as part of the TIMESPEC, or a version that still uses atoi().
82 crm_timeout_unit_ms="ms"
83+ else
84+ # this one likely failed with
85+ # crmadmin: Cannot parse integer value “100ms” for -t
86+ # (>= 2.0.5, < 2.1)
87+ crm_timeout_unit_ms=""
88 fi
89 }
90
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 @@
60006-initscript-add-start-runlevels.patch60006-initscript-add-start-runlevels.patch
70007-drbdmon-buildflags.patch70007-drbdmon-buildflags.patch
80008-drbdmon-pass-LDFLAGS-to-g.patch80008-drbdmon-pass-LDFLAGS-to-g.patch
9lp2043817-fix-timeout-pacemaker-jammy-01.patch
10lp2043817-fix-timeout-pacemaker-jammy-02.patch

Subscribers

People subscribed via source and target branches