From 238056e5aadb597e3f6165757990a88952644866 Mon Sep 17 00:00:00 2001
From: Martin Schwenke <mschwenke@ddn.com>
Date: Mon, 13 Feb 2023 14:59:18 +1100
Subject: [PATCH] ctdb-scripts: Avoid using testparm to process its own output

When testparm processes the output of "testparm -v" (which includes
default values) it appears to do global checks (or some other sort of
initialisation logic) for all specified values.  This includes a DNS
lookup for the node's hostname, as a side-effect of a libldap
ldap_set_option() call when processing "ldap debug level".  If DNS
servers are down then this can induce timeouts, possibly resulting in
monitor timeouts.

Avoid this by using sed to extract configuration values from the
testparm cache file.

This is already shown to work when retrieving share paths, where
testparm is basically used as cat.  Update the sed pattern to avoid
matching empty values on the right-hand side of the equals ('=') -
this avoids the default empty path value (and "smb ports" never has an
empty value).

Corresponding test changes:

* 50.samba.monitor.111.sh no longer expects a failure from being
  unable to set smb ports, since testparm is no longer used in that
  code path.

* smb ports needs to be set in fake smb.conf so it is in the default
  output and can be extracted using sed.

* Although testparm --parameter-name is no longer used in
  50.samba.script, update the stub implementation (in case it is ever
  used again) to extract from fake smb.conf, since "smb ports" is now
  set there.  The change from $parameter to $param allows a long line
  to stay below 80 columns.

Signed-off-by: Martin Schwenke <mschwenke@ddn.com>
Reviewed-by: Volker Lendecke <vl@samba.org>

Autobuild-User(master): Volker Lendecke <vl@samba.org>
Autobuild-Date(master): Tue Feb 14 08:43:53 UTC 2023 on atb-devel-224
---
 ctdb/config/events/legacy/50.samba.script     | 15 +++++++-----
 .../UNIT/eventscripts/50.samba.monitor.111.sh |  4 +---
 .../UNIT/eventscripts/etc/samba/smb.conf      |  1 +
 ctdb/tests/UNIT/eventscripts/stubs/testparm   | 23 +++++++------------
 4 files changed, 19 insertions(+), 24 deletions(-)

diff --git a/ctdb/config/events/legacy/50.samba.script b/ctdb/config/events/legacy/50.samba.script
index 44ae0a4e63b..84600e25024 100755
--- a/ctdb/config/events/legacy/50.samba.script
+++ b/ctdb/config/events/legacy/50.samba.script
@@ -109,21 +109,24 @@ testparm_background_update()
 	testparm_foreground_update "$_timeout" >/dev/null 2>&1 </dev/null &
 }
 
-testparm_cat()
+testparm_get ()
 {
-	testparm -s "$smbconf_cache" "$@" 2>/dev/null
+	_param="$1"
+
+	sed -n \
+	    -e "s|^[[:space:]]*${_param}[[:space:]]*=[[:space:]]\(..*\)|\1|p" \
+	    "$smbconf_cache"
+
 }
 
 list_samba_shares()
 {
-	testparm_cat |
-		sed -n -e 's@^[[:space:]]*path[[:space:]]*=[[:space:]]@@p' |
-		sed -e 's/"//g'
+	testparm_get "path" | sed -e 's/"//g'
 }
 
 list_samba_ports()
 {
-	testparm_cat --parameter-name="smb ports"
+	testparm_get "smb ports"
 }
 
 ###########################
diff --git a/ctdb/tests/UNIT/eventscripts/50.samba.monitor.111.sh b/ctdb/tests/UNIT/eventscripts/50.samba.monitor.111.sh
index 55df6da807e..d72855f2291 100755
--- a/ctdb/tests/UNIT/eventscripts/50.samba.monitor.111.sh
+++ b/ctdb/tests/UNIT/eventscripts/50.samba.monitor.111.sh
@@ -10,7 +10,7 @@ ok_null
 simple_test
 
 export FAKE_TESTPARM_FAIL="yes"
-required_result 1 <<EOF
+ok <<EOF
 WARNING: smb.conf cache update failed - using old cache file
 Load smb config files from ${CTDB_SYS_ETCDIR}/samba/smb.conf
 rlimit_max: increasing rlimit_max (2048) to minimum Windows limit (16384)
@@ -19,7 +19,5 @@ Processing section "[share2]"
 Processing section "[share3]"
 Loaded services file OK.
 WARNING: 'workgroup' and 'netbios name' must differ.
-
-Failed to set smb ports
 EOF
 simple_test
diff --git a/ctdb/tests/UNIT/eventscripts/etc/samba/smb.conf b/ctdb/tests/UNIT/eventscripts/etc/samba/smb.conf
index da89db2b81f..45976cd655a 100644
--- a/ctdb/tests/UNIT/eventscripts/etc/samba/smb.conf
+++ b/ctdb/tests/UNIT/eventscripts/etc/samba/smb.conf
@@ -40,3 +40,4 @@
 	map readonly = no
 	ea support = yes
 	dmapi support = no
+	smb ports = 445 139
diff --git a/ctdb/tests/UNIT/eventscripts/stubs/testparm b/ctdb/tests/UNIT/eventscripts/stubs/testparm
index 0f8216ff8b3..faa0f347885 100755
--- a/ctdb/tests/UNIT/eventscripts/stubs/testparm
+++ b/ctdb/tests/UNIT/eventscripts/stubs/testparm
@@ -44,29 +44,22 @@ fi
 # the global one, unless some other file is specified.
 
 file=""
-parameter=""
+param=""
 for i; do
 	case "$i" in
-	--parameter-name=*) parameter="${i#--parameter-name=}" ;;
+	--parameter-name=*) param="${i#--parameter-name=}" ;;
 	-*) : ;;
 	*) file="$i" ;;
 	esac
 done
 
-# Just hard-code parameter requests for now.  Later on they could be
-# parsed out of the file.
-case "$parameter" in
-security)
-	echo "ADS"
+# Parse out parameter request
+if [ -n "$param" ]; then
+	sed -n \
+	    -e "s|^[[:space:]]*${param}[[:space:]]*=[[:space:]]\(..*\)|\1|p" \
+	    "${file:-"${CTDB_SYS_ETCDIR}/samba/smb.conf"}"
 	exit 0
-	;;
-smb*ports)
-	echo "445 139"
-	exit 0
-	;;
-?*) not_implemented "--parameter-name=$parameter" ;;
-	# Fall through if $parameter not set
-esac
+fi
 
 if [ -n "$file" ]; then
 	# This should include the shares, since this is used when the