Merge ~rafaeldtinoco/ubuntu/+source/resource-agents:lp1895348-groovy into ubuntu/+source/resource-agents:ubuntu/groovy-devel

[Rafael David Tinoco]

BUG: https://bugs.launchpad.net/ubuntu/+source/resource-agents/+bug/1895348
BUILDS: https://launchpad.net/~rafaeldtinoco/+archive/ubuntu/builds

[Bryce Harrington (bryce)]

  - [d22700fc] azure-lb: Don't redirect nc listener output to pidfile

LGTM +1, this looks like a cleaner approach for capturing the pid, than from the eval string.

  - [3b0ffc59] build: fix distcheck issue introduced by including
               READMEs in heartbeat directory

LGTM +1, trivial

  - [73551ac0] galera: Fix automatic recovery when a cluster was not
               gracefully stopped

LGTM +1, I had to think the refactoring logic through but it does make sense for the described behavior. I did not reproduce the problem myself, but can see now how it would occur.

review: Approve

[Rafael David Tinoco (rafaeldtinoco)]

Thanks for reviewing this @bryce!! That was quick!

$ git ubuntu tag --upload

$ git describe
upload/1%4.6.1-1ubuntu2

$ git push pkg upload/1%4.6.1-1ubuntu2
Counting objects: 14, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (14/14), done.
Writing objects: 100% (14/14), 4.31 KiB | 441.00 KiB/s, done.
Total 14 (delta 6), reused 0 (delta 0)
To ssh://git.launchpad.net/ubuntu/+source/resource-agents
 * [new tag] upload/1%4.6.1-1ubuntu2 -> upload/1%4.6.1-1ubuntu2

$ dput ubuntu resource-agents_4.6.1-1ubuntu2_source.changes
Checking signature on .changes
gpg: /home/rafaeldtinoco/work/sources/ubuntu/resource-agents_4.6.1-1ubuntu2_source.changes: Valid signature from A93E0E0AD83C0D0F
Checking signature on .dsc
gpg: /home/rafaeldtinoco/work/sources/ubuntu/resource-agents_4.6.1-1ubuntu2.dsc: Valid signature from A93E0E0AD83C0D0F
Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading resource-agents_4.6.1-1ubuntu2.dsc: done.
  Uploading resource-agents_4.6.1-1ubuntu2.debian.tar.xz: done.
  Uploading resource-agents_4.6.1-1ubuntu2_source.buildinfo: done.
  Uploading resource-agents_4.6.1-1ubuntu2_source.changes: done.
Successfully uploaded packages.

review: Approve

