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

diff --git a/ctdb/config/events/legacy/50.samba.script b/ctdb/config/events/legacy/50.samba.script
index 44ae0a4e63b91b73761c59627943d90ec2eba2c1..84600e25024ae7b19c3b502619d95094c7981c23 100755 (executable)
--- 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 55df6da807ed23667de92e620756ef492b7e317a..d72855f2291e6350581233e4008736f4c43a57f4 100755 (executable)
--- 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 da89db2b81f039005810bc4d30634535144b8478..45976cd655ac3a99644b0132de40005def699521 100644 (file)
--- 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 0f8216ff8b377179a78354041b48a6eccab8d31a..faa0f347885c30b2c3161e57e85c9196f9a78726 100755 (executable)
--- 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