gpinitsystem: fix the defects the Behave suite exposes

gpMgmt/bin/gpinitsystem

@@ -313,7 +313,7 @@ CHK_PARAMS () {
 		fi
 
 		if [ x"$ETCD_HOST_CONFIG" = x"" ] || [ x"$CLUSTER_BOOT_MODE" = x"DEMO" ]; then
-			LOG_MSG "[WARN]:-No ETCD cluster host config provided, use default configuration."
+			LOG_MSG "[INFO]:-No ETCD cluster host config provided, use default configuration."
 		else
 			ETCD_HOST_MACHINE_LIST=(`$CAT $ETCD_HOST_CONFIG`)
 			ETCD_HOST_CONFIG_NUM=${#ETCD_HOST_MACHINE_LIST[@]}
@@ -323,7 +323,7 @@ CHK_PARAMS () {
 		fi
 
 		if [ x"$FTS_HOST_CONFIG" = x"" ] || [ x"$CLUSTER_BOOT_MODE" = x"DEMO" ]; then
-			LOG_MSG "[WARN]:-No FTS cluster host config provided, use default configuration."
+			LOG_MSG "[INFO]:-No FTS cluster host config provided, use default configuration."
 		else
 			FTS_HOST_MACHINE_LIST=(`$CAT $FTS_HOST_CONFIG`)
 			FTS_HOST_CONFIG_NUM=${#FTS_HOST_MACHINE_LIST[@]}
@@ -1531,11 +1531,18 @@ CREATE_STANDBY_QD () {
 		if [ $HBA_HOSTNAMES -eq 1 ]; then
 		    INIT_STANDBY_ARGS+=" --hba-hostnames"
 		fi
-		if [ x"$CLUSTER_BOOT_MODE" = x"PRODUCTION" ]; then
-		    export COORDINATOR_DATA_DIRECTORY=${COORDINATOR_DIRECTORY}/${SEG_PREFIX}-1;$INIT_STANDBY_PROG -s $STANDBY_HOSTNAME $INIT_STANDBY_ARGS -a -f $FTS_HOST_STRING_LIST
-		else
-		    export COORDINATOR_DATA_DIRECTORY=${COORDINATOR_DIRECTORY}/${SEG_PREFIX}-1;$INIT_STANDBY_PROG -s $STANDBY_HOSTNAME $INIT_STANDBY_ARGS -a
-		fi
+		# -f takes the list of external FTS hosts, and only exists when the
+		# cluster was given one. Guard it the same way as the arguments above:
+		# CLUSTER_BOOT_MODE is PRODUCTION for every cluster whose coordinator
+		# and segments are on different hosts -- which is every real one --
+		# but FTS_HOST_STRING_LIST is empty unless -f/-F was passed to
+		# gpinitsystem, and "-f" with nothing after it makes gpinitstandby
+		# exit 2 on "-f option requires 1 argument" before it writes a line of
+		# its own log.
+		if [ x"" != x"$FTS_HOST_STRING_LIST" ]; then
+		    INIT_STANDBY_ARGS+=" -f $FTS_HOST_STRING_LIST"
+		fi
+		export COORDINATOR_DATA_DIRECTORY=${COORDINATOR_DIRECTORY}/${SEG_PREFIX}-1;$INIT_STANDBY_PROG -s $STANDBY_HOSTNAME $INIT_STANDBY_ARGS -a
 		STANDBY_RET_CODE=$?
 		if [ $STANDBY_RET_CODE -eq 0 ]; then
 			LOG_MSG "[INFO]:-Successfully completed standby coordinator initialization" 1
@@ -1641,6 +1648,12 @@ FORCE_FTS_PROBE () {
         if [ x"$RESULT" == x"f" ]; then
         	break
         fi
+
+        # Ask FTS to probe now rather than waiting out gp_fts_probe_interval;
+        # without this the loop just re-reads an unchanged
+        # gp_segment_configuration sixty times in a couple of seconds and gives
+        # up before FTS has looked at the segments even once.
+        $PSQL -p $GP_PORT -d "$DEFAULTDB" -c "select gp_request_fts_probe_scan()" >> ${LOG_FILE} 2>&1
     done
     LOG_MSG "[INFO]:-End Function $FUNCNAME"
 }
@@ -1994,7 +2007,7 @@ CBDB_PRECHECK() {
                 if [ $? -eq 0 ];then
                         ERROR_EXIT "[FATAL]: FTS HOST $fts has alived fts process, please cleanup the environment first."
                 fi
-                if [ -z FTS_HOST_STRING_LIST ];then
+                if [ -z "$FTS_HOST_STRING_LIST" ];then
                          FTS_HOST_STRING_LIST="$fts"
                 else
                          FTS_HOST_STRING_LIST="$FTS_HOST_STRING_LIST,$fts"

gpMgmt/bin/lib/gp_bash_functions.sh

@@ -231,6 +231,9 @@ LOG_MSG () {
 		*FATAL*)
 			EXIT_STATUS=1
 			;;
+		*ERROR*)
+			EXIT_STATUS=1
+			;;
 		esac
 
 		if [ x"" == x"$DEBUG_LEVEL" ];then
@@ -389,9 +392,9 @@ SED_PG_CONF () {
 				fi
 			else
 				if [ $KEEP_PREV -eq 0 ];then
-					$SED -i'.bak1' -e "s/${SEARCH_TXT}/${SUB_TXT} #${SEARCH_TXT}/" $FILENAME
+					$SED -i'.bak1' -e "s/^[ ]*${SEARCH_TXT}[ ]*=/${SUB_TXT} #${SEARCH_TXT}/" $FILENAME
 				else
-					$SED -i'.bak1' -e "s/${SEARCH_TXT}.*/${SUB_TXT}/" $FILENAME
+					$SED -i'.bak1' -e "s/^[ ]*${SEARCH_TXT}[ ]*=.*/${SUB_TXT}/" $FILENAME
 				fi
 				RETVAL=$?
 				if [ $RETVAL -ne 0 ]; then
@@ -400,7 +403,7 @@ SED_PG_CONF () {
 					LOG_MSG "[INFO]:-Replaced line in $FILENAME"
 					$RM -f ${FILENAME}.bak1
 				fi
-				$SED -i'.bak2' -e "s/^#${SEARCH_TXT}/${SEARCH_TXT}/" $FILENAME
+				$SED -i'.bak2' -e "s/^[ ]*#${SEARCH_TXT}[ ]*=/${SEARCH_TXT}=/" $FILENAME
 				RETVAL=$?
 				if [ $RETVAL -ne 0 ]; then
 					ERROR_EXIT "[FATAL]:-Failed to replace #$SEARCH_TXT in $FILENAME"
@@ -431,9 +434,9 @@ SED_PG_CONF () {
 			fi
 		else
 			if [ $KEEP_PREV -eq 0 ];then
-				SED_COMMAND="s/${SEARCH_TXT}/${SUB_TXT} #${SEARCH_TXT}/"
+				SED_COMMAND="s/^[ ]*${SEARCH_TXT}[ ]*=/${SUB_TXT} #${SEARCH_TXT}/"
 			else
-				SED_COMMAND="s/${SEARCH_TXT}.*/${SUB_TXT}/"
+				SED_COMMAND="s/^[ ]*${SEARCH_TXT}[ ]*=.*/${SUB_TXT}/"
 			fi
 			$TRUSTED_SHELL $SED_HOST sed -i'.bak1' -f /dev/stdin "$FILENAME" <<< "$SED_COMMAND" > /dev/null 2>&1
 			if [ $RETVAL -ne 0 ]; then
@@ -443,7 +446,7 @@ SED_PG_CONF () {
 				$TRUSTED_SHELL $SED_HOST "$RM -f ${FILENAME}.bak1" > /dev/null 2>&1
 			fi
 
-			SED_COMMAND="s/^#${SEARCH_TXT}/${SEARCH_TXT}/"
+			SED_COMMAND="s/^[ ]*#${SEARCH_TXT}[ ]*=/${SEARCH_TXT}=/"
 			$TRUSTED_SHELL $SED_HOST sed -i'.bak2' -f /dev/stdin "$FILENAME" <<< "$SED_COMMAND" > /dev/null 2>&1
 			if [ $RETVAL -ne 0 ]; then
 				ERROR_EXIT "[FATAL]:-Failed to substitute #${SEARCH_TXT} in $FILENAME on $SED_HOST"
@@ -783,6 +786,11 @@ BUILD_COORDINATOR_PG_HBA_FILE () {
         else
             $ECHO "host     all         $USER_NAME         localhost    trust" >> ${GP_DIR}/$PG_HBA
             $ECHO "host     all         $USER_NAME         $COORDINATOR_HOSTNAME       trust" >> ${GP_DIR}/$PG_HBA
+            # Also add explicit IPv4 and IPv6 loopback entries so that
+            # connections via 127.0.0.1 and ::1 are always accepted
+            # regardless of hostname resolution order.
+            $ECHO "host     all         $USER_NAME         127.0.0.1/32    trust" >> ${GP_DIR}/$PG_HBA
+            $ECHO "host     all         $USER_NAME         ::1/128         trust" >> ${GP_DIR}/$PG_HBA
         fi
 
 
@@ -1231,7 +1239,7 @@ PRECHECK_CBDB_CONFIG_FILE() {
 
 CLEANUP_ETCD() {
     if [ "$#" -ne 1 ];then
-        LOG_MSG "[ERROR]: RETRIVE_ETCD_CONFIG_VAL invalid params..."
+        LOG_MSG "[ERROR]: CLEANUP_ETCD invalid params..."
     fi
     local etcd_account_id=`RETRIVE_ETCD_CONFIG_VAL $ETCD_ACCOUNT_ID`
     local etcd_cluster_id=`RETRIVE_ETCD_CONFIG_VAL $ETCD_CLUSTER_ID`
@@ -1315,7 +1323,7 @@ SETUP_FTS() {
 
 CHECK_FTS () {
     ret=1
-    if [ "$#" -ne 2 ];then
+    if [ "$#" -ne 1 ];then
         LOG_MSG "[ERROR]: CHECK_FTS invalid params..."
     fi
     FTS_HOST=$1

gpMgmt/bin/lib/gpcreateseg.sh

@@ -224,6 +224,20 @@ CREATE_QES_MIRROR () {
     fi
     RUN_COMMAND_REMOTE ${PRIMARY_HOSTADDRESS} "${EXPORT_GPHOME}; . ${GPHOME}/cloudberry-env.sh; cat - >> ${PRIMARY_DIR}/pg_hba.conf; pg_ctl -D ${PRIMARY_DIR} reload" <<< "${PG_HBA_ENTRIES}"
     RUN_COMMAND_REMOTE ${GP_HOSTADDRESS} "${EXPORT_GPHOME}; . ${GPHOME}/cloudberry-env.sh; rm -rf ${GP_DIR}; ${GPHOME}/bin/pg_basebackup --wal-method=stream --create-slot --slot='internal_wal_replication_slot' -R -c fast -E ./db_dumps -D ${GP_DIR} -h ${PRIMARY_HOSTADDRESS} -p ${PRIMARY_PORT} --target-gp-dbid ${GP_DBID};"
+    # pg_basebackup copies the primary's postgresql.conf verbatim, so the
+    # mirror would otherwise start on the primary's port. Append the right
+    # one before starting; the last setting in the file wins.
+    #
+    # $PORT_TXT ("#port") finds nothing in the copied file -- the port line is
+    # already active there -- so SED_PG_CONF takes its append path. That is
+    # what we want: its replace path runs an unanchored
+    # "s/<search>/<sub> #<search>/", which with a search string of "port"
+    # would also rewrite every "support", "report" and "transport" in the
+    # file's comments.
+    LOG_MSG "[INFO][$INST_COUNT]:-Configuring segment $PG_CONF"
+    SED_PG_CONF ${GP_DIR}/$PG_CONF "$PORT_TXT" port=$GP_PORT 0 $GP_HOSTADDRESS
+    RETVAL=$?
+    PARA_EXIT $RETVAL "Update port number to $GP_PORT"
     START_QE "-w"
     RETVAL=$?
     PARA_EXIT $RETVAL "pg_basebackup of segment data directory from ${PRIMARY_HOSTADDRESS} to ${GP_HOSTADDRESS}"

gpMgmt/test/behave/mgmt_utils/gp_bash_functions.feature

@@ -1,5 +1,16 @@
 @gp_bash_functions.sh
 Feature: gp_bash_funtions.sh unit test cases
+    @sed_pg_conf
+    Scenario Outline: SED_PG_CONF changes only the requested parameter
+        Then SED_PG_CONF preserves unrelated settings on "<host>" with KEEP_PREV "<keep_prev>"
+
+        Examples:
+            | host      | keep_prev |
+            | local     | 0         |
+            | local     | 1         |
+            | localhost | 0         |
+            | localhost | 1         |
+
     Scenario: REMOTE_EXECUTE_AND_GET_OUTPUT returns proper output when no banner is set
         Given a standard local demo cluster is running
         Then source gp_bash_functions and run simple echo

gpMgmt/test/behave/mgmt_utils/gpinitsystem.feature

@@ -63,6 +63,14 @@ Feature: gpinitsystem tests
         Given the user runs "gpstate"
          Then gpstate should return a return code of 2
 
+    Scenario: gpinitsystem exits with status 1 when the user provides invalid option
+        Given create demo cluster config
+         When the user runs command "gpinitsystem -a -c ../gpAux/gpdemo/clusterConfigFile --ignore-warnings" eok
+         Then gpinitsystem should return a return code of 1
+          And gpinitsystem should print "Unknown option --ignore-warnings" to stdout
+        Given the user runs "gpstate"
+         Then gpstate should return a return code of 2
+
     Scenario: gpinitsystem creates a cluster when the user confirms the dialog
         Given create demo cluster config
          When the user runs command "echo y | gpinitsystem -c ../gpAux/gpdemo/clusterConfigFile"
@@ -143,8 +151,6 @@ Feature: gpinitsystem tests
        When the user runs "gpinitsystem -a -c ../gpAux/gpdemo/clusterConfigFile"
        Then gpinitsystem should return a return code of 0
 
-    # needs_fix: gpinitsystem fixes (#1639 b971d481862)
-    @needs_fix
     Scenario: gpinitsystem should warn but not fail when standby cannot be instantiated
         Given the database is running
         And all the segments are running
@@ -187,8 +193,6 @@ Feature: gpinitsystem tests
         And gpconfig should print "Coordinator value: off" to stdout
         And gpconfig should print "Segment     value: off" to stdout
 
-    # needs_fix: gpinitsystem fixes (#1639 b971d481862)
-    @needs_fix
     Scenario: gpinitsystem should warn but not fail when standby cannot be instantiated
         Given the database is running
         And all the segments are running
@@ -224,8 +228,6 @@ Feature: gpinitsystem tests
         And the startup timezone matches "HST"
         And "TZ" environment variable should be restored
 
-    # needs_fix: gpinitsystem fixes (#1639 b971d481862)
-    @needs_fix
     Scenario: gpinitsystem should print FQDN in pg_hba.conf when HBA_HOSTNAMES=1
         Given the cluster config is generated with HBA_HOSTNAMES "1"
         When generate cluster config file "/tmp/output_config_file"
@@ -234,8 +236,6 @@ Feature: gpinitsystem tests
         Then verify that the file "../gpAux/gpdemo/datadirs/qddir/demoDataDir-1/pg_hba.conf" contains FQDN only for trusted host
         And verify that the file "../gpAux/gpdemo/datadirs/dbfast1/demoDataDir0/pg_hba.conf" contains FQDN only for trusted host
 
-    # needs_fix: gpinitsystem fixes (#1639 b971d481862)
-    @needs_fix
     Scenario: gpinitsystem should print CIDR in pg_hba.conf when HBA_HOSTNAMES=0
         Given the cluster config is generated with HBA_HOSTNAMES "0"
         When generate cluster config file "/tmp/output_config_file"
@@ -244,8 +244,6 @@ Feature: gpinitsystem tests
         Then verify that the file "../gpAux/gpdemo/datadirs/qddir/demoDataDir-1/pg_hba.conf" contains CIDR only for trusted host
         And verify that the file "../gpAux/gpdemo/datadirs/dbfast1/demoDataDir0/pg_hba.conf" contains CIDR only for trusted host
 
-    # needs_fix: gpinitsystem fixes (#1639 b971d481862)
-    @needs_fix
     Scenario: gpinitsystem should print FQDN in pg_hba.conf for standby when HBA_HOSTNAMES=1
         Given the database is running
         And all the segments are running
@@ -260,8 +258,6 @@ Feature: gpinitsystem tests
         And verify that the file "../gpAux/gpdemo/datadirs/dbfast1/demoDataDir0/pg_hba.conf" contains FQDN only for trusted host
         And verify that the file "../gpAux/gpdemo/datadirs/qddir/demoDataDir-1/newstandby/pg_hba.conf" contains FQDN only for trusted host
 
-    # needs_fix: gpinitsystem fixes (#1639 b971d481862)
-    @needs_fix
     Scenario: gpinitsystem on a DCA system is able to set the DCA specific GUCs
 	Given create demo cluster config
         And the user runs command "rm -r ~/gpAdminLogs/gpinitsystem*"
@@ -319,8 +315,6 @@ Feature: gpinitsystem tests
         And a demo cluster is created using gpinitsystem args " "
         And gpinitsystem should return a return code of 0
 
-    # needs_fix: mirror port fix in gpinitsystem fixes (#1639 b971d481862)
-    @needs_fix
     Scenario: gpinitsystem should create consistent port entry on segments postgresql.conf file
         Given the database is not running
         When a demo cluster is created using gpinitsystem args " "
@@ -337,7 +331,7 @@ Feature: gpinitsystem tests
 
     Scenario: gpinitsystem should pass the default value of trusted_shell properly to gpcreateseg
         Given create demo cluster config
-        When the user runs command "gpinitsystem -a -c ../gpAux/gpdemo/clusterConfigFile -h ../gpAux/gpdemo/hostfile --ignore-warnings"
+        When the user runs command "gpinitsystem -a -c ../gpAux/gpdemo/clusterConfigFile -h ../gpAux/gpdemo/hostfile"
         Then gpinitsystem should return a return code of 0
         When the user runs command "grep -q '.*gpcreateseg\.sh.*Completed .*lalshell.*' ~/gpAdminLogs/gpinitsystem*log"
         Then grep should return a return code of 0

gpMgmt/test/behave/mgmt_utils/steps/gp_bash_functions.py

@@ -0,0 +1,48 @@
+import os
+from pathlib import Path
+import subprocess
+import tempfile
+
+from behave import then
+
+
+@then('SED_PG_CONF preserves unrelated settings on "{host}" with KEEP_PREV "{keep_prev}"')
+def check_sed_pg_conf_parameter_boundary(context, host, keep_prev):
+    # Use an active work_mem setting so the replacement branch is exercised;
+    # a file containing only the default #work_mem would take the append path.
+    unrelated_lines = [
+        "maintenance_work_mem = 65536",
+        "# work_mem should not change inside a comment",
+    ]
+    original = "  work_mem = 4096\n" + "\n".join(unrelated_lines) + "\n"
+    functions = Path(os.environ["GPHOME"]) / "bin/lib/gp_bash_functions.sh"
+
+    # localhost uses the real SSH branch against the same temporary file.
+    # No database is required; SSH must be configured as for the other tests
+    # in gp_bash_functions.feature.
+    with tempfile.TemporaryDirectory(prefix="sed_pg_conf_") as temp_dir:
+        config = Path(temp_dir) / "postgresql.conf"
+        config.write_text(original)
+        script = '''
+source "$1" || exit $?
+LOG_FILE="$2"
+SED_PG_CONF "$3" work_mem work_mem=999 "$4" "$5"
+'''
+        result = subprocess.run(
+            ["bash", "-c", script, "sed_pg_conf_test", str(functions),
+             str(Path(temp_dir) / "test.log"), str(config), keep_prev,
+             "" if host == "local" else host],
+            stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+            universal_newlines=True, timeout=60,
+        )
+        assert result.returncode == 0, (
+            "SED_PG_CONF failed: stdout={!r}, stderr={!r}".format(
+                result.stdout, result.stderr)
+        )
+        lines = config.read_text().splitlines()
+        assert lines[1:] == unrelated_lines, (
+            "SED_PG_CONF modified unrelated settings or comments: {!r}".format(lines)
+        )
+        assert lines[0].split("#", 1)[0].strip() == "work_mem=999", (
+            "SED_PG_CONF did not update work_mem: {!r}".format(lines)
+        )