diff --git a/debian/changelog b/debian/changelog
index 440d124..21a29d6 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,14 @@
+resource-agents (1:4.6.1-1ubuntu2) groovy; urgency=medium
+
+  * d/p/u/lp1895348-*: Post Release Fixes (LP: #1895348):
+    - [d22700fc] azure-lb: Don't redirect nc listener output to pidfile
+    - [3b0ffc59] build: fix distcheck issue introduced by including
+                 READMEs in heartbeat directory
+    - [73551ac0] galera: Fix automatic recovery when a cluster was not
+                 gracefully stopped
+
+ -- Rafael David Tinoco <[e-mail]>  Fri, 11 Sep 2020 19:30:14 +0000
+
 resource-agents (1:4.6.1-1ubuntu1) groovy; urgency=medium
 
   * Merge with Debian unstable (LP: #1886603). Remaining changes:
diff --git a/debian/patches/series b/debian/patches/series
index c54dc17..3b9ba6c 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -7,3 +7,7 @@ gitignore.patch
 reproducible.patch
 var-run.patch
 bashism.patch
+# ubuntu: after release fixes
+ubuntu/lp1895348-azure-lb-Don-t-redirect-nc-listener.patch
+ubuntu/lp1895348-galera-Fix-automatic-recovery.patch
+ubuntu/lp1895348-build-fix-distcheck-issue.patch
diff --git a/debian/patches/ubuntu/lp1895348-azure-lb-Don-t-redirect-nc-listener.patch b/debian/patches/ubuntu/lp1895348-azure-lb-Don-t-redirect-nc-listener.patch
new file mode 100644
index 0000000..1e2ff23
--- /dev/null
+++ b/debian/patches/ubuntu/lp1895348-azure-lb-Don-t-redirect-nc-listener.patch
@@ -0,0 +1,57 @@
+From d22700fc5d5098c683b465ea0fede43803fd4d6b Mon Sep 17 00:00:00 2001
+From: Reid wahl <[e-mail]>
+Date: Tue, 7 Jul 2020 02:18:09 -0700
+Subject: [PATCH] azure-lb: Don't redirect nc listener output to pidfile
+
+The `lb_start()` function spawns an `nc` listener background process
+and echoes the resulting pid to `$pidfile`. Due to a bug in the
+redirection, all future data received by the `nc` process is also
+appended to `$pidfile`.
+
+If binary data is received later and appended to `$pidfile`, the
+monitor operation fails when `grep` searches the now-binary file.
+
+```
+line 97: kill: Binary: arguments must be process or job IDs ]
+line 97: kill: file: arguments must be process or job IDs ]
+line 97: kill: /var/run/nc_PF2_02.pid: arguments must be process or job
+    IDs ]
+line 97: kill: matches: arguments must be process or job IDs ]
+```
+
+Then the start operation fails during recovery. `lb_start()` spawns a
+new `nc` process, but the old process is still running and using the
+configured port.
+
+```
+nc_PF2_02_start_0:777:stderr [ Ncat: bind to :::62502: Address
+    already in use. QUITTING. ]
+```
+
+This patch fixes the issue by removing the `nc &` command from the
+section whose output gets redirected to `$pidfile`. Now, only the `nc`
+PID is echoed to `$pidfile`.
+
+Resolves: RHBZ#1850778
+Resolves: RHBZ#1850779
+---
+ heartbeat/azure-lb | 3 ++-
+ 1 file changed, 2 insertions(+), 1 deletion(-)
+
+diff --git a/heartbeat/azure-lb b/heartbeat/azure-lb
+index 05c13451..05755d77 100755
+--- a/heartbeat/azure-lb
++++ b/heartbeat/azure-lb
+@@ -113,7 +113,8 @@ lb_start() {
+ 	if ! lb_monitor; then
+ 		ocf_log debug "Starting $process: $cmd"
+ 		# Execute the command as created above
+-		eval "$cmd & echo \$!" > $pidfile
++		$cmd &
++		echo $! > $pidfile
+ 		if lb_monitor; then
+ 			ocf_log debug "$process: $cmd started successfully, calling monitor"
+ 			lb_monitor
+-- 
+2.27.0
+
diff --git a/debian/patches/ubuntu/lp1895348-build-fix-distcheck-issue.patch b/debian/patches/ubuntu/lp1895348-build-fix-distcheck-issue.patch
new file mode 100644
index 0000000..992f940
--- /dev/null
+++ b/debian/patches/ubuntu/lp1895348-build-fix-distcheck-issue.patch
@@ -0,0 +1,17 @@
+From 3b0ffc598ea75d8dad2c7efe52c73437e5eadb17 Mon Sep 17 00:00:00 2001
+From: Oyvind Albrigtsen <[e-mail]>
+Date: Tue, 25 Aug 2020 13:03:09 +0200
+Subject: [PATCH] build: fix distcheck issue introduced by including READMEs in
+ heartbeat directory
+
+--- resource-agents-4.6.1.orig/heartbeat/Makefile.am
++++ resource-agents-4.6.1/heartbeat/Makefile.am
+@@ -21,7 +21,7 @@ MAINTAINERCLEANFILES = Makefile.in
+ 
+ EXTRA_DIST		= $(ocf_SCRIPTS) $(ocfcommon_DATA) \
+ 			  $(common_DATA) $(hb_DATA) $(dtd_DATA) \
+-			  README
++			  README README.galera
+ 
+ AM_CPPFLAGS		= -I$(top_srcdir)/include -I$(top_srcdir)/linux-ha
+ 
diff --git a/debian/patches/ubuntu/lp1895348-galera-Fix-automatic-recovery.patch b/debian/patches/ubuntu/lp1895348-galera-Fix-automatic-recovery.patch
new file mode 100644
index 0000000..fb55f06
--- /dev/null
+++ b/debian/patches/ubuntu/lp1895348-galera-Fix-automatic-recovery.patch
@@ -0,0 +1,62 @@
+From 73551ac029c7480cf710c392ad2d500ef2a7b07a Mon Sep 17 00:00:00 2001
+From: Petr Pavlu <[e-mail]>
+Date: Wed, 26 Aug 2020 13:59:16 +0200
+Subject: [PATCH] galera: Fix automatic recovery when a cluster was not
+ gracefully stopped
+
+When selecting a bootstrap node, the Galera resource agent primarily
+depends on the safe_to_bootstrap flag in grastate.dat. If none of the
+nodes have this flag set to 1 then functions detect_last_commit() +
+detect_first_master() provide a recovery logic to select the bootstrap
+node based on each node's last commit, as obtained from grastate.dat or
+'mysqld_safe --wsrep-recover'.
+
+Fix 65f35e9172407e64ded90f29ea8fc0dfca9643e3 introduced a problem for
+this recovery logic. If a whole cluster is not gracefully stopped,
+grastate.dat on every node contains "safe_to_bootstrap: 0" and
+"seqno: -1". Function detect_safe_to_bootstrap() then considers each
+node with this seqno as not suitable for bootstraping and clears the
+safe_to_bootstrap attribute. Nonetheless, functions detect_last_commit()
++ detect_first_master() successfully find a bootstrap node, relying on
+the recovery logic. However, when the promote operation is invoked,
+function galera_promote() runs check '"$(get_safe_to_bootstrap)" = "0"'
+which fails and prevents the code from writing "safe_to_bootstrap: 1"
+into grastate.dat of the selected node to mark it as a bootstrap node.
+The end result is that Galera refuses to be started on this node and
+therefore the whole cluster remains down.
+
+The patch fixes the problem by adjusting detect_safe_to_bootstrap() to
+accept the combination of "safe_to_bootstrap: 0" and "seqno: -1" and
+allow a node with this state to potentially become a bootstrap node.
+---
+ heartbeat/galera | 11 ++++++++---
+ 1 file changed, 8 insertions(+), 3 deletions(-)
+
+diff --git a/heartbeat/galera b/heartbeat/galera
+index 4a313e24..74f11d8c 100755
+--- a/heartbeat/galera
++++ b/heartbeat/galera
+@@ -604,12 +604,17 @@ detect_safe_to_bootstrap()
+         seqno=$(sed -n 's/^seqno:\s*\(.*\)$/\1/p' < ${OCF_RESKEY_datadir}/grastate.dat)
+     fi
+ 
+-    if [ -z "$uuid" ] || [ -z "$seqno" ] || \
+-       [ "$uuid" = "00000000-0000-0000-0000-000000000000" ] || \
+-       [ "$seqno" = "-1" ]; then
++    if [ -z "$uuid" ] || \
++       [ "$uuid" = "00000000-0000-0000-0000-000000000000" ]; then
+         clear_safe_to_bootstrap
+         return
+     fi
++    if [ "$safe_to_bootstrap" = "1" ]; then
++        if [ -z "$seqno" ] || [ "$seqno" = "-1" ]; then
++            clear_safe_to_bootstrap
++            return
++        fi
++    fi
+ 
+     if [ "$safe_to_bootstrap" = "1" ] || [ "$safe_to_bootstrap" = "0" ]; then
+         set_safe_to_bootstrap $safe_to_bootstrap
+-- 
+2.27.0
+