Bug#1040415: bullseye-pu: package pacemaker/2.1.5-1+deb12u1
Control: tag -1 - confirmed
Jonathan Wiltshire <jmw@debian.org> writes:
> On Wed, Jul 05, 2023 at 07:14:09PM +0200, Ferenc Wágner wrote:
>
>> Shortly after the release of bookworm we got a report that Pacemaker
>> regressed in certain migration scenarios when compared to the bullseye
>> version. Upstream identified the cause (a bug already fixed in 2.1.6),
>> and after backporting the fix the submitter acknowledged that they can't
>> reproduce the bug anymore with the proposed packages.
>> https://bugs.clusterlabs.org/show_bug.cgi?id=5521
>> Pacemaker package bug opened after discussion on the mailing list:
>> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1040165
>
> Please go ahead, and bear in mind the upload window closes next weekend.
Thanks, Jonathan! Does it mean that I have to upload before 15th of July?
On the other hand, meanwhile upstream notified me that to fully fix this
bug I need to backport one more patch, which in turn required including
a third one. So the debdiff grew a little, please reconfirm the upload:
$ debdiff pacemaker_2.1.5-1.dsc pacemaker_2.1.5-1+deb12u1.dsc
dpkg-source: warning: extracting unsigned source package (/home/wferi/ha/pacemaker/pacemaker_2.1.5-1.dsc)
diff -Nru pacemaker-2.1.5/debian/changelog pacemaker-2.1.5/debian/changelog
--- pacemaker-2.1.5/debian/changelog 2023-01-22 16:38:34.000000000 +0100
+++ pacemaker-2.1.5/debian/changelog 2023-07-09 23:10:45.000000000 +0200
@@ -1,3 +1,17 @@
+pacemaker (2.1.5-1+deb12u1) bookworm; urgency=medium
+
+ * [0c22be8] New patches fixing migration regression.
+ Backport of https://github.com/ClusterLabs/pacemaker/pull/3020/ to
+ Pacemaker 2.1.5 (without the CTS changes, which we don't ship):
+ 5754a2af9 Refactor: scheduler: improve xpath efficiency when unpacking
+ 3f6f524f1 Low: scheduler: unknown_on_node() should ignore pending actions
+ ad9fd9548 Fix: scheduler: handle cleaned migrate_from history correctly
+ The starting refactor is required by the other two patches, but the
+ third patch still needed backporting.
+ Thanks to Ken Gaillot (Closes: #1040165)
+
+ -- Ferenc Wágner <wferi@debian.org> Sun, 09 Jul 2023 23:10:45 +0200
+
pacemaker (2.1.5-1) unstable; urgency=medium
* [5792d59] Work around lazy loading of GitHub release pages in watch file
diff -Nru pacemaker-2.1.5/debian/gbp.conf pacemaker-2.1.5/debian/gbp.conf
--- pacemaker-2.1.5/debian/gbp.conf 2023-01-22 13:10:39.000000000 +0100
+++ pacemaker-2.1.5/debian/gbp.conf 2023-07-09 22:33:06.000000000 +0200
@@ -1,5 +1,5 @@
[DEFAULT]
-debian-branch = debian/master
+debian-branch = debian/bookworm
upstream-branch = upstream/latest
[import-orig]
diff -Nru pacemaker-2.1.5/debian/patches/Fix-scheduler-handle-cleaned-migrate_from-history-correct.patch pacemaker-2.1.5/debian/patches/Fix-scheduler-handle-cleaned-migrate_from-history-correct.patch
--- pacemaker-2.1.5/debian/patches/Fix-scheduler-handle-cleaned-migrate_from-history-correct.patch 1970-01-01 01:00:00.000000000 +0100
+++ pacemaker-2.1.5/debian/patches/Fix-scheduler-handle-cleaned-migrate_from-history-correct.patch 2023-07-09 23:07:30.000000000 +0200
@@ -0,0 +1,30 @@
+From: Ken Gaillot <kgaillot@redhat.com>
+Date: Wed, 1 Feb 2023 17:12:13 -0600
+Subject: Fix: scheduler: handle cleaned migrate_from history correctly
+
+Fixes T623
+---
+ lib/pengine/unpack.c | 10 ++++++++++
+ 1 file changed, 10 insertions(+)
+
+diff --git a/lib/pengine/unpack.c b/lib/pengine/unpack.c
+index e9fcae1..99a2dc4 100644
+--- a/lib/pengine/unpack.c
++++ b/lib/pengine/unpack.c
+@@ -2937,6 +2937,16 @@ unpack_migrate_to_success(pe_resource_t *rsc, pe_node_t *node, xmlNode *xml_op,
+ }
+
+ } else { // Pending, or complete but erased
++
++ /* If there is no history at all for the resource on an online target, then
++ * it was likely cleaned. Just return, and we'll schedule a probe. Once we
++ * have the probe result, it will be reflected in target_newer_state.
++ */
++ if ((target_node != NULL) && target_node->details->online
++ && unknown_on_node(rsc, target)) {
++ return;
++ }
++
+ /* If the resource has newer state on the target, this migrate_to no
+ * longer matters for the target.
+ */
diff -Nru pacemaker-2.1.5/debian/patches/Low-scheduler-unknown_on_node-should-ignore-pending-actio.patch pacemaker-2.1.5/debian/patches/Low-scheduler-unknown_on_node-should-ignore-pending-actio.patch
--- pacemaker-2.1.5/debian/patches/Low-scheduler-unknown_on_node-should-ignore-pending-actio.patch 1970-01-01 01:00:00.000000000 +0100
+++ pacemaker-2.1.5/debian/patches/Low-scheduler-unknown_on_node-should-ignore-pending-actio.patch 2023-07-09 23:07:30.000000000 +0200
@@ -0,0 +1,80 @@
+From: Ken Gaillot <kgaillot@redhat.com>
+Date: Thu, 2 Feb 2023 10:25:53 -0600
+Subject: Low: scheduler: unknown_on_node() should ignore pending actions
+
+Previously, unknown_on_node() looked for any lrm_rsc_op at all to decide
+whether a resource is known on a node. However if the only action is pending,
+the resource is not yet known.
+
+Also drop a redundant argument and add a doxygen block. (The rsc argument is
+not const due to a getDocPtr() call in the chain, as well as libxml2 calls that
+are likely const in practice but aren't marked as such.)
+---
+ lib/pengine/unpack.c | 37 +++++++++++++++++++++++++------------
+ 1 file changed, 25 insertions(+), 12 deletions(-)
+
+diff --git a/lib/pengine/unpack.c b/lib/pengine/unpack.c
+index 41cb980..e9fcae1 100644
+--- a/lib/pengine/unpack.c
++++ b/lib/pengine/unpack.c
+@@ -2654,19 +2654,32 @@ find_lrm_resource(const char *rsc_id, const char *node_name,
+ return xml;
+ }
+
++/*!
++ * \internal
++ * \brief Check whether a resource has no completed action history on a node
++ *
++ * \param[in,out] rsc Resource to check
++ * \param[in] node_name Node to check
++ *
++ * \return true if \p rsc_id is unknown on \p node_name, otherwise false
++ */
+ static bool
+-unknown_on_node(const char *rsc_id, const char *node_name,
+- pe_working_set_t *data_set)
++unknown_on_node(pe_resource_t *rsc, const char *node_name)
+ {
+- xmlNode *lrm_resource = NULL;
+-
+- lrm_resource = find_lrm_resource(rsc_id, node_name, data_set);
++ bool result = false;
++ xmlXPathObjectPtr search;
++ GString *xpath = g_string_sized_new(256);
+
+- /* If the resource has no lrm_rsc_op history on the node, that means its
+- * state is unknown there.
+- */
+- return (lrm_resource == NULL
+- || first_named_child(lrm_resource, XML_LRM_TAG_RSC_OP) == NULL);
++ pcmk__g_strcat(xpath,
++ XPATH_NODE_STATE "[@" XML_ATTR_UNAME "='", node_name, "']"
++ SUB_XPATH_LRM_RESOURCE "[@" XML_ATTR_ID "='", rsc->id, "']"
++ SUB_XPATH_LRM_RSC_OP "[@" XML_LRM_ATTR_RC "!='193']",
++ NULL);
++ search = xpath_search(rsc->cluster->input, (const char *) xpath->str);
++ result = (numXpathResults(search) == 0);
++ freeXpathObject(search);
++ g_string_free(xpath, TRUE);
++ return result;
+ }
+
+ /*!
+@@ -2979,7 +2992,7 @@ unpack_migrate_to_failure(pe_resource_t *rsc, pe_node_t *node, xmlNode *xml_op,
+ * Don't just consider it running there. We will get back here anyway in
+ * case the probe detects it's running there.
+ */
+- !unknown_on_node(rsc->id, target, data_set)
++ !unknown_on_node(rsc, target)
+ /* If the resource has newer state on the target after the migration
+ * events, this migrate_to no longer matters for the target.
+ */
+@@ -3031,7 +3044,7 @@ unpack_migrate_from_failure(pe_resource_t *rsc, pe_node_t *node,
+ * Don't just consider it running there. We will get back here anyway in
+ * case the probe detects it's running there.
+ */
+- !unknown_on_node(rsc->id, source, data_set)
++ !unknown_on_node(rsc, source)
+ /* If the resource has newer state on the source after the migration
+ * events, this migrate_from no longer matters for the source.
+ */
diff -Nru pacemaker-2.1.5/debian/patches/Refactor-scheduler-improve-xpath-efficiency-when-unpackin.patch pacemaker-2.1.5/debian/patches/Refactor-scheduler-improve-xpath-efficiency-when-unpackin.patch
--- pacemaker-2.1.5/debian/patches/Refactor-scheduler-improve-xpath-efficiency-when-unpackin.patch 1970-01-01 01:00:00.000000000 +0100
+++ pacemaker-2.1.5/debian/patches/Refactor-scheduler-improve-xpath-efficiency-when-unpackin.patch 2023-07-09 23:07:30.000000000 +0200
@@ -0,0 +1,55 @@
+From: Ken Gaillot <kgaillot@redhat.com>
+Date: Thu, 2 Feb 2023 09:42:01 -0600
+Subject: Refactor: scheduler: improve xpath efficiency when unpacking
+
+Using "//" means that every child must be searched recursively. If we know the
+exact path, we should explicitly specify it.
+---
+ lib/pengine/unpack.c | 20 ++++++++++++--------
+ 1 file changed, 12 insertions(+), 8 deletions(-)
+
+diff --git a/lib/pengine/unpack.c b/lib/pengine/unpack.c
+index 5fcba3b..41cb980 100644
+--- a/lib/pengine/unpack.c
++++ b/lib/pengine/unpack.c
+@@ -2577,6 +2577,13 @@ set_node_score(gpointer key, gpointer value, gpointer user_data)
+ node->weight = *score;
+ }
+
++#define XPATH_NODE_STATE "/" XML_TAG_CIB "/" XML_CIB_TAG_STATUS \
++ "/" XML_CIB_TAG_STATE
++#define SUB_XPATH_LRM_RESOURCE "/" XML_CIB_TAG_LRM \
++ "/" XML_LRM_TAG_RESOURCES \
++ "/" XML_LRM_TAG_RESOURCE
++#define SUB_XPATH_LRM_RSC_OP "/" XML_LRM_TAG_RSC_OP
++
+ static xmlNode *
+ find_lrm_op(const char *resource, const char *op, const char *node, const char *source,
+ int target_rc, pe_working_set_t *data_set)
+@@ -2589,10 +2596,9 @@ find_lrm_op(const char *resource, const char *op, const char *node, const char *
+
+ xpath = g_string_sized_new(256);
+ pcmk__g_strcat(xpath,
+- "//" XML_CIB_TAG_STATE "[@" XML_ATTR_UNAME "='", node, "']"
+- "//" XML_LRM_TAG_RESOURCE
+- "[@" XML_ATTR_ID "='", resource, "']"
+- "/" XML_LRM_TAG_RSC_OP "[@" XML_LRM_ATTR_TASK "='", op, "'",
++ XPATH_NODE_STATE "[@" XML_ATTR_UNAME "='", node, "']"
++ SUB_XPATH_LRM_RESOURCE "[@" XML_ATTR_ID "='", resource, "']"
++ SUB_XPATH_LRM_RSC_OP "[@" XML_LRM_ATTR_TASK "='", op, "'",
+ NULL);
+
+ /* Need to check against transition_magic too? */
+@@ -2637,10 +2643,8 @@ find_lrm_resource(const char *rsc_id, const char *node_name,
+
+ xpath = g_string_sized_new(256);
+ pcmk__g_strcat(xpath,
+- "//" XML_CIB_TAG_STATE
+- "[@" XML_ATTR_UNAME "='", node_name, "']"
+- "//" XML_LRM_TAG_RESOURCE
+- "[@" XML_ATTR_ID "='", rsc_id, "']",
++ XPATH_NODE_STATE "[@" XML_ATTR_UNAME "='", node_name, "']"
++ SUB_XPATH_LRM_RESOURCE "[@" XML_ATTR_ID "='", rsc_id, "']",
+ NULL);
+
+ xml = get_xpath_object((const char *) xpath->str, data_set->input,
diff -Nru pacemaker-2.1.5/debian/patches/series pacemaker-2.1.5/debian/patches/series
--- pacemaker-2.1.5/debian/patches/series 2023-01-22 13:31:42.000000000 +0100
+++ pacemaker-2.1.5/debian/patches/series 2023-07-09 23:07:30.000000000 +0200
@@ -5,3 +5,6 @@
Shipping-the-CTS-is-not-useful.patch
Always-run-Inkscape-under-the-C.UTF-8-locale.patch
Fix-typos-resouce-resource.patch
+Refactor-scheduler-improve-xpath-efficiency-when-unpackin.patch
+Low-scheduler-unknown_on_node-should-ignore-pending-actio.patch
+Fix-scheduler-handle-cleaned-migrate_from-history-correct.patch
diff -Nru pacemaker-2.1.5/debian/salsa-ci.yml pacemaker-2.1.5/debian/salsa-ci.yml
--- pacemaker-2.1.5/debian/salsa-ci.yml 2023-01-22 13:10:39.000000000 +0100
+++ pacemaker-2.1.5/debian/salsa-ci.yml 2023-07-09 22:33:06.000000000 +0200
@@ -5,6 +5,7 @@
variables:
SALSA_CI_REPROTEST_ENABLE_DIFFOSCOPE: 1
+ RELEASE: bookworm
autopkgtest:
extends: .test-autopkgtest
--
Thanks,
Feri.
Reply to